Skip to content

Moving an option that variants use orphans their pairs, and the option comes back blank under the old attribute #51

Description

@wmdhosting

Summary

An option that variants still use can be moved to another attribute, either with the "Attribute" select in its sidebar or by dragging it in the Variant Attributes structure. The variants keep the old pair, so after the move:

  • the plugin no longer counts those variants against the moved option, so it can be deleted, which gets around the in-use delete check added in 4.2.1;
  • the next save of any of those variants re-registers the old pair, so a new, empty option with the same name is created under the old attribute. Its custom field values (swatch, image, etc.), SKU partial and price modifier stay on the moved copy, which nothing uses.

Reproduce

Variant Manager 4.2.2, Craft 5.11.3, Commerce 5.7.4.

  1. Attribute Color with option Red, stored on 12 variants (Color: Red).
  2. Open Red, change its "Attribute" to Frame, save. (Dragging Red onto Frame in the index does the same.)
  3. Red is now under Frame. The 12 variants still store Color: Red.
  4. Delete Red. The in-use check passes, because it looks for Frame: Red.
  5. Instead of step 4, save any one of the 12 variants. A new Red appears under Color, with none of the original's field values.

We ran steps 2–5 in a rolled-back transaction against real data: the save and the drag both succeed, isOptionInUse() returns false after the move, deleteElement() returns true while 12 variants still store Color: Red, and saving one variant creates a new option under Color.

Cause

VariantAttribute::afterSave() and afterMoveInStructure() change attributeId without touching the variants. The Structures::EVENT_BEFORE_UPDATE_ELEMENT handler in Plugin.php refuses a nested attribute, an unnested option, and a name clash, but not an option that is in use. variantQueryForOption() matches on the current parent's name, so once the option moves, its variants no longer count. The variant-save handler that calls ensureFromAttributePairs() then re-creates the old pair.

Suggested fix

Either:

  • refuse the move while isOptionInUse() is true (the same check beforeDelete() makes), for both the sidebar save and the structure move; or
  • rewrite the stored pair on the affected variants from OldAttribute: Option to NewAttribute: Option as part of the move (a queue job, since this is the JSON search over all variants).

The first option matches how delete behaves now. The second is what a merchant fixing a misfiled option most likely expects.

Related: 4.2.0 "Added the ability to move an option to another variant attribute", 4.2.1 in-use delete confirmation.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions