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

Legacy Promotions: Fix shipping promo application after state machine reload #5797

Merged

Conversation

mamhoff
Copy link
Contributor

@mamhoff mamhoff commented Jun 19, 2024

Summary

This fixes a flaky spec in solidus_starter_frontend. Let me explain: solidus_legacy_promotions adds a

before_transition to: :delivery, :apply_shipping_promotions

The specs for solidus_starter_frontend include one that tests correctly applied shipping promotions.

But another spec in solidus_starter_frontend reloads the state machine using the following mechanism:

@old_checkout_flow = Spree::Order.checkout_flow
# modification to state machine here
Spree::Order.checkout_flow(&@old_checkout_flow)

What this PR does is add the apply_shipping_promotion before-transition hook directly to the default state machine, rather than evaling it into the order class, where it does not survice reloads of the checkout flow.

Notice I had to give the implementation method an argument, because
RSpec stubs do not have the correct `arity`; and `state_machines` will
send the event, resulting in an ArgumentError otherwise.
Some extensions rely on the default state machine being fully
reloadable, and as long as we use solidus_legacy_promotions, the
before_transition method `:apply_shipping_promotions` must survive a
reload of the state machine.

Rather than patching Spree::Order and adding the transition here, we add
it where all the other default transitions are added: In the class
methods of the order state machine.
@mamhoff mamhoff requested a review from a team as a code owner June 19, 2024 12:23
@github-actions github-actions bot added the changelog:repository Changes to the repository not within any gem label Jun 19, 2024
Copy link

codecov bot commented Jun 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.92%. Comparing base (e582263) to head (dbcf66d).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5797   +/-   ##
=======================================
  Coverage   88.92%   88.92%           
=======================================
  Files         718      719    +1     
  Lines       16939    16944    +5     
=======================================
+ Hits        15063    15068    +5     
  Misses       1876     1876           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tvdeyen
Copy link
Member

tvdeyen commented Jun 19, 2024

Specs related only. Merging

@tvdeyen tvdeyen merged commit ebe0d08 into solidusio:main Jun 19, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:repository Changes to the repository not within any gem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants