fix: includes variant prices when listing products using a search query (#1607)

* fix: return prices when lsiting products with a query

* add: integration test

* fix: integration test

* fix: move WithRequiredProperty to common.ts

* fix: comment

integration-tests/api/__tests__/admin/product.js

@@ -6,7 +6,11 @@ const { initDb, useDb } = require("../../../helpers/use-db")
 
 const adminSeeder = require("../../helpers/admin-seeder")
 const productSeeder = require("../../helpers/product-seeder")
-const { ProductVariant, ProductOptionValue, MoneyAmount } = require("@medusajs/medusa")
+const {
+  ProductVariant,
+  ProductOptionValue,
+  MoneyAmount,
+} = require("@medusajs/medusa")
 const priceListSeeder = require("../../helpers/price-list-seeder")
 
 jest.setTimeout(50000)
@@ -232,6 +236,7 @@ describe("/admin/products", () => {
         })
 
       expect(response.status).toEqual(200)
+      expect(response.data.count).toEqual(1)
       expect(response.data.products).toEqual([
         expect.objectContaining({
           id: "test-product_filtering_4",
@@ -256,6 +261,36 @@ describe("/admin/products", () => {
       expect(response.data.products.length).toEqual(2)
     })
 
+    it("returns a list of products with free text query including variant prices", async () => {
+      const api = useApi()
+
+      const response = await api
+        .get("/admin/products?q=test+product1", {
+          headers: {
+            Authorization: "Bearer test_token",
+          },
+        })
+        .catch((err) => {
+          console.log(err)
+        })
+
+      const expectedVariantPrices = response.data.products[0].variants
+        .map((v) => v.prices)
+        .flat(1)
+
+      expect(response.status).toEqual(200)
+      expect(expectedVariantPrices).toEqual(
+        expect.arrayContaining([
+          expect.objectContaining({
+            id: "test-price4",
+          }),
+          expect.objectContaining({
+            id: "test-price3",
+          }),
+        ])
+      )
+    })
+
     it("returns a list of products with free text query and offset", async () => {
       const api = useApi()
 
@@ -1403,7 +1438,7 @@ describe("/admin/products", () => {
       await db.teardown()
     })
 
-    it('should return the variants related to the requested product', async () => {
+    it("should return the variants related to the requested product", async () => {
       const api = useApi()
 
       const res = await api
@@ -1420,10 +1455,22 @@ describe("/admin/products", () => {
       expect(res.data.variants.length).toBe(4)
       expect(res.data.variants).toEqual(
         expect.arrayContaining([
-          expect.objectContaining({ id: "test-variant", product_id: "test-product" }),
+          expect.objectContaining({
-          expect.objectContaining({ id: "test-variant_1", product_id: "test-product" }),
+            id: "test-variant",
-          expect.objectContaining({ id: "test-variant_2", product_id: "test-product" }),
+            product_id: "test-product",
-          expect.objectContaining({ id: "test-variant-sale", product_id: "test-product" }),
+          }),
+          expect.objectContaining({
+            id: "test-variant_1",
+            product_id: "test-product",
+          }),
+          expect.objectContaining({
+            id: "test-variant_2",
+            product_id: "test-product",
+          }),
+          expect.objectContaining({
+            id: "test-variant-sale",
+            product_id: "test-product",
+          }),
         ])
       )
     })
@@ -1812,16 +1859,22 @@ describe("/admin/products", () => {
       expect(optValPost).toEqual(undefined)
 
       // Validate that the option still exists in the DB with deleted_at
-      const optValDeleted = await dbConnection.manager.findOne(ProductOptionValue, {
+      const optValDeleted = await dbConnection.manager.findOne(
+        ProductOptionValue,
+        {
           variant_id: "test-variant_2",
-      }, {
+        },
+        {
           withDeleted: true,
-      })
+        }
+      )
 
-      expect(optValDeleted).toEqual(expect.objectContaining({
+      expect(optValDeleted).toEqual(
+        expect.objectContaining({
           deleted_at: expect.any(Date),
           variant_id: "test-variant_2",
-      }))
+        })
+      )
     })
 
     it("successfully deletes a product and any option value associated with one of its variants", async () => {
@@ -1854,16 +1907,22 @@ describe("/admin/products", () => {
       expect(optValPost).toEqual(undefined)
 
       // Validate that the option still exists in the DB with deleted_at
-      const optValDeleted = await dbConnection.manager.findOne(ProductOptionValue, {
+      const optValDeleted = await dbConnection.manager.findOne(
+        ProductOptionValue,
+        {
           variant_id: "test-variant_2",
-      }, {
+        },
+        {
           withDeleted: true,
-      })
+        }
+      )
 
-      expect(optValDeleted).toEqual(expect.objectContaining({
+      expect(optValDeleted).toEqual(
+        expect.objectContaining({
           deleted_at: expect.any(Date),
           variant_id: "test-variant_2",
-      }))
+        })
+      )
     })
 
     it("successfully deletes a product variant and its associated prices", async () => {
@@ -1889,26 +1948,29 @@ describe("/admin/products", () => {
       expect(response.status).toEqual(200)
 
       // Validate that the price was deleted
-      const pricePost = await dbConnection.manager.findOne(
+      const pricePost = await dbConnection.manager.findOne(MoneyAmount, {
-        MoneyAmount,
-        {
         id: "test-price",
-        }
+      })
-      )
 
       expect(pricePost).toEqual(undefined)
 
       // Validate that the price still exists in the DB with deleted_at
-      const optValDeleted = await dbConnection.manager.findOne(MoneyAmount, {
+      const optValDeleted = await dbConnection.manager.findOne(
+        MoneyAmount,
+        {
           id: "test-price",
-      }, {
+        },
+        {
           withDeleted: true,
-      })
+        }
+      )
 
-      expect(optValDeleted).toEqual(expect.objectContaining({
+      expect(optValDeleted).toEqual(
+        expect.objectContaining({
           deleted_at: expect.any(Date),
           id: "test-price",
-      }))
+        })
+      )
     })
 
     it("successfully deletes a product and any prices associated with one of its variants", async () => {
@@ -1938,16 +2000,22 @@ describe("/admin/products", () => {
       expect(pricePost).toEqual(undefined)
 
       // Validate that the price still exists in the DB with deleted_at
-      const optValDeleted = await dbConnection.manager.findOne(MoneyAmount, {
+      const optValDeleted = await dbConnection.manager.findOne(
+        MoneyAmount,
+        {
           id: "test-price",
-      }, {
+        },
+        {
           withDeleted: true,
-      })
+        }
+      )
 
-      expect(optValDeleted).toEqual(expect.objectContaining({
+      expect(optValDeleted).toEqual(
+        expect.objectContaining({
           deleted_at: expect.any(Date),
           id: "test-price",
-      }))
+        })
+      )
     })
 
     it("successfully creates product with soft-deleted product handle and deletes it again", async () => {

packages/medusa/src/repositories/product.ts

@@ -1,5 +1,6 @@
 import { flatten, groupBy, map, merge } from "lodash"
 import {
+  Brackets,
   EntityRepository,
   FindManyOptions,
   FindOperator,
@@ -8,8 +9,9 @@ import {
   Repository,
 } from "typeorm"
 import { ProductTag } from ".."
-import { Product } from "../models/product"
 import { PriceList } from "../models/price-list"
+import { Product } from "../models/product"
+import { WithRequiredProperty } from "../types/common"
 
 type DefaultWithoutRelations = Omit<FindManyOptions<Product>, "relations">
 
@@ -103,7 +105,9 @@ export class ProductRepository extends Repository<Product> {
     return [entities, count]
   }
 
-  private getGroupedRelations(relations: Array<keyof Product>): {
+  private getGroupedRelations(
+    relations: Array<keyof Product>
+  ): {
     [toplevel: string]: string[]
   } {
     const groupedRelations: { [toplevel: string]: string[] } = {}
@@ -227,15 +231,16 @@ export class ProductRepository extends Repository<Product> {
     )
 
     const entitiesAndRelations = entitiesIdsWithRelations.concat(entities)
-    const entitiesToReturn =
+    const entitiesToReturn = this.mergeEntitiesWithRelations(
-      this.mergeEntitiesWithRelations(entitiesAndRelations)
+      entitiesAndRelations
+    )
 
     return [entitiesToReturn, count]
   }
 
   public async findWithRelations(
     relations: Array<keyof Product> = [],
-    idsOrOptionsWithoutRelations: FindWithRelationsOptions = {},
+    idsOrOptionsWithoutRelations: FindWithRelationsOptions | string[] = {},
     withDeleted = false
   ): Promise<Product[]> {
     let entities: Product[]
@@ -257,7 +262,10 @@ export class ProductRepository extends Repository<Product> {
       return []
     }
 
-    if (relations.length === 0) {
+    if (
+      relations.length === 0 &&
+      !Array.isArray(idsOrOptionsWithoutRelations)
+    ) {
       return await this.findByIds(entitiesIds, idsOrOptionsWithoutRelations)
     }
 
@@ -269,8 +277,9 @@ export class ProductRepository extends Repository<Product> {
     )
 
     const entitiesAndRelations = entitiesIdsWithRelations.concat(entities)
-    const entitiesToReturn =
+    const entitiesToReturn = this.mergeEntitiesWithRelations(
-      this.mergeEntitiesWithRelations(entitiesAndRelations)
+      entitiesAndRelations
+    )
 
     return entitiesToReturn
   }
@@ -314,4 +323,60 @@ export class ProductRepository extends Repository<Product> {
 
     return this.findByIds(productIds)
   }
+
+  public async getFreeTextSearchResultsAndCount(
+    q: string,
+    options: CustomOptions = { where: {} },
+    relations: (keyof Product)[] = []
+  ): Promise<[Product[], number]> {
+    const cleanedOptions = this._cleanOptions(options)
+
+    let qb = this.createQueryBuilder("product")
+      .leftJoinAndSelect("product.variants", "variant")
+      .leftJoinAndSelect("product.collection", "collection")
+      .select(["product.id"])
+      .where(cleanedOptions.where)
+      .andWhere(
+        new Brackets((qb) => {
+          qb.where(`product.description ILIKE :q`, { q: `%${q}%` })
+            .orWhere(`product.title ILIKE :q`, { q: `%${q}%` })
+            .orWhere(`variant.title ILIKE :q`, { q: `%${q}%` })
+            .orWhere(`variant.sku ILIKE :q`, { q: `%${q}%` })
+            .orWhere(`collection.title ILIKE :q`, { q: `%${q}%` })
+        })
+      )
+      .skip(cleanedOptions.skip)
+      .take(cleanedOptions.take)
+
+    if (cleanedOptions.withDeleted) {
+      qb = qb.withDeleted()
+    }
+
+    const [results, count] = await qb.getManyAndCount()
+
+    const products = await this.findWithRelations(
+      relations,
+      results.map((r) => r.id),
+      cleanedOptions.withDeleted
+    )
+
+    return [products, count]
+  }
+
+  private _cleanOptions(
+    options: CustomOptions
+  ): WithRequiredProperty<CustomOptions, "where"> {
+    const where = options.where ?? {}
+    if ("description" in where) {
+      delete where.description
+    }
+    if ("title" in where) {
+      delete where.title
+    }
+
+    return {
+      ...options,
+      where,
+    }
+  }
 }

packages/medusa/src/services/product.js

@@ -1,8 +1,7 @@
 import { MedusaError } from "medusa-core-utils"
 import { BaseService } from "medusa-interfaces"
-import { Brackets } from "typeorm"
-import { formatException } from "../utils/exception-formatter"
 import { defaultAdminProductsVariantsRelations } from "../api/routes/admin/products"
+import { formatException } from "../utils/exception-formatter"
 
 /**
  * Provides layer to manipulate products.
@@ -127,13 +126,13 @@ class ProductService extends BaseService {
     const { q, query, relations } = this.prepareListQuery_(selector, config)
 
     if (q) {
-      const qb = this.getFreeTextQueryBuilder_(productRepo, query, q)
+      const [products] = await productRepo.getFreeTextSearchResultsAndCount(
-      const raw = await qb.getMany()
+        q,
-      return productRepo.findWithRelations(
+        query,
-        relations,
+        relations
-        raw.map((i) => i.id),
-        query.withDeleted ?? false
       )
+
+      return products
     }
 
     const products = productRepo.findWithRelations(relations, query)
@@ -182,22 +181,20 @@ class ProductService extends BaseService {
 
     const { q, query, relations } = this.prepareListQuery_(selector, config)
 
+    let products
+    let count
     if (q) {
-      const qb = this.getFreeTextQueryBuilder_(productRepo, query, q)
+      ;[products, count] = await productRepo.getFreeTextSearchResultsAndCount(
-      const [raw, count] = await qb.getManyAndCount()
+        q,
-
+        query,
-      const products = await productRepo.findWithRelations(
+        relations
-        relations,
-        raw.map((i) => i.id),
-        query.withDeleted ?? false
       )
-      return [products, count]
+    } else {
-    }
+      ;[products, count] = await productRepo.findWithRelationsAndCount(
-
-    const [products, count] = await productRepo.findWithRelationsAndCount(
         relations,
         query
       )
+    }
 
     if (priceIndex > -1) {
       const productsWithAdditionalPrices = await this.setAdditionalPrices(
@@ -1004,44 +1001,6 @@ class ProductService extends BaseService {
     }
   }
 
-  /**
-   * Creates a QueryBuilder that can fetch products based on free text.
-   * @param {ProductRepository} productRepo - an instance of a ProductRepositry
-   * @param {FindOptions<Product>} query - the query to get products by
-   * @param {string} q - the text to perform free text search from
-   * @return {QueryBuilder<Product>} a query builder that can fetch products
-   */
-  getFreeTextQueryBuilder_(productRepo, query, q) {
-    const where = query.where
-
-    delete where.description
-    delete where.title
-
-    let qb = productRepo
-      .createQueryBuilder("product")
-      .leftJoinAndSelect("product.variants", "variant")
-      .leftJoinAndSelect("product.collection", "collection")
-      .select(["product.id"])
-      .where(where)
-      .andWhere(
-        new Brackets((qb) => {
-          qb.where(`product.description ILIKE :q`, { q: `%${q}%` })
-            .orWhere(`product.title ILIKE :q`, { q: `%${q}%` })
-            .orWhere(`variant.title ILIKE :q`, { q: `%${q}%` })
-            .orWhere(`variant.sku ILIKE :q`, { q: `%${q}%` })
-            .orWhere(`collection.title ILIKE :q`, { q: `%${q}%` })
-        })
-      )
-      .skip(query.skip)
-      .take(query.take)
-
-    if (query.withDeleted) {
-      qb = qb.withDeleted()
-    }
-
-    return qb
-  }
-
   /**
    * Set additional prices on a list of products.
    * @param {Product[] | Product} products list of products on which to set additional prices
@@ -1078,25 +1037,22 @@ class ProductService extends BaseService {
 
       const productArray = Array.isArray(products) ? products : [products]
 
-      const priceSelectionStrategy = this.priceSelectionStrategy_.withTransaction(
+      const priceSelectionStrategy =
-        manager
+        this.priceSelectionStrategy_.withTransaction(manager)
-      )
 
       const productsWithPrices = await Promise.all(
         productArray.map(async (p) => {
           if (p.variants?.length) {
             p.variants = await Promise.all(
               p.variants.map(async (v) => {
-                const prices = await priceSelectionStrategy.calculateVariantPrice(
+                const prices =
-                  v.id,
+                  await priceSelectionStrategy.calculateVariantPrice(v.id, {
-                  {
                     region_id: regionId,
                     currency_code: currencyCode,
                     cart_id: cart_id,
                     customer_id: customer_id,
                     include_discount_prices,
-                  }
+                  })
-                )
 
                 return {
                   ...v,

packages/medusa/src/types/common.ts

@@ -10,6 +10,15 @@ import "reflect-metadata"
 import { FindManyOptions, FindOperator, OrderByCondition } from "typeorm"
 import { transformDate } from "../utils/validators/date-transform"
 
+/**
+ * Utility type used to remove some optional attributes (coming from K) from a type T
+ */
+export type WithRequiredProperty<T, K extends keyof T> = T &
+  {
+    // -? removes 'optional' from a property
+    [Property in K]-?: T[Property]
+  }
+
 export type PartialPick<T, K extends keyof T> = {
   [P in K]?: T[P]
 }