Do not override a resolved menu item URL with a localized one

[AIC-BV]

Problem

Winter\Pages\Classes\Menu resolves a menu item's URL from its reference for every type but url, which owns its URL:

if ($item->type == 'url') {
    $parentReference->url = $item->url;
} else {
    $itemInfo = Event::fire('pages.menuitem.resolveItem', [$item->type, $item, $currentUrl, $this->theme], true);
    if (!$item->replace && isset($itemInfo['url'])) {
        $parentReference->url = $itemInfo['url'];
    }
}

The pages.menu.referencesGenerated listener added by this plugin then applies every localized field, including url, without that distinction — so a stored localized url silently overrides the URL that was just resolved from the reference.

EventRegistry::registerMenuItemTranslation() makes url translatable for every item type, but the backend form only shows the URL field for the url type. That makes the stored value invisible and unremovable through the UI: change an item from "URL" to a reference type and the localized URLs stay behind in viewBag.locale.<code>.url, and from then on the item keeps linking to the old address in those locales while the form shows a reference.

That is how we ran into it: a footer item was converted from a URL to a blog-detail reference. The form showed the reference, the front-end kept emitting the old path for one locale, and there was no field to clear.

Change

Skip the url field when the item's type is not url. Localized titles keep working for every type, and a url item keeps its localized URL.

Tests

tests/unit/MenuItemLocalizationTest.php covers the four cases: a localized title is applied, a localized URL is applied to a url item, a localized URL is ignored when the URL came from a reference, and nested items are treated the same way. The two reference cases fail without the change.

The fixtures stand in for MenuItemReference with stdClass, so the test does not require Winter.Pages to be installed.

Summary by CodeRabbit

• Bug Fixes

  • Menu items now retain their correctly resolved URLs for page, post, and other reference-based types.
  • Localized URLs are applied only to menu items explicitly configured as URL entries.
  • Nested menu items follow the same corrected URL behavior.

• Tests

  • Added coverage for localized titles, URL entries, reference-based URLs, and nested menu items.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: ed60184d-acf5-470f-9b35-0fb77f5c2730

📥 Commits

Reviewing files that changed from the base of the PR and between 1144951 and 2d1e5c2.

📒 Files selected for processing (2)

• Plugin.php
• tests/unit/MenuItemLocalizationTest.php

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

Walkthrough

The pages.menu.referencesGenerated handler now skips localized url fields for non-url menu items. Reference-resolved URLs remain unchanged, while localized URLs still apply to url items. New PHPUnit tests cover localized titles, URL items, stale URLs, and nested menu items.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Merge Risk: ⚪ Minimal · up to 2d1e5

Reference-based menu items keep their resolved URLs while direct URL items retain localized URLs, with coverage for the changed behavior; no merge-blocking risk is identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: preventing a localized URL from replacing a resolved menu item URL.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

Warning

Some tools did not complete. Review the errors below.

🔧 ast-grep (0.45.3)

Plugin.php

ast-grep timed out on this file

🔧 PHPStan (2.2.12)

Composer install failed: dependency resolution error. Check composer.json and composer.lock for version constraints.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[AIC-BV]

In short,

I once set my Pages > Menu > Item as type URL.
I swapped this last week to my custom type BLOG - DETAIL but the URL was still being used on the frontend because it still has a value filled in (hidden in the form)