Repository navigation
Open
Description
External routes ignore params. resolveRouteByRole(role, { courseId }) uses courseId when an installed app provides the role, and ignores it when an external route does.
Found while porting Learning's progress tab (openedx/frontend-app-learning#2190), which links to course pages by role. Related to #348.
Note
The details below were generated by Claude (Claude Code) and reviewed before posting.
Claude Plan
How a role with params resolves today
- A role is declared on a route, and the route's path decides which params the role takes:
/gradebook/:courseIdcarriesorg.openedx.frontend.role.gradebook(src/routes.tsxin openedx/frontend-app-gradebook),instructor-dashboard/:courseIdcarriesorg.openedx.frontend.role.instructorDashboard(src/routes.tsxin openedx/frontend-app-instructor-dashboard). - A caller passes those params:
resolveRouteByRole(gradebookRole, { courseId })in the instructor dashboard,resolveRouteByRole(instructorDashboardRole, { courseId })in gradebook. resolveRouteByRolereturnsisInternal ? generatePath(path, params) : path. A path in this site has its params filled; for anything else the params are ignored, with no warning, and the URL comes back unchanged. The TSDoc says so ("an external URL is returned as configured"), and a test pins it by passing{ courseId: 'c1' }for a role whose external URL ishttps://apps.example.com/profile/and expecting that URL back.ExternalRouteis a role and a URL. An external route written ashttps://lms.example.com/courses/:courseId/progresscomes back with the literal:courseIdin it.- The caller cannot tell which case it is in before it calls: who provides a role is a property of the site, not of the role.
- One exception follows from the same line: an external route written as a path (a single leading
/) is classed as internal byisInternalUrl, so its params are filled, and it is linked through react-router.
What the docs say
- The migration guide's URL Config changes presents the two as interchangeable: URLs "should now be expressed as part of the
appsconfig as route roles, or asexternalRoutes, and resolved in code viaresolveRouteByRole(). It substitutes route params, drops a trailing splat, and reports whether the result is a path in this site or an external URL". It does not say the substitution applies to one of the two only. - No ADR in
docs/decisions/covers route roles with params or external routes; 0013 mentions roles for the course bar'sprovides.
How it got here
- feat: resolve routes by role and link to them in the client #320 added
resolveRouteByRole(closing Provide a helper for linking to another app's route by role #317 and Header links to in-site routes trigger full page loads #319) to replace logic two apps had each written by hand. Both substituted params into whatever the role resolved to, an external URL included: fix: navigate to and from the app without full page loads frontend-app-gradebook#627 calledgetUrlByRouteRole(instructorDashboardRole)?.replace(':courseId', courseId), and fix: use an SPA link for "view studio roles" when the route exists frontend-app-instructor-dashboard#246'ssrc/utils/routeByRole.tsranurl.replace(`:${name}`, value)for each param before it tested whether the URL was absolute. Neither had a test of an external URL with a param. - feat: resolve routes by role and link to them in the client #320's description states the new behavior ("an external URL is returned as configured") and gives no reason; the PR has no review comments.
- react-router's
generatePath, which feat: resolve routes by role and link to them in the client #320 uses for the substitution, does not take an absolute URL: on react-router 6.30.6,generatePath('https://lms.example.com/courses/:courseId/progress', { courseId })returnshttps:/lms.example.com/courses/<courseId>/progress, the//collapsed.
Where it bites
- Both call sites above keep a URL of their own for when the role resolves to nothing (
gradebookRoute?.url ?? data.gradebookUrl,dashboardRoute?.url ?? instructorDashboardUrl(courseId)), and that fallback is the only way either reaches a destination outside the site with the course in it. A site that declares an external route for one of those roles replaces the fallback with a URL that cannot name the course. - The shell's own
ProfileLinkMenuItemreaches a user's profile by appending/u/<username>to the profile role's URL, which works for an app route and an external route alike because the role's route takes no param: openedx/frontend-app-profile declares the role onprofileand serves the user's page from its childu/:username(src/routes.tsx). - Learning declares
org.openedx.frontend.role.courseOutline,courseDates,courseProgressandcoursewareon routes undercourse/:courseId, and its progress tab links to them by role (Port the progress tab to frontend-base frontend-app-learning#2190). If A site cannot override an installed app's route for a role with an external route #348 lets a site point one of them at an external route, the link the site gets has no course in it.
Questions
- Should a site be able to provide a role whose route takes params with an external route?
- If so, how does an external route say where the params go?
- Until then, where does an app author declaring a role, or an operator writing
externalRoutes, learn that params are dropped for an external URL? - Should the resolver report it when it is handed params it will not use?
Metadata
Metadata
Assignees
Labels
No labels
Type
Projects
- StatusShow more project fieldsTodo
Activity