Introduce a null promotion configuration#5667
Conversation
github-actions
bot
added
changelog:solidus_api
mamhoff
force-pushed
the
null-promo-config
branch
2 times, most recently
from
a7f58db to
dcc8f86
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5667 +/- ##
==========================================
+ Coverage 88.62% 88.64% +0.02%
==========================================
Files 688 691 +3
Lines 16493 16523 +30
==========================================
+ Hits 14617 14647 +30
Misses 1876 1876 ☔ View full report in Codecov by Sentry. |
| # frozen_string_literal: true | ||
|
|
||
| module Spree | ||
| class NullAdjuster |
There was a problem hiding this comment.
Maybe a more specific name would help to understand the purpose of the object.
What about?
Spree::NullPromotionAdjusterThere was a problem hiding this comment.
I second this question, unless you see a reason to keep it more generic. In that case, can you please help us understand where it is?
There was a problem hiding this comment.
My observation was that the class doesn't actually do anything promotion-specific, and the interface (#call) also doesn't necessarily do anything. I was suspecting (but wrongly) that also tax adjuster might share that interface.
Whatever: It's completely fine to call it NullPromotionAdjuster, and I've amended the PR to reflect that.
|
@mamhoff the PR title is a bit confusing, though. Mind to adjust it? |
Done, thank you! |
kennyadsl
left a comment
kennyadsl
left a comment
There was a problem hiding this comment.
Well done, thanks Martin, left a comment.
| # frozen_string_literal: true | ||
|
|
||
| module Spree | ||
| class NullAdjuster |
There was a problem hiding this comment.
I second this question, unless you see a reason to keep it more generic. In that case, can you please help us understand where it is?
Spree::Core::PromotionConfiguration will have to live in the solidus_legacy_promotions gem. We will need a stand-in! But for that stand-in, we also need an extension point.
This allows us to have a stand-in for a promotion system when, in fact, we don't have one.
This is a stand-in for all the promotion handlers we have. It implements all the methods from the promotion handler "coupon".
We need a promotion configuration when solidus_legacy_promotions is out of core. This provides it.
mamhoff
force-pushed
the
null-promo-config
branch
from
dcc8f86 to
e29983f
Compare
github-actions
bot
removed
changelog:solidus_api
4 participants
Summary
This adds a "null" promotion configuration, consisting of only those extension points for the promotion system that are used from within core (rather than from within the promotion system).
This is a nice exercise as it also tells us what extension points we need for any promo system: A class to help recalculate orders, and one to add coupon codes to orders.
This includes the commits from #5635 . I'll rebase once that is merged.
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: