diff --git a/.changeset/tame-insects-marry.md b/.changeset/tame-insects-marry.md new file mode 100644 index 0000000000..fc0bc71456 --- /dev/null +++ b/.changeset/tame-insects-marry.md @@ -0,0 +1,6 @@ +--- +"@medusajs/product": patch +"@medusajs/utils": patch +--- + +fix returned updates order in upsertWithReplace diff --git a/integration-tests/http/__tests__/product/admin/product.spec.ts b/integration-tests/http/__tests__/product/admin/product.spec.ts index e472fbe77f..edefbc4a8f 100644 --- a/integration-tests/http/__tests__/product/admin/product.spec.ts +++ b/integration-tests/http/__tests__/product/admin/product.spec.ts @@ -3325,6 +3325,130 @@ medusaIntegrationTestRunner({ }) ) }) + + // https://linear.app/medusajs/issue/SUP-1631/price-differences + it("should successfully handle successive updates on prices", async () => { + const response = await api.post( + "/admin/products/batch", + { + create: [ + getProductFixture({ + title: "title1", + variants: [ + { + title: "variant1", + prices: [{ currency_code: "usd", amount: 100 }], + }, + ], + }), + getProductFixture({ + title: "title2", + variants: [ + { + title: "variant2", + prices: [{ currency_code: "usd", amount: 200 }], + }, + ], + }), + getProductFixture({ + title: "title3", + variants: [ + { + title: "variant3", + prices: [{ currency_code: "usd", amount: 300 }], + }, + ], + }), + ], + update: [], + delete: [baseProduct.id], + }, + adminHeaders + ) + const created = response.data.created + const created1 = created.find((p) => p.title === "title1") + const created2 = created.find((p) => p.title === "title2") + const created3 = created.find((p) => p.title === "title3") + + // Update the first and third product price to shuffle their order in the DB + await api.post( + `/admin/products/batch`, + { + update: [ + { + id: created1.id, + variants: [ + { + id: created1.variants[0].id, + prices: [{ currency_code: "usd", amount: 101 }], + }, + ], + }, + { + id: created3.id, + variants: [ + { + id: created3.variants[0].id, + prices: [{ currency_code: "usd", amount: 301 }], + }, + ], + }, + ], + }, + adminHeaders + ) + + // Update all products now + await api.post( + `/admin/products/batch`, + { + update: [ + { + id: created1.id, + variants: [ + { + id: created1.variants[0].id, + prices: [{ currency_code: "usd", amount: 102 }], + }, + ], + }, + { + id: created2.id, + variants: [ + { + id: created2.variants[0].id, + prices: [{ currency_code: "usd", amount: 202 }], + }, + ], + }, + { + id: created3.id, + variants: [ + { + id: created3.variants[0].id, + prices: [{ currency_code: "usd", amount: 302 }], + }, + ], + }, + ], + }, + adminHeaders + ) + + // Get the first product + const product1 = ( + await api.get(`/admin/products/${created1.id}`, adminHeaders) + ).data.product + const product2 = ( + await api.get(`/admin/products/${created2.id}`, adminHeaders) + ).data.product + const product3 = ( + await api.get(`/admin/products/${created3.id}`, adminHeaders) + ).data.product + expect(product1.variants[0].prices[0].amount).toEqual(102) + expect(product2.variants[0].prices[0].amount).toEqual(202) + expect(product3.variants[0].prices[0].amount).toEqual(302) + }) }) // TODO: Discuss how this should be handled diff --git a/packages/core/utils/src/dal/mikro-orm/integration-tests/__tests__/mikro-orm-repository.spec.ts b/packages/core/utils/src/dal/mikro-orm/integration-tests/__tests__/mikro-orm-repository.spec.ts index 3e2dffa0f0..44b73ad930 100644 --- a/packages/core/utils/src/dal/mikro-orm/integration-tests/__tests__/mikro-orm-repository.spec.ts +++ b/packages/core/utils/src/dal/mikro-orm/integration-tests/__tests__/mikro-orm-repository.spec.ts @@ -227,39 +227,68 @@ describe("mikroOrmRepository", () => { ) }) - it("should successfully update a flat entity", async () => { + it("batch updates should retain order of entities", async () => { const entity1 = { id: "1", title: "en1" } + const entity2 = { id: "2", title: "en2" } + const entity3 = { id: "3", title: "en3" } const { performedActions: performedActions1 } = - await manager1().upsertWithReplace([entity1]) + await manager1().upsertWithReplace([entity1, entity2, entity3]) expect(performedActions1).toEqual({ created: { - [Entity1.name]: [expect.objectContaining({ id: entity1.id })], + [Entity1.name]: [ + expect.objectContaining({ id: entity1.id }), + expect.objectContaining({ id: entity2.id }), + expect.objectContaining({ id: entity3.id }), + ], }, updated: {}, deleted: {}, }) - entity1.title = "newen1" + // Doing this to shuffle the physical order of rows in the DB + entity1.title = "en1-update-1" + entity3.title = "en3-update-1" + await manager1().upsertWithReplace([entity1, entity3]) + + entity1.title = "en1-update-2" + entity2.title = "en2-update-2" + entity3.title = "en3-update-2" const { performedActions: performedActions2 } = - await manager1().upsertWithReplace([entity1]) + await manager1().upsertWithReplace([entity1, entity2, entity3]) expect(performedActions2).toEqual({ created: {}, updated: { - [Entity1.name]: [expect.objectContaining({ id: entity1.id })], + [Entity1.name]: [ + expect.objectContaining({ id: entity1.id }), + expect.objectContaining({ id: entity2.id }), + expect.objectContaining({ id: entity3.id }), + ], }, deleted: {}, }) - const listedEntities = await manager1().find() - - expect(listedEntities).toHaveLength(1) - expect(wrap(listedEntities[0]).toPOJO()).toEqual( + const readEntity1 = await manager1().find({ where: { id: "1" } }) + expect(wrap(readEntity1[0]).toPOJO()).toEqual( expect.objectContaining({ id: "1", - title: "newen1", + title: "en1-update-2", + }) + ) + const readEntity2 = await manager1().find({ where: { id: "2" } }) + expect(wrap(readEntity2[0]).toPOJO()).toEqual( + expect.objectContaining({ + id: "2", + title: "en2-update-2", + }) + ) + const readEntity3 = await manager1().find({ where: { id: "3" } }) + expect(wrap(readEntity3[0]).toPOJO()).toEqual( + expect.objectContaining({ + id: "3", + title: "en3-update-2", }) ) }) diff --git a/packages/core/utils/src/dal/mikro-orm/mikro-orm-repository.ts b/packages/core/utils/src/dal/mikro-orm/mikro-orm-repository.ts index 8e67f7da73..63e30b14e0 100644 --- a/packages/core/utils/src/dal/mikro-orm/mikro-orm-repository.ts +++ b/packages/core/utils/src/dal/mikro-orm/mikro-orm-repository.ts @@ -1012,11 +1012,13 @@ export function mikroOrmBaseRepositoryFactory( { ctx: manager.getTransactionContext() } ) .then((res) => { - performedActions.updated[entityName] ??= [] const updatedRows = res.rows ?? [] - performedActions.updated[entityName].push( - ...updatedRows.map((d) => ({ id: d.id })) - ) + const updatedRowsMap = new Map(updatedRows.map((d) => [d.id, d])) + + performedActions.updated[entityName] = toUpdate + .map((d) => updatedRowsMap.get(d.id)) + .filter((row) => row !== undefined) + .map((d) => ({ id: d.id })) }) ) } diff --git a/packages/modules/product/src/repositories/product.ts b/packages/modules/product/src/repositories/product.ts index dc245f46d4..2b8380441b 100644 --- a/packages/modules/product/src/repositories/product.ts +++ b/packages/modules/product/src/repositories/product.ts @@ -72,7 +72,12 @@ export class ProductRepository extends DALUtils.mikroOrmBaseRepositoryFactory( wrap(product!).assign(update) } - return products + // Doing this to ensure updates are returned in the same order they were provided, + // since some core flows rely on this. + // This is a high level of coupling though. + return updates + .map((update) => productsMap.get(update.id)) + .filter((product) => product !== undefined) } /**