From a727ec1ea84281600587de843ee900dd2f1f2ac7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Schultz=20Madsen?= Date: Wed, 7 Oct 2026 11:47:36 +0200 Subject: [PATCH] fix(menu): remove the dead Reports menu item by its link alone (#2136) 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 --- .../DeadReportsMenuCleanupTests.cs | 96 ++++++++++++++++++- .../Helpers/DeadReportsMenuCleaner.cs | 22 +++-- 2 files changed, 108 insertions(+), 10 deletions(-) diff --git a/eFormAPI/Plugins/ItemsPlanning.Pn/ItemsPlanning.Pn.Test/DeadReportsMenuCleanupTests.cs b/eFormAPI/Plugins/ItemsPlanning.Pn/ItemsPlanning.Pn.Test/DeadReportsMenuCleanupTests.cs index 202a4db9..52a6f527 100644 --- a/eFormAPI/Plugins/ItemsPlanning.Pn/ItemsPlanning.Pn.Test/DeadReportsMenuCleanupTests.cs +++ b/eFormAPI/Plugins/ItemsPlanning.Pn/ItemsPlanning.Pn.Test/DeadReportsMenuCleanupTests.cs @@ -41,7 +41,8 @@ namespace ItemsPlanning.Pn.Test; /// /// Plugin start removes the dead "Reports" menu entry (#2136) from the host's Angular database: the item, -/// its template and their child rows go; a second start changes nothing; other menu entries are untouched. +/// its template and their child rows go, whatever E2E id they carry; a second start changes nothing; other +/// menu entries, including ones whose link merely starts like the dead one, are untouched. /// Runs against a real MariaDB, the way the host calls it. /// [TestFixture] @@ -169,6 +170,99 @@ public async Task PluginStart_RemovesTheDeadReportsEntry_AndASecondStartIsANoOp( "a second start changes nothing"); } + [Test] + public async Task PluginStart_RemovesTheDeadLink_WhateverItsE2EIdAndWithoutATemplate() + { + int advancedItemId, trailingSlashItemId, lookalikeItemId, renamedTemplateId; + await using (var db = NewDbContext()) + { + var securityGroup = new SecurityGroup { Name = "Example group" }; + db.SecurityGroups.Add(securityGroup); + await db.SaveChangesAsync(); + + // The shape a menu save leaves behind: the dead link under the "advanced" E2E id, no template. + var advanced = new MenuItem + { + Name = "Reports", + E2EId = "advanced", + Link = DeadReportsMenuCleaner.Link, + Type = MenuItemTypeEnum.Link, + Translations = { new MenuItemTranslation { Name = "Reports", LocaleName = "en-US", Language = "English" } }, + MenuItemSecurityGroups = { new MenuItemSecurityGroup { SecurityGroup = securityGroup } } + }; + var trailingSlash = new MenuItem + { + Name = "Reports", + E2EId = "example-reports", + Link = DeadReportsMenuCleaner.Link + "/", + Type = MenuItemTypeEnum.Link + }; + // A user's own entry whose link only starts like the dead one: it stays. + var lookalike = new MenuItem + { + Name = "Example reports archive", + E2EId = "advanced", + Link = DeadReportsMenuCleaner.Link + "-archive", + Type = MenuItemTypeEnum.Link, + Translations = + { + new MenuItemTranslation { Name = "Example reports archive", LocaleName = "en-US", Language = "English" } + }, + MenuItemSecurityGroups = { new MenuItemSecurityGroup { SecurityGroup = securityGroup } } + }; + // A template on the dead link under another E2E id goes too. + var renamedTemplate = new MenuTemplate + { + Name = "Reports", + E2EId = "example-reports-template", + DefaultLink = DeadReportsMenuCleaner.Link, + Translations = + { + new MenuTemplateTranslation { Name = "Reports", LocaleName = "en-US", Language = "English" } + } + }; + db.MenuItems.AddRange(advanced, trailingSlash, lookalike); + db.MenuTemplates.Add(renamedTemplate); + await db.SaveChangesAsync(); + + advancedItemId = advanced.Id; + trailingSlashItemId = trailingSlash.Id; + lookalikeItemId = lookalike.Id; + renamedTemplateId = renamedTemplate.Id; + } + + var before = await SnapshotAsync(); + + new EformItemsPlanningPlugin().Configure(_appBuilder); + + var after = await SnapshotAsync(); + Assert.That(after, Is.EqualTo(before with + { + MenuItemIds = before.MenuItemIds.Where(x => x != advancedItemId && x != trailingSlashItemId).ToList(), + MenuItemTranslations = before.MenuItemTranslations - 1, + MenuItemSecurityGroups = before.MenuItemSecurityGroups - 1, + MenuTemplates = before.MenuTemplates - 1, + MenuTemplateTranslations = before.MenuTemplateTranslations - 1 + }).Using(SameSnapshot)); + + await using (var db = NewDbContext()) + { + Assert.That(await db.MenuTemplates.AnyAsync(x => x.Id == renamedTemplateId), Is.False); + var kept = await db.MenuItems.SingleAsync(x => x.Id == lookalikeItemId); + Assert.That(kept.Link, Is.EqualTo(DeadReportsMenuCleaner.Link + "-archive")); + Assert.That(kept.E2EId, Is.EqualTo("advanced")); + Assert.That(await db.MenuItemTranslations.CountAsync(x => x.MenuItemId == lookalikeItemId), + Is.EqualTo(1)); + Assert.That(await db.MenuItemSecurityGroups.CountAsync(x => x.MenuItemId == lookalikeItemId), + Is.EqualTo(1)); + } + + new EformItemsPlanningPlugin().Configure(_appBuilder); + + Assert.That(await SnapshotAsync(), Is.EqualTo(after).Using(SameSnapshot), + "a second start changes nothing"); + } + [Test] public void PluginStart_WithoutTheAngularDatabase_DoesNotThrow() { diff --git a/eFormAPI/Plugins/ItemsPlanning.Pn/ItemsPlanning.Pn/Infrastructure/Helpers/DeadReportsMenuCleaner.cs b/eFormAPI/Plugins/ItemsPlanning.Pn/ItemsPlanning.Pn/Infrastructure/Helpers/DeadReportsMenuCleaner.cs index 926529fb..b216472a 100644 --- a/eFormAPI/Plugins/ItemsPlanning.Pn/ItemsPlanning.Pn/Infrastructure/Helpers/DeadReportsMenuCleaner.cs +++ b/eFormAPI/Plugins/ItemsPlanning.Pn/ItemsPlanning.Pn/Infrastructure/Helpers/DeadReportsMenuCleaner.cs @@ -32,21 +32,24 @@ namespace ItemsPlanning.Pn.Infrastructure.Helpers; /// The "reports" page left the Angular module in 2023, but the plugin kept registering a "Reports" menu /// entry for it. Clicking it matches no route and the frontend's catch-all silently lands on "My eForms" /// (#2136). The host only ever adds plugin menu entries, so the plugin removes its own dead one on start. -/// Only rows that carry both the dead link and its E2E id are touched; a second run finds nothing. +/// Rows are matched on the dead link alone (with or without a trailing slash): a menu save can re-create +/// the item with another E2E id and no template, and no live page answers that link anyway. A second run +/// finds nothing. /// public static class DeadReportsMenuCleaner { public const string Link = "/plugins/items-planning-pn/reports"; public const string E2EId = "items-planning-pn-reports"; + private const string LinkWithTrailingSlash = Link + "/"; /// The number of menu items plus menu templates removed. public static int Remove(BaseDbContext dbContext, ILogger logger) { var menuItems = dbContext.MenuItems - .Where(x => x.Link == Link && x.E2EId == E2EId) + .Where(x => x.Link == Link || x.Link == LinkWithTrailingSlash) .ToList(); var menuTemplates = dbContext.MenuTemplates - .Where(x => x.DefaultLink == Link && x.E2EId == E2EId) + .Where(x => x.DefaultLink == Link || x.DefaultLink == LinkWithTrailingSlash) .ToList(); if (menuItems.Count == 0 && menuTemplates.Count == 0) @@ -57,24 +60,25 @@ public static int Remove(BaseDbContext dbContext, ILogger logger) // Child rows are matched through their parent's link rather than an id list, so the queries need no // IN (...) translation. dbContext.MenuItemTranslations.RemoveRange(dbContext.MenuItemTranslations - .Where(x => x.MenuItem.Link == Link && x.MenuItem.E2EId == E2EId)); + .Where(x => x.MenuItem.Link == Link || x.MenuItem.Link == LinkWithTrailingSlash)); dbContext.MenuItemSecurityGroups.RemoveRange(dbContext.MenuItemSecurityGroups - .Where(x => x.MenuItem.Link == Link && x.MenuItem.E2EId == E2EId)); + .Where(x => x.MenuItem.Link == Link || x.MenuItem.Link == LinkWithTrailingSlash)); dbContext.MenuItems.RemoveRange(menuItems); // A menu item that still points at a removed template would block the delete; detach it the way // the host does when it removes a plugin's templates. foreach (var remaining in dbContext.MenuItems - .Where(x => x.MenuTemplate.DefaultLink == Link && x.MenuTemplate.E2EId == E2EId) - .Where(x => x.Link != Link || x.E2EId != E2EId)) + .Where(x => x.MenuTemplate.DefaultLink == Link + || x.MenuTemplate.DefaultLink == LinkWithTrailingSlash) + .Where(x => !(x.Link == Link || x.Link == LinkWithTrailingSlash))) { remaining.MenuTemplateId = null; } dbContext.MenuTemplateTranslations.RemoveRange(dbContext.MenuTemplateTranslations - .Where(x => x.MenuTemplate.DefaultLink == Link && x.MenuTemplate.E2EId == E2EId)); + .Where(x => x.MenuTemplate.DefaultLink == Link || x.MenuTemplate.DefaultLink == LinkWithTrailingSlash)); dbContext.MenuTemplatePermissions.RemoveRange(dbContext.MenuTemplatePermissions - .Where(x => x.MenuTemplate.DefaultLink == Link && x.MenuTemplate.E2EId == E2EId)); + .Where(x => x.MenuTemplate.DefaultLink == Link || x.MenuTemplate.DefaultLink == LinkWithTrailingSlash)); dbContext.MenuTemplates.RemoveRange(menuTemplates); dbContext.SaveChanges();