add support for mega menu (WP-980)

[vsolovei-smartling]

No description provided.

[PavelLoparev]

@vsolovei-smartling this is generated with /sl-core-dev:claude-pr-review-local cc skill. This is exactly the same skill that runs PR reviews for the private GH repos. Try it. First time your run it it will guide you through the installation process.

Code Review: WP-980-menu-translation vs master

Profile: fullstack + production-tier | 1 commit

---

Issues

🟡 Warnings

MegaMenu.php:37–44 — Silent translation loss for items without _id
getTranslatableStrings uses $index as fallback key when _id is absent, but setTargetContent matches only on _id string. Translations keyed by integer index are silently dropped during apply. Either enforce _id as required (log/skip missing ones), or align the matching strategy with the key derivation.

MegaMenu.php:62–83 — Unrestricted property write into menu_items entries

$this->settings['menu_items'][$index][$property] = $value;

Any key from $values (caller-controlled) can overwrite structural keys like _id, elType, or __dynamic__. Restrict $property to an allowlist (e.g. item_title) or validate $value is scalar.

MegaMenu.php:62–83 — Fragile parent/child ordering in setTargetContent
The current sequence (call parent → re-read raw → apply string loop → write back) is safe but brittle. Future parent changes writing into menu_items could be silently overwritten. Add a comment documenting this ordering dependency.

MegaMenu.php:63–65 — No combined test for menu_name + menu_items in one call
menu_name is applied by the parent; menu_items by the child. No single test exercises both in one call — a regression in the re-read sequence would pass individually but break combined.

---

🔵 Suggestions

MegaMenu.php:21–22 — Icon key paths are magic strings. Extract as private constants.

MegaMenuTest.php:85–99 — Add a combined menu_name + menu_items translation test.

MegaMenuTest.php — Add a test for menu_items entries without _id to document expected behavior.

---

🟣 Questions

MegaMenu.php:44–50 — Is extracting only item_title the intentional scope for this iteration, or are other fields (badge, description, tooltip, ARIA) being deferred? A comment would clarify.

MegaMenu.php:29 — Confirm ArrayHelper::structurize handles numeric path segments correctly and that array_replace_recursive doesn't re-index menu_items when writing back.

---

Recommendations

• No production logging — setTargetContent drops translations silently when no matching _id is found. Add at least debug-level logging via the existing LoggerSafeTrait. Critical for diagnosing translation pipeline issues in production.
• setTargetContent override pattern inconsistency — Gallery skips parent::setTargetContent entirely; MegaMenu calls it. This asymmetry leaves icon/dynamic-tag handling inconsistent across sibling elements. Consider a documented contract or template-method hook.
• Elements/ directory writability — ElementFactory auto-discovers any PHP file in the directory. Ensure it's not writable by untrusted processes.

---

Assessment

Ready to merge? With fixes

Reasoning: The core logic is sound and happy-path tests pass, but the _id-fallback mismatch creates a silent translation-loss bug, and the unrestricted property write is an input-validation gap in a pipeline that can receive external translation data.

---

🤖 Generated with Claude Code

[vsolovei-smartling]

@vsolovei-smartling this is generated with /sl-core-dev:claude-pr-review-local cc skill. This is exactly the same skill that runs PR reviews for the private GH repos. Try it. First time your run it it will guide you through the installation process.
I've been using the sl-core-dev:common:pull-request-review, that gave verdict of minor consistency issues Neither is a blocker — the PR is ready to merge as-is, but the array_key_exists change would align with the rest of the codebase

[sl-mmuradov]

I took a look at the PR in static analyzer mode ) No issues were found.