Promotion configuration #5635
Promotion configuration #5635
Conversation
github-actions
bot
added
changelog:solidus_api
mamhoff
force-pushed
the
promotion-configuration
branch
from
6ffe4c0 to
0dc0086
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5635 +/- ##
==========================================
+ Coverage 88.58% 88.62% +0.03%
==========================================
Files 687 688 +1
Lines 16453 16490 +37
==========================================
+ Hits 14575 14614 +39
+ Misses 1878 1876 -2 ☔ View full report in Codecov by Sentry. |
|
Also, I'm very open to comments here. Some of the choices I'm not so sure about are:
|
I prefer that. |
…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.
This implements deprecated setters and getters for the promotion-specific preferences on `Spree::AppConfiguration`. Now, if a developer wants to set their
The contents of this class are promotion-related and should move to `Spree::PromotionConfiguration`. We can also use the default option of the `add_class_set` method here. There were originally no direct specs for `Spree::Core::Environment::Promotions`. I'm not adding any, because the class is indirectly tested through `Spree::AppConfiguration` and now only has deprecated methods.
mamhoff
force-pushed
the
promotion-configuration
branch
2 times, most recently
from
982ce6f to
e494180
Compare
It's working!
It seems we want to be able to test files without loading all of Solidus. Unfortunately, we do load Solidus' configuration, and this PR needs the configuration object to be able to deprecate extension points. What this commit does is it allows loading a module that defined `Spree.deprecator` and loads that module in both `spree/core.rb` and the `spec_helper.rb`.
mamhoff
force-pushed
the
promotion-configuration
branch
from
cd7fdef to
4b4f8ea
Compare
There was a problem hiding this comment.
0dc0086. Should this really live in the promotions conf object, or in the main Solidus one, and the promo extensions just use it? Seems generic enough.
Let's go as is for now, I don't know either buy maybe with the rest of the implementations we will have a better picture, and it's "easy" to move.
Summary
This extracts the remaining promotion-related things from
Spree::AppConfigurationandRails.application.config.spreeand puts them intoSpree::Core::PromotionConfiguration.Spree::Core::PromotionConfigurationwill become the blueprint for a configuration class for a promotion system.#5634 depends on this or comparable work, because we can't reference classes from
Spree::AppConfigurationthat aren't defined insidesolidus_coreor its direct dependencies.I'm extracting a few commits that used to be in here into their own PR because they don't move existing endpoints.
In a nutshell: All promotion configuration is now found under
Spree::Config.promotions, instead of under eitherSpree::ConfigorRails.application.config.spree.promotionsorRails.application.config.spree.calculators. Yay!Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: