- Issue created by @jsacksick
- Status changed to Needs review
over 1 year ago 11:20am 7 February 2024 - last update
over 1 year ago Patch Failed to Apply -
jsacksick →
committed b8acf119 on 8.x-2.x
Issue #3419737 by jsacksick: Enforce a single price for the same...
-
jsacksick →
committed b8acf119 on 8.x-2.x
- Status changed to Fixed
over 1 year ago 11:22am 7 February 2024 - 🇩🇪Germany simonbaese Berlin
Can you please explain how to handle the same entry with different currencies? In our current implementation, we can not define a unique key with
'type', 'purchasable_entity', 'price_list_id', 'quantity'.
We would also need
'price__currency_code'.
We select which price is applicable for the current market with a hook on the commerce pricelist item query tag. Not sure if this use case is not accounted for or if our implementation approach is wrong.
If you do not want to change the behavior, could please introduce a hook to alter the key definition?
- 🇮🇱Israel jsacksick
Oof! Sorry for that, what about the attached patch? If you're ok with it, I'll tag a new release urgently...
- 🇮🇱Israel jsacksick
Btw, I'm thinking that it's probably risky to just delete duplicate prices... Perhaps we should display a warning and we should let people cleanup their prices...
-
jsacksick →
committed 61db275c on 8.x-2.x
Issue #3419737 followup: Stop deleting duplicate prices and support...
-
jsacksick →
committed 61db275c on 8.x-2.x
- 🇮🇱Israel jsacksick
Sorry went ahead and committed a slightly different patch, I'm now skipping the duplicate prices deletion... Perhaps I should have let the update fail in case the schema could not be updated, right now I'm catching the exception and displaying a message.
I think it was urgent to fix before people mass upgrade to it... - 🇩🇪Germany simonbaese Berlin
Thanks for the quick reply. I understand the motivation to prevent duplicates in the price list item table. But this approach may be too restrictive, since the
PriceListItem
is a fieldable entity and the query inPriceListRepository::loadItems()
allows alterations.We can refactor our implementation to satisy an extended
PriceListItemStorageSchema
. But users with less technical experience probably would struggle with this. - 🇮🇱Israel jsacksick
Well, this is an important feature to me... Having several prices for the same quantity tier within the same pricelist is a bug to me...
This causes a random price to be resolved, and that was causing a price history feature I built on a project to be "confused" as it was detecting a new price constantly.
I added the currency code as suggested. I don't really see the case for allowing that... - 🇩🇪Germany simonbaese Berlin
As already said, I understand the case for the "vanilla" state of the module. I am giving feedback that the module offers extendability and this extendability is disrupted in a minor release. If you are extending the functionality in a price history module, why not enforce the schema from there?
Automatically closed - issue fixed for 2 weeks with no activity.
- 🇨🇳China lawxen
Same sku && quantity with different price by conditional field of price_list_item like "Time period".
This is out use case. The commit confilct with it. - 🇨🇳China lawxen
#11 🐛 Enforce a single price for the same quantity tier per pricelist Active I don't really see the case for allowing that..
For these possible usage like #14 🐛 Enforce a single price for the same quantity tier per pricelist Active Do you sussgest to add the feature back? @jsacksick
- 🇮🇱Israel jsacksick
Same sku && quantity with different price by conditional field of price_list_item like "Time period".
Why not defining a different pricelist then?
For these possible usage like #14 Do you sussgest to add the feature back? @jsacksick
No plan of doing that for now.
Regarding the comment #12 that I somehow missed:
If you are extending the functionality in a price history module, why not enforce the schema from there?
True, could have done that, maybe a less disruptive change. But now that this change is in, it's also going to be disruptive to revert.
- 🇨🇳China lawxen
Why not defining a different pricelist then?
If do this:
A lot of new price list wiil be created.
Group module need be introduced to group the dispersed price list. But this increases the complexity of the system.