Skip to content

Commit e9cb74a

Browse files
arbrandesclaude
andcommitted
fix: use an SPA link for "back to dashboard" when the route exists
Co-Authored-By: Claude <noreply@anthropic.com>
1 parent f920287 commit e9cb74a

6 files changed

Lines changed: 85 additions & 17 deletions

File tree

‎src/components/GradebookHeader/index.jsx‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
1-
import { useIntl } from '@openedx/frontend-base';
1+
import { Link } from 'react-router-dom';
2+
3+
import { getUrlByRouteRole, useIntl } from '@openedx/frontend-base';
24
import { Button } from '@openedx/paragon';
35

46
import { instructorDashboardUrl } from '@src/data/services/lms/urls';
57
import useGradebookHeaderData from './hooks';
68
import messages from './messages';
79

10+
const instructorDashboardRole = 'org.openedx.frontend.role.instructorDashboard';
11+
812
export const GradebookHeader = () => {
913
const { formatMessage } = useIntl();
1014
const {
@@ -15,13 +19,24 @@ export const GradebookHeader = () => {
1519
showBulkManagement,
1620
toggleViewMessage,
1721
} = useGradebookHeaderData();
18-
const dashboardUrl = instructorDashboardUrl();
22+
// Prefer the instructor dashboard route if the running site provides one,
23+
// so navigation stays within the SPA; otherwise fall back to a full page
24+
// load of the legacy LMS dashboard.
25+
const dashboardRoute = getUrlByRouteRole(instructorDashboardRole)?.replace(':courseId', courseId);
26+
const isInternalRoute = !!dashboardRoute && !/^[a-z][a-z0-9+.-]*:/i.test(dashboardRoute);
27+
const backLinkContent = (
28+
<>
29+
<span aria-hidden="true">{'<< '}</span>
30+
{formatMessage(messages.backToDashboard)}
31+
</>
32+
);
1933
return (
2034
<div className="gradebook-header">
21-
<a href={dashboardUrl} className="mb-3">
22-
<span aria-hidden="true">{'<< '}</span>
23-
{formatMessage(messages.backToDashboard)}
24-
</a>
35+
{isInternalRoute ? (
36+
<Link to={dashboardRoute} className="mb-3">{backLinkContent}</Link>
37+
) : (
38+
<a href={dashboardRoute ?? instructorDashboardUrl()} className="mb-3">{backLinkContent}</a>
39+
)}
2540
<h1>{formatMessage(messages.gradebook)}</h1>
2641
<div className="subtitle-row d-flex justify-content-between align-items-center">
2742
<h2 className="text-break">{courseId}</h2>

‎src/components/GradebookHeader/index.test.jsx‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,17 @@ import { renderWithAllProviders, initializeMocks } from '@src/testUtils';
22
import { screen } from '@testing-library/react';
33
import userEvent from '@testing-library/user-event';
44

5+
import { getUrlByRouteRole } from '@openedx/frontend-base';
56
import { instructorDashboardUrl } from '@src/data/services/lms/urls';
67

78
import { GradebookHeader } from './index';
89
import useGradebookHeaderData from './hooks';
910
import messages from './messages';
1011

12+
jest.mock('@openedx/frontend-base', () => ({
13+
...jest.requireActual('@openedx/frontend-base'),
14+
getUrlByRouteRole: jest.fn(),
15+
}));
1116
jest.mock('@src/data/services/lms/urls', () => ({
1217
instructorDashboardUrl: jest.fn(),
1318
}));
@@ -20,6 +25,7 @@ describe('GradebookHeader', () => {
2025

2126
beforeEach(() => {
2227
jest.clearAllMocks();
28+
getUrlByRouteRole.mockReturnValue(null);
2329
instructorDashboardUrl.mockReturnValue('https://example.com/dashboard');
2430
});
2531

@@ -83,6 +89,28 @@ describe('GradebookHeader', () => {
8389
expect(instructorDashboardUrl).toHaveBeenCalled();
8490
});
8591

92+
it('renders an SPA link when the site provides an instructor dashboard route', () => {
93+
getUrlByRouteRole.mockReturnValue('/instructor-dashboard/:courseId');
94+
renderWithAllProviders(<GradebookHeader />);
95+
const dashboardLink = screen.getByRole('link');
96+
expect(dashboardLink).toHaveAttribute(
97+
'href',
98+
'/instructor-dashboard/course-v1:TestU+CS101+2024',
99+
);
100+
expect(instructorDashboardUrl).not.toHaveBeenCalled();
101+
});
102+
103+
it('renders a plain anchor when the instructor dashboard route is external', () => {
104+
getUrlByRouteRole.mockReturnValue('https://other.example.com/dashboard');
105+
renderWithAllProviders(<GradebookHeader />);
106+
const dashboardLink = screen.getByRole('link');
107+
expect(dashboardLink).toHaveAttribute(
108+
'href',
109+
'https://other.example.com/dashboard',
110+
);
111+
expect(instructorDashboardUrl).not.toHaveBeenCalled();
112+
});
113+
86114
it('calls useGradebookHeaderData hook', () => {
87115
renderWithAllProviders(<GradebookHeader />);
88116
expect(useGradebookHeaderData).toHaveBeenCalled();

‎src/data/services/lms/urls.js‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,20 +3,22 @@ import { historyRecordLimit } from './constants';
33
import { filterQuery, stringifyUrl } from './utils';
44
import { getSiteConfig } from '@openedx/frontend-base';
55

6-
const courseId = window.location.pathname.split('/').filter(Boolean).pop() || '';
6+
// Evaluated at call time: with client-side navigation, this module can be
7+
// imported before the URL reflects the gradebook route.
8+
const getCourseId = () => window.location.pathname.split('/').filter(Boolean).pop() || '';
79

810
export const getUrlPrefix = () => `${getSiteConfig().lmsBaseUrl}/api/`;
9-
export const getBulkGradesUrl = () => `${getUrlPrefix()}bulk_grades/course/${courseId}/`;
11+
export const getBulkGradesUrl = () => `${getUrlPrefix()}bulk_grades/course/${getCourseId()}/`;
1012
export const getEnrollmentUrl = () => `${getUrlPrefix()}enrollment/v2/`;
1113
export const getGradesUrl = () => `${getUrlPrefix()}grades/v1/`;
12-
export const getGradebookUrl = () => `${getGradesUrl()}gradebook/${courseId}/`;
14+
export const getGradebookUrl = () => `${getGradesUrl()}gradebook/${getCourseId()}/`;
1315
export const getBulkUpdateUrl = () => `${getGradebookUrl()}bulk-update`;
1416
export const getInterventionUrl = () => `${getBulkGradesUrl()}intervention/`;
15-
export const getCohortsUrl = () => `${getUrlPrefix()}cohorts/v1/courses/${courseId}/cohorts/`;
16-
export const getTracksUrl = () => `${getEnrollmentUrl()}course/${courseId}?include_expired=1`;
17+
export const getCohortsUrl = () => `${getUrlPrefix()}cohorts/v1/courses/${getCourseId()}/cohorts/`;
18+
export const getTracksUrl = () => `${getEnrollmentUrl()}course/${getCourseId()}?include_expired=1`;
1719
export const getBulkHistoryUrl = () => `${getBulkUpdateUrl()}history/`;
1820
export const getAssignmentTypesUrl = () => stringifyUrl(`${getGradebookUrl()}grading-info`, { graded_only: true });
19-
export const getRolesUrl = () => stringifyUrl(`${getEnrollmentUrl()}roles/`, { courseId });
21+
export const getRolesUrl = () => stringifyUrl(`${getEnrollmentUrl()}roles/`, { courseId: getCourseId() });
2022
/**
2123
* bulkGradesUrlByCourseAndRow(courseId, rowId)
2224
* returns the bulkGrades url with the given rowId.
@@ -37,7 +39,7 @@ export const sectionOverrideHistoryUrl = (subsectionId, userId) => stringifyUrl(
3739
);
3840

3941
export const instructorDashboardUrl = () => (
40-
`${getSiteConfig().lmsBaseUrl}/courses/${courseId}/instructor`
42+
`${getSiteConfig().lmsBaseUrl}/courses/${getCourseId()}/instructor`
4143
);
4244

4345
export default StrictDict({

‎src/data/services/lms/urls.test.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
1+
import { getSiteConfig } from '@openedx/frontend-base';
2+
13
import { historyRecordLimit } from './constants';
24
import * as utils from './utils';
35
import urls, {
46
bulkGradesUrlByRow,
57
gradeCsvUrl,
8+
instructorDashboardUrl,
69
interventionExportCsvUrl,
710
sectionOverrideHistoryUrl,
811
} from './urls';
@@ -47,6 +50,15 @@ describe('lms api url methods', () => {
4750
);
4851
});
4952
});
53+
describe('instructorDashboardUrl', () => {
54+
it('returns the LMS dashboard url for the course in the current location', () => {
55+
const courseId = 'course-v1:TestU+CS101+2024';
56+
window.history.pushState({}, '', `/gradebook/${courseId}`);
57+
expect(instructorDashboardUrl()).toEqual(
58+
`${getSiteConfig().lmsBaseUrl}/courses/${courseId}/instructor`,
59+
);
60+
});
61+
});
5062
describe('sectionOverrideHistoryUrl', () => {
5163
it('returns grades url with subsection id, and user_id/history_record_limit query', () => {
5264
const subsectionId = 'a sub section';

‎src/routes.test.tsx‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import MockedMain from './Main';
2+
import routes from './routes';
3+
4+
jest.mock('./Main', () => () => null);
5+
6+
describe('routes', () => {
7+
it('lazy-loads Main as the gradebook route component', async () => {
8+
const { Component } = await routes[0].lazy();
9+
expect(Component).toBe(MockedMain);
10+
});
11+
});

‎src/routes.tsx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
1-
import { lazy } from 'react';
21
import { authenticatedLoader } from '@openedx/frontend-base';
32

4-
const Main = lazy(() => import('./Main'));
5-
63
const routes = [
74
{
85
id: 'org.openedx.frontend.route.gradebook.main',
96
path: '/gradebook/:courseId',
107
loader: authenticatedLoader,
11-
Component: Main,
8+
async lazy() {
9+
const { default: Main } = await import('./Main');
10+
return { Component: Main };
11+
},
1212
handle: {
1313
roles: ['org.openedx.frontend.role.gradebook'],
1414
},

0 commit comments

Comments
 (0)