[v3.1] Merge pull request #4639 from mamhoff/do-not-discard-variant-prices-on-variant-discard #4691
Conversation
This file contains 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
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
7 tasks
7 tasks
When we soft-delete a variant, we also soft-delete the variant's prices. In my opinion, this is not necessary, as the association between prices and variants includes discarded prices, and harmful, as it leads to price inconsistencies between a variant before discarding and after undiscarding (if the variant has a newer, but discarded, price, we have no way of knowing that that price should not be un-discarded along with the variant). (cherry picked from commit 5d8531d)
Using the `currently_valid` scope in this instance method leads to an "n+1" issue when getting prices for more than one variant. By sorting these in memory rather than in the database, we can save a lot of database round-trips. (cherry picked from commit 7611923)
This uses the redefined public method from the previous commit to detect the default price. Note that through using `reverse_each.detect` rather than `min` with a block we should be able to save some iterations. (cherry picked from commit cb89fc4)
(cherry picked from commit bb132ee)
This was the previous behavior. (cherry picked from commit 46fe34d)
We don't need to be using the weirdly named `currently_valid_prices` method. As this is the only spot in core where we use that method, we can now deprecate it. (cherry picked from commit 5f4e3cb)
(cherry picked from commit 23cff1c)
(cherry picked from commit afbf64f)
This uses the price selector in order to find the default price rather than having to re-implement the default price selector's logic. (cherry picked from commit e273da9)
Previous Solidus behavior included soft-deleted prices when looking at the default price. The intention of this behavior was that a a variant that is soft-deleted, and has its prices soft-deleted as a consequence, still has a `default_price` record. However, for a `kept` variant, we do not want to see a discarded `default_price` - otherwise what sense does it make to even add the delete action to the price? Especially given that deletion touches the variant and will then move it forward in the array of sorted prices to be considered for a given set of pricing attributes. This commit changes the `prices` association to include discarded prices, but then filters out discarded prices for non-discarded variants in the price selector. (cherry picked from commit 3f4578a)
kennyadsl
force-pushed
the
backport/v3.1/pr-4639
branch
from
4cc8d3b to
48465ad
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Backport
This will backport the following commits from
mastertov3.1:Questions ?
Please refer to the Backport tool documentation