feat(dashboard): order fulfillment UI #7262
Conversation
fPolic
added
area: admin
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Ignored Deployments
|
🦋 Changeset detectedLatest commit: 78924e0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 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 |
fPolic
changed the title
|
Does it make sense to have the link between Order and Fulfillment as a real link and not create |
Yes, agree! I wasn't sure what was originally planned regarding Fulfillement<>Order link so I added this as a temp solution to have E2E functionality but the flow should be revisited anyways and params need to be revisited as well. I can push the change tomorrow (or we can change this when we update the workflow). EDIT: Opened a ticket in linear to revisit this relationship since there seems to be |
…p to the workflow
...reate-fulfillment/components/order-create-fulfillment-form/order-create-fulfillment-form.tsx
Show resolved
Hide resolved
...reate-fulfillment/components/order-create-fulfillment-form/order-create-fulfillment-item.tsx
Outdated
Show resolved
Hide resolved
packages/core/types/src/workflow/fulfillment/create-fulfillment.ts
Outdated
Show resolved
Hide resolved
| @@ -23,13 +23,15 @@ export type AdminCancelFulfillmentType = z.infer<typeof AdminCancelFulfillment> | |||
| export const AdminCancelFulfillment = z.object({}) | |||
|
|
|||
| export type AdminCreateFulfillmentType = z.infer<typeof AdminCreateFulfillment> | |||
| // TODO: revisit the data shape this endpoint accepts | |||
There was a problem hiding this comment.
question: What do we need to change?
There was a problem hiding this comment.
- Accept either
orderororder_id - We are missing notification flag
- Should the address be infered from the order instead of passing one?
- Same thing for
provider_id, isn't shipping options/provider_id already set for the order?
packages/modules/link-modules/src/definitions/order-fulfillment.ts
Outdated
Show resolved
Hide resolved
| sku: item.variant_sku || "", | ||
| } | ||
| }), | ||
| // TODO: should be optional in the enpoint? |
There was a problem hiding this comment.
Yeah, we should allow these to be manually passed. If you're using a provider, it will be responsible for the labels, but in some cases you would like to manually fulfill orders.
| @@ -180,4 +180,7 @@ export type CreateFulfillmentWorkflowInput = { | |||
| * The associated fulfillment order. | |||
| */ | |||
| order: CreateFulfillmentOrderWorkflowDTO | |||
|
|
|||
| // TODO: revisit - either remove `order_id` or `order` | |||
| order_id: string | |||
There was a problem hiding this comment.
thought(non-blocking): I think we should call order something different. It is really just additional context that you can pass upon creating a fulfillment. But not super important for this PR.
| args: { | ||
| methodSuffix: "Fulfillments", | ||
| }, | ||
| isList: true, |
There was a problem hiding this comment.
question: Shouldn't this be in the extends?
There was a problem hiding this comment.
Not sure if this was already changed. But the file looks ok.
# Conflicts: # packages/admin-next/dashboard/src/lib/client/client.ts
|
|
||
| // TODO call the createReturn from the fulfillment provider | ||
|
|
||
| const link = transform( |
There was a problem hiding this comment.
comment: removed from here since we link order <> fulfillment in the createFulfillmentWorkflow called aabove
WHAT
order_idon Fulfillment but this might changeScreen.Recording.2024-05-07.at.18.20.00.mov