fix(product): returned updates order in upsertWithReplace (#12486)
* add failing test for upsertWithReplace order * reproduce prices update shuffling issue * fix: fix order of returned updates in updateMany * fix: fix order of returned updates in ProductService * fix: reset test count to 1 * Create tame-insects-marry.md --------- Co-authored-by: Oli Juhl <59018053+olivermrbl@users.noreply.github.com>
This commit is contained in:
@@ -0,0 +1,6 @@
|
|||||||
|
---
|
||||||
|
"@medusajs/product": patch
|
||||||
|
"@medusajs/utils": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
fix returned updates order in upsertWithReplace
|
||||||
@@ -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
|
// TODO: Discuss how this should be handled
|
||||||
|
|||||||
+40
-11
@@ -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 entity1 = { id: "1", title: "en1" }
|
||||||
|
const entity2 = { id: "2", title: "en2" }
|
||||||
|
const entity3 = { id: "3", title: "en3" }
|
||||||
|
|
||||||
const { performedActions: performedActions1 } =
|
const { performedActions: performedActions1 } =
|
||||||
await manager1().upsertWithReplace([entity1])
|
await manager1().upsertWithReplace([entity1, entity2, entity3])
|
||||||
|
|
||||||
expect(performedActions1).toEqual({
|
expect(performedActions1).toEqual({
|
||||||
created: {
|
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: {},
|
updated: {},
|
||||||
deleted: {},
|
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 } =
|
const { performedActions: performedActions2 } =
|
||||||
await manager1().upsertWithReplace([entity1])
|
await manager1().upsertWithReplace([entity1, entity2, entity3])
|
||||||
|
|
||||||
expect(performedActions2).toEqual({
|
expect(performedActions2).toEqual({
|
||||||
created: {},
|
created: {},
|
||||||
updated: {
|
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: {},
|
deleted: {},
|
||||||
})
|
})
|
||||||
|
|
||||||
const listedEntities = await manager1().find()
|
const readEntity1 = await manager1().find({ where: { id: "1" } })
|
||||||
|
expect(wrap(readEntity1[0]).toPOJO()).toEqual(
|
||||||
expect(listedEntities).toHaveLength(1)
|
|
||||||
expect(wrap(listedEntities[0]).toPOJO()).toEqual(
|
|
||||||
expect.objectContaining({
|
expect.objectContaining({
|
||||||
id: "1",
|
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",
|
||||||
})
|
})
|
||||||
)
|
)
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -1012,11 +1012,13 @@ export function mikroOrmBaseRepositoryFactory<const T extends object>(
|
|||||||
{ ctx: manager.getTransactionContext() }
|
{ ctx: manager.getTransactionContext() }
|
||||||
)
|
)
|
||||||
.then((res) => {
|
.then((res) => {
|
||||||
performedActions.updated[entityName] ??= []
|
|
||||||
const updatedRows = res.rows ?? []
|
const updatedRows = res.rows ?? []
|
||||||
performedActions.updated[entityName].push(
|
const updatedRowsMap = new Map(updatedRows.map((d) => [d.id, d]))
|
||||||
...updatedRows.map((d) => ({ id: d.id }))
|
|
||||||
)
|
performedActions.updated[entityName] = toUpdate
|
||||||
|
.map((d) => updatedRowsMap.get(d.id))
|
||||||
|
.filter((row) => row !== undefined)
|
||||||
|
.map((d) => ({ id: d.id }))
|
||||||
})
|
})
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -72,7 +72,12 @@ export class ProductRepository extends DALUtils.mikroOrmBaseRepositoryFactory(
|
|||||||
wrap(product!).assign(update)
|
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)
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
Reference in New Issue
Block a user