Skip to content

Commit 027f72c

Browse files
arbrandesclaude
andcommitted
fix: use role-based routing
Routes are nested by default, but navigation constants used absolute paths like '/login', causing navigate() and <Navigate> to go to /login instead of /authn/login. Replace path constants in navigation calls with getUrlByRouteRole() where possible, which resolves the URL for a given route role from the route config. Path constants are still generally useful, but are now all relative (and camelCase). Centralize route roles, path segments, and the dashboard role in src/constants.ts. Last but not least, prefer SPA navigation (<Navigate>, useNavigate) over window.location.href where the target URL may be internal. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 0323313 commit 027f72c

29 files changed

Lines changed: 229 additions & 227 deletions

src/common-components/EmbeddedRegistrationRoute.jsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import PropTypes from 'prop-types';
22
import { Navigate } from 'react-router-dom';
33

4-
import { PAGE_NOT_FOUND } from '../data/constants';
4+
import { notFoundPath } from '../constants';
55
import { isHostAvailableInQueryParams } from '../data/utils';
66

77
/**
@@ -16,7 +16,7 @@ const EmbeddedRegistrationRoute = ({ children }) => {
1616
return children;
1717
}
1818

19-
return <Navigate to={PAGE_NOT_FOUND} replace />;
19+
return <Navigate to={`../${notFoundPath}`} replace />;
2020
};
2121

2222
EmbeddedRegistrationRoute.propTypes = {

src/common-components/EnterpriseSSO.jsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,23 @@
11
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
2-
import { useAppConfig, getSiteConfig, useIntl } from '@openedx/frontend-base';
2+
import { useAppConfig, getSiteConfig, getUrlByRouteRole, useIntl } from '@openedx/frontend-base';
33
import {
44
Button, Form,
55
Icon,
66
} from '@openedx/paragon';
77
import { Login } from '@openedx/paragon/icons';
88
import PropTypes from 'prop-types';
9+
import { useNavigate } from 'react-router-dom';
910

10-
import { LOGIN_PAGE, SUPPORTED_ICON_CLASSES } from '../data/constants';
11+
import { loginRole } from '../constants';
12+
import { SUPPORTED_ICON_CLASSES } from '../data/constants';
1113
import messages from './messages';
1214

1315
/**
1416
* This component renders the Single sign-on (SSO) button only for the tpa provider passed
1517
* */
1618
const EnterpriseSSO = (props) => {
1719
const { formatMessage } = useIntl();
20+
const navigate = useNavigate();
1821
const tpaProvider = props.provider;
1922
const hideRegistrationLink = useAppConfig().ALLOW_PUBLIC_ACCOUNT_CREATION === false
2023
|| useAppConfig().SHOW_REGISTRATION_LINKS === false;
@@ -26,7 +29,7 @@ const EnterpriseSSO = (props) => {
2629

2730
const handleClick = (e) => {
2831
e.preventDefault();
29-
window.location.href = LOGIN_PAGE;
32+
navigate(getUrlByRouteRole(loginRole));
3033
};
3134

3235
if (tpaProvider) {

src/common-components/RedirectLogistration.jsx

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
1-
import { useAppConfig, getSiteConfig } from '@openedx/frontend-base';
1+
import { useAppConfig, getSiteConfig, getUrlByRouteRole } from '@openedx/frontend-base';
22
import PropTypes from 'prop-types';
33
import { Navigate } from 'react-router-dom';
44

5-
import {
6-
AUTHN_PROGRESSIVE_PROFILING, REDIRECT,
7-
} from '../data/constants';
5+
import { welcomeRole } from '../constants';
6+
import { REDIRECT } from '../data/constants';
87
import { setCookie } from '../data/utils';
98

109
const RedirectLogistration = (props) => {
@@ -48,7 +47,7 @@ const RedirectLogistration = (props) => {
4847
const registrationResult = { redirectUrl: finalRedirectUrl, success };
4948
return (
5049
<Navigate
51-
to={AUTHN_PROGRESSIVE_PROFILING}
50+
to={getUrlByRouteRole(welcomeRole)}
5251
state={{
5352
registrationResult,
5453
optionalFields,

src/common-components/SocialAuthProviders.jsx

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ import { Icon } from '@openedx/paragon';
44
import { Login } from '@openedx/paragon/icons';
55
import PropTypes from 'prop-types';
66

7-
import { LOGIN_PAGE, SUPPORTED_ICON_CLASSES } from '../data/constants';
7+
import { loginPath } from '../constants';
8+
import { SUPPORTED_ICON_CLASSES } from '../data/constants';
89
import messages from './messages';
910

1011
const SocialAuthProviders = (props) => {
@@ -24,7 +25,7 @@ const SocialAuthProviders = (props) => {
2425
key={provider.id}
2526
type="button"
2627
className={`btn-social btn-${provider.id} ${index % 2 === 0 ? 'mr-3' : ''}`}
27-
data-provider-url={referrer === LOGIN_PAGE ? provider.loginUrl : provider.registerUrl}
28+
data-provider-url={referrer === loginPath ? provider.loginUrl : provider.registerUrl}
2829
onClick={handleSubmit}
2930
>
3031
{provider.iconImage ? (
@@ -43,7 +44,7 @@ const SocialAuthProviders = (props) => {
4344
)}
4445
<span id="provider-name" className="notranslate mr-auto pl-2" aria-hidden="true">{provider.name}</span>
4546
<span className="sr-only">
46-
{referrer === LOGIN_PAGE
47+
{referrer === loginPath
4748
? formatMessage(messages['sso.sign.in.with'], { providerName: provider.name })
4849
: formatMessage(messages['sso.create.account.using'], { providerName: provider.name })}
4950
</span>
@@ -55,7 +56,7 @@ const SocialAuthProviders = (props) => {
5556
};
5657

5758
SocialAuthProviders.defaultProps = {
58-
referrer: LOGIN_PAGE,
59+
referrer: loginPath,
5960
socialAuthProviders: [],
6061
};
6162

src/common-components/ThirdPartyAuth.jsx

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,8 @@ import classNames from 'classnames';
77
import PropTypes from 'prop-types';
88
import Skeleton from 'react-loading-skeleton';
99

10-
import {
11-
ENTERPRISE_LOGIN_URL, LOGIN_PAGE, PENDING_STATE, REGISTER_PAGE,
12-
} from '../data/constants';
10+
import { loginPath, registerPath } from '../constants';
11+
import { ENTERPRISE_LOGIN_URL, PENDING_STATE } from '../data/constants';
1312
import messages from './messages';
1413

1514
import {
@@ -75,7 +74,7 @@ const ThirdPartyAuth = (props) => {
7574
<div className="row m-0">
7675
<SocialAuthProviders
7776
socialAuthProviders={providers}
78-
referrer={isLoginPage ? LOGIN_PAGE : REGISTER_PAGE}
77+
referrer={isLoginPage ? loginPath : registerPath}
7978
/>
8079
</div>
8180
)}

src/common-components/ThirdPartyAuthAlert.jsx

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { getSiteConfig, useIntl } from '@openedx/frontend-base';
22
import { Alert } from '@openedx/paragon';
33
import PropTypes from 'prop-types';
44

5-
import { LOGIN_PAGE, REGISTER_PAGE } from '../data/constants';
5+
import { loginPath, registerPath } from '../constants';
66
import messages from './messages';
77

88
const ThirdPartyAuthAlert = (props) => {
@@ -11,7 +11,7 @@ const ThirdPartyAuthAlert = (props) => {
1111
const platformName = getSiteConfig().siteName;
1212
let message;
1313

14-
if (referrer === LOGIN_PAGE) {
14+
if (referrer === loginPath) {
1515
message = formatMessage(messages['login.third.party.auth.account.not.linked'], { currentProvider, platformName });
1616
} else {
1717
message = formatMessage(messages['register.third.party.auth.account.not.linked'], { currentProvider, platformName });
@@ -23,13 +23,13 @@ const ThirdPartyAuthAlert = (props) => {
2323

2424
return (
2525
<>
26-
<Alert id="tpa-alert" className={referrer === REGISTER_PAGE ? 'alert-success mt-n2 mb-5' : 'alert-warning mt-n2 mb-5'}>
27-
{referrer === REGISTER_PAGE ? (
26+
<Alert id="tpa-alert" className={referrer === registerPath ? 'alert-success mt-n2 mb-5' : 'alert-warning mt-n2 mb-5'}>
27+
{referrer === registerPath ? (
2828
<Alert.Heading>{formatMessage(messages['tpa.alert.heading'])}</Alert.Heading>
2929
) : null}
3030
<p>{message}</p>
3131
</Alert>
32-
{referrer === REGISTER_PAGE ? (
32+
{referrer === registerPath ? (
3333
<h4 className="mt-4 mb-4">{formatMessage(messages['registration.using.tpa.form.heading'])}</h4>
3434
) : null}
3535
</>
@@ -38,7 +38,7 @@ const ThirdPartyAuthAlert = (props) => {
3838

3939
ThirdPartyAuthAlert.defaultProps = {
4040
currentProvider: '',
41-
referrer: LOGIN_PAGE,
41+
referrer: loginPath,
4242
};
4343

4444
ThirdPartyAuthAlert.propTypes = {

src/common-components/UnAuthOnlyRoute.jsx

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
import { useEffect, useState } from 'react';
22

3-
import { fetchAuthenticatedUser, getAuthenticatedUser, getSiteConfig } from '@openedx/frontend-base';
3+
import { fetchAuthenticatedUser, getAuthenticatedUser, getUrlByRouteRole } from '@openedx/frontend-base';
44
import PropTypes from 'prop-types';
5+
import { Navigate } from 'react-router-dom';
56

6-
import {
7-
DEFAULT_REDIRECT_URL,
8-
} from '../data/constants';
7+
import { dashboardRole } from '../constants';
98

109
/**
1110
* This wrapper redirects the requester to our default redirect url if they are
@@ -24,8 +23,7 @@ const UnAuthOnlyRoute = ({ children }) => {
2423

2524
if (isReady) {
2625
if (authUser && authUser.username) {
27-
global.location.href = getSiteConfig().lmsBaseUrl.concat(DEFAULT_REDIRECT_URL);
28-
return null;
26+
return <Navigate to={getUrlByRouteRole(dashboardRole)} replace />;
2927
}
3028

3129
return children;

src/common-components/tests/EmbeddedRegistrationRoute.test.jsx

Lines changed: 26 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,27 @@
1-
/* eslint-disable import/no-import-module-exports */
2-
/* eslint-disable react/function-component-definition */
3-
41
import { getSiteConfig } from '@openedx/frontend-base';
5-
import { render } from '@testing-library/react';
6-
import { act } from 'react-dom/test-utils';
2+
import { render, waitFor } from '@testing-library/react';
73
import {
8-
MemoryRouter, Route, BrowserRouter as Router, Routes,
4+
MemoryRouter, Navigate, Outlet, Route, Routes,
95
} from 'react-router-dom';
106

11-
import { PAGE_NOT_FOUND, REGISTER_EMBEDDED_PAGE } from '../../data/constants';
7+
import { notFoundPath, registerEmbeddedPath } from '../../constants';
128
import EmbeddedRegistrationRoute from '../EmbeddedRegistrationRoute';
139

14-
const RRD = require('react-router-dom');
15-
// Just render plain div with its children
16-
// eslint-disable-next-line react/prop-types
17-
RRD.BrowserRouter = ({ children }) => <div>{children}</div>;
18-
module.exports = RRD;
19-
20-
const TestApp = () => (
21-
<Router>
22-
<div>
23-
<Routes>
24-
<Route
25-
path={REGISTER_EMBEDDED_PAGE}
26-
element={<EmbeddedRegistrationRoute><span>Embedded Register Page</span></EmbeddedRegistrationRoute>}
27-
/>
28-
<Route
29-
path={PAGE_NOT_FOUND}
30-
element={<span>Page not found</span>}
31-
/>
32-
</Routes>
33-
</div>
34-
</Router>
35-
);
36-
3710
describe('EmbeddedRegistrationRoute', () => {
3811
const routerWrapper = () => (
39-
<MemoryRouter initialEntries={[REGISTER_EMBEDDED_PAGE]}>
40-
<TestApp />
12+
<MemoryRouter initialEntries={[`/authn/${registerEmbeddedPath}`]}>
13+
<Routes>
14+
<Route path="/authn" element={<Outlet />}>
15+
<Route
16+
path={registerEmbeddedPath}
17+
element={<EmbeddedRegistrationRoute><span>Embedded Register Page</span></EmbeddedRegistrationRoute>}
18+
/>
19+
<Route
20+
path={notFoundPath}
21+
element={<span>Page not found</span>}
22+
/>
23+
</Route>
24+
</Routes>
4125
</MemoryRouter>
4226
);
4327

@@ -46,30 +30,25 @@ describe('EmbeddedRegistrationRoute', () => {
4630
});
4731

4832
it('should not render embedded register page if host query param is not available in the url', async () => {
49-
let embeddedRegistrationPage = null;
50-
await act(async () => {
51-
const { container } = await render(routerWrapper());
52-
embeddedRegistrationPage = container;
53-
});
33+
const { container } = render(routerWrapper());
5434

55-
const renderedPage = embeddedRegistrationPage.querySelector('span');
56-
expect(renderedPage.textContent).toBe('Page not found');
35+
await waitFor(() => {
36+
const renderedPage = container.querySelector('span');
37+
expect(renderedPage).not.toBeNull();
38+
expect(renderedPage.textContent).toBe('Page not found');
39+
});
5740
});
5841

59-
it('should render embedded register page if host query param is available in the url (embedded)', async () => {
42+
it('should render embedded register page if host query param is available in the url (embedded)', () => {
6043
delete window.location;
6144
window.location = {
62-
href: getSiteConfig().baseUrl.concat(REGISTER_EMBEDDED_PAGE),
45+
href: getSiteConfig().baseUrl.concat('/', registerEmbeddedPath),
6346
search: '?host=http://localhost/host-websit',
6447
};
6548

66-
let embeddedRegistrationPage = null;
67-
await act(async () => {
68-
const { container } = await render(routerWrapper());
69-
embeddedRegistrationPage = container;
70-
});
49+
const { container } = render(routerWrapper());
7150

72-
const renderedPage = embeddedRegistrationPage.querySelector('span');
51+
const renderedPage = container.querySelector('span');
7352
expect(renderedPage).toBeTruthy();
7453
expect(renderedPage.textContent).toBe('Embedded Register Page');
7554
});

src/common-components/tests/ThirdPartyAuthAlert.test.jsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import { IntlProvider } from '@openedx/frontend-base';
22
import renderer from 'react-test-renderer';
33

4-
import { PENDING_STATE, REGISTER_PAGE } from '../../data/constants';
4+
import { registerPath } from '../../constants';
5+
import { PENDING_STATE } from '../../data/constants';
56
import ThirdPartyAuthAlert from '../ThirdPartyAuthAlert';
67

78
describe('ThirdPartyAuthAlert', () => {
@@ -26,7 +27,7 @@ describe('ThirdPartyAuthAlert', () => {
2627
it('should match register page third party auth alert message snapshot', () => {
2728
props = {
2829
...props,
29-
referrer: REGISTER_PAGE,
30+
referrer: registerPath,
3031
};
3132

3233
const tree = renderer.create(

src/common-components/tests/UnAuthOnlyRoute.test.jsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,13 @@ import {
99
} from 'react-router-dom';
1010

1111
import { UnAuthOnlyRoute } from '..';
12-
import { REGISTER_PAGE } from '../../data/constants';
12+
import { registerPath } from '../../constants';
1313

1414
jest.mock('@openedx/frontend-base', () => ({
1515
...jest.requireActual('@openedx/frontend-base'),
1616
getAuthenticatedUser: jest.fn(),
1717
fetchAuthenticatedUser: jest.fn(),
18+
getUrlByRouteRole: jest.fn((role) => `http://localhost:18000/${role.split('.').pop()}`),
1819
}));
1920

2021
const RRD = require('react-router-dom');
@@ -27,15 +28,15 @@ const TestApp = () => (
2728
<Router>
2829
<div>
2930
<Routes>
30-
<Route path={REGISTER_PAGE} element={<UnAuthOnlyRoute><span>Register Page</span></UnAuthOnlyRoute>} />
31+
<Route path={`/${registerPath}`} element={<UnAuthOnlyRoute><span>Register Page</span></UnAuthOnlyRoute>} />
3132
</Routes>
3233
</div>
3334
</Router>
3435
);
3536

3637
describe('UnAuthOnlyRoute', () => {
3738
const routerWrapper = () => (
38-
<MemoryRouter initialEntries={[REGISTER_PAGE]}>
39+
<MemoryRouter initialEntries={[`/${registerPath}`]}>
3940
<TestApp />
4041
</MemoryRouter>
4142
);

0 commit comments

Comments
 (0)