We already have a state machine in Order, Payments and Refunds, I believe that we should implement that in the Fulfillment too so that we can make things like: create labels for shipping, create tracking code (this depends on carrier), notify users of the state of the fulfillment by email and many other things...
'Pending' | 'Shipped' | 'Delivered' | 'Cancelled'I created a PR about this issue that is WIP, so then we can discuss how this should work.
For now, the PR just implement the state machine in Fulfillment
Thanks for taking the time to think about this issue. Some feedback from me:
Refunded, which to me seems like the concern of a Payment rather than a Fulfillment. I would propose: 'Pending' | 'Shipped' | 'Delivered' | 'Cancelled'.Nice @michaelbromley, I agree with everything you said!
I had progress in this PR #427, now in Vendure is possible to define a fulfillmentOption that can have a custom Fulfillment process like you describe. I change the states like you propose too.
My next steps will be:
@michaelbromley I am learning Angular yet, so I can't make changes in the Admin UI. I believe that won't be hard to do because will be very similar with order / payment UI. So, can you create the UI for this PR?
Hey Jonthan, this is amazing work so far. A pretty complex task you are taking on but you're definitely moving in the right direction!
Implement ShippingFulfillmentMethod class
Can you elaborate a bit on what this class is for and how it will work?
So, can you create the UI for this PR?
Yes, you just worry about the server side parts. For me the UI part should not be too much extra work.
I don't know the name and methods yet, we need to discuss that, but the main idea is to have a class similar to ShippingCalculator or PaymentMethod where user can define how he want to handle the fulfillment. We can add in this class methods like ship (executed when the Fulfillment transit beetween Pending to Shipping), stopShipping. This class should be defined in Vendure Config, and them in Admin UI, the user can config this FulfillmentMethod. Maybe we can relate the ShippingMethod with FullfilmentMethod (this is why I called ShippingFulfillmentMethod).
We have many ways to do this PR. For me, if we have a way to have state in Fulfillment and a proper way to change this state based on ShippingMethod it's what matter. What I want to achieve with this PR later, it's to create label / tracking code for order automatically based on order shipping method, change the state or the order when the package is delivered and order things related to that.
OK, I think I get the idea.
This is quite a large scope for a single PR. I'd suggest if possible to limit the initial work to establishing the underlying state machine mechanism for Fulfillments. Then in a separate PR you build on it with the rest of what you described. Otherwise it'll be harder to track all the changes.
So, can somebody else work on the admin UI in parallel? What is it expected? The states of fullfillment option can be transitioned from the UI such as in "Orders" case?
@dimeros I'd suggest that the server-side groundwork be completed first before anyone starts on the Admin UI aspect. I'm still not 100% clear on @jonyw4's plans on the design, so I'm just waiting to see any further commits to the PR #427
@michaelbromley, I create another issue just to discuss custom fulfillment methods.
Here in this issue and in the #427 PR I will discuss just add a state machine in Fulfillment Method for maintaining the current scope
@dimeros About the admin UI transition, I think that we can have a button like Refunds and Payment to transit between the Fulfillment state. And then we could implement a class to define what the fulfillment should do when transiting to a specific state. I create the #447 to discuss that
Sorry, I realize that if we will have CustomFulfillmentProcess, we will not need to implement the issue #447.
Hey @michaelbromley, today in Order we have Fulfilled and PartiallyFulfilled but with creation of new Fulfillment states I thought in maybe change the defaults states in Order to cover these new states.
Using PaymentMethod as example and the current default Order states, we can have these new states:
ShippedTo reach this state all Fulfillment must be in Shipped state.
PartiallyFulfilled _(or maybe change to PartiallyDelivered)_To reach this state, the Order must have at least one Fulfillment in Delivered state. We can also remove this state.
Fulfilled _(or maybe change to Delivered)_To reach this state all Fulfillment must be in Delivered state.
I think that we should change the names of the old states to be more accurate, but I don't know if this could break something in Vendure...
@jonyw4 yes, I agree with all of the above. Also changing the state names to "delivered". Yes, it'll be a breaking change but hopefully TypeScript will help to avoid uncaught breakages.
Okay I will change that in the PR.
@michaelbromley I little update here, when I started to update the order e2e tests I realize that we will need PartiallyShipped order state too.
Today If you forget to fulfill an order item, the order state will continue in PartiallyFulfilled. We need to achieve that in this PR too. So the new order states must be that:
PartiallyShippedTo reach this state, the Order must have at least one Fulfillment in Shipped state.
ShippedTo reach this state all order items must have a Fulfillment and all Fulfillment must be in Shipped state
PartiallyDelivered (old PartiallyFulfilled)To reach this state, the Order must have at least one Fulfillment in Delivered state.
Delivered (old Fulfilled)To reach this state all order items must have a Fulfillment and all Fulfillment must be in Delivered state
The PR implementing the server-side of this is now merged into next.
Tasks outstanding: