chore(utils): Add default ordering to the internal service factory list/listAndCount (#6436)

**What**
Default ordering. By default, only the top level entity ordering is applied using the primary keys, the relations are default ordered by the foreign keys.

It include tests fixing for deterministic data ordering

.changeset/giant-berries-rush.md

@@ -0,0 +1,5 @@
+---
+"@medusajs/utils": patch
+---
+
+chore(utils): Add default ordering to the internal service factory list/listAndCount

packages/auth/integration-tests/__tests__/services/auth-user/index.spec.ts

@@ -38,15 +38,15 @@ describe("AuthUser Service", () => {
       const serialized = JSON.parse(JSON.stringify(authUsers))
 
       expect(serialized).toEqual([
-        expect.objectContaining({
-          provider: "manual",
-        }),
-        expect.objectContaining({
-          provider: "manual",
-        }),
         expect.objectContaining({
           provider: "store",
         }),
+        expect.objectContaining({
+          provider: "manual",
+        }),
+        expect.objectContaining({
+          provider: "manual",
+        }),
       ])
     })
 
@@ -87,15 +87,15 @@ describe("AuthUser Service", () => {
 
       expect(count).toEqual(3)
       expect(serialized).toEqual([
-        expect.objectContaining({
-          provider: "manual",
-        }),
-        expect.objectContaining({
-          provider: "manual",
-        }),
         expect.objectContaining({
           provider: "store",
         }),
+        expect.objectContaining({
+          provider: "manual",
+        }),
+        expect.objectContaining({
+          provider: "manual",
+        }),
       ])
     })
 

packages/auth/integration-tests/__tests__/services/module/auth-user.spec.ts

@@ -43,15 +43,15 @@ describe("AuthModuleService - AuthUser", () => {
       const authUsers = await service.list()
 
       expect(authUsers).toEqual([
-        expect.objectContaining({
-          provider: "manual",
-        }),
-        expect.objectContaining({
-          provider: "manual",
-        }),
         expect.objectContaining({
           provider: "store",
         }),
+        expect.objectContaining({
+          provider: "manual",
+        }),
+        expect.objectContaining({
+          provider: "manual",
+        }),
       ])
     })
 
@@ -89,15 +89,15 @@ describe("AuthModuleService - AuthUser", () => {
 
       expect(count).toEqual(3)
       expect(authUsers).toEqual([
-        expect.objectContaining({
-          provider: "manual",
-        }),
-        expect.objectContaining({
-          provider: "manual",
-        }),
         expect.objectContaining({
           provider: "store",
         }),
+        expect.objectContaining({
+          provider: "manual",
+        }),
+        expect.objectContaining({
+          provider: "manual",
+        }),
       ])
     })
 

packages/fulfillment/integration-tests/__tests__/fulfillment-module-service.spec.ts

@@ -44,12 +44,44 @@ moduleIntegrationTestRunner({
                     },
                   ],
                 },
+                {
+                  name: "test2",
+                  geo_zones: [
+                    {
+                      type: GeoZoneType.COUNTRY,
+                      country_code: "fr",
+                    },
+                  ],
+                },
+                {
+                  name: "_test",
+                  geo_zones: [
+                    {
+                      type: GeoZoneType.COUNTRY,
+                      country_code: "fr",
+                    },
+                  ],
+                },
               ],
             })
 
-            let listedSets = await service.list({
+            let listedSets = await service.list(
-              type: createdSet1.type,
+              {
-            })
+                type: createdSet1.type,
+              },
+              {
+                relations: ["service_zones"],
+              }
+            )
+
+            const listedSets2 = await service.list(
+              {
+                type: createdSet1.type,
+              },
+              {
+                relations: ["service_zones"],
+              }
+            )
 
             expect(listedSets).toEqual(
               expect.arrayContaining([
@@ -58,6 +90,15 @@ moduleIntegrationTestRunner({
               ])
             )
 
+            // Respecting order id by default
+            expect(listedSets[1].service_zones).toEqual([
+              expect.objectContaining({ name: "test" }),
+              expect.objectContaining({ name: "test2" }),
+              expect.objectContaining({ name: "_test" }),
+            ])
+
+            expect(listedSets2).toEqual(listedSets2)
+
             listedSets = await service.list({
               name: createdSet2.name,
             })
@@ -1206,9 +1247,11 @@ moduleIntegrationTestRunner({
 
             expect(updatedFulfillmentSets).toHaveLength(2)
 
-            let i = 0
             for (const data_ of updateData) {
-              expect(updatedFulfillmentSets[i]).toEqual(
+              const expectedFulfillmentSet = updatedFulfillmentSets.find(
+                (f) => f.id === data_.id
+              )
+              expect(expectedFulfillmentSet).toEqual(
                 expect.objectContaining({
                   id: data_.id,
                   name: data_.name,
@@ -1231,7 +1274,6 @@ moduleIntegrationTestRunner({
                   ]),
                 })
               )
-              ++i
             }
 
             const serviceZones = await service.listServiceZones()
@@ -1310,16 +1352,18 @@ moduleIntegrationTestRunner({
 
             expect(updatedFulfillmentSets).toHaveLength(2)
 
-            let i = 0
             for (const data_ of updateData) {
-              expect(updatedFulfillmentSets[i]).toEqual(
+              const expectedFulfillmentSet = updatedFulfillmentSets.find(
+                (f) => f.id === data_.id
+              )
+              expect(expectedFulfillmentSet).toEqual(
                 expect.objectContaining({
                   id: data_.id,
                   name: data_.name,
                   type: data_.type,
                   service_zones: expect.arrayContaining([
                     expect.objectContaining({
-                      id: createdFulfillmentSets[i].service_zones[0].id,
+                      id: expect.any(String),
                     }),
                     expect.objectContaining({
                       id: expect.any(String),
@@ -1338,7 +1382,6 @@ moduleIntegrationTestRunner({
                   ]),
                 })
               )
-              ++i
             }
 
             const serviceZones = await service.listServiceZones()
@@ -1470,9 +1513,11 @@ moduleIntegrationTestRunner({
 
             expect(updatedServiceZones).toHaveLength(2)
 
-            let i = 0
             for (const data_ of updateData) {
-              expect(updatedServiceZones[i]).toEqual(
+              const expectedServiceZone = updatedServiceZones.find(
+                (serviceZone) => serviceZone.id === data_.id
+              )
+              expect(expectedServiceZone).toEqual(
                 expect.objectContaining({
                   id: data_.id,
                   name: data_.name,
@@ -1485,7 +1530,6 @@ moduleIntegrationTestRunner({
                   ]),
                 })
               )
-              ++i
             }
           })
 
@@ -1617,16 +1661,17 @@ moduleIntegrationTestRunner({
 
             expect(updatedGeoZones).toHaveLength(2)
 
-            let i = 0
             for (const data_ of updateData) {
-              expect(updatedGeoZones[i]).toEqual(
+              const expectedGeoZone = updatedGeoZones.find(
+                (geoZone) => geoZone.id === data_.id
+              )
+              expect(expectedGeoZone).toEqual(
                 expect.objectContaining({
                   id: data_.id,
                   type: data_.type,
                   country_code: data_.country_code,
                 })
               )
-              ++i
             }
           })
         })

packages/pricing/integration-tests/__tests__/services/currency/index.spec.ts

@@ -1,18 +1,18 @@
 import { SqlEntityManager } from "@mikro-orm/postgresql"
 
 import { Currency } from "@models"
-import { CurrencyService } from "@services"
 
 import { createCurrencies } from "../../../__fixtures__/currency"
 import { MikroOrmWrapper } from "../../../utils"
 import { createMedusaContainer } from "@medusajs/utils"
 import { asValue } from "awilix"
 import ContainerLoader from "../../../../src/loaders/container"
+import { ModulesSdkTypes } from "@medusajs/types"
 
 jest.setTimeout(30000)
 
 describe("Currency Service", () => {
-  let service: CurrencyService
+  let service: ModulesSdkTypes.InternalModuleService<any>
   let testManager: SqlEntityManager
   let repositoryManager: SqlEntityManager
   let data!: Currency[]
@@ -57,14 +57,14 @@ describe("Currency Service", () => {
       const currenciesResult = await service.list()
 
       expect(currenciesResult).toEqual([
-        expect.objectContaining({
-          code: "USD",
-          name: "US Dollar",
-        }),
         expect.objectContaining({
           code: "CAD",
           name: "Canadian Dollar",
         }),
+        expect.objectContaining({
+          code: "USD",
+          name: "US Dollar",
+        }),
       ])
     })
 
@@ -86,14 +86,14 @@ describe("Currency Service", () => {
 
       expect(count).toEqual(2)
       expect(currenciesResult).toEqual([
-        expect.objectContaining({
-          code: "USD",
-          name: "US Dollar",
-        }),
         expect.objectContaining({
           code: "CAD",
           name: "Canadian Dollar",
         }),
+        expect.objectContaining({
+          code: "USD",
+          name: "US Dollar",
+        }),
       ])
     })
 
@@ -120,8 +120,8 @@ describe("Currency Service", () => {
       expect(count).toEqual(2)
       expect(currenciesResult).toEqual([
         expect.objectContaining({
-          code: "CAD",
+          code: "USD",
-          name: "Canadian Dollar",
+          name: "US Dollar",
         }),
       ])
     })
@@ -140,7 +140,7 @@ describe("Currency Service", () => {
       expect(count).toEqual(2)
       expect(serialized).toEqual([
         {
-          code: "USD",
+          code: "CAD",
         },
       ])
     })

packages/pricing/integration-tests/__tests__/services/money-amount/index.spec.ts

@@ -214,8 +214,8 @@ describe("MoneyAmount Service", () => {
       expect(count).toEqual(3)
       expect(serialized).toEqual([
         {
-          id: "money-amount-USD",
+          id: "money-amount-CAD",
-          amount: 500,
+          amount: 600,
         },
       ])
     })

packages/pricing/integration-tests/__tests__/services/pricing-module/currency.spec.ts

@@ -46,10 +46,6 @@ describe("PricingModule Service - Currency", () => {
       const currenciesResult = await service.listCurrencies()
 
       expect(currenciesResult).toEqual([
-        expect.objectContaining({
-          code: "USD",
-          name: "US Dollar",
-        }),
         expect.objectContaining({
           code: "CAD",
           name: "Canadian Dollar",
@@ -58,6 +54,10 @@ describe("PricingModule Service - Currency", () => {
           code: "EUR",
           name: "Euro",
         }),
+        expect.objectContaining({
+          code: "USD",
+          name: "US Dollar",
+        }),
       ])
     })
 
@@ -79,10 +79,6 @@ describe("PricingModule Service - Currency", () => {
 
       expect(count).toEqual(3)
       expect(currenciesResult).toEqual([
-        expect.objectContaining({
-          code: "USD",
-          name: "US Dollar",
-        }),
         expect.objectContaining({
           code: "CAD",
           name: "Canadian Dollar",
@@ -91,6 +87,10 @@ describe("PricingModule Service - Currency", () => {
           code: "EUR",
           name: "Euro",
         }),
+        expect.objectContaining({
+          code: "USD",
+          name: "US Dollar",
+        }),
       ])
     })
 
@@ -117,8 +117,10 @@ describe("PricingModule Service - Currency", () => {
       expect(count).toEqual(3)
       expect(currenciesResult).toEqual([
         expect.objectContaining({
-          code: "CAD",
+          code: "EUR",
-          name: "Canadian Dollar",
+          name: "Euro",
+          symbol: "€",
+          symbol_native: "€",
         }),
       ])
     })
@@ -137,7 +139,7 @@ describe("PricingModule Service - Currency", () => {
       expect(count).toEqual(3)
       expect(serialized).toEqual([
         {
-          code: "USD",
+          code: "CAD",
         },
       ])
     })

packages/pricing/integration-tests/__tests__/services/pricing-module/money-amount.spec.ts

@@ -201,7 +201,7 @@ describe("PricingModule Service - MoneyAmount", () => {
       expect(count).toEqual(3)
       expect(serialized).toEqual([
         {
-          id: "money-amount-USD",
+          id: "money-amount-CAD",
           amount: null,
         },
       ])

packages/pricing/integration-tests/__tests__/services/pricing-module/price-set-money-amount-rules.spec.ts

@@ -273,16 +273,14 @@ describe("PricingModule Service - PriceSetMoneyAmountRules", () => {
         },
       ])
 
-      const priceSetMoneyAmountRules =
+      const [created] = await service.listPriceSetMoneyAmountRules(
-        await service.listPriceSetMoneyAmountRules(
+        {
-          {},
+          value: ["New priceSetMoneyAmountRule"],
-          {
+        },
-            relations: ["price_set_money_amount", "rule_type"],
+        {
-          }
+          relations: ["price_set_money_amount", "rule_type"],
-        )
+        }
-
+      )
-      const created =
-        priceSetMoneyAmountRules[priceSetMoneyAmountRules.length - 1]
 
       expect(created).toEqual(
         expect.objectContaining({

packages/pricing/src/services/__tests__/currency.spec.ts

@@ -92,6 +92,9 @@ describe("Currency service", function () {
           fields: undefined,
           limit: 15,
           offset: 0,
+          orderBy: {
+            code: "ASC",
+          },
           populate: [],
           withDeleted: undefined,
         },
@@ -130,6 +133,9 @@ describe("Currency service", function () {
           fields: undefined,
           limit: 15,
           offset: 0,
+          orderBy: {
+            code: "ASC",
+          },
           populate: [],
           withDeleted: undefined,
         },
@@ -168,6 +174,9 @@ describe("Currency service", function () {
           fields: undefined,
           limit: 15,
           offset: 0,
+          orderBy: {
+            code: "ASC",
+          },
           withDeleted: undefined,
           populate: ["tags"],
         },
@@ -206,6 +215,9 @@ describe("Currency service", function () {
           fields: undefined,
           limit: 15,
           offset: 0,
+          orderBy: {
+            code: "ASC",
+          },
           withDeleted: undefined,
           populate: ["tags"],
         },

packages/product/src/services/__tests__/product.spec.ts

@@ -91,6 +91,9 @@ describe("Product service", function () {
           fields: undefined,
           limit: 15,
           offset: 0,
+          orderBy: {
+            id: "ASC",
+          },
           populate: [],
           withDeleted: undefined,
         },
@@ -129,6 +132,9 @@ describe("Product service", function () {
           fields: undefined,
           limit: 15,
           offset: 0,
+          orderBy: {
+            id: "ASC",
+          },
           populate: [],
           withDeleted: undefined,
         },
@@ -167,6 +173,9 @@ describe("Product service", function () {
           fields: undefined,
           limit: 15,
           offset: 0,
+          orderBy: {
+            id: "ASC",
+          },
           withDeleted: undefined,
           populate: ["tags"],
         },
@@ -205,6 +214,9 @@ describe("Product service", function () {
           fields: undefined,
           limit: 15,
           offset: 0,
+          orderBy: {
+            id: "ASC",
+          },
           withDeleted: undefined,
           populate: ["tags"],
         },

packages/sales-channel/src/services/__tests__/sales-channle.spec.ts

@@ -34,6 +34,9 @@ describe("Sales channel service", function () {
           fields: ["id", "name"],
           limit: 15,
           offset: 0,
+          orderBy: {
+            id: "ASC",
+          },
           withDeleted: undefined,
           populate: [],
         },

packages/utils/src/modules-sdk/__tests__/internal-module-service-factory.spec.ts

@@ -3,7 +3,12 @@ import { lowerCaseFirst } from "../../common"
 
 const defaultContext = { __type: "MedusaContext" }
 
-class Model {}
+class Model {
+  static __meta = {
+    primaryKeys: ["id"],
+  }
+}
+
 describe("Internal Module Service Factory", () => {
   const modelRepositoryName = `${lowerCaseFirst(Model.name)}Repository`
 
@@ -114,6 +119,34 @@ describe("Internal Module Service Factory", () => {
       expect(result).toEqual(entities)
     })
 
+    it("should list entities and relation with default ordering successfully", async () => {
+      const entities = [
+        { id: "1", name: "Item" },
+        { id: "2", name: "Item2" },
+      ]
+      containerMock[modelRepositoryName].find.mockResolvedValueOnce(entities)
+
+      const result = await instance.list(
+        {},
+        {
+          relations: ["relation"],
+        }
+      )
+
+      expect(result).toEqual(entities)
+      expect(containerMock[modelRepositoryName].find).toHaveBeenCalledWith(
+        expect.objectContaining({
+          options: expect.objectContaining({
+            populate: ["relation"],
+            orderBy: {
+              id: "ASC",
+            },
+          }),
+        }),
+        expect.any(Object)
+      )
+    })
+
     it("should list and count entities successfully", async () => {
       const entities = [
         { id: "1", name: "Item" },

packages/utils/src/modules-sdk/internal-module-service-factory.ts

@@ -63,6 +63,24 @@ export function internalModuleServiceFactory<
       return keys.map((k) => data[k]).join("_")
     }
 
+    /**
+     * Only apply top level default ordering as the relation
+     * default ordering is already applied through the foreign key
+     * @param config
+     */
+    static applyDefaultOrdering(config: FindConfig<any>) {
+      if (config.order) {
+        return
+      }
+
+      config.order = {}
+
+      const primaryKeys = AbstractService_.retrievePrimaryKeys(model)
+      primaryKeys.forEach((primaryKey) => {
+        config.order![primaryKey] = "ASC"
+      })
+    }
+
     @InjectManager(propertyRepositoryName)
     async retrieve(
       idOrObject: string | object,
@@ -129,6 +147,7 @@ export function internalModuleServiceFactory<
       config: FindConfig<any> = {},
       @MedusaContext() sharedContext: Context = {}
     ): Promise<TEntity[]> {
+      AbstractService_.applyDefaultOrdering(config)
       const queryOptions = buildQuery(filters, config)
 
       return await this[propertyRepositoryName].find(
@@ -143,6 +162,7 @@ export function internalModuleServiceFactory<
       config: FindConfig<any> = {},
       @MedusaContext() sharedContext: Context = {}
     ): Promise<[TEntity[], number]> {
+      AbstractService_.applyDefaultOrdering(config)
       const queryOptions = buildQuery(filters, config)
 
       return await this[propertyRepositoryName].findAndCount(