Refactor Promotions Environment Configuration #3239
Merged
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
kennyadsl
force-pushed
the
kennyadsl/improve-env-promo
branch
from
0fd60ea to
01ceb06
Compare
ericsaupe
approved these changes
Jun 26, 2019
kennyadsl
force-pushed
the
kennyadsl/improve-env-promo
branch
2 times, most recently
from
152c016 to
f4e901c
Compare
|
@eric1234 sure, I'll add this to the CHANGELOG but please keep in mind that this should be transparent for users and if they customized the deprecated class, they will receive a deprecation warning with customizations preserved. I've just added some specs around this here to be sure it works as expected: https://github.com/solidusio/solidus/pull/3239/files#diff-31874dd9132638da9df25acb9f1e3d1f |
jacobherrington
approved these changes
Jul 3, 2019
kennyadsl
force-pushed
the
kennyadsl/improve-env-promo
branch
from
f4e901c to
bac9f60
Compare
This allows to remove items from the list of constantized class for a specific preferences set.
This will be helpful to refactor this part with confidence. The important thing here is to have tests around the usage that could be done into stores and extensions, so we are sure to don't break it.
This class is needed to reflect the class/filesystem of other sub-environment classes, like Spree::Core::Environment::Calculators. They are the same thing but they are placed in two different folders without a specific reason.
Spree::Promo::Environment has been deprecated in favor of the new class that will do the exact same thing.
There's no need to redefine it as Spree::Core::Environment::Promotions since it's already that thing, see it's initializer in core/lib/spree/core/environment.rb and that's exactly what we are already doing with calculators.
kennyadsl
force-pushed
the
kennyadsl/improve-env-promo
branch
from
bac9f60 to
cfada51
Compare
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.
Description
🚨This PR lives on top of #3238 and should be merged after that one. 🚨-> Merged and rebased.At the moment, there are some small issues with how
spree.config.promotionsis defined.First of all, it's an "
Environment" object, likeSpree::Core::Environment::Calculators(both include theEnvironmentExtensionconcern), but they are in two different places on the filesystem, without any valid reason (only looking at git history).Another issue is that we are currently redefining it every time we create the main Environment object, but that assignment is useless since it's already initialized with the same thing.
This PR fixes these issues and deprecates the old class used to represent
env.promotions(Spree::Promo::Environment).Checklist:
- [ ] I have updated Guides and README accordingly to this change (if needed)