From f81b6641cb52aab6b99c8c5007a53058b031de35 Mon Sep 17 00:00:00 2001 From: Rex Lorenzo Date: Mon, 10 Aug 2026 03:49:14 -0700 Subject: [PATCH 1/2] VPR-158 fix(raps): confirm role template saves and keep nav highlighted - Saving roles on a template now returns to the listing with a success message instead of silently staying put - Role Templates stays highlighted on the edit and apply pages, which have no nav entry of their own, via NavMenuItem.ChildPageURLs - Move nav match logic out of the Razor view into LeftNavHighlight so it is unit testable, and cover the existing instance-link behaviour - Disable submit until both role reads succeed; a failed read left the selection empty and saving wiped the template's roles - Apply the same missing-confirmation fix to the delegate roles page --- eslint.config.mjs | 2 + test/Classes/LeftNavHighlightTests.cs | 173 ++++++++++++++++++ web/Areas/RAPS/Controllers/RAPSController.cs | 8 +- .../RAPS/Views/Roles/DelegateRoles.cshtml | 23 ++- .../RAPS/Views/Roles/TemplateRoles.cshtml | 43 +++-- web/Areas/RAPS/Views/Roles/Templates.cshtml | 1 + web/Classes/LeftNavHighlight.cs | 108 +++++++++++ web/Classes/NavMenuItem.cs | 6 + .../Shared/Components/LeftNav/Default.cshtml | 52 +----- web/wwwroot/js/qtable.js | 37 ++++ web/wwwroot/js/site.js | 4 + 11 files changed, 389 insertions(+), 68 deletions(-) create mode 100644 test/Classes/LeftNavHighlightTests.cs create mode 100644 web/Classes/LeftNavHighlight.cs diff --git a/eslint.config.mjs b/eslint.config.mjs index 48af0f47a..d5c1b7075 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -83,6 +83,8 @@ export default [ getItemFromStorage: "readonly", putItemInStorage: "readonly", showStatusNotification: "readonly", + queueStatusNotification: "readonly", + showQueuedStatusNotification: "readonly", Quasar: "readonly", }, }, diff --git a/test/Classes/LeftNavHighlightTests.cs b/test/Classes/LeftNavHighlightTests.cs new file mode 100644 index 000000000..6154f3e48 --- /dev/null +++ b/test/Classes/LeftNavHighlightTests.cs @@ -0,0 +1,173 @@ +using Viper.Classes; + +namespace Viper.test.Classes +{ + public class LeftNavHighlightTests + { + // Stands in for IUrlHelper.Content. The prefix mimics the "/2" PathBase used on + // TEST and PROD, so resolution is exercised the way it behaves off the app root. + private const string PathBase = "/2"; + private static string ResolveAppPath(string url) => PathBase + url.TrimStart('~'); + + private static (int ActiveIndex, int SecondaryActiveIndex) FindActive(List items, string requestPath) + => LeftNavHighlight.FindActive(items, requestPath, ResolveAppPath); + + private static NavMenuItem Link(string url) => new() { MenuItemText = url, MenuItemURL = url }; + + [Fact] + public void FindActive_RelativeLinkForCurrentPage_IsPrimary() + { + var items = new List { Link("Rolelist"), Link("RoleTemplateList") }; + + var (active, secondary) = FindActive(items, "/2/raps/Viper/RoleTemplateList"); + + Assert.Equal(1, active); + Assert.Equal(-1, secondary); + } + + [Fact] + public void FindActive_ChildPage_HighlightsParentItem() + { + // VPR-158: RoleTemplateRoles has no nav entry, so the Role Templates item stays lit. + var items = new List + { + Link("Rolelist"), + new() { MenuItemText = "Role Templates", MenuItemURL = "RoleTemplateList", ChildPageURLs = { "RoleTemplateRoles", "RoleTemplateApply" } } + }; + + var (active, secondary) = FindActive(items, "/2/raps/Viper/RoleTemplateRoles"); + + Assert.Equal(1, active); + Assert.Equal(-1, secondary); + } + + [Fact] + public void FindActive_ChildPageWithQueryString_HighlightsParentItem() + { + // The child page is always reached with ?roleTemplateId=, which is not part of the path. + var items = new List + { + new() { MenuItemText = "Role Templates", MenuItemURL = "RoleTemplateList", ChildPageURLs = { "RoleTemplateApply?roleTemplateId=1" } } + }; + + var (active, _) = FindActive(items, "/2/raps/Viper/RoleTemplateApply"); + + Assert.Equal(0, active); + } + + [Fact] + public void FindActive_UnrelatedPage_HighlightsNothing() + { + var items = new List + { + Link("Rolelist"), + new() { MenuItemText = "Role Templates", MenuItemURL = "RoleTemplateList", ChildPageURLs = { "RoleTemplateRoles" } } + }; + + var (active, secondary) = FindActive(items, "/2/raps/Viper/AuditTrail"); + + Assert.Equal(-1, active); + Assert.Equal(-1, secondary); + } + + [Fact] + public void FindActive_OnlyInstanceLinkMatches_IsPromotedToPrimary() + { + var items = new List { Link("~/raps/Viper/RoleList"), Link("~/raps/VMACS.VMTH/RoleList") }; + + var (active, secondary) = FindActive(items, "/2/raps/Viper/RoleList"); + + Assert.Equal(0, active); + Assert.Equal(-1, secondary); + } + + [Fact] + public void FindActive_PageAndInstanceLinkMatch_PageLinkIsPrimary() + { + var items = new List { Link("~/raps/Viper/Rolelist"), Link("Rolelist") }; + + var (active, secondary) = FindActive(items, "/2/raps/Viper/Rolelist"); + + Assert.Equal(1, active); + Assert.Equal(0, secondary); + } + + [Fact] + public void FindActive_MatchIsCaseInsensitiveAndIgnoresTrailingSlash() + { + var items = new List { Link("rolelist/") }; + + var (active, _) = FindActive(items, "/2/raps/Viper/RoleList"); + + Assert.Equal(0, active); + } + + [Fact] + public void FindActive_ExternalAndEmptyUrls_NeverMatch() + { + var items = new List + { + new() { MenuItemText = "Header", MenuItemURL = "" }, + Link("https://ucdavis.edu/2/raps/Viper/RoleList"), + Link("mailto:someone@ucdavis.edu") + }; + + var (active, secondary) = FindActive(items, "/2/raps/Viper/RoleList"); + + Assert.Equal(-1, active); + Assert.Equal(-1, secondary); + } + + [Fact] + public void FindActive_UnresolvableInstanceLink_NeverMatches() + { + // IUrlHelper.Content is nullable, so a URL it cannot resolve must not be + // compared against the request path as if it had resolved to nothing. + var items = new List { Link("~/raps/Viper/RoleList") }; + + var (active, secondary) = LeftNavHighlight.FindActive(items, "/2/raps/Viper/RoleList", _ => null); + + Assert.Equal(-1, active); + Assert.Equal(-1, secondary); + } + + [Fact] + public void FindActive_NullChildPageUrl_IsSkippedNotThrown() + { + // CMS nav URLs come from a nullable column. A null must be skipped so the + // remaining child URLs are still considered and the nav still renders. + var items = new List + { + new() { MenuItemText = "Role Templates", MenuItemURL = "RoleTemplateList", ChildPageURLs = { null!, "RoleTemplateRoles" } } + }; + + var (active, _) = FindActive(items, "/2/raps/Viper/RoleTemplateRoles"); + + Assert.Equal(0, active); + } + + [Fact] + public void FindActive_RequestPathWithTrailingSlash_StillMatches() + { + var items = new List { Link("RoleList") }; + + var (active, _) = FindActive(items, "/2/raps/Viper/RoleList/"); + + Assert.Equal(0, active); + } + + [Fact] + public void FindActive_RootRelativeUrl_MatchesWithoutBasePath() + { + // Regression guard, and not as trivial as it looks: Uri parses a leading-slash + // path as an absolute file:// URI on Unix but not on Windows, so ordering the + // absolute-URL check before the root-relative one passes on a dev machine and + // fails on the Linux CI runner. + var items = new List { Link("/2/raps/Viper/RoleList") }; + + var (active, _) = FindActive(items, "/2/raps/Viper/RoleList"); + + Assert.Equal(0, active); + } + } +} diff --git a/web/Areas/RAPS/Controllers/RAPSController.cs b/web/Areas/RAPS/Controllers/RAPSController.cs index bd1d7d515..11f3769a4 100644 --- a/web/Areas/RAPS/Controllers/RAPSController.cs +++ b/web/Areas/RAPS/Controllers/RAPSController.cs @@ -128,7 +128,13 @@ public async Task Nav(int? roleId, int? permissionId, string? memberId, } if (_securityService.IsAllowedTo("ViewRoles", instance)) { - nav.Add(new NavMenuItem { MenuItemText = "Role Templates", MenuItemURL = "RoleTemplateList" }); + nav.Add(new NavMenuItem + { + MenuItemText = "Role Templates", + MenuItemURL = "RoleTemplateList", + //these pages are reached from the template listing and have no nav entry of their own + ChildPageURLs = { "RoleTemplateRoles", "RoleTemplateApply" } + }); } if (selectedRole != null && RAPSSecurityService.RoleBelongsToInstance(instance, selectedRole)) { diff --git a/web/Areas/RAPS/Views/Roles/DelegateRoles.cshtml b/web/Areas/RAPS/Views/Roles/DelegateRoles.cshtml index 2b1818408..dbbd907d0 100644 --- a/web/Areas/RAPS/Views/Roles/DelegateRoles.cshtml +++ b/web/Areas/RAPS/Views/Roles/DelegateRoles.cshtml @@ -37,28 +37,29 @@ methods: { loadRoles: async function() { this.rolesLoaded = false - var childRoles = await viperFetch(this, "Roles/ControlledBy/" + + this.urlParams.get("roleId")) - var allRoles = await viperFetch(this, "Roles?Application=0") + this.loadingRoles = true + const [childRoles, allRoles] = await Promise.all([ + viperFetch(this, "Roles/ControlledBy/" + this.urlParams.get("roleId")), + viperFetch(this, "Roles?Application=0") + ]) + this.loadingRoles = false // A failed or still-pending read must not look like "nothing is // selected": submitting that would PUT an empty list and wipe the // existing delegations. Stay disabled until both reads succeed. if (childRoles === undefined || allRoles === undefined) { return } + const childRoleIds = new Set(childRoles.map(cr => cr.roleId)) this.selectedRoles = childRoles - this.roles = childRoles.concat( - allRoles.filter(r => childRoles.find(cr => cr.roleId === r.roleId) === undefined)) + this.roles = childRoles.concat(allRoles.filter(r => !childRoleIds.has(r.roleId))) this.rolesLoaded = true }, submitChanges: async function() { if (!this.rolesLoaded) { return } - var roleIds = this.selectedRoles.reduce( (result, role) => { - result.push(role.roleId) - return result - }, []) - viperFetch(this, + const roleIds = this.selectedRoles.map(role => role.roleId) + const result = await viperFetch(this, "Roles/ControlledBy/" + this.urlParams.get("roleId"), { method: "PUT", @@ -67,12 +68,14 @@ }, [this.loadRoles] ) + if (result !== undefined) { + showStatusNotification("Delegated roles updated") + } } }, async mounted() { await this.loadRoles() this.role = (await viperFetch(this, "Roles/" + this.urlParams.get("roleId"))) ?? {} - } }) diff --git a/web/Areas/RAPS/Views/Roles/TemplateRoles.cshtml b/web/Areas/RAPS/Views/Roles/TemplateRoles.cshtml index 89f742c15..10002d5b8 100644 --- a/web/Areas/RAPS/Views/Roles/TemplateRoles.cshtml +++ b/web/Areas/RAPS/Views/Roles/TemplateRoles.cshtml @@ -12,7 +12,7 @@ :pagination="{rowsPerPage:0}" :loading="loadingRoles">