feat: resolve routes by role and link to them in the client - #320
Merged
Merged
Conversation
Closes openedx#317 and openedx#319. Co-Authored-By: Claude <noreply@anthropic.com>
arbrandes
force-pushed
the
arbrandes/role-links
branch
from
September 22, 2026 21:15
005f3e3 to
45ee032
Compare
|
🎉 This PR is included in version 2.0.0-alpha.15 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This was referenced Sep 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
LinkMenuItemrendered every variant as a plain anchor, so a header link whose role an installed app provides, such as Account once openedx/frontend-app-account#1470 lands, reloaded the whole shell instead of navigating within it;Logodid the same for the home role. Both now spreadgetLinkProps, which renders a react-routerLinkfor a path in this site and a plain anchor otherwise, the way the Login and Register buttons have since #305. The helper moves out of the anonymous menu intoruntime/routing, together withisInternalUrland a newresolveRouteByRole(role, params)that returns the resolvedurlwithisInternal. An app route's path has its params filled and a trailing splat dropped the way react-router'sgeneratePathdoes, so an omitted optional param disappears and a missing required one throws; an external URL is returned as configured. This replaces the logic frontend-app-gradebook and frontend-app-instructor-dashboard each hand-rolled for their cross-app links. All three are exported from the runtime and documented in the migration how-to, which also had the export misspelled asgetUrlForRouteRole.The
navLinkvariant keeps itsactivecomputation, and a link is still hidden when nothing provides its role. Tests cover the helpers and everyLinkMenuItemvariant navigating in the client for an app route and through an anchor for an external one; theLogotests now render inside a router, as the component does in the shell.Follow-ups: migrate the two apps above to
resolveRouteByRole(the copies came in with openedx/frontend-app-gradebook#627 and openedx/frontend-app-instructor-dashboard#246), and #318 for a pending indicator, now that header links soft-navigate into lazily loaded apps.Closes #317. Closes #319.
LLM usage notice
Built with assistance from Claude.