Allow setShippingMethod order states to be configurable

[JamesLAllen]

Is your feature request related to a problem? Please describe. setShippingMethod currently asserts that the order is in the AddingItems order state, which removes our ability to change the shipping in our own custom states.

Describe the solution you'd like It would be ideal to be able to define where we want to enable and disable order transactions such as setShippingMethod. For instance, if we could define our custom OrderProcess to override the default and move the shipping method to a new custom state such as ArrangingShipping in the Vendure config.

Describe alternatives you've considered The current workaround is just copying the existing implementation and overriding the existing resolver or creating a new mutation that replaces const validationError = this.assertAddingItemsState(order); with something like const validationError = order.state === 'ArrangingShipping' || order.state === 'ArrangingPayment'

Additional context Quick fix would just be a simple flag similar to the new arrangingPaymentRequiresCustomer:

      configureDefaultOrderProcess({
        arrangingPaymentRequiresCustomer: false,
        allowSetShippingInAnyState: true,
      }),

Or a longer yet more customizable option could allow the developer to add order-specific transactions to their custom OrderStates:

transitions: {
    AddingItems: {
      to: ['ApprovalPending'],
      mergeStrategy: 'replace',
      allowSetShipping: false,
    },
    ApprovalPending: {
      to: ['ArrangingShipping', 'AddingItems'],
      mergeStrategy: 'replace',
      allowSetShipping: false,
    },
    ArrangingShipping: {
      to: ['ArrangingPayment'],
      mergeStrategy: 'replace',
      allowSetShipping: true,
    },
  },

[michaelbromley]

Hi James,

So I've been trying to figure out what the high-level, more abstract problem here is. The assertAddingItemsState() method is used a few times in the OrderService:

• addItemToOrder
• adjustOrderLine
• removeItemFromOrder
• removeAllItemsFromOrder
• setShippingMethod

All of the above actions will change the price of an Order and potentially affect promotions, taxes and shipping. I think the basic purpose is to ensure that the Order is in a state where it is safe to modify these things. I.e. we don't want to modify the contents of an order that is in the PaymentSettled state.

The idea of "ok to modify" seems related to the Order.active boolean property, which in turn is controlled by the OrderPlacedStrategy. However, the difference is that by default, the ArrangingPayment state is not yet "placed", but we also typically don't want to allow order modifications in this state.

So I'm thinking that we need to be able to define a function that takes the current order and returns a boolean of whether the contents may be modified. This could perhaps be incorporated into the OrderProcess interface.

So eg.

export interface OrderProcess<State extends keyof CustomOrderStates | string> extends InjectableStrategy {
    transitions?: Transitions<State, State | OrderState> & Partial<Transitions<OrderState | State>>;
    onTransitionStart?: OnTransitionStartFn<State | OrderState, OrderTransitionData>;
    onTransitionEnd?: OnTransitionEndFn<State | OrderState, OrderTransitionData>;
    onTransitionError?: OnTransitionErrorFn<State | OrderState>;

    // proposal
    canModifyOrder()?: (ctx: RequestContext, order: Order): boolean | Promise<boolean>;
}

where the default implementation would be:

class DefaultOrderProcess {
  // ..

  canModifyOrder(ctx, order) {
    return order.state === 'ArrangingPayment' || order.state === 'Draft';
  }

This would then allow you to provide a custom OrderProcess that simply adds:

const myOrderProcess: OrderProcess<'ArrangingShipping'> = { 
  transitions: { /* your custom transitions */ },
  canModifyOrder: (ctx, order) => order.state === 'ArrangingShipping',
}

Internally, Vendure would loop through the configured OrderProcess objects and if at least 1 of the canModifyOrder methods evaluates to true, then the modification is allowed.

Does that seems like a reasonable solution?

[JamesLAllen]

I like this, yeah. canModifyOrder is state-agnostic and can be subject to any rules someone might need.

[JamesLAllen]

Thinking about it further, and I realize this probably isn't a huge deal however I think keeping the assert might be more descriptive of what the overridable method does, ie assertCanModifyOrder might be a good replacement for assertAddingItemsState.