Repository navigation
Fix nested group URLs in the admin console - #52389
Conversation
Signed-off-by: George Taylor <3026030+georgepstaylor@users.noreply.github.com>
c30f4c0 to
c741fe6
Compare
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Fixes nested group navigation in the admin console by avoiding React Router’s dynamic param encoding of / for hierarchical group IDs and adding regression tests to prevent regressions.
Changes:
- Update
toGroups()to generate URLs via the wildcard route (*) so nested IDs likeparent/childremain unencoded. - Add Vitest regression coverage for realm and organization nested group URL generation.
- Add a test for root groups URL generation when no
idis provided.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| js/apps/admin-ui/src/groups/routes/Groups.tsx | Switch URL generation to use the wildcard (*) param so nested group paths keep / separators. |
| js/apps/admin-ui/src/groups/tests/Groups.test.ts | Add regression tests covering nested realm/org group URL generation and root groups URL. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
LeftNav resolves access by matching registered routes to nav paths. After removing the legacy groups :id route, the remaining splat route no longer matched /groups, so the Groups sidebar item was hidden. Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
There was a problem hiding this comment.
🟢 Approval recommended
The focused implementation addresses the regression and includes appropriate coverage for both affected routing contexts.
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 0 new
- Review effort level: Balanced
* Fix nested group URL generation * removue unused group with id route * Fix Groups nav item matching for splat routes LeftNav resolves access by matching registered routes to nav paths. After removing the legacy groups :id route, the remaining splat route no longer matched /groups, so the Groups sidebar item was hidden. --------- (cherry picked from commit cbb99d4) Signed-off-by: George Taylor <3026030+georgepstaylor@users.noreply.github.com> Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com> Co-authored-by: George Taylor <3026030+georgepstaylor@users.noreply.github.com>
* Fix nested group URL generation * removue unused group with id route * Fix Groups nav item matching for splat routes LeftNav resolves access by matching registered routes to nav paths. After removing the legacy groups :id route, the remaining splat route no longer matched /groups, so the Groups sidebar item was hidden. --------- (cherry picked from commit cbb99d4) Signed-off-by: George Taylor <3026030+georgepstaylor@users.noreply.github.com> Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com> Co-authored-by: George Taylor <3026030+georgepstaylor@users.noreply.github.com>
Fixes #52241
Clicking a child group in the admin console currently builds a URL with
%2Fbetween the parent and child IDs, so the group details page fails to load.This started with 26.7.3 when PR #52136 upgraded React Router from 6.30.4 to 7.18.2 (commit). React Router 7 now escapes
/in:idparameters. Since the group URL builder passes the whole hierarchy as one ID, the separator gets escaped too.The fix builds the nested path as separate URL segments instead of passing the whole hierarchy through a single
:idparameter. Tests cover both realm and organization group paths.Tested with:
vitest run src/groups/__tests__/Groups.test.ts