Nested Class Set extension, Promotion configuration object #5658

mamhoff · 2024-02-12T10:03:28Z

Summary

This is an extraction from the #5635, in which I create a new nested class set that can be used to associate calculators to classes in a way that allows adding more classes without needing to add more configuration endpoints.

I've also opted to use the new add_nested_class_set method in a new promotions object that can serve as a nucleus for the full new promotion configuration. Furthermore, there's a change from #5635 in which the getter for the promotion configuration is now Spree::Config.promotions. Changing the promotion configuration is not yet supported.

So in order for a developer to add a custom calculator that should be available to the Spree::Promotion::Actions::CreateItemAdjustments action, instead of doing the following in your initializer:

[
  'MyCalculator'
].each do |calc|
  Rails.application.config.spree.calculators.promotion_actions_create_item_adjustments << calc
end

You would do the following:

[
  'MyCalculator'
].each do |calc|
  Spree::Config.promotions.calculators["Spree::Promotion::Actions::CreateItemAdjustments"] << calc
end

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

I have written a thorough PR description.
I have kept my commits small and atomic.
I have used clear, explanatory commit messages.

The following are not always needed:

📖 I have updated the README to account for my changes.
📑 I have documented new code with YARD.
🛣️ I have opened a PR to update the guides.
✅ I have added automated tests to cover my changes.
📸 I have attached screenshots to demo visual changes.

This extends the `add_class_set` command to accept a default value. Useful for, well, default values.

Helpful for directly initializing a class set.

This is useful especially for defining a set of classes and classes they can work with, like calculables and calculators.

This new method can be used like so: ``` class MyConfig include Spree::Core::EnvironmentExtension add_nested_class_set :promotion_calculators, default: { "Spree::Promotion::Actions::CreateAdjustments" => [], "Spree::Promotion::Actions::CreateItemAdjustments" => ["Spree::Calculator::FlexiRate"] } end ``` Having an configuration method that allows nesting e.g. calculators by the classes that support them allows us to not hard-code names like these `:promotion_actions_create_adjustments` versus `promotion_actions_create_item_adjustments`.

codecov · 2024-02-12T12:25:05Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (020f3c2) 88.55% compared to head (0b70bfc) 88.58%.
Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5658      +/-   ##
==========================================
+ Coverage   88.55%   88.58%   +0.03%     
==========================================
  Files         685      687       +2     
  Lines       16408    16453      +45     
==========================================
+ Hits        14530    14575      +45     
  Misses       1878     1878

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

tvdeyen

Nice feature.

For now, this class only contains the promotion calculators. More configuration to come.

No need for referencing different extension points here.

We want promotion calculator configuration to live on the Spree::Config.promotions.calculators object, and with the new Nested Class Set, this can be nicely accomplished.

kennyadsl

@mamhoff thanks, nice work!

I'm generally in favor, but can you please add some details in the PR description about the steps that someone using the old preference scheme should take to migrate and be able to add custom calculators now?

mamhoff · 2024-02-14T14:14:29Z

I'm generally in favor, but can you please add some details in the PR description about the steps that someone using the old preference scheme should take to migrate and be able to add custom calculators now?

I've added an example!

…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 requested a review from a team as a code owner February 12, 2024 10:03

github-actions bot added changelog:solidus_backend Changes to the solidus_backend gem changelog:solidus_core Changes to the solidus_core gem labels Feb 12, 2024

mamhoff force-pushed the legacy-promos-prep/class-set-extensions branch from 4f68a7b to 32565d7 Compare February 12, 2024 11:18

mamhoff added 4 commits February 12, 2024 13:16

Environment Extension: Allow default value

3c5d7f1

This extends the `add_class_set` command to accept a default value. Useful for, well, default values.

Class Constantizer: Allow initializing with a default

0402095

Helpful for directly initializing a class set.

Add nested class set

d218c50

This is useful especially for defining a set of classes and classes they can work with, like calculables and calculators.

mamhoff force-pushed the legacy-promos-prep/class-set-extensions branch from 8881014 to f72b464 Compare February 12, 2024 12:25

tvdeyen approved these changes Feb 12, 2024

View reviewed changes

mamhoff added 5 commits February 12, 2024 15:58

Add a promotion configuration class

25602bd

For now, this class only contains the promotion calculators. More configuration to come.

Add a getter for the promotion configuration

6b24c07

Allow promotion actions to know their calculators

a38ff8e

No need for referencing different extension points here.

Backend: Use promotion_action.available_calculators

a8dec8e

Move config for promo calculators to Spree::PromotionConfiguration

f106451

We want promotion calculator configuration to live on the Spree::Config.promotions.calculators object, and with the new Nested Class Set, this can be nicely accomplished.

mamhoff force-pushed the legacy-promos-prep/class-set-extensions branch from f72b464 to f106451 Compare February 12, 2024 14:59

kennyadsl reviewed Feb 14, 2024

View reviewed changes

Add spec for Spree::Core::Environment::Calculators

0b70bfc

mamhoff force-pushed the legacy-promos-prep/class-set-extensions branch from f0466a0 to 0b70bfc Compare February 14, 2024 16:08

jarednorman approved these changes Feb 14, 2024

View reviewed changes

kennyadsl merged commit 395d909 into solidusio:main Feb 16, 2024
14 checks passed

mamhoff deleted the legacy-promos-prep/class-set-extensions branch February 17, 2024 12:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Nested Class Set extension, Promotion configuration object #5658

Nested Class Set extension, Promotion configuration object #5658

mamhoff commented Feb 12, 2024 •

edited by kennyadsl

Loading

codecov bot commented Feb 12, 2024 •

edited

Loading

tvdeyen left a comment

kennyadsl left a comment

mamhoff commented Feb 14, 2024

Nested Class Set extension, Promotion configuration object #5658

Nested Class Set extension, Promotion configuration object #5658

Conversation

mamhoff commented Feb 12, 2024 • edited by kennyadsl Loading

Summary

Checklist

codecov bot commented Feb 12, 2024 • edited Loading

Codecov Report

tvdeyen left a comment

Choose a reason for hiding this comment

kennyadsl left a comment

Choose a reason for hiding this comment

mamhoff commented Feb 14, 2024

mamhoff commented Feb 12, 2024 •

edited by kennyadsl

Loading

codecov bot commented Feb 12, 2024 •

edited

Loading