Skip to content

Persist line item action quantity when updated#6443

Open
AlistairNorman wants to merge 1 commit intosolidusio:mainfrom
SuperGoodSoft:quantity-adjustment-issue
Open

Persist line item action quantity when updated#6443
AlistairNorman wants to merge 1 commit intosolidusio:mainfrom
SuperGoodSoft:quantity-adjustment-issue

Conversation

@AlistairNorman
Copy link
Copy Markdown
Contributor

Summary

There's a test failure in Solidus Starter Frontend that pointed toward an actual issue in the legacy promotion system. We are not persisting quantity changes in some cases. It seems that if the order is recalculated then the issue is resolved but when the quantity is adjusted or when new items are added to the cart then the quantity on the line_item_action is not persisted which further causes issues in figuring out the used quantity in the CreateQuantityAdjustments action.

The fix requires us to find the line item action based off of the line item in memory so that it can be properly referenced later and to add a line to persist the line item actions when that is the adjustable.

The failing test in Solidus Starter Frontend is spec/system/quantity_promotions_spec.rb:68 which can be seen failing in recent actions runs such as https://github.com/solidusio/solidus_starter_frontend/actions/runs/24611089582/job/71965516590?pr=438

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

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.

@AlistairNorman AlistairNorman requested a review from a team as a code owner April 18, 2026 21:38
@github-actions github-actions Bot added the changelog:solidus_legacy_promotions Changes to the solidus_legacy_promotions gem label Apr 18, 2026
@AlistairNorman AlistairNorman force-pushed the quantity-adjustment-issue branch from 481d0ba to de2b4bf Compare April 18, 2026 21:41
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.67%. Comparing base (852cb8e) to head (3889424).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6443   +/-   ##
=======================================
  Coverage   89.66%   89.67%           
=======================================
  Files         990      990           
  Lines       20792    20797    +5     
=======================================
+ Hits        18644    18649    +5     
  Misses       2148     2148           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@AlistairNorman AlistairNorman marked this pull request as draft April 18, 2026 23:41
@AlistairNorman
Copy link
Copy Markdown
Contributor Author

I see I have some failures, back to draft while I sort that out.

@AlistairNorman AlistairNorman force-pushed the quantity-adjustment-issue branch from de2b4bf to 9183b22 Compare April 20, 2026 05:03
@AlistairNorman AlistairNorman marked this pull request as ready for review April 20, 2026 05:04
Copy link
Copy Markdown
Member

@jarednorman jarednorman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable, but I'll re-review once you've sorted out the issues.

@AlistairNorman
Copy link
Copy Markdown
Contributor Author

I did sort out the issues! That's why I marked it was ready!

New records are always persisted when their parent record that they
belong to is persisted so the quantity is persisted properly on the
first save. The amount is always persisted because the compute_amount
method returns the correct value but the quantity is set as a side
effect of compute_amount and then is not explicitly persisted.

In order to fix this and fix the quantity calculations on each line item
we need the quantity to be set on the line item action associated with
the line item in memory and then that needs to be explicitly persisted.
@AlistairNorman AlistairNorman force-pushed the quantity-adjustment-issue branch from 26860c3 to 3889424 Compare April 20, 2026 05:17
Copy link
Copy Markdown
Member

@jarednorman jarednorman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's something about the way you wrote that commit message that makes me 100% sure it wasn't written by AI, which I love. (I love that it wasn't LLM-generated and I love that you write with a style that looks human.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog:solidus_legacy_promotions Changes to the solidus_legacy_promotions gem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants