Introduce SolidusPromotions #5805

mamhoff · 2024-06-26T15:49:28Z

Summary

This imports the solidus_friendly_promotions into the main solidusio/solidus repository, including all history.
This has been done using git filter-branch.

This is a big pull request, because it keeps the entire git history of the solidus_friendly_promotions repository. I encourage reviewers to review the last few commits, but better yet: Try it out on your stores.

See the README file as well as the MIGRATING.md file for instructions how to move from the legacy promotion system to solidus_promotions.

This gem supports

stackable promotions
much better code readability
the old and the new admin
creating discounted line items

And many more things. It should be well-linted, and has an extensive test suite.

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.

codecov · 2024-06-26T18:14:47Z

Codecov Report

Attention: Patch coverage is 98.73853% with 11 lines in your changes missing coverage. Please review.

Project coverage is 89.57%. Comparing base (25a41bf) to head (bf743d2).
Report is 525 commits behind head on main.

Files with missing lines	Patch %	Lines
...nd/solidus_promotions/admin/benefits_controller.rb	87.75%	6 Missing ⚠️
.../admin/solidus_promotions/promotions_controller.rb	87.50%	3 Missing ⚠️
...motions/app/models/solidus_promotions/condition.rb	93.54%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5805      +/-   ##
==========================================
+ Coverage   89.32%   89.57%   +0.24%     
==========================================
  Files         759      782      +23     
  Lines       17682    17965     +283     
==========================================
+ Hits        15795    16092     +297     
+ Misses       1887     1873      -14

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

kennyadsl

I reviewed this with @mamhoff live and we left some comments together for things that might be worth improving. Looking forward to merge this!

promotions/README.md

promotions/app/decorators/models/solidus_promotions/order_recalculator_decorator.rb

promotions/app/javascript/solidus_promotions.js

...ns/app/models/concerns/solidus_promotions/conditions/line_item_applicable_order_condition.rb

promotions/app/models/solidus_promotions/benefit.rb

promotions/bin/importmap

promotions/lib/controllers/backend/solidus_promotions/admin/base_controller.rb

promotions/lib/generators/solidus_promotions/install/install_generator.rb

promotions/lib/generators/solidus_promotions/install/templates/initializer.rb

promotions/lib/solidus_promotions/promotion_map.rb

kennyadsl · 2024-08-08T13:49:59Z

Some things we'd need help with at this stage:

Testing the upgrade path on a store with many promotions (bonus points for custom promotion rules/actions and for orders in the checkout state with a legacy promotion applied).
Check the admin UI to see if everything is clear.
Help rebasing the PR removing the merge commits (which refer to the old repository where this work started).

promotions/app/models/solidus_promotions/promotion_code/batch_builder.rb

jarednorman · 2024-09-03T17:04:11Z

I keep coming back and looking at this PR... and I'm still both excited and intimidated by it.

mamhoff · 2024-09-03T18:10:09Z

I keep coming back and looking at this PR... and I'm still both excited and intimidated by it.

Is there anything I can do to augment the excitement and reduce the anxiety?

jarednorman · 2024-09-03T22:06:59Z

I probably should just take it for a spin on a store.

kennyadsl · 2024-09-04T08:02:38Z

@jarednorman my anxiety level dropped after a very useful 1-hour session live with @mamhoff. 😄 BTW, testing this on a real/demo store could be a good way for getting the approve button pressed, which I'm going to do now.

tvdeyen · 2024-09-04T08:07:20Z

@kennyadsl @jarednorman @mamhoff would it be helpful to record a session? So that the community also has a chance to get some insights on the design decision made here?

tvdeyen

We use this in a real live store with lots of volume and personally have been attending countless pairings and reviews with @mamhoff during the last couple of months.

I am hitting the precious approve button now as well.

Amazing work Martin. I cannot add enough emojis to show my gratitude.

jarednorman · 2024-09-04T16:57:28Z

would it be helpful to record a session?

That would be amazing.

mamhoff · 2024-10-08T07:55:55Z

@jarednorman What about we find a time and record it together? I have a hard time talking to the computer alone. :)

tvdeyen · 2024-10-08T13:10:38Z

@jarednorman What about we find a time and record it together? I have a hard time talking to the computer alone. :)

Do I sense a Dead Code episode?

jarednorman · 2024-10-08T22:32:23Z

@mamhoff Sorry for being very busy... how about a morning the week of November 21st? I don't mind getting up a little early, so the time zones work. Hit me up using the email on my profile.

I'd love to finally see this merged.

This reflects better what it does.

This looked a bit garbled without these extra w-100s.

Solidus Core now ships with a Null promotion handler.

We'll just be using Spree.solidus_version instead, like all the other gems.

This is to indicate clearly that this JS is used for the backend UI.

…tion This rename is for consistency only.

This allows developers to more clearly see what they need to do in order to implement custom promotion benefits.

This should give developers better hints as to how to use this gem.

As pointed out in https://github.com/solidusio/solidus/pull/5383/files#r1391519251, the new promotion code batch builder had worse performance characteristics than the one in the legacy promotion system. This gets the characteristics back to those of the legacy promotion system. Time spent and memory usage still go up significantly and linearly with the number of batches. I ran some specs with the following code: ```rb context "with a very large number of promotion codes" do let(:number_of_codes) { 10000 } it "creates the correct number of codes" do puts Benchmark.measure { subject.build_promotion_codes } expect(promotion.codes.size).to eq(number_of_codes) end end ``` With a 1000 codes, I measured: ``` Randomized with seed 49097 1.036322 0.028977 1.065299 ( 1.074396) ``` With 10000 codes, I measured: ``` Randomized with seed 33250 9.606364 0.278920 9.885284 ( 9.978242) ``` Memory usage went up linearly as well.

Much of this code has been lifted from some tax abstraction in core, and some of the comments had not been updated.

With the removal of our own .rubocop.yml, a few new linting problems appeared.

mamhoff · 2024-10-25T13:33:46Z

I've managed to clean up the git history. We've gotten rid of the merge commits from solidus_friendly_promotions, and all the commits along with their messages are still there. I think this is now ready to go in 🥳

jarednorman

This is exciting!

kennyadsl · 2024-10-26T03:15:06Z

Thanks again @mamhoff for the time to upstream this incredible work.

jarednorman · 2024-10-28T20:31:26Z

Let's go!!! 👏🏻 🎉

github-actions bot added the changelog:repository Changes to the repository not within any gem label Jun 26, 2024

mamhoff force-pushed the import-solidus-friendly-promotions branch 2 times, most recently from f35fce1 to 7b1c00e Compare June 26, 2024 17:28

mamhoff changed the title ~~WIP Import solidus friendly promotions~~ Introduce SolidusPromotions Jun 26, 2024

mamhoff force-pushed the import-solidus-friendly-promotions branch from d1ba821 to 96ef390 Compare June 26, 2024 18:03

mamhoff marked this pull request as ready for review June 27, 2024 11:41

mamhoff requested a review from a team as a code owner June 27, 2024 11:41

mamhoff force-pushed the import-solidus-friendly-promotions branch 2 times, most recently from ec3837a to 7979816 Compare July 3, 2024 04:17

kennyadsl reviewed Aug 8, 2024

View reviewed changes

kjriga reviewed Aug 16, 2024

View reviewed changes

promotions/app/models/solidus_promotions/promotion_code/batch_builder.rb Show resolved Hide resolved

mamhoff force-pushed the import-solidus-friendly-promotions branch from 4ed9c4c to 49e99be Compare August 26, 2024 10:40

mamhoff force-pushed the import-solidus-friendly-promotions branch from 64f18c1 to a2afec0 Compare September 2, 2024 20:38

kennyadsl approved these changes Sep 4, 2024

View reviewed changes

tvdeyen approved these changes Sep 4, 2024

View reviewed changes

tvdeyen added this to the 5.0 milestone Oct 9, 2024

tvdeyen added release:major Breaking change on hold until next major release and removed release:major Breaking change on hold until next major release labels Oct 9, 2024

tvdeyen removed this from the 5.0 milestone Oct 9, 2024

mamhoff added 21 commits October 25, 2024 14:17

Test promotion codes admin

bd1a355

Rename route scope to solidus

0229961

Rename spec descriptions to solidus

088690a

Rename factory files

4db86f7

Rename FriendlyPromotionAdjuster to OrderAdjuster

1ca6fb2

This reflects better what it does.

Fix promotions table filter

e0d10ae

This looked a bit garbled without these extra w-100s.

Remove duplicate methods from BaseController

996c4e2

Remove importmap binstub

06312f2

Remove Null Promotion Handler

7555730

Solidus Core now ships with a Null promotion handler.

Adapt README file

3d89ee3

Add comment to remove order recalculator decorator

44a3a4d

Remove version.rb file

dd6df37

We'll just be using Spree.solidus_version instead, like all the other gems.

Namespace JS under backend

f2bfd06

This is to indicate clearly that this JS is used for the backend UI.

LineItemApplicableOrderCondition to LineItemApplicableOrderLevelCondi…

eb90119

…tion This rename is for consistency only.

Improve error messaging for unimplemented methods on Benefit

0212f9b

This allows developers to more clearly see what they need to do in order to implement custom promotion benefits.

Improve documentation and install generator

326f117

This should give developers better hints as to how to use this gem.

Add documentation to promotion map

cab3b5e

Lint

0136666

Lint promotino map

471db28

Fix documentation in persist_discounted_order.rb

4c75a16

Much of this code has been lifted from some tax abstraction in core, and some of the comments had not been updated.

mamhoff force-pushed the import-solidus-friendly-promotions branch from 21b59e8 to fa25fd3 Compare October 25, 2024 12:18

mamhoff added 2 commits October 25, 2024 14:53

Build solidus_promotions on CI

98c7d4f

Fix linting issues

bf743d2

With the removal of our own .rubocop.yml, a few new linting problems appeared.

mamhoff force-pushed the import-solidus-friendly-promotions branch from fa25fd3 to bf743d2 Compare October 25, 2024 12:53

jarednorman approved these changes Oct 25, 2024

View reviewed changes

kennyadsl merged commit b36be11 into solidusio:main Oct 26, 2024
14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce SolidusPromotions #5805

Introduce SolidusPromotions #5805

mamhoff commented Jun 26, 2024 •

edited

Loading

codecov bot commented Jun 26, 2024 •

edited

Loading

kennyadsl left a comment

kennyadsl commented Aug 8, 2024

jarednorman commented Sep 3, 2024

mamhoff commented Sep 3, 2024

jarednorman commented Sep 3, 2024

kennyadsl commented Sep 4, 2024

tvdeyen commented Sep 4, 2024

tvdeyen left a comment

jarednorman commented Sep 4, 2024

mamhoff commented Oct 8, 2024

tvdeyen commented Oct 8, 2024

jarednorman commented Oct 8, 2024

mamhoff commented Oct 25, 2024

jarednorman left a comment

kennyadsl commented Oct 26, 2024

jarednorman commented Oct 28, 2024

Introduce SolidusPromotions #5805

Introduce SolidusPromotions #5805

Conversation

mamhoff commented Jun 26, 2024 • edited Loading

Summary

Checklist

codecov bot commented Jun 26, 2024 • edited Loading

Codecov Report

kennyadsl left a comment

Choose a reason for hiding this comment

kennyadsl commented Aug 8, 2024

jarednorman commented Sep 3, 2024

mamhoff commented Sep 3, 2024

jarednorman commented Sep 3, 2024

kennyadsl commented Sep 4, 2024

tvdeyen commented Sep 4, 2024

tvdeyen left a comment

Choose a reason for hiding this comment

jarednorman commented Sep 4, 2024

mamhoff commented Oct 8, 2024

tvdeyen commented Oct 8, 2024

jarednorman commented Oct 8, 2024

mamhoff commented Oct 25, 2024

jarednorman left a comment

Choose a reason for hiding this comment

kennyadsl commented Oct 26, 2024

jarednorman commented Oct 28, 2024

mamhoff commented Jun 26, 2024 •

edited

Loading

codecov bot commented Jun 26, 2024 •

edited

Loading