Skip to content
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

Updating order through API returns order to ADDRESS state #8569

Open
victortorrescosta opened this issue Jan 31, 2018 · 4 comments
Open

Updating order through API returns order to ADDRESS state #8569

victortorrescosta opened this issue Jan 31, 2018 · 4 comments
Labels

Comments

@victortorrescosta
Copy link

victortorrescosta commented Jan 31, 2018

When updating an order through the OrderUpdateAPI, using Spree 3.2.7.

https://guides.spreecommerce.org/api/checkouts.html#updating-an-order

It causes the order to return to the ADDRESS state. Since there is no documentation regarding this, I would like to know if it's intended or a bug.

Context

I would like to use order update API to update order contents without necessarily changing state.
For example: when in PAYMENT state, I would like to change my address (shipping rate fixed) but would like the order continue in the PAYMENT state. Or while in PAYMENT state, I would like to change my payment method to a different one before continuing.

The problem is that regardless of what I updated and of the state of the order, using the OrderUpdateAPI always causes the order to return to the ADDRESS state automatically.

Expected Behavior

The documentation is not clear, but I expect the order state to remain the same when an update does not contain anything critical.

Actual Behavior

Upon update, the order returns to ADDRESS state.

Possible Fix

I did some digging but all I could find is this, which mentions that order would transition to address state on confirm failure.

https://github.com/spree/spree/blob/master/core/app/models/spree/order.rb#L212

But I could not find any traces in the source to indicate where this is done.

Steps to Reproduce

  1. Create an order
  2. Add LineItems and move order to DELIVERY state
  3. Add Addresses and move order to PAYMENT state
  4. Use the OrderUpdateAPI PUT /api/v1/orders/:number.json?order_token=abcdef123456 to insert a payment method to the order without advancing it to the next.
  5. You will see that the order is now back to the ADDRESS state.

Your Environment

@saravanak
Copy link
Contributor

saravanak commented Feb 8, 2018

I have added a repo which replicates the case of updating the address while the order is in state PAYMENT

Observations:

  1. The checkout and order api can both be used for updating an order. I think one of this should be disabled (preferably order api).

  2. For the case of updating the address while in the payment state, the checkout api returns an error(~requires payment info - this behavior is correct IMO - we need to take the state back to address and then update the address, which is a valid and safe flow. ), while the order method changes the state the address.

  3. The checkout api is driven by the statemachine and it handles domain logic better, which is another argument against the order api.

EDIT
Another approach can be to make order api delegate to checkout api to maintain backwards compatibility.

If we need to disable the order api, I am all game for a PR.

@victortorrescosta
Copy link
Author

@saravanak thank you so much!

Do you know if I can use the checkout API to change the contents of an order without advancing the state? For example if I am in payment state and then would like to include payment information without progressing the order.

@saravanak
Copy link
Contributor

@vitutc , If you use the order API, this resetting of state happens in restart_checkout_flow from the ensure_updated_shipments method. So you can't bypass this.

The checkout api, checks this inside update:

if @order.completed? || @order.next

So we can get something like what you want by customizing the CheckoutController. From content/developer/customization/logic.md , We can customize update like this:

#app/controllers/spree/api/v1/checkouts_controller.rb
Spree::Api::V1::CheckoutsController.class_eval do
    def update
          authorize! :update, @order, order_token

          if @order.update_from_params(params, permitted_checkout_attributes, request.headers.env)
            if current_api_user.has_spree_role?('admin') && user_id.present?
              @order.associate_user!(Spree.user_class.find(user_id))
            end

            return if after_update_attributes

            # Modified here
            if params[:stagnate] and @order.completed? || @order.next
              state_callback(:after)
              respond_with(@order, default_template: 'spree/api/v1/orders/show')
            else
              respond_with(@order, default_template: 'spree/api/v1/orders/could_not_transition', status: 422)
            end
          else
            invalid_resource!(@order)
          end
        end
end

You could then use the stagnate param , achieving what you want. I will check and report back if this works by tomorrow.

@saravanak
Copy link
Contributor

@vitutc, there are minor changes in the approach as described above. Refer this gist.

The above replicator repo has been updated with the required specs too!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants