feat(medusa,workflows) Create cart workflow #4685
Conversation
🦋 Changeset detectedLatest commit: d7af8e9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
riqwan
force-pushed
the
feat/create-cart-workflow
branch
from
1d07a6c to
e69286b
Compare
riqwan
force-pushed
the
feat/create-cart-workflow
branch
from
0296e96 to
7233e2f
Compare
riqwan
force-pushed
the
feat/create-cart-workflow
branch
from
a70a13b to
7fe07e9
Compare
riqwan
force-pushed
the
feat/create-cart-workflow
branch
from
7fe07e9 to
e06e0b9
Compare
riqwan
changed the title
| } | ||
|
|
||
| type HandlerInputData = { | ||
| cart: { |
There was a problem hiding this comment.
suggestion: should we just expect id, email and this handler becomes just a createOrRetrieve customer? to be mre reusable?
There was a problem hiding this comment.
I still think we should scope it, that would make it easier for the handler to accept from other handlers and for each of those to not clash. Maybe not for this, but it would be good to have a generic structure for these handlers.
We could still make it more reusable.
{
customer: {
id?: string
email?: string
}
}
| context, | ||
| data, | ||
| }: WorkflowArguments<HandlerInputData>): Promise<AttachCustomerDetailsDTO> { | ||
| const customerDTO: AttachCustomerDetailsDTO = {} |
| }: WorkflowArguments<HandlerInputData>): Promise<AttachCustomerDetailsDTO> { | ||
| const customerDTO: AttachCustomerDetailsDTO = {} | ||
| const customerService = container.resolve("customerService") | ||
| const entityManager = container.resolve("manager") |
| const customerDTO: AttachCustomerDetailsDTO = {} | ||
| const customerService = container.resolve("customerService") | ||
| const entityManager = container.resolve("manager") | ||
| const customerId = data[Aliases.Cart].customer_id |
There was a problem hiding this comment.
question: do you think we should use the alias here? in theory it is more for the consumer since here we know what we have access to with the typings as well. data.cart
| } | ||
|
|
||
| type HandlerInputData = { | ||
| cart: { |
There was a problem hiding this comment.
suggestion: should we expect cart only with all the data such as
type HandlerInputData = {
metadata,
address,
customer,
region
}and on the workflow you have another middleware wich takes just care of creating this object from any other objects? The idea is that this method should be reusable, so if the user already have a prepared cart object with all the required data, he shouldn't have to then split those data in different objects right?
| } | ||
|
|
||
| type HandlerInputData = { | ||
| cart: { |
There was a problem hiding this comment.
suggestion: could we just expect region_id instead? my idea is that you can use that handler to use the region id or retrieve the default, but not necessarely in the context of a cart only, wdyt?
There was a problem hiding this comment.
I think it is about the same suggestion I will have for every handler, except attach-(sales-channel/region) which looks more like a preparation and does attach the data to the cart but in fact retrieve the data. I think you should have preparation middleware instead of handlers wdyt?
| } | ||
|
|
||
| type HandlerInputData = { | ||
| createdCart: { |
There was a problem hiding this comment.
suggestion: should we just expect a cart: { id } instead?
| // thought: if an error occurs on the invoke level, it'll stop the invoke flow | ||
| // and begin the compensation flow. If another error shows up on the compensate level, | ||
| // it doesn't make sense to throw that error since the user won't be able to do anything about it. | ||
| // In these cases, we probably need to silently issue a notification to the developers that this has happened. |
There was a problem hiding this comment.
we can remove this comment. This will likely be handled at a centralized place like the Workflow manager listening to Transaction Orchestrator events to handle what has failed.
There was a problem hiding this comment.
But feel free to change that on the exportWorkflow helper to filter out errors from the compensate stage.
There was a problem hiding this comment.
Yes, that would be a good addition. Will remove the comment, it was just there to remind
| workflows: Workflows.CreateCart, | ||
| }) | ||
|
|
||
| if (isWorkflowEnabled && !productModuleService) { |
There was a problem hiding this comment.
todo: rm productModuleService and warning
| ) | ||
| } | ||
|
|
||
| if (productModuleService) { |
There was a problem hiding this comment.
todo: use the isEnabled instead
| }, | ||
| }, | ||
| }, | ||
| resultFrom: "retrieveCart", |
There was a problem hiding this comment.
todo: add to the workflowExport
| noCompensation: true, | ||
| }, | ||
| { | ||
| action: CreateCartActions.attachSalesChannel, |
| noCompensation: true, | ||
| }, | ||
| { | ||
| action: CreateCartActions.attachRegion, |
|
|
||
| export async function findOrCreateAddresses({ | ||
| container, | ||
| context, |
| }: WorkflowArguments<HandlerInputData>): Promise<CustomerDTO> { | ||
| const customerDTO: CustomerDTO = {} | ||
| const customerService = container.resolve("customerService") | ||
| const entityManager = container.resolve("manager") |
| data, | ||
| }: WorkflowArguments<HandlerInputData>): Promise<void> { | ||
| const cartService = container.resolve("cartService") | ||
| const entityManager = container.resolve("manager") |
What:
RESOLVES CORE-1461