fix: preserve option_type_id across syncs + don't clobber attrs with empty values - #5
Merged
mageaustralia merged 3 commits intoJun 2, 2026
Conversation
…empty values Fixes #3, #4. - CustomOptionsTrait: match incoming values to existing values by title within each option and preserve the destination option_type_id in the payload, so core's option save handler updates rows in place instead of delete+recreate. Stops churning option_type_ids referenced by historical order/quote item options and 3rd-party dependent-option modules. - Default options_mode flipped from 'replace' to 'merge' (3 call sites). 'replace' is still available via _entity_options['options_mode']. - Custom-attribute write loop now skips writes where the incoming value is empty AND the attribute's default_value is non-empty, preserving the destination value. Closes the options_container PDP-options-hidden issue.
mageaustralia
pushed a commit
that referenced
this pull request
May 27, 2026
…un read
- Adds the apply path (was previously discovery-only): reuses
datasync/adapter_openmage + datasync/engine with on_duplicate=merge and an
entity_ids filter, batched in configurable chunks (--batch-size, default 50).
Same engine path as datasync:incremental, just bypassing the live tracker.
- Fixes a real bug introduced in the previous version: $dryRun was referenced
but never assigned from $input->getOption('dry-run'), so APPLY mode always
ran regardless of the flag. Now correctly defaults to dry-run preview unless
--dry-run is omitted.
Safe to pair with PR #5 (value-level merge) — re-syncing a stale product no
longer churns option_type_ids, so the dependent-options config restored by the
companion CLI in maho-module-dependentoptions stays valid.
Resolve CustomOptionsTrait conflict by taking main's value-level merge (from #8, which superseded this branch's equivalent trait change). Retains this branch's unique changes in Product.php: options_mode default flipped to 'merge' (4 sites) + empty-overwrite guard for attrs with a non-empty default (closes #4).
mageaustralia
deleted the
fix/value-level-merge-and-options-container-guard
branch
July 8, 2026 01:29
This file contains hidden or 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
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.
Fixes #3, #4.
What
CustomOptionsTrait— value-level title merge: incoming option values are matched to existing values by title within their option, and the destinationoption_type_idis preserved in the payload. Core's product-option save handler then UPDATEs the row in place instead of delete+recreate, sooption_type_ids stay stable across syncs.Default
options_modeflipped from'replace'to'merge'(3 call sites).'replace'is still available when explicitly requested via_entity_options['options_mode'].Guard against empty overwrites of attributes with non-empty defaults: in the custom-attribute write loop, if the incoming value is empty/null AND the attribute's
default_valueis non-empty, the write is skipped (destination value preserved). Closes Empty sourceoptions_containeroverwrites destination → frontend options hidden #4 — generalises to any UI-gating attribute with a default.Why
A single sync that carried
custom_optionswas orphaning the entire dependent-options config on a real install (~450 products in one go) — every value'soption_type_idwas recreated, breaking historical order/quote item options and 3rd-party modules keyed off those IDs. Theoptions_containerclear was hiding product option dropdowns site-wide for any product whose source payload had an empty value.Test plan
datasync:incrementalafter marking the product indatasync_change_tracker).catalog_product_option_type_value.option_type_idvalues for that product are unchanged after the sync (snapshot before/after).option_type_ids (e.g. Pektsekye-styleoptiondependent_value.option_type_id) remain valid after the sync.options_container = ''and whose destination has a non-empty value — confirm the destination value is preserved.Notes
replacedeletes existing options first → new options have no$option->getId()→ the existing-by-title map is empty → behaviour identical to before.mergesetsoption_idfrom title match →getId()returns the destination id → existing values fetched → title-keyed preserve kicks in.appendadds only new → no existing match → same asreplacepath for the value step._createOptionValues()is dead code (never called from this flow —$option->setData('values', …)+$option->save()goes through core's own value handler). Left alone here; a separate cleanup PR can remove it.