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

Edit path for completed orders #2078

Closed
brchristian opened this issue Jul 11, 2017 · 5 comments · Fixed by #3391
Closed

Edit path for completed orders #2078

brchristian opened this issue Jul 11, 2017 · 5 comments · Fixed by #3391

Comments

@brchristian
Copy link
Contributor

If I visit the path of a completed order, e.g., http://localhost:3000/orders/R465318167, I can see a receipt of the order. If I then append edit to that, and navigate to http://localhost:3000/orders/R465318167/edit, I am shown the Cart page.

This is unexpected behavior, IMO. I’d expect to either be able to edit the order (i.e., change address or cancel or something like that), or to be told that I can’t edit this order because it’s locked or finalized, etc.

But seeing the current_order Cart page rendered at the /edit URL of a completely different order is definitely not expected!

@solidusio solidusio deleted a comment from jasonfb Jul 14, 2017
@mamhoff
Copy link
Contributor

mamhoff commented Jul 14, 2017

@jasonfb There was a comment here that was almost impossible to read, and I'm not even sure it wasn't a spam filter overreacting. Would you mind posting your answer again?

@kennyadsl
Copy link
Member

I think we should just put a flash message that inform the user that the order is not editable.

@JDutil
Copy link
Contributor

JDutil commented Oct 11, 2018

@kennyadsl was just looking at this for Hacktoberfest, and I think this may already be resolved in that I can't reproduce that behavior on master. I don't see any flash message it just redirects me to the shipments not the cart.

@kennyadsl
Copy link
Member

If possible I think it would be just easier to exclude the edit action from resources :orders. What do you think about this approach?

It makes sense for me. 👍

I think that route is there since there are stores that allow changing orders info after purchase but they can just add that route back and customize Spree::OrdersController#edit.

@JDutil
Copy link
Contributor

JDutil commented Oct 23, 2018

I don’t think it’s an easier approach although I did like the idea.

I just briefly tried removing the controller action and route, and it effected a lot of places currently using the route helper. I imagine many extensions or applications are still using the route as well.

The simpler solution would be the flash message and redirect even if it’s not as nice.

How exactly did you reproduce this issue? Perhaps I’m missing something, but I didn’t see the problem when I tried. My orders must be in a different state than expected to reproduce the issue.

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

Successfully merging a pull request may close this issue.

4 participants