-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
feat(core-flows,medusa,order,types): update orders #10373
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
6 Skipped Deployments
|
|
} | ||
}) | ||
|
||
const change = createOrderChangeStep(orderChangeInput) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self-comment: should we move the next 4 calls to an order service method like registerOrderChange
that would create a dummy change record in case we wanna note that some generic change occurred on an order.
Method would:
- create already confirmed order change
- create a generic action for the change that is already "applied"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a good point. think it would be good to have this generic method on the order class.
That could be used for general "comments" on the order as well.
import { setActionReference } from "../set-action-reference" | ||
|
||
OrderChangeProcessing.registerActionType( | ||
ChangeActionType.CHANGE_SHIPPING_ADDRESS, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought: I would be curious to hear @carlos-r-l-rodrigues's thoughts on creating a more generic action, that can be used for updates that does not have any validation and where the operation is basically just updating the property on the order.
For example:
OrderChangeProcessing.registerActionType(
ChangeActionType.ORDER_UPDATE,
{
operation({ action, currentOrder, options }) {
/**
* NOOP: used as a reference for the change
*/
setActionReference(currentOrder, action, options)
},
validate({ action }) {
/* noop */
},
}
)
The order change action generated for this type of update would then be:
Shipping address
const actionInput = transform(
{ order, input, change },
({ order, input, change }) => [
{
order_change_id: change.id,
order_id: input.order_id,
action: ChangeActionType.ORDER_UPDATE,
version: change.version,
reference: "shipping_address"
applied: true,
reference_id: order.shipping_address?.id,
details: input.shipping_address as Record<string, unknown>,
},
]
)
const actionInput = transform(
{ order, input, change },
({ order, input, change }) => [
{
order_change_id: change.id,
order_id: input.order_id,
action: ChangeActionType.ORDER_UPDATE,
version: change.version,
reference: "email"
applied: true,
reference_id: order.email,
details: { email: input.email },
},
]
)
Billing address
const actionInput = transform(
{ order, input, change },
({ order, input, change }) => [
{
order_change_id: change.id,
order_id: input.order_id,
action: ChangeActionType.ORDER_UPDATE,
version: change.version,
reference: "billing_address"
applied: true,
reference_id: order.billing_address?.id,
details: input.billing_address as Record<string, unknown>,
},
]
)
Perhaps, a question to ask: Do we gain anything from having the specific actions? If not, we made as well save ourselves from having multiple actions for these updates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, that action accepts anything, even if there is no "action" associated with it on the Order module.
So, it is really up to us to decide. To me, an ORDER_UPDATE seems to fit well, and anything to distinguish them on the UI, can go on the details
} | ||
}) | ||
|
||
const change = createOrderChangeStep(orderChangeInput) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a good point. think it would be good to have this generic method on the order class.
That could be used for general "comments" on the order as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a neat approach. LGTM – would be good to have @carlos-r-l-rodrigues take a look at it as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice... LGTM
@fPolic, should we merge this? |
**What** - add order update endpoint - add workflows and steps for updating orders - add `registerChanges` method to Order module + workflow step --- CLOSES CMRC-633
What
registerChanges
method to Order module + workflow stepCLOSES CMRC-633