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

Prepare for Rails 5.2 #7421

Merged
merged 10 commits into from
Apr 19, 2021

Conversation

andrewpbrett
Copy link
Contributor

@andrewpbrett andrewpbrett commented Apr 14, 2021

What? Why?

More preparation for merging Rails 5.2

What should we test?

Let's do a quick test: submitting an order and checking the payment state and shipment state are okay. Basically checking for any regressions related to this issue: #7003

Release notes

Updated code to prepare for Rails 5.2

Changelog Category: Technical changes

Dependencies

Documentation updates

Matt-Yorkley and others added 10 commits April 14, 2021 09:14
Leaving the object with unpersisted changes breaks order locking with this error (in various places):

RuntimeError:
        Locking a record with unpersisted changes is not supported. Use `save` to persist the changes, or `reload` to discard them explicitly.
This guard clause was returning an instance of an unpermitted Params object (containing a blank hash) here, which was causing unexpected results in various places. Returning a blank hash explicitly resolved the issue.

Fixes:

4) Admin::OrderCyclesController create as a manager of a shop when creation is successful returns success: true and a valid edit path
     Failure/Error:
       @order_cycle_params ||= PermittedAttributes::OrderCycle.new(params).call.
         to_h.with_indifferent_access

     ActionController::UnfilteredParameters:
       unable to convert unpermitted parameters to hash
     # ./app/controllers/admin/order_cycles_controller.rb:245:in `order_cycle_params'
     # ./app/controllers/admin/order_cycles_controller.rb:44:in `create'
     # ./spec/support/controller_requests_helper.rb:49:in `process_action_with_route'
     # ./spec/support/controller_requests_helper.rb:27:in `spree_post'
     # ./spec/controllers/admin/order_cycles_controller_spec.rb:124:in `block (5 levels) in <module:Admin>'
…ntroller, params are now sent inside a Parameters object, not as a hash
In the test setups here order.payments is empty
@codecov
Copy link

codecov bot commented Apr 14, 2021

Codecov Report

Merging #7421 (265cf84) into master (76112fc) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 265cf84 differs from pull request most recent head fa14a80. Consider uploading reports for the commit fa14a80 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master    #7421   +/-   ##
=======================================
  Coverage   93.07%   93.07%           
=======================================
  Files         633      633           
  Lines       18145    18147    +2     
=======================================
+ Hits        16889    16891    +2     
  Misses       1256     1256           
Impacted Files Coverage Δ
app/models/customer.rb 100.00% <100.00%> (ø)
app/models/spree/payment.rb 99.08% <100.00%> (ø)
app/services/cart_service.rb 100.00% <100.00%> (ø)
app/services/permitted_attributes/enterprise.rb 100.00% <100.00%> (ø)
app/services/permitted_attributes/order_cycle.rb 100.00% <100.00%> (ø)
app/services/permitted_attributes/subscription.rb 100.00% <100.00%> (ø)
lib/spree/core/controller_helpers/order.rb 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 76112fc...fa14a80. Read the comment docs.

@andrewpbrett andrewpbrett requested a review from sauloperez April 14, 2021 17:21
@filipefurtad0 filipefurtad0 self-assigned this Apr 19, 2021
@filipefurtad0 filipefurtad0 added the pr-staged-au staging.openfoodnetwork.org.au label Apr 19, 2021
@filipefurtad0
Copy link
Contributor

Hey @andrewpbrett ,

Tested orders using:

  • cash
  • Stripe-SCA

both on the backoffice and shopfront.

Spotted nothing regarding order states; all as before.

Ready to go 🚀

@filipefurtad0 filipefurtad0 removed the pr-staged-au staging.openfoodnetwork.org.au label Apr 19, 2021
@andrewpbrett andrewpbrett merged commit aca19ed into openfoodfoundation:master Apr 19, 2021
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 this pull request may close these issues.

5 participants