diff --git a/.changeset/giant-berries-rush.md b/.changeset/giant-berries-rush.md new file mode 100644 index 0000000000..36fe5f9924 --- /dev/null +++ b/.changeset/giant-berries-rush.md @@ -0,0 +1,5 @@ +--- +"@medusajs/utils": patch +--- + +chore(utils): Add default ordering to the internal service factory list/listAndCount diff --git a/packages/auth/integration-tests/__tests__/services/auth-user/index.spec.ts b/packages/auth/integration-tests/__tests__/services/auth-user/index.spec.ts index 32dd1aa819..3aae9c7e48 100644 --- a/packages/auth/integration-tests/__tests__/services/auth-user/index.spec.ts +++ b/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", + }), ]) }) diff --git a/packages/auth/integration-tests/__tests__/services/module/auth-user.spec.ts b/packages/auth/integration-tests/__tests__/services/module/auth-user.spec.ts index 2a45edeeb6..263ee6268f 100644 --- a/packages/auth/integration-tests/__tests__/services/module/auth-user.spec.ts +++ b/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", + }), ]) }) diff --git a/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service.spec.ts b/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service.spec.ts index 5922f8e9b6..a1aaa62d8b 100644 --- a/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service.spec.ts +++ b/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({ - type: createdSet1.type, - }) + let listedSets = await service.list( + { + 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 } }) }) diff --git a/packages/pricing/integration-tests/__tests__/services/currency/index.spec.ts b/packages/pricing/integration-tests/__tests__/services/currency/index.spec.ts index cf054ed3ae..fc14191baa 100644 --- a/packages/pricing/integration-tests/__tests__/services/currency/index.spec.ts +++ b/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 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", - name: "Canadian Dollar", + code: "USD", + name: "US Dollar", }), ]) }) @@ -140,7 +140,7 @@ describe("Currency Service", () => { expect(count).toEqual(2) expect(serialized).toEqual([ { - code: "USD", + code: "CAD", }, ]) }) diff --git a/packages/pricing/integration-tests/__tests__/services/money-amount/index.spec.ts b/packages/pricing/integration-tests/__tests__/services/money-amount/index.spec.ts index 618857e6db..592c060644 100644 --- a/packages/pricing/integration-tests/__tests__/services/money-amount/index.spec.ts +++ b/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", - amount: 500, + id: "money-amount-CAD", + amount: 600, }, ]) }) diff --git a/packages/pricing/integration-tests/__tests__/services/pricing-module/currency.spec.ts b/packages/pricing/integration-tests/__tests__/services/pricing-module/currency.spec.ts index ae2865a504..2ecd9a880d 100644 --- a/packages/pricing/integration-tests/__tests__/services/pricing-module/currency.spec.ts +++ b/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", - name: "Canadian Dollar", + code: "EUR", + name: "Euro", + symbol: "€", + symbol_native: "€", }), ]) }) @@ -137,7 +139,7 @@ describe("PricingModule Service - Currency", () => { expect(count).toEqual(3) expect(serialized).toEqual([ { - code: "USD", + code: "CAD", }, ]) }) diff --git a/packages/pricing/integration-tests/__tests__/services/pricing-module/money-amount.spec.ts b/packages/pricing/integration-tests/__tests__/services/pricing-module/money-amount.spec.ts index c1e47a73a9..6bec0199c1 100644 --- a/packages/pricing/integration-tests/__tests__/services/pricing-module/money-amount.spec.ts +++ b/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, }, ]) diff --git a/packages/pricing/integration-tests/__tests__/services/pricing-module/price-set-money-amount-rules.spec.ts b/packages/pricing/integration-tests/__tests__/services/pricing-module/price-set-money-amount-rules.spec.ts index db74001808..909399ce56 100644 --- a/packages/pricing/integration-tests/__tests__/services/pricing-module/price-set-money-amount-rules.spec.ts +++ b/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 = - await service.listPriceSetMoneyAmountRules( - {}, - { - relations: ["price_set_money_amount", "rule_type"], - } - ) - - const created = - priceSetMoneyAmountRules[priceSetMoneyAmountRules.length - 1] + const [created] = await service.listPriceSetMoneyAmountRules( + { + value: ["New priceSetMoneyAmountRule"], + }, + { + relations: ["price_set_money_amount", "rule_type"], + } + ) expect(created).toEqual( expect.objectContaining({ diff --git a/packages/pricing/src/services/__tests__/currency.spec.ts b/packages/pricing/src/services/__tests__/currency.spec.ts index 46cc0f789c..125d5c1a86 100644 --- a/packages/pricing/src/services/__tests__/currency.spec.ts +++ b/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"], }, diff --git a/packages/product/src/services/__tests__/product.spec.ts b/packages/product/src/services/__tests__/product.spec.ts index fb1a6819d3..5d3a3e1707 100644 --- a/packages/product/src/services/__tests__/product.spec.ts +++ b/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"], }, diff --git a/packages/sales-channel/src/services/__tests__/sales-channle.spec.ts b/packages/sales-channel/src/services/__tests__/sales-channle.spec.ts index 61e5bb1a1f..22ec375ba1 100644 --- a/packages/sales-channel/src/services/__tests__/sales-channle.spec.ts +++ b/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: [], }, diff --git a/packages/utils/src/modules-sdk/__tests__/internal-module-service-factory.spec.ts b/packages/utils/src/modules-sdk/__tests__/internal-module-service-factory.spec.ts index b8825dd3e6..d5633294e3 100644 --- a/packages/utils/src/modules-sdk/__tests__/internal-module-service-factory.spec.ts +++ b/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" }, diff --git a/packages/utils/src/modules-sdk/internal-module-service-factory.ts b/packages/utils/src/modules-sdk/internal-module-service-factory.ts index 58af0431fc..d792d75992 100644 --- a/packages/utils/src/modules-sdk/internal-module-service-factory.ts +++ b/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) { + 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 = {}, @MedusaContext() sharedContext: Context = {} ): Promise { + AbstractService_.applyDefaultOrdering(config) const queryOptions = buildQuery(filters, config) return await this[propertyRepositoryName].find( @@ -143,6 +162,7 @@ export function internalModuleServiceFactory< config: FindConfig = {}, @MedusaContext() sharedContext: Context = {} ): Promise<[TEntity[], number]> { + AbstractService_.applyDefaultOrdering(config) const queryOptions = buildQuery(filters, config) return await this[propertyRepositoryName].findAndCount(