Add extension point: Promotion finder #5677
Conversation
This is intended to be used by the API Promotions Controller.
This configurable class finds a promotion by its code.
The default has to be the Null promotion finder.
The API promotions controller is pretty straightforward, and the only touch point here is finding a promotion. With that added to the promotion configuration, we can now use it with no changes to our specs.
This allows us to change the promotion system with factories, because we stop using factories in this spec.
github-actions
bot
added
changelog:solidus_api
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5677 +/- ##
==========================================
+ Coverage 88.86% 88.87% +0.01%
==========================================
Files 695 697 +2
Lines 16589 16599 +10
==========================================
+ Hits 14741 14752 +11
+ Misses 1848 1847 -1 ☔ View full report in Codecov by Sentry. |
| # @return [Class] an object that conforms to the API of | ||
| # the standard promotion finder class | ||
| # Spree::NullPromotionFinder. | ||
| class_name_attribute :promotion_finder_class, default: 'Spree::NullPromotionFinder' |
There was a problem hiding this comment.
I probably missed or forgot something in the past PRs but I don't understand how and where are we using this class (NullPromotionConfiguration). Can you please refresh my mind?
There was a problem hiding this comment.
Sure. Once the legacy promotion system is out of core, we have no more Spree::Promotion in core. All the touch points between the core system will be in a class called Spree::Core::PromotionConfiguration. However, even that class contains references to the legacy promotion system, so core will ship with a null object, the Spree::NullPromotionConfiguration. That class serves as documentation as to what a promotion configuration needs to provide, and is a stand-in that provides a bunch of service objects that do nothing but help us test the interaction between core and the configured promotion system.
There was a problem hiding this comment.
Nice, so the idea is that each "adapter" ships with its own configuration object, and what Solidus does, is using the right configuration class to understand which promotion handler to use, right? Shouldn't we have a Spree::LegacyPromotionConfiguration somewhere as well?
There was a problem hiding this comment.
Spree::Core::PromotionConfiguration is the Spree::LegacyPromotionConfiguration. I can rename it once it's moved to the legacy_promotions gem, but I would prefer to keep that name as the legacy_promotions gem doesn't really have legacy in its namespace in order to keep the constants the same once the legacy_promotions gem is loaded.
Summary
The API gem references the promotion system in very few places, namely in the promotions endpoint. This endpoint is only accessible to admins, and our admin doesn't actually check it, so another option would be to remove the endpoint entirely.
However, it's simple enough to create a class that does what we need here.
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: