Repository navigation
fix(menu): remove the dead Reports menu item by its link alone (#2136) - #2139
Conversation
The dead "Reports" item can carry another E2E id and no template after a menu save re-created it, so the Link + E2EId match from the first fix left it behind. Menu items and templates are now matched on the dead link alone (with or without a trailing slash); child rows go with them, items hanging on a removed template are detached, and a second start is a no-op. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The broadened permission cleanup and template detachment paths lack coverage for alternate E2E IDs.
Review effort: Balanced
Findings: 1
What changed in this PR
Broadens startup cleanup of the obsolete Reports menu route.
Changes:
- Matches dead items/templates by link, including a trailing slash.
- Adds integration coverage for alternate E2E IDs and link lookalikes.
| File | Description |
|---|---|
DeadReportsMenuCleaner.cs |
Broadens dead-link cleanup criteria. |
DeadReportsMenuCleanupTests.cs |
Tests expanded cleanup behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| var renamedTemplate = new MenuTemplate | ||
| { | ||
| Name = "Reports", | ||
| E2EId = "example-reports-template", | ||
| DefaultLink = DeadReportsMenuCleaner.Link, |
There was a problem hiding this comment.
Considered, not acted on: this is a coverage suggestion, not a correctness defect. The permission-removal and detachment queries use the same link-only predicate as the item/template queries this test does pin down, and both relationships are already exercised end-to-end (with permission and detached item) by PluginStart_RemovesTheDeadReportsEntry_AndASecondStartIsANoOp.
Summary
Follow-up to #2137. The dead "Reports" menu item (
/plugins/items-planning-pn/reports) can exist with a different E2E id (e.g.advanced) and no menu template, because an earlier menu save re-created it. #2137 only removed rows matching both Link and E2EId, so this shape survived.DeadReportsMenuCleanernow matches:Translations and security groups/permissions are removed as before; items still pointing at a removed template are detached; a second start changes nothing; removed ids are logged.
Note: a menu item a user created by hand with exactly this link is removed too — intentionally, since no route answers that link.
Tests
DeadReportsMenuCleanupTests.PluginStart_RemovesTheDeadLink_WhateverItsE2EIdAndWithoutATemplate: the "E2E idadvanced, no template" shape, a trailing-slash variant, a template on the dead link with another E2E id — all removed with their child rows; a user item at/plugins/items-planning-pn/reports-archive(E2E idadvanced) stays untouched; second start is a no-op.Not verified locally
Tests run in CI only; locally
dotnet buildof the test project succeeded.Refs #2136
🤖 Generated with Claude Code