From fa9194c0da6a2a42a684eada5fdd1eaa22f90419 Mon Sep 17 00:00:00 2001 From: pganesh-apphelix Date: Thu, 7 Aug 2025 07:12:49 +0000 Subject: [PATCH 1/4] feat: enhance course optimizer page design in studio --- src/data/api.ts | 1 + src/optimizer-page/CourseOptimizerPage.tsx | 69 ++-- src/optimizer-page/messages.js | 6 +- .../scan-results/PreviousRunLinkTable.tsx | 102 +++++ .../PreviousRunSectionCollapsible.tsx | 70 ++++ .../scan-results/ScanResults.scss | 5 + .../scan-results/ScanResults.tsx | 347 +++++++++++++----- src/optimizer-page/scan-results/messages.js | 16 +- src/optimizer-page/types.ts | 17 + 9 files changed, 504 insertions(+), 129 deletions(-) create mode 100644 src/optimizer-page/scan-results/PreviousRunLinkTable.tsx create mode 100644 src/optimizer-page/scan-results/PreviousRunSectionCollapsible.tsx diff --git a/src/data/api.ts b/src/data/api.ts index cddb466a28..e76d95ede4 100644 --- a/src/data/api.ts +++ b/src/data/api.ts @@ -32,6 +32,7 @@ export async function getCourseDetail(courseId: string, username: string) { */ export const waffleFlagDefaults = { enableCourseOptimizer: false, + enableCourseOptimizerCheckPrevRunLinks: false, useNewHomePage: true, useNewCustomPages: true, useNewScheduleDetailsPage: true, diff --git a/src/optimizer-page/CourseOptimizerPage.tsx b/src/optimizer-page/CourseOptimizerPage.tsx index 251d12ddae..14b4ccadad 100644 --- a/src/optimizer-page/CourseOptimizerPage.tsx +++ b/src/optimizer-page/CourseOptimizerPage.tsx @@ -5,13 +5,12 @@ import { import { useDispatch, useSelector } from 'react-redux'; import { useIntl } from '@edx/frontend-platform/i18n'; import { - Badge, Container, Layout, Button, Card, + Badge, Container, Layout, Button, Card, Spinner, } from '@openedx/paragon'; import { Helmet } from 'react-helmet'; import CourseStepper from '../generic/course-stepper'; import ConnectionErrorAlert from '../generic/ConnectionErrorAlert'; -import SubHeader from '../generic/sub-header/SubHeader'; import { RequestFailureStatuses } from '../data/constants'; import messages from './messages'; import { @@ -53,7 +52,6 @@ const CourseOptimizerPage: FC<{ courseId: string }> = ({ courseId }) => { const linkCheckResult = useSelector(getLinkCheckResult); const lastScannedAt = useSelector(getLastScannedAt); const { msg: errorMessage } = useSelector(getError); - const isShowExportButton = !linkCheckInProgress || errorMessage; const isLoadingDenied = (RequestFailureStatuses as string[]).includes(loadingStatus); const isSavingDenied = (RequestFailureStatuses as string[]).includes(savingStatus); const interval = useRef(undefined); @@ -136,45 +134,50 @@ const CourseOptimizerPage: FC<{ courseId: string }> = ({ courseId }) => {
- - {intl.formatMessage(messages.headingTitle)} - {intl.formatMessage(messages.new)} - - ) - } - subtitle={intl.formatMessage(messages.headingSubtitle)} - /> - +
+
+

Tools

+
+

{intl.formatMessage(messages.headingTitle)}

+ {intl.formatMessage(messages.new)} +
+
+ +
+ +

{intl.formatMessage(messages.description)}

+
-

{intl.formatMessage(messages.description)}

- {isShowExportButton && ( -

{lastScannedAt && `${intl.formatMessage(messages.lastScannedOn)} ${intl.formatDate(lastScannedAt, { year: 'numeric', month: 'long', day: 'numeric' })}`}

 - )} {showStepper && ( = ({ href }) => { + const handleClick = (event: React.MouseEvent) => { + event.preventDefault(); + window.open(href, '_blank'); + }; + + return ( + + ); +}; + +const GoToBlock: FC<{ block: { url: string, displayName: string } }> = ({ block }) => { + const handleClick = (event: React.MouseEvent) => { + event.preventDefault(); + window.open(block.url, '_blank'); + }; + + return ( + + ); +}; + +const PreviousRunLinksCol: FC<{ block: { url: string, displayName: string }, href: string }> = ({ block, href }) => ( + + + + + +); + +interface PreviousRunLinkTableProps { + unit: Unit; +} + +type TableData = { + Links: JSX.Element; +}[]; + +const PreviousRunLinkTable: FC = ({ + unit, +}) => { + const previousRunLinkList = unit.blocks.reduce( + ( + acc: TableData, + block, + ) => { + if (block.previousRunLinks && block.previousRunLinks.length > 0) { + const blockPreviousRunLinks = block.previousRunLinks.map((link) => ({ + Links: ( + + ), + })); + acc.push(...blockPreviousRunLinks); + } + + return acc; + }, + [], + ); + + if (previousRunLinkList.length === 0) { + return null; + } + + return ( + +

{unit.displayName}

+ + + ); +}; + +export default PreviousRunLinkTable; diff --git a/src/optimizer-page/scan-results/PreviousRunSectionCollapsible.tsx b/src/optimizer-page/scan-results/PreviousRunSectionCollapsible.tsx new file mode 100644 index 0000000000..b71b8027ec --- /dev/null +++ b/src/optimizer-page/scan-results/PreviousRunSectionCollapsible.tsx @@ -0,0 +1,70 @@ +import { FC } from 'react'; +import { + Collapsible, + Icon, +} from '@openedx/paragon'; +import { + ArrowRight, + ArrowDropDown, +} from '@openedx/paragon/icons'; + +interface Props { + index: number; + handleToggle: Function; + isOpen: boolean; + hasPrevAndIsOpen: boolean; + hasNextAndIsOpen: boolean; + title: string; + children: React.ReactNode; + previousRunLinksCount: number; + className?: string; +} + +const PreviousRunSectionCollapsible: FC = ({ + index, + handleToggle, + isOpen, + hasPrevAndIsOpen, + hasNextAndIsOpen, + title, + children, + previousRunLinksCount, + className, +}) => { + const styling = `card-lg open-section-rounded ${hasPrevAndIsOpen ? 'closed-section-rounded-top' : ''} ${hasNextAndIsOpen ? 'closed-section-rounded-bottom' : ''}`; + const collapsibleTitle = ( +
+
+ +

{title}

+
+
+
+

{previousRunLinksCount > 0 ? previousRunLinksCount : '-'}

+
+
+
+ ); + + return ( +
+ + {collapsibleTitle} +

+ )} + iconWhenClosed="" + iconWhenOpen="" + open={isOpen} + onToggle={() => handleToggle(index)} + > + {children} + +
+ ); +}; + +export default PreviousRunSectionCollapsible; diff --git a/src/optimizer-page/scan-results/ScanResults.scss b/src/optimizer-page/scan-results/ScanResults.scss index cd006ce1a5..618eee96f7 100644 --- a/src/optimizer-page/scan-results/ScanResults.scss +++ b/src/optimizer-page/scan-results/ScanResults.scss @@ -183,6 +183,7 @@ display: flex; align-items: center; align-self: center; + font-size: 18px; } } @@ -332,3 +333,7 @@ line-height: 24px; margin: 0; } + +.scan-course-btn:focus::before { + border: none !important; +} diff --git a/src/optimizer-page/scan-results/ScanResults.tsx b/src/optimizer-page/scan-results/ScanResults.tsx index c034c2aad8..44c3efac10 100644 --- a/src/optimizer-page/scan-results/ScanResults.tsx +++ b/src/optimizer-page/scan-results/ScanResults.tsx @@ -18,10 +18,13 @@ import { import { useIntl } from '@edx/frontend-platform/i18n'; import messages from './messages'; import SectionCollapsible from './SectionCollapsible'; +import PreviousRunSectionCollapsible from './PreviousRunSectionCollapsible'; import BrokenLinkTable from './BrokenLinkTable'; +import PreviousRunLinkTable from './PreviousRunLinkTable'; import { LinkCheckResult } from '../types'; import { countBrokenLinks } from '../utils'; import FilterModal from './filterModal'; +import { useWaffleFlags } from '../../data/apiHooks'; const InfoCard: FC<{ text: string }> = ({ text }) => ( @@ -41,6 +44,7 @@ interface Props { const ScanResults: FC = ({ data }) => { let hasSectionsRendered = false; const intl = useIntl(); + const waffleFlags = useWaffleFlags(); const [isOpen, open, close] = useToggle(false); const initialFilters = { brokenLinks: false, @@ -50,12 +54,109 @@ const ScanResults: FC = ({ data }) => { const [filters, setFilters] = useState(initialFilters); const [openStates, setOpenStates] = useState([]); const [buttonRef, setButtonRef] = useState(null); + const [prevRunOpenStates, setPrevRunOpenStates] = useState([]); + const { sections } = data || {}; + + const virtualSections = useMemo(() => { + const createVirtualSection = ( + items: any[], + sectionId: string, + messageKey: keyof typeof messages, + ) => { + const itemsWithLinks = items.filter(item => (item.brokenLinks && item.brokenLinks.length > 0) + || (item.lockedLinks && item.lockedLinks.length > 0) + || (item.externalForbiddenLinks && item.externalForbiddenLinks.length > 0) + || (item.previousRunLinks && item.previousRunLinks.length > 0)); + + if (itemsWithLinks.length === 0) { return null; } + + return { + id: sectionId, + displayName: intl.formatMessage(messages[messageKey]), + subsections: [{ + id: `${sectionId}-subsection`, + displayName: `${intl.formatMessage(messages[messageKey])} Subsection`, + units: itemsWithLinks.map(item => ({ + id: item.name, + displayName: item.name, + url: item.url, + blocks: [{ + id: `${item.name}-block`, + displayName: item.name, + url: item.url, + brokenLinks: item.brokenLinks || [], + lockedLinks: item.lockedLinks || [], + externalForbiddenLinks: item.externalForbiddenLinks || [], + previousRunLinks: item.previousRunLinks || [], + }], + })), + }], + }; + }; + + const vSections = []; + + if (data?.courseUpdates?.length > 0) { + const courseUpdatesSection = createVirtualSection(data.courseUpdates, 'course-updates', 'courseUpdatesHeader'); + if (courseUpdatesSection) { + vSections.push(courseUpdatesSection); + } + } + + if (data?.customPages?.length > 0) { + const customPagesSection = createVirtualSection( + data.customPages, + 'custom-pages', + 'customPagesHeader', + ); + if (customPagesSection) { + vSections.push(customPagesSection); + } + } + + return vSections; + }, [data?.courseUpdates, data?.customPages, intl]); + + // Combine virtual sections with regular sections + const allSectionsForBrokenLinks = useMemo( + () => [...virtualSections, ...(sections || [])], + [virtualSections, sections], + ); + + const allSectionsForPrevRun = useMemo( + () => [...virtualSections, ...(sections || [])], + [virtualSections, sections], + ); const { brokenLinksCounts, lockedLinksCounts, externalForbiddenLinksCounts, - } = useMemo(() => countBrokenLinks(data), [data?.sections]); + } = useMemo(() => countBrokenLinks({ sections: allSectionsForBrokenLinks }), [allSectionsForBrokenLinks]); + + // Calculate if there are any previous run links across all sections + const hasPreviousRunLinks = useMemo( + // eslint-disable-next-line max-len + () => allSectionsForPrevRun.some(section => section.subsections.some(subsection => subsection.units.some(unit => unit.blocks.some(block => block.previousRunLinks && block.previousRunLinks.length > 0)))), + [allSectionsForPrevRun], + ); + + // Calculate previous run links count for each section (including virtual sections) + const previousRunLinksCounts = useMemo(() => { + if (!allSectionsForPrevRun) { return []; } + return allSectionsForPrevRun.map(section => section.subsections.reduce( + (sectionTotal, subsection) => sectionTotal + + subsection.units.reduce( + (unitTotal, unit) => unitTotal + + unit.blocks.reduce( + (blockTotal, block) => blockTotal + (block.previousRunLinks?.length || 0), + 0, + ), + 0, + ), + 0, + )); + }, [allSectionsForPrevRun]); const activeFilters = Object.keys(filters).filter(key => filters[key]); const [filterBy, { @@ -63,16 +164,19 @@ const ScanResults: FC = ({ data }) => { }] = useCheckboxSetValues(activeFilters); useEffect(() => { - setOpenStates(data?.sections ? data.sections.map(() => false) : []); - }, [data?.sections]); - if (!data?.sections) { + setOpenStates(allSectionsForBrokenLinks ? allSectionsForBrokenLinks.map(() => false) : []); + setPrevRunOpenStates(allSectionsForPrevRun ? allSectionsForPrevRun.map(() => false) : []); + }, [allSectionsForBrokenLinks, allSectionsForPrevRun]); + + if (!data) { return ; } - - const { sections } = data; const handleToggle = (index: number) => { setOpenStates(prev => prev.map((isOpened, i) => (i === index ? !isOpened : isOpened))); }; + const handlePrevRunToggle = (index: number) => { + setPrevRunOpenStates(prev => prev.map((isOpened, i) => (i === index ? !isOpened : isOpened))); + }; const filterOptions = [ { name: intl.formatMessage(messages.brokenLabel), value: 'brokenLinks' }, { name: intl.formatMessage(messages.manualLabel), value: 'externalForbiddenLinks' }, @@ -98,7 +202,7 @@ const ScanResults: FC = ({ data }) => { const findNextVisibleSection = (currentIndex: number): number => { let nextIndex = currentIndex + 1; - while (nextIndex < sections.length) { + while (nextIndex < allSectionsForBrokenLinks.length) { if (shouldSectionRender(nextIndex)) { return nextIndex; } @@ -108,42 +212,41 @@ const ScanResults: FC = ({ data }) => { }; return ( -
-
-

{intl.formatMessage(messages.scanHeader)}

-
-
-
-

{intl.formatMessage(messages.brokenLinksHeader)}

- -
-
- +
+
+
+

{intl.formatMessage(messages.brokenLinksHeader)}

+ +
+
+ - {activeFilters.length > 0 &&
} - {activeFilters.length > 0 && ( + onClose={close} + onApply={setFilters} + positionRef={buttonRef} + filterOptions={filterOptions} + initialFilters={filters} + activeFilters={activeFilters} + filterBy={filterBy} + add={add} + remove={remove} + set={set} + /> + {activeFilters.length > 0 &&
} + {activeFilters.length > 0 && (
{activeFilters.map(filter => ( @@ -174,62 +277,124 @@ const ScanResults: FC = ({ data }) => { {intl.formatMessage(messages.clearFilters)}
+ )} + + {allSectionsForBrokenLinks && allSectionsForBrokenLinks.length > 0 ? ( + allSectionsForBrokenLinks.map((section, index) => { + if (!shouldSectionRender(index)) { + return null; + } + hasSectionsRendered = true; + return ( + 0 ? (() => { + const prevVisibleIndex = findPreviousVisibleSection(index); + return prevVisibleIndex >= 0 && openStates[prevVisibleIndex]; + })() : true} + hasNextAndIsOpen={index < allSectionsForBrokenLinks.length - 1 ? (() => { + const nextVisibleIndex = findNextVisibleSection(index); + return nextVisibleIndex >= 1 && openStates[nextVisibleIndex]; + })() : true} + key={section.id} + title={section.displayName} + brokenNumber={brokenLinksCounts[index]} + manualNumber={externalForbiddenLinksCounts[index]} + lockedNumber={lockedLinksCounts[index]} + className="section-collapsible-header" + > + {section.subsections.map((subsection) => ( + <> + {subsection.units.map((unit) => { + if ( + (!filters.brokenLinks && !filters.externalForbiddenLinks && !filters.lockedLinks) + || (filters.brokenLinks && unit.blocks.some(block => block.brokenLinks.length > 0)) + || (filters.externalForbiddenLinks + && unit.blocks.some(block => block.externalForbiddenLinks.length > 0)) + || (filters.lockedLinks && unit.blocks.some(block => block.lockedLinks.length > 0)) + ) { + return ( +
+ +
+ ); + } + return null; + })} + + ))} + + ); + }) + ) : ( +
+

{intl.formatMessage(messages.noResultsFound)}

+
+ )} + + {hasSectionsRendered === false && allSectionsForBrokenLinks && allSectionsForBrokenLinks.length > 0 && ( +
+

{intl.formatMessage(messages.noResultsFound)}

+
+ )} +
+ {waffleFlags.enableCourseOptimizerCheckPrevRunLinks + && allSectionsForPrevRun + && allSectionsForPrevRun.length > 0 + && hasPreviousRunLinks && ( +
+
+
+

{intl.formatMessage(messages.linkToPrevCourseRun)}

+
+
+ {allSectionsForPrevRun?.map((section, index) => ( + 0 ? prevRunOpenStates[index - 1] : true} + hasNextAndIsOpen={index < allSectionsForPrevRun.length - 1 ? prevRunOpenStates[index + 1] : true} + key={section.id} + title={section.displayName} + previousRunLinksCount={previousRunLinksCounts[index]} + className="section-collapsible-header" + > + {section.subsections.map((subsection) => ( + <> + {subsection.units.map((unit) => { + // Only render units that have previous run links + if (hasPreviousRunLinks) { + return ( +
+ +
+ ); + } + return null; + })} + + ))} + + ))} +
)} - {sections?.map((section, index) => { - if (!shouldSectionRender(index)) { - return null; - } - hasSectionsRendered = true; - return ( - 0 ? (() => { - const prevVisibleIndex = findPreviousVisibleSection(index); - return prevVisibleIndex >= 0 && openStates[prevVisibleIndex]; - })() : true} - hasNextAndIsOpen={index < sections.length - 1 ? (() => { - const nextVisibleIndex = findNextVisibleSection(index); - return nextVisibleIndex >= 1 && openStates[nextVisibleIndex]; - })() : true} - key={section.id} - title={section.displayName} - brokenNumber={brokenLinksCounts[index]} - manualNumber={externalForbiddenLinksCounts[index]} - lockedNumber={lockedLinksCounts[index]} - className="section-collapsible-header" - > - {section.subsections.map((subsection) => ( - <> - {subsection.units.map((unit) => { - if ( - (!filters.brokenLinks && !filters.externalForbiddenLinks && !filters.lockedLinks) - || (filters.brokenLinks && unit.blocks.some(block => block.brokenLinks.length > 0)) - || (filters.externalForbiddenLinks - && unit.blocks.some(block => block.externalForbiddenLinks.length > 0)) - || (filters.lockedLinks && unit.blocks.some(block => block.lockedLinks.length > 0)) - ) { - return ( -
- -
- ); - } - return null; - })} - - ))} - - ); - })} - {hasSectionsRendered === false && ( + {waffleFlags.enableCourseOptimizerCheckPrevRunLinks && !hasPreviousRunLinks && ( +
+
+
+

{intl.formatMessage(messages.linkToPrevCourseRun)}

+
+

{intl.formatMessage(messages.noResultsFound)}

+
)} -
+ ); }; diff --git a/src/optimizer-page/scan-results/messages.js b/src/optimizer-page/scan-results/messages.js index 83b48bc1b5..7f188bb3ea 100644 --- a/src/optimizer-page/scan-results/messages.js +++ b/src/optimizer-page/scan-results/messages.js @@ -13,14 +13,14 @@ const messages = defineMessages({ id: 'course-authoring.course-optimizer.emptyResultsCard', defaultMessage: 'No broken links found', }, + linkToPrevCourseRun: { + id: 'course-authoring.course-optimizer.linkToPrevCourseRun', + defaultMessage: 'Links to previous course run', + }, noResultsFound: { id: 'course-authoring.course-optimizer.noResultsFound', defaultMessage: 'No results found', }, - scanHeader: { - id: 'course-authoring.course-optimizer.scanHeader', - defaultMessage: 'Scan results', - }, brokenLinksHeader: { id: 'course-authoring.course-optimizer.brokenLinksHeader', defaultMessage: 'Broken links', @@ -62,6 +62,14 @@ const messages = defineMessages({ id: 'course-authoring.course-optimizer.clearFilters', defaultMessage: 'Clear filters', }, + customPagesHeader: { + id: 'course-authoring.course-optimizer.customPagesHeader', + defaultMessage: 'Custom pages', + }, + courseUpdatesHeader: { + id: 'course-authoring.course-optimizer.courseUpdatesHeader', + defaultMessage: 'Course updates', + }, }); export default messages; diff --git a/src/optimizer-page/types.ts b/src/optimizer-page/types.ts index 9e8d946bcc..6faf1491ee 100644 --- a/src/optimizer-page/types.ts +++ b/src/optimizer-page/types.ts @@ -8,6 +8,7 @@ export interface Unit { brokenLinks: string[]; lockedLinks: string[]; externalForbiddenLinks: string[]; + previousRunLinks: string[]; }[]; } @@ -25,6 +26,22 @@ export interface Section { export interface LinkCheckResult { sections: Section[]; + course_updates?: { + name: string; + url: string; + brokenLinks: string[]; + lockedLinks: string[]; + externalForbiddenLinks: string[]; + previousRunLinks: string[]; + }[]; + custom_pages?: { + name: string; + url: string; + brokenLinks: string[]; + lockedLinks: string[]; + externalForbiddenLinks: string[]; + previousRunLinks: string[]; + }[]; } export interface Filters { From 2b44e8f01a5756916e07044ca723566dae5c60a8 Mon Sep 17 00:00:00 2001 From: pganesh-apphelix Date: Tue, 12 Aug 2025 13:02:25 +0000 Subject: [PATCH 2/4] fix: refactor test cases and code --- .../CourseOptimizerPage.test.js | 113 ++++++++- src/optimizer-page/CourseOptimizerPage.tsx | 2 +- src/optimizer-page/mocks/mockApiResponse.js | 98 ++++++++ .../scan-results/BrokenLinkTable.tsx | 150 ++++++----- .../scan-results/PreviousRunLinkTable.tsx | 102 -------- .../PreviousRunSectionCollapsible.tsx | 70 ------ .../scan-results/ScanResults.tsx | 233 +++++++++++------- .../scan-results/SectionCollapsible.test.tsx | 89 +++++++ .../scan-results/SectionCollapsible.tsx | 48 ++-- src/optimizer-page/scan-results/messages.js | 4 - src/optimizer-page/types.ts | 4 +- 11 files changed, 562 insertions(+), 351 deletions(-) delete mode 100644 src/optimizer-page/scan-results/PreviousRunLinkTable.tsx delete mode 100644 src/optimizer-page/scan-results/PreviousRunSectionCollapsible.tsx create mode 100644 src/optimizer-page/scan-results/SectionCollapsible.test.tsx diff --git a/src/optimizer-page/CourseOptimizerPage.test.js b/src/optimizer-page/CourseOptimizerPage.test.js index a497b02624..27aaf961ee 100644 --- a/src/optimizer-page/CourseOptimizerPage.test.js +++ b/src/optimizer-page/CourseOptimizerPage.test.js @@ -15,8 +15,14 @@ import generalMessages from '../messages'; import scanResultsMessages from './scan-results/messages'; import CourseOptimizerPage, { pollLinkCheckDuringScan } from './CourseOptimizerPage'; import { postLinkCheckCourseApiUrl, getLinkCheckStatusApiUrl } from './data/api'; -import { mockApiResponse, mockApiResponseForNoResultFound } from './mocks/mockApiResponse'; +import { + mockApiResponse, + mockApiResponseForNoResultFound, + mockApiResponseWithPreviousRunLinks, + mockApiResponseEmpty, +} from './mocks/mockApiResponse'; import * as thunks from './data/thunks'; +import { useWaffleFlags } from '../data/apiHooks'; let store; let axiosMock; @@ -29,6 +35,19 @@ jest.mock('../generic/model-store', () => ({ }), })); +// Mock the waffle flags hook +jest.mock('../data/apiHooks', () => ({ + useWaffleFlags: jest.fn(() => ({ + enableCourseOptimizerCheckPrevRunLinks: false, + })), +})); + +jest.mock('../generic/model-store', () => ({ + useModel: jest.fn().mockReturnValue({ + name: 'About Node JS', + }), +})); + const OptimizerPage = () => ( @@ -155,7 +174,7 @@ describe('CourseOptimizerPage', () => { expect(getByText(messages.headingTitle.defaultMessage)).toBeInTheDocument(); fireEvent.click(getByText(messages.buttonTitle.defaultMessage)); await waitFor(() => { - expect(getByText(scanResultsMessages.noBrokenLinksCard.defaultMessage)).toBeInTheDocument(); + expect(getByText(scanResultsMessages.noResultsFound.defaultMessage)).toBeInTheDocument(); }); }); @@ -180,7 +199,7 @@ describe('CourseOptimizerPage', () => { } = await setupOptimizerPage(); // Check if the modal is opened expect(getByText('Locked')).toBeInTheDocument(); - // Select the broken links checkbox + // Select the locked links checkbox fireEvent.click(getByLabelText(scanResultsMessages.lockedLabel.defaultMessage)); const collapsibleTrigger = container.querySelector('.collapsible-trigger'); @@ -205,7 +224,6 @@ describe('CourseOptimizerPage', () => { expect(getByText('Broken')).toBeInTheDocument(); // Select the broken links checkbox fireEvent.click(getByLabelText(scanResultsMessages.brokenLabel.defaultMessage)); - const collapsibleTrigger = container.querySelector('.collapsible-trigger'); expect(collapsibleTrigger).toBeInTheDocument(); fireEvent.click(collapsibleTrigger); @@ -317,14 +335,14 @@ describe('CourseOptimizerPage', () => { expect(collapsibleTrigger).toBeInTheDocument(); fireEvent.click(collapsibleTrigger); - // Assert that all links are displayed + // Assert that both links are displayed await waitFor(() => { expect(getByText('Test Broken Links')).toBeInTheDocument(); expect(getByText('Test Manual Links')).toBeInTheDocument(); expect(queryByText('Test Locked Links')).not.toBeInTheDocument(); }); - // Click on the "Broken" chip to filter the results + // Click on the "Broken" chip to remove the broken filter (should leave only manual) const brokenChip = getByTestId('chip-brokenLinks'); fireEvent.click(brokenChip); @@ -361,5 +379,88 @@ describe('CourseOptimizerPage', () => { expect(getByText(scanResultsMessages.noResultsFound.defaultMessage)).toBeInTheDocument(); }); }); + + it('should always show broken links section header even when no data', async () => { + axiosMock.onGet(getLinkCheckStatusApiUrl(courseId)).reply(200, mockApiResponseEmpty); + const { getByText } = render(); + + fireEvent.click(getByText(messages.buttonTitle.defaultMessage)); + + await waitFor(() => { + expect(getByText(scanResultsMessages.brokenLinksHeader.defaultMessage)).toBeInTheDocument(); + expect(getByText(scanResultsMessages.noResultsFound.defaultMessage)).toBeInTheDocument(); + }); + }); + + describe('Previous Run Links Feature', () => { + beforeEach(() => { + // Enable the waffle flag for previous run links + useWaffleFlags.mockReturnValue({ + enableCourseOptimizerCheckPrevRunLinks: true, + }); + }); + + afterEach(() => { + // Reset to default (disabled) + useWaffleFlags.mockReturnValue({ + enableCourseOptimizerCheckPrevRunLinks: false, + }); + }); + + it('should show previous run links section when waffle flag is enabled and links exist', async () => { + axiosMock.onGet(getLinkCheckStatusApiUrl(courseId)).reply(200, mockApiResponseWithPreviousRunLinks); + const { getByText } = render(); + + fireEvent.click(getByText(messages.buttonTitle.defaultMessage)); + + await waitFor(() => { + expect(getByText(scanResultsMessages.linkToPrevCourseRun.defaultMessage)).toBeInTheDocument(); + }); + }); + + it('should show no results found for previous run links when flag is enabled but no links exist', async () => { + axiosMock.onGet(getLinkCheckStatusApiUrl(courseId)).reply(200, mockApiResponseForNoResultFound); + const { getByText, getAllByText } = render(); + + fireEvent.click(getByText(messages.buttonTitle.defaultMessage)); + + await waitFor(() => { + expect(getByText(scanResultsMessages.linkToPrevCourseRun.defaultMessage)).toBeInTheDocument(); + // Should show "No results found" for previous run section + const noResultsElements = getAllByText(scanResultsMessages.noResultsFound.defaultMessage); + expect(noResultsElements.length).toBeGreaterThan(0); + }); + }); + + it('should not show previous run links section when waffle flag is disabled', async () => { + // Disable the flag + useWaffleFlags.mockReturnValue({ + enableCourseOptimizerCheckPrevRunLinks: false, + }); + + axiosMock.onGet(getLinkCheckStatusApiUrl(courseId)).reply(200, mockApiResponseWithPreviousRunLinks); + const { getByText, queryByText } = render(); + + fireEvent.click(getByText(messages.buttonTitle.defaultMessage)); + + await waitFor(() => { + expect(queryByText(scanResultsMessages.linkToPrevCourseRun.defaultMessage)).not.toBeInTheDocument(); + }); + }); + + it('should handle previous run links in course updates and custom pages', async () => { + axiosMock.onGet(getLinkCheckStatusApiUrl(courseId)).reply(200, mockApiResponseWithPreviousRunLinks); + const { getByText, container } = render(); + + fireEvent.click(getByText(messages.buttonTitle.defaultMessage)); + + await waitFor(() => { + expect(getByText(scanResultsMessages.linkToPrevCourseRun.defaultMessage)).toBeInTheDocument(); + + const prevRunSections = container.querySelectorAll('.scan-results'); + expect(prevRunSections.length).toBeGreaterThan(1); + }); + }); + }); }); }); diff --git a/src/optimizer-page/CourseOptimizerPage.tsx b/src/optimizer-page/CourseOptimizerPage.tsx index 14b4ccadad..a3a9bf109f 100644 --- a/src/optimizer-page/CourseOptimizerPage.tsx +++ b/src/optimizer-page/CourseOptimizerPage.tsx @@ -151,7 +151,7 @@ const CourseOptimizerPage: FC<{ courseId: string }> = ({ courseId }) => { size="md" className="px-4 rounded-0 scan-course-btn" onClick={() => dispatch(startLinkCheck(courseId))} - disabled={linkCheckInProgress && !errorMessage} + disabled={(!!linkCheckInProgress) && !errorMessage} > {linkCheckInProgress && !errorMessage ? ( <> diff --git a/src/optimizer-page/mocks/mockApiResponse.js b/src/optimizer-page/mocks/mockApiResponse.js index 98e9db5219..cfb38764a6 100644 --- a/src/optimizer-page/mocks/mockApiResponse.js +++ b/src/optimizer-page/mocks/mockApiResponse.js @@ -21,6 +21,7 @@ export const mockApiResponse = { brokenLinks: ['https://example.com/broken-link-algo1'], lockedLinks: [], externalForbiddenLinks: [], + previousRunLinks: [], }, ], }, @@ -34,6 +35,7 @@ export const mockApiResponse = { brokenLinks: [], lockedLinks: [], externalForbiddenLinks: ['https://outsider.com/forbidden-link-algo'], + previousRunLinks: [], }, ], }, @@ -47,6 +49,7 @@ export const mockApiResponse = { brokenLinks: [], lockedLinks: ['https://example.com/locked-link-algo'], externalForbiddenLinks: [], + previousRunLinks: [], }, ], }, @@ -72,6 +75,7 @@ export const mockApiResponse = { brokenLinks: ['https://example.com/broken-link-algo1'], lockedLinks: [], externalForbiddenLinks: [], + previousRunLinks: [], }, { id: 'block-1-1-1-6', @@ -79,6 +83,7 @@ export const mockApiResponse = { brokenLinks: ['https://example.com/broken-link-algo1'], lockedLinks: ['https://example.com/locked-link-algo'], externalForbiddenLinks: [], + previousRunLinks: [], }, { id: 'block-1-1-1-6', @@ -86,6 +91,7 @@ export const mockApiResponse = { brokenLinks: ['https://example.com/broken-link-algo1'], lockedLinks: [], externalForbiddenLinks: ['https://outsider.com/forbidden-link-algo'], + previousRunLinks: [], }, ], }, @@ -99,6 +105,7 @@ export const mockApiResponse = { brokenLinks: ['https://example.com/broken-link-algo1'], lockedLinks: [], externalForbiddenLinks: ['https://outsider.com/forbidden-link-algo'], + previousRunLinks: [], }, ], }, @@ -112,6 +119,7 @@ export const mockApiResponse = { brokenLinks: ['https://example.com/broken-link-algo1'], lockedLinks: ['https://example.com/locked-link-algo'], externalForbiddenLinks: ['https://outsider.com/forbidden-link-algo'], + previousRunLinks: [], }, ], }, @@ -120,6 +128,28 @@ export const mockApiResponse = { ], }, ], + courseUpdates: [ + { + id: 'update-1', + displayName: 'Course Update 1', + url: 'https://example.com/course-update-1', + brokenLinks: [], + lockedLinks: [], + externalForbiddenLinks: [], + previousRunLinks: [], + }, + ], + customPages: [ + { + id: 'custom-1', + displayName: 'About Page', + url: 'https://example.com/about', + brokenLinks: [], + lockedLinks: [], + externalForbiddenLinks: [], + previousRunLinks: [], + }, + ], }, }; @@ -146,6 +176,42 @@ export const mockApiResponseForNoResultFound = { brokenLinks: ['https://example.com/broken-link-algo1'], lockedLinks: [], externalForbiddenLinks: [], + previousRunLinks: [], + }, + ], + }, + ], + }, + ], + }, + ], + }, +}; + +export const mockApiResponseWithPreviousRunLinks = { + LinkCheckStatus: 'Succeeded', + LinkCheckCreatedAt: '2024-12-14T00:26:50.838350Z', + LinkCheckOutput: { + sections: [ + { + id: 'section-1', + displayName: 'Introduction to Programming', + subsections: [ + { + id: 'subsection-1-1', + displayName: 'Getting Started', + units: [ + { + id: 'unit-1-1-1', + displayName: 'Test Previous Run Links', + blocks: [ + { + id: 'block-1-1-1-5', + url: 'https://example.com/welcome-video', + brokenLinks: [], + lockedLinks: [], + externalForbiddenLinks: [], + previousRunLinks: ['https://example.com/old-course-run/content'], }, ], }, @@ -154,5 +220,37 @@ export const mockApiResponseForNoResultFound = { ], }, ], + courseUpdates: [ + { + id: 'update-1', + displayName: 'Course Update with Previous Run Link', + url: 'https://example.com/course-update-1', + brokenLinks: [], + lockedLinks: [], + externalForbiddenLinks: [], + previousRunLinks: ['https://example.com/old-course-run/update'], + }, + ], + customPages: [ + { + id: 'custom-2', + displayName: 'About Page with Previous Run', + url: 'https://example.com/about', + brokenLinks: [], + lockedLinks: [], + externalForbiddenLinks: [], + previousRunLinks: ['https://example.com/old-course-run/about'], + }, + ], + }, +}; + +export const mockApiResponseEmpty = { + LinkCheckStatus: 'Succeeded', + LinkCheckCreatedAt: '2024-12-14T00:26:50.838350Z', + LinkCheckOutput: { + sections: [], + courseUpdates: [], + customPages: [], }, }; diff --git a/src/optimizer-page/scan-results/BrokenLinkTable.tsx b/src/optimizer-page/scan-results/BrokenLinkTable.tsx index 90ff18cc73..bdab930cc7 100644 --- a/src/optimizer-page/scan-results/BrokenLinkTable.tsx +++ b/src/optimizer-page/scan-results/BrokenLinkTable.tsx @@ -60,26 +60,34 @@ const iconsMap = { }, }; -const LinksCol: FC<{ block: { url: string, displayName: string }, href: string, linkType: string }> = ( - { block, href, linkType }, -) => ( +const LinksCol: FC<{ + block: { url: string, displayName: string }, + href: string, + linkType?: string, + showIcon?: boolean +}> = ({ + block, href, linkType, showIcon = true, +}) => ( -
- -
+ {showIcon && linkType && iconsMap[linkType] && ( +
+ +
+ )} ); interface BrokenLinkTableProps { unit: Unit; - filters: Filters; + filters?: Filters; + linkType?: 'broken' | 'previous'; } type TableData = { @@ -89,60 +97,80 @@ type TableData = { const BrokenLinkTable: FC = ({ unit, filters, + linkType = 'broken', }) => { const brokenLinkList = unit.blocks.reduce( ( acc: TableData, block, ) => { - if ( - filters.brokenLinks - || (!filters.brokenLinks && !filters.externalForbiddenLinks && !filters.lockedLinks) - ) { - const blockBrokenLinks = block.brokenLinks.map((link) => ({ - Links: ( - - ), - })); - acc.push(...blockBrokenLinks); - } - - if ( - filters.lockedLinks - || (!filters.brokenLinks && !filters.externalForbiddenLinks && !filters.lockedLinks) - ) { - const blockLockedLinks = block.lockedLinks.map((link) => ({ - Links: ( - - ), - })); - - acc.push(...blockLockedLinks); - } - - if ( - filters.externalForbiddenLinks - || (!filters.brokenLinks && !filters.externalForbiddenLinks && !filters.lockedLinks) - ) { - const externalForbiddenLinks = block.externalForbiddenLinks.map((link) => ({ - Links: ( - - ), - })); - - acc.push(...externalForbiddenLinks); + if (linkType === 'previous') { + // Handle previous run links (no filtering, no icons) + if (block.previousRunLinks && block.previousRunLinks.length > 0) { + const blockPreviousRunLinks = block.previousRunLinks.map((link) => ({ + Links: ( + + ), + })); + acc.push(...blockPreviousRunLinks); + } + } else { + // Handle broken links with filtering and icons + if (!filters) { return acc; } + + if ( + filters.brokenLinks + || (!filters.brokenLinks && !filters.externalForbiddenLinks && !filters.lockedLinks) + ) { + const blockBrokenLinks = block.brokenLinks.map((link) => ({ + Links: ( + + ), + })); + acc.push(...blockBrokenLinks); + } + + if ( + filters.lockedLinks + || (!filters.brokenLinks && !filters.externalForbiddenLinks && !filters.lockedLinks) + ) { + const blockLockedLinks = block.lockedLinks.map((link) => ({ + Links: ( + + ), + })); + + acc.push(...blockLockedLinks); + } + + if ( + filters.externalForbiddenLinks + || (!filters.brokenLinks && !filters.externalForbiddenLinks && !filters.lockedLinks) + ) { + const externalForbiddenLinks = block.externalForbiddenLinks.map((link) => ({ + Links: ( + + ), + })); + + acc.push(...externalForbiddenLinks); + } } return acc; @@ -150,6 +178,10 @@ const BrokenLinkTable: FC = ({ [], ); + if (brokenLinkList.length === 0) { + return null; + } + return (

{unit.displayName}

diff --git a/src/optimizer-page/scan-results/PreviousRunLinkTable.tsx b/src/optimizer-page/scan-results/PreviousRunLinkTable.tsx deleted file mode 100644 index 8ddc8f61eb..0000000000 --- a/src/optimizer-page/scan-results/PreviousRunLinkTable.tsx +++ /dev/null @@ -1,102 +0,0 @@ -import { - Card, Icon, DataTable, -} from '@openedx/paragon'; -import { - ArrowForwardIos, -} from '@openedx/paragon/icons'; -import { FC } from 'react'; -import { Unit } from '../types'; - -const PreviousRunLinkHref: FC<{ href: string }> = ({ href }) => { - const handleClick = (event: React.MouseEvent) => { - event.preventDefault(); - window.open(href, '_blank'); - }; - - return ( - - ); -}; - -const GoToBlock: FC<{ block: { url: string, displayName: string } }> = ({ block }) => { - const handleClick = (event: React.MouseEvent) => { - event.preventDefault(); - window.open(block.url, '_blank'); - }; - - return ( - - ); -}; - -const PreviousRunLinksCol: FC<{ block: { url: string, displayName: string }, href: string }> = ({ block, href }) => ( - - - - - -); - -interface PreviousRunLinkTableProps { - unit: Unit; -} - -type TableData = { - Links: JSX.Element; -}[]; - -const PreviousRunLinkTable: FC = ({ - unit, -}) => { - const previousRunLinkList = unit.blocks.reduce( - ( - acc: TableData, - block, - ) => { - if (block.previousRunLinks && block.previousRunLinks.length > 0) { - const blockPreviousRunLinks = block.previousRunLinks.map((link) => ({ - Links: ( - - ), - })); - acc.push(...blockPreviousRunLinks); - } - - return acc; - }, - [], - ); - - if (previousRunLinkList.length === 0) { - return null; - } - - return ( - -

{unit.displayName}

- - - ); -}; - -export default PreviousRunLinkTable; diff --git a/src/optimizer-page/scan-results/PreviousRunSectionCollapsible.tsx b/src/optimizer-page/scan-results/PreviousRunSectionCollapsible.tsx deleted file mode 100644 index b71b8027ec..0000000000 --- a/src/optimizer-page/scan-results/PreviousRunSectionCollapsible.tsx +++ /dev/null @@ -1,70 +0,0 @@ -import { FC } from 'react'; -import { - Collapsible, - Icon, -} from '@openedx/paragon'; -import { - ArrowRight, - ArrowDropDown, -} from '@openedx/paragon/icons'; - -interface Props { - index: number; - handleToggle: Function; - isOpen: boolean; - hasPrevAndIsOpen: boolean; - hasNextAndIsOpen: boolean; - title: string; - children: React.ReactNode; - previousRunLinksCount: number; - className?: string; -} - -const PreviousRunSectionCollapsible: FC = ({ - index, - handleToggle, - isOpen, - hasPrevAndIsOpen, - hasNextAndIsOpen, - title, - children, - previousRunLinksCount, - className, -}) => { - const styling = `card-lg open-section-rounded ${hasPrevAndIsOpen ? 'closed-section-rounded-top' : ''} ${hasNextAndIsOpen ? 'closed-section-rounded-bottom' : ''}`; - const collapsibleTitle = ( -
-
- -

{title}

-
-
-
-

{previousRunLinksCount > 0 ? previousRunLinksCount : '-'}

-
-
-
- ); - - return ( -
- - {collapsibleTitle} -

- )} - iconWhenClosed="" - iconWhenOpen="" - open={isOpen} - onToggle={() => handleToggle(index)} - > - {children} - -
- ); -}; - -export default PreviousRunSectionCollapsible; diff --git a/src/optimizer-page/scan-results/ScanResults.tsx b/src/optimizer-page/scan-results/ScanResults.tsx index 44c3efac10..5a9c3c20e2 100644 --- a/src/optimizer-page/scan-results/ScanResults.tsx +++ b/src/optimizer-page/scan-results/ScanResults.tsx @@ -18,9 +18,7 @@ import { import { useIntl } from '@edx/frontend-platform/i18n'; import messages from './messages'; import SectionCollapsible from './SectionCollapsible'; -import PreviousRunSectionCollapsible from './PreviousRunSectionCollapsible'; import BrokenLinkTable from './BrokenLinkTable'; -import PreviousRunLinkTable from './PreviousRunLinkTable'; import { LinkCheckResult } from '../types'; import { countBrokenLinks } from '../utils'; import FilterModal from './filterModal'; @@ -42,7 +40,6 @@ interface Props { } const ScanResults: FC = ({ data }) => { - let hasSectionsRendered = false; const intl = useIntl(); const waffleFlags = useWaffleFlags(); const [isOpen, open, close] = useToggle(false); @@ -57,8 +54,8 @@ const ScanResults: FC = ({ data }) => { const [prevRunOpenStates, setPrevRunOpenStates] = useState([]); const { sections } = data || {}; - const virtualSections = useMemo(() => { - const createVirtualSection = ( + const renderableSections = useMemo(() => { + const buildSectionData = ( items: any[], sectionId: string, messageKey: keyof typeof messages, @@ -77,12 +74,12 @@ const ScanResults: FC = ({ data }) => { id: `${sectionId}-subsection`, displayName: `${intl.formatMessage(messages[messageKey])} Subsection`, units: itemsWithLinks.map(item => ({ - id: item.name, - displayName: item.name, + id: item.id, + displayName: item.displayName, url: item.url, blocks: [{ - id: `${item.name}-block`, - displayName: item.name, + id: item.id, + displayName: item.displayName, url: item.url, brokenLinks: item.brokenLinks || [], lockedLinks: item.lockedLinks || [], @@ -94,38 +91,38 @@ const ScanResults: FC = ({ data }) => { }; }; - const vSections = []; + const rSections: any[] = []; - if (data?.courseUpdates?.length > 0) { - const courseUpdatesSection = createVirtualSection(data.courseUpdates, 'course-updates', 'courseUpdatesHeader'); + if (data?.courseUpdates && data.courseUpdates.length > 0) { + const courseUpdatesSection = buildSectionData(data.courseUpdates, 'course-updates', 'courseUpdatesHeader'); if (courseUpdatesSection) { - vSections.push(courseUpdatesSection); + rSections.push(courseUpdatesSection); } } - if (data?.customPages?.length > 0) { - const customPagesSection = createVirtualSection( + if (data?.customPages && data.customPages.length > 0) { + const customPagesSection = buildSectionData( data.customPages, 'custom-pages', 'customPagesHeader', ); if (customPagesSection) { - vSections.push(customPagesSection); + rSections.push(customPagesSection); } } - return vSections; + return rSections; }, [data?.courseUpdates, data?.customPages, intl]); - // Combine virtual sections with regular sections + // Combine renderable sections with regular sections const allSectionsForBrokenLinks = useMemo( - () => [...virtualSections, ...(sections || [])], - [virtualSections, sections], + () => [...renderableSections, ...(sections || [])], + [renderableSections, sections], ); const allSectionsForPrevRun = useMemo( - () => [...virtualSections, ...(sections || [])], - [virtualSections, sections], + () => [...renderableSections, ...(sections || [])], + [renderableSections, sections], ); const { @@ -169,7 +166,7 @@ const ScanResults: FC = ({ data }) => { }, [allSectionsForBrokenLinks, allSectionsForPrevRun]); if (!data) { - return ; + return ; } const handleToggle = (index: number) => { setOpenStates(prev => prev.map((isOpened, i) => (i === index ? !isOpened : isOpened))); @@ -182,12 +179,32 @@ const ScanResults: FC = ({ data }) => { { name: intl.formatMessage(messages.manualLabel), value: 'externalForbiddenLinks' }, { name: intl.formatMessage(messages.lockedLabel), value: 'lockedLinks' }, ]; - const shouldSectionRender = (sectionIndex: number): boolean => ( - (!filters.brokenLinks && !filters.externalForbiddenLinks && !filters.lockedLinks) - || (filters.brokenLinks && brokenLinksCounts[sectionIndex] > 0) - || (filters.externalForbiddenLinks && externalForbiddenLinksCounts[sectionIndex] > 0) - || (filters.lockedLinks && lockedLinksCounts[sectionIndex] > 0) - ); + // Only show sections that have at least one unit with a visible link (not just previousRunLinks) + const shouldSectionRender = (sectionIndex: number): boolean => { + const section = allSectionsForBrokenLinks[sectionIndex]; + // eslint-disable-next-line max-len + const hasVisibleUnit = section.subsections.some((subsection) => subsection.units.some((unit) => unit.blocks.some((block) => { + const hasBroken = block.brokenLinks?.length > 0; + const hasLocked = block.lockedLinks?.length > 0; + const hasExternal = block.externalForbiddenLinks?.length > 0; + + const noFilters = !filters.brokenLinks + && !filters.lockedLinks + && !filters.externalForbiddenLinks; + + const showBroken = filters.brokenLinks && hasBroken; + const showLocked = filters.lockedLinks && hasLocked; + const showExternal = filters.externalForbiddenLinks && hasExternal; + + return ( + showBroken + || showLocked + || showExternal + || (noFilters && (hasBroken || hasLocked || hasExternal)) + ); + }))); + return hasVisibleUnit; + }; const findPreviousVisibleSection = (currentIndex: number): number => { let prevIndex = currentIndex - 1; @@ -279,12 +296,24 @@ const ScanResults: FC = ({ data }) => {
)} - {allSectionsForBrokenLinks && allSectionsForBrokenLinks.length > 0 ? ( - allSectionsForBrokenLinks.map((section, index) => { + {(() => { + // Find all visible sections + const visibleSections = allSectionsForBrokenLinks && allSectionsForBrokenLinks.length > 0 + ? allSectionsForBrokenLinks + .map((_, index) => (shouldSectionRender(index) ? index : -1)) + .filter(idx => idx !== -1) + : []; + if (visibleSections.length === 0) { + return ( +
+

{intl.formatMessage(messages.noResultsFound)}

+
+ ); + } + return allSectionsForBrokenLinks.map((section, index) => { if (!shouldSectionRender(index)) { return null; } - hasSectionsRendered = true; return ( = ({ data }) => { {section.subsections.map((subsection) => ( <> {subsection.units.map((unit) => { - if ( - (!filters.brokenLinks && !filters.externalForbiddenLinks && !filters.lockedLinks) - || (filters.brokenLinks && unit.blocks.some(block => block.brokenLinks.length > 0)) - || (filters.externalForbiddenLinks - && unit.blocks.some(block => block.externalForbiddenLinks.length > 0)) - || (filters.lockedLinks && unit.blocks.some(block => block.lockedLinks.length > 0)) - ) { + // Determine if any block in this unit should be shown based on filters + const hasVisibleBlock = unit.blocks.some((block) => { + const hasBroken = block.brokenLinks?.length > 0; + const hasLocked = block.lockedLinks?.length > 0; + const hasExternal = block.externalForbiddenLinks?.length > 0; + + const showBroken = filters.brokenLinks && hasBroken; + const showLocked = filters.lockedLinks && hasLocked; + const showExternal = filters.externalForbiddenLinks && hasExternal; + + const noFilters = !filters.brokenLinks + && !filters.lockedLinks + && !filters.externalForbiddenLinks; + + return showBroken + || showLocked + || showExternal + || (noFilters && (hasBroken || hasLocked || hasExternal)); + }); + + if (hasVisibleBlock) { return ( -
+
); @@ -327,60 +370,72 @@ const ScanResults: FC = ({ data }) => { ))} ); - }) - ) : ( -
-

{intl.formatMessage(messages.noResultsFound)}

-
- )} - - {hasSectionsRendered === false && allSectionsForBrokenLinks && allSectionsForBrokenLinks.length > 0 && ( -
-

{intl.formatMessage(messages.noResultsFound)}

-
- )} + }); + })()}
+ {waffleFlags.enableCourseOptimizerCheckPrevRunLinks && allSectionsForPrevRun && allSectionsForPrevRun.length > 0 - && hasPreviousRunLinks && ( -
-
-
-

{intl.formatMessage(messages.linkToPrevCourseRun)}

-
+ && hasPreviousRunLinks && (() => { + // Filter out sections/subsections/units that have no previous run links + const filteredSections = allSectionsForPrevRun.map((section) => { + // Filter subsections + const filteredSubsections = section.subsections.map(subsection => { + // Filter units + const filteredUnits = subsection.units.filter(unit => unit.blocks.some(block => { + const hasPreviousLinks = block.previousRunLinks?.length > 0; + return hasPreviousLinks; + })); + return { + ...subsection, + units: filteredUnits, + }; + }).filter(subsection => subsection.units.length > 0); + return { + ...section, + subsections: filteredSubsections, + }; + }).filter(section => section.subsections.length > 0); + + if (filteredSections.length === 0) { + return null; + } + + return ( +
+
+
+

{intl.formatMessage(messages.linkToPrevCourseRun)}

+
+
+ {filteredSections.map((section, index) => ( + 0 ? prevRunOpenStates[index - 1] : true} + hasNextAndIsOpen={index < filteredSections.length - 1 ? prevRunOpenStates[index + 1] : true} + key={section.id} + title={section.displayName} + previousRunLinksCount={previousRunLinksCounts[index]} + isPreviousRunLinks + className="section-collapsible-header" + > + {section.subsections.map((subsection) => ( + <> + {subsection.units.map((unit) => ( +
+ +
+ ))} + + ))} + + ))}
- {allSectionsForPrevRun?.map((section, index) => ( - 0 ? prevRunOpenStates[index - 1] : true} - hasNextAndIsOpen={index < allSectionsForPrevRun.length - 1 ? prevRunOpenStates[index + 1] : true} - key={section.id} - title={section.displayName} - previousRunLinksCount={previousRunLinksCounts[index]} - className="section-collapsible-header" - > - {section.subsections.map((subsection) => ( - <> - {subsection.units.map((unit) => { - // Only render units that have previous run links - if (hasPreviousRunLinks) { - return ( -
- -
- ); - } - return null; - })} - - ))} - - ))} -
- )} + ); + })()} {waffleFlags.enableCourseOptimizerCheckPrevRunLinks && !hasPreviousRunLinks && (
diff --git a/src/optimizer-page/scan-results/SectionCollapsible.test.tsx b/src/optimizer-page/scan-results/SectionCollapsible.test.tsx new file mode 100644 index 0000000000..e0cbf9296a --- /dev/null +++ b/src/optimizer-page/scan-results/SectionCollapsible.test.tsx @@ -0,0 +1,89 @@ +import { render, screen, fireEvent } from '@testing-library/react'; +import { IntlProvider } from '@edx/frontend-platform/i18n'; +import SectionCollapsible from './SectionCollapsible'; + +const intlWrapper = (ui: React.ReactElement) => render( + + {ui} + , +); + +describe('SectionCollapsible', () => { + const defaultProps = { + index: 1, + handleToggle: jest.fn(), + isOpen: false, + hasPrevAndIsOpen: false, + hasNextAndIsOpen: false, + title: 'Section Title', + children:
Section Content
, + className: 'test-class', + }; + + describe('Regular mode (broken/manual/locked links)', () => { + const regularProps = { + ...defaultProps, + brokenNumber: 3, + manualNumber: 2, + lockedNumber: 1, + isPreviousRunLinks: false, + }; + + it('renders with open state and shows children', () => { + intlWrapper(); + expect(screen.getByText('Section Content')).toBeInTheDocument(); + }); + + it('calls handleToggle with index when header is clicked', () => { + const handleToggle = jest.fn(); + intlWrapper(); + + const header = screen.getByText('Section Title').closest('.section-collapsible-header-item'); + if (header) { + fireEvent.click(header); + } else { + fireEvent.click(screen.getByText('Section Title')); + } + expect(handleToggle).toHaveBeenCalledWith(1); + }); + }); + + describe('Previous run links mode', () => { + const prevRunProps = { + ...defaultProps, + previousRunLinksCount: 5, + isPreviousRunLinks: true, + }; + + it('renders with previous run links count', () => { + intlWrapper(); + expect(screen.getByText('Section Title')).toBeInTheDocument(); + expect(screen.getByText('5')).toBeInTheDocument(); + // Should not show broken/manual/locked icons in previous run mode + expect(screen.queryByText('3')).not.toBeInTheDocument(); + }); + + it('shows dash when previousRunLinksCount is 0', () => { + intlWrapper(); + expect(screen.getByText('-')).toBeInTheDocument(); + }); + + it('renders with open state and shows children', () => { + intlWrapper(); + expect(screen.getByText('Section Content')).toBeInTheDocument(); + }); + + it('calls handleToggle with index when header is clicked', () => { + const handleToggle = jest.fn(); + intlWrapper(); + + const header = screen.getByText('Section Title').closest('.section-collapsible-header-item'); + if (header) { + fireEvent.click(header); + } else { + fireEvent.click(screen.getByText('Section Title')); + } + expect(handleToggle).toHaveBeenCalledWith(1); + }); + }); +}); diff --git a/src/optimizer-page/scan-results/SectionCollapsible.tsx b/src/optimizer-page/scan-results/SectionCollapsible.tsx index d90591360b..064d8d739f 100644 --- a/src/optimizer-page/scan-results/SectionCollapsible.tsx +++ b/src/optimizer-page/scan-results/SectionCollapsible.tsx @@ -21,9 +21,11 @@ interface Props { hasNextAndIsOpen: boolean; title: string; children: React.ReactNode; - brokenNumber: number; - manualNumber: number; - lockedNumber: number; + brokenNumber?: number; + manualNumber?: number; + lockedNumber?: number; + previousRunLinksCount?: number; + isPreviousRunLinks?: boolean; className?: string; } @@ -35,9 +37,11 @@ const SectionCollapsible: FC = ({ hasNextAndIsOpen, title, children, - brokenNumber, - manualNumber, - lockedNumber, + brokenNumber = 0, + manualNumber = 0, + lockedNumber = 0, + previousRunLinksCount = 0, + isPreviousRunLinks = false, className, }) => { const styling = `card-lg open-section-rounded ${hasPrevAndIsOpen ? 'closed-section-rounded-top' : ''} ${hasNextAndIsOpen ? 'closed-section-rounded-bottom' : ''}`; @@ -48,18 +52,26 @@ const SectionCollapsible: FC = ({

{title}

-
- -

{brokenNumber}

-
-
- -

{manualNumber}

-
-
- -

{lockedNumber}

-
+ {isPreviousRunLinks ? ( +
+

{previousRunLinksCount > 0 ? previousRunLinksCount : '-'}

+
+ ) : ( + <> +
+ +

{brokenNumber}

+
+
+ +

{manualNumber}

+
+
+ +

{lockedNumber}

+
+ + )}
); diff --git a/src/optimizer-page/scan-results/messages.js b/src/optimizer-page/scan-results/messages.js index 7f188bb3ea..84499726fc 100644 --- a/src/optimizer-page/scan-results/messages.js +++ b/src/optimizer-page/scan-results/messages.js @@ -9,10 +9,6 @@ const messages = defineMessages({ id: 'course-authoring.course-optimizer.noDataCard', defaultMessage: 'No Scan data available', }, - noBrokenLinksCard: { - id: 'course-authoring.course-optimizer.emptyResultsCard', - defaultMessage: 'No broken links found', - }, linkToPrevCourseRun: { id: 'course-authoring.course-optimizer.linkToPrevCourseRun', defaultMessage: 'Links to previous course run', diff --git a/src/optimizer-page/types.ts b/src/optimizer-page/types.ts index 6faf1491ee..621cf173d0 100644 --- a/src/optimizer-page/types.ts +++ b/src/optimizer-page/types.ts @@ -26,7 +26,7 @@ export interface Section { export interface LinkCheckResult { sections: Section[]; - course_updates?: { + courseUpdates?: { name: string; url: string; brokenLinks: string[]; @@ -34,7 +34,7 @@ export interface LinkCheckResult { externalForbiddenLinks: string[]; previousRunLinks: string[]; }[]; - custom_pages?: { + customPages?: { name: string; url: string; brokenLinks: string[]; From 75edd3c3f873399e97713831f807a89be08b2fde Mon Sep 17 00:00:00 2001 From: pganesh-apphelix Date: Wed, 13 Aug 2025 06:10:28 +0000 Subject: [PATCH 3/4] fix: correct element alignment in optimizer page --- src/optimizer-page/scan-results/ScanResults.tsx | 6 +++--- src/optimizer-page/scan-results/SectionCollapsible.tsx | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/optimizer-page/scan-results/ScanResults.tsx b/src/optimizer-page/scan-results/ScanResults.tsx index 5a9c3c20e2..e0439d16c3 100644 --- a/src/optimizer-page/scan-results/ScanResults.tsx +++ b/src/optimizer-page/scan-results/ScanResults.tsx @@ -231,7 +231,7 @@ const ScanResults: FC = ({ data }) => { return ( <>
-
+

{intl.formatMessage(messages.brokenLinksHeader)}