Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core-flows): set SalesChannels on Product update #7272

Merged
merged 13 commits into from
May 21, 2024
5 changes: 5 additions & 0 deletions .changeset/spicy-panthers-exist.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@medusajs/core-flows": patch
---

fix(core-flows): set SalesChannels for Products on update
117 changes: 117 additions & 0 deletions integration-tests/api/__tests__/admin/product.js
Original file line number Diff line number Diff line change
Expand Up @@ -1749,6 +1749,123 @@ medusaIntegrationTestRunner({
)
})

it("updates products sales channels", async () => {
const [productId, salesChannel1Id, salesChannel2Id] = await breaking(
async () => {
const salesChannel1 = await simpleSalesChannelFactory(
dbConnection,
{
name: "test name 1",
description: "test description",
products: [baseProduct],
}
)

const salesChannel2 = await simpleSalesChannelFactory(
dbConnection,
{
name: "test name 2",
description: "test description",
salesChannel1,
// no assigned products
}
)

return [baseProduct.id, salesChannel1.id, salesChannel2.id]
},
async () => {
const salesChannelService = getContainer().resolve(
ModuleRegistrationName.SALES_CHANNEL
)

const salesChannel1 = await salesChannelService.create({
name: "test name 1",
description: "test description",
})

const salesChannel2 = await salesChannelService.create({
name: "test name 2",
description: "test description",
})

const newProduct = (
await api.post(
"/admin/products",
getProductFixture({
title: "Test saleschannel",
sales_channels: [{ id: salesChannel1.id }],
}),
adminHeaders
)
).data.product
return [newProduct.id, salesChannel1.id, salesChannel2.id]
}
)

await api.post(
`/admin/products/${productId}`,
{
title: "new name",
sales_channels: [
{ id: salesChannel1Id },
{ id: salesChannel2Id },
],
},
adminHeaders
)

let res = await api.get(
`/admin/products/${productId}?fields=*sales_channels`,
adminHeaders
)

expect(res.status).toEqual(200)
expect(res.data.product).toEqual(
expect.objectContaining({
id: productId,
title: "new name",
sales_channels: expect.arrayContaining([
expect.objectContaining({
id: salesChannel1Id,
name: "test name 1",
}),
expect.objectContaining({
id: salesChannel2Id,
name: "test name 2",
}),
]),
})
)

await api.post(
`/admin/products/${productId}`,
{
title: "new name 2",
sales_channels: [{ id: salesChannel2Id }], // update channels again to remove the first one
},
adminHeaders
)

res = await api.get(
`/admin/products/${productId}?fields=*sales_channels`,
adminHeaders
)

expect(res.status).toEqual(200)
expect(res.data.product).toEqual(
expect.objectContaining({
id: productId,
title: "new name 2",
sales_channels: expect.arrayContaining([
expect.objectContaining({
id: salesChannel2Id,
name: "test name 2",
}),
]),
})
)
})

it("fails to update product with invalid status", async () => {
const payload = {
status: null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
} from "@medusajs/utils"
import { StepResponse, createStep } from "@medusajs/workflows-sdk"

type UpdateProductsStepInput =
export type UpdateProductsStepInput =
| {
selector: ProductTypes.FilterableProductProps
update: ProductTypes.UpdateProductDTO
Expand Down
161 changes: 151 additions & 10 deletions packages/core/core-flows/src/product/workflows/update-products.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,167 @@
import { ProductTypes } from "@medusajs/types"
import { WorkflowData, createWorkflow } from "@medusajs/workflows-sdk"
import {
createWorkflow,
transform,
WorkflowData,
} from "@medusajs/workflows-sdk"
import { updateProductsStep } from "../steps/update-products"

import {
createLinkStep,
removeRemoteLinkStep,
useRemoteQueryStep,
} from "../../common"
import { arrayDifference } from "@medusajs/utils"
import { DeleteEntityInput, Modules } from "@medusajs/modules-sdk"

type UpdateProductsStepInputSelector = {
selector: ProductTypes.FilterableProductProps
update: ProductTypes.UpdateProductDTO & {
sales_channels?: { id: string }[]
}
}

type UpdateProductsStepInputProducts = {
products: (ProductTypes.UpsertProductDTO & {
sales_channels?: { id: string }[]
})[]
}

type UpdateProductsStepInput =
| {
selector: ProductTypes.FilterableProductProps
update: ProductTypes.UpdateProductDTO
}
| {
products: ProductTypes.UpsertProductDTO[]
}
| UpdateProductsStepInputSelector
| UpdateProductsStepInputProducts

type WorkflowInput = UpdateProductsStepInput
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: We have a mix of workflow types in the types package and in the core-flows package, it will require a cleanup at some point, not now but just raising it. we also have different places in the types package where the workflow types are define, workflows, workflow, and in some dir. maybe we can create a ticket

Copy link
Contributor

@olivermrbl olivermrbl May 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prepareUpdateProductInput, updateProductIds, and prepareToDeleteLinks are all named as if they were generic transformers for updating products, but they are coupled with sales channel management as far as I can tell. So what I mean is, if we add another link to product, we will need to refactor these to account for that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh sorry maybe i ve named them wrongly, but we can rework that, i just based the name on the workflow step they were used for. I didnt have a better idea at the moment but the main thing was to extract them (not necessarily generic) to have a better readability of the workflow and transformers themselves

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, those transformers are not exported they are only local so very specific here, i am fine with finding another name or rework the implementation a bit


function prepareUpdateProductInput({
input,
}: {
input: WorkflowInput
}): UpdateProductsStepInput {
if ("products" in input) {
return {
products: input.products.map((p) => ({
...p,
sales_channels: undefined,
})),
}
}

return {
selector: input.selector,
update: {
...input.update,
sales_channels: undefined,
},
}
}

function updateProductIds({
updatedProducts,
input,
}: {
updatedProducts: ProductTypes.ProductDTO[]
input: WorkflowInput
}) {
if ("products" in input) {
let productIds = updatedProducts.map((p) => p.id)
const discardedProductIds: string[] = input.products
.filter((p) => !p.sales_channels)
.map((p) => p.id as string)
return arrayDifference(productIds, discardedProductIds)
}

return !input.update.sales_channels ? [] : undefined
}

function prepareSalesChannelLinks({
input,
updatedProducts,
}: {
updatedProducts: ProductTypes.ProductDTO[]
input: WorkflowInput
}): Record<string, Record<string, any>>[] {
if ("products" in input) {
return input.products
.filter((p) => p.sales_channels)
.flatMap((p) =>
p.sales_channels!.map((sc) => ({
[Modules.PRODUCT]: {
product_id: p.id,
},
[Modules.SALES_CHANNEL]: {
sales_channel_id: sc.id,
},
}))
)
}

if (input.selector && input.update.sales_channels?.length) {
return updatedProducts.flatMap((p) =>
input.update.sales_channels!.map((channel) => ({
[Modules.PRODUCT]: {
product_id: p.id,
},
[Modules.SALES_CHANNEL]: {
sales_channel_id: channel.id,
},
}))
)
}

return []
}

function prepareToDeleteLinks({
currentLinks,
}: {
currentLinks: {
product_id: string
sales_channel_id: string
}[]
}) {
return currentLinks.map(({ product_id, sales_channel_id }) => ({
[Modules.PRODUCT]: {
product_id,
},
[Modules.SALES_CHANNEL]: {
sales_channel_id,
},
}))
}

export const updateProductsWorkflowId = "update-products"
export const updateProductsWorkflow = createWorkflow(
updateProductsWorkflowId,
(
input: WorkflowData<WorkflowInput>
): WorkflowData<ProductTypes.ProductDTO[]> => {
fPolic marked this conversation as resolved.
Show resolved Hide resolved
// TODO: Delete price sets for removed variants
// TODO Update sales channel links
return updateProductsStep(input)

const toUpdateInput = transform({ input }, prepareUpdateProductInput)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: I don't know if I like, that the steps that were added here are so specific to sales channel management. E.g. as soon as we add another link to products, we will need to revisit all of these. Or create separate steps.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to relook into it but your comment is on the transformer, are you referring to the transformer of the next step or the step itself just to be sure

const updatedProducts = updateProductsStep(toUpdateInput)
const updatedProductIds = transform(
{ updatedProducts, input },
updateProductIds
)

const currentLinks = useRemoteQueryStep({
entry_point: "product_sales_channel",
Copy link
Member

@adrien2p adrien2p May 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: service: Links.productSalesChannel

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just saw that it requires a little adaptation of that step but it should be good to have so that we don't rely on the string for the link themselves wdyt?

fields: ["product_id", "sales_channel_id"],
variables: { product_id: updatedProductIds },
})

const toDeleteLinks = transform({ currentLinks }, prepareToDeleteLinks)

removeRemoteLinkStep(toDeleteLinks as DeleteEntityInput[])

const salesChannelLinks = transform(
{ input, updatedProducts },
prepareSalesChannelLinks
)

createLinkStep(salesChannelLinks)

return updatedProducts
}
)
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Modules } from "@medusajs/modules-sdk"
import { ContainerRegistrationKeys } from "@medusajs/utils"
import { StepResponse, createStep } from "@medusajs/workflows-sdk"
import { createStep, StepResponse } from "@medusajs/workflows-sdk"

interface StepInput {
links: {
Expand Down
Loading