Fix array-to-string conversion in ThemeConfig - #33
Open
ImanuelBertrand wants to merge 2 commits into
Open
ImanuelBertrand wants to merge 2 commits into
ImanuelBertrand wants to merge 2 commits into
Conversation
Saving Content > Design > Configuration posts the whole design config form, and the imageUploader fields in it (head_shortcut_icon, header_logo_src, email_logo, the watermark images) post a list of file descriptors, not a scalar -- e.g. [['name' => 'logo.png', 'url' => '...', 'size' => 1234]]. normalizeValue() cast whatever it got with (string), so those fields raised "Array to string conversion". With swissup/module-ignition installed that warning is promoted to an ErrorException, SaveAfter aborts and AbstractActivityObserver swallows it -- the design config is saved but no activity is logged for the change. Flatten arrays instead, the way the sibling Activity\SystemConfig::flattenValue() already does for system config values, and prefer the descriptor's file name so the logged value still compares against what core_config_data holds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R1c3CaPXcg4sxvfv9icv69
Follow-up to f4c844a, which left three gaps: Descriptor values were still cast unguarded, so a hand-crafted POST nesting one level deeper (head_shortcut_icon[0][file][]=x) re-raised the very "Array to string conversion" that commit set out to remove. Guard the cast with is_scalar() and fall back to encoding the whole descriptor. json_encode() returns false on malformed UTF-8, and (string)false is '', so a real change could compare equal to an empty old value and never be logged. Encode through a single encodeValue() that falls back to '[unserializable]', the placeholder FieldTracker::truncateValue() already uses, and apply the same fallback to the sibling SystemConfig::flattenValue(). Add the unit tests the branch had none of: the regression itself (re-posting an untouched image descriptor is not a change), the name fallback, replaced and cleared images, scalar lists, associative arrays, and both hardening cases above. The nested-array test asserts through a scoped error handler that no PHP warning fires, rather than relying on convertWarningsToExceptions, which PHPUnit 10 dropped. Rename ThemeConfig::flattenValue() to flattenPostedValue(): it shares a name but not semantics with SystemConfig::flattenValue(), and unifying the two would break the comparison against core_config_data. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ByaeLVN4rdcm6PbfzVBUJK
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
1 participant
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.
ThemeConfig::normalizeValue()cast values with(string), so array values from the design config form (image uploader fields like favicon and logos post a list of file descriptors) triggered an "Array to string conversion" warning and normalized to the literal "Array" — making every such field look unchanged, or changed to the same useless value.Arrays are now flattened instead: a list of file descriptors is reduced to its file values (the stored config value, as set by
Theme\Model\Design\Backend\File::afterLoad()), falling back to name, and anything else is JSON-encoded.json_encode()failures fall back to an[unserializable]placeholder — applied toSystemConfig::flattenValue()too, which had the same unchecked(string)json_encode(...).Covered by new unit tests in
Test/Unit/Model/Activity/ThemeConfigTest.phpandSystemConfigTest.php.Generated with Claude Code