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.

Sign up for GitHub

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

Jump to bottom

Handle soft deleted variants in order edit #3938

Merged
merged 1 commit into from
Jul 22, 2019
Merged

Handle soft deleted variants in order edit #3938

merged 1 commit into from
Jul 22, 2019

Conversation

sauloperez
Copy link
Contributor

What? Why?

Closes #3903

This allows us to test openfoodfoundation/spree#41. If it passes testing I will then revert it back to the 2-0-4-stable branch but updating the target commit in the Gemfile.lock.

What should we test?

  1. Delete a product that it's been purchased in an existing order
  2. Go edit said order from /admin

You should see the page, including the deleted product, without any problem.

Release notes

Orders listing products that got deleted after the purchase can still be edited from the admin section

Changelog Category: Fixed

@sauloperez sauloperez self-assigned this Jun 14, 2019
@sauloperez sauloperez mentioned this pull request Jun 14, 2019
Merged
@sauloperez
Copy link
Contributor Author

Moving to test ready as the underlying openfoodfoundation/spree#41 got 2 approvals and this is only meant for testing now.

@RachL RachL self-assigned this Jun 19, 2019
@RachL RachL added pr-staged-es pr-staged-fr staging.coopcircuits.fr and removed pr-staged-es pr-staged-fr staging.coopcircuits.fr labels Jun 19, 2019
@Matt-Yorkley
Copy link
Contributor

@sauloperez This has failed with a fatal error multiple times when being staged...

Relevant error lines:

bundle exec rake assets:precompile:primary RAILS_GROUPS=assets RAILS_ENV=staging"]
...
"rake aborted!", "Sass::SyntaxError: Invalid CSS after \"\": expected selector, was \"@-webkit-keyfra...\"
in /home/openfoodnetwork/apps/openfoodnetwork/current/app/assets/stylesheets/application.css.scss"

@Matt-Yorkley
Copy link
Contributor

Matt-Yorkley commented Jun 20, 2019 •
edited
Loading

This PR is not testable as it seems to break asset precompile and can't be staged.

Moving back to dev ready as Pau is away, if anyone wants to have a go at it before he gets back.

@Matt-Yorkley Matt-Yorkley removed their assignment Jul 4, 2019
@sauloperez sauloperez force-pushed the get-last-spree-fork-patch branch 2 times, most recently from d81a040 to fa37a39 Compare July 16, 2019 14:05
@RachL
Copy link
Contributor

RachL commented Jul 19, 2019

Deployment failed. As discussed with Pau on Slack, moving back to in dev

https://semaphoreci.com/openfoodfoundation/openfoodnetwork-2/servers/katuma-staging/deploys/90

@sauloperez sauloperez force-pushed the get-last-spree-fork-patch branch from fa37a39 to 01f77ea Compare July 22, 2019 08:48
@luisramos0
Copy link
Contributor

I removed the accidental sass upgrade.

all good now in staging katuma, back to test ready ;-)

@sauloperez
Update Gemfile to get spree from a feature branch
be4fbc4 
This allows us to test
openfoodfoundation/spree#41. If it passes
testing I will then revert it back to the `2-0-4-stable` branch but
updating the target commit in the Gemfile.lock.
@sauloperez sauloperez force-pushed the get-last-spree-fork-patch branch from bdbf651 to be4fbc4 Compare July 22, 2019 13:41
@sauloperez
Copy link
Contributor Author

Thanks @luisramos0 ! I removed sass from the change.

@RachL
Copy link
Contributor

RachL commented Jul 22, 2019

Staging worked and now when I delete a product I still can access the previous orders, both as super admin and manager.

Two examples:

https://staging.katuma.org/admin/orders/R742456565/edit

https://staging.katuma.org/admin/orders/R622102456/edit

Ready to go!

@luisramos0 luisramos0 merged commit e98b896 into master Jul 22, 2019
@luisramos0 luisramos0 deleted the get-last-spree-fork-patch branch July 22, 2019 20:05
@luisramos0 luisramos0 mentioned this pull request Jul 22, 2019
Merged
@luisramos0
Copy link
Contributor

I was too quick to merge, this was not supposed to be merged... I reverted it in #4061 sorry!

@luisramos0 luisramos0 restored the get-last-spree-fork-patch branch July 22, 2019 20:08
@sauloperez sauloperez deleted the get-last-spree-fork-patch branch July 23, 2019 12:42
@sauloperez
Copy link
Contributor Author

sauloperez commented Jul 23, 2019 •
edited
Loading

Yes, sorry this PR and its Spree counterpart are a bit confusing. I knew this would happen.

@sauloperez sauloperez mentioned this pull request Jul 23, 2019
Merged
@luisramos0
Copy link
Contributor

I just merged this PR and reverted it immediately. Nothing else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@sauloperez sauloperez

@RachL RachL

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

When deleting a product, all orders dealing with that product show an error 500
4 participants
@sauloperez @Matt-Yorkley @RachL @luisramos0