Skip to content

Commit 4a12a31

Browse files
authored
fix: course optimizer block links (#2946)
1 parent 4ceea74 commit 4a12a31

6 files changed

Lines changed: 66 additions & 24 deletions

File tree

src/optimizer-page/CourseOptimizerPage.tsx

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -213,9 +213,7 @@ const CourseOptimizerPage = () => {
213213
)}
214214
<Container size="xl" className="mt-4 px-4 export">
215215
<section className="setting-items mb-4">
216-
<Layout
217-
lg={[{ span: 12 }, { span: 0 }]}
218-
>
216+
<Layout>
219217
<Layout.Element>
220218
<article>
221219
<div className="d-flex flex-wrap justify-content-between align-items-center mb-3 p-3">

src/optimizer-page/scan-results/BrokenLinkTable.test.tsx

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
fireEvent,
66
waitFor,
77
} from '@testing-library/react';
8+
import { MemoryRouter } from 'react-router-dom';
89
import { IntlProvider } from '@edx/frontend-platform/i18n';
910
import { AppProvider } from '@edx/frontend-platform/react';
1011
import { initializeMockApp } from '@edx/frontend-platform';
@@ -13,6 +14,10 @@ import BrokenLinkTable from './BrokenLinkTable';
1314
import { Unit, Filters } from '../types';
1415
import initializeStore from '../../store';
1516

17+
jest.mock('@src/CourseAuthoringContext', () => ({
18+
useCourseAuthoringContext: jest.fn(() => ({ courseId: 'course-v1:TestX+Test101+2024' })),
19+
}));
20+
1621
let store: any;
1722

1823
const mockOnUpdateLink = jest.fn();
@@ -82,9 +87,11 @@ const BrokenLinkTableWrapper: React.FC<BrokenLinkTableWrapperProps> = ({
8287

8388
const intlWrapper = (ui: React.ReactElement) =>
8489
render(
85-
<IntlProvider locale="en" messages={{}}>
86-
{ui}
87-
</IntlProvider>,
90+
<MemoryRouter>
91+
<IntlProvider locale="en" messages={{}}>
92+
{ui}
93+
</IntlProvider>
94+
</MemoryRouter>,
8895
);
8996

9097
describe('BrokenLinkTable', () => {
@@ -709,11 +716,11 @@ describe('BrokenLinkTable', () => {
709716

710717
const goToAnchor = screen.getByText('Test Block');
711718

712-
fireEvent.click(goToAnchor);
713-
714-
await waitFor(() => {
715-
expect(window.open).toHaveBeenCalledWith('https://example.com/block', '_blank');
716-
});
719+
expect(goToAnchor.closest('a')).toHaveAttribute(
720+
'href',
721+
'/course/course-v1:TestX+Test101+2024/container/unit-1#block-1',
722+
);
723+
expect(goToAnchor.closest('a')).toHaveAttribute('target', '_blank');
717724
});
718725

719726
it('BrokenLinkHref anchor opens the href URL', async () => {

src/optimizer-page/scan-results/BrokenLinkTable.tsx

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ import {
2323
LOCKED,
2424
MANUAL,
2525
} from '../../constants';
26+
import { buildBlockContainerUrl } from '../utils';
27+
import { useCourseAuthoringContext } from '@src/CourseAuthoringContext';
28+
import { Link } from 'react-router-dom';
2629

2730
const BrokenLinkHref: FC<{ href: string; }> = ({ href }) => {
2831
const handleClick = (event: React.MouseEvent<HTMLAnchorElement>) => {
@@ -40,16 +43,11 @@ const BrokenLinkHref: FC<{ href: string; }> = ({ href }) => {
4043
};
4144

4245
const GoToBlock: FC<{ block: { url: string; displayName?: string; }; }> = ({ block }) => {
43-
const handleClick = (event: React.MouseEvent<HTMLAnchorElement>) => {
44-
event.preventDefault();
45-
window.open(block.url, '_blank');
46-
};
47-
4846
return (
4947
<div className="go-to-block-link-container">
50-
<a href={block.url} onClick={handleClick} className="broken-link" rel="noreferrer">
48+
<Link to={block.url} className="broken-link" rel="noreferrer" target="_blank">
5149
{block.displayName}
52-
</a>
50+
</Link>
5351
</div>
5452
);
5553
};
@@ -181,6 +179,7 @@ const BrokenLinkTable: FC<BrokenLinkTableProps> = ({
181179
updatedLinkMap = {},
182180
updatedLinkInProgress = {},
183181
}) => {
182+
const { courseId } = useCourseAuthoringContext();
184183
const brokenLinkList = unit.blocks.reduce(
185184
(
186185
acc: TableData,
@@ -208,7 +207,11 @@ const BrokenLinkTable: FC<BrokenLinkTableProps> = ({
208207
return {
209208
Links: (
210209
<LinksCol
211-
block={{ url: block.url, displayName: block.displayName || 'Go to block', id: block.id }}
210+
block={{
211+
url: buildBlockContainerUrl(courseId, unit.id, block.id),
212+
displayName: block.displayName || 'Go to block',
213+
id: block.id,
214+
}}
212215
href={displayLink}
213216
showIcon={false}
214217
showUpdateButton
@@ -237,7 +240,10 @@ const BrokenLinkTable: FC<BrokenLinkTableProps> = ({
237240
const blockBrokenLinks = block.brokenLinks.map((link) => ({
238241
Links: (
239242
<LinksCol
240-
block={{ url: block.url, displayName: block.displayName || 'Go to block' }}
243+
block={{
244+
url: buildBlockContainerUrl(courseId, unit.id, block.id),
245+
displayName: block.displayName || 'Go to block',
246+
}}
241247
href={link}
242248
linkType={BROKEN}
243249
/>
@@ -253,7 +259,10 @@ const BrokenLinkTable: FC<BrokenLinkTableProps> = ({
253259
const blockLockedLinks = block.lockedLinks.map((link) => ({
254260
Links: (
255261
<LinksCol
256-
block={{ url: block.url, displayName: block.displayName || 'Go to block' }}
262+
block={{
263+
url: buildBlockContainerUrl(courseId, unit.id, block.id),
264+
displayName: block.displayName || 'Go to block',
265+
}}
257266
href={link}
258267
linkType={LOCKED}
259268
/>
@@ -270,7 +279,10 @@ const BrokenLinkTable: FC<BrokenLinkTableProps> = ({
270279
const externalForbiddenLinks = block.externalForbiddenLinks.map((link) => ({
271280
Links: (
272281
<LinksCol
273-
block={{ url: block.url, displayName: block.displayName || 'Go to block' }}
282+
block={{
283+
url: buildBlockContainerUrl(courseId, unit.id, block.id),
284+
displayName: block.displayName || 'Go to block',
285+
}}
274286
href={link}
275287
linkType={MANUAL}
276288
/>

src/optimizer-page/scan-results/ScanResults.test.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,10 @@ jest.mock('../../data/apiHooks', () => ({
172172
})),
173173
}));
174174

175+
jest.mock('@src/CourseAuthoringContext', () => ({
176+
useCourseAuthoringContext: jest.fn(() => ({ courseId: 'test-course-id' })),
177+
}));
178+
175179
// Mock the thunks
176180
jest.mock('../data/thunks', () => ({
177181
updateSinglePreviousRunLink: jest.fn(() => () => Promise.resolve({ status: 'Succeeded' })),

src/optimizer-page/utils.test.js

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { mockApiResponse } from './mocks/mockApiResponse';
2-
import { countBrokenLinks, isDataEmpty } from './utils';
2+
import { countBrokenLinks, isDataEmpty, buildBlockContainerUrl } from './utils';
33

44
describe('countBrokenLinks', () => {
55
it('should return the count of broken links', () => {
@@ -104,3 +104,17 @@ describe('isDataEmpty', () => {
104104
expect(isDataEmpty(data)).toBe(false);
105105
});
106106
});
107+
108+
describe('buildBlockContainerUrl', () => {
109+
it('should build a correct internal route for block container', () => {
110+
const courseId = 'course-v1:Test+Course+2024';
111+
const unitId = 'unit123';
112+
const blockId = 'block456';
113+
114+
const result = buildBlockContainerUrl(courseId, unitId, blockId);
115+
116+
expect(result).toBe(
117+
`/course/${courseId}/container/${unitId}#${blockId}`,
118+
);
119+
});
120+
});

src/optimizer-page/utils.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
1-
/* eslint-disable import/prefer-default-export */
21
import { LinkCheckResult } from './types';
32

3+
export const buildBlockContainerUrl = (
4+
courseId: string,
5+
unitId: string,
6+
blockId: string,
7+
): string => {
8+
return `/course/${courseId}/container/${unitId}#${blockId}`;
9+
};
10+
411
export const countBrokenLinks = (
512
data: LinkCheckResult | null,
613
): {

0 commit comments

Comments
 (0)