diff --git a/docs/how_tos/migrate-frontend-app.md b/docs/how_tos/migrate-frontend-app.md
index dbbe109d..8fc6d5ef 100644
--- a/docs/how_tos/migrate-frontend-app.md
+++ b/docs/how_tos/migrate-frontend-app.md
@@ -738,7 +738,7 @@ Other configuration is now optional, and many values have been given sensible de
URL Config changes
------------------
-Note that the .env files and env.config.js files also include a number of URLs for various micro-frontends and services. These URLs should now be expressed as part of the `apps` config as route roles, and used in code via `getUrlForRouteRole()`. Or as externalRoutes.
+Note that the .env files and env.config.js files also include a number of URLs for various micro-frontends and services. These URLs should now be expressed as part of the `apps` config as route roles, or as `externalRoutes`, and resolved in code via `resolveRouteByRole()`. It substitutes route params, drops a trailing splat, and reports whether the result is a path in this site or an external URL, so a link can stay in the client when it is. `getLinkProps()` turns any URL into the props for a component that accepts `as`, a react-router `Link` for a path in this site and a plain anchor otherwise.
```js
// Creating a route role with for 'example' in an App
@@ -754,8 +754,14 @@ const app: App = {
}],
};
-// Using the role in code to link to the page
-const examplePageUrl = getUrlForRouteRole('example');
+// Using the role in code to link to the page: { url: '/example', isInternal: true }, or null
+// when no app or external route provides it
+const examplePage = resolveRouteByRole('example');
+
+// Rendering the link, so that a route in this site navigates without a page load
+{examplePage && (
+
+)}
```
App-specific config values
diff --git a/runtime/index.ts b/runtime/index.ts
index c0ea2574..018a2de0 100644
--- a/runtime/index.ts
+++ b/runtime/index.ts
@@ -127,9 +127,13 @@ export {
export {
authenticatedLoader,
+ getLinkProps,
getUrlByRouteRole,
- isRoleRouteObject
+ isInternalUrl,
+ isRoleRouteObject,
+ resolveRouteByRole
} from './routing';
+export type { LinkProps, ResolvedRoute } from './routing/utils';
export {
clearAllSubscriptions,
diff --git a/runtime/routing/utils.test.ts b/runtime/routing/utils.test.ts
index 55e3b3bb..83511f52 100644
--- a/runtime/routing/utils.test.ts
+++ b/runtime/routing/utils.test.ts
@@ -1,4 +1,6 @@
-import { getUrlByRouteRole } from './utils';
+import { Link } from 'react-router-dom';
+
+import { getLinkProps, getUrlByRouteRole, isInternalUrl, resolveRouteByRole } from './utils';
import { getSiteConfig } from '../config';
jest.mock('../config');
@@ -60,3 +62,81 @@ describe('getUrlByRouteRole', () => {
expect(getUrlByRouteRole('nonexistent')).toBeNull();
});
});
+
+describe('isInternalUrl', () => {
+ it('accepts a path in this site', () => {
+ expect(isInternalUrl('/account')).toBe(true);
+ expect(isInternalUrl('/')).toBe(true);
+ });
+
+ it('rejects absolute and protocol-relative URLs', () => {
+ expect(isInternalUrl('https://example.com/account')).toBe(false);
+ expect(isInternalUrl('//example.com/account')).toBe(false);
+ expect(isInternalUrl('mailto:help@example.com')).toBe(false);
+ });
+});
+
+describe('resolveRouteByRole', () => {
+ beforeEach(() => {
+ mockGetSiteConfig.mockReturnValue({
+ apps: [{
+ appId: 'test-app',
+ routes: [
+ { path: '/account', handle: { roles: ['account'] } },
+ { path: '/gradebook/:courseId', handle: { roles: ['gradebook'] } },
+ { path: '/learning/:courseId/:unitId?', handle: { roles: ['learning'] } },
+ { path: '/learner-dashboard/*', handle: { roles: ['dashboard'] } },
+ { path: '/*', handle: { roles: ['catch-all'] } },
+ ],
+ }],
+ externalRoutes: [{
+ role: 'profile',
+ url: 'https://apps.example.com/profile/',
+ }],
+ } as any);
+ });
+
+ it('resolves an app route to a path in this site', () => {
+ expect(resolveRouteByRole('account')).toEqual({ url: '/account', isInternal: true });
+ });
+
+ it('substitutes route params, optional ones included', () => {
+ expect(resolveRouteByRole('gradebook', { courseId: 'course-v1:edX+DemoX+Demo_Course' }))
+ .toEqual({ url: '/gradebook/course-v1:edX+DemoX+Demo_Course', isInternal: true });
+ expect(resolveRouteByRole('learning', { courseId: 'c1', unitId: 'u1' }))
+ .toEqual({ url: '/learning/c1/u1', isInternal: true });
+ });
+
+ it('drops an optional param that is not given', () => {
+ expect(resolveRouteByRole('learning', { courseId: 'c1' })).toEqual({ url: '/learning/c1', isInternal: true });
+ });
+
+ it('throws when a required param is not given', () => {
+ expect(() => resolveRouteByRole('gradebook')).toThrow('Missing ":courseId" param');
+ });
+
+ it('drops a trailing splat', () => {
+ expect(resolveRouteByRole('dashboard')).toEqual({ url: '/learner-dashboard', isInternal: true });
+ expect(resolveRouteByRole('catch-all')).toEqual({ url: '/', isInternal: true });
+ });
+
+ it('resolves an external route to its URL as configured', () => {
+ expect(resolveRouteByRole('profile', { courseId: 'c1' }))
+ .toEqual({ url: 'https://apps.example.com/profile/', isInternal: false });
+ });
+
+ it('returns null when nothing provides the role', () => {
+ expect(resolveRouteByRole('nonexistent')).toBeNull();
+ });
+});
+
+describe('getLinkProps', () => {
+ it('links a path in this site through react-router', () => {
+ expect(getLinkProps('/account')).toEqual({ as: Link, to: '/account' });
+ });
+
+ it('links anything else through a plain anchor', () => {
+ expect(getLinkProps('https://example.com/account')).toEqual({ href: 'https://example.com/account' });
+ expect(getLinkProps('//example.com/account')).toEqual({ href: '//example.com/account' });
+ });
+});
diff --git a/runtime/routing/utils.ts b/runtime/routing/utils.ts
index f34c6f4e..ca7c5382 100644
--- a/runtime/routing/utils.ts
+++ b/runtime/routing/utils.ts
@@ -1,4 +1,6 @@
-import { RouteObject } from 'react-router';
+import { ElementType } from 'react';
+import { generatePath, RouteObject } from 'react-router';
+import { Link } from 'react-router-dom';
import { RoleRouteObject } from '../../types';
import { getSiteConfig } from '../config';
@@ -50,3 +52,74 @@ export function getUrlByRouteRole(role: string) {
export function isRoleRouteObject(match: RouteObject): match is RoleRouteObject {
return match.handle !== undefined && 'roles' in match.handle;
}
+
+/**
+ * Whether `url` is a path within this site, one react-router can navigate to, rather than an
+ * absolute or protocol-relative URL.
+ *
+ * @param {string} url
+ * @returns {boolean}
+ */
+export function isInternalUrl(url: string): boolean {
+ return url.startsWith('/') && !url.startsWith('//');
+}
+
+export interface ResolvedRoute {
+ /** The path or URL to link to. */
+ url: string;
+ /** Whether `url` is a path in this site, so a react-router `Link` can navigate to it. */
+ isInternal: boolean;
+}
+
+/**
+ * Resolves the route the site provides for `role`, if any, ready to link to: the path of an
+ * installed app's route, or the URL of an external route, as `getUrlByRouteRole` finds it. An app
+ * route's path has its params filled from `params` and a trailing splat dropped, as react-router's
+ * `generatePath` does, so a missing required param throws; an external URL is returned as
+ * configured. `isInternal` tells the two apart.
+ *
+ * ```
+ * const gradebook = resolveRouteByRole('org.openedx.frontend.role.gradebook', { courseId });
+ * if (gradebook?.isInternal) {
+ * return ;
+ * }
+ * return ;
+ * ```
+ *
+ * @param {string} role
+ * @param {Object.} [params] Values for the route's params, by name.
+ * @returns {ResolvedRoute|null} `null` when no app or external route provides the role.
+ */
+export function resolveRouteByRole(role: string, params: Record = {}): ResolvedRoute | null {
+ const path = getUrlByRouteRole(role);
+ if (path === null) {
+ return null;
+ }
+ const isInternal = isInternalUrl(path);
+ return { url: isInternal ? generatePath(path, params) : path, isInternal };
+}
+
+export interface LinkProps {
+ as?: ElementType;
+ to?: string;
+ href?: string;
+}
+
+/**
+ * Props for linking to `url` from a component that accepts `as`, such as Paragon's `Button`,
+ * `Hyperlink`, `NavLink` or `Dropdown.Item`: a react-router `Link` for a path in this site, so the
+ * navigation stays in the client, and a plain anchor `href` for anything else.
+ *
+ * ```
+ *
+ * ```
+ *
+ * @param {string} url
+ * @returns {LinkProps}
+ */
+export function getLinkProps(url: string): LinkProps {
+ if (isInternalUrl(url)) {
+ return { as: Link, to: url };
+ }
+ return { href: url };
+}
diff --git a/shell/Logo.test.tsx b/shell/Logo.test.tsx
index f3f86c52..c921bb23 100644
--- a/shell/Logo.test.tsx
+++ b/shell/Logo.test.tsx
@@ -1,14 +1,31 @@
import '@testing-library/jest-dom';
-import { render } from '@testing-library/react';
+import { ReactElement } from 'react';
+import { fireEvent, render, screen } from '@testing-library/react';
+import { MemoryRouter, useLocation } from 'react-router-dom';
import { getSiteConfig, mergeSiteConfig, setSiteConfig } from '../runtime/config';
import { SiteConfig } from '../types';
+import { homeRole } from './constants';
import Logo from './Logo';
+function LocationDisplay() {
+ const { pathname } = useLocation();
+ return
{pathname}
;
+}
+
+function renderWithRouter(ui: ReactElement, initialEntry = '/elsewhere') {
+ return render(
+
+ {ui}
+
+
+ );
+}
+
describe('Logo component', () => {
let originalConfig: SiteConfig;
beforeEach(() => {
- // Shallow clone is sufficient here since we only modify headerLogoImageUrl, a top-level field.
+ // Shallow clone is sufficient here since we only modify top-level fields.
originalConfig = { ...getSiteConfig() };
});
@@ -17,39 +34,55 @@ describe('Logo component', () => {
});
it('renders the image with default URL and links to / when no props are provided', async () => {
- const { getByRole } = render();
- const image = getByRole('img');
+ renderWithRouter();
+ const image = screen.getByRole('img');
expect(image).toHaveAttribute('src', 'https://edx-cdn.org/v3/default/logo.svg');
- const link = getByRole('link');
+ const link = screen.getByRole('link');
expect(link).toHaveAttribute('href', '/');
});
it('renders the image with provided imageUrl and links to / by default', async () => {
const testUrl = 'https://example.com/test-logo.svg';
- const { getByRole } = render();
- const image = getByRole('img');
+ renderWithRouter();
+ const image = screen.getByRole('img');
expect(image).toHaveAttribute('src', testUrl);
- const link = getByRole('link');
+ const link = screen.getByRole('link');
expect(link).toHaveAttribute('href', '/');
});
it('renders the image with headerLogoImageUrl when set in site config', async () => {
const configLogoUrl = 'https://example.com/config-logo.svg';
mergeSiteConfig({ headerLogoImageUrl: configLogoUrl });
- const { getByRole } = render();
- const image = getByRole('img');
+ renderWithRouter();
+ const image = screen.getByRole('img');
expect(image).toHaveAttribute('src', configLogoUrl);
- const link = getByRole('link');
+ const link = screen.getByRole('link');
expect(link).toHaveAttribute('href', '/');
});
it('renders the image wrapped in a Hyperlink when destinationUrl is provided', async () => {
const testDestinationUrl = 'https://example.com';
- const { getByRole } = render();
- const link = getByRole('link');
+ renderWithRouter();
+ const link = screen.getByRole('link');
expect(link).toHaveAttribute('href', testDestinationUrl);
- const image = getByRole('img');
+ const image = screen.getByRole('img');
expect(image).toBeInTheDocument();
expect(image).toHaveAttribute('src', 'https://edx-cdn.org/v3/default/logo.svg');
});
+
+ it('navigates to the home route in the client when an app provides one', async () => {
+ mergeSiteConfig({
+ apps: [{
+ appId: 'org.openedx.frontend.app.logoTest',
+ routes: [{ path: '/learner-dashboard/*', handle: { roles: [homeRole] } }],
+ }],
+ });
+ renderWithRouter();
+
+ const link = screen.getByRole('link');
+ expect(link).toHaveAttribute('href', '/learner-dashboard');
+
+ fireEvent.click(link);
+ expect(screen.getByTestId('location')).toHaveTextContent('/learner-dashboard');
+ });
});
diff --git a/shell/Logo.tsx b/shell/Logo.tsx
index 905e8c35..5ddc4df5 100644
--- a/shell/Logo.tsx
+++ b/shell/Logo.tsx
@@ -1,7 +1,7 @@
import { IntlProvider } from 'react-intl';
import { Hyperlink, Image } from '@openedx/paragon';
import { getSiteConfig } from '../runtime/config';
-import { getUrlByRouteRole } from '../runtime/routing';
+import { getLinkProps, resolveRouteByRole } from '../runtime/routing';
import { homeRole } from './constants';
interface LogoProps {
@@ -11,7 +11,7 @@ interface LogoProps {
export default function Logo({
imageUrl = getSiteConfig().headerLogoImageUrl ?? 'https://edx-cdn.org/v3/default/logo.svg',
- destinationUrl = getUrlByRouteRole(homeRole) || '/'
+ destinationUrl = resolveRouteByRole(homeRole)?.url ?? '/'
}: LogoProps) {
const image = (
@@ -21,9 +21,10 @@ export default function Logo({
return image;
}
+ // A path in this site is a react-router Link, so the navigation stays in the client.
return (
-
+
{image}
diff --git a/shell/header/anonymous-menu/LoginButton.tsx b/shell/header/anonymous-menu/LoginButton.tsx
index 0a2b1508..20f78148 100644
--- a/shell/header/anonymous-menu/LoginButton.tsx
+++ b/shell/header/anonymous-menu/LoginButton.tsx
@@ -1,8 +1,7 @@
import { Button } from '@openedx/paragon';
-import { getUrlByRouteRole, useSiteConfig, useIntl } from '../../../runtime';
+import { getLinkProps, getUrlByRouteRole, useSiteConfig, useIntl } from '../../../runtime';
import { loginRole } from '../../constants';
import messages from '../../Shell.messages';
-import { getLinkProps } from './utils';
export default function LoginButton({ ...props }) {
const config = useSiteConfig();
diff --git a/shell/header/anonymous-menu/RegisterButton.tsx b/shell/header/anonymous-menu/RegisterButton.tsx
index 7d1aad17..f709c674 100644
--- a/shell/header/anonymous-menu/RegisterButton.tsx
+++ b/shell/header/anonymous-menu/RegisterButton.tsx
@@ -1,9 +1,8 @@
import { Button } from '@openedx/paragon';
-import { getUrlByRouteRole, useSiteConfig, useIntl } from '../../../runtime';
+import { getLinkProps, getUrlByRouteRole, useSiteConfig, useIntl } from '../../../runtime';
import { registerRole } from '../../constants';
import messages from '../../Shell.messages';
-import { getLinkProps } from './utils';
export default function RegisterButton({ ...props }) {
const config = useSiteConfig();
diff --git a/shell/header/anonymous-menu/utils.ts b/shell/header/anonymous-menu/utils.ts
deleted file mode 100644
index 202a0b9e..00000000
--- a/shell/header/anonymous-menu/utils.ts
+++ /dev/null
@@ -1,19 +0,0 @@
-import { ElementType } from 'react';
-import { Link } from 'react-router-dom';
-
-interface LinkProps {
- as?: ElementType;
- to?: string;
- href?: string;
-}
-
-/**
- * Builds the props needed to link to a URL, keeping navigation inside the
- * client when the URL is a route in this site rather than an external one.
- */
-export function getLinkProps(url: string): LinkProps {
- if (url.startsWith('/')) {
- return { as: Link, to: url };
- }
- return { href: url };
-}
diff --git a/shell/menus/LinkMenuItem.test.tsx b/shell/menus/LinkMenuItem.test.tsx
new file mode 100644
index 00000000..56c023bd
--- /dev/null
+++ b/shell/menus/LinkMenuItem.test.tsx
@@ -0,0 +1,87 @@
+import '@testing-library/jest-dom';
+import { ComponentProps } from 'react';
+import { fireEvent, render, screen } from '@testing-library/react';
+import { MemoryRouter, useLocation } from 'react-router-dom';
+
+import { mergeSiteConfig } from '../../runtime';
+import { IntlProvider } from '../../runtime/i18n';
+import LinkMenuItem from './LinkMenuItem';
+
+const accountRole = 'org.openedx.frontend.role.linkMenuItemTest.account';
+const profileRole = 'org.openedx.frontend.role.linkMenuItemTest.profile';
+const externalUrl = 'https://apps.example.com/profile/';
+
+mergeSiteConfig({
+ apps: [{
+ appId: 'org.openedx.frontend.app.linkMenuItemTest',
+ routes: [{ path: '/account/*', handle: { roles: [accountRole] } }],
+ }],
+ externalRoutes: [{ role: profileRole, url: externalUrl }],
+});
+
+function LocationDisplay() {
+ const { pathname } = useLocation();
+ return
{pathname}
;
+}
+
+function renderItem(props: Partial>, initialEntry = '/home') {
+ return render(
+
+
+
+
+
+
+ );
+}
+
+// jsdom cannot navigate, so keep a plain anchor's click from reaching it.
+function clickWithoutNavigating(link: HTMLElement) {
+ link.addEventListener('click', (event) => event.preventDefault());
+ fireEvent.click(link);
+}
+
+const variants = ['hyperlink', 'navLink', 'navDropdownItem', 'dropdownItem'] as const;
+
+describe('LinkMenuItem', () => {
+ describe.each(variants)('as %s', (variant) => {
+ it('navigates to a route an app provides without leaving the client', () => {
+ renderItem({ role: accountRole, variant });
+
+ const link = screen.getByRole('link', { name: 'Account' });
+ expect(link).toHaveAttribute('href', '/account');
+
+ fireEvent.click(link);
+ expect(screen.getByTestId('location')).toHaveTextContent('/account');
+ });
+
+ it('links to an external route with a plain anchor', () => {
+ renderItem({ role: profileRole, variant });
+
+ const link = screen.getByRole('link', { name: 'Account' });
+ expect(link).toHaveAttribute('href', externalUrl);
+
+ clickWithoutNavigating(link);
+ expect(screen.getByTestId('location')).toHaveTextContent('/home');
+ });
+ });
+
+ it('treats a URL given directly the same way', () => {
+ renderItem({ url: '/help', variant: 'navLink' });
+
+ fireEvent.click(screen.getByRole('link', { name: 'Account' }));
+ expect(screen.getByTestId('location')).toHaveTextContent('/help');
+ });
+
+ it('marks the nav link active on its own route', () => {
+ renderItem({ role: accountRole, variant: 'navLink' }, '/account/');
+
+ expect(screen.getByRole('link', { name: 'Account' })).toHaveClass('active');
+ });
+
+ it('renders nothing when no app or external route provides the role', () => {
+ renderItem({ role: 'org.openedx.frontend.role.linkMenuItemTest.missing', variant: 'dropdownItem' });
+
+ expect(screen.queryByRole('link')).not.toBeInTheDocument();
+ });
+});
diff --git a/shell/menus/LinkMenuItem.tsx b/shell/menus/LinkMenuItem.tsx
index 08e32ca3..993eb436 100644
--- a/shell/menus/LinkMenuItem.tsx
+++ b/shell/menus/LinkMenuItem.tsx
@@ -2,7 +2,7 @@ import { Dropdown, Hyperlink, NavDropdown, NavLink } from '@openedx/paragon';
import { useIntl } from 'react-intl';
import { useLocation } from 'react-router-dom';
-import { getUrlByRouteRole } from '../../runtime/routing';
+import { getLinkProps, resolveRouteByRole } from '../../runtime/routing';
import {
MenuItemName
} from '../../types';
@@ -24,7 +24,7 @@ export default function LinkMenuItem({ label, role, url, variant = 'hyperlink' }
let finalUrl: string | null | undefined;
if (role !== undefined) {
- finalUrl = getUrlByRouteRole(role);
+ finalUrl = resolveRouteByRole(role)?.url;
} else if (url !== undefined) {
finalUrl = url;
}
@@ -35,27 +35,31 @@ export default function LinkMenuItem({ label, role, url, variant = 'hyperlink' }
return null;
}
+ // A path in this site is a react-router Link, so the navigation stays in the client; anything
+ // else is a plain anchor.
+ const linkProps = getLinkProps(finalUrl);
+
if (variant === 'hyperlink') {
return (
-
+
{finalLabel}
);
} else if (variant === 'navLink') {
return (
-
+
{finalLabel}
);
} else if (variant === 'navDropdownItem') {
return (
-
+
{finalLabel}
);
} else if (variant === 'dropdownItem') {
return (
-
+
{finalLabel}
);