SyliusTierpricePlugin
SyliusTierpricePlugin copied to clipboard
Issue updating tier prices for customer groups
In order to reproduce (in Admin UI) the problem I'm facing, make sure you have an existing product with only one tier price for one customer group.
Delete the existing tier price (don't save yet) and then add new one using exactly the same values as the one deleted. Save the tier price and you will get an exception.
Trying the same scenario with no customer group selected isn't throwing the exception.
Thank you for reporting the bug. One thing is for sure. There shouldn't be an exception when you save them. But I don't know, if the underlying constraint is incorrect. A tierprice with the same value and the same price should not exist if the customer group is null. Because if the customer group is null then it applies to all customers. So it will be a matter of which entity is loaded first to determine the price.
What are your thoughts on that?
Since TierPriceUniqueValidator
won't allow operations into database for duplicates, I don't think the database constraint is needed. Removing the unique constraint from TierPrice.orm.xml/database solves the problem.
The validator should have been called before it gets inserted into the database. So the validator should have caught that. I will check that.
Just had a quick look at this, the validator won't pick it up as it stands as it doesn't look at the values in the database, only at the data submitted in the form (I didn't think to test deleting and adding the same tier price in one save).
The UniqueEntitiy Validator (https://symfony.com/doc/current/reference/constraints/UniqueEntity.html) should catch this case I think, however as mentioned in the docs that validator can only validate against collections that have already been persisted and won't catch cases where someone tries to add two identical tier prices in the same save action, hence why I wrote the current TierPriceUniqueValidator
.
Not sure on the best way of handling this, perhaps add a check to TierPriceUniqueValidator
that selects from the database with the same values and if the ids are different flag it as a duplicate, but I think that would then cause an issue of marking the new one as a duplicate whilst it doesn't show the old one at all leading to confusion.
I think technically deleting a tier price then adding a new one should succeed but it seems the way symfony forms/doctrine are handling this the deletes are the last query type to run when for this to work they would have to happen before the inserts.
My solution so far was to remove the unique constraint from database.
Yeah, unfortunately the constraint doesn't catch every case anyway as customer_group_id can be null and databases treat nulls as unique values so it doesn't stop you inserting two tier prices with the same quantity if the customer group isn't set (you can see this by repeating your example but by not setting a customer group and this time it submits fine). So removing the constraint might be a better option given it doesn't catch every case anyway.
Not sure if its possible but my ideal solution would be to see if there is a way to make sure the deletes happen before the inserts, but I'm fairly sure that not something you can easily do from a plugin as its likely down to core code in Symfony Forms or Doctrine and I suspect there is a good reason why the database operations happen in the order they do.