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

[Spree 2 Upgrade] Soft delete shipping methods and enable destroys #2126

Closed
1 of 3 tasks
daniellemoorhead opened this issue Mar 9, 2018 · 4 comments
Closed
1 of 3 tasks

Comments

@daniellemoorhead
Copy link
Contributor

As per the detail in PR #1659:

This removes the "hack" that forbids destroying shipping methods from /admin and makes use of Spree's soft delete implementation (deleted_at column in spree_shipping_methods).

This way admins can disable a particular shipping method, so no one uses it anymore, while still showing the shipping method for past orders in reports.

TODO

  • remove N+1
  • Check /admin/orders and /accounts
  • QA order_cycle_management and customers reports: They must work like now and shipping_methods should be deletable.

How is this related to Spree upgrade?

You are right to ask. app/controllers/spree/admin/shipping_methods_controller_decorator.rb below references the column shipping_method_id from the spree_orders table and this won't be longer available in Spree 2.0 aka. step 7.

By shipping this simple change into a release before the step 7 we reduce the chance of introducing bugs and more important, we make that actual step 7 release more lightweight, which people testing will thank.

@sauloperez
Copy link
Contributor

👏

@sauloperez
Copy link
Contributor

Do you mind if I close this issue, which just copies the PR's description, and we stick to #2010 instead? that one is already linked to its parent epic #2011.

@luisramos0 luisramos0 changed the title Soft delete shipping methods and enable destroys [Spree 2 Upgrade] Soft delete shipping methods and enable destroys Aug 24, 2018
@luisramos0
Copy link
Contributor

Closing this as it is a duplicate of #2010 (please correct me if I am wrong).

@sauloperez
Copy link
Contributor

I'm not sure. There is a discussion in #1659 about it and the conclusion is not entirely clear to me (I just skimmed through the text).

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

No branches or pull requests

3 participants