Extract Legacy Promotion System: Move ActiveRecord Models and Factories #5634
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
changelog:solidus_api
mamhoff
force-pushed
the
extract-legacy-promotions
branch
2 times, most recently
from
bcaff25
to
be37df1
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5634 +/- ##
==========================================
+ Coverage 88.92% 88.94% +0.01%
==========================================
Files 719 724 +5
Lines 16944 16974 +30
==========================================
+ Hits 15068 15097 +29
- Misses 1876 1877 +1 ☔ View full report in Codecov by Sentry. |
mamhoff
added a commit
to mamhoff/solidus
that referenced
this pull request
Jan 29, 2024
We want to be able to move all promotion-related things out of `solidus_core` in PR solidusio#5634. However, Spree::AppConfiguration also does all the promotion-specific things, and we can't move the app configuration out of core. So what this does is it introduces a new configuration object containing those attributes of the core app_configuration class and moves them into a `promotion_configuration` object. This object is now available as Spree::Promotion::Config.
|
Depends on #5635 |
|
Thanks for your work on this @mamhoff. Now to find time to actually review it properly... 😅 |
tvdeyen
reviewed
Feb 2, 2024
This was referenced Feb 3, 2024
mamhoff
added a commit
to mamhoff/solidus
that referenced
this pull request
Feb 17, 2024
…ration We want to be able to move all promotion-related things out of `solidus_core` in PR solidusio#5634. However, Spree::AppConfiguration also does all the promotion-specific things, and we can't move the app configuration out of core. In solidusio#5658, we've added a promotion configuration object as a nucleus for core's promotion system's configuration, `Spree::Core::PromotionConfiguration`. This implements all the promotion-specific configuration preferences from `Spree::AppConfiguration` there.
mamhoff
added a commit
to mamhoff/solidus
that referenced
this pull request
Feb 18, 2024
…ration We want to be able to move all promotion-related things out of `solidus_core` in PR solidusio#5634. However, Spree::AppConfiguration also does all the promotion-specific things, and we can't move the app configuration out of core. In solidusio#5658, we've added a promotion configuration object as a nucleus for core's promotion system's configuration, `Spree::Core::PromotionConfiguration`. This implements all the promotion-specific configuration preferences from `Spree::AppConfiguration` there.
spaghetticode
pushed a commit
to nebulab/solidus
that referenced
this pull request
Feb 26, 2024
…ration We want to be able to move all promotion-related things out of `solidus_core` in PR solidusio#5634. However, Spree::AppConfiguration also does all the promotion-specific things, and we can't move the app configuration out of core. In solidusio#5658, we've added a promotion configuration object as a nucleus for core's promotion system's configuration, `Spree::Core::PromotionConfiguration`. This implements all the promotion-specific configuration preferences from `Spree::AppConfiguration` there.
mamhoff
force-pushed
the
extract-legacy-promotions
branch
2 times, most recently
from
f57fc58
to
ba90efe
Compare
tvdeyen
reviewed
Feb 27, 2024
legacy_promotions/db/migrate/20231031175215_add_promotion_order_promotion_foreign_key.rb
Outdated
Show resolved
Hide resolved
legacy_promotions/spec/mailers/spree/promotion_code_batch_mailer_spec.rb
Outdated
Show resolved
Hide resolved
legacy_promotions/db/migrate/20160101010001_solidus_one_four_promotions.rb
Outdated
Show resolved
Hide resolved
legacy_promotions/db/migrate/20161017102621_create_spree_promotion_code_batch.rb
Outdated
Show resolved
Hide resolved
legacy_promotions/app/decorators/solidus_legacy_promotions/models/spree/order_decorator.rb
Outdated
Show resolved
Hide resolved
mamhoff
force-pushed
the
extract-legacy-promotions
branch
2 times, most recently
from
14d959e
to
679f6b6
Compare
This was referenced Feb 28, 2024
mamhoff
force-pushed
the
extract-legacy-promotions
branch
2 times, most recently
from
8b1a5f9
to
4525f8b
Compare
mamhoff
changed the title
kennyadsl
reviewed
Jun 18, 2024
legacy_promotions/db/migrate/20160101010001_solidus_one_four_promotions.rb
Outdated
Show resolved
Hide resolved
mamhoff
force-pushed
the
extract-legacy-promotions
branch
5 times, most recently
from
800900b
to
3a194ce
Compare
`Spree::PromotionAction` is not in core any longer, and the null promo adjuster can not actually produce any adjustments.
mamhoff
force-pushed
the
extract-legacy-promotions
branch
from
3a194ce
to
2c2e371
Compare
Since these promotions will be installed into all stores, we need to make sure they don't do anything to the existing promotion system. For new stores, they must do the things though.
We need to purge references to Solidus' legacy promotion system from the core.
Except for the flat rate calculator, as that one is so generic it's often just used as a stand-in calculator.
In legacy_promotions, we'll just keep them in `app`, where they're tested, too.
We don't need #recalculate for anything in core, and we don´t need the association to promotion codes.
I'm keeping the spec to test what the model does, but moving the extended spec with the actual promotion to solidus_legacy_promotions.
mamhoff
force-pushed
the
extract-legacy-promotions
branch
from
2c2e371
to
ee5a91d
Compare
This should help everyones test suite's to stay green.
mamhoff
force-pushed
the
extract-legacy-promotions
branch
from
ee5a91d
to
a78e28d
Compare
|
@kennyadsl are we ready to go? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This extracts the legacy promotion system into a gem that lives within the solidus codebase. I've talked about this with @kennyadsl as a way to integrate a new solidus_promotions gem later, modeled after solidus_friendly_promotions.
This is the final piece of the extraction process, after many merged PRs. Thank you to @kennyadsl , @elia , @jarednorman , @adammathys and @tvdeyen for many reviews. This is the final one, after that we'll have managed to extract all of the promotion system into
solidus_legacy_promotions.In this PR, we move the ActiveRecord models and specs that are part of the legacy promotion system into
solidus_legacy_promotions.Checklist
The following are mandatory for all PRs: