Build discounted linte items in memory #6385
Merged
Conversation
This file contains hidden or 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
Instead of destroying. See code comments and spec changes. This is similar to how we handled this in the legacy promotion system. Co-authored-by: Adam Mueller <adam@super.gd> Co-authored-by: Alistair Norman <alistair@super.gd> Co-authored-by: An Stewart <andrew@super.gd> Co-authored-by: benjamin wil <benjamin@super.gd> Co-authored-by: Harmony Evangelina <harmony@super.gd> Co-authored-by: Jared Norman <jared@super.gd> Co-authored-by: Nick Van Doorn <nick@super.gd> Co-authored-by: Senem Soy <senem@super.gd> Co-authored-by: Sofia Besenski <sofia@super.gd>
When computing item totals after a line item benefit is removed, we need to ensure we aren't using the marked for destruction items. We also needed to make a change to the legacy promotion order updater patch because it is incorrectly being used in our tests even when using the new promotion system. It also did not have logic to ensure adjustments that were marked for destruction were ignored in the adjustment totals calculation. Co-authored-by: Jared Norman <jared@super.gd> Co-authored-by: Noah Silvera <noah@super.gd> Co-authored-by: Alistair Norman <alistair@super.gd>
github-actions
bot
added
changelog:solidus_core
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6385 +/- ##
=======================================
Coverage 89.45% 89.45%
=======================================
Files 974 974
Lines 20322 20323 +1
=======================================
+ Hits 18179 18180 +1
Misses 2143 2143 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
forkata
force-pushed
the
build-discounted-item-benefit-in-memory
branch
from
f6444c3 to
91f7ddd
Compare
As part of the work to move towards reducing database writes, this change attempts to build discounted item benefits in-memory and allow the order updater to recalculate their totals correctly. Co-authored-by: Adam Mueller <adam@super.gd> Co-authored-by: Alistair Norman <alistair@super.gd> Co-authored-by: Andrew Stewart <andrew@super.gd> Co-authored-by: benjamin wil<benjamin@super.gd> Co-authored-by: Chris Todorov <chris@super.gd> Co-authored-by: Harmony Bouvier <harmony@super.gd> Co-authored-by: Jared Norman <jared@super.gd> Co-authored-by: Nick Van Doorn <nick@super.gd> Co-authored-by: Noah Silvera <noah@super.gd> Co-authored-by: Senem Soy <senem@super.gd> Co-authored-by: Sofia Besenski <sofia@super.gd>
In an associated change we made the `CreateDiscountedItem` benefit create in-memory line items. That causes an issue during the recalculation of the item totals. Co-authored-by: benjamin wil <benjamin@super.gd> Co-authored-by: Kendra Riga <kendra@super.gd> Co-authored-by: Noah Silvera <noah@super.gd> Co-authored-by: Jared Norman <jared@super.gd> Co-authored-by: Alistair Norman <alistair@super.gd>
We want to make sure we don't persist anything when computing amounts becuase that will cause issues with the in-memory order updater.
forkata
force-pushed
the
build-discounted-item-benefit-in-memory
branch
from
91f7ddd to
64ecee0
Compare
Contributor
|
I do have a suggestion for the PR title: |
forkata
changed the title
tvdeyen
approved these changes
Nov 27, 2025
Member
tvdeyen
left a comment
tvdeyen
left a comment
There was a problem hiding this comment.
Amazing mob programming work 👏🏻
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
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.
Summary
This change builds on the work in #6382 and depends on that to work correctly. The first two commits here are from #6382 and will be rebased out once that has been merged.
These were extracted from #5872 to make that PR smaller and easier to review.
This change refactors how we build discounted items and allows the order updater
to correctly update their totals.
This change also adds some test coverage and assertions for the legacy promotion system to ensure we don't introduce unnecessary writes to the database in the future.
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: