Fix parent product price cleared when variant price is changed - #2974
Fix parent product price cleared when variant price is changed#2974dpfaffenbauer with Copilot wants to merge 6 commits into
Conversation
Replace clone of ProductStoreValues with new instance creation via factory to prevent shared Doctrine identity (ID) from causing the parent's database record to be updated when persisting the variant's store values. Fixes the regression where changing a variant's price would reset the parent product's price and VAT to 0. Co-authored-by: dpfaffenbauer <5981845+dpfaffenbauer@users.noreply.github.com>
|
…tance Creating the new record via the factory alone is not enough: the inherited entity is shared with the parent product, stays managed and keeps the change set that broke the inheritance, so flush() writes those changes onto the parent products record. Roll it back to its persisted state and detach it. The new record also gets its own ProductUnitDefinitionPrice instances instead of leaving them behind on the inherited entity.
🚦 PR Guardrail
❌ R1 — Branch nameBranch ➡️ Rename the branch to ℹ️ Some CI checks are still running. The CI result is re-evaluated automatically once they finish. Automated comment by the PR guardrail — details and FAQ: |
…tance Enumerating the fields to copy drops everything a project added to its own ProductStoreValues model. Reuse the approach of createDataCopy() instead: clone the record and drop its identity, which keeps every other field by construction, and extract that into a shared helper.
A copy of a store values record is a new, unowned record. Saying that once in __clone() replaces the reflection juggling in the field definition, keeps the fields of a custom ProductStoreValues model by construction, and gives both the programmatic and the admin path the same behaviour. Projects subclassing the model can extend __clone() for their own associations.
|
Changing a variant's price overwrites the parent product's price/VAT to 0.
Root cause
When a variant inherits its store values and something is changed,
StoreValues::save()breaks the inheritance and gives the variant its own record, withclone $storeValuesEntity.The clone is not the problem in itself — but it kept the
idof the record it was copied from, and with it the identity of the parent's row.ProductStoreValuesinstance the parent product holds. Changing the price on the variant therefore modifies the parent's managed entity.save()callscomputeChangeSet()on it to detect the change, which registers a pending update in the Unit of Work. The followingflush()writes the variant's price onto the parent's row.EntityMergerre-registers the clone under the parent's id withproductpointing at the variant. The parent then has no own store values row at all and falls back to a price of 0, which is what the issue describes.Fix
A copy of a store values record is a new record that belongs to nobody yet.
ProductStoreValuesnow says so itself:That is all three copy sites need.
createDataCopy()loses itsReflectionClassblock and is a plainclone;getDataFromEditmode()keeps itscloneunchanged and is correct by construction; and thesave()branch clones instead of copying field by field.Expressing it on the entity rather than in the field definition matters for more than tidiness:
protected, so the model is the natural place to define what a copy is — no reflection from the outside, and nothing that PHP 8.5 deprecates (ReflectionProperty::setAccessible()).coreshop.resources.product_store_values.classes.modelis overridable. A project that replaces the model with its own subclass and adds persisted fields gets the right behaviour by default, and can extend__clone()for its own associations. Enumerating fields to copy would silently drop those fields the moment inheritance is broken.Two things remain in
save(), both needed because the entity being copied is the parent's:ProductUnitDefinitionPriceinstances (the originals stay with the parent's record, which is orphan-removal owned);Verification
Replaying the Unit of Work sequence against doctrine/orm 3.6.8, with a
ProductStoreValuessubclass that adds a persistedcustomField— as a project would:clone, identity kept)refresh()detach()clonewith__clone()+ rollback +detach()refresh()does not help: the change set computed earlier survives it.Tests
features/domain/product/product_store_values_inheritance.featuregets two scenarios that actually reach the branch. The existing steps could not: they go throughsetStoreValuesOfType(), which reads withInheritanceHelper::useInheritedValues(..., false)and therefore never operates on the parent's entity. The new setup step reads the store values with inheritance, changes the price and saves — the flow from the issue — and the assertions run against the products reloaded from the database.Regression of #1486, previously patched in #1491 for CoreShop 2.2.