From 922e150020747f2f387e799c52fa43cd8e3c5f5f Mon Sep 17 00:00:00 2001 From: Matchu Date: Thu, 4 Feb 2021 23:34:43 -0800 Subject: [PATCH] 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! --- cypress/integration/ItemSearchPage.spec.js | 17 ++++ .../WardrobePage/SearchPanel.spec.js | 36 ++++++++ src/app/WardrobePage/ItemsAndSearchPanels.js | 1 + src/app/WardrobePage/SearchPanel.js | 23 ++--- src/app/WardrobePage/SearchToolbar.js | 1 + src/server/loaders.js | 87 ++----------------- src/server/types/Item.js | 45 +++++++++- 7 files changed, 114 insertions(+), 96 deletions(-) create mode 100644 cypress/integration/ItemSearchPage.spec.js create mode 100644 cypress/integration/WardrobePage/SearchPanel.spec.js diff --git a/cypress/integration/ItemSearchPage.spec.js b/cypress/integration/ItemSearchPage.spec.js new file mode 100644 index 0000000..4dba92a --- /dev/null +++ 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" + ); + }); +}); diff --git a/cypress/integration/WardrobePage/SearchPanel.spec.js b/cypress/integration/WardrobePage/SearchPanel.spec.js new file mode 100644 index 0000000..ab8eb27 --- /dev/null +++ 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"); + }); +}); diff --git a/src/app/WardrobePage/ItemsAndSearchPanels.js b/src/app/WardrobePage/ItemsAndSearchPanels.js index 8141a99..b60e17b 100644 --- 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" > { // The Autosuggest tries to change the _entire_ value of the element // when navigating suggestions, which isn't actually what we want. diff --git a/src/server/loaders.js b/src/server/loaders.js index b6a6e46..ee65a27 100644 --- 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( diff --git a/src/server/types/Item.js b/src/server/types/Item.js index f9b7b35..79172c1 100644 --- 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,