Extend itemSearch, deprecate itemSearchToFit

I'm gonna extend `itemSearch` to also look up the total number of results, and the fragmentation between `itemSearch` and `itemSearchToFit` finally caught up with me :p I've deprecated `itemSearchToFit`, and moved the fit parameters into a new optional `fitsPet` parameter for `itemSearch`. I'm going to keep `itemSearchToFit` around for now, because old JS builds still use it, and I'd like to avoid disrupting folks. But I'm not going to add the new total results field to the results object it returns, and that's gonna be okay!
2021-02-04 23:34:43 -08:00 · 2021-02-04 23:34:43 -08:00 · 922e150020
commit 922e150020
parent b9aac7d8d2
7 changed files with 114 additions and 96 deletions
--- a/cypress/integration/ItemSearchPage.spec.js
+++ b/cypress/integration/ItemSearchPage.spec.js
@ -0,0 +1,17 @@
+// Our local dev server is slow, give it plenty of breathing room!
+// (For me, it can often take 10-15 seconds when working correctly.)
+const networkTimeout = { timeout: 20000 };
+
+describe("ItemSearchPage", () => {
+  // NOTE: This test depends on specific search results on certain pages, and
+  //       could break if a lot of matching items are added to the site!
+  it("Searches by keyword", () => {
+    cy.visit("/items/search");
+
+    // The first page should contain this item.
+    cy.get("[data-test-id=item-search-input]").type("winter");
+    cy.contains("A Warm Winters Night Background", networkTimeout).should(
+      "exist"
+    );
+  });
+});
--- a/cypress/integration/WardrobePage/SearchPanel.spec.js
+++ b/cypress/integration/WardrobePage/SearchPanel.spec.js
@ -0,0 +1,36 @@
+// Our local dev server is slow, give it plenty of breathing room!
+// (For me, it can often take 10-15 seconds when working correctly.)
+const networkTimeout = { timeout: 20000 };
+
+describe("WardrobePage: SearchPanel", () => {
+  // NOTE: This test depends on specific search results on certain pages, and
+  //       could break if a lot of matching items are added to the site!
+  it("Searches by keyword", () => {
+    cy.visit("/outfits/new");
+
+    // The first page should contain this item.
+    cy.get("[data-test-id=item-search-input]").type("winter");
+    cy.contains("A Warm Winters Night Background", networkTimeout).should(
+      "exist"
+    );
+
+    // And the second page should contain this item.
+    cy.get("[data-test-id=search-panel-scroll-container]").scrollTo("bottom");
+    cy.contains(
+      "Dyeworks Green: Winter Poinsettia Staff",
+      networkTimeout
+    ).should("exist");
+  });
+
+  it("Only shows items that fit", () => {
+    cy.visit("/outfits/new");
+
+    // Searching for Christmas paintbrush items should show the Acara items,
+    // but not the Aisha items.
+    cy.get("[data-test-id=item-search-input]")
+      .type("pb{enter}")
+      .type("christmas");
+    cy.contains("Christmas Acara Coat", networkTimeout).should("exist");
+    cy.contains("Christmas Aisha Collar").should("not.exist");
+  });
+});
--- a/src/app/WardrobePage/ItemsAndSearchPanels.js
+++ b/src/app/WardrobePage/ItemsAndSearchPanels.js
@ -44,6 +44,7 @@ function ItemsAndSearchPanels({ loading, outfitState, dispatchToOutfit }) {
          position="relative"
          overflow="auto"
          ref={scrollContainerRef}
+          data-test-id="search-panel-scroll-container"
        >
          <Box px="4" py="2">
            <SearchPanel
--- a/src/app/WardrobePage/SearchPanel.js
+++ b/src/app/WardrobePage/SearchPanel.js
@ -252,6 +252,7 @@ function useSearchResults(query, outfitState) {
    gql`
      query SearchPanel(
        $query: String!
+        $fitsPet: FitsPetSearchFilter
        $itemKind: ItemKindSearchFilter
        $currentUserOwnsOrWants: OwnsOrWants
        $zoneIds: [ID!]!
@ -259,13 +260,12 @@ function useSearchResults(query, outfitState) {
        $colorId: ID!
        $offset: Int!
      ) {
-        itemSearchToFit(
+        itemSearch(
          query: $query
+          fitsPet: $fitsPet
          itemKind: $itemKind
          currentUserOwnsOrWants: $currentUserOwnsOrWants
          zoneIds: $zoneIds
-          speciesId: $speciesId
-          colorId: $colorId
          offset: $offset
          limit: 50
        ) {
@ -309,12 +309,13 @@ function useSearchResults(query, outfitState) {
    {
      variables: {
        query: debouncedQuery.value,
+        fitsPet: { speciesId, colorId },
        itemKind: debouncedQuery.filterToItemKind,
        currentUserOwnsOrWants: debouncedQuery.filterToCurrentUserOwnsOrWants,
        zoneIds: filterToZoneIds,
+        offset: 0,
        speciesId,
        colorId,
-        offset: 0,
      },
      context: { sendAuth: true },
      skip:
@ -328,7 +329,7 @@ function useSearchResults(query, outfitState) {
        // `fetchMore`, with the extended results. But, on the first time, this
        // logic can tell us whether we're at the end of the list, by counting
        // whether there was <30. We also have to check in `fetchMore`!
-        const items = d && d.itemSearchToFit && d.itemSearchToFit.items;
+        const items = d && d.itemSearch && d.itemSearch.items;
        if (items && items.length < 30) {
          setIsEndOfResults(true);
        }
@ -340,7 +341,7 @@ function useSearchResults(query, outfitState) {
  );

  // Smooth over the data a bit, so that we can use key fields with confidence!
-  const result = data?.itemSearchToFit;
+  const result = data?.itemSearch;
  const resultValue = result?.query;
  const zoneStr = filterToZoneIds.sort().join(",");
  const resultZoneStr = (result?.zones || [])
@ -387,17 +388,17 @@ function useSearchResults(query, outfitState) {
          // we'd need to return the total result count... a bit annoying to
          // potentially double the query runtime? We'd need to see how slow it
          // actually makes things.
-          if (fetchMoreResult.itemSearchToFit.items.length < 30) {
+          if (fetchMoreResult.itemSearch.items.length < 30) {
            setIsEndOfResults(true);
          }

          return {
            ...prev,
-            itemSearchToFit: {
-              ...(prev?.itemSearchToFit || {}),
+            itemSearch: {
+              ...(prev?.itemSearch || {}),
              items: [
-                ...(prev?.itemSearchToFit?.items || []),
-                ...(fetchMoreResult?.itemSearchToFit?.items || []),
+                ...(prev?.itemSearch?.items || []),
+                ...(fetchMoreResult?.itemSearch?.items || []),
              ],
            },
          };
--- a/src/app/WardrobePage/SearchToolbar.js
+++ b/src/app/WardrobePage/SearchToolbar.js
@ -275,6 +275,7 @@ function SearchToolbar({
        value: query.value || "",
        ref: searchQueryRef,
        minWidth: 0,
+        "data-test-id": "item-search-input",
        onChange: (e, { newValue, method }) => {
          // The Autosuggest tries to change the _entire_ value of the element
          // when navigating suggestions, which isn't actually what we want.
--- a/src/server/loaders.js
+++ b/src/server/loaders.js
@ -288,84 +288,6 @@ const buildItemSearchLoader = (db, loaders) =>
    // This isn't actually optimized as a batch query, we're just using a
    // DataLoader API consistency with our other loaders!
    const queryPromises = queries.map(
-      async ({
-        query,
-        itemKind,
-        currentUserOwnsOrWants,
-        currentUserId,
-        zoneIds = [],
-        offset,
-        limit,
-      }) => {
-        const actualOffset = offset || 0;
-        const actualLimit = Math.min(limit || 30, 30);
-
-        // Split the query into words, and search for each word as a substring
-        // of the name.
-        const words = query.split(/\s+/);
-        const wordMatchersForMysql = words.map(
-          (word) => "%" + word.replace(/_%/g, "\\$0") + "%"
-        );
-        const matcherPlaceholders = words
-          .map((_) => "t.name LIKE ?")
-          .join(" AND ");
-
-        const itemKindCondition = itemSearchKindConditions[itemKind] || "1";
-        const zoneIdsPlaceholder =
-          zoneIds.length > 0
-            ? `swf_assets.zone_id IN (${zoneIds.map((_) => "?").join(", ")})`
-            : "1";
-        const currentUserJoin = currentUserOwnsOrWants
-          ? `INNER JOIN closet_hangers ch ON ch.item_id = items.id`
-          : "";
-        const currentUserCondition = currentUserOwnsOrWants
-          ? `ch.user_id = ? AND ch.owned = ?`
-          : "1";
-        const currentUserValues = currentUserOwnsOrWants
-          ? [currentUserId, currentUserOwnsOrWants === "OWNS" ? "1" : "0"]
-          : [];
-
-        const [rows, _] = await db.execute(
-          `SELECT DISTINCT items.*, t.name FROM items
-           INNER JOIN item_translations t ON t.item_id = items.id
-           INNER JOIN parents_swf_assets rel
-               ON rel.parent_type = "Item" AND rel.parent_id = items.id
-           INNER JOIN swf_assets ON rel.swf_asset_id = swf_assets.id
-           ${currentUserJoin}
-           WHERE ${matcherPlaceholders} AND t.locale = "en" AND
-               ${zoneIdsPlaceholder} AND ${itemKindCondition} AND
-               ${currentUserCondition}
-           ORDER BY t.name
-           LIMIT ? OFFSET ?`,
-          [
-            ...wordMatchersForMysql,
-            ...zoneIds,
-            ...currentUserValues,
-            actualLimit,
-            actualOffset,
-          ]
-        );
-
-        const entities = rows.map(normalizeRow);
-
-        for (const item of entities) {
-          loaders.itemLoader.prime(item.id, item);
-        }
-
-        return entities;
-      }
-    );
-
-    const responses = await Promise.all(queryPromises);
-
-    return responses;
-  });
-
-const buildItemSearchToFitLoader = (db, loaders) =>
-  new DataLoader(async (queryAndBodyIdPairs) => {
-    // This isn't actually optimized as a batch query, we're just using a
-    // DataLoader API consistency with our other loaders!
-    const queryPromises = queryAndBodyIdPairs.map(
      async ({
        query,
        bodyId,
@ -390,6 +312,10 @@ const buildItemSearchToFitLoader = (db, loaders) =>
          .join(" AND ");

        const itemKindCondition = itemSearchKindConditions[itemKind] || "1";
+        const bodyIdCondition = bodyId
+          ? "(swf_assets.body_id = ? OR swf_assets.body_id = 0)"
+          : "1";
+        const bodyIdValues = bodyId ? [bodyId] : [];
        const zoneIdsCondition =
          zoneIds.length > 0
            ? `swf_assets.zone_id IN (${zoneIds.map((_) => "?").join(", ")})`
@ -412,14 +338,14 @@ const buildItemSearchToFitLoader = (db, loaders) =>
           INNER JOIN swf_assets ON rel.swf_asset_id = swf_assets.id
           ${currentUserJoin}
           WHERE ${matcherPlaceholders} AND t.locale = "en" AND
-               (swf_assets.body_id = ? OR swf_assets.body_id = 0) AND
+               ${bodyIdCondition} AND
               ${zoneIdsCondition} AND ${itemKindCondition} AND
               ${currentUserCondition}
           ORDER BY t.name
           LIMIT ? OFFSET ?`,
          [
            ...wordMatchersForMysql,
-            bodyId,
+            ...bodyIdValues,
            ...zoneIds,
            ...currentUserValues,
            actualLimit,
@ -1292,7 +1218,6 @@ function buildLoaders(db) {
  loaders.itemTranslationLoader = buildItemTranslationLoader(db);
  loaders.itemByNameLoader = buildItemByNameLoader(db, loaders);
  loaders.itemSearchLoader = buildItemSearchLoader(db, loaders);
-  loaders.itemSearchToFitLoader = buildItemSearchToFitLoader(db, loaders);
  loaders.newestItemsLoader = buildNewestItemsLoader(db, loaders);
  loaders.itemsThatNeedModelsLoader = buildItemsThatNeedModelsLoader(db);
  loaders.itemBodiesWithAppearanceDataLoader = buildItemBodiesWithAppearanceDataLoader(
--- a/src/server/types/Item.js
+++ b/src/server/types/Item.js
@ -97,6 +97,11 @@ const typeDefs = gql`
    restrictedZones: [Zone!]!
  }

+  input FitsPetSearchFilter {
+    speciesId: ID!
+    colorId: ID!
+  }
+
  enum ItemKindSearchFilter {
    NC
    NP
@ -133,12 +138,16 @@ const typeDefs = gql`
    # Search for items with fuzzy matching.
    itemSearch(
      query: String!
+      fitsPet: FitsPetSearchFilter
      itemKind: ItemKindSearchFilter
      currentUserOwnsOrWants: OwnsOrWants
      zoneIds: [ID!]
      offset: Int
      limit: Int
    ): ItemSearchResult!
+
+    # Deprecated: an alias for itemSearch, but with speciesId and colorId
+    # required, serving the same purpose as fitsPet in itemSearch.
    itemSearchToFit(
      query: String!
      itemKind: ItemKindSearchFilter
@ -438,11 +447,34 @@ const resolvers = {
    },
    itemSearch: async (
      _,
-      { query, itemKind, currentUserOwnsOrWants, zoneIds = [], offset, limit },
-      { itemSearchLoader, currentUserId }
+      {
+        query,
+        fitsPet,
+        itemKind,
+        currentUserOwnsOrWants,
+        zoneIds = [],
+        offset,
+        limit,
+      },
+      { itemSearchLoader, petTypeBySpeciesAndColorLoader, currentUserId }
    ) => {
+      let bodyId = null;
+      if (fitsPet) {
+        const petType = await petTypeBySpeciesAndColorLoader.load({
+          speciesId: fitsPet.speciesId,
+          colorId: fitsPet.colorId,
+        });
+        if (!petType) {
+          throw new Error(
+            `pet type not found: speciesId=${fitsPet.speciesId}, ` +
+              `colorId: ${fitsPet.colorId}`
+          );
+        }
+        bodyId = petType.bodyId;
+      }
      const items = await itemSearchLoader.load({
        query: query.trim(),
+        bodyId,
        itemKind,
        currentUserOwnsOrWants,
        currentUserId,
@ -465,14 +497,19 @@ const resolvers = {
        offset,
        limit,
      },
-      { petTypeBySpeciesAndColorLoader, itemSearchToFitLoader, currentUserId }
+      { petTypeBySpeciesAndColorLoader, itemSearchLoader, currentUserId }
    ) => {
      const petType = await petTypeBySpeciesAndColorLoader.load({
        speciesId,
        colorId,
      });
+      if (!petType) {
+        throw new Error(
+          `pet type not found: speciesId=${speciesId}, colorId: ${colorId}`
+        );
+      }
      const { bodyId } = petType;
-      const items = await itemSearchToFitLoader.load({
+      const items = await itemSearchLoader.load({
        query: query.trim(),
        itemKind,
        currentUserOwnsOrWants,