From 1b537c3617596630fa96962c4246ad6893eabb72 Mon Sep 17 00:00:00 2001 From: edX requirements bot <49161187+edx-requirements-bot@users.noreply.github.com> Date: Sun, 30 Aug 2026 20:29:16 -0400 Subject: [PATCH 01/16] chore: update browserslist DB (#3221) Co-authored-by: abdullahwaheed <42172960+abdullahwaheed@users.noreply.github.com> --- package-lock.json | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index c3f55e5ca9..19cea7accb 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9535,9 +9535,9 @@ "license": "MIT" }, "node_modules/baseline-browser-mapping": { - "version": "2.11.13", - "resolved": "https://registry.npmjs.org/baseline-browser-mapping/-/baseline-browser-mapping-2.11.13.tgz", - "integrity": "sha512-k9HNuUVMlqVjQ9UHzfPjIqiDbWw7WqT1AoT7GL8VwvF3r0ZfArtgiSPAlmupyNquNgOJHTuH4CKYf8ttMTWBTQ==", + "version": "2.11.20", + "resolved": "https://registry.npmjs.org/baseline-browser-mapping/-/baseline-browser-mapping-2.11.20.tgz", + "integrity": "sha512-H0ulySigv6icDJ1F7SjtdCD6PrhTpdYCmP0CactWy1+ekh0AFd0o1Wn5T8b+hnTmdBx19u9yhL6wvCylXMY7zw==", "license": "Apache-2.0", "bin": { "baseline-browser-mapping": "dist/cli.cjs" @@ -9918,9 +9918,9 @@ } }, "node_modules/caniuse-lite": { - "version": "1.0.30001809", - "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001809.tgz", - "integrity": "sha512-xxWVywk6a6Arlk+hymeycyn/VgqEfLDxupvhH/xiY5SJ/18kmi9o6MiO320DCUzypORHLtvh0I4i04tUhCNHNQ==", + "version": "1.0.30001810", + "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001810.tgz", + "integrity": "sha512-TITQPUkaz+aVk5GL6NhOdwk1aEaNTSDPsGFWrTuhKGtjTF70jL/Oht2W4c6rXUe5fu7Ie19VIahAXHIIiWWNeg==", "funding": [ { "type": "opencollective", From a5be74da2faba9e8a513aad04f3d6a6f0dd20c61 Mon Sep 17 00:00:00 2001 From: Jacobo Dominguez Date: Mon, 31 Aug 2026 14:48:44 -0600 Subject: [PATCH 02/16] feat: permission checks on course outline page for content editing part 1 (#3196) --- .../course-apps/proctoring/Settings.test.jsx | 3 +- src/CourseAuthoringContext.tsx | 17 ++ src/authz/constants.ts | 2 + src/authz/permissionHelpers.test.ts | 4 + src/authz/permissionHelpers.ts | 4 + src/course-outline/CourseOutline.test.tsx | 6 + src/course-outline/CourseOutline.tsx | 15 +- src/course-outline/OutlineNode.tsx | 11 +- src/course-outline/OutlineTree.tsx | 7 +- .../card-header/CardHeader.test.tsx | 39 +++- src/course-outline/card-header/CardHeader.tsx | 208 +++++++++--------- .../OutlineSidebarPagesContext.tsx | 21 +- .../status-bar/StatusBar.test.tsx | 39 +++- src/course-outline/status-bar/StatusBar.tsx | 17 +- src/generic/sidebar/Sidebar.tsx | 9 +- src/studio-home/StudioHome.tsx | 16 +- 16 files changed, 285 insertions(+), 133 deletions(-) diff --git a/plugins/course-apps/proctoring/Settings.test.jsx b/plugins/course-apps/proctoring/Settings.test.jsx index 6a07c7143c..4ee6953dad 100644 --- a/plugins/course-apps/proctoring/Settings.test.jsx +++ b/plugins/course-apps/proctoring/Settings.test.jsx @@ -472,7 +472,8 @@ describe('ProctoredExamSettings', () => { }); // (1) for studio settings // (2) for course details - expect(axiosMock.history.get.length).toBe(2); + // (3) for user course permissions + expect(axiosMock.history.get.length).toBe(3); expect(axiosMock.history.get[0].url.includes('proctored_exam_settings')).toEqual(true); }); diff --git a/src/CourseAuthoringContext.tsx b/src/CourseAuthoringContext.tsx index bc444bd7cb..b62f6dbbeb 100644 --- a/src/CourseAuthoringContext.tsx +++ b/src/CourseAuthoringContext.tsx @@ -10,6 +10,8 @@ import { type UnitXBlock, type XBlock } from '@src/data/types'; import { CourseDetailsData } from './data/api'; import { useCourseDetails } from './data/apiHooks'; import { RequestStatusType } from './data/constants'; +import { useCourseUserPermissions } from '@src/authz/hooks'; +import { getCourseOutlinePermissions } from '@src/authz/permissionHelpers'; export type ModalState = { value?: XBlock | UnitXBlock; @@ -29,6 +31,9 @@ export type CourseAuthoringContextData = { currentUnlinkModalData?: ModalState; openUnlinkModal: (value: ModalState) => void; closeUnlinkModal: () => void; + isLoadingPermissions: boolean; + canEditCourseContent: boolean; + canPublishCourseContent: boolean; }; /** @@ -58,6 +63,12 @@ export const CourseAuthoringProvider = ({ closeUnlinkModal, ] = useToggleWithValue(); + const { + canEditCourseContent, + canPublishCourseContent, + isLoading: isLoadingUserPermissions, + } = useCourseUserPermissions(courseId, getCourseOutlinePermissions(courseId)); + const getUnitUrl = (locator: string) => `/course/${courseId}/container/${locator}`; /** @@ -78,6 +89,9 @@ export const CourseAuthoringProvider = ({ openUnlinkModal, closeUnlinkModal, currentUnlinkModalData, + isLoadingPermissions: isLoadingUserPermissions, + canEditCourseContent, + canPublishCourseContent, }), [ courseId, courseDetails, @@ -89,6 +103,9 @@ export const CourseAuthoringProvider = ({ openUnlinkModal, closeUnlinkModal, currentUnlinkModalData, + canEditCourseContent, + canPublishCourseContent, + isLoadingUserPermissions, ]); return ( diff --git a/src/authz/constants.ts b/src/authz/constants.ts index 028a395c3d..5944c910d9 100644 --- a/src/authz/constants.ts +++ b/src/authz/constants.ts @@ -17,7 +17,9 @@ export const CONTENT_LIBRARY_PERMISSIONS = { export const COURSE_PERMISSIONS = { VIEW_COURSE: 'courses.view_course', + CREATE_COURSE: 'courses.create_course', EDIT_COURSE_CONTENT: 'courses.edit_course_content', + PUBLISH_COURSE_CONTENT: 'courses.publish_course_content', MANAGE_ADVANCED_SETTINGS: 'courses.manage_advanced_settings', diff --git a/src/authz/permissionHelpers.test.ts b/src/authz/permissionHelpers.test.ts index 7798b9d5ca..781becd11d 100644 --- a/src/authz/permissionHelpers.test.ts +++ b/src/authz/permissionHelpers.test.ts @@ -164,6 +164,10 @@ describe('permissionHelpers', () => { action: COURSE_PERMISSIONS.EDIT_COURSE_CONTENT, scope: courseId, }, + canPublishCourseContent: { + action: COURSE_PERMISSIONS.PUBLISH_COURSE_CONTENT, + scope: courseId, + }, }); }); }); diff --git a/src/authz/permissionHelpers.ts b/src/authz/permissionHelpers.ts index 3520e80f10..c8f301b5d1 100644 --- a/src/authz/permissionHelpers.ts +++ b/src/authz/permissionHelpers.ts @@ -75,6 +75,10 @@ export const getCourseOutlinePermissions = (courseId: string) => ({ action: COURSE_PERMISSIONS.EDIT_COURSE_CONTENT, scope: courseId, }, + canPublishCourseContent: { + action: COURSE_PERMISSIONS.PUBLISH_COURSE_CONTENT, + scope: courseId, + }, }); export const getLibraryUpdatesPermissions = (courseId: string) => ({ diff --git a/src/course-outline/CourseOutline.test.tsx b/src/course-outline/CourseOutline.test.tsx index bc97222ff7..55e28655e0 100644 --- a/src/course-outline/CourseOutline.test.tsx +++ b/src/course-outline/CourseOutline.test.tsx @@ -9,6 +9,7 @@ import { getClipboardUrl } from '@src/generic/data/api'; import { ContainerType } from '@src/generic/key-utils'; import { getDownstreamApiUrl } from '@src/generic/unlink-modal/data/api'; import { CourseAuthoringProvider } from '@src/CourseAuthoringContext'; +import { mockWaffleFlags } from '@src/data/apiHooks.mock'; import { act, fireEvent, @@ -552,6 +553,11 @@ const renderComponent = () => describe('', () => { beforeEach(async () => { const mocks = initializeMocks(); + mockWaffleFlags({ enableAuthzCourseAuthoring: false }); + mocks.validateUserPermissionsMock.mockResolvedValue({ + canEditCourseContent: true, + canPublishCourseContent: true, + }); selectedContainerId = undefined; // restore index mock — use reorder outline spec (section[0] has 2 subsections for configure/drag tests) courseOutlineIndexMock = buildTestOutline({ diff --git a/src/course-outline/CourseOutline.tsx b/src/course-outline/CourseOutline.tsx index 1bc151a171..6174941c6f 100644 --- a/src/course-outline/CourseOutline.tsx +++ b/src/course-outline/CourseOutline.tsx @@ -51,6 +51,8 @@ const CourseOutline = () => { const location = useLocation(); const { courseId, + canEditCourseContent, + isLoadingPermissions, } = useCourseAuthoringContext(); const { courseUsageKey, @@ -94,7 +96,14 @@ const CourseOutline = () => { const [showSuccessAlert, setShowSuccessAlert] = useState(false); const isInternetConnectionAlertFailed = savingStatus === RequestStatus.FAILED; - const isReIndexShow = Boolean(reindexLink); + const isReIndexShow = canEditCourseContent && Boolean(reindexLink); + + // The header's "+ Add" button creates course content, so gate its visibility behind the + // edit permission. This is scoped to the header actions and leaves the outline tree unaffected. + const headerCourseActions = useMemo( + () => ({ ...courseActions, childAddable: canEditCourseContent && courseActions.childAddable }), + [courseActions, canEditCourseContent], + ); const handleAddBlock = useCreateCourseBlock(courseId); const pasteMutation = usePasteItem(courseId); @@ -169,7 +178,7 @@ const CourseOutline = () => { } }, [location, courseId, courseName]); - if (isLoading) { + if (isLoading || isLoadingPermissions) { // eslint-disable-next-line react/jsx-no-useless-fragment return ( @@ -249,7 +258,7 @@ const CourseOutline = () => { headerNavigationsActions={headerNavigationsActions} isDisabledReindexButton={isDisabledReindexButton} hasSections={Boolean(sections.length)} - courseActions={courseActions} + courseActions={headerCourseActions} errors={errors} sections={sections} /> diff --git a/src/course-outline/OutlineNode.tsx b/src/course-outline/OutlineNode.tsx index 5881792589..625233e705 100644 --- a/src/course-outline/OutlineNode.tsx +++ b/src/course-outline/OutlineNode.tsx @@ -105,7 +105,7 @@ const OutlineNode = ({ const { activeId, overId } = useContext(DragContext); const { selectedContainerState, openContainerSidebar, setSelectedContainerState } = useOutlineSidebarContext(); - const { courseId, openUnlinkModal, getUnitUrl } = useCourseAuthoringContext(); + const { courseId, openUnlinkModal, getUnitUrl, canEditCourseContent } = useCourseAuthoringContext(); const duplicateMutation = useDuplicateItem(courseId); const { openPublishModal } = useCourseOutlineContext(); const queryClient = useQueryClient(); @@ -236,7 +236,7 @@ const OutlineNode = ({ else { onOrderChange(effectiveSection, getPossibleMoves!(index, 1)); } }; - const isDraggable = model.isDraggable(actions, isHeaderVisible); + const isDraggable = canEditCourseContent && model.isDraggable(actions, isHeaderVisible); const titleComponent = depth < 2 ? ( @@ -335,7 +335,6 @@ const OutlineNode = ({ {...(depth === 2 ? { isVertical: true, - enableCopyPasteUnits: blk.enableCopyPasteUnits ?? false, onClickCopy: () => copyToClipboard(blk.id), discussionEnabled: blk.discussionEnabled, discussionsSettings, @@ -352,7 +351,7 @@ const OutlineNode = ({ data-testid={levelConfig.contentTestId} onClick={(e) => onClickCard(e, false)} > - {depth === 0 && onOpenHighlightsModal && ( + {canEditCourseContent && depth === 0 && onOpenHighlightsModal && (
} - iconAs={EditIcon} - onClick={onEditClick} - disabled={editMutation.isPending} - /> + {canEditCourseContent && + ( + {intl.formatMessage(messages.altButtonRename)}} + iconAs={EditIcon} + onClick={onEditClick} + disabled={editMutation.isPending} + /> + )} )}
@@ -313,105 +314,110 @@ const CardHeader = ({ onClick={onClickSync} /> )} - - - - {isSequential && proctoringExamConfigurationLink && ( - - {intl.formatMessage(messages.menuProctoringLinkText)} - - )} - - {intl.formatMessage(messages.menuPublish)} - - - {intl.formatMessage(messages.menuConfigure)} - - {getConfig().ENABLE_TAGGING_TAXONOMY_PAGES === 'true' && ( + {canEditCourseContent && ( + + + + {isSequential && proctoringExamConfigurationLink && ( + + {intl.formatMessage(messages.menuProctoringLinkText)} + + )} + {canPublishCourseContent && + ( + + {intl.formatMessage(messages.menuPublish)} + + )} - {intl.formatMessage(messages.menuManageTags)} + {intl.formatMessage(messages.menuConfigure)} - )} + {getConfig().ENABLE_TAGGING_TAXONOMY_PAGES === 'true' && ( + + {intl.formatMessage(messages.menuManageTags)} + + )} - {isVertical && enableCopyPasteUnits && ( - - {intl.formatMessage(messages.menuCopy)} - - )} - {actions.duplicable && ( - - {intl.formatMessage(messages.menuDuplicate)} - - )} - {actions.draggable && ( - <> + {isVertical && ( + + {intl.formatMessage(messages.menuCopy)} + + )} + {actions.duplicable && ( - {intl.formatMessage(messages.menuMoveUp)} + {intl.formatMessage(messages.menuDuplicate)} + )} + {actions.draggable && ( + <> + + {intl.formatMessage(messages.menuMoveUp)} + + + {intl.formatMessage(messages.menuMoveDown)} + + + )} + {((actions.unlinkable ?? null) !== null || actions.deletable) && } + {(actions.unlinkable ?? null) !== null && ( - {intl.formatMessage(messages.menuMoveDown)} + {intl.formatMessage(messages.menuUnlink)} - - )} - {((actions.unlinkable ?? null) !== null || actions.deletable) && } - {(actions.unlinkable ?? null) !== null && ( - - {intl.formatMessage(messages.menuUnlink)} - - )} - {actions.deletable && ( - - {intl.formatMessage(messages.menuDelete)} - - )} - - + )} + {actions.deletable && ( + + + + )} + + + )}
diff --git a/src/course-outline/outline-sidebar/OutlineSidebarPagesContext.tsx b/src/course-outline/outline-sidebar/OutlineSidebarPagesContext.tsx index e95adfce4f..7f35350e83 100644 --- a/src/course-outline/outline-sidebar/OutlineSidebarPagesContext.tsx +++ b/src/course-outline/outline-sidebar/OutlineSidebarPagesContext.tsx @@ -14,25 +14,28 @@ import { OutlineAlignSidebar } from './OutlineAlignSidebar'; import OutlineHelpSidebar from './OutlineHelpSidebar'; import { InfoSidebar } from './info-sidebar/InfoSidebar'; import messages from './messages'; +import { useCourseAuthoringContext } from '@src/CourseAuthoringContext'; export type OutlineSidebarPages = { info: SidebarPage; help: SidebarPage; - add: SidebarPage; + add?: SidebarPage; align?: SidebarPage; }; -const getOutlineSidebarPages = () => ({ +const getOutlineSidebarPages = (canEditCourseContent: boolean = false) => ({ info: { component: InfoSidebar, icon: Info, title: messages.sidebarButtonInfo, }, - add: { - component: AddSidebar, - icon: Plus, - title: messages.sidebarButtonAdd, - }, + ...(canEditCourseContent && { + add: { + component: AddSidebar, + icon: Plus, + title: messages.sidebarButtonAdd, + }, + }), ...(getConfig().ENABLE_TAGGING_TAXONOMY_PAGES === 'true' && { align: { component: OutlineAlignSidebar, @@ -84,9 +87,11 @@ type OutlineSidebarPagesProviderProps = { }; export const OutlineSidebarPagesProvider = ({ children }: OutlineSidebarPagesProviderProps) => { + const { canEditCourseContent } = useCourseAuthoringContext(); + // align page is sometimes not added when getOutlineSidebarPages() is called at the top level. // So if we call it inside the hook, getConfig has updated values and align page is added. - const sidebarPages = useMemo(getOutlineSidebarPages, []); + const sidebarPages = useMemo(() => getOutlineSidebarPages(canEditCourseContent), [canEditCourseContent]); return ( diff --git a/src/course-outline/status-bar/StatusBar.test.tsx b/src/course-outline/status-bar/StatusBar.test.tsx index 597c20b7e7..21d2ea7175 100644 --- a/src/course-outline/status-bar/StatusBar.test.tsx +++ b/src/course-outline/status-bar/StatusBar.test.tsx @@ -1,8 +1,10 @@ import { VIDEO_SHARING_OPTIONS } from '@src/course-outline/constants'; import { CourseOutlineStatusBar } from '@src/course-outline/data/types'; import { initializeMocks, render, screen } from '@src/testUtils'; +import { mockWaffleFlags } from '@src/data/apiHooks.mock'; import { StatusBar, StatusBarProps } from './StatusBar'; import messages from './messages'; +import { CourseAuthoringProvider } from '@src/CourseAuthoringContext'; const courseId = 'course-v1:123'; const isLoading = false; @@ -36,6 +38,13 @@ jest.mock('@src/course-outline/data/apiHooks', () => ({ isLoading: false, }), })); + +let validateUserPermissionsMock; +const mockPermissions = (canEditCourseContent = true) => { + mockWaffleFlags({ enableAuthzCourseAuthoring: !canEditCourseContent }); + validateUserPermissionsMock.mockResolvedValue({ canEditCourseContent }); +}; + const mockOpenEnableHighlightsModal = jest.fn(); const mockHandleVideoSharingOptionChange = jest.fn(); @@ -49,13 +58,18 @@ const renderComponent = (props?: Partial) => handleVideoSharingOptionChange={mockHandleVideoSharingOptionChange} {...props} />, + { + extraWrapper: ({ children }) => {children}, + }, ); describe('', () => { beforeEach(() => { - initializeMocks(); + const mocks = initializeMocks(); + validateUserPermissionsMock = mocks.validateUserPermissionsMock; jest.useFakeTimers(); jest.setSystemTime(new Date('2013-03-05')); + mockPermissions(true); }); it('renders StatusBar component correctly', async () => { @@ -145,6 +159,29 @@ describe('', () => { expect(await screen.findByText(messages.highlightEmailsEnabled.defaultMessage)).toBeInTheDocument(); }); + it('does not render the enable highlights button when canEditCourseContent is false', async () => { + mockPermissions(false); + renderComponent(); + + // Wait for the status bar to render, then confirm the enable button is absent. + expect(await screen.findByText('Feb 05, 2013 - Apr 09, 2013')).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Enable highlights emails' })).not.toBeInTheDocument(); + }); + + it('still shows the enabled highlights message when canEditCourseContent is false', async () => { + mockPermissions(false); + renderComponent({ + statusBarData: { + ...statusBarData, + highlightsEnabledForMessaging: true, + }, + }); + + // The "enabled" message is informational (not an edit action), so it renders regardless of permission. + expect(await screen.findByText(messages.highlightEmailsEnabled.defaultMessage)).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Enable highlights emails' })).not.toBeInTheDocument(); + }); + it('does render video sharing dropdown if enabled', async () => { renderComponent({ statusBarData: { diff --git a/src/course-outline/status-bar/StatusBar.tsx b/src/course-outline/status-bar/StatusBar.tsx index 634292a2f1..cfed93b1dc 100644 --- a/src/course-outline/status-bar/StatusBar.tsx +++ b/src/course-outline/status-bar/StatusBar.tsx @@ -29,6 +29,7 @@ import { useHelpUrls } from '@src/help-urls/hooks'; import messages from './messages'; import { NotificationStatusIcon } from './NotificationStatusIcon'; +import { useCourseAuthoringContext } from '@src/CourseAuthoringContext'; const CourseBadge = ({ startDate, endDate }: { startDate: Moment; endDate: Moment; }) => { const now = moment().utc(); @@ -192,19 +193,23 @@ const Highlights = ({ highlightsEnabledForMessaging, openEnableHighlightsModal } }) => { const intl = useIntl(); + const { canEditCourseContent } = useCourseAuthoringContext(); + if (highlightsEnabledForMessaging) { return ( {intl.formatMessage(messages.highlightEmailsEnabled)} ); - } else { - return ( - - ); } + if (!canEditCourseContent) { + return null; + } + return ( + + ); }; const VideoSharingDropdown = ({ handleVideoSharingOptionChange, videoSharingOptions }: { diff --git a/src/generic/sidebar/Sidebar.tsx b/src/generic/sidebar/Sidebar.tsx index 35a83739a1..b4e292e916 100644 --- a/src/generic/sidebar/Sidebar.tsx +++ b/src/generic/sidebar/Sidebar.tsx @@ -88,12 +88,17 @@ export function Sidebar({ }: SidebarProps) { const intl = useIntl(); + // currentPageKey may point to a page that isn't currently available (e.g. one hidden by + // permissions or a stale value coming from the URL). Fall back to the first available page + // so the sidebar renders instead of crashing on an undefined page. + const [fallbackPageKey] = Object.keys(pages) as (keyof T)[]; + const effectivePageKey = pages[currentPageKey] ? currentPageKey : fallbackPageKey; const { component: SidebarComponent, icon: SidebarIcon, title, - } = pages[currentPageKey]; - const activeKey = isOpen ? currentPageKey : undefined; + } = pages[effectivePageKey]; + const activeKey = isOpen ? effectivePageKey : undefined; return ( diff --git a/src/studio-home/StudioHome.tsx b/src/studio-home/StudioHome.tsx index 6f99812abc..5d06648f34 100644 --- a/src/studio-home/StudioHome.tsx +++ b/src/studio-home/StudioHome.tsx @@ -1,4 +1,5 @@ import React, { useCallback } from 'react'; +import classNames from 'classnames'; import { Button, Container, @@ -85,7 +86,11 @@ const StudioHome = () => { if (canViewConsoleTeams) { headerButtons.push( -
+
} + alt={intl.formatMessage(commonMessages.editTooltip)} + onClick={isCertificateActive ? editModalOpen : handleEditAll} + /> + {intl.formatMessage(commonMessages.deleteTooltip)}
} + alt={intl.formatMessage(commonMessages.deleteTooltip)} + onClick={confirmOpen} + /> + + ) : + undefined} > diff --git a/src/certificates/certificate-signatories/CertificateSignatories.test.tsx b/src/certificates/certificate-signatories/CertificateSignatories.test.tsx index 6fab2b4a26..965769380b 100644 --- a/src/certificates/certificate-signatories/CertificateSignatories.test.tsx +++ b/src/certificates/certificate-signatories/CertificateSignatories.test.tsx @@ -1,4 +1,7 @@ import { initializeMocks, render, screen, within, userEvent } from '@src/testUtils'; +import { CourseAuthoringProvider } from '@src/CourseAuthoringContext'; +import { CertificatesProvider } from '@src/certificates/context'; +import { mockWaffleFlags } from '@src/data/apiHooks.mock'; import { signatoriesMock } from '../__mocks__'; import commonMessages from '../messages'; @@ -20,7 +23,11 @@ const mockArrayHelpers = { const renderComponent = (props) => render( - , + + + + + , ); const defaultProps = { @@ -38,6 +45,7 @@ const defaultProps = { describe('CertificateSignatories', () => { beforeEach(() => { initializeMocks(); + mockWaffleFlags({ enableAuthzCourseAuthoring: false }); mockUseEditSignatory.mockReturnValue({ toggleEditSignatory: jest.fn(), @@ -72,6 +80,7 @@ describe('CertificateSignatories', () => { describe('CertificateSignatories - real useEditSignatory', () => { beforeEach(() => { initializeMocks(); + mockWaffleFlags({ enableAuthzCourseAuthoring: false }); // Use the real implementation so handleDeleteSignatory actually calls arrayHelpers.remove const realUseEditSignatory = jest.requireActual('./hooks/useEditSignatory').default; diff --git a/src/certificates/certificate-signatories/signatory/Signatory.test.tsx b/src/certificates/certificate-signatories/signatory/Signatory.test.tsx index 9df5c3e4d4..1264f03aa4 100644 --- a/src/certificates/certificate-signatories/signatory/Signatory.test.tsx +++ b/src/certificates/certificate-signatories/signatory/Signatory.test.tsx @@ -1,4 +1,8 @@ import { initializeMocks, render, screen, userEvent } from '@src/testUtils'; +import { CourseAuthoringProvider } from '@src/CourseAuthoringContext'; +import { CertificatesProvider } from '@src/certificates/context'; +import { mockWaffleFlags } from '@src/data/apiHooks.mock'; +import { useCourseUserPermissions } from '@src/authz/hooks'; import { signatoriesMock } from '../../__mocks__'; import commonMessages from '../../messages'; @@ -7,9 +11,26 @@ import Signatory from './Signatory'; const mockHandleEdit = jest.fn(); +jest.mock('@src/authz/hooks', () => ({ + useCourseUserPermissions: jest.fn(), +})); + +const mockPermissions = (overrides = {}) => + jest.mocked(useCourseUserPermissions).mockReturnValue({ + isLoading: false, + isAuthzEnabled: true, + canViewCertificates: true, + canManageCertificates: true, + ...overrides, + } as ReturnType); + const renderSignatory = (props) => render( - , + + + + + , ); const defaultProps = { ...signatoriesMock[0], handleEdit: mockHandleEdit, index: 0 }; @@ -17,6 +38,8 @@ const defaultProps = { ...signatoriesMock[0], handleEdit: mockHandleEdit, index: describe('Signatory Component', () => { beforeEach(() => { initializeMocks(); + mockWaffleFlags({ enableAuthzCourseAuthoring: false }); + mockPermissions(); }); it('renders signatory data in view mode', () => { @@ -45,4 +68,12 @@ describe('Signatory Component', () => { expect(mockHandleEdit).toHaveBeenCalled(); }); + + it('hides edit button in view-only mode', () => { + mockPermissions({ canManageCertificates: false }); + renderSignatory(defaultProps); + + expect(screen.queryByRole('button', { name: commonMessages.editTooltip.defaultMessage })).not + .toBeInTheDocument(); + }); }); diff --git a/src/certificates/certificate-signatories/signatory/Signatory.tsx b/src/certificates/certificate-signatories/signatory/Signatory.tsx index 7a26dd7bed..c98e363b1b 100644 --- a/src/certificates/certificate-signatories/signatory/Signatory.tsx +++ b/src/certificates/certificate-signatories/signatory/Signatory.tsx @@ -12,6 +12,7 @@ import { getConfig } from '@edx/frontend-platform'; import commonMessages from '../../messages'; import messages from '../messages'; +import { useCertificatesContext } from '@src/certificates/context'; interface SignatoryProps { name: string; @@ -31,6 +32,7 @@ const Signatory = ({ handleEdit, }: SignatoryProps) => { const intl = useIntl(); + const { canManageCertificates } = useCertificatesContext(); return (
@@ -49,14 +51,16 @@ const Signatory = ({ - {intl.formatMessage(commonMessages.editTooltip)}
} - onClick={handleEdit} - /> + {canManageCertificates && ( + {intl.formatMessage(commonMessages.editTooltip)}} + onClick={handleEdit} + /> + )}
{signatureImagePath && ( void; savingIsSuccess: boolean; savingErrorMessage?: string; + canViewCertificates: boolean; + canManageCertificates: boolean; activationStatusMutation: UseMutationResult; updateCertificateMutation: UseMutationResult; deleteCertificateMutation: UseMutationResult; @@ -30,6 +34,12 @@ const CertificatesContext = createContext(u export const CertificatesProvider = ({ children }) => { const { courseId } = useCourseAuthoringContext(); const [componentMode, setComponentMode] = useState(MODE_STATES.noModes); + + const { + canViewCertificates, + canManageCertificates, + } = useCourseUserPermissions(courseId, getCertificatesPermissions(courseId)); + const activationStatusMutation = useUpdateCertificateActiveStatus(courseId); const updateCertificateMutation = useUpdateCertificate(courseId); const deleteCertificateMutation = useDeleteCertificate(courseId); @@ -49,6 +59,8 @@ export const CertificatesProvider = ({ children }) => { const contextValue = { componentMode, setComponentMode, + canViewCertificates, + canManageCertificates, activationStatusMutation, updateCertificateMutation, deleteCertificateMutation, @@ -61,6 +73,8 @@ export const CertificatesProvider = ({ children }) => { }, [ componentMode, setComponentMode, + canViewCertificates, + canManageCertificates, activationStatusMutation, updateCertificateMutation, ]); diff --git a/src/certificates/empty-certificates-with-modes/EmptyCertificatesWithModes.test.tsx b/src/certificates/empty-certificates-with-modes/EmptyCertificatesWithModes.test.tsx index e8a332b0f8..dd5f2b006a 100644 --- a/src/certificates/empty-certificates-with-modes/EmptyCertificatesWithModes.test.tsx +++ b/src/certificates/empty-certificates-with-modes/EmptyCertificatesWithModes.test.tsx @@ -5,6 +5,8 @@ import { userEvent, } from '@src/testUtils'; import { CertificatesProvider, useCertificatesContext } from '@src/certificates/context'; +import { mockWaffleFlags } from '@src/data/apiHooks.mock'; +import { useCourseUserPermissions } from '@src/authz/hooks'; import { MODE_STATES } from '../data/constants'; import messages from '../messages'; import EmptyCertificatesWithModes from './EmptyCertificatesWithModes'; @@ -15,6 +17,19 @@ const ComponentModeDisplay = () => { return
{componentMode}
; }; +jest.mock('@src/authz/hooks', () => ({ + useCourseUserPermissions: jest.fn(), +})); + +const mockPermissions = (overrides = {}) => + jest.mocked(useCourseUserPermissions).mockReturnValue({ + isLoading: false, + isAuthzEnabled: true, + canViewCertificates: true, + canManageCertificates: true, + ...overrides, + } as ReturnType); + const renderComponent = () => render( @@ -28,6 +43,8 @@ const renderComponent = () => describe('EmptyCertificatesWithModes', () => { beforeEach(() => { initializeMocks(); + mockWaffleFlags({ enableAuthzCourseAuthoring: false }); + mockPermissions(); }); it('renders correctly', () => { @@ -47,4 +64,13 @@ describe('EmptyCertificatesWithModes', () => { expect(screen.getByTestId('component-mode')).toHaveTextContent(MODE_STATES.create); }); + + it('hides add button in view-only mode', () => { + mockPermissions({ canManageCertificates: false }); + renderComponent(); + + expect(screen.getByText(messages.noCertificatesText.defaultMessage)).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: messages.setupCertificateBtn.defaultMessage })).not + .toBeInTheDocument(); + }); }); diff --git a/src/certificates/empty-certificates-with-modes/EmptyCertificatesWithModes.tsx b/src/certificates/empty-certificates-with-modes/EmptyCertificatesWithModes.tsx index e748ee25df..f238d542b7 100644 --- a/src/certificates/empty-certificates-with-modes/EmptyCertificatesWithModes.tsx +++ b/src/certificates/empty-certificates-with-modes/EmptyCertificatesWithModes.tsx @@ -8,7 +8,7 @@ import { useCertificatesContext } from '../context'; const EmptyCertificatesWithModes = () => { const intl = useIntl(); - const { setComponentMode } = useCertificatesContext(); + const { setComponentMode, canManageCertificates } = useCertificatesContext(); const handleCreateMode = () => { setComponentMode(MODE_STATES.create); }; @@ -19,12 +19,14 @@ const EmptyCertificatesWithModes = () => { {intl.formatMessage(messages.noCertificatesText)} - + {canManageCertificates && ( + + )} diff --git a/src/certificates/layout/MainLayout.tsx b/src/certificates/layout/MainLayout.tsx index a6c4563075..227fae49e9 100644 --- a/src/certificates/layout/MainLayout.tsx +++ b/src/certificates/layout/MainLayout.tsx @@ -4,6 +4,7 @@ import { useIntl } from '@edx/frontend-platform/i18n'; import { SavingErrorAlert } from '@src/generic/saving-error-alert'; import SubHeader from '@src/generic/sub-header/SubHeader'; +import ViewOnlyPermissionsAlert from '@src/generic/ViewOnlyPermissionsAlert'; import messages from '../messages'; import CertificatesSidebar from './certificates-sidebar/CertificatesSidebar'; import HeaderButtons from './header-buttons/HeaderButtons'; @@ -20,6 +21,7 @@ const MainLayout = ({ showHeaderButtons = false, children }: MainLayoutProps) => const { savingIsSuccess, savingErrorMessage, + canManageCertificates, } = useCertificatesContext(); useEffect(() => { @@ -39,6 +41,7 @@ const MainLayout = ({ showHeaderButtons = false, children }: MainLayoutProps) => headerActions={showHeaderButtons ? : null} + banner={!canManageCertificates ? : null} />
({ + useCourseUserPermissions: jest.fn(), +})); + +const mockPermissions = (overrides = {}) => + jest.mocked(useCourseUserPermissions).mockReturnValue({ + isLoading: false, + isAuthzEnabled: true, + canViewCertificates: true, + canManageCertificates: true, + ...overrides, + } as ReturnType); + const renderComponent = () => render( @@ -29,6 +44,8 @@ describe('HeaderButtons Component', () => { beforeEach(() => { const mocks = initializeMocks(); axiosMock = mocks.axiosMock; + mockWaffleFlags({ enableAuthzCourseAuthoring: false }); + mockPermissions(); axiosMock .onGet(getCertificatesApiUrl(courseId)) .reply(200, certificatesDataMock); @@ -106,4 +123,16 @@ describe('HeaderButtons Component', () => { expect(screen.queryByRole('button', { name: messages.headingActionsDeactivate.defaultMessage })).not .toBeInTheDocument(); }); + + it('hides activate button in view-only mode', async () => { + mockPermissions({ canManageCertificates: false }); + renderComponent(); + + await waitFor(() => screen.getByRole('link', { name: messages.headingActionsPreview.defaultMessage })); + + expect(screen.queryByRole('button', { name: messages.headingActionsActivate.defaultMessage })).not + .toBeInTheDocument(); + expect(screen.queryByRole('button', { name: messages.headingActionsDeactivate.defaultMessage })).not + .toBeInTheDocument(); + }); }); diff --git a/src/certificates/layout/header-buttons/HeaderButtons.tsx b/src/certificates/layout/header-buttons/HeaderButtons.tsx index 6dd15b15ad..2930486444 100644 --- a/src/certificates/layout/header-buttons/HeaderButtons.tsx +++ b/src/certificates/layout/header-buttons/HeaderButtons.tsx @@ -8,16 +8,18 @@ import { import messages from '../../messages'; import useHeaderButtons from './hooks/useHeaderButtons'; +import { useCertificatesContext } from '@src/certificates/context'; const HeaderButtons = () => { const intl = useIntl(); + const { canManageCertificates } = useCertificatesContext(); const { previewUrl, courseModes, dropdowmItem, - isCertificateActive, setDropdowmItem, handleActivationStatus, + isCertificateActive, } = useHeaderButtons(); return ( @@ -34,14 +36,16 @@ const HeaderButtons = () => { > {intl.formatMessage(messages.headingActionsPreview)} - + {canManageCertificates && ( + + )} ); }; diff --git a/src/course-outline/outline-sidebar/info-sidebar/CourseInfoSidebar.tsx b/src/course-outline/outline-sidebar/info-sidebar/CourseInfoSidebar.tsx index 9bcb1b3c3d..adb46b6195 100644 --- a/src/course-outline/outline-sidebar/info-sidebar/CourseInfoSidebar.tsx +++ b/src/course-outline/outline-sidebar/info-sidebar/CourseInfoSidebar.tsx @@ -141,7 +141,7 @@ const SettingsTab = () => { isNewPage /> )} - {perms.canManageAdvancedSettings && ( + {perms.canViewAdvancedSettings && ( ); }; @@ -91,7 +91,7 @@ describe('HelpSidebar', () => { ['canViewScheduleAndDetails', messages.sidebarLinkToScheduleAndDetails], ['canViewGradingSettings', messages.sidebarLinkToGrading], ['canManageGroupConfigurations', messages.sidebarLinkToGroupConfigurations], - ['canManageAdvancedSettings', messages.sidebarLinkToAdvancedSettings], + ['canViewAdvancedSettings', messages.sidebarLinkToAdvancedSettings], ])('renders the %s link only when the permission is granted', async (permission, message) => { mockCoursePermissions({ [permission]: true }); const { queryByText, unmount } = renderHelpSidebar(props); @@ -130,7 +130,7 @@ describe('HelpSidebar', () => { canViewGradingSettings: false, canViewCourseTeam: false, canManageGroupConfigurations: false, - canManageAdvancedSettings: false, + canViewAdvancedSettings: false, }, { isLoading: true }); const { queryByText } = renderHelpSidebar(props); diff --git a/src/generic/help-sidebar/HelpSidebar.tsx b/src/generic/help-sidebar/HelpSidebar.tsx index 99c019c1eb..480a955546 100644 --- a/src/generic/help-sidebar/HelpSidebar.tsx +++ b/src/generic/help-sidebar/HelpSidebar.tsx @@ -109,7 +109,7 @@ const HelpSidebar = ({ isNewPage /> )} - {showOtherLink(advancedSettings) && perms.canManageAdvancedSettings && ( + {showOtherLink(advancedSettings) && perms.canViewAdvancedSettings && ( { jest.mocked(useCourseUserPermissions).mockReturnValue({ isLoading: false, isAuthzEnabled: false, - canManageAdvancedSettings: true, + canViewAdvancedSettings: true, canViewGradingSettings: true, canViewScheduleAndDetails: true, canViewCourseTeam: true, canManageGroupConfigurations: true, - canManageCertificates: true, + canViewCertificates: true, } as ReturnType); }); @@ -314,7 +314,7 @@ describe('header utils', () => { jest.mocked(useCourseUserPermissions).mockReturnValue({ isLoading: false, isAuthzEnabled: true, - canManageAdvancedSettings: true, + canViewAdvancedSettings: true, canViewGradingSettings: true, canViewScheduleAndDetails: true, } as ReturnType); @@ -329,7 +329,7 @@ describe('header utils', () => { mockWaffleFlags({ enableAuthzCourseAuthoring: false }); jest.mocked(useCourseUserPermissions).mockReturnValue({ isLoading: false, - canManageAdvancedSettings: true, + canViewAdvancedSettings: true, canViewCourseTeam: true, } as any); const actualItems = @@ -352,7 +352,7 @@ describe('header utils', () => { jest.mocked(useCourseUserPermissions).mockReturnValue({ isLoading: false, isAuthzEnabled: true, - canManageAdvancedSettings: true, + canViewAdvancedSettings: true, canViewCourseTeam: true, } as any); const courseIdWithSpecialChars = 'course-v1:org+course+run'; @@ -369,7 +369,7 @@ describe('header utils', () => { jest.mocked(useCourseUserPermissions).mockReturnValue({ isLoading: false, isAuthzEnabled: true, - canManageAdvancedSettings: false, + canViewAdvancedSettings: false, canViewGradingSettings: true, canViewScheduleAndDetails: true, } as ReturnType); @@ -399,7 +399,7 @@ describe('header utils', () => { isLoading: false, isAuthzEnabled: true, canViewScheduleAndDetails: true, - canManageAdvancedSettings: true, + canViewAdvancedSettings: true, canViewGradingSettings: true, } as ReturnType); const { result } = renderHook(() => useSettingMenuItems('course-123'), { wrapper: createWrapper() }); @@ -415,7 +415,7 @@ describe('header utils', () => { isLoading: false, isAuthzEnabled: true, canViewScheduleAndDetails: false, - canManageAdvancedSettings: true, + canViewAdvancedSettings: true, canViewGradingSettings: true, } as ReturnType); const { result } = renderHook(() => useSettingMenuItems('course-123'), { wrapper: createWrapper() }); @@ -431,7 +431,7 @@ describe('header utils', () => { isLoading: false, isAuthzEnabled: true, canViewGradingSettings: true, - canManageAdvancedSettings: true, + canViewAdvancedSettings: true, canViewScheduleAndDetails: true, } as ReturnType); const { result } = renderHook(() => useSettingMenuItems('course-123'), { wrapper: createWrapper() }); @@ -447,7 +447,7 @@ describe('header utils', () => { isLoading: false, isAuthzEnabled: true, canViewGradingSettings: false, - canManageAdvancedSettings: true, + canViewAdvancedSettings: true, canViewScheduleAndDetails: true, } as ReturnType); const { result } = renderHook(() => useSettingMenuItems('course-123'), { wrapper: createWrapper() }); @@ -462,7 +462,7 @@ describe('header utils', () => { jest.mocked(useCourseUserPermissions).mockReturnValue({ isLoading: false, isAuthzEnabled: true, - canManageAdvancedSettings: true, + canViewAdvancedSettings: true, canViewGradingSettings: true, canViewScheduleAndDetails: true, canViewCourseTeam: true, @@ -504,7 +504,7 @@ describe('header utils', () => { expect(actualItemsTitle).not.toContain('Group Configurations'); }); - it('when authz flag is enabled and user lacks canManageCertificates should not include certificates option', () => { + it('when authz flag is enabled and user lacks canViewCertificates should not include certificates option', () => { mockWaffleFlags({ enableAuthzCourseAuthoring: true }); setConfig({ ...getConfig(), @@ -513,7 +513,7 @@ describe('header utils', () => { jest.mocked(useCourseUserPermissions).mockReturnValue({ isLoading: false, isAuthzEnabled: true, - canManageCertificates: false, + canViewCertificates: false, } as any); const actualItemsTitle = renderHook(() => useSettingMenuItems('course-123'), { wrapper: createWrapper() }).result .current.map((item) => item.title); diff --git a/src/header/hooks.tsx b/src/header/hooks.tsx index 71022dd223..65bee3e53c 100644 --- a/src/header/hooks.tsx +++ b/src/header/hooks.tsx @@ -96,7 +96,7 @@ export const useSettingMenuItems = (courseId: string) => { // legacy value: it would briefly show the link (and the Settings dropdown) to users // that authz then denies. const canAccessAdvancedSettings = perms.isAuthzEnabled - ? perms.canManageAdvancedSettings + ? perms.canViewAdvancedSettings : !perms.isLoading && legacyCanAccessAdvancedSettings; const items = [ @@ -138,7 +138,7 @@ export const useSettingMenuItems = (courseId: string) => { }] : []), ]; - if (getConfig().ENABLE_CERTIFICATE_PAGE === 'true' && perms.canManageCertificates) { + if (getConfig().ENABLE_CERTIFICATE_PAGE === 'true' && perms.canViewCertificates) { items.push({ href: `/course/${courseId}/certificates`, title: intl.formatMessage(messages['header.links.certificates']), From 91d76aa03f96ff513ad3781d966a3e1603626dac Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 2 Sep 2026 17:57:13 -0700 Subject: [PATCH 05/16] chore(deps): bump fast-uri from 3.1.5 to 3.1.7 (#3228) Bumps [fast-uri](https://github.com/fastify/fast-uri) from 3.1.5 to 3.1.7. - [Release notes](https://github.com/fastify/fast-uri/releases) - [Commits](https://github.com/fastify/fast-uri/compare/v3.1.5...v3.1.7) --- updated-dependencies: - dependency-name: fast-uri dependency-version: 3.1.7 dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 19cea7accb..2ab2e802fe 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13078,9 +13078,9 @@ "license": "MIT" }, "node_modules/fast-uri": { - "version": "3.1.5", - "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.5.tgz", - "integrity": "sha512-gHwA1O9LDIcKunMKhObS/HimwtehO1nPUECKAu5TpKgaO19fcWEl4bliWe1jWxVFvIXztJjjQ4L8XQ1EU9f7Jw==", + "version": "3.1.7", + "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.7.tgz", + "integrity": "sha512-dOvZVzjdZdz7phd9v6jCbwxrBW3fK6n8Rc0CtdmM4bumzMnxywBYhuph6J819RRw/ku+rLbelwfMunktuzVVHg==", "funding": [ { "type": "github", From c12c9a51ca7f05e24d979c4a7bc34cb31040d871 Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Sat, 5 Sep 2026 11:20:18 +1000 Subject: [PATCH 06/16] feat(authz): add group configuration permission validation (#3207) * feat: add view permission and update access for group config * refactor: hide buttons under manage configuration * refactor: convert group configuration files modified to typescript * refactor(i18n): use declarative API * test: update testing conventions * refactor: remove sidebar intruction in read-only --- src/authz/constants.ts | 1 + src/authz/permissionHelpers.test.ts | 6 +- src/authz/permissionHelpers.ts | 4 + .../info-sidebar/CourseInfoSidebar.tsx | 2 +- src/generic/help-sidebar/HelpSidebar.test.tsx | 6 +- src/generic/help-sidebar/HelpSidebar.tsx | 2 +- .../GroupConfigurations.test.tsx | 24 ++- .../ContentGroupCard.test.jsx | 99 ----------- .../ContentGroupCard.test.tsx | 149 ++++++++++++++++ ...tentGroupCard.jsx => ContentGroupCard.tsx} | 111 ++++-------- .../ContentGroupsSection.test.jsx | 67 ------- .../ContentGroupsSection.test.tsx | 165 ++++++++++++++++++ .../{index.jsx => index.tsx} | 39 +++-- .../EmptyPlaceholder.test.jsx | 23 --- .../EmptyPlaceholder.test.tsx | 37 ++++ .../empty-placeholder/index.jsx | 42 ----- .../empty-placeholder/index.tsx | 47 +++++ .../empty-placeholder/messages.ts | 5 + ...tCard.test.jsx => ExperimentCard.test.tsx} | 46 +++-- ...{ExperimentCard.jsx => ExperimentCard.tsx} | 135 +++++--------- .../ExperimentConfigurationsSection.test.jsx | 54 ------ .../ExperimentConfigurationsSection.test.tsx | 109 ++++++++++++ .../{index.jsx => index.tsx} | 72 +++----- .../GroupConfigurationSidebar.test.jsx | 78 --------- .../GroupConfigurationSidebar.test.tsx | 102 +++++++++++ .../{index.jsx => index.tsx} | 33 ++-- .../group-configuration-sidebar/utils.jsx | 61 ------- .../group-configuration-sidebar/utils.tsx | 96 ++++++++++ src/group-configurations/index.tsx | 7 +- src/group-configurations/types.ts | 38 +++- src/header/hooks.test.tsx | 6 +- src/header/hooks.tsx | 2 +- 32 files changed, 962 insertions(+), 706 deletions(-) delete mode 100644 src/group-configurations/content-groups-section/ContentGroupCard.test.jsx create mode 100644 src/group-configurations/content-groups-section/ContentGroupCard.test.tsx rename src/group-configurations/content-groups-section/{ContentGroupCard.jsx => ContentGroupCard.tsx} (62%) delete mode 100644 src/group-configurations/content-groups-section/ContentGroupsSection.test.jsx create mode 100644 src/group-configurations/content-groups-section/ContentGroupsSection.test.tsx rename src/group-configurations/content-groups-section/{index.jsx => index.tsx} (75%) delete mode 100644 src/group-configurations/empty-placeholder/EmptyPlaceholder.test.jsx create mode 100644 src/group-configurations/empty-placeholder/EmptyPlaceholder.test.tsx delete mode 100644 src/group-configurations/empty-placeholder/index.jsx create mode 100644 src/group-configurations/empty-placeholder/index.tsx rename src/group-configurations/experiment-configurations-section/{ExperimentCard.test.jsx => ExperimentCard.test.tsx} (74%) rename src/group-configurations/experiment-configurations-section/{ExperimentCard.jsx => ExperimentCard.tsx} (57%) delete mode 100644 src/group-configurations/experiment-configurations-section/ExperimentConfigurationsSection.test.jsx create mode 100644 src/group-configurations/experiment-configurations-section/ExperimentConfigurationsSection.test.tsx rename src/group-configurations/experiment-configurations-section/{index.jsx => index.tsx} (54%) delete mode 100644 src/group-configurations/group-configuration-sidebar/GroupConfigurationSidebar.test.jsx create mode 100644 src/group-configurations/group-configuration-sidebar/GroupConfigurationSidebar.test.tsx rename src/group-configurations/group-configuration-sidebar/{index.jsx => index.tsx} (69%) delete mode 100644 src/group-configurations/group-configuration-sidebar/utils.jsx create mode 100644 src/group-configurations/group-configuration-sidebar/utils.tsx diff --git a/src/authz/constants.ts b/src/authz/constants.ts index 90fd228e54..f1b0db0b80 100644 --- a/src/authz/constants.ts +++ b/src/authz/constants.ts @@ -44,6 +44,7 @@ export const COURSE_PERMISSIONS = { VIEW_COURSE_TEAM: 'courses.view_course_team', + VIEW_GROUP_CONFIGURATIONS: 'courses.view_group_configurations', MANAGE_GROUP_CONFIGURATIONS: 'courses.manage_group_configurations', VIEW_CERTIFICATES: 'courses.view_certificates', diff --git a/src/authz/permissionHelpers.test.ts b/src/authz/permissionHelpers.test.ts index ce1157d166..45b00e370f 100644 --- a/src/authz/permissionHelpers.test.ts +++ b/src/authz/permissionHelpers.test.ts @@ -208,10 +208,14 @@ describe('permissionHelpers', () => { }); describe('getGroupConfigurationsPermissions', () => { - it('returns MANAGE_GROUP_CONFIGURATIONS permission with the correct action and scope', () => { + it('returns VIEW_GROUP_CONFIGURATIONS and MANAGE_GROUP_CONFIGURATIONS permissions with the correct actions and scope', () => { const result = getGroupConfigurationsPermissions(courseId); expect(result).toEqual({ + canViewGroupConfigurations: { + action: COURSE_PERMISSIONS.VIEW_GROUP_CONFIGURATIONS, + scope: courseId, + }, canManageGroupConfigurations: { action: COURSE_PERMISSIONS.MANAGE_GROUP_CONFIGURATIONS, scope: courseId, diff --git a/src/authz/permissionHelpers.ts b/src/authz/permissionHelpers.ts index 10b36a6baf..1a1cb1878a 100644 --- a/src/authz/permissionHelpers.ts +++ b/src/authz/permissionHelpers.ts @@ -100,6 +100,10 @@ export const getCourseTeamPermissions = (courseId: string) => ({ }); export const getGroupConfigurationsPermissions = (courseId: string) => ({ + canViewGroupConfigurations: { + action: COURSE_PERMISSIONS.VIEW_GROUP_CONFIGURATIONS, + scope: courseId, + }, canManageGroupConfigurations: { action: COURSE_PERMISSIONS.MANAGE_GROUP_CONFIGURATIONS, scope: courseId, diff --git a/src/course-outline/outline-sidebar/info-sidebar/CourseInfoSidebar.tsx b/src/course-outline/outline-sidebar/info-sidebar/CourseInfoSidebar.tsx index adb46b6195..a5557c6cc4 100644 --- a/src/course-outline/outline-sidebar/info-sidebar/CourseInfoSidebar.tsx +++ b/src/course-outline/outline-sidebar/info-sidebar/CourseInfoSidebar.tsx @@ -133,7 +133,7 @@ const SettingsTab = () => { /> ) )} - {perms.canManageGroupConfigurations && ( + {perms.canViewGroupConfigurations && ( ); @@ -90,7 +90,7 @@ describe('HelpSidebar', () => { it.each([ ['canViewScheduleAndDetails', messages.sidebarLinkToScheduleAndDetails], ['canViewGradingSettings', messages.sidebarLinkToGrading], - ['canManageGroupConfigurations', messages.sidebarLinkToGroupConfigurations], + ['canViewGroupConfigurations', messages.sidebarLinkToGroupConfigurations], ['canViewAdvancedSettings', messages.sidebarLinkToAdvancedSettings], ])('renders the %s link only when the permission is granted', async (permission, message) => { mockCoursePermissions({ [permission]: true }); @@ -129,7 +129,7 @@ describe('HelpSidebar', () => { canViewScheduleAndDetails: false, canViewGradingSettings: false, canViewCourseTeam: false, - canManageGroupConfigurations: false, + canViewGroupConfigurations: false, canViewAdvancedSettings: false, }, { isLoading: true }); const { queryByText } = renderHelpSidebar(props); diff --git a/src/generic/help-sidebar/HelpSidebar.tsx b/src/generic/help-sidebar/HelpSidebar.tsx index 480a955546..1ca15dce4b 100644 --- a/src/generic/help-sidebar/HelpSidebar.tsx +++ b/src/generic/help-sidebar/HelpSidebar.tsx @@ -100,7 +100,7 @@ const HelpSidebar = ({ /> ) )} - {showOtherLink(groupConfigurations) && perms.canManageGroupConfigurations && ( + {showOtherLink(groupConfigurations) && perms.canViewGroupConfigurations && ( jest.mocked(useCourseUserPermissions).mockReturnValue({ isLoading: false, isAuthzEnabled: true, + canViewGroupConfigurations: true, canManageGroupConfigurations: true, ...overrides, } as ReturnType); @@ -48,8 +49,8 @@ describe('', () => { mockPermissions(); }); - it('shows PermissionDeniedAlert when user lacks manage group configurations permission', async () => { - mockPermissions({ canManageGroupConfigurations: false }); + it('shows PermissionDeniedAlert when user lacks view group configurations permission', async () => { + mockPermissions({ canViewGroupConfigurations: false, canManageGroupConfigurations: false }); renderComponent(); expect(await screen.findByTestId('permissionDeniedAlert')).toBeInTheDocument(); expect(screen.queryByText(messages.headingSubtitle.defaultMessage)).not.toBeInTheDocument(); @@ -79,6 +80,25 @@ describe('', () => { expect(screen.getByText(teamGroups.name)).toBeInTheDocument(); }); + it('hides every action button when the user can view but not manage group configurations', async () => { + mockPermissions({ canManageGroupConfigurations: false }); + renderComponent(); + + const mainContent = await screen.findByTestId('group-configurations-main-content-wrapper'); + + expect(within(mainContent).getByText(contentGroups.name)).toBeInTheDocument(); + expect( + within(mainContent).queryByText(contentGroupsMessages.addNewGroup.defaultMessage), + ).not.toBeInTheDocument(); + expect( + within(mainContent).queryByText(experimentMessages.addNewGroup.defaultMessage), + ).not.toBeInTheDocument(); + expect(within(mainContent).queryAllByTestId('content-group-card-header-edit')).toHaveLength(0); + expect(within(mainContent).queryAllByTestId('content-group-card-header-delete')).toHaveLength(0); + expect(within(mainContent).queryAllByTestId('configuration-card-header-edit')).toHaveLength(0); + expect(within(mainContent).queryAllByTestId('configuration-card-header-delete')).toHaveLength(0); + }); + it('does not render an empty section for enrollment track groups if it is empty', async () => { const shouldNotShowEnrollmentTrackResponse = { ...groupConfigurationResponseMock, diff --git a/src/group-configurations/content-groups-section/ContentGroupCard.test.jsx b/src/group-configurations/content-groups-section/ContentGroupCard.test.jsx deleted file mode 100644 index a576cf9855..0000000000 --- a/src/group-configurations/content-groups-section/ContentGroupCard.test.jsx +++ /dev/null @@ -1,99 +0,0 @@ -import { render } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; -import { IntlProvider } from '@edx/frontend-platform/i18n'; - -import { contentGroupsMock } from '../__mocks__'; -import commonMessages from '../common/messages'; -import rootMessages from '../messages'; -import ContentGroupCard from './ContentGroupCard'; - -const handleCreateMock = jest.fn(); -const handleDeleteMock = jest.fn(); -const handleEditMock = jest.fn(); -const contentGroupActions = { - handleCreate: handleCreateMock, - handleDelete: handleDeleteMock, - handleEdit: handleEditMock, -}; - -const handleEditGroupMock = jest.fn(); -const contentGroup = contentGroupsMock.groups[0]; -const contentGroupWithUsages = contentGroupsMock.groups[1]; -const contentGroupWithOnlyOneUsage = contentGroupsMock.groups[2]; - -const renderComponent = (props = {}) => - render( - - group.name)} - parentGroupId={contentGroupsMock.id} - contentGroupActions={contentGroupActions} - handleEditGroup={handleEditGroupMock} - {...props} - /> - , - ); - -describe('', () => { - it('renders component correctly', () => { - const { getByText, getByTestId } = renderComponent(); - expect(getByText(contentGroup.name)).toBeInTheDocument(); - expect( - getByText( - commonMessages.titleId.defaultMessage.replace('{id}', contentGroup.id), - ), - ).toBeInTheDocument(); - expect(getByText(rootMessages.notInUse.defaultMessage)).toBeInTheDocument(); - expect(getByTestId('content-group-card-header-edit')).toBeInTheDocument(); - expect(getByTestId('content-group-card-header-delete')).toBeInTheDocument(); - }); - - it('expands/collapses the container group content on title click', async () => { - const user = userEvent.setup(); - const { - getByText, - queryByTestId, - getByTestId, - queryByText, - } = renderComponent(); - const cardTitle = getByTestId('configuration-card-header-button'); - await user.click(cardTitle); - expect(queryByTestId('content-group-card-content')).toBeInTheDocument(); - expect( - queryByText(rootMessages.notInUse.defaultMessage), - ).not.toBeInTheDocument(); - - await user.click(cardTitle); - expect(queryByTestId('content-group-card-content')).not.toBeInTheDocument(); - expect(getByText(rootMessages.notInUse.defaultMessage)).toBeInTheDocument(); - }); - - it('renders content group badge with used only one location', () => { - const { queryByTestId } = renderComponent({ - group: contentGroupWithOnlyOneUsage, - }); - const usageBlock = queryByTestId('configuration-card-header-button-usage'); - expect(usageBlock).toBeInTheDocument(); - }); - - it('renders content group badge with used locations', () => { - const { queryByTestId } = renderComponent({ - group: contentGroupWithUsages, - }); - const usageBlock = queryByTestId('configuration-card-header-button-usage'); - expect(usageBlock).toBeInTheDocument(); - }); - - it('renders group controls without access to units', async () => { - const user = userEvent.setup(); - const { queryByText, getByTestId } = renderComponent(); - expect( - queryByText(commonMessages.accessTo.defaultMessage), - ).not.toBeInTheDocument(); - - const cardTitle = getByTestId('configuration-card-header-button'); - await user.click(cardTitle); - expect(getByTestId('configuration-card-usage-empty')).toBeInTheDocument(); - }); -}); diff --git a/src/group-configurations/content-groups-section/ContentGroupCard.test.tsx b/src/group-configurations/content-groups-section/ContentGroupCard.test.tsx new file mode 100644 index 0000000000..330d7ae8a0 --- /dev/null +++ b/src/group-configurations/content-groups-section/ContentGroupCard.test.tsx @@ -0,0 +1,149 @@ +import { + initializeMocks, + render, + screen, + userEvent, + waitFor, +} from '@src/testUtils'; +import deleteModalMessages from '@src/generic/delete-modal/messages'; +import { Group } from '@src/group-configurations/types'; +import { contentGroupsMock } from '../__mocks__'; +import commonMessages from '../common/messages'; +import rootMessages from '../messages'; +import messages from './messages'; +import ContentGroupCard from './ContentGroupCard'; + +const courseId = 'course-v1:org+101+101'; +const handleCreateMock = jest.fn(); +const handleDeleteMock = jest.fn(); +const handleEditMock = jest.fn(); +const contentGroupActions = { + handleCreate: handleCreateMock, + handleDelete: handleDeleteMock, + handleEdit: handleEditMock, +}; + +const handleEditGroupMock = jest.fn(); +const contentGroup = contentGroupsMock.groups[0] as Group; +const contentGroupWithUsages = contentGroupsMock.groups[1] as Group; +const contentGroupWithOnlyOneUsage = contentGroupsMock.groups[2] as Group; + +const renderComponent = (props = {}) => + render( + group.name)} + parentGroupId={contentGroupsMock.id} + contentGroupActions={contentGroupActions} + handleEditGroup={handleEditGroupMock} + {...props} + />, + { path: '/course/:courseId/group_configurations', params: { courseId } }, + ); + +/** The card title is a button whose accessible name includes the group name and its id. */ +const getCardTitle = (group: Group) => screen.getByRole('button', { name: new RegExp(group.name) }); + +describe('', () => { + beforeEach(() => { + initializeMocks(); + }); + + it('renders component correctly', () => { + renderComponent(); + + expect(screen.getByText(contentGroup.name)).toBeInTheDocument(); + expect( + screen.getByText( + commonMessages.titleId.defaultMessage.replace('{id}', String(contentGroup.id)), + ), + ).toBeInTheDocument(); + expect(screen.getByText(rootMessages.notInUse.defaultMessage)).toBeInTheDocument(); + expect(screen.getByRole('button', { name: messages.actionEdit.defaultMessage })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: messages.actionDelete.defaultMessage })).toBeInTheDocument(); + }); + + it('expands/collapses the container group content on title click', async () => { + const user = userEvent.setup(); + renderComponent(); + const cardTitle = getCardTitle(contentGroup); + + await user.click(cardTitle); + expect(screen.queryByText(rootMessages.notInUse.defaultMessage)).not.toBeInTheDocument(); + expect(screen.getByRole('link', { name: messages.courseOutline.defaultMessage })).toBeInTheDocument(); + + await user.click(cardTitle); + expect(screen.getByText(rootMessages.notInUse.defaultMessage)).toBeInTheDocument(); + }); + + it('renders content group badge with used only one location', () => { + renderComponent({ group: contentGroupWithOnlyOneUsage }); + + expect(screen.getByText('Used in 1 location')).toBeInTheDocument(); + }); + + it('renders content group badge with used locations', () => { + renderComponent({ group: contentGroupWithUsages }); + + expect( + screen.getByText(`Used in ${contentGroupWithUsages.usage?.length} locations`), + ).toBeInTheDocument(); + }); + + it('renders group controls without access to units', async () => { + const user = userEvent.setup(); + renderComponent(); + + expect(screen.queryByText(commonMessages.accessTo.defaultMessage)).not.toBeInTheDocument(); + + await user.click(getCardTitle(contentGroup)); + expect(screen.getByRole('link', { name: messages.courseOutline.defaultMessage })).toBeInTheDocument(); + }); + + it('deletes the group with its parent id on delete confirmation', async () => { + const user = userEvent.setup(); + renderComponent(); + + await user.click(screen.getByRole('button', { name: messages.actionDelete.defaultMessage })); + await user.click( + screen.getByRole('button', { name: deleteModalMessages.deleteButton.defaultMessage }), + ); + + expect(handleDeleteMock).toHaveBeenCalledWith(contentGroupsMock.id, contentGroup.id); + }); + + it('does not delete when the card has no parent group id', async () => { + const user = userEvent.setup(); + renderComponent({ parentGroupId: undefined }); + + await user.click(screen.getByRole('button', { name: messages.actionDelete.defaultMessage })); + await user.click( + screen.getByRole('button', { name: deleteModalMessages.deleteButton.defaultMessage }), + ); + + expect(handleDeleteMock).not.toHaveBeenCalled(); + }); + + it('edits the group with its id on save', async () => { + const user = userEvent.setup(); + renderComponent(); + + await user.click(screen.getByRole('button', { name: messages.actionEdit.defaultMessage })); + const nameInput = screen.getByPlaceholderText( + messages.newGroupInputPlaceholder.defaultMessage, + ); + await user.clear(nameInput); + await user.type(nameInput, 'Renamed group'); + await user.click( + screen.getByRole('button', { name: messages.saveButton.defaultMessage }), + ); + + await waitFor(() => { + expect(handleEditGroupMock).toHaveBeenCalledWith( + contentGroup.id, + expect.objectContaining({ newGroupName: 'Renamed group' }), + expect.any(Function), + ); + }); + }); +}); diff --git a/src/group-configurations/content-groups-section/ContentGroupCard.jsx b/src/group-configurations/content-groups-section/ContentGroupCard.tsx similarity index 62% rename from src/group-configurations/content-groups-section/ContentGroupCard.jsx rename to src/group-configurations/content-groups-section/ContentGroupCard.tsx index 2121d3e993..4b44f14d0a 100644 --- a/src/group-configurations/content-groups-section/ContentGroupCard.jsx +++ b/src/group-configurations/content-groups-section/ContentGroupCard.tsx @@ -1,8 +1,7 @@ import { useState } from 'react'; -import PropTypes from 'prop-types'; import { useParams } from 'react-router-dom'; import { getConfig } from '@edx/frontend-platform'; -import { useIntl } from '@edx/frontend-platform/i18n'; +import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; import { ActionRow, Hyperlink, @@ -15,27 +14,41 @@ import { EditOutline as EditOutlineIcon, } from '@openedx/paragon/icons'; -import DeleteModal from '../../generic/delete-modal/DeleteModal'; +import DeleteModal from '@src/generic/delete-modal/DeleteModal'; +import { ContentGroupFormValues, Group, GroupActions } from '@src/group-configurations/types'; import TitleButton from '../common/TitleButton'; import UsageList from '../common/UsageList'; import ContentGroupForm from './ContentGroupForm'; import messages from './messages'; +interface ContentGroupCardProps { + group: Group; + groupNames?: string[]; + parentGroupId?: number; + readOnly?: boolean; + contentGroupActions?: Partial; + handleEditGroup?: ( + id: number, + values: ContentGroupFormValues, + callbackToClose: () => void, + ) => void; +} + const ContentGroupCard = ({ group, - groupNames, + groupNames = [], parentGroupId, - readOnly, - contentGroupActions, + readOnly = false, + contentGroupActions = {}, handleEditGroup, -}) => { +}: ContentGroupCardProps) => { const { formatMessage } = useIntl(); const { courseId } = useParams(); const [isExpanded, setIsExpanded] = useState(false); const [isEditMode, switchOnEditMode, switchOffEditMode] = useToggle(false); const [isOpenDeleteModal, openDeleteModal, closeDeleteModal] = useToggle(false); const { id, name, usage } = group; - const isUsedInLocation = !!usage.length; + const isUsedInLocation = !!usage?.length; const { href: outlineUrl } = new URL( `/course/${courseId}`, @@ -44,7 +57,7 @@ const ContentGroupCard = ({ const outlineComponentLink = ( - {formatMessage(messages.courseOutline)} + ); @@ -53,7 +66,7 @@ const ContentGroupCard = ({ className="small text-gray-700" data-testid="configuration-card-usage-empty" > - {formatMessage(messages.emptyContentGroups, { outlineComponentLink })} + ); @@ -62,7 +75,9 @@ const ContentGroupCard = ({ }; const handleDeleteGroup = () => { - contentGroupActions.handleDelete(parentGroupId, id); + if (parentGroupId !== undefined) { + contentGroupActions.handleDelete?.(parentGroupId, id); + } closeDeleteModal(); }; @@ -76,7 +91,7 @@ const ContentGroupCard = ({ isUsedInLocation={isUsedInLocation} overrideValue={name} onCancelClick={switchOffEditMode} - onEditClick={(values) => handleEditGroup(id, values, switchOffEditMode)} + onEditClick={(values: ContentGroupFormValues) => handleEditGroup?.(id, values, switchOffEditMode)} /> ) : ( @@ -90,7 +105,7 @@ const ContentGroupCard = ({ {!readOnly && ( } alt={formatMessage(messages.actionEdit)} src={EditOutlineIcon} iconAs={Icon} @@ -99,11 +114,13 @@ const ContentGroupCard = ({ /> + } alt={formatMessage(messages.actionDelete)} src={DeleteOutlineIcon} iconAs={Icon} @@ -124,7 +141,6 @@ const ContentGroupCard = ({ ) : guideHowToAdd} @@ -142,61 +158,4 @@ const ContentGroupCard = ({ ); }; -ContentGroupCard.defaultProps = { - group: { - id: undefined, - name: '', - usage: [], - version: undefined, - }, - readOnly: false, - groupNames: [], - parentGroupId: null, - handleEditGroup: null, - contentGroupActions: {}, -}; - -ContentGroupCard.propTypes = { - group: PropTypes.shape({ - id: PropTypes.number.isRequired, - name: PropTypes.string.isRequired, - usage: PropTypes.arrayOf( - PropTypes.shape({ - label: PropTypes.string, - url: PropTypes.string, - }), - ), - version: PropTypes.number.isRequired, - active: PropTypes.bool, - description: PropTypes.string, - groups: PropTypes.arrayOf( - PropTypes.shape({ - id: PropTypes.number, - name: PropTypes.string, - usage: PropTypes.arrayOf( - PropTypes.shape({ - label: PropTypes.string, - url: PropTypes.string, - }), - ), - version: PropTypes.number, - }), - ), - parameters: PropTypes.shape({ - courseId: PropTypes.string, - }), - readOnly: PropTypes.bool, - scheme: PropTypes.string, - }), - groupNames: PropTypes.arrayOf(PropTypes.string), - parentGroupId: PropTypes.number, - readOnly: PropTypes.bool, - handleEditGroup: PropTypes.func, - contentGroupActions: PropTypes.shape({ - handleCreate: PropTypes.func, - handleDelete: PropTypes.func, - handleEdit: PropTypes.func, - }), -}; - export default ContentGroupCard; diff --git a/src/group-configurations/content-groups-section/ContentGroupsSection.test.jsx b/src/group-configurations/content-groups-section/ContentGroupsSection.test.jsx deleted file mode 100644 index 725e713420..0000000000 --- a/src/group-configurations/content-groups-section/ContentGroupsSection.test.jsx +++ /dev/null @@ -1,67 +0,0 @@ -import { IntlProvider } from '@edx/frontend-platform/i18n'; -import userEvent from '@testing-library/user-event'; -import { render } from '@testing-library/react'; - -import { contentGroupsMock } from '../__mocks__'; -import placeholderMessages from '../empty-placeholder/messages'; -import messages from './messages'; -import ContentGroupsSection from '.'; - -const handleCreateMock = jest.fn(); -const handleDeleteMock = jest.fn(); -const handleEditMock = jest.fn(); -const contentGroupActions = { - handleCreate: handleCreateMock, - handleDelete: handleDeleteMock, - handleEdit: handleEditMock, -}; - -const renderComponent = (props = {}) => - render( - - - , - ); - -describe('', () => { - it('renders component correctly', () => { - const { getByText, getByRole, getAllByTestId } = renderComponent(); - expect(getByText(contentGroupsMock.name)).toBeInTheDocument(); - expect( - getByRole('button', { name: messages.addNewGroup.defaultMessage }), - ).toBeInTheDocument(); - - expect(getAllByTestId('content-group-card')).toHaveLength( - contentGroupsMock.groups.length, - ); - }); - - it('renders empty section', () => { - const { getByTestId } = renderComponent({ availableGroup: {} }); - expect( - getByTestId('group-configurations-empty-placeholder'), - ).toBeInTheDocument(); - }); - - it('renders container with new group on create click if section is empty', async () => { - const user = userEvent.setup(); - const { getByRole, getByTestId } = renderComponent({ availableGroup: {} }); - await user.click( - getByRole('button', { name: placeholderMessages.button.defaultMessage }), - ); - expect(getByTestId('content-group-form')).toBeInTheDocument(); - }); - - it('renders container with new group on create click if section has groups', async () => { - const user = userEvent.setup(); - const { getByRole, getByTestId } = renderComponent(); - await user.click( - getByRole('button', { name: messages.addNewGroup.defaultMessage }), - ); - expect(getByTestId('content-group-form')).toBeInTheDocument(); - }); -}); diff --git a/src/group-configurations/content-groups-section/ContentGroupsSection.test.tsx b/src/group-configurations/content-groups-section/ContentGroupsSection.test.tsx new file mode 100644 index 0000000000..2b0bc554b4 --- /dev/null +++ b/src/group-configurations/content-groups-section/ContentGroupsSection.test.tsx @@ -0,0 +1,165 @@ +import { + initializeMocks, + render, + screen, + userEvent, + waitFor, +} from '@src/testUtils'; +import { AvailableGroup } from '@src/group-configurations/types'; +import { contentGroupsMock } from '../__mocks__'; +import placeholderMessages from '../empty-placeholder/messages'; +import messages from './messages'; +import ContentGroupsSection from '.'; + +const courseId = 'course-v1:org+101+101'; +const handleCreateMock = jest.fn(); +const handleDeleteMock = jest.fn(); +const handleEditMock = jest.fn(); +const contentGroupActions = { + handleCreate: handleCreateMock, + handleDelete: handleDeleteMock, + handleEdit: handleEditMock, +}; + +const renderComponent = (props = {}) => + render( + , + { path: '/course/:courseId/group_configurations', params: { courseId } }, + ); + +describe('', () => { + beforeEach(() => { + initializeMocks(); + }); + + it('renders component correctly', () => { + renderComponent(); + + expect(screen.getByText(contentGroupsMock.name)).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: messages.addNewGroup.defaultMessage }), + ).toBeInTheDocument(); + contentGroupsMock.groups.forEach(({ name }) => { + expect(screen.getByRole('button', { name: new RegExp(name) })).toBeInTheDocument(); + }); + }); + + it('renders empty section', () => { + renderComponent({ availableGroup: {} }); + + expect(screen.getByText(placeholderMessages.title.defaultMessage)).toBeInTheDocument(); + }); + + it('renders container with new group on create click if section is empty', async () => { + const user = userEvent.setup(); + renderComponent({ availableGroup: {} }); + + await user.click( + screen.getByRole('button', { name: placeholderMessages.button.defaultMessage }), + ); + + expect(screen.getByText(messages.newGroupHeader.defaultMessage)).toBeInTheDocument(); + }); + + it('renders container with new group on create click if section has groups', async () => { + const user = userEvent.setup(); + renderComponent(); + + await user.click( + screen.getByRole('button', { name: messages.addNewGroup.defaultMessage }), + ); + + expect(screen.getByText(messages.newGroupHeader.defaultMessage)).toBeInTheDocument(); + }); + + it('creates a group with the new name appended to the existing ones', async () => { + const user = userEvent.setup(); + renderComponent(); + + await user.click( + screen.getByRole('button', { name: messages.addNewGroup.defaultMessage }), + ); + await user.type( + screen.getByPlaceholderText(messages.newGroupInputPlaceholder.defaultMessage), + 'Brand new group', + ); + await user.click( + screen.getByRole('button', { name: messages.createButton.defaultMessage }), + ); + + await waitFor(() => { + expect(handleCreateMock).toHaveBeenCalledWith( + expect.objectContaining({ + groups: [ + ...contentGroupsMock.groups, + expect.objectContaining({ name: 'Brand new group' }), + ], + }), + expect.any(Function), + ); + }); + }); + + it('edits a group by renaming only the matching one', async () => { + const user = userEvent.setup(); + const [editedGroup] = contentGroupsMock.groups; + renderComponent(); + + await user.click( + screen.getAllByRole('button', { name: messages.actionEdit.defaultMessage })[0], + ); + const nameInput = screen.getByPlaceholderText( + messages.newGroupInputPlaceholder.defaultMessage, + ); + await user.clear(nameInput); + await user.type(nameInput, 'Renamed group'); + await user.click( + screen.getByRole('button', { name: messages.saveButton.defaultMessage }), + ); + + await waitFor(() => { + expect(handleEditMock).toHaveBeenCalledWith( + expect.objectContaining({ + groups: expect.arrayContaining([ + { ...editedGroup, name: 'Renamed group' }, + ]), + }), + expect.any(Function), + ); + }); + }); + + describe('when readOnly', () => { + it('hides the add group button and the card action buttons', () => { + renderComponent({ readOnly: true }); + + contentGroupsMock.groups.forEach(({ name }) => { + expect(screen.getByRole('button', { name: new RegExp(name) })).toBeInTheDocument(); + }); + expect( + screen.queryByRole('button', { name: messages.addNewGroup.defaultMessage }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: messages.actionEdit.defaultMessage }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: messages.actionDelete.defaultMessage }), + ).not.toBeInTheDocument(); + }); + + it('renders the read-only placeholder without the create button if section is empty', () => { + renderComponent({ availableGroup: {}, readOnly: true }); + + expect( + screen.getByText(placeholderMessages.readOnlyTitle.defaultMessage), + ).toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: placeholderMessages.button.defaultMessage }), + ).not.toBeInTheDocument(); + }); + }); +}); diff --git a/src/group-configurations/content-groups-section/index.jsx b/src/group-configurations/content-groups-section/index.tsx similarity index 75% rename from src/group-configurations/content-groups-section/index.jsx rename to src/group-configurations/content-groups-section/index.tsx index 6f1edfd1ea..5525c3218d 100644 --- a/src/group-configurations/content-groups-section/index.jsx +++ b/src/group-configurations/content-groups-section/index.tsx @@ -1,25 +1,30 @@ -import PropTypes from 'prop-types'; -import { useIntl } from '@edx/frontend-platform/i18n'; +import { FormattedMessage } from '@edx/frontend-platform/i18n'; import { Button, useToggle } from '@openedx/paragon'; import { Add as AddIcon } from '@openedx/paragon/icons'; -import { availableGroupPropTypes } from '../constants'; +import { AvailableGroup, ContentGroupFormValues, GroupActions } from '@src/group-configurations/types'; import EmptyPlaceholder from '../empty-placeholder'; import ContentGroupCard from './ContentGroupCard'; import ContentGroupForm from './ContentGroupForm'; import { initialContentGroupObject } from './utils'; import messages from './messages'; +interface ContentGroupsSectionProps { + availableGroup: AvailableGroup; + contentGroupActions: GroupActions; + readOnly?: boolean; +} + const ContentGroupsSection = ({ availableGroup, contentGroupActions, -}) => { - const { formatMessage } = useIntl(); + readOnly = false, +}: ContentGroupsSectionProps) => { const [isNewGroupVisible, openNewGroup, hideNewGroup] = useToggle(false); const { id: parentGroupId, groups, name } = availableGroup; const groupNames = groups?.map((group) => group.name); - const handleCreateNewGroup = (values) => { + const handleCreateNewGroup = (values: ContentGroupFormValues) => { const updatedContentGroups = { ...availableGroup, groups: [ @@ -30,7 +35,11 @@ const ContentGroupsSection = ({ contentGroupActions.handleCreate(updatedContentGroups, hideNewGroup); }; - const handleEditContentGroup = (id, { newGroupName }, callbackToClose) => { + const handleEditContentGroup = ( + id: number, + { newGroupName }: ContentGroupFormValues, + callbackToClose: () => void, + ) => { const updatedContentGroups = { ...availableGroup, groups: availableGroup.groups.map((group) => (group.id === id ? { ...group, name: newGroupName } : group)), @@ -52,11 +61,12 @@ const ContentGroupsSection = ({ groupNames={groupNames} parentGroupId={parentGroupId} key={group.id} + readOnly={readOnly} contentGroupActions={contentGroupActions} handleEditGroup={handleEditContentGroup} /> ))} - {!isNewGroupVisible && ( + {!readOnly && !isNewGroupVisible && ( )} ) : ( - !isNewGroupVisible && + !isNewGroupVisible && )} {isNewGroupVisible && ( - render( - - - , - ); - -describe('', () => { - it('renders EmptyPlaceholder component correctly', () => { - const { getByText, getByRole } = renderComponent(); - - expect(getByText(messages.title.defaultMessage)).toBeInTheDocument(); - expect(getByRole('button', { name: messages.button.defaultMessage })).toBeInTheDocument(); - }); -}); diff --git a/src/group-configurations/empty-placeholder/EmptyPlaceholder.test.tsx b/src/group-configurations/empty-placeholder/EmptyPlaceholder.test.tsx new file mode 100644 index 0000000000..a2bc4098e2 --- /dev/null +++ b/src/group-configurations/empty-placeholder/EmptyPlaceholder.test.tsx @@ -0,0 +1,37 @@ +import { initializeMocks, render, screen } from '@src/testUtils'; + +import messages from './messages'; +import EmptyPlaceholder from '.'; + +const onCreateNewGroup = jest.fn(); + +const renderComponent = (props = {}) => render(); + +describe('', () => { + beforeEach(() => { + initializeMocks(); + }); + + it('renders EmptyPlaceholder component correctly', () => { + renderComponent(); + + expect(screen.getByText(messages.title.defaultMessage)).toBeInTheDocument(); + expect(screen.getByRole('button', { name: messages.button.defaultMessage })).toBeInTheDocument(); + }); + + it('renders the read-only message without the create button when readOnly', () => { + renderComponent({ readOnly: true }); + + expect(screen.getByText(messages.readOnlyTitle.defaultMessage)).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: messages.button.defaultMessage })).not.toBeInTheDocument(); + }); + + it('renders the read-only message without the create button when readOnly and isExperiment', () => { + renderComponent({ readOnly: true, isExperiment: true }); + + expect(screen.getByText(messages.readOnlyTitle.defaultMessage)).toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: messages.experimentalButton.defaultMessage }), + ).not.toBeInTheDocument(); + }); +}); diff --git a/src/group-configurations/empty-placeholder/index.jsx b/src/group-configurations/empty-placeholder/index.jsx deleted file mode 100644 index 80b780d1d5..0000000000 --- a/src/group-configurations/empty-placeholder/index.jsx +++ /dev/null @@ -1,42 +0,0 @@ -import PropTypes from 'prop-types'; -import { useIntl } from '@edx/frontend-platform/i18n'; -import { Add as IconAdd } from '@openedx/paragon/icons'; -import { Button } from '@openedx/paragon'; - -import messages from './messages'; - -const EmptyPlaceholder = ({ onCreateNewGroup, isExperiment }) => { - const { formatMessage } = useIntl(); - const titleMessage = isExperiment - ? messages.experimentalTitle - : messages.title; - const buttonMessage = isExperiment - ? messages.experimentalButton - : messages.button; - - return ( -
-

{formatMessage(titleMessage)}

- -
- ); -}; - -EmptyPlaceholder.defaultProps = { - isExperiment: false, -}; - -EmptyPlaceholder.propTypes = { - onCreateNewGroup: PropTypes.func.isRequired, - isExperiment: PropTypes.bool, -}; - -export default EmptyPlaceholder; diff --git a/src/group-configurations/empty-placeholder/index.tsx b/src/group-configurations/empty-placeholder/index.tsx new file mode 100644 index 0000000000..d92bb44985 --- /dev/null +++ b/src/group-configurations/empty-placeholder/index.tsx @@ -0,0 +1,47 @@ +import { FormattedMessage } from '@edx/frontend-platform/i18n'; +import { Add as IconAdd } from '@openedx/paragon/icons'; +import { Button } from '@openedx/paragon'; + +import messages from './messages'; + +interface EmptyPlaceholderProps { + onCreateNewGroup?: () => void; + isExperiment?: boolean; + readOnly?: boolean; +} + +const EmptyPlaceholder = ({ + onCreateNewGroup, + isExperiment = false, + readOnly = false, +}: EmptyPlaceholderProps) => { + // Read-only users cannot create anything here, so the copy states the course has + // no group configurations instead of inviting them to add the first one. + const titleMessage = readOnly + ? messages.readOnlyTitle + : (isExperiment ? messages.experimentalTitle : messages.title); + const buttonMessage = isExperiment + ? messages.experimentalButton + : messages.button; + + return ( +
+

+ +

+ {!readOnly && ( + + )} +
+ ); +}; + +export default EmptyPlaceholder; diff --git a/src/group-configurations/empty-placeholder/messages.ts b/src/group-configurations/empty-placeholder/messages.ts index 29fcf3b2cb..dcb15ca3cb 100644 --- a/src/group-configurations/empty-placeholder/messages.ts +++ b/src/group-configurations/empty-placeholder/messages.ts @@ -6,6 +6,11 @@ const messages = defineMessages({ defaultMessage: 'You have not created any content groups yet.', description: 'Title displayed when there are no content groups created yet.', }, + readOnlyTitle: { + id: 'course-authoring.group-configurations.empty-placeholder.read-only.title', + defaultMessage: 'No group configurations have been added to this course yet.', + description: 'Title displayed when there are no group configurations and the user cannot create them.', + }, experimentalTitle: { id: 'course-authoring.group-configurations.experimental-empty-placeholder.title', defaultMessage: 'You have not created any group configurations yet.', diff --git a/src/group-configurations/experiment-configurations-section/ExperimentCard.test.jsx b/src/group-configurations/experiment-configurations-section/ExperimentCard.test.tsx similarity index 74% rename from src/group-configurations/experiment-configurations-section/ExperimentCard.test.jsx rename to src/group-configurations/experiment-configurations-section/ExperimentCard.test.tsx index 0997dbd206..c92dd64a10 100644 --- a/src/group-configurations/experiment-configurations-section/ExperimentCard.test.jsx +++ b/src/group-configurations/experiment-configurations-section/ExperimentCard.test.tsx @@ -1,9 +1,8 @@ -import { render } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; -import { IntlProvider } from '@edx/frontend-platform/i18n'; - +import { initializeMocks, render, userEvent } from '@src/testUtils'; +import { AvailableGroup } from '@src/group-configurations/types'; import { experimentGroupConfigurationsMock } from '../__mocks__'; import commonMessages from '../common/messages'; +import messages from './messages'; import ExperimentCard from './ExperimentCard'; const handleCreateMock = jest.fn(); @@ -16,21 +15,26 @@ const experimentConfigurationActions = { }; const onCreateMock = jest.fn(); -const experimentConfiguration = experimentGroupConfigurationsMock[0]; +const experimentConfiguration = experimentGroupConfigurationsMock[0] as AvailableGroup; + +const courseId = 'course-v1:org+101+101'; const renderComponent = (props = {}) => render( - - - , + , + { path: '/course/:courseId/group_configurations', params: { courseId } }, ); describe('', () => { + beforeEach(() => { + initializeMocks(); + }); + it('renders component correctly', () => { const { getByText, getByTestId } = renderComponent(); expect(getByText(experimentConfiguration.name)).toBeInTheDocument(); @@ -38,7 +42,7 @@ describe('', () => { getByText( commonMessages.titleId.defaultMessage.replace( '{id}', - experimentConfiguration.id, + String(experimentConfiguration.id), ), ), ).toBeInTheDocument(); @@ -77,6 +81,20 @@ describe('', () => { ).toBeInTheDocument(); }); + it('hides the action buttons but keeps the outline link when readOnly', async () => { + const user = userEvent.setup(); + const { queryByTestId, getByTestId, getByRole } = renderComponent({ + configuration: { ...experimentConfiguration, usage: [] }, + readOnly: true, + }); + expect(queryByTestId('configuration-card-header-edit')).not.toBeInTheDocument(); + expect(queryByTestId('configuration-card-header-delete')).not.toBeInTheDocument(); + + await user.click(getByTestId('configuration-card-header-button')); + expect(getByTestId('experiment-configuration-card-usage-empty')).toBeInTheDocument(); + expect(getByRole('link', { name: messages.courseOutline.defaultMessage })).toBeInTheDocument(); + }); + it('renders usage with validation error message', async () => { const user = userEvent.setup(); const experimentConfigurationUpdated = { diff --git a/src/group-configurations/experiment-configurations-section/ExperimentCard.jsx b/src/group-configurations/experiment-configurations-section/ExperimentCard.tsx similarity index 57% rename from src/group-configurations/experiment-configurations-section/ExperimentCard.jsx rename to src/group-configurations/experiment-configurations-section/ExperimentCard.tsx index f5d26c1321..84beab270a 100644 --- a/src/group-configurations/experiment-configurations-section/ExperimentCard.jsx +++ b/src/group-configurations/experiment-configurations-section/ExperimentCard.tsx @@ -1,8 +1,7 @@ import { useState, useEffect } from 'react'; -import PropTypes from 'prop-types'; import { useParams } from 'react-router-dom'; import { getConfig } from '@edx/frontend-platform'; -import { useIntl } from '@edx/frontend-platform/i18n'; +import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; import { ActionRow, Hyperlink, @@ -15,7 +14,8 @@ import { EditOutline as EditOutlineIcon, } from '@openedx/paragon/icons'; -import DeleteModal from '../../generic/delete-modal/DeleteModal'; +import DeleteModal from '@src/generic/delete-modal/DeleteModal'; +import { AvailableGroup, GroupActions } from '@src/group-configurations/types'; import TitleButton from '../common/TitleButton'; import UsageList from '../common/UsageList'; import ExperimentCardGroup from './ExperimentCardGroup'; @@ -23,12 +23,21 @@ import ExperimentForm from './ExperimentForm'; import messages from './messages'; import { initialExperimentConfiguration } from './constants'; +interface ExperimentCardProps { + configuration: AvailableGroup; + experimentConfigurationActions: GroupActions; + isExpandedByDefault?: boolean; + onCreate?: (configuration: AvailableGroup) => void; + readOnly?: boolean; +} + const ExperimentCard = ({ configuration, experimentConfigurationActions, - isExpandedByDefault, + isExpandedByDefault = false, onCreate, -}) => { + readOnly = false, +}: ExperimentCardProps) => { const { formatMessage } = useIntl(); const { courseId } = useParams(); const [isExpanded, setIsExpanded] = useState(false); @@ -54,7 +63,7 @@ const ExperimentCard = ({ const outlineComponentLink = ( - {formatMessage(messages.courseOutline)} + ); @@ -63,7 +72,7 @@ const ExperimentCard = ({ className="small text-gray-700" data-testid="experiment-configuration-card-usage-empty" > - {formatMessage(messages.emptyExperimentGroup, { outlineComponentLink })} + ); @@ -82,7 +91,7 @@ const ExperimentCard = ({ closeDeleteModal(); }; - const handleEditConfiguration = (values) => { + const handleEditConfiguration = (values: AvailableGroup) => { experimentConfigurationActions.handleEdit(values, switchOffEditMode); }; @@ -103,7 +112,7 @@ const ExperimentCard = ({
setIsExpanded((prevState) => !prevState)} isExperiment /> - - - - + {!readOnly && ( + + } + alt={formatMessage(messages.actionEdit)} + src={EditOutlineIcon} + iconAs={Icon} + onClick={switchOnEditMode} + data-testid="configuration-card-header-edit" + /> + + } + alt={formatMessage(messages.actionDelete)} + src={DeleteOutlineIcon} + iconAs={Icon} + onClick={openDeleteModal} + data-testid="configuration-card-header-delete" + disabled={isUsedInLocation} + /> + + )}
{isExpanded && (
({ - ...jest.requireActual('react-router-dom'), - useLocation: () => ({ - pathname: mockPathname, - }), -})); - -const renderComponent = (props) => - render( - - - , - ); - -describe('', () => { - it('renders component correctly', () => { - const { getByText, getByRole, getAllByTestId } = renderComponent(); - expect(getByText(messages.title.defaultMessage)).toBeInTheDocument(); - expect( - getByRole('button', { name: messages.addNewGroup.defaultMessage }), - ).toBeInTheDocument(); - expect(getAllByTestId('configuration-card')).toHaveLength( - experimentGroupConfigurationsMock.length, - ); - }); - - it('renders empty section', () => { - const { getByTestId } = renderComponent({ availableGroups: [] }); - expect( - getByTestId('group-configurations-empty-placeholder'), - ).toBeInTheDocument(); - }); -}); diff --git a/src/group-configurations/experiment-configurations-section/ExperimentConfigurationsSection.test.tsx b/src/group-configurations/experiment-configurations-section/ExperimentConfigurationsSection.test.tsx new file mode 100644 index 0000000000..ed128a944f --- /dev/null +++ b/src/group-configurations/experiment-configurations-section/ExperimentConfigurationsSection.test.tsx @@ -0,0 +1,109 @@ +import { + initializeMocks, + render, + screen, + userEvent, + waitFor, +} from '@src/testUtils'; +import { AvailableGroup } from '@src/group-configurations/types'; +import { experimentGroupConfigurationsMock } from '../__mocks__'; +import placeholderMessages from '../empty-placeholder/messages'; +import messages from './messages'; +import ExperimentConfigurationsSection from '.'; + +const courseId = 'course-v1:org+101+101'; +const handleCreateMock = jest.fn(); +const handleDeleteMock = jest.fn(); +const handleEditMock = jest.fn(); +const experimentConfigurationActions = { + handleCreate: handleCreateMock, + handleDelete: handleDeleteMock, + handleEdit: handleEditMock, +}; + +const renderComponent = (props = {}) => + render( + , + { path: '/course/:courseId/group_configurations', params: { courseId } }, + ); + +describe('', () => { + beforeEach(() => { + initializeMocks(); + }); + + it('renders component correctly', () => { + renderComponent(); + + expect(screen.getByText(messages.title.defaultMessage)).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: messages.addNewGroup.defaultMessage }), + ).toBeInTheDocument(); + experimentGroupConfigurationsMock.forEach(({ name }) => { + expect(screen.getByRole('button', { name: new RegExp(name) })).toBeInTheDocument(); + }); + }); + + it('renders empty section', () => { + renderComponent({ availableGroups: [] }); + + expect(screen.getByText(placeholderMessages.experimentalTitle.defaultMessage)).toBeInTheDocument(); + }); + + it('creates a configuration from the new configuration form', async () => { + const user = userEvent.setup(); + renderComponent(); + + await user.click( + screen.getByRole('button', { name: messages.addNewGroup.defaultMessage }), + ); + await user.type( + screen.getByPlaceholderText(messages.experimentConfigurationNamePlaceholder.defaultMessage), + 'New configuration', + ); + await user.click( + screen.getByRole('button', { name: messages.experimentConfigurationCreate.defaultMessage }), + ); + + await waitFor(() => { + expect(handleCreateMock).toHaveBeenCalledWith( + expect.objectContaining({ name: 'New configuration' }), + expect.any(Function), + ); + }); + }); + + describe('when readOnly', () => { + it('hides the add configuration button and the card action buttons', () => { + renderComponent({ readOnly: true }); + + experimentGroupConfigurationsMock.forEach(({ name }) => { + expect(screen.getByRole('button', { name: new RegExp(name) })).toBeInTheDocument(); + }); + expect( + screen.queryByRole('button', { name: messages.addNewGroup.defaultMessage }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: messages.actionEdit.defaultMessage }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: messages.actionDelete.defaultMessage }), + ).not.toBeInTheDocument(); + }); + + it('renders the read-only placeholder without the create button if section is empty', () => { + renderComponent({ availableGroups: [], readOnly: true }); + + expect( + screen.getByText(placeholderMessages.readOnlyTitle.defaultMessage), + ).toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: placeholderMessages.experimentalButton.defaultMessage }), + ).not.toBeInTheDocument(); + }); + }); +}); diff --git a/src/group-configurations/experiment-configurations-section/index.jsx b/src/group-configurations/experiment-configurations-section/index.tsx similarity index 54% rename from src/group-configurations/experiment-configurations-section/index.jsx rename to src/group-configurations/experiment-configurations-section/index.tsx index 26ec0b518f..4f025fd440 100644 --- a/src/group-configurations/experiment-configurations-section/index.jsx +++ b/src/group-configurations/experiment-configurations-section/index.tsx @@ -1,37 +1,42 @@ -import PropTypes from 'prop-types'; import { Button, useToggle } from '@openedx/paragon'; -import { useIntl } from '@edx/frontend-platform/i18n'; +import { FormattedMessage } from '@edx/frontend-platform/i18n'; import { Add as AddIcon } from '@openedx/paragon/icons'; -import { useScrollToHashElement } from '../../hooks'; +import { AvailableGroup, GroupActions } from '@src/group-configurations/types'; +import { useScrollToHashElement } from '@src/hooks'; import EmptyPlaceholder from '../empty-placeholder'; import ExperimentForm from './ExperimentForm'; import ExperimentCard from './ExperimentCard'; import { initialExperimentConfiguration } from './constants'; import messages from './messages'; +interface ExperimentConfigurationsSectionProps { + availableGroups?: AvailableGroup[]; + experimentConfigurationActions: GroupActions; + readOnly?: boolean; +} + const ExperimentConfigurationsSection = ({ - courseId, - availableGroups, + availableGroups = [], experimentConfigurationActions, -}) => { - const { formatMessage } = useIntl(); + readOnly = false, +}: ExperimentConfigurationsSectionProps) => { const [ isNewConfigurationVisible, openNewConfiguration, hideNewConfiguration, ] = useToggle(false); - const handleCreateConfiguration = (configuration) => { + const handleCreateConfiguration = (configuration: AvailableGroup) => { experimentConfigurationActions.handleCreate(configuration, hideNewConfiguration); }; - const { elementWithHash } = useScrollToHashElement({ courseId, isLoading: true }); + const { elementWithHash } = useScrollToHashElement({ isLoading: true }); return (

- {formatMessage(messages.title)} +

{availableGroups.length ? ( @@ -41,11 +46,12 @@ const ExperimentConfigurationsSection = ({ key={configuration.id} configuration={configuration} experimentConfigurationActions={experimentConfigurationActions} - isExpandedByDefault={configuration.id === +elementWithHash} + isExpandedByDefault={configuration.id === Number(elementWithHash)} onCreate={handleCreateConfiguration} + readOnly={readOnly} /> ))} - {!isNewConfigurationVisible && ( + {!readOnly && !isNewConfigurationVisible && ( )} @@ -63,6 +69,7 @@ const ExperimentConfigurationsSection = ({ ) )} @@ -77,43 +84,4 @@ const ExperimentConfigurationsSection = ({ ); }; -ExperimentConfigurationsSection.defaultProps = { - availableGroups: [], -}; - -ExperimentConfigurationsSection.propTypes = { - availableGroups: PropTypes.arrayOf( - PropTypes.shape({ - active: PropTypes.bool, - description: PropTypes.string, - groups: PropTypes.arrayOf( - PropTypes.shape({ - id: PropTypes.number, - name: PropTypes.string, - usage: PropTypes.arrayOf( - PropTypes.shape({ - label: PropTypes.string, - url: PropTypes.string, - }), - ), - version: PropTypes.number, - }).isRequired, - ), - id: PropTypes.number, - name: PropTypes.string, - parameters: PropTypes.shape({ - courseId: PropTypes.string, - }), - readOnly: PropTypes.bool, - scheme: PropTypes.string, - version: PropTypes.number, - }).isRequired, - ), - experimentConfigurationActions: PropTypes.shape({ - handleCreate: PropTypes.func, - handleDelete: PropTypes.func, - }).isRequired, - courseId: PropTypes.string.isRequired, -}; - export default ExperimentConfigurationsSection; diff --git a/src/group-configurations/group-configuration-sidebar/GroupConfigurationSidebar.test.jsx b/src/group-configurations/group-configuration-sidebar/GroupConfigurationSidebar.test.jsx deleted file mode 100644 index 3ffe151031..0000000000 --- a/src/group-configurations/group-configuration-sidebar/GroupConfigurationSidebar.test.jsx +++ /dev/null @@ -1,78 +0,0 @@ -// @ts-check -import { initializeMocks, render } from '../../testUtils'; -import GroupConfigurationSidebar from '.'; -import messages from './messages'; - -const courseId = 'course-123'; -const enrollmentTrackTitle = messages.about_3_title.defaultMessage; -const contentGroupTitle = messages.aboutTitle.defaultMessage; -const experimentGroupTitle = messages.about_2_title.defaultMessage; - -const renderComponent = (props) => render(); - -describe('GroupConfigurationSidebar', () => { - beforeEach(() => { - initializeMocks(); - }); - - it('renders all groups when all props are true', async () => { - const { findAllByRole } = renderComponent({ - shouldShowExperimentGroups: true, - shouldShowContentGroup: true, - shouldShowEnrollmentTrackGroup: true, - }); - const titles = await findAllByRole('heading', { level: 4 }); - - expect(titles[0]).toHaveTextContent(enrollmentTrackTitle); - expect(titles[1]).toHaveTextContent(contentGroupTitle); - expect(titles[2]).toHaveTextContent(experimentGroupTitle); - }); - - it('renders no groups when all props are false', async () => { - const { queryByText } = renderComponent({ - shouldShowExperimentGroups: false, - shouldShowContentGroup: false, - shouldShowEnrollmentTrackGroup: false, - }); - - expect(queryByText(enrollmentTrackTitle)).not.toBeInTheDocument(); - expect(queryByText(contentGroupTitle)).not.toBeInTheDocument(); - expect(queryByText(experimentGroupTitle)).not.toBeInTheDocument(); - }); - - it('renders only content group when shouldShowContentGroup is true', async () => { - const { queryByText, getByText } = renderComponent({ - shouldShowExperimentGroups: false, - shouldShowContentGroup: true, - shouldShowEnrollmentTrackGroup: false, - }); - - expect(queryByText(enrollmentTrackTitle)).not.toBeInTheDocument(); - expect(getByText(contentGroupTitle)).toBeInTheDocument(); - expect(queryByText(experimentGroupTitle)).not.toBeInTheDocument(); - }); - - it('renders only experiment group when shouldShowExperimentGroups is true', async () => { - const { queryByText, getByText } = renderComponent({ - shouldShowExperimentGroups: true, - shouldShowContentGroup: false, - shouldShowEnrollmentTrackGroup: false, - }); - - expect(queryByText(enrollmentTrackTitle)).not.toBeInTheDocument(); - expect(queryByText(contentGroupTitle)).not.toBeInTheDocument(); - expect(getByText(experimentGroupTitle)).toBeInTheDocument(); - }); - - it('renders only enrollment track group when shouldShowEnrollmentTrackGroup is true', async () => { - const { queryByText, getByText } = renderComponent({ - shouldShowExperimentGroups: false, - shouldShowContentGroup: false, - shouldShowEnrollmentTrackGroup: true, - }); - - expect(getByText(enrollmentTrackTitle)).toBeInTheDocument(); - expect(queryByText(contentGroupTitle)).not.toBeInTheDocument(); - expect(queryByText(experimentGroupTitle)).not.toBeInTheDocument(); - }); -}); diff --git a/src/group-configurations/group-configuration-sidebar/GroupConfigurationSidebar.test.tsx b/src/group-configurations/group-configuration-sidebar/GroupConfigurationSidebar.test.tsx new file mode 100644 index 0000000000..ca69123008 --- /dev/null +++ b/src/group-configurations/group-configuration-sidebar/GroupConfigurationSidebar.test.tsx @@ -0,0 +1,102 @@ +import { initializeMocks, render, screen } from '@src/testUtils'; + +import GroupConfigurationSidebar from '.'; +import messages from './messages'; + +const courseId = 'course-123'; +const enrollmentTrackTitle = messages.about_3_title.defaultMessage; +const contentGroupTitle = messages.aboutTitle.defaultMessage; +const experimentGroupTitle = messages.about_2_title.defaultMessage; +// The paragraphs explaining how to add, edit and delete groups are the only ones naming these +// buttons, so they stand in for the instructions that read-only users should not see. +const contentGroupInstructions = messages.aboutDescription_3_strong.defaultMessage; +const experimentGroupInstructions = messages.about_2_description_2_strong.defaultMessage; + +const renderComponent = (props = {}) => + render( + , + ); + +describe('', () => { + beforeEach(() => { + initializeMocks(); + }); + + it('renders all groups when all props are true', async () => { + renderComponent({ + shouldShowExperimentGroups: true, + shouldShowContentGroup: true, + shouldShowEnrollmentTrackGroup: true, + }); + const titles = await screen.findAllByRole('heading', { level: 4 }); + + expect(titles[0]).toHaveTextContent(enrollmentTrackTitle); + expect(titles[1]).toHaveTextContent(contentGroupTitle); + expect(titles[2]).toHaveTextContent(experimentGroupTitle); + }); + + it('renders no groups when all props are false', () => { + renderComponent(); + + expect(screen.queryByText(enrollmentTrackTitle)).not.toBeInTheDocument(); + expect(screen.queryByText(contentGroupTitle)).not.toBeInTheDocument(); + expect(screen.queryByText(experimentGroupTitle)).not.toBeInTheDocument(); + }); + + it('renders only content group when shouldShowContentGroup is true', () => { + renderComponent({ shouldShowContentGroup: true }); + + expect(screen.queryByText(enrollmentTrackTitle)).not.toBeInTheDocument(); + expect(screen.getByText(contentGroupTitle)).toBeInTheDocument(); + expect(screen.queryByText(experimentGroupTitle)).not.toBeInTheDocument(); + }); + + it('renders only experiment group when shouldShowExperimentGroups is true', () => { + renderComponent({ shouldShowExperimentGroups: true }); + + expect(screen.queryByText(enrollmentTrackTitle)).not.toBeInTheDocument(); + expect(screen.queryByText(contentGroupTitle)).not.toBeInTheDocument(); + expect(screen.getByText(experimentGroupTitle)).toBeInTheDocument(); + }); + + it('renders only enrollment track group when shouldShowEnrollmentTrackGroup is true', () => { + renderComponent({ shouldShowEnrollmentTrackGroup: true }); + + expect(screen.getByText(enrollmentTrackTitle)).toBeInTheDocument(); + expect(screen.queryByText(contentGroupTitle)).not.toBeInTheDocument(); + expect(screen.queryByText(experimentGroupTitle)).not.toBeInTheDocument(); + }); + + it('renders the instructions to add, edit and delete groups by default', () => { + renderComponent({ + shouldShowExperimentGroups: true, + shouldShowContentGroup: true, + shouldShowEnrollmentTrackGroup: true, + }); + + expect(screen.getByText(contentGroupInstructions)).toBeInTheDocument(); + expect(screen.getByText(experimentGroupInstructions)).toBeInTheDocument(); + }); + + it('hides the instructions to add, edit and delete groups when readOnly', () => { + renderComponent({ + shouldShowExperimentGroups: true, + shouldShowContentGroup: true, + shouldShowEnrollmentTrackGroup: true, + readOnly: true, + }); + + expect(screen.queryByText(contentGroupInstructions)).not.toBeInTheDocument(); + expect(screen.queryByText(experimentGroupInstructions)).not.toBeInTheDocument(); + // The rest of the help text is still shown. + expect(screen.getByText(messages.aboutDescription_1.defaultMessage)).toBeInTheDocument(); + expect(screen.getByText(messages.about_2_description_1.defaultMessage)).toBeInTheDocument(); + expect(screen.getByText(messages.about_3_description_3.defaultMessage)).toBeInTheDocument(); + }); +}); diff --git a/src/group-configurations/group-configuration-sidebar/index.jsx b/src/group-configurations/group-configuration-sidebar/index.tsx similarity index 69% rename from src/group-configurations/group-configuration-sidebar/index.jsx rename to src/group-configurations/group-configuration-sidebar/index.tsx index dc318b1d03..4a7c1a0866 100644 --- a/src/group-configurations/group-configuration-sidebar/index.jsx +++ b/src/group-configurations/group-configuration-sidebar/index.tsx @@ -1,27 +1,35 @@ import { Fragment } from 'react'; -import PropTypes from 'prop-types'; import { useIntl } from '@edx/frontend-platform/i18n'; import { Hyperlink } from '@openedx/paragon'; -import { HelpSidebar } from '../../generic/help-sidebar'; -import { useHelpUrls } from '../../help-urls/hooks'; +import { HelpSidebar } from '@src/generic/help-sidebar'; +import { useHelpUrls } from '@src/help-urls/hooks'; import { getSidebarData } from './utils'; import messages from './messages'; +interface GroupConfigurationSidebarProps { + courseId: string; + shouldShowExperimentGroups: boolean; + shouldShowContentGroup: boolean; + shouldShowEnrollmentTrackGroup: boolean; + readOnly?: boolean; +} + const GroupConfigurationSidebar = ({ courseId, shouldShowExperimentGroups, shouldShowContentGroup, shouldShowEnrollmentTrackGroup, -}) => { + readOnly = false, +}: GroupConfigurationSidebarProps) => { const intl = useIntl(); const urls = useHelpUrls(['groupConfigurations', 'enrollmentTracks', 'contentGroups']); const sidebarData = getSidebarData({ - messages, intl, shouldShowExperimentGroups, shouldShowContentGroup, shouldShowEnrollmentTrackGroup, + readOnly, }); return ( @@ -36,15 +44,15 @@ const GroupConfigurationSidebar = ({

{title}

- {paragraphs.map((text) => ( -

- {text} + {paragraphs.map(({ id, content }) => ( +

+ {content}

))} {intl.formatMessage(messages.learnMoreBtn)} @@ -56,11 +64,4 @@ const GroupConfigurationSidebar = ({ ); }; -GroupConfigurationSidebar.propTypes = { - courseId: PropTypes.string.isRequired, - shouldShowContentGroup: PropTypes.bool.isRequired, - shouldShowExperimentGroups: PropTypes.bool.isRequired, - shouldShowEnrollmentTrackGroup: PropTypes.bool.isRequired, -}; - export default GroupConfigurationSidebar; diff --git a/src/group-configurations/group-configuration-sidebar/utils.jsx b/src/group-configurations/group-configuration-sidebar/utils.jsx deleted file mode 100644 index 0d0cac6343..0000000000 --- a/src/group-configurations/group-configuration-sidebar/utils.jsx +++ /dev/null @@ -1,61 +0,0 @@ -/** - * Compiles the sidebar data for the course authoring sidebar. - * - * @param {Object} messages - The localized messages. - * @param {Object} intl - The intl object for formatting messages. - * @param {boolean} shouldShowExperimentGroups - Flag to include experiment group configuration data. - * @param {boolean} shouldShowContentGroup - Flag to include content group data. - * @param {boolean} shouldShowEnrollmentTrackGroup - Flag to include enrollment track group data. - * @returns {Object[]} The array of sidebar data groups. - */ -const getSidebarData = ({ - messages, - intl, - shouldShowExperimentGroups, - shouldShowContentGroup, - shouldShowEnrollmentTrackGroup, -}) => { - const groups = []; - - if (shouldShowEnrollmentTrackGroup) { - groups.push({ - urlKey: 'enrollmentTracks', - title: intl.formatMessage(messages.about_3_title), - paragraphs: [ - intl.formatMessage(messages.about_3_description_1), - intl.formatMessage(messages.about_3_description_2), - intl.formatMessage(messages.about_3_description_3), - ], - }); - } - if (shouldShowContentGroup) { - groups.push({ - urlKey: 'contentGroups', - title: intl.formatMessage(messages.aboutTitle), - paragraphs: [ - intl.formatMessage(messages.aboutDescription_1), - intl.formatMessage(messages.aboutDescription_2), - intl.formatMessage(messages.aboutDescription_3, { - strongText: {intl.formatMessage(messages.aboutDescription_3_strong)}, - strongText2: {intl.formatMessage(messages.aboutDescription_strong_edit)}, - }), - ], - }); - } - if (shouldShowExperimentGroups) { - groups.push({ - urlKey: 'groupConfigurations', - title: intl.formatMessage(messages.about_2_title), - paragraphs: [ - intl.formatMessage(messages.about_2_description_1), - intl.formatMessage(messages.about_2_description_2, { - strongText: {intl.formatMessage(messages.about_2_description_2_strong)}, - strongText2: {intl.formatMessage(messages.aboutDescription_strong_edit)}, - }), - ], - }); - } - return groups; -}; - -export { getSidebarData }; diff --git a/src/group-configurations/group-configuration-sidebar/utils.tsx b/src/group-configurations/group-configuration-sidebar/utils.tsx new file mode 100644 index 0000000000..b066818ef4 --- /dev/null +++ b/src/group-configurations/group-configuration-sidebar/utils.tsx @@ -0,0 +1,96 @@ +import { type IntlShape } from '@edx/frontend-platform/i18n'; +import { type ReactNode } from 'react'; +import type { MessageDescriptor } from 'react-intl'; + +import messages from './messages'; + +interface SidebarParagraph { + id: string; + content: ReactNode; +} + +interface SidebarGroup { + urlKey: string; + title: string; + paragraphs: SidebarParagraph[]; +} + +interface GetSidebarDataArgs { + intl: IntlShape; + shouldShowExperimentGroups: boolean; + shouldShowContentGroup: boolean; + shouldShowEnrollmentTrackGroup: boolean; + readOnly?: boolean; +} + +/** Formats a message into a paragraph keyed by its message id. */ +const paragraph = ( + intl: IntlShape, + message: MessageDescriptor, + values?: Record, +): SidebarParagraph => ({ + id: String(message.id), + content: intl.formatMessage(message, values), +}); + +/** + * Compiles the sidebar data for the course authoring sidebar. + * + * Read-only users cannot add, edit or delete anything on this page, so the paragraphs + * explaining how to do so are left out for them. + */ +const getSidebarData = ({ + intl, + shouldShowExperimentGroups, + shouldShowContentGroup, + shouldShowEnrollmentTrackGroup, + readOnly = false, +}: GetSidebarDataArgs): SidebarGroup[] => { + const groups: SidebarGroup[] = []; + + if (shouldShowEnrollmentTrackGroup) { + groups.push({ + urlKey: 'enrollmentTracks', + title: intl.formatMessage(messages.about_3_title), + paragraphs: [ + paragraph(intl, messages.about_3_description_1), + paragraph(intl, messages.about_3_description_2), + paragraph(intl, messages.about_3_description_3), + ], + }); + } + if (shouldShowContentGroup) { + groups.push({ + urlKey: 'contentGroups', + title: intl.formatMessage(messages.aboutTitle), + paragraphs: [ + paragraph(intl, messages.aboutDescription_1), + paragraph(intl, messages.aboutDescription_2), + ...(readOnly ? [] : [ + paragraph(intl, messages.aboutDescription_3, { + strongText: {intl.formatMessage(messages.aboutDescription_3_strong)}, + strongText2: {intl.formatMessage(messages.aboutDescription_strong_edit)}, + }), + ]), + ], + }); + } + if (shouldShowExperimentGroups) { + groups.push({ + urlKey: 'groupConfigurations', + title: intl.formatMessage(messages.about_2_title), + paragraphs: [ + paragraph(intl, messages.about_2_description_1), + ...(readOnly ? [] : [ + paragraph(intl, messages.about_2_description_2, { + strongText: {intl.formatMessage(messages.about_2_description_2_strong)}, + strongText2: {intl.formatMessage(messages.aboutDescription_strong_edit)}, + }), + ]), + ], + }); + } + return groups; +}; + +export { getSidebarData }; diff --git a/src/group-configurations/index.tsx b/src/group-configurations/index.tsx index d362daee75..cdd4322219 100644 --- a/src/group-configurations/index.tsx +++ b/src/group-configurations/index.tsx @@ -39,6 +39,7 @@ const GroupConfigurations = () => { const { isLoading: isLoadingUserPermissions, + canViewGroupConfigurations, canManageGroupConfigurations, } = useCourseUserPermissions(courseId, getGroupConfigurationsPermissions(courseId)); @@ -53,7 +54,7 @@ const GroupConfigurations = () => { allGroupConfigurations = [], } = groupConfigurations ?? {}; - if (!isLoadingUserPermissions && !canManageGroupConfigurations) { + if (!isLoadingUserPermissions && !canViewGroupConfigurations) { return ; } @@ -120,13 +121,14 @@ const GroupConfigurations = () => { )} {shouldShowExperimentGroups && ( )} @@ -137,6 +139,7 @@ const GroupConfigurations = () => { shouldShowExperimentGroups={shouldShowExperimentGroups} shouldShowContentGroup={!!contentGroup} shouldShowEnrollmentTrackGroup={!!enrollmentTrackGroup} + readOnly={!canManageGroupConfigurations} /> diff --git a/src/group-configurations/types.ts b/src/group-configurations/types.ts index 4341ff3b84..1760043c67 100644 --- a/src/group-configurations/types.ts +++ b/src/group-configurations/types.ts @@ -1,20 +1,33 @@ /* eslint-disable import/no-extraneous-dependencies */ import { AxiosError } from 'axios'; +export interface UsageValidation { + text: string; + type: string; +} + export interface Usage { label: string; url: string; + validation?: UsageValidation; } export interface Group { id: number; name: string; - usage?: Usage[]; + usage?: Usage[] | null; version: number; } +/** A group that has not been persisted yet -- the backend assigns its id. */ +export type NewGroup = Omit; + +export interface ContentGroupFormValues { + newGroupName: string; +} + export interface AvailableGroupParameters { - courseId: string; + courseId?: string; } export interface AvailableGroup { @@ -26,7 +39,28 @@ export interface AvailableGroup { parameters?: AvailableGroupParameters; readOnly?: boolean; scheme: string; + usage?: Usage[] | null; version: number; } +/** The body sent when creating a group: the group being added has no id yet. */ +export interface GroupCreatePayload extends Omit { + groups: (Group | NewGroup)[]; +} + export type OnErrorCallbackFunc = (error: AxiosError) => void; + +/** + * Create/edit/delete handlers for a group configuration section. + * + * `handleDelete` takes the id of the thing being deleted, plus the id of the group + * within it for content groups, which are nested under a parent configuration. + * + * The create/edit handlers are async, but callers close their form from the callback + * rather than from the returned promise, so the return value is typed as `void`. + */ +export interface GroupActions { + handleCreate: (group: GroupCreatePayload, callbackToClose: () => void) => void; + handleEdit: (group: AvailableGroup, callbackToClose: () => void) => void; + handleDelete: (id: number, groupId?: number) => void; +} diff --git a/src/header/hooks.test.tsx b/src/header/hooks.test.tsx index 051bd21b15..c50d53e79f 100644 --- a/src/header/hooks.test.tsx +++ b/src/header/hooks.test.tsx @@ -283,7 +283,7 @@ describe('header utils', () => { canViewGradingSettings: true, canViewScheduleAndDetails: true, canViewCourseTeam: true, - canManageGroupConfigurations: true, + canViewGroupConfigurations: true, canViewCertificates: true, } as ReturnType); }); @@ -492,12 +492,12 @@ describe('header utils', () => { expect(actualItemsTitle).not.toContain('Roles and Permissions'); }); - it('when authz flag is enabled and user lacks canManageGroupConfigurations should not include group configurations option', () => { + it('when authz flag is enabled and user lacks canViewGroupConfigurations should not include group configurations option', () => { mockWaffleFlags({ enableAuthzCourseAuthoring: true }); jest.mocked(useCourseUserPermissions).mockReturnValue({ isLoading: false, isAuthzEnabled: true, - canManageGroupConfigurations: false, + canViewGroupConfigurations: false, } as any); const actualItemsTitle = renderHook(() => useSettingMenuItems('course-123'), { wrapper: createWrapper() }).result .current.map((item) => item.title); diff --git a/src/header/hooks.tsx b/src/header/hooks.tsx index 65bee3e53c..b859fcbedb 100644 --- a/src/header/hooks.tsx +++ b/src/header/hooks.tsx @@ -125,7 +125,7 @@ export const useSettingMenuItems = (courseId: string) => { }, ] : []), - ...(perms.canManageGroupConfigurations + ...(perms.canViewGroupConfigurations ? [{ href: `/course/${courseId}/group_configurations`, title: intl.formatMessage(messages['header.links.groupConfigurations']), From 44c0d579bbcb0ec98f658a343cd0313a77744874 Mon Sep 17 00:00:00 2001 From: Aditya Bhalsod Date: Tue, 8 Sep 2026 22:50:22 +0530 Subject: [PATCH 07/16] feat: add in-platform transcript editor to the videos page (#3215) Course authors can now edit video transcripts directly in Studio: "Edit transcript" in a transcript row's action menu opens an editor with per-cue text editing, SRT validation, debounced auto-save, and click-to-seek playback with active-cue highlighting against the video preview. The video's Info content moves below the preview (InfoTab), the sidebar lists transcripts directly, and the row menu label becomes "Info and transcripts" for videos. --- .../generic/FileMenu.test.tsx | 2 +- src/files-and-videos/generic/FileMenu.tsx | 2 +- src/files-and-videos/generic/FileTable.jsx | 44 +- src/files-and-videos/generic/InfoModal.jsx | 3 + src/files-and-videos/generic/messages.ts | 10 + .../table-custom-columns/MoreInfoColumn.jsx | 2 +- .../MoreInfoColumn.test.jsx | 2 +- .../table-custom-columns/TranscriptColumn.jsx | 30 +- .../TranscriptColumn.test.tsx | 42 ++ src/files-and-videos/index.scss | 10 + .../videos-page/CourseVideosTable.tsx | 7 +- .../videos-page/VideosPage.test.tsx | 30 +- src/files-and-videos/videos-page/data/api.js | 13 +- .../info-sidebar/TranscriptTab.jsx | 315 ++++++++- .../info-sidebar/TranscriptTab.test.jsx | 286 +++++--- .../info-sidebar/VideoInfoModalSidebar.jsx | 40 +- .../videos-page/info-sidebar/messages.ts | 50 ++ .../transcript-item/LanguageSelect.jsx | 58 +- .../transcript-item/LanguageSelect.scss | 19 + .../transcript-item/Transcript.jsx | 179 +++-- .../transcript-item/Transcript.test.tsx | 35 + .../transcript-item/TranscriptMenu.jsx | 22 +- .../transcript-item/TranscriptMenu.test.tsx | 39 ++ .../info-sidebar/transcript-item/messages.ts | 20 + .../transcript-editor/TranscriptEditor.scss | 218 ++++++ .../TranscriptEditor.test.tsx | 403 +++++++++++ .../transcript-editor/TranscriptEditor.tsx | 660 ++++++++++++++++++ .../videos-page/transcript-editor/index.ts | 1 + .../videos-page/transcript-editor/messages.ts | 101 +++ .../transcript-editor/srtUtils.test.ts | 99 +++ .../videos-page/transcript-editor/srtUtils.ts | 90 +++ 31 files changed, 2549 insertions(+), 283 deletions(-) create mode 100644 src/files-and-videos/generic/table-components/table-custom-columns/TranscriptColumn.test.tsx create mode 100644 src/files-and-videos/videos-page/info-sidebar/transcript-item/Transcript.test.tsx create mode 100644 src/files-and-videos/videos-page/info-sidebar/transcript-item/TranscriptMenu.test.tsx create mode 100644 src/files-and-videos/videos-page/transcript-editor/TranscriptEditor.scss create mode 100644 src/files-and-videos/videos-page/transcript-editor/TranscriptEditor.test.tsx create mode 100644 src/files-and-videos/videos-page/transcript-editor/TranscriptEditor.tsx create mode 100644 src/files-and-videos/videos-page/transcript-editor/index.ts create mode 100644 src/files-and-videos/videos-page/transcript-editor/messages.ts create mode 100644 src/files-and-videos/videos-page/transcript-editor/srtUtils.test.ts create mode 100644 src/files-and-videos/videos-page/transcript-editor/srtUtils.ts diff --git a/src/files-and-videos/generic/FileMenu.test.tsx b/src/files-and-videos/generic/FileMenu.test.tsx index 06706b7739..1d8ee81738 100644 --- a/src/files-and-videos/generic/FileMenu.test.tsx +++ b/src/files-and-videos/generic/FileMenu.test.tsx @@ -132,7 +132,7 @@ describe('FileMenu', () => { expect(screen.queryByText('Copy video ID')).not.toBeInTheDocument(); expect(screen.queryByText('Download')).not.toBeInTheDocument(); - expect(screen.getByText('Info')).toBeInTheDocument(); + expect(screen.getByText('Info and transcripts')).toBeInTheDocument(); }); }); }); diff --git a/src/files-and-videos/generic/FileMenu.tsx b/src/files-and-videos/generic/FileMenu.tsx index 29b58a5568..bc9e84ed9d 100644 --- a/src/files-and-videos/generic/FileMenu.tsx +++ b/src/files-and-videos/generic/FileMenu.tsx @@ -70,7 +70,7 @@ const FileMenu = ({ )} - + {permissions.canDeleteFiles && ( <> diff --git a/src/files-and-videos/generic/FileTable.jsx b/src/files-and-videos/generic/FileTable.jsx index d2064958b5..b5f0022775 100644 --- a/src/files-and-videos/generic/FileTable.jsx +++ b/src/files-and-videos/generic/FileTable.jsx @@ -1,4 +1,10 @@ -import { useCallback, useEffect, useState } from 'react'; +import { + useCallback, + useEffect, + useMemo, + useRef, + useState, +} from 'react'; import { useSelector } from 'react-redux'; import PropTypes from 'prop-types'; import isEmpty from 'lodash/isEmpty'; @@ -24,6 +30,7 @@ import { MoreInfoColumn, FilterStatus, Footer, + TranscriptColumn, } from './table-components'; import ApiStatusToast from './ApiStatusToast'; import DeleteConfirmationModal from './DeleteConfirmationModal'; @@ -42,6 +49,7 @@ const FileTable = ({ maxFileSize, thumbnailPreview, infoModalSidebar, + infoModalContentUnderPreview = /** @type {(file: any) => React.ReactNode} */ (/* istanbul ignore next */ () => null), permissions = { canCreateFiles: true, canDeleteFiles: true, @@ -203,10 +211,31 @@ const FileTable = ({ }), }; - const hasMoreInfoColumn = tableColumns.filter(col => col.id === 'moreInfo').length === 1; - if (!hasMoreInfoColumn) { - tableColumns.push({ ...moreInfoColumn }); - } + const handleOpenFileInfoRef = useRef(handleOpenFileInfo); + handleOpenFileInfoRef.current = handleOpenFileInfo; + + // Copy the caller's columns instead of mutating them. Memoized (with the ref above) + // so the columns identity stays stable across re-renders: a new identity makes + // DataTable reset its internal state. + const columns = useMemo(() => { + const cols = [...tableColumns]; + if (cols.filter(col => col.id === 'moreInfo').length !== 1) { + cols.push({ ...moreInfoColumn }); + } + const transcriptColIndex = cols.findIndex(col => col.id === 'transcriptStatus'); + if (transcriptColIndex !== -1) { + cols[transcriptColIndex] = { + ...cols[transcriptColIndex], + Cell: ({ row }) => ( + handleOpenFileInfoRef.current(file)} + /> + ), + }; + } + return cols; + }, [tableColumns]); return (
@@ -235,7 +264,7 @@ const FileTable = ({ initialState={initialState} tableActions={headerActions} bulkActions={headerActions} - columns={tableColumns} + columns={columns} itemCount={files.length} pageCount={pageCount} data={files} @@ -322,6 +351,7 @@ const FileTable = ({ usagePathStatus={usagePathStatus} error={usageErrorMessages} sidebar={infoModalSidebar} + contentUnderPreview={infoModalContentUnderPreview(selectedRows[0].original)} /> )}
@@ -354,6 +384,8 @@ FileTable.propTypes = { maxFileSize: PropTypes.number.isRequired, thumbnailPreview: PropTypes.func.isRequired, infoModalSidebar: PropTypes.func.isRequired, + // Render function `(file) => ReactNode` shown in the info modal under the file preview. + infoModalContentUnderPreview: PropTypes.func, permissions: PropTypes.shape({ canCreateFiles: PropTypes.bool, canDeleteFiles: PropTypes.bool, diff --git a/src/files-and-videos/generic/InfoModal.jsx b/src/files-and-videos/generic/InfoModal.jsx index 85862bcaa2..54e542a058 100644 --- a/src/files-and-videos/generic/InfoModal.jsx +++ b/src/files-and-videos/generic/InfoModal.jsx @@ -24,6 +24,7 @@ const InfoModal = ({ usagePathStatus, error, sidebar, + contentUnderPreview = /* istanbul ignore next */ null, }) => { const intl = useIntl(); const [activeTab, setActiveTab] = useState('fileInfo'); @@ -75,6 +76,7 @@ const InfoModal = ({ thumbnailPreview={thumbnailPreview} imageSize={{ width: '503px', height: '281px' }} /> + {contentUnderPreview}
@@ -114,6 +116,7 @@ InfoModal.propTypes = { error: PropTypes.arrayOf(PropTypes.string).isRequired, thumbnailPreview: PropTypes.func.isRequired, sidebar: PropTypes.func.isRequired, + contentUnderPreview: PropTypes.node, }; InfoModal.defaultProps = { diff --git a/src/files-and-videos/generic/messages.ts b/src/files-and-videos/generic/messages.ts index bb4badf86f..303e8c6bab 100644 --- a/src/files-and-videos/generic/messages.ts +++ b/src/files-and-videos/generic/messages.ts @@ -115,6 +115,11 @@ const messages = defineMessages({ defaultMessage: 'Info', description: 'Label for info button in card menu dropdown', }, + infoAndTranscriptsTitle: { + id: 'course-authoring.files-and-uploads.cardMenu.infoAndTranscriptsTitle', + defaultMessage: 'Info and transcripts', + description: 'Label for video info button in card menu dropdown', + }, downloadEncodingsTitle: { id: 'course-authoring.files-and-uploads.cardMenu.downloadEncodingsTitle', defaultMessage: 'Download video list (.csv)', @@ -214,6 +219,11 @@ const messages = defineMessages({ defaultMessage: 'Upload a file', description: 'Accessible (screen reader) label for file input', }, + transcriptCountLabel: { + id: 'course-authoring.videos-page.table.transcriptColumn.message', + defaultMessage: '{numOfTranscripts, plural, =0 {No transcripts available} one {{numOfTranscripts} transcript available} other {{numOfTranscripts} transcripts available}}', + description: 'Message with the number of transcripts available for a video', + }, }); export default messages; diff --git a/src/files-and-videos/generic/table-components/table-custom-columns/MoreInfoColumn.jsx b/src/files-and-videos/generic/table-components/table-custom-columns/MoreInfoColumn.jsx index 03b76691fd..21e1e9f0dd 100644 --- a/src/files-and-videos/generic/table-components/table-custom-columns/MoreInfoColumn.jsx +++ b/src/files-and-videos/generic/table-components/table-custom-columns/MoreInfoColumn.jsx @@ -122,7 +122,7 @@ const MoreInfoColumn = ({ variant="tertiary" onClick={() => handleOpenFileInfo(row.original)} > - + {permissions.canDeleteFiles && ( diff --git a/src/files-and-videos/generic/table-components/table-custom-columns/MoreInfoColumn.test.jsx b/src/files-and-videos/generic/table-components/table-custom-columns/MoreInfoColumn.test.jsx index 0e58c00fff..0bb4179af8 100644 --- a/src/files-and-videos/generic/table-components/table-custom-columns/MoreInfoColumn.test.jsx +++ b/src/files-and-videos/generic/table-components/table-custom-columns/MoreInfoColumn.test.jsx @@ -159,7 +159,7 @@ describe('MoreInfoColumn', () => { expect(screen.queryByText('Copy video ID')).not.toBeInTheDocument(); expect(screen.queryByText('Download')).not.toBeInTheDocument(); - expect(screen.getByText('Info')).toBeInTheDocument(); + expect(screen.getByText('Info and transcripts')).toBeInTheDocument(); }); it('calls handleBulkDownload when Download is clicked', async () => { diff --git a/src/files-and-videos/generic/table-components/table-custom-columns/TranscriptColumn.jsx b/src/files-and-videos/generic/table-components/table-custom-columns/TranscriptColumn.jsx index 230a7e259a..46060c6578 100644 --- a/src/files-and-videos/generic/table-components/table-custom-columns/TranscriptColumn.jsx +++ b/src/files-and-videos/generic/table-components/table-custom-columns/TranscriptColumn.jsx @@ -1,26 +1,35 @@ import React from 'react'; import PropTypes from 'prop-types'; import { FormattedMessage } from '@edx/frontend-platform/i18n'; -import { Icon } from '@openedx/paragon'; +import { Button, Icon } from '@openedx/paragon'; import { Info } from '@openedx/paragon/icons'; import { TRANSCRIPT_FAILURE_STATUSES } from '../../../videos-page/data/constants'; +import messages from '../../messages'; -const TranscriptColumn = ({ row }) => { +const TranscriptColumn = ({ row, handleOpenFileInfo = /** @type {((file: any) => void) | null} */ (null) }) => { const { transcripts, transcriptionStatus } = row.original; - const numOfTranscripts = transcripts?.length; - const transcriptMessage = numOfTranscripts > 0 ? `(${numOfTranscripts}) available` : null; + const numOfTranscripts = transcripts?.length ?? 0; + const label = ; return (
{TRANSCRIPT_FAILURE_STATUSES.includes(transcriptionStatus) && ( )} - + {handleOpenFileInfo ? + ( + + ) : + label}
); }; @@ -32,6 +41,7 @@ TranscriptColumn.propTypes = { transcriptionStatus: PropTypes.string.isRequired, }.isRequired, }.isRequired, + handleOpenFileInfo: PropTypes.func, }; export default TranscriptColumn; diff --git a/src/files-and-videos/generic/table-components/table-custom-columns/TranscriptColumn.test.tsx b/src/files-and-videos/generic/table-components/table-custom-columns/TranscriptColumn.test.tsx new file mode 100644 index 0000000000..165c0c0857 --- /dev/null +++ b/src/files-and-videos/generic/table-components/table-custom-columns/TranscriptColumn.test.tsx @@ -0,0 +1,42 @@ +import { + fireEvent, + initializeMocks, + render, + screen, +} from '@src/testUtils'; +import TranscriptColumn from './TranscriptColumn'; + +const rowWith = (transcripts: string[], transcriptionStatus = '') => ({ + original: { transcripts, transcriptionStatus }, +}); + +describe('TranscriptColumn', () => { + beforeEach(() => { + initializeMocks(); + }); + + it('pluralizes the transcript count', () => { + const { rerender } = render(); + expect(screen.getByText('1 transcript available')).toBeInTheDocument(); + + rerender(); + expect(screen.getByText('3 transcripts available')).toBeInTheDocument(); + + rerender(); + expect(screen.getByText('No transcripts available')).toBeInTheDocument(); + }); + + it('renders plain text when no handler is provided', () => { + render(); + expect(screen.queryByRole('button')).not.toBeInTheDocument(); + }); + + it('opens the file info from the count link, including when there are no transcripts yet', () => { + const handleOpenFileInfo = jest.fn(); + const row = rowWith([]); + render(); + + fireEvent.click(screen.getByRole('button', { name: 'No transcripts available' })); + expect(handleOpenFileInfo).toHaveBeenCalledWith(row.original); + }); +}); diff --git a/src/files-and-videos/index.scss b/src/files-and-videos/index.scss index ee5c3d58c8..bd8b97c9ad 100644 --- a/src/files-and-videos/index.scss +++ b/src/files-and-videos/index.scss @@ -79,3 +79,13 @@ .video-upload-warning-text { font-size: 18px; } + +.transcript-upload-button { + border-radius: 5px; + border: 2px solid; + + &:focus::before, + &:focus-visible::before { + display: none !important; + } +} diff --git a/src/files-and-videos/videos-page/CourseVideosTable.tsx b/src/files-and-videos/videos-page/CourseVideosTable.tsx index 1aeb4c69a1..0840505754 100644 --- a/src/files-and-videos/videos-page/CourseVideosTable.tsx +++ b/src/files-and-videos/videos-page/CourseVideosTable.tsx @@ -29,6 +29,7 @@ import { } from '@src/files-and-videos/videos-page/data/thunks'; import { getFormattedDuration, resampleFile } from '@src/files-and-videos/videos-page/data/utils'; import VideoInfoModalSidebar from '@src/files-and-videos/videos-page/info-sidebar'; +import InfoTab from '@src/files-and-videos/videos-page/info-sidebar/InfoTab'; import messages from '@src/files-and-videos/videos-page/messages'; import TranscriptSettings from '@src/files-and-videos/videos-page/transcript-settings'; import UploadModal from '@src/files-and-videos/videos-page/upload-modal'; @@ -159,9 +160,8 @@ export const CourseVideosTable = () => { handleAddThumbnail, videoImageSettings, }); - const infoModalSidebar = (video, activeTab, setActiveTab) => ( - - ); + const infoModalSidebar = (video) => ; + const infoModalContentUnderPreview = (video) => ; const maxFileSize = videoUploadMaxFileSize * 1073741824; const transcriptColumn = { id: 'transcriptStatus', @@ -279,6 +279,7 @@ export const CourseVideosTable = () => { maxFileSize, thumbnailPreview, infoModalSidebar, + infoModalContentUnderPreview, files: videos, }} /> diff --git a/src/files-and-videos/videos-page/VideosPage.test.tsx b/src/files-and-videos/videos-page/VideosPage.test.tsx index 4a39689213..91e2ddb042 100644 --- a/src/files-and-videos/videos-page/VideosPage.test.tsx +++ b/src/files-and-videos/videos-page/VideosPage.test.tsx @@ -502,10 +502,10 @@ describe('Videos page', () => { }); fireEvent.click(within(videoMenuButton).getByLabelText('file-menu-toggle')); - fireEvent.click(screen.getByText('Info')); + fireEvent.click(screen.getByText(messages.infoAndTranscriptsTitle.defaultMessage)); await waitFor(() => { - expect(screen.getByText(messages.infoTitle.defaultMessage)).toBeVisible(); + expect(screen.getByText(messages.usageTitle.defaultMessage)).toBeVisible(); }); const { usageStatus } = store.getState().videos; @@ -521,15 +521,12 @@ describe('Videos page', () => { axiosMock.onGet(`${getVideosUrl(courseId)}/mOckID1/usage`).reply(201, { usageLocations: [] }); fireEvent.click(within(videoMenuButton).getByLabelText('file-menu-toggle')); - fireEvent.click(screen.getByText('Info')); + fireEvent.click(screen.getByText(messages.infoAndTranscriptsTitle.defaultMessage)); await waitFor(() => { expect(screen.getByText(messages.usageNotInUseMessage.defaultMessage)).toBeVisible(); }); - const infoTab = screen.getAllByRole('tab')[0]; - expect(infoTab).toBeVisible(); - - expect(infoTab).toHaveClass('active'); + expect(screen.getByText(messages.usageTitle.defaultMessage)).toBeVisible(); }); it('should open video info modal and show transcript tab', async () => { @@ -538,16 +535,12 @@ describe('Videos page', () => { axiosMock.onGet(`${getVideosUrl(courseId)}/mOckID1/usage`).reply(201, { usageLocations: [] }); fireEvent.click(within(videoMenuButton).getByLabelText('file-menu-toggle')); - fireEvent.click(screen.getByText('Info')); + fireEvent.click(screen.getByText(messages.infoAndTranscriptsTitle.defaultMessage)); await waitFor(() => { expect(screen.getByText(messages.usageNotInUseMessage.defaultMessage)).toBeVisible(); }); - const transcriptTab = screen.getAllByRole('tab')[1]; - fireEvent.click(transcriptTab); - expect(transcriptTab).toBeVisible(); - - expect(transcriptTab).toHaveClass('active'); + expect(screen.getByText('Transcript (0)')).toBeVisible(); }); it('should show transcript error', async () => { @@ -556,12 +549,11 @@ describe('Videos page', () => { axiosMock.onGet(`${getVideosUrl(courseId)}/mOckID3/usage`).reply(201, { usageLocations: [] }); fireEvent.click(within(videoMenuButton).getByLabelText('file-menu-toggle')); - fireEvent.click(screen.getByText('Info')); - - const transcriptTab = screen.getAllByRole('tab')[1]; - fireEvent.click(transcriptTab); + fireEvent.click(screen.getByText(messages.infoAndTranscriptsTitle.defaultMessage)); - expect(screen.getByText('Transcript (1)')).toBeVisible(); + await waitFor(() => { + expect(screen.getByText('Transcript (1)')).toBeVisible(); + }); }); }); @@ -712,7 +704,7 @@ describe('Videos page', () => { axiosMock.onGet(`${getVideosUrl(courseId)}/mOckID3/usage`).reply(404); fireEvent.click(within(videoMenuButton).getByLabelText('file-menu-toggle')); - fireEvent.click(screen.getByText('Info')); + fireEvent.click(screen.getByText(messages.infoAndTranscriptsTitle.defaultMessage)); await executeThunk( getUsagePaths({ courseId, diff --git a/src/files-and-videos/videos-page/data/api.js b/src/files-and-videos/videos-page/data/api.js index b6d1f41d82..655c27aea5 100644 --- a/src/files-and-videos/videos-page/data/api.js +++ b/src/files-and-videos/videos-page/data/api.js @@ -75,12 +75,21 @@ export async function downloadTranscript({ apiUrl, filename, }) { - const { data } = await getAuthenticatedHttpClient() - .get(`${getApiBaseUrl()}${apiUrl}?edx_video_id=${videoId}&language_code=${language}`); + const data = await fetchTranscriptContent({ videoId, language, apiUrl }); const file = new Blob([data], { type: 'text/plain;charset=utf-8' }); saveAs(file, filename); } +export async function fetchTranscriptContent({ + videoId, + language, + apiUrl, +}) { + const { data } = await getAuthenticatedHttpClient() + .get(`${getApiBaseUrl()}${apiUrl}?edx_video_id=${videoId}&language_code=${language}`); + return data; +} + export async function uploadTranscript({ videoId, newLanguage, diff --git a/src/files-and-videos/videos-page/info-sidebar/TranscriptTab.jsx b/src/files-and-videos/videos-page/info-sidebar/TranscriptTab.jsx index c46fe8a305..25fe9a3c4f 100644 --- a/src/files-and-videos/videos-page/info-sidebar/TranscriptTab.jsx +++ b/src/files-and-videos/videos-page/info-sidebar/TranscriptTab.jsx @@ -1,14 +1,35 @@ -import React, { useEffect, useState, useRef } from 'react'; +import React, { + useEffect, + useMemo, + useState, + useRef, +} from 'react'; import PropTypes from 'prop-types'; import { useDispatch, useSelector } from 'react-redux'; import { isEmpty } from 'lodash'; -import { Button, Stack } from '@openedx/paragon'; -import { Add } from '@openedx/paragon/icons'; +import { + Button, + Icon, + IconButton, + Spinner, + Stack, + Toast, +} from '@openedx/paragon'; +import { + Add, + CheckCircle, + Delete, + Error as ErrorIcon, + FileUpload, + Article, +} from '@openedx/paragon/icons'; import { useIntl } from '@edx/frontend-platform/i18n'; import ErrorAlert from '../../../editors/sharedComponents/ErrorAlerts/ErrorAlert'; import { getLanguages, getSortedTranscripts } from '../data/utils'; import Transcript from './transcript-item'; +import LanguageSelect from './transcript-item/LanguageSelect'; +import { FileInput, useFileInput } from '../../generic'; import { deleteVideoTranscript, downloadVideoTranscript, @@ -17,6 +38,7 @@ import { } from '../data/thunks'; import { RequestStatus } from '../../../data/constants'; import messages from './messages'; +import { isValidSrt } from '../transcript-editor/srtUtils'; const TranscriptTab = ({ video, @@ -35,9 +57,44 @@ const TranscriptTab = ({ transcriptDownloadHandlerUrl, } = videoTranscriptSettings; const { transcripts, id, displayName } = video; - const languages = getLanguages(transcriptAvailableLanguages); + const languages = useMemo( + () => getLanguages(transcriptAvailableLanguages), + [transcriptAvailableLanguages], + ); let sortedTranscripts = getSortedTranscripts(languages, transcripts); const [previousSelection, setPreviousSelection] = useState(sortedTranscripts); + const [isAddingTranscript, setIsAddingTranscript] = useState(false); + const [newLanguage, setNewLanguage] = useState(''); + const [selectedFile, setSelectedFile] = useState(null); + const [pendingFileName, setPendingFileName] = useState(''); + const [isSubmittingNewTranscript, setIsSubmittingNewTranscript] = useState(false); + const [isSubmittingReplace, setIsSubmittingReplace] = useState(false); + const [showAddTranscriptError, setShowAddTranscriptError] = useState(false); + const [showAddedToast, setShowAddedToast] = useState(false); + const [invalidSrtFile, setInvalidSrtFile] = useState(false); + + const addTranscriptFileInput = useFileInput({ + onAddFile: (files) => { + const [file] = files; + if (!file) { + return; + } + const reader = new FileReader(); + reader.onload = (e) => { + if (!isValidSrt(e.target.result)) { + setInvalidSrtFile(true); + setSelectedFile(null); + } else { + setInvalidSrtFile(false); + setSelectedFile(file); + setShowAddTranscriptError(false); + } + }; + reader.readAsText(file); + }, + setSelectedRows: () => {}, + setAddOpen: () => {}, + }); useEffect(() => { dispatch(resetErrors({ errorType: 'transcript' })); @@ -45,22 +102,59 @@ const TranscriptTab = ({ setPreviousSelection(sortedTranscripts); }, [transcripts]); - const handleAddEmptyTranscript = () => { - setPreviousSelection(['', ...previousSelection]); - if (divRef?.current?.scrollTo) { - divRef.current.scrollTo({ top: 0, behavior: 'smooth' }); + useEffect(() => { + if (isAddingTranscript) { + const firstAvailableLanguage = Object.keys(languages).find((lang) => !previousSelection.includes(lang)); + setNewLanguage(firstAvailableLanguage || ''); + setSelectedFile(null); } - }; + }, [isAddingTranscript]); + + useEffect(() => { + if (!isAddingTranscript || !isSubmittingNewTranscript) { + return; + } + + if (transcriptStatus === RequestStatus.SUCCESSFUL) { + setIsSubmittingNewTranscript(false); + setPendingFileName(''); + setIsAddingTranscript(false); + setSelectedFile(null); + setShowAddTranscriptError(false); + setShowAddedToast(true); + } + + if (transcriptStatus === RequestStatus.FAILED) { + setIsSubmittingNewTranscript(false); + setPendingFileName(''); + setSelectedFile(null); + setShowAddTranscriptError(true); + } + }, [isAddingTranscript, isSubmittingNewTranscript, transcriptStatus]); + + useEffect(() => { + if (!isSubmittingReplace) { + return; + } + if (transcriptStatus === RequestStatus.SUCCESSFUL) { + setIsSubmittingReplace(false); + setShowAddedToast(true); + } + if (transcriptStatus === RequestStatus.FAILED) { + setIsSubmittingReplace(false); + } + }, [isSubmittingReplace, transcriptStatus]); const handleTranscript = (data, actionType) => { const { language, - newLanguage, + newLanguage: transcriptNewLanguage, file, } = data; dispatch(resetErrors({ errorType: 'transcript' })); switch (actionType) { case 'delete': + /* istanbul ignore if -- legacy empty-row path; the add form no longer creates empty rows */ if (isEmpty(language)) { const updatedSelection = previousSelection; updatedSelection.shift(); @@ -83,26 +177,48 @@ const TranscriptTab = ({ })); break; case 'upload': + if (!isEmpty(language)) { + setIsSubmittingReplace(true); + } dispatch(uploadVideoTranscript({ language, videoId: id, apiUrl: transcriptUploadHandlerUrl, - newLanguage, + newLanguage: transcriptNewLanguage, file, transcripts, })); break; + /* istanbul ignore next */ default: break; } }; + const availableLanguages = Object.entries(languages) + .filter(([lang]) => !previousSelection.includes(lang)); + + const handleSubmitNewTranscript = () => { + /* istanbul ignore if -- the submit button is disabled in this state */ + if (!newLanguage || !selectedFile) { + return; + } + setShowAddTranscriptError(false); + setIsSubmittingNewTranscript(true); + setPendingFileName(selectedFile.name); + handleTranscript({ + language: '', + newLanguage, + file: selectedFile, + }, 'upload'); + }; + return ( -
+
    {errors.transcript.map(message => ( @@ -112,30 +228,157 @@ const TranscriptTab = ({ ))}
- {previousSelection.map((transcript, idx) => ( - - ))} -
-
- + {isAddingTranscript ? + ( + +
{intl.formatMessage(messages.newTranscriptTitle)}
+ +
+ { + setNewLanguage(lang); + setShowAddTranscriptError(false); + }} + /> +
+ + {!selectedFile && !isSubmittingNewTranscript && ( + + )} + + {isSubmittingNewTranscript ? + ( + + + {pendingFileName} + + ) : + null} + + {selectedFile && !isSubmittingNewTranscript ? + ( + + + + {selectedFile.name} + + setSelectedFile(null)} + className="flex-shrink-0" + /> + + ) : + null} + + {showAddTranscriptError && ( + + + {intl.formatMessage(messages.addTranscriptFailedLabel)} + + )} + + {invalidSrtFile && ( + + + {intl.formatMessage(messages.invalidSrtFormat)} + + )} + + {!selectedFile &&
{intl.formatMessage(messages.uploadHelpText)} +
} + + + + + + + + + ) : + ( + <> + {previousSelection.map((transcript, idx) => ( + + ))} + + )}
+ {!isAddingTranscript && ( +
+ +
+ )} + setShowAddedToast(false)}> + + + {intl.formatMessage(messages.newTranscriptAddedLabel)} + + ); }; diff --git a/src/files-and-videos/videos-page/info-sidebar/TranscriptTab.test.jsx b/src/files-and-videos/videos-page/info-sidebar/TranscriptTab.test.jsx index 656214024d..3b9faa735b 100644 --- a/src/files-and-videos/videos-page/info-sidebar/TranscriptTab.test.jsx +++ b/src/files-and-videos/videos-page/info-sidebar/TranscriptTab.test.jsx @@ -4,7 +4,9 @@ import { fireEvent, screen, waitFor, + within, } from '@testing-library/react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; import userEvent from '@testing-library/user-event'; import ReactDOM from 'react-dom'; @@ -27,8 +29,8 @@ import { import { getApiBaseUrl } from '../data/api'; import messages from './messages'; -import genericMessages from '../../generic/messages'; import transcriptRowMessages from './transcript-item/messages'; +import editorMessages from '../transcript-editor/messages'; import VideosPageProvider from '../VideosPageProvider'; import { deleteVideoTranscript } from '../data/thunks'; @@ -51,20 +53,42 @@ const defaultProps = { let axiosMock; let store; +let queryClient; jest.mock('file-saver'); const renderComponent = (props) => { render( - - - - - - - , + + + + + + + + + , ); }; +const openAddForm = async () => { + const addButton = screen.getByText(messages.uploadButtonLabel.defaultMessage); + await act(async () => { + fireEvent.click(addButton); + }); +}; + +const selectLanguage = async (languageText, dropdownEl = null) => { + const dropdown = dropdownEl || screen.getByTestId('language-select-dropdown'); + await act(async () => { + fireEvent.click(dropdown); + }); + const allMatches = screen.getAllByText(languageText); + const option = allMatches.find(el => el.closest('.pgn__menu')) || allMatches[allMatches.length - 1]; + await act(async () => { + fireEvent.click(option); + }); +}; + describe('TranscriptTab', () => { beforeEach(async () => { initializeMockApp({ @@ -77,97 +101,92 @@ describe('TranscriptTab', () => { }); store = initializeStore(initialState); axiosMock = new MockAdapter(getAuthenticatedHttpClient()); + queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + axiosMock.onGet(/course_waffle_flags/).reply(200, {}); }); describe('with no transcripts preloaded', () => { it('should have add transcript button', async () => { renderComponent(defaultProps); const addButton = screen.getByText(messages.uploadButtonLabel.defaultMessage); - const transcriptRow = screen.queryByTestId('transcript', { exact: false }); + const transcriptRow = screen.queryByTestId('transcript-', { exact: false }); expect(addButton).toBeInTheDocument(); expect(transcriptRow).toBeNull(); }); - it('should delete empty transcript row', async () => { + it('should open and cancel new transcript form', async () => { renderComponent(defaultProps); - const addButton = screen.getByText(messages.uploadButtonLabel.defaultMessage); - await act(async () => { - fireEvent.click(addButton); - }); - - const deleteButton = screen.getByLabelText('delete empty transcript'); - await act(async () => { - fireEvent.click(deleteButton); - }); + await openAddForm(); - expect(screen.getByText(transcriptRowMessages.deleteConfirmationHeader.defaultMessage)).toBeVisible(); + expect(screen.getByText(messages.newTranscriptTitle.defaultMessage)).toBeVisible(); - const confirmButton = screen.getByText(transcriptRowMessages.confirmDeleteLabel.defaultMessage); + const cancelButton = screen.getByText(messages.cancelButtonLabel.defaultMessage); await act(async () => { - fireEvent.click(confirmButton); + fireEvent.click(cancelButton); }); - expect(screen.queryByTestId('transcript-')).toBeNull(); + expect(screen.queryByText(messages.newTranscriptTitle.defaultMessage)).toBeNull(); }); describe('uploadVideoTranscript as add function', () => { - let addButton; - const file = new File(['(⌐□_□)'], 'download.srt', { type: 'text/srt' }); - beforeEach(async () => { - renderComponent(defaultProps); - addButton = screen.getByText(messages.uploadButtonLabel.defaultMessage); - - await act(async () => { - fireEvent.click(addButton); - }); - }); + const srtContent = '1\n00:00:00,000 --> 00:00:01,000\nTest\n'; + const file = new File([srtContent], 'download.srt', { type: 'text/srt' }); it('should upload new transcript', async () => { const user = userEvent.setup(); + renderComponent(defaultProps); axiosMock.onPost(`${getApiBaseUrl()}/transcript_upload/`).reply(204); + + await openAddForm(); + await selectLanguage('Arabic'); + await act(async () => { - const addFileInput = screen.getByLabelText(genericMessages.fileInputAriaLabel.defaultMessage); - expect(addFileInput).toBeInTheDocument(); + fireEvent.click(screen.getByText(messages.uploadFileLabel.defaultMessage)); + }); + const fileInputs = document.querySelectorAll('input[type="file"]'); + await act(async () => { + await user.upload(fileInputs[fileInputs.length - 1], file); + }); - await user.upload(addFileInput, file); + const submitButton = screen.getByText(messages.addTranscriptButtonLabel.defaultMessage); + await act(async () => { + fireEvent.click(submitButton); }); - const addStatus = store.getState().videos.transcriptStatus; + const addStatus = store.getState().videos.transcriptStatus; expect(addStatus).toEqual(RequestStatus.SUCCESSFUL); }); - it('should show default error message', async () => { + it('should show default error message on upload failure', async () => { const user = userEvent.setup(); + renderComponent(defaultProps); axiosMock.onPost(`${getApiBaseUrl()}/transcript_upload/`).reply(404); - await act(async () => { - const addFileInput = screen.getByLabelText(genericMessages.fileInputAriaLabel.defaultMessage); - await user.upload(addFileInput, file); - }); - const addStatus = store.getState().videos.transcriptStatus; - expect(addStatus).toEqual(RequestStatus.FAILED); + await openAddForm(); + await selectLanguage('Arabic'); - expect(screen.getAllByText('Failed to add .')[0]).toBeVisible(); - }); + await act(async () => { + fireEvent.click(screen.getByText(messages.uploadFileLabel.defaultMessage)); + }); + const fileInputs = document.querySelectorAll('input[type="file"]'); + await act(async () => { + await user.upload(fileInputs[fileInputs.length - 1], file); + }); - it('should show api provided error message', async () => { - const user = userEvent.setup(); - axiosMock.onPost(`${getApiBaseUrl()}/transcript_upload/`).reply(404, { error: 'api error' }); + const submitButton = screen.getByText(messages.addTranscriptButtonLabel.defaultMessage); await act(async () => { - const addFileInput = screen.getByLabelText(genericMessages.fileInputAriaLabel.defaultMessage); - await user.upload(addFileInput, file); + fireEvent.click(submitButton); }); - const addStatus = store.getState().videos.transcriptStatus; + const addStatus = store.getState().videos.transcriptStatus; expect(addStatus).toEqual(RequestStatus.FAILED); - - expect(screen.getAllByText('api error')[0]).toBeVisible(); }); }); }); - describe('with one transcripts preloaded', () => { + describe('with one transcript preloaded', () => { const updatedProps = { ...defaultProps, transcripts: ['ar'] }; + beforeEach(() => { renderComponent(updatedProps); }); @@ -195,12 +214,12 @@ describe('TranscriptTab', () => { await waitFor(() => { fireEvent.click(cancelButton); }); - expect(screen.queryByText(transcriptRowMessages.deleteConfirmationHeader.defaultMessage)).toBeNull(); }); it('should open delete confirmation modal and handle delete', async () => { - const confirmButton = screen.getByText(transcriptRowMessages.confirmDeleteLabel.defaultMessage); + const confirmButton = screen.getAllByText(transcriptRowMessages.confirmDeleteLabel.defaultMessage) + .find(el => el.closest('.btn-danger') || el.classList.contains('btn-danger')); axiosMock.onDelete(`${getApiBaseUrl()}/transcript_delete/${courseId}/mOckID0/ar`).reply(204); await act(async () => { fireEvent.click(confirmButton); @@ -215,14 +234,13 @@ describe('TranscriptTab', () => { ); }); const deleteStatus = store.getState().videos.transcriptStatus; - expect(deleteStatus).toEqual(RequestStatus.SUCCESSFUL); - expect(screen.queryByText(transcriptRowMessages.deleteConfirmationHeader.defaultMessage)).toBeNull(); }); - it('should show error message', async () => { - const confirmButton = screen.getByText(transcriptRowMessages.confirmDeleteLabel.defaultMessage); + it('should show error message on delete failure', async () => { + const confirmButton = screen.getAllByText(transcriptRowMessages.confirmDeleteLabel.defaultMessage) + .find(el => el.closest('.btn-danger') || el.classList.contains('btn-danger')); axiosMock.onDelete(`${getApiBaseUrl()}/transcript_delete/${courseId}/mOckID0/ar`).reply(404); await act(async () => { fireEvent.click(confirmButton); @@ -237,11 +255,8 @@ describe('TranscriptTab', () => { ); }); const deleteStatus = store.getState().videos.transcriptStatus; - expect(deleteStatus).toEqual(RequestStatus.FAILED); - expect(screen.queryByText(transcriptRowMessages.deleteConfirmationHeader.defaultMessage)).toBeNull(); - expect(screen.getAllByText('Failed to delete ar transcript.')[0]).toBeVisible(); }); }); @@ -266,11 +281,10 @@ describe('TranscriptTab', () => { fireEvent.click(downloadButton); }); const downloadStatus = store.getState().videos.transcriptStatus; - expect(downloadStatus).toEqual(RequestStatus.SUCCESSFUL); }); - it('should show error message', async () => { + it('should show error message on download failure', async () => { const filename = 'mOckID0.mp4-ar.srt'; axiosMock.onGet( `${getApiBaseUrl()}/transcript_download/?edx_video_id=${updatedProps.id}&language_code=ar`, @@ -279,9 +293,7 @@ describe('TranscriptTab', () => { fireEvent.click(downloadButton); }); const downloadStatus = store.getState().videos.transcriptStatus; - expect(downloadStatus).toEqual(RequestStatus.FAILED); - expect(screen.getAllByText(`Failed to download ${filename}.`)[0]).toBeVisible(); }); }); @@ -289,19 +301,15 @@ describe('TranscriptTab', () => { describe('with multiple transcripts preloaded', () => { describe('uploadVideoTranscript as replace function', () => { - const file = new File(['(⌐□_□)'], 'download.srt', { type: 'text/srt' }); + const srtContent = '1\n00:00:00,000 --> 00:00:01,000\nTest\n'; + const file = new File([srtContent], 'download.srt', { type: 'text/srt' }); + beforeEach(async () => { const updatedProps = { ...defaultProps, transcripts: ['fr', 'ar'] }; renderComponent(updatedProps); - const dropdownButton = screen.getAllByTestId('language-select-dropdown')[0]; - await waitFor(() => { - fireEvent.click(dropdownButton); - }); - const englishOption = screen.getByText('English'); - await act(async () => { - fireEvent.click(englishOption); - }); + const dropdownButtons = screen.getAllByTestId('language-select-dropdown'); + await selectLanguage('English', dropdownButtons[0]); const menuButton = screen.getByTestId('ar-transcript-menu'); await waitFor(() => { @@ -318,33 +326,139 @@ describe('TranscriptTab', () => { axiosMock.onPost(`${getApiBaseUrl()}/transcript_upload/`).reply(204); await act(async () => { - const addFileInput = screen.getAllByLabelText(genericMessages.fileInputAriaLabel.defaultMessage)[0]; - await user.upload(addFileInput, file); + const fileInputs = document.querySelectorAll('input[type="file"]'); + await user.upload(fileInputs[0], file); }); - const addStatus = store.getState().videos.transcriptStatus; + const addStatus = store.getState().videos.transcriptStatus; expect(addStatus).toEqual(RequestStatus.SUCCESSFUL); const updatedTranscripts = store.getState().models.videos[defaultProps.id].transcripts; - expect(updatedTranscripts).toEqual(['fr', 'en']); }); - it('should show error message', async () => { + it('should show error message on replace failure', async () => { const user = userEvent.setup(); axiosMock.onPost(`${getApiBaseUrl()}/transcript_upload/`).reply(404); await act(async () => { - const addFileInput = screen.getAllByLabelText(genericMessages.fileInputAriaLabel.defaultMessage)[0]; - await user.upload(addFileInput, file); + const fileInputs = document.querySelectorAll('input[type="file"]'); + await user.upload(fileInputs[0], file); }); const addStatus = store.getState().videos.transcriptStatus; - expect(addStatus).toEqual(RequestStatus.FAILED); - expect(screen.getAllByText('Failed to replace ar with en.')[0]).toBeVisible(); }); }); }); + describe('add transcript form interactions', () => { + const srt = '1\n00:00:01,000 --> 00:00:02,000\nHello\n'; + const chooseFile = async (file) => { + await act(async () => { + fireEvent.change(document.querySelector('input[type="file"]'), { target: { files: file ? [file] : [] } }); + }); + }; + + it('shows the selected SRT and lets the user remove it before submitting', async () => { + renderComponent(defaultProps); + await openAddForm(); + await chooseFile(new File([srt], 'valid.srt', { type: 'text/srt' })); + expect(await screen.findByText('valid.srt')).toBeInTheDocument(); + + fireEvent.click(screen.getByRole('button', { name: messages.removeSelectedFileLabel.defaultMessage })); + await waitFor(() => expect(screen.queryByText('valid.srt')).not.toBeInTheDocument()); + }); + + it('rejects an invalid SRT file on add', async () => { + renderComponent(defaultProps); + await openAddForm(); + await chooseFile(new File(['not an srt'], 'bad.srt', { type: 'text/srt' })); + expect(await screen.findByText(messages.invalidSrtFormat.defaultMessage)).toBeInTheDocument(); + expect(screen.queryByText('bad.srt')).not.toBeInTheDocument(); + }); + + it('ignores an empty file selection', async () => { + renderComponent(defaultProps); + await openAddForm(); + await chooseFile(null); + expect(screen.queryByText(messages.invalidSrtFormat.defaultMessage)).not.toBeInTheDocument(); + expect(screen.getByRole('button', { name: messages.addTranscriptButtonLabel.defaultMessage })).toBeDisabled(); + }); + + it('filters the language list from the search box and reports no results', async () => { + renderComponent(defaultProps); + await openAddForm(); + await act(async () => { + fireEvent.click(screen.getByTestId('language-select-dropdown')); + }); + const search = screen.getByPlaceholderText(transcriptRowMessages.searchLanguagesPlaceholder.defaultMessage); + fireEvent.change(search, { target: { value: 'zzzz' } }); + expect(screen.getByText(transcriptRowMessages.noLanguageResults.defaultMessage)).toBeInTheDocument(); + fireEvent.change(search, { target: { value: 'Eng' } }); + const options = screen.getAllByText('English'); + await act(async () => { + fireEvent.click(options[options.length - 1]); + }); + expect(screen.getByTestId('language-select-dropdown')).toHaveTextContent('English'); + }); + + it('submits a new transcript, then shows a dismissible success toast', async () => { + axiosMock.onPost(`${getApiBaseUrl()}/transcript_upload/`).reply(204); + renderComponent(defaultProps); + await openAddForm(); + await chooseFile(new File([srt], 'valid.srt', { type: 'text/srt' })); + await screen.findByText('valid.srt'); + + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: messages.addTranscriptButtonLabel.defaultMessage })); + }); + expect(await screen.findByText(messages.newTranscriptAddedLabel.defaultMessage)).toBeInTheDocument(); + + fireEvent.click(screen.getByRole('button', { name: /close/i })); + await waitFor(() => + expect(screen.queryByText(messages.newTranscriptAddedLabel.defaultMessage)).not.toBeInTheDocument() + ); + }); + }); + + describe('transcript row interactions', () => { + const rowProps = { ...defaultProps, transcripts: ['ar'] }; + // The row's hidden file input is rendered as a sibling of the row element, first in document order. + const rowFileInput = () => document.querySelectorAll('input[type="file"]')[0]; + + it('rejects an invalid SRT file on replace', async () => { + renderComponent(rowProps); + await act(async () => { + fireEvent.change(rowFileInput(), { target: { files: [new File(['nope'], 'bad.srt', { type: 'text/srt' })] } }); + }); + expect(await screen.findByText(messages.invalidSrtFormat.defaultMessage)).toBeInTheDocument(); + }); + + it('ignores an empty replace selection', async () => { + renderComponent(rowProps); + await act(async () => { + fireEvent.change(rowFileInput(), { target: { files: [] } }); + }); + expect(screen.queryByText(messages.invalidSrtFormat.defaultMessage)).not.toBeInTheDocument(); + }); + + it('opens the transcript editor from the row menu and closes it again', async () => { + axiosMock.onGet(/edx_video_id/).reply(200, ''); + renderComponent(rowProps); + const row = screen.getByTestId('transcript-ar'); + fireEvent.click(within(row).getByRole('button', { name: 'Actions dropdown' })); + await act(async () => { + fireEvent.click(await screen.findByText(transcriptRowMessages.editTranscript.defaultMessage)); + }); + expect(await screen.findByRole('button', { name: editorMessages.insertCueLabel.defaultMessage })) + .toBeInTheDocument(); + + fireEvent.click(screen.getByRole('button', { name: editorMessages.cancelButtonLabel.defaultMessage })); + await waitFor(() => + expect(screen.queryByRole('button', { name: editorMessages.insertCueLabel.defaultMessage })).not + .toBeInTheDocument() + ); + }); + }); }); diff --git a/src/files-and-videos/videos-page/info-sidebar/VideoInfoModalSidebar.jsx b/src/files-and-videos/videos-page/info-sidebar/VideoInfoModalSidebar.jsx index d567c54e3c..82381e456d 100644 --- a/src/files-and-videos/videos-page/info-sidebar/VideoInfoModalSidebar.jsx +++ b/src/files-and-videos/videos-page/info-sidebar/VideoInfoModalSidebar.jsx @@ -1,46 +1,22 @@ import React from 'react'; import PropTypes from 'prop-types'; import { useIntl } from '@edx/frontend-platform/i18n'; -import { - Tabs, - Tab, -} from '@openedx/paragon'; -import InfoTab from './InfoTab'; +import { Stack } from '@openedx/paragon'; import TranscriptTab from './TranscriptTab'; import messages from './messages'; -import { TRANSCRIPT_FAILURE_STATUSES } from '../data/constants'; const VideoInfoModalSidebar = ({ video, - activeTab, - setActiveTab, }) => { const intl = useIntl(); return ( - setActiveTab(tab)} - > - - - - - {intl.formatMessage(messages.notificationScreenReaderText)} - - )} - > - - - + +
+ {intl.formatMessage(messages.transcriptTabTitle, { transcriptCount: video.transcripts.length })} +
+ + ); }; @@ -54,8 +30,6 @@ VideoInfoModalSidebar.propTypes = { transcripts: PropTypes.arrayOf(PropTypes.string), transcriptionStatus: PropTypes.string.isRequired, }), - activeTab: PropTypes.string.isRequired, - setActiveTab: PropTypes.func.isRequired, }; VideoInfoModalSidebar.defaultProps = { diff --git a/src/files-and-videos/videos-page/info-sidebar/messages.ts b/src/files-and-videos/videos-page/info-sidebar/messages.ts index a1823e4667..7246ad4205 100644 --- a/src/files-and-videos/videos-page/info-sidebar/messages.ts +++ b/src/files-and-videos/videos-page/info-sidebar/messages.ts @@ -40,6 +40,56 @@ const messages = defineMessages({ defaultMessage: 'Add a transcript', description: 'Label for upload button', }, + newTranscriptTitle: { + id: 'course-authoriong.video-uploads.file-info.transcriptTab.newTranscript.title', + defaultMessage: 'New transcript', + description: 'Heading shown when adding a new transcript', + }, + uploadFileLabel: { + id: 'course-authoriong.video-uploads.file-info.transcriptTab.uploadFile.label', + defaultMessage: 'Upload file', + description: 'Label for selecting a transcript file', + }, + uploadHelpText: { + id: 'course-authoriong.video-uploads.file-info.transcriptTab.uploadFile.helpText', + defaultMessage: 'SRT file, max 25MB', + description: 'Help text shown under transcript upload button', + }, + languageSelectPlaceholder: { + id: 'course-authoriong.video-uploads.file-info.transcriptTab.languageSelectPlaceholder', + defaultMessage: 'Select language', + description: 'Placeholder for language selector in new transcript form', + }, + cancelButtonLabel: { + id: 'course-authoriong.video-uploads.file-info.transcriptTab.cancel.label', + defaultMessage: 'Cancel', + description: 'Cancel button text in new transcript form', + }, + addTranscriptButtonLabel: { + id: 'course-authoriong.video-uploads.file-info.transcriptTab.addTranscript.label', + defaultMessage: 'Add transcript', + description: 'Submit button label in new transcript form', + }, + removeSelectedFileLabel: { + id: 'course-authoriong.video-uploads.file-info.transcriptTab.removeSelectedFile.label', + defaultMessage: 'Remove selected file', + description: 'Accessible label for removing selected transcript file', + }, + addTranscriptFailedLabel: { + id: 'course-authoriong.video-uploads.file-info.transcriptTab.addTranscript.failed', + defaultMessage: 'Upload failed, please try again', + description: 'Inline error shown when adding transcript fails', + }, + invalidSrtFormat: { + id: 'course-authoriong.video-uploads.file-info.transcriptTab.invalidSrtFormat', + defaultMessage: 'Invalid SRT format. Please upload a valid .srt file.', + description: 'Error shown when uploaded transcript file has invalid SRT content', + }, + newTranscriptAddedLabel: { + id: 'course-authoriong.video-uploads.file-info.transcriptTab.addTranscript.success', + defaultMessage: 'New transcript added', + description: 'Toast message shown when adding transcript succeeds', + }, }); export default messages; diff --git a/src/files-and-videos/videos-page/info-sidebar/transcript-item/LanguageSelect.jsx b/src/files-and-videos/videos-page/info-sidebar/transcript-item/LanguageSelect.jsx index e1a57cd9cf..d26b20a5b1 100644 --- a/src/files-and-videos/videos-page/info-sidebar/transcript-item/LanguageSelect.jsx +++ b/src/files-and-videos/videos-page/info-sidebar/transcript-item/LanguageSelect.jsx @@ -1,15 +1,23 @@ import React, { useState } from 'react'; import PropTypes from 'prop-types'; +import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; import { Button, + Form, Icon, ModalPopup, Menu, MenuItem, useToggle, } from '@openedx/paragon'; -import { Check, ExpandMore, ExpandLess } from '@openedx/paragon/icons'; +import messages from './messages'; +import { + Check, + ExpandMore, + ExpandLess, + Search, +} from '@openedx/paragon/icons'; import { isEmpty } from 'lodash'; const LanguageSelect = ({ @@ -18,39 +26,68 @@ const LanguageSelect = ({ options, handleSelect, placeholderText, + className = 'col-9 p-0', }) => { + const intl = useIntl(); const currentSelection = isEmpty(value) ? placeholderText : options[value]; const [isOpen, , close, toggle] = useToggle(); const [target, setTarget] = useState(null); + const [searchQuery, setSearchQuery] = useState(''); + + const normalizedQuery = searchQuery.trim().toLowerCase(); + const filteredOptions = Object.entries(options).filter(([, text]) => ( + text?.toLowerCase().includes(normalizedQuery) + )); + + const handleClose = () => { + setSearchQuery(''); + close(); + }; return ( <> -
+
- {Object.entries(options).map(([valueKey, text]) => { +
+ setSearchQuery(e.target.value)} + placeholder={intl.formatMessage(messages.searchLanguagesPlaceholder)} + controlClassName="w-100" + floatingLabel={null} + /> +
+ {filteredOptions.length === 0 && ( +
+ +
+ )} + {filteredOptions.map(([valueKey, text]) => { if (valueKey === value) { return ( { handleSelect(valueKey); - close(); + handleClose(); }} key={`${valueKey}-item`} > @@ -95,7 +132,7 @@ const LanguageSelect = ({
- +
@@ -108,6 +145,7 @@ LanguageSelect.propTypes = { handleSelect: PropTypes.func.isRequired, placeholderText: PropTypes.string.isRequired, previousSelection: PropTypes.arrayOf(PropTypes.string).isRequired, + className: PropTypes.string, }; export default LanguageSelect; diff --git a/src/files-and-videos/videos-page/info-sidebar/transcript-item/LanguageSelect.scss b/src/files-and-videos/videos-page/info-sidebar/transcript-item/LanguageSelect.scss index 9e893d971f..09127601fc 100644 --- a/src/files-and-videos/videos-page/info-sidebar/transcript-item/LanguageSelect.scss +++ b/src/files-and-videos/videos-page/info-sidebar/transcript-item/LanguageSelect.scss @@ -1,3 +1,22 @@ +.language-select-dropdown-btn { + border-radius: 6px !important; + border-color: var(--pgn-color-gray-500) !important; + + &__text { + flex: 1; + text-align: left; + } + + &__divider { + display: inline-block; + width: 1px; + height: 1rem; + background-color: var(--pgn-color-gray-500); + margin: 0 .5rem; + align-self: center; + } +} + .language-select { padding: 8px 0; margin: 0; diff --git a/src/files-and-videos/videos-page/info-sidebar/transcript-item/Transcript.jsx b/src/files-and-videos/videos-page/info-sidebar/transcript-item/Transcript.jsx index 77301d7010..4b1feba4c8 100644 --- a/src/files-and-videos/videos-page/info-sidebar/transcript-item/Transcript.jsx +++ b/src/files-and-videos/videos-page/info-sidebar/transcript-item/Transcript.jsx @@ -1,29 +1,38 @@ import React, { useEffect, useState } from 'react'; import PropTypes from 'prop-types'; import { - Card, + ActionRow, + AlertModal, Button, Icon, IconButton, + Stack, useToggle, } from '@openedx/paragon'; -import { DeleteOutline } from '@openedx/paragon/icons'; +import { DeleteOutline, Error as ErrorIcon } from '@openedx/paragon/icons'; import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; import { isEmpty } from 'lodash'; import LanguageSelect from './LanguageSelect'; import TranscriptMenu from './TranscriptMenu'; import messages from './messages'; +import sidebarMessages from '../messages'; import { FileInput, useFileInput } from '../../../generic'; +import TranscriptEditor from '../../transcript-editor'; +import { isValidSrt } from '../../transcript-editor/srtUtils'; const Transcript = ({ languages, transcript, previousSelection, handleTranscript, + video, + transcriptSettings, }) => { const intl = useIntl(); const [isConfirmationOpen, openConfirmation, closeConfirmation] = useToggle(); const [newLanguage, setNewLanguage] = useState(transcript); + const [editorLanguage, setEditorLanguage] = useState(''); + const [invalidSrtFile, setInvalidSrtFile] = useState(false); const language = transcript; useEffect(() => { @@ -33,11 +42,23 @@ const Transcript = ({ const input = useFileInput({ onAddFile: (files) => { const [file] = files; - handleTranscript({ - file, - language, - newLanguage, - }, 'upload'); + if (!file) { + return; + } + const reader = new FileReader(); + reader.onload = (e) => { + if (!isValidSrt(e.target.result)) { + setInvalidSrtFile(true); + } else { + setInvalidSrtFile(false); + handleTranscript({ + file, + language, + newLanguage, + }, 'upload'); + } + }; + reader.readAsText(file); }, setSelectedRows: () => {}, setAddOpen: () => {}, @@ -52,70 +73,81 @@ const Transcript = ({ return ( <> - {isConfirmationOpen ? - ( - - } /> - - - - - - - - - - - ) : - ( -
- + + {transcript === '' ? + ( + - {transcript === '' ? - ( - - ) : - ( - - )} -
- )} + ) : + ( + + )} +
+ {invalidSrtFile && ( + + + {intl.formatMessage(sidebarMessages.invalidSrtFormat)} + + )} + } + isOpen={isConfirmationOpen} + onClose={closeConfirmation} + footerNode={ + + + + + } + > +

+ +

+ + setEditorLanguage('')} + video={video} + language={editorLanguage} + languages={languages} + transcriptSettings={transcriptSettings} + /> ); }; @@ -125,6 +157,15 @@ Transcript.propTypes = { transcript: PropTypes.string.isRequired, previousSelection: PropTypes.arrayOf(PropTypes.string).isRequired, handleTranscript: PropTypes.func.isRequired, + video: PropTypes.shape({ + id: PropTypes.string.isRequired, + displayName: PropTypes.string.isRequired, + downloadLink: PropTypes.string, + }).isRequired, + transcriptSettings: PropTypes.shape({ + transcriptDownloadHandlerUrl: PropTypes.string.isRequired, + transcriptUploadHandlerUrl: PropTypes.string.isRequired, + }).isRequired, }; export default Transcript; diff --git a/src/files-and-videos/videos-page/info-sidebar/transcript-item/Transcript.test.tsx b/src/files-and-videos/videos-page/info-sidebar/transcript-item/Transcript.test.tsx new file mode 100644 index 0000000000..8071e75541 --- /dev/null +++ b/src/files-and-videos/videos-page/info-sidebar/transcript-item/Transcript.test.tsx @@ -0,0 +1,35 @@ +import { + fireEvent, + initializeMocks, + render, + screen, +} from '@src/testUtils'; +import Transcript from './Transcript'; + +describe('Transcript (new, empty row)', () => { + beforeEach(() => { + initializeMocks(); + }); + + it('opens the file picker once a language is chosen for the new transcript', () => { + const clickSpy = jest.spyOn(HTMLInputElement.prototype, 'click').mockImplementation(() => {}); + render( + , + ); + + fireEvent.click(screen.getByTestId('language-select-dropdown')); + fireEvent.click(screen.getByText('Arabic')); + expect(clickSpy).toHaveBeenCalled(); + clickSpy.mockRestore(); + }); +}); diff --git a/src/files-and-videos/videos-page/info-sidebar/transcript-item/TranscriptMenu.jsx b/src/files-and-videos/videos-page/info-sidebar/transcript-item/TranscriptMenu.jsx index 5f321691e8..e9ca341158 100644 --- a/src/files-and-videos/videos-page/info-sidebar/transcript-item/TranscriptMenu.jsx +++ b/src/files-and-videos/videos-page/info-sidebar/transcript-item/TranscriptMenu.jsx @@ -19,6 +19,7 @@ export const TranscriptActionMenu = ({ launchDeleteConfirmation, handleTranscript, input, + onEdit = () => {}, }) => { const [isOpen, , close, toggle] = useToggle(); const [target, setTarget] = useState(null); @@ -39,19 +40,30 @@ export const TranscriptActionMenu = ({ onClose={close} onEscapeKey={close} > - + { + onEdit(language); + close(); + }} + > + + + { + input.click(); + close(); + }} > handleTranscript({ language }, 'download')} > @@ -60,7 +72,6 @@ export const TranscriptActionMenu = ({ @@ -75,6 +86,7 @@ TranscriptActionMenu.propTypes = { language: PropTypes.string.isRequired, handleTranscript: PropTypes.func.isRequired, launchDeleteConfirmation: PropTypes.func.isRequired, + onEdit: PropTypes.func, input: PropTypes.shape({ click: PropTypes.func.isRequired, }).isRequired, diff --git a/src/files-and-videos/videos-page/info-sidebar/transcript-item/TranscriptMenu.test.tsx b/src/files-and-videos/videos-page/info-sidebar/transcript-item/TranscriptMenu.test.tsx new file mode 100644 index 0000000000..b790aaeef0 --- /dev/null +++ b/src/files-and-videos/videos-page/info-sidebar/transcript-item/TranscriptMenu.test.tsx @@ -0,0 +1,39 @@ +import { + fireEvent, + initializeMocks, + render, + screen, +} from '@src/testUtils'; +import { TranscriptActionMenu } from './TranscriptMenu'; +import messages from './messages'; + +describe('TranscriptActionMenu', () => { + beforeEach(() => { + initializeMocks(); + }); + + const renderMenu = (props = {}) => + render( + , + ); + + it('calls onEdit with the language and closes the menu', () => { + const onEdit = jest.fn(); + renderMenu({ onEdit }); + fireEvent.click(screen.getByRole('button', { name: 'Actions dropdown' })); + fireEvent.click(screen.getByText(messages.editTranscript.defaultMessage)); + expect(onEdit).toHaveBeenCalledWith('ar'); + }); + + it('tolerates a missing onEdit handler', () => { + renderMenu(); + fireEvent.click(screen.getByRole('button', { name: 'Actions dropdown' })); + expect(() => fireEvent.click(screen.getByText(messages.editTranscript.defaultMessage))).not.toThrow(); + }); +}); diff --git a/src/files-and-videos/videos-page/info-sidebar/transcript-item/messages.ts b/src/files-and-videos/videos-page/info-sidebar/transcript-item/messages.ts index d7d5e0d52e..e25303abe7 100644 --- a/src/files-and-videos/videos-page/info-sidebar/transcript-item/messages.ts +++ b/src/files-and-videos/videos-page/info-sidebar/transcript-item/messages.ts @@ -1,6 +1,11 @@ import { defineMessages } from '@edx/frontend-platform/i18n'; const messages = defineMessages({ + editTranscript: { + id: 'course-authoriong.video-uploads.file-info.transcript.editTranscript', + defaultMessage: 'Edit transcript', + description: 'Message Presented To user for action to edit transcript', + }, fileSizeError: { id: 'course-authoriong.video-uploads.file-info.transcript.error.fileSizeError', defaultMessage: 'Transcript file size exceeds the maximum. Please try again.', @@ -46,6 +51,21 @@ const messages = defineMessages({ defaultMessage: 'Delete this transcript?', description: 'Title for Warning which allows users to select next step in the process of deleting a transcript', }, + searchLanguagesPlaceholder: { + id: 'course-authoriong.video-uploads.file-info.transcript.searchLanguagesPlaceholder', + defaultMessage: 'Search languages', + description: 'Placeholder for the search box in the transcript language selector', + }, + noLanguageResults: { + id: 'course-authoriong.video-uploads.file-info.transcript.noLanguageResults', + defaultMessage: 'No results', + description: 'Shown in the transcript language selector when no languages match the search', + }, + deleteEmptyTranscriptLabel: { + id: 'course-authoriong.video-uploads.file-info.transcript.deleteEmptyTranscript', + defaultMessage: 'Delete empty transcript', + description: 'Accessible label for the button that removes an empty transcript row', + }, }); export default messages; diff --git a/src/files-and-videos/videos-page/transcript-editor/TranscriptEditor.scss b/src/files-and-videos/videos-page/transcript-editor/TranscriptEditor.scss new file mode 100644 index 0000000000..cf282811b1 --- /dev/null +++ b/src/files-and-videos/videos-page/transcript-editor/TranscriptEditor.scss @@ -0,0 +1,218 @@ +.transcript-editor-modal { + &__video-container { + background: var(--pgn-color-black); + height: clamp(8rem, 40vh, 32rem); + } + + &__video { + display: block; + width: 100%; + max-height: 100%; + max-width: 100%; + background: var(--pgn-color-black); + object-fit: contain; + } + + &.pgn__modal { + width: 80vw; + max-width: 80vw; + height: 95vh; + max-height: 95vh; + overflow: hidden; + + .pgn__modal-header, + .pgn__modal-footer { + flex: 0 0 auto; + } + } + + .pgn__modal-body { + flex: 1 1 auto; + min-height: 0; + } + + .pgn__modal-body-content { + display: flex; + flex: 1 1 auto; + flex-direction: column; + min-height: 0; + padding-left: 0; + padding-right: 0; + padding-bottom: 0; + } + + &__cue-list { + min-height: 0; + overscroll-behavior: contain; + scroll-behavior: smooth; + } + + &__cue-wrapper { + border-left: 3px solid transparent; + border-radius: .375rem; + transition: background-color .15s ease, border-color .15s ease, box-shadow .15s ease; + transition: border-color .15s ease; + + &.transcript-editor-modal__cue--active { + background: linear-gradient(90deg, rgb(0 117 180 / .08) 0%, transparent 100%); + border-left: 3px solid var(--pgn-color-info-500); + border-radius: .25rem; + box-shadow: 0 0 0 2px rgb(0 117 180 / .2); + + .transcript-editor-modal__cue-text.form-control { + border-color: var(--pgn-color-info-500) !important; + background: var(--pgn-color-white) !important; + color: var(--pgn-color-gray-900); + font-weight: 500; + } + + .transcript-editor-modal__time.form-control { + border-color: var(--pgn-color-info-500); + color: var(--pgn-color-gray-900); + } + } + } + + &__cue-row { + flex-wrap: nowrap; + align-items: center; + } + + &__cue-text-wrap { + flex: 1 1 auto; + min-width: 0; + position: relative; + + .pgn__form-control-feedback { + position: absolute; + top: 100%; + left: 0; + font-size: .75rem; + white-space: nowrap; + margin-top: .1rem; + } + } + + &__cue-text.form-control { + resize: none; + font-size: .875rem; + border-radius: .25rem; + padding: .25rem .4rem; + border: 1px solid transparent !important; + background: transparent !important; + box-shadow: none !important; + transition: border-color .15s ease, background-color .15s ease, box-shadow .15s ease; + + &:hover { + border: 1px solid var(--pgn-color-gray-300) !important; + background: transparent !important; + box-shadow: none !important; + } + + &:focus { + border: 1px solid var(--pgn-color-info-500) !important; + background: var(--pgn-color-white) !important; + box-shadow: 0 0 0 2px rgb(0 117 180 / .2) !important; + outline: none !important; + } + + &.is-invalid { + border: 1px solid var(--pgn-color-danger-500) !important; + background: var(--pgn-color-danger-100) !important; + box-shadow: none !important; + } + } + + &__time-wrap { + flex: 0 0 auto; + width: 8rem; + + .pgn__form-control-decorator-group { + width: 100%; + } + } + + &__time.form-control { + width: 100%; + font-family: ui-monospace, SFMono-Regular, Menlo, Consolas, monospace; + font-size: .65rem; + padding: .15rem .25rem; + height: auto; + border: 1px solid var(--pgn-color-gray-300); + border-radius: .25rem; + color: var(--pgn-color-gray-700); + background: var(--pgn-color-white); + + &:focus { + outline: none; + border-color: var(--pgn-color-info-500); + box-shadow: 0 0 0 2px rgb(0 117 180 / .2); + } + } + + &__time-sep { + color: var(--pgn-color-gray-500); + flex-shrink: 0; + } + + &__cue-row .btn-icon { + flex-shrink: 0; + } + + &__insert-divider { + display: flex; + align-items: center; + justify-content: center; + position: relative; + width: 100%; + height: 0; + visibility: hidden; + transition: height .15s ease; + } + + &__insert-divider-line { + width: 100%; + position: absolute; + margin: 0; + border: 0; + border-top: 1px dashed var(--pgn-color-light-500); + z-index: 0; + } + + &__insert-btn.btn { + background: var(--pgn-color-white); + border: 1px dashed var(--pgn-color-gray-500); + color: var(--pgn-color-gray-700); + font-size: .75rem; + padding: .15rem .6rem; + border-radius: 999px; + position: relative; + z-index: 1; + } + + &__cue-wrapper:hover &__insert-divider { + height: 2rem; + visibility: visible; + } + + @media (width <= 767.98px) { + &.pgn__modal { + width: calc(100vw - 1rem); + max-width: calc(100vw - 1rem); + height: calc(100vh - .5rem); + max-height: calc(100vh - .5rem); + margin: 0; + } + } + + @media (width <= 991.98px) { + &__cue-text-wrap { + flex-basis: 100%; + } + + &__time-wrap { + flex: 1 1 7.5rem; + max-width: 9rem; + } + } +} diff --git a/src/files-and-videos/videos-page/transcript-editor/TranscriptEditor.test.tsx b/src/files-and-videos/videos-page/transcript-editor/TranscriptEditor.test.tsx new file mode 100644 index 0000000000..859318454d --- /dev/null +++ b/src/files-and-videos/videos-page/transcript-editor/TranscriptEditor.test.tsx @@ -0,0 +1,403 @@ +import type MockAdapter from 'axios-mock-adapter'; +import { logError } from '@edx/frontend-platform/logging'; + +import { + act, + fireEvent, + initializeMocks, + render, + screen, + waitFor, +} from '@src/testUtils'; +import TranscriptEditor from './TranscriptEditor'; +import messages from './messages'; + +jest.mock('@edx/frontend-platform/logging', () => ({ + logError: jest.fn(), +})); + +const SRT = [ + '1', + '00:00:01,000 --> 00:00:02,000', + 'Hello', + '', + '2', + '00:00:03,000 --> 00:00:04,000', + 'World', +].join('\n'); + +const defaultProps = { + isOpen: true, + onClose: jest.fn(), + language: 'en', + languages: { en: 'English' }, + video: { + id: 'vid-1', + displayName: 'My Video', + downloadLink: 'https://cdn.example.com/video.mp4', + }, + transcriptSettings: { + transcriptDownloadHandlerUrl: '/transcript_download/', + transcriptUploadHandlerUrl: '/transcript_upload/', + }, +}; + +let axiosMock: MockAdapter; +let playSpy: jest.SpyInstance; + +const readFileText = (blob: Blob) => + new Promise((resolve, reject) => { + const reader = new FileReader(); + reader.onload = () => resolve(reader.result as string); + reader.onerror = () => reject(reader.error); + reader.readAsText(blob); + }); + +// Lets an in-flight request settle and React process the result, so assertions run +// after any state update the response would have triggered. +const flushPendingResponses = () => + act(async () => { + await new Promise((resolve) => { + setTimeout(resolve, 0); + }); + }); + +const renderEditor = (props = {}) => render(); + +const mockTranscript = (body: string = SRT) => { + axiosMock.onGet(/transcript_download/).reply(200, body); +}; + +beforeAll(() => { + // jsdom does not implement these; the component guards on their presence. + global.URL.createObjectURL = jest.fn(() => 'blob:mock-track'); + global.URL.revokeObjectURL = jest.fn(); + Element.prototype.scrollTo = jest.fn(); +}); + +beforeEach(() => { + ({ axiosMock } = initializeMocks()); + jest.clearAllMocks(); + playSpy = jest.spyOn(HTMLMediaElement.prototype, 'play').mockResolvedValue(undefined); +}); + +afterEach(() => { + playSpy.mockRestore(); + jest.useRealTimers(); +}); + +describe('TranscriptEditor', () => { + it('fetches the transcript for the video/language and renders its cues', async () => { + mockTranscript(); + renderEditor(); + + expect(await screen.findByDisplayValue('Hello')).toBeInTheDocument(); + expect(screen.getByDisplayValue('World')).toBeInTheDocument(); + expect(screen.getByText('English')).toBeInTheDocument(); + expect(axiosMock.history.get[0].url).toContain( + '/transcript_download/?edx_video_id=vid-1&language_code=en', + ); + }); + + it('shows a distinct error alert when the transcript fails to load', async () => { + axiosMock.onGet(/transcript_download/).reply(500); + renderEditor(); + + expect(await screen.findByText(messages.loadFailedLabel.defaultMessage)).toBeInTheDocument(); + expect(screen.queryByText(messages.saveFailedLabel.defaultMessage)).not.toBeInTheDocument(); + }); + + it('gives cue fields accessible names', async () => { + mockTranscript(); + renderEditor(); + await screen.findByDisplayValue('Hello'); + + expect(screen.getByRole('textbox', { name: 'Cue 1 text' })).toHaveValue('Hello'); + expect(screen.getByRole('textbox', { name: 'Cue 1 start time' })).toHaveValue('00:00:01,000'); + expect(screen.getByRole('textbox', { name: 'Cue 2 end time' })).toHaveValue('00:00:04,000'); + }); + + it('preserves multi-line cue text through editing and saving', async () => { + mockTranscript(); + axiosMock.onPost(/transcript_upload/).reply(200); + renderEditor(); + + const cueText = await screen.findByRole('textbox', { name: 'Cue 1 text' }); + expect(cueText.tagName).toBe('TEXTAREA'); + fireEvent.change(cueText, { target: { value: 'first line\nsecond line' } }); + expect(screen.getByRole('textbox', { name: 'Cue 1 text' })).toHaveValue('first line\nsecond line'); + + fireEvent.click(screen.getByRole('button', { name: messages.saveButtonLabel.defaultMessage })); + await waitFor(() => expect(axiosMock.history.post).toHaveLength(1)); + const file = (axiosMock.history.post[0].data as FormData).get('file') as File; + expect(await readFileText(file)).toContain('00:00:01,000 --> 00:00:02,000\nfirst line\nsecond line'); + }); + + it('saves edited cues as a re-uploaded SRT file and shows the saved indicator', async () => { + mockTranscript(); + axiosMock.onPost(/transcript_upload/).reply(200); + renderEditor(); + + fireEvent.change(await screen.findByDisplayValue('Hello'), { target: { value: 'Hello edited' } }); + fireEvent.click(screen.getByRole('button', { name: messages.saveButtonLabel.defaultMessage })); + + await waitFor(() => expect(axiosMock.history.post).toHaveLength(1)); + const formData = axiosMock.history.post[0].data as FormData; + expect(formData.get('edx_video_id')).toBe('vid-1'); + expect(formData.get('language_code')).toBe('en'); + expect(formData.get('new_language_code')).toBe('en'); + const file = formData.get('file') as File; + expect(file.name).toBe('My Video-en.srt'); + expect(await readFileText(file)).toContain('Hello edited'); + + expect(await screen.findByText(messages.savedLabel.defaultMessage)).toBeInTheDocument(); + + // The "Saved" indicator clears itself after five seconds. + await waitFor( + () => expect(screen.queryByText(messages.savedLabel.defaultMessage)).not.toBeInTheDocument(), + { timeout: 6000 }, + ); + }, 15000); + + it('surfaces save failures and logs the error', async () => { + mockTranscript(); + axiosMock.onPost(/transcript_upload/).reply(500); + renderEditor(); + + fireEvent.change(await screen.findByDisplayValue('Hello'), { target: { value: 'Hello edited' } }); + fireEvent.click(screen.getByRole('button', { name: messages.saveButtonLabel.defaultMessage })); + + expect(await screen.findByText(messages.saveFailedLabel.defaultMessage)).toBeInTheDocument(); + expect(logError).toHaveBeenCalled(); + expect(screen.getByRole('button', { name: messages.saveButtonLabel.defaultMessage })).toBeEnabled(); + }); + + it('closes immediately when there are no unsaved changes', async () => { + const onClose = jest.fn(); + mockTranscript(); + renderEditor({ onClose }); + + await screen.findByDisplayValue('Hello'); + fireEvent.click(screen.getByRole('button', { name: messages.cancelButtonLabel.defaultMessage })); + + expect(onClose).toHaveBeenCalledWith(false); + expect(screen.queryByText(messages.unsavedModalTitle.defaultMessage)).not.toBeInTheDocument(); + }); + + it('asks for confirmation before discarding unsaved changes', async () => { + const onClose = jest.fn(); + mockTranscript(); + renderEditor({ onClose }); + + fireEvent.change(await screen.findByDisplayValue('Hello'), { target: { value: 'Changed' } }); + fireEvent.click(screen.getByRole('button', { name: messages.cancelButtonLabel.defaultMessage })); + + expect(await screen.findByText(messages.unsavedModalDescription.defaultMessage)).toBeInTheDocument(); + + fireEvent.click(screen.getByRole('button', { name: messages.keepEditingButtonLabel.defaultMessage })); + expect(onClose).not.toHaveBeenCalled(); + + fireEvent.click(screen.getByRole('button', { name: messages.cancelButtonLabel.defaultMessage })); + fireEvent.click(await screen.findByRole('button', { name: messages.closeEditorButtonLabel.defaultMessage })); + expect(onClose).toHaveBeenCalledWith(false); + }); + + it('inserts a first cue into an empty transcript and flags empty text as invalid', async () => { + mockTranscript(''); + renderEditor(); + + fireEvent.click(await screen.findByRole('button', { name: messages.insertCueLabel.defaultMessage })); + + expect(screen.getByDisplayValue('00:00:00,000')).toBeInTheDocument(); + expect(screen.getByDisplayValue('00:00:02,000')).toBeInTheDocument(); + expect(screen.getByText(messages.invalidCueTextLabel.defaultMessage)).toBeInTheDocument(); + expect(screen.getByRole('button', { name: messages.saveButtonLabel.defaultMessage })).toBeDisabled(); + }); + + it('inserts a cue after an existing one and deletes cues', async () => { + mockTranscript(); + renderEditor(); + await screen.findByDisplayValue('Hello'); + + fireEvent.click(screen.getAllByRole('button', { name: messages.insertCueLabel.defaultMessage })[0]); + // New cue starts where the first cue ends, capped at the next cue's start. + expect(screen.getAllByDisplayValue('00:00:02,000')).toHaveLength(2); + expect(screen.getAllByDisplayValue('00:00:03,000')).toHaveLength(2); + + const deleteButtons = screen.getAllByRole('button', { name: messages.deleteCueLabel.defaultMessage }); + expect(deleteButtons).toHaveLength(3); + fireEvent.click(deleteButtons[1]); + expect(screen.getAllByRole('button', { name: messages.deleteCueLabel.defaultMessage })).toHaveLength(2); + }); + + it('normalizes timestamp edits on blur and flags invalid ranges', async () => { + mockTranscript(); + renderEditor(); + await screen.findByDisplayValue('Hello'); + + const start = screen.getByDisplayValue('00:00:03,000'); + fireEvent.change(start, { target: { value: '00:00:03.500' } }); + // Dots are normalized to commas while typing... + expect(screen.getByDisplayValue('00:00:03,500')).toBeInTheDocument(); + fireEvent.blur(screen.getByDisplayValue('00:00:03,500'), { target: { value: '00:00:03,500' } }); + expect(screen.getByDisplayValue('00:00:03,500')).toBeInTheDocument(); + + // ...and an end time before the start time is flagged, disabling Save. + const end = screen.getByDisplayValue('00:00:04,000'); + fireEvent.change(end, { target: { value: '00:00:01,000' } }); + expect(screen.getByText(messages.invalidTimestampLabel.defaultMessage)).toBeInTheDocument(); + expect(screen.getByRole('button', { name: messages.saveButtonLabel.defaultMessage })).toBeDisabled(); + }); + + it('seeks the preview on cue play, ignoring interrupted play() but logging other errors', async () => { + mockTranscript(); + renderEditor(); + await screen.findByDisplayValue('Hello'); + + const abortError = new Error('interrupted'); + abortError.name = 'AbortError'; + playSpy.mockRejectedValueOnce(abortError); + + const seekButtons = screen.getAllByRole('button', { name: messages.seekCueLabel.defaultMessage }); + fireEvent.click(seekButtons[0]); + await waitFor(() => expect(playSpy).toHaveBeenCalledTimes(1)); + expect(logError).not.toHaveBeenCalled(); + // Seeking into the first cue's range marks it active. + expect(document.querySelector('.transcript-editor-modal__cue--active')).not.toBeNull(); + + playSpy.mockRejectedValueOnce(new Error('autoplay blocked')); + fireEvent.click(seekButtons[1]); + await waitFor(() => expect(logError).toHaveBeenCalledTimes(1)); + }); + it('leaves invalid timestamps unchanged on blur, for both start and end fields', async () => { + mockTranscript(); + renderEditor(); + await screen.findByDisplayValue('Hello'); + + const start = screen.getByRole('textbox', { name: 'Cue 1 start time' }); + fireEvent.change(start, { target: { value: '00:00:01,5' } }); + fireEvent.blur(start, { target: { value: '00:00:01,5' } }); + expect(start).toHaveValue('00:00:01,5'); + + const end = screen.getByRole('textbox', { name: 'Cue 1 end time' }); + fireEvent.change(end, { target: { value: '00:00:02,5' } }); + fireEvent.blur(end, { target: { value: '00:00:02,5' } }); + expect(end).toHaveValue('00:00:02,5'); + }); + + it('updates the active cue from video timeupdate events', async () => { + mockTranscript(); + renderEditor(); + await screen.findByDisplayValue('Hello'); + + const video = document.querySelector('video') as HTMLVideoElement; + Object.defineProperty(video, 'currentTime', { value: 1.5, configurable: true }); + fireEvent.timeUpdate(video); + await waitFor(() => expect(document.querySelector('.transcript-editor-modal__cue--active')).not.toBeNull()); + }); + + it('does not close while a save is in progress', async () => { + mockTranscript(); + let finishSave: (value: [number]) => void = () => {}; + axiosMock.onPost(/transcript_upload/).reply(() => + new Promise<[number]>((resolve) => { + finishSave = resolve; + }) + ); + const onClose = jest.fn(); + renderEditor({ onClose }); + + fireEvent.change(await screen.findByDisplayValue('Hello'), { target: { value: 'Hello edited' } }); + fireEvent.click(screen.getByRole('button', { name: messages.saveButtonLabel.defaultMessage })); + await screen.findAllByText(messages.saveInProgressLabel.defaultMessage); + + fireEvent.click(screen.getByRole('button', { name: 'Close' })); + expect(onClose).not.toHaveBeenCalled(); + + finishSave([200]); + expect(await screen.findByText(messages.savedLabel.defaultMessage)).toBeInTheDocument(); + }); + + it('falls back to setting scrollTop when scrollTo is unavailable', async () => { + const original = Element.prototype.scrollTo; + Object.defineProperty(Element.prototype, 'scrollTo', { value: undefined, configurable: true, writable: true }); + try { + mockTranscript(); + renderEditor(); + await screen.findByDisplayValue('Hello'); + fireEvent.click(screen.getAllByRole('button', { name: messages.seekCueLabel.defaultMessage })[0]); + await waitFor(() => expect(document.querySelector('.transcript-editor-modal__cue--active')).not.toBeNull()); + } finally { + Object.defineProperty(Element.prototype, 'scrollTo', { value: original, configurable: true, writable: true }); + } + }); + + // The fetch effect guards every state update behind an isMounted flag, so a response + // that lands after the editor closes must neither render nor report anything. + it.each([ + ['a transcript', () => mockTranscript()], + ['a load failure', () => { + axiosMock.onGet(/transcript_download/).reply(500); + }], + ])('ignores %s that arrives after the editor unmounted', async (_label, mockResponse) => { + mockResponse(); + const { unmount } = renderEditor(); + unmount(); + await waitFor(() => expect(axiosMock.history.get).toHaveLength(1)); + + await flushPendingResponses(); + + expect(screen.queryByDisplayValue('Hello')).not.toBeInTheDocument(); + expect(screen.queryByText(messages.loadFailedLabel.defaultMessage)).not.toBeInTheDocument(); + expect(logError).not.toHaveBeenCalled(); + }); + + it('closes the unsaved-changes dialog with Escape', async () => { + mockTranscript(); + renderEditor(); + fireEvent.change(await screen.findByDisplayValue('Hello'), { target: { value: 'Changed' } }); + fireEvent.click(screen.getByRole('button', { name: messages.cancelButtonLabel.defaultMessage })); + const description = await screen.findByText(messages.unsavedModalDescription.defaultMessage); + + fireEvent.keyDown(description, { key: 'Escape', code: 'Escape', keyCode: 27 }); + await waitFor(() => + expect(screen.queryByText(messages.unsavedModalDescription.defaultMessage)).not.toBeInTheDocument() + ); + }); + it('inserts after the last cue using a default two-second window', async () => { + mockTranscript(); + renderEditor(); + await screen.findByDisplayValue('Hello'); + + const insertButtons = screen.getAllByRole('button', { name: messages.insertCueLabel.defaultMessage }); + fireEvent.click(insertButtons[insertButtons.length - 1]); + // Last cue ends at 00:00:04,000, so the new cue spans 04,000 -> 06,000. + expect(screen.getAllByDisplayValue('00:00:04,000')).toHaveLength(2); + expect(screen.getByDisplayValue('00:00:06,000')).toBeInTheDocument(); + }); + + it('falls back to the language code when no display name is configured', async () => { + mockTranscript(); + renderEditor({ languages: {} }); + await screen.findByDisplayValue('Hello'); + expect(screen.getByText('en')).toBeInTheDocument(); + }); + + it('renders without fetching when no language is selected', () => { + renderEditor({ language: undefined, languages: undefined }); + expect(axiosMock.history.get).toHaveLength(0); + expect(screen.getByRole('button', { name: messages.insertCueLabel.defaultMessage })).toBeInTheDocument(); + }); + it('uses a plain two-second window when the next cue starts before the current one ends', async () => { + mockTranscript(); + renderEditor(); + await screen.findByDisplayValue('Hello'); + + // Push cue 1's end past cue 2's start (00:00:03,000), then insert after cue 1. + const end = screen.getByRole('textbox', { name: 'Cue 1 end time' }); + fireEvent.change(end, { target: { value: '00:00:05,000' } }); + fireEvent.click(screen.getAllByRole('button', { name: messages.insertCueLabel.defaultMessage })[0]); + expect(screen.getAllByDisplayValue('00:00:05,000')).toHaveLength(2); + expect(screen.getByDisplayValue('00:00:07,000')).toBeInTheDocument(); + }); +}); diff --git a/src/files-and-videos/videos-page/transcript-editor/TranscriptEditor.tsx b/src/files-and-videos/videos-page/transcript-editor/TranscriptEditor.tsx new file mode 100644 index 0000000000..a908d0f863 --- /dev/null +++ b/src/files-and-videos/videos-page/transcript-editor/TranscriptEditor.tsx @@ -0,0 +1,660 @@ +import { + useEffect, + useMemo, + useRef, + useState, +} from 'react'; +import type { SyntheticEvent } from 'react'; +import { logError } from '@edx/frontend-platform/logging'; +import { useIntl } from '@edx/frontend-platform/i18n'; +import { + Alert, + Button, + Form, + Icon, + IconButton, + ModalDialog, + Spinner, + Stack, +} from '@openedx/paragon'; +import { + Add, + CheckCircle, + DeleteOutline, + Info, + PlayArrow, +} from '@openedx/paragon/icons'; + +import { fetchTranscriptContent, uploadTranscript } from '@src/files-and-videos/videos-page/data/api'; +import { + formatTimestamp, + parseSrt, + parseTimestamp, + serializeSrt, +} from './srtUtils'; +import type { Cue } from './srtUtils'; +import messages from './messages'; +import './TranscriptEditor.scss'; + +type EditorCue = Cue & { id: string; }; +type SaveStatus = 'idle' | 'saving' | 'saved'; + +interface TranscriptEditorProps { + isOpen: boolean; + onClose: (submitted: boolean) => void; + video: { + id: string; + displayName: string; + downloadLink?: string | null; + }; + language?: string; + languages?: Record; + transcriptSettings: { + transcriptDownloadHandlerUrl: string; + transcriptUploadHandlerUrl: string; + }; +} + +const toVttTimestamp = (timestamp: string) => timestamp.replace(',', '.'); +const TIMESTAMP_REGEX = /^\d{2}:\d{2}:\d{2},\d{3}$/; +const hasInvalidCueText = (text = '') => { + const lines = text.split(/\r?\n/); + return text.trim().length === 0 || lines.some((line) => line.trim().length === 0); +}; + +const serializeVtt = (cues: Cue[]) => { + const body = cues + .map((cue, index) => + `${index + 1}\n${toVttTimestamp(cue.startTime)} --> ${toVttTimestamp(cue.endTime)}\n${cue.text}` + ) + .join('\n\n'); + + return `WEBVTT\n\n${body}`; +}; + +const TranscriptEditor = ({ + isOpen, + onClose, + video, + language = '', + languages = {}, + transcriptSettings, +}: TranscriptEditorProps) => { + const intl = useIntl(); + const videoRef = useRef(null); + const cueIdRef = useRef(0); + const cueListRef = useRef(null); + const cueWrapperRefs = useRef>({}); + const pendingFocusId = useRef(null); + + const [loading, setLoading] = useState(false); + const [saving, setSaving] = useState(false); + const [saveError, setSaveError] = useState(''); + const [cues, setCues] = useState([]); + const [captionTrackUrl, setCaptionTrackUrl] = useState(''); + const [currentTime, setCurrentTime] = useState(0); + const [initialCuesSnapshot, setInitialCuesSnapshot] = useState('[]'); + const [isUnsavedModalOpen, setIsUnsavedModalOpen] = useState(false); + const [saveStatus, setSaveStatus] = useState('idle'); + + const attachCueIds = (nextCues: (Cue & { id?: string; })[]): EditorCue[] => + nextCues.map((cue) => ({ + ...cue, + id: cue.id ?? `cue-${cueIdRef.current++}`, + })); + + const serializeComparableCues = (nextCues: Cue[]) => + JSON.stringify(nextCues.map((cue) => ({ + startTime: cue.startTime, + endTime: cue.endTime, + text: cue.text, + }))); + + const hasUnsavedChanges = useMemo( + () => serializeComparableCues(cues) !== initialCuesSnapshot, + [cues, initialCuesSnapshot], + ); + + const hasInvalidCueTextInEditor = useMemo( + () => cues.some((cue) => hasInvalidCueText(cue.text)), + [cues], + ); + + const hasInvalidTimestamp = (cue: Cue) => ( + !TIMESTAMP_REGEX.test(cue.startTime) + || !TIMESTAMP_REGEX.test(cue.endTime) + || parseTimestamp(cue.endTime) <= parseTimestamp(cue.startTime) + ); + + const hasInvalidTimestampsInEditor = useMemo( + () => cues.some(hasInvalidTimestamp), + [cues], + ); + + const activeCueId = useMemo(() => { + const activeCue = cues.find((cue) => ( + TIMESTAMP_REGEX.test(cue.startTime) + && TIMESTAMP_REGEX.test(cue.endTime) + && currentTime >= parseTimestamp(cue.startTime) + && currentTime < parseTimestamp(cue.endTime) + )); + + return activeCue?.id || null; + }, [cues, currentTime]); + + useEffect(() => { + if (pendingFocusId.current) { + const wrapper = cueWrapperRefs.current[pendingFocusId.current]; + const el = wrapper?.querySelector('textarea'); + if (el) { + el.focus(); + pendingFocusId.current = null; + } + } + }, [cues]); + + useEffect(() => { + const cueList = cueListRef.current; + const activeCue = activeCueId ? cueWrapperRefs.current[activeCueId] : null; + + if (!cueList || !activeCue) { + return; + } + + const cueListRect = cueList.getBoundingClientRect(); + const activeCueRect = activeCue.getBoundingClientRect(); + const nextScrollTop = cueList.scrollTop + + activeCueRect.top + - cueListRect.top + - ((cueListRect.height - activeCueRect.height) / 2); + + if (typeof cueList.scrollTo === 'function') { + cueList.scrollTo({ + top: Math.max(0, nextScrollTop), + behavior: 'smooth', + }); + } else { + cueList.scrollTop = Math.max(0, nextScrollTop); + } + }, [activeCueId]); + + const inlineErrorMessage = saveError; + + useEffect(() => { + if (!isOpen || !language) { + return undefined; + } + + let isMounted = true; + setLoading(true); + setSaveError(''); + setIsUnsavedModalOpen(false); + setCurrentTime(0); + setSaveStatus('idle'); + + fetchTranscriptContent({ + videoId: video.id, + language, + apiUrl: transcriptSettings.transcriptDownloadHandlerUrl, + }) + .then((text: string) => { + if (!isMounted) { + return undefined; + } + const parsedCues = parseSrt(text); + setCues(attachCueIds(parsedCues)); + setInitialCuesSnapshot(serializeComparableCues(parsedCues)); + return undefined; + }) + .catch(() => { + if (!isMounted) { + return undefined; + } + setCues([]); + setInitialCuesSnapshot('[]'); + setSaveError(intl.formatMessage(messages.loadFailedLabel)); + return undefined; + }) + .finally(() => { + if (isMounted) { + setLoading(false); + } + }); + + return () => { + isMounted = false; + }; + }, [ + isOpen, + language, + video.id, + transcriptSettings.transcriptDownloadHandlerUrl, + intl, + ]); + + const handleCueChange = (index: number, value: string) => { + setCues(prev => + prev.map((cue, cueIndex) => ( + cueIndex === index ? { ...cue, text: value } : cue + )) + ); + }; + + const handleCueTimeChange = (index: number, field: 'startTime' | 'endTime', value: string) => { + const normalized = value + .replace(/\./g, ',') + .replace(/[^\d:,]/g, '') + .slice(0, 12); + + setCues(prev => + prev.map((cue, cueIndex) => ( + cueIndex === index ? { ...cue, [field]: normalized } : cue + )) + ); + }; + + const handleCueTimeBlur = (index: number, field: 'startTime' | 'endTime', value: string) => { + const normalized = value.replace(/\./g, ',').trim(); + if (!TIMESTAMP_REGEX.test(normalized)) { + return; + } + + setCues(prev => + prev.map((cue, cueIndex) => ( + cueIndex === index + ? { ...cue, [field]: formatTimestamp(parseTimestamp(normalized)) } + : cue + )) + ); + }; + + const handleDeleteCue = (index: number) => { + setCues(prev => prev.filter((_, cueIndex) => cueIndex !== index)); + }; + + const handleInsertCueAfter = (index: number) => { + setCues((prev) => { + // index === -1 means insert as the very first cue (empty list) + if (index === -1) { + const newId = `cue-${cueIdRef.current++}`; + pendingFocusId.current = newId; + return [{ + id: newId, + startTime: formatTimestamp(0), + endTime: formatTimestamp(2), + text: '', + }]; + } + + const currentCue = prev[index]; + /* istanbul ignore if -- insert buttons always pass a valid index */ + if (!currentCue) { + return prev; + } + + const currentEnd = parseTimestamp(currentCue.endTime); + const nextStart = prev[index + 1] + ? parseTimestamp(prev[index + 1].startTime) + : currentEnd + 2; + + const insertedStart = currentEnd; + const insertedEnd = nextStart > currentEnd ? Math.min(currentEnd + 2, nextStart) : currentEnd + 2; + + const newId = `cue-${cueIdRef.current++}`; + pendingFocusId.current = newId; + const newCue = { + id: newId, + startTime: formatTimestamp(insertedStart), + endTime: formatTimestamp(insertedEnd), + text: '', + }; + + const updated = [...prev]; + updated.splice(index + 1, 0, newCue); + return updated; + }); + }; + + const seekTo = (timeCode: string) => { + if (videoRef.current) { + videoRef.current.currentTime = parseTimestamp(timeCode); + const playPromise = videoRef.current.play(); + if (playPromise !== undefined) { + playPromise.catch((error: Error) => { + if (error.name !== 'AbortError') { + logError(error); + } + }); + } + setCurrentTime(parseTimestamp(timeCode)); + } + }; + + const handleVideoTimeUpdate = (event: SyntheticEvent) => { + setCurrentTime(event.currentTarget.currentTime); + }; + + useEffect(() => { + if (!cues.length) { + setCaptionTrackUrl(''); + return undefined; + } + + const vttBlob = new Blob([serializeVtt(cues)], { type: 'text/vtt' }); + const nextUrl = URL.createObjectURL(vttBlob); + setCaptionTrackUrl(nextUrl); + + return () => { + URL.revokeObjectURL(nextUrl); + }; + }, [cues]); + + useEffect(() => { + if (saveStatus !== 'saved') { + return undefined; + } + + const timerId = setTimeout(() => { + setSaveStatus('idle'); + }, 5000); + + return () => clearTimeout(timerId); + }, [saveStatus]); + + const handleSave = async () => { + const hasInvalidCueTextValue = cues.some((cue) => hasInvalidCueText(cue.text)); + /* istanbul ignore if -- the save button is disabled in this state */ + if (hasInvalidCueTextValue) { + return; + } + + setSaving(true); + setSaveStatus('saving'); + setSaveError(''); + + try { + const fileName = `${video.displayName}-${language}.srt`; + const file = new File([serializeSrt(cues)], fileName, { type: 'text/plain' }); + + await uploadTranscript({ + videoId: video.id, + language, + newLanguage: language, + apiUrl: transcriptSettings.transcriptUploadHandlerUrl, + file, + }); + + setInitialCuesSnapshot(serializeComparableCues(cues)); + setSaveStatus('saved'); + } catch (error) { + logError(error as Error); + setSaveStatus('idle'); + setSaveError(intl.formatMessage(messages.saveFailedLabel)); + } finally { + setSaving(false); + } + }; + + const handleRequestClose = () => { + if (saving) { + return; + } + + if (hasUnsavedChanges) { + setIsUnsavedModalOpen(true); + return; + } + + onClose(false); + }; + + return ( + + + +
+
+

{video.displayName}

+
{languages?.[language] || language}
+
+ {saveStatus === 'saving' && ( +
+ + {intl.formatMessage(messages.saveInProgressLabel)} +
+ )} + {saveStatus === 'saved' && ( +
+ + {intl.formatMessage(messages.savedLabel)} +
+ )} +
+ + + + + {inlineErrorMessage && ( + + {inlineErrorMessage} + + )} + {loading ? + ( +
+ +
+ ) : + ( + <> + {video.downloadLink && ( +
+ {/* eslint-disable-next-line jsx-a11y/media-has-caption */} + +
+ )} + +
+ {cues.length === 0 && ( +
+ +
+ )} + {cues.map((cue, index) => { + const isActive = activeCueId === cue.id; + const hasCueTextError = hasInvalidCueText(cue.text); + return ( + { + if (el) { + cueWrapperRefs.current[cue.id] = el; + } else { + delete cueWrapperRefs.current[cue.id]; + } + }} + gap={2} + className={`px-3 py-2 transcript-editor-modal__cue-wrapper${ + isActive ? ' transcript-editor-modal__cue--active' : '' + }`} + > + +
+ ) => + handleCueChange(index, e.target.value)} + className="transcript-editor-modal__cue-text" + isInvalid={hasCueTextError} + /> + {hasCueTextError && ( + + {intl.formatMessage(messages.invalidCueTextLabel)} + + )} +
+
+ ) => + handleCueTimeChange(index, 'startTime', e.target.value)} + onBlur={(e: React.FocusEvent) => + handleCueTimeBlur(index, 'startTime', e.target.value)} + /> +
+ → +
+ ) => + handleCueTimeChange(index, 'endTime', e.target.value)} + onBlur={(e: React.FocusEvent) => + handleCueTimeBlur(index, 'endTime', e.target.value)} + /> +
+ seekTo(cue.startTime)} + /> + handleDeleteCue(index)} + /> + + {hasInvalidTimestamp(cue) && ( +
+ {intl.formatMessage(messages.invalidTimestampLabel)} +
+ )} +
+
+ +
+ + ); + })} +
+ + )} + + + + + + + + {isUnsavedModalOpen && ( + setIsUnsavedModalOpen(false)} + > + + {intl.formatMessage(messages.unsavedModalTitle)} + + +

{intl.formatMessage(messages.unsavedModalDescription)}

+ + + + + + + )} + + ); +}; + +export default TranscriptEditor; diff --git a/src/files-and-videos/videos-page/transcript-editor/index.ts b/src/files-and-videos/videos-page/transcript-editor/index.ts new file mode 100644 index 0000000000..678c3fcb31 --- /dev/null +++ b/src/files-and-videos/videos-page/transcript-editor/index.ts @@ -0,0 +1 @@ +export { default } from './TranscriptEditor'; diff --git a/src/files-and-videos/videos-page/transcript-editor/messages.ts b/src/files-and-videos/videos-page/transcript-editor/messages.ts new file mode 100644 index 0000000000..f996064910 --- /dev/null +++ b/src/files-and-videos/videos-page/transcript-editor/messages.ts @@ -0,0 +1,101 @@ +import { defineMessages } from '@edx/frontend-platform/i18n'; + +const messages = defineMessages({ + loadingLabel: { + id: 'course-authoring.video-uploads.transcriptEditor.loading', + defaultMessage: 'Loading transcript...', + description: 'Loading state for transcript editor', + }, + saveButtonLabel: { + id: 'course-authoring.video-uploads.transcriptEditor.save', + defaultMessage: 'Save', + description: 'Save button label for transcript editor', + }, + cancelButtonLabel: { + id: 'course-authoring.video-uploads.transcriptEditor.cancel', + defaultMessage: 'Cancel', + description: 'Cancel button label for transcript editor', + }, + saveFailedLabel: { + id: 'course-authoring.video-uploads.transcriptEditor.saveFailed', + defaultMessage: 'Unable to save transcript, please try again.', + description: 'Error shown when transcript save fails', + }, + saveInProgressLabel: { + id: 'course-authoring.video-uploads.transcriptEditor.saving', + defaultMessage: 'Saving...', + description: 'Label shown while transcript save is in progress', + }, + savedLabel: { + id: 'course-authoring.video-uploads.transcriptEditor.saved', + defaultMessage: 'Saved', + description: 'Label shown when transcript save is successful', + }, + insertCueLabel: { + id: 'course-authoring.video-uploads.transcriptEditor.insertCue', + defaultMessage: 'Insert cue here', + description: 'Label for inserting a new cue between transcript cues', + }, + deleteCueLabel: { + id: 'course-authoring.video-uploads.transcriptEditor.deleteCue', + defaultMessage: 'Delete cue', + description: 'Accessible label for deleting a transcript cue', + }, + seekCueLabel: { + id: 'course-authoring.video-uploads.transcriptEditor.seekCue', + defaultMessage: 'Seek to cue time', + description: 'Accessible label for seek button in cue row', + }, + invalidTimestampLabel: { + id: 'course-authoring.video-uploads.transcriptEditor.invalidTimestamp', + defaultMessage: 'Please use valid cue times in HH:MM:SS,mmm format.', + description: 'Error shown when one or more cue timestamps are invalid', + }, + unsavedModalTitle: { + id: 'course-authoring.video-uploads.transcriptEditor.unsavedModalTitle', + defaultMessage: 'Unsaved changes', + description: 'Title for unsaved changes warning modal', + }, + unsavedModalDescription: { + id: 'course-authoring.video-uploads.transcriptEditor.unsavedModalDescription', + defaultMessage: 'Are you sure you want to leave the editor? All unsaved changes will be lost.', + description: 'Description for unsaved changes warning modal', + }, + keepEditingButtonLabel: { + id: 'course-authoring.video-uploads.transcriptEditor.keepEditing', + defaultMessage: 'Keep editing', + description: 'Button label to keep editing transcript changes', + }, + closeEditorButtonLabel: { + id: 'course-authoring.video-uploads.transcriptEditor.closeEditor', + defaultMessage: 'Leave editor', + description: 'Button label to close transcript editor and discard changes', + }, + invalidCueTextLabel: { + id: 'course-authoring.video-uploads.transcriptEditor.invalidCueText', + defaultMessage: 'Cue text cannot be empty.', + description: 'Error shown when one or more cue texts are empty', + }, + loadFailedLabel: { + id: 'course-authoring.video-uploads.transcriptEditor.loadFailed', + defaultMessage: 'Unable to load transcript, please try again.', + description: 'Error shown when the transcript could not be downloaded into the editor', + }, + cueTextLabel: { + id: 'course-authoring.video-uploads.transcriptEditor.cueTextLabel', + defaultMessage: 'Cue {index} text', + description: 'Accessible label for the text field of a transcript cue', + }, + cueStartTimeLabel: { + id: 'course-authoring.video-uploads.transcriptEditor.cueStartTimeLabel', + defaultMessage: 'Cue {index} start time', + description: 'Accessible label for the start timestamp field of a transcript cue', + }, + cueEndTimeLabel: { + id: 'course-authoring.video-uploads.transcriptEditor.cueEndTimeLabel', + defaultMessage: 'Cue {index} end time', + description: 'Accessible label for the end timestamp field of a transcript cue', + }, +}); + +export default messages; diff --git a/src/files-and-videos/videos-page/transcript-editor/srtUtils.test.ts b/src/files-and-videos/videos-page/transcript-editor/srtUtils.test.ts new file mode 100644 index 0000000000..039cbdec5a --- /dev/null +++ b/src/files-and-videos/videos-page/transcript-editor/srtUtils.test.ts @@ -0,0 +1,99 @@ +import { + formatTimestamp, + isValidSrt, + parseSrt, + parseTimestamp, + serializeSrt, +} from './srtUtils'; + +const SRT = [ + '1', + '00:00:01,000 --> 00:00:02,500', + 'Hello there', + '', + '2', + '00:00:03,000 --> 00:00:04,000', + 'Second cue', +].join('\n'); + +describe('parseSrt', () => { + it('parses indexed cue blocks', () => { + expect(parseSrt(SRT)).toEqual([ + { startTime: '00:00:01,000', endTime: '00:00:02,500', text: 'Hello there' }, + { startTime: '00:00:03,000', endTime: '00:00:04,000', text: 'Second cue' }, + ]); + }); + + it('parses cue blocks without index lines', () => { + const noIndex = '00:00:01,000 --> 00:00:02,000\nHi\n\n00:00:03,000 --> 00:00:04,000\nBye'; + expect(parseSrt(noIndex)).toEqual([ + { startTime: '00:00:01,000', endTime: '00:00:02,000', text: 'Hi' }, + { startTime: '00:00:03,000', endTime: '00:00:04,000', text: 'Bye' }, + ]); + }); + + it('keeps multi-line cue text together', () => { + const multi = '1\n00:00:01,000 --> 00:00:02,000\nline one\nline two'; + expect(parseSrt(multi)[0].text).toBe('line one\nline two'); + }); + + it('drops blocks with a malformed timestamp line', () => { + const partiallyBroken = `${SRT}\n\n3\nnot a timestamp\nBroken`; + expect(parseSrt(partiallyBroken)).toHaveLength(2); + }); + + it('drops blocks that have fewer than two lines', () => { + expect(parseSrt(`${SRT}\n\nlonely line`)).toHaveLength(2); + }); + + it('returns an empty list for empty or non-string input', () => { + expect(parseSrt('')).toEqual([]); + expect(parseSrt(null)).toEqual([]); + expect(parseSrt(undefined)).toEqual([]); + }); +}); + +describe('serializeSrt', () => { + it('re-numbers cues and round-trips through parseSrt', () => { + const cues = parseSrt(SRT); + const serialized = serializeSrt(cues); + expect(serialized.startsWith('1\n00:00:01,000 --> 00:00:02,500\nHello there')).toBe(true); + expect(parseSrt(serialized)).toEqual(cues); + }); +}); + +describe('parseTimestamp / formatTimestamp', () => { + it('parses SRT timestamps into seconds', () => { + expect(parseTimestamp('01:02:03,450')).toBeCloseTo(3723.45); + expect(parseTimestamp('00:00:00,000')).toBe(0); + expect(parseTimestamp('00:01:00')).toBe(60); + }); + + it('formats seconds into SRT timestamps and round-trips', () => { + expect(formatTimestamp(3723.45)).toBe('01:02:03,450'); + expect(parseTimestamp(formatTimestamp(59.999))).toBeCloseTo(59.999); + }); + + it('clamps negative and non-numeric input to zero', () => { + expect(formatTimestamp(-5)).toBe('00:00:00,000'); + expect(formatTimestamp(NaN)).toBe('00:00:00,000'); + }); +}); + +describe('isValidSrt', () => { + it('accepts well-formed content, with or without index lines', () => { + expect(isValidSrt(SRT)).toBe(true); + expect(isValidSrt('00:00:01,000 --> 00:00:02,000\nHi')).toBe(true); + }); + + it('treats empty or blank content as a valid empty transcript', () => { + expect(isValidSrt('')).toBe(true); + expect(isValidSrt(null)).toBe(true); + expect(isValidSrt(' \n \n ')).toBe(true); + }); + + it('rejects content where any block is malformed', () => { + expect(isValidSrt(`${SRT}\n\n3\nnot a timestamp\nBroken`)).toBe(false); + expect(isValidSrt('just some text')).toBe(false); + }); +}); diff --git a/src/files-and-videos/videos-page/transcript-editor/srtUtils.ts b/src/files-and-videos/videos-page/transcript-editor/srtUtils.ts new file mode 100644 index 0000000000..dfacd9a5d9 --- /dev/null +++ b/src/files-and-videos/videos-page/transcript-editor/srtUtils.ts @@ -0,0 +1,90 @@ +export interface Cue { + startTime: string; + endTime: string; + text: string; +} + +export const parseSrt = (text: string | null | undefined): Cue[] => { + if (!text || typeof text !== 'string') { + return []; + } + + return text + .trim() + .split(/\n\s*\n/) + .map((block) => block.split('\n')) + .map((lines): Cue | null => { + if (lines.length < 2) { + return null; + } + + const hasIndex = /^\d+$/.test(lines[0].trim()); + const timeLine = hasIndex ? lines[1] : lines[0]; + const textLines = hasIndex ? lines.slice(2) : lines.slice(1); + + const match = timeLine.match(/(\d{2}:\d{2}:\d{2},\d{3})\s*-->\s*(\d{2}:\d{2}:\d{2},\d{3})/); + if (!match) { + return null; + } + + return { + startTime: match[1], + endTime: match[2], + text: textLines.join('\n').trim(), + }; + }) + .filter((cue): cue is Cue => cue !== null); +}; + +export const serializeSrt = (cues: Cue[]): string => + cues + .map((cue, index) => `${index + 1}\n${cue.startTime} --> ${cue.endTime}\n${cue.text}`) + .join('\n\n'); + +export const parseTimestamp = (timestamp: string): number => { + const [hms, ms] = timestamp.split(','); + const [hours, minutes, seconds] = hms.split(':').map(Number); + return (hours * 3600) + (minutes * 60) + seconds + (Number(ms || 0) / 1000); +}; + +export const formatTimestamp = (totalSeconds: number): string => { + const safeSeconds = Math.max(0, Number(totalSeconds) || 0); + const hours = Math.floor(safeSeconds / 3600); + const minutes = Math.floor((safeSeconds % 3600) / 60); + const seconds = Math.floor(safeSeconds % 60); + const milliseconds = Math.round((safeSeconds - Math.floor(safeSeconds)) * 1000); + + const pad2 = (value: number) => String(value).padStart(2, '0'); + const pad3 = (value: number) => String(value).padStart(3, '0'); + + return `${pad2(hours)}:${pad2(minutes)}:${pad2(seconds)},${pad3(milliseconds)}`; +}; + +/** + * Strictly validates SRT content. + * Returns true for empty/blank files (valid empty transcript). + * Returns false if the file has cue blocks where ANY block has a malformed + * timestamp line (partial corruption is not allowed). + */ +export const isValidSrt = (text: string | null | undefined): boolean => { + if (!text || typeof text !== 'string') { + return true; + } + + const blocks = text.trim().split(/\n\s*\n/).filter((b) => b.trim().length > 0); + if (blocks.length === 0) { + return true; + } + + const TIMESTAMP_LINE = /^\d{2}:\d{2}:\d{2},\d{3}\s*-->\s*\d{2}:\d{2}:\d{2},\d{3}/; + + return blocks.every((block) => { + const lines = block.trim().split('\n').map((l) => l.trim()); + if (lines.length < 2) { + return false; + } + const hasIndex = /^\d+$/.test(lines[0]); + const timeLine = hasIndex ? lines[1] : lines[0]; + return TIMESTAMP_LINE.test(timeLine); + }); +}; From 87ce85da911a2717394ad82564199f583c1bd0fd Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 8 Sep 2026 13:25:11 -0400 Subject: [PATCH 08/16] chore(deps): bump dompurify from 3.4.12 to 3.4.13 (#3190) Bumps [dompurify](https://github.com/cure53/DOMPurify) from 3.4.12 to 3.4.13. - [Release notes](https://github.com/cure53/DOMPurify/releases) - [Commits](https://github.com/cure53/DOMPurify/compare/3.4.12...3.4.13) --- updated-dependencies: - dependency-name: dompurify dependency-version: 3.4.13 dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 2ab2e802fe..9d788ea393 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11493,9 +11493,9 @@ } }, "node_modules/dompurify": { - "version": "3.4.12", - "resolved": "https://registry.npmjs.org/dompurify/-/dompurify-3.4.12.tgz", - "integrity": "sha512-zQvGet8Z2sWbQhCmfFz/T5QWH2oBmjnqK3qvOjaqaNLrLEF912WamU+ohnTp0TCep/MFVHpdJuCZEdFOdTnEFg==", + "version": "3.4.13", + "resolved": "https://registry.npmjs.org/dompurify/-/dompurify-3.4.13.tgz", + "integrity": "sha512-2vmYIoqjze2d+kakP8S/nS5shfsl587kzwEjcGlTdiksUVgFHnFCsLYDVj/JNqJVOQZGSYBTmuycv0PodwmnMQ==", "license": "(MPL-2.0 OR Apache-2.0)", "optionalDependencies": { "@types/trusted-types": "^2.0.7" From db5e4809df488861e0b188f25ec5f374efe51a26 Mon Sep 17 00:00:00 2001 From: Taimoor Ahmed <68893403+taimoor-ahmed-1@users.noreply.github.com> Date: Sat, 12 Sep 2026 01:14:02 +0500 Subject: [PATCH 09/16] refactor: migrate course_details and grading callers to standardized v3 APIs (#3136) Course details reads/writes now use /api/contentstore/v3/course_details/. The grading write uses PATCH /api/contentstore/v3/authoring_grading/; the read stays on v1 since v3 exposes no GET. Refs openedx/frontend-app-authoring#3127 Co-authored-by: Claude --- src/course-outline/data/api.ts | 3 ++- src/grading-settings/GradingSettings.test.jsx | 6 +++--- src/grading-settings/data/api.js | 7 ++++++- src/schedule-and-details/data/api.ts | 3 ++- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/course-outline/data/api.ts b/src/course-outline/data/api.ts index 4a95b4d19b..992375b00a 100644 --- a/src/course-outline/data/api.ts +++ b/src/course-outline/data/api.ts @@ -19,7 +19,8 @@ export const getCourseOutlineIndexApiUrl = ( courseId: string, ) => `${getApiBaseUrl()}/api/contentstore/v1/course_index/${courseId}`; -export const getCourseDetailsApiUrl = (courseId) => `${getApiBaseUrl()}/api/contentstore/v1/course_details/${courseId}`; +export const getCourseDetailsApiUrl = (courseId) => + `${getApiBaseUrl()}/api/contentstore/v3/course_details/${courseId}/`; export const getCourseBestPracticesApiUrl = ({ courseId, diff --git a/src/grading-settings/GradingSettings.test.jsx b/src/grading-settings/GradingSettings.test.jsx index 8b9325f9ef..f6bd7010d7 100644 --- a/src/grading-settings/GradingSettings.test.jsx +++ b/src/grading-settings/GradingSettings.test.jsx @@ -11,7 +11,7 @@ import { mockWaffleFlags } from '@src/data/apiHooks.mock'; import { useCourseUserPermissions } from '@src/authz/hooks'; import gradingSettings from './__mocks__/gradingSettings'; -import { getGradingSettingsApiUrl } from './data/api'; +import { getGradingSettingsApiUrl, getAuthoringGradingApiUrl } from './data/api'; import * as apiHooks from './data/apiHooks'; import GradingSettings from './GradingSettings'; import messages from './messages'; @@ -45,7 +45,7 @@ describe('', () => { .onGet(getGradingSettingsApiUrl(courseId)) .reply(200, gradingSettings); axiosMock - .onPost(getGradingSettingsApiUrl(courseId)) + .onPatch(getAuthoringGradingApiUrl(courseId)) .reply(200, {}); axiosMock.onGet(getCourseSettingsApiUrl(courseId)) .reply(200, {}); @@ -147,7 +147,7 @@ describe(' permissions', () => { Object.defineProperty(window, 'scrollTo', { value: jest.fn(), writable: true }); const { axiosMock: mock } = mocks; mock.onGet(getGradingSettingsApiUrl(courseId)).reply(200, gradingSettings); - mock.onPost(getGradingSettingsApiUrl(courseId)).reply(200, {}); + mock.onPatch(getAuthoringGradingApiUrl(courseId)).reply(200, {}); mock.onGet(getCourseSettingsApiUrl(courseId)).reply(200, {}); jest.mocked(useCourseUserPermissions).mockReturnValue({ isLoading: false, diff --git a/src/grading-settings/data/api.js b/src/grading-settings/data/api.js index e83ca2c647..52467e5455 100644 --- a/src/grading-settings/data/api.js +++ b/src/grading-settings/data/api.js @@ -4,9 +4,14 @@ import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { deepConvertingKeysToCamelCase, deepConvertingKeysToSnakeCase } from '../../utils'; const getApiBaseUrl = () => getConfig().STUDIO_BASE_URL; + export const getGradingSettingsApiUrl = (courseId) => `${getApiBaseUrl()}/api/contentstore/v1/course_grading/${courseId}`; +// v3 authoring_grading exposes only PATCH and has no GET, so the read stays on v1. +export const getAuthoringGradingApiUrl = (courseId) => + `${getApiBaseUrl()}/api/contentstore/v3/authoring_grading/${courseId}/`; + /** * Get's grading setting for a course. * @param {string} courseId @@ -26,6 +31,6 @@ export async function getGradingSettings(courseId) { */ export async function sendGradingSettings(courseId, settings) { const { data } = await getAuthenticatedHttpClient() - .post(getGradingSettingsApiUrl(courseId), deepConvertingKeysToSnakeCase(settings)); + .patch(getAuthoringGradingApiUrl(courseId), deepConvertingKeysToSnakeCase(settings)); return camelCaseObject(data); } diff --git a/src/schedule-and-details/data/api.ts b/src/schedule-and-details/data/api.ts index 7c5b82fc5f..a3a2670075 100644 --- a/src/schedule-and-details/data/api.ts +++ b/src/schedule-and-details/data/api.ts @@ -3,7 +3,8 @@ import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { convertObjectToSnakeCase } from '@src/utils'; const getApiBaseUrl = () => getConfig().STUDIO_BASE_URL; -export const getCourseDetailsApiUrl = (courseId) => `${getApiBaseUrl()}/api/contentstore/v1/course_details/${courseId}`; +export const getCourseDetailsApiUrl = (courseId) => + `${getApiBaseUrl()}/api/contentstore/v3/course_details/${courseId}/`; export const getUploadAssetsUrl = (courseId) => `${getApiBaseUrl()}/assets/${courseId}/`; // TODO: This interface has only basic data, all the rest needs to be added. From 8d5f6b5495d634c0bd91af4d5242c756132ae488 Mon Sep 17 00:00:00 2001 From: AShatsila Date: Sat, 12 Sep 2026 00:36:29 +0400 Subject: [PATCH 10/16] feat: add Competency Tree on the Competency Management page (#3218) Adds /taxonomy/:taxonomyId/competencies: a read-only, expand/collapse tree of a taxonomy's tags, with each row's Competency ID shown alongside it. Closes openedx/openedx-core#680 Co-authored-by: Ales Co-authored-by: Claude Co-authored-by: AShatsila <319486855+AShatsila@users.noreply.github.com> --- src/generic/ExpandCollapseIconButton.test.tsx | 62 +++++ src/generic/ExpandCollapseIconButton.tsx | 66 +++++ src/index.jsx | 3 +- .../CompetencyManagementPage.test.tsx | 81 ++++++ .../CompetencyManagementPage.tsx | 66 +++++ .../competency-management/CompetencyTree.scss | 198 ++++++++++++++ .../CompetencyTree.test.tsx | 242 ++++++++++++++++++ .../competency-management/CompetencyTree.tsx | 175 +++++++++++++ .../CompetencyTreeItem.tsx | 84 ++++++ src/taxonomy/competency-management/index.ts | 1 + .../competency-management/messages.ts | 36 +++ src/taxonomy/data/apiHooks.ts | 2 +- src/taxonomy/data/types.ts | 4 +- src/taxonomy/index.ts | 1 + src/taxonomy/tag-list/OptionalExpandLink.tsx | 34 +-- 15 files changed, 1025 insertions(+), 30 deletions(-) create mode 100644 src/generic/ExpandCollapseIconButton.test.tsx create mode 100644 src/generic/ExpandCollapseIconButton.tsx create mode 100644 src/taxonomy/competency-management/CompetencyManagementPage.test.tsx create mode 100644 src/taxonomy/competency-management/CompetencyManagementPage.tsx create mode 100644 src/taxonomy/competency-management/CompetencyTree.scss create mode 100644 src/taxonomy/competency-management/CompetencyTree.test.tsx create mode 100644 src/taxonomy/competency-management/CompetencyTree.tsx create mode 100644 src/taxonomy/competency-management/CompetencyTreeItem.tsx create mode 100644 src/taxonomy/competency-management/index.ts create mode 100644 src/taxonomy/competency-management/messages.ts diff --git a/src/generic/ExpandCollapseIconButton.test.tsx b/src/generic/ExpandCollapseIconButton.test.tsx new file mode 100644 index 0000000000..4a4776647e --- /dev/null +++ b/src/generic/ExpandCollapseIconButton.test.tsx @@ -0,0 +1,62 @@ +import { fireEvent, render, screen } from '@testing-library/react'; + +import ExpandCollapseIconButton from './ExpandCollapseIconButton'; + +describe('', () => { + it('renders an invisible, disabled placeholder when canExpand is false, regardless of isExpanded, and ignores clicks', () => { + const onToggle = jest.fn(); + render( + , + ); + + const button = screen.getByRole('button', { hidden: true }); + expect(button).toBeDisabled(); + expect(button).toHaveAttribute('aria-hidden', 'true'); + expect(button).toHaveClass('invisible'); + + fireEvent.click(button); + expect(onToggle).not.toHaveBeenCalled(); + }); + + it('shows the expand-action label and aria-expanded="false" while collapsed, and calls onToggle when clicked', () => { + const onToggle = jest.fn(); + render( + , + ); + + // The accessible name comes from `aria-label`, which this component + // sets to `expandLabel` while collapsed and `collapseLabel` while + // expanded. + const button = screen.getByRole('button', { name: 'Expand' }); + expect(button).toHaveAttribute('aria-expanded', 'false'); + + fireEvent.click(button); + expect(onToggle).toHaveBeenCalledTimes(1); + }); + + it('shows the collapse-action label and aria-expanded="true" while expanded', () => { + render( + , + ); + + const button = screen.getByRole('button', { name: 'Collapse' }); + expect(button).toHaveAttribute('aria-expanded', 'true'); + }); +}); diff --git a/src/generic/ExpandCollapseIconButton.tsx b/src/generic/ExpandCollapseIconButton.tsx new file mode 100644 index 0000000000..34e03cb4a6 --- /dev/null +++ b/src/generic/ExpandCollapseIconButton.tsx @@ -0,0 +1,66 @@ +import { IconButton } from '@openedx/paragon'; +import { ExpandLess, ExpandMore } from '@openedx/paragon/icons'; + +interface ExpandCollapseIconButtonProps { + canExpand: boolean; + isExpanded: boolean; + onToggle?: () => void; + /** Label for the "expand" action - shown while the row is collapsed. */ + expandLabel: string; + /** Label for the "collapse" action - shown while the row is expanded. */ + collapseLabel: string; + className?: string; +} + +/** ExpandCollapseIconButton + * Shared disclosure button for one row of a tree or nested list, toggling + * between an expanded and a collapsed state. + * + * Takes plain booleans/callback rather than a TanStack `Row`, so it works + * equally for a `
    /
  • `-rendered tree and for a table row wrapping one. + * When the row has no children (`canExpand` is false), this renders an + * invisible, disabled placeholder icon instead of nothing, so every row's + * label still starts at the same horizontal position. + * + * `expandLabel`/`collapseLabel` are already-formatted strings, not + * message objects, so this component stays free of any particular i18n + * message set - each caller owns its own wording. + */ +const ExpandCollapseIconButton = ({ + canExpand, + isExpanded, + onToggle, + expandLabel, + collapseLabel, + className, +}: ExpandCollapseIconButtonProps) => { + if (!canExpand) { + return ( + + ); + } + + const label = isExpanded ? collapseLabel : expandLabel; + + return ( + + ); +}; + +export default ExpandCollapseIconButton; diff --git a/src/index.jsx b/src/index.jsx index 2d527ae1f0..d8625aedb1 100755 --- a/src/index.jsx +++ b/src/index.jsx @@ -36,7 +36,7 @@ import CourseAuthoringRoutes from './CourseAuthoringRoutes'; import Head from './head/Head'; import { StudioHome } from './studio-home'; import CourseRerun from './course-rerun'; -import { TaxonomyLayout, TaxonomyDetailPage, TaxonomyListPage } from './taxonomy'; +import { TaxonomyLayout, TaxonomyDetailPage, TaxonomyListPage, CompetencyManagementPage } from './taxonomy'; import { ContentTagsDrawer } from './content-tags-drawer'; import AccessibilityPage from './accessibility-page'; import { ToastProvider } from './generic/toast-context'; @@ -110,6 +110,7 @@ const App = () => { }> } /> + } /> render(, { path, params }); + +describe('', () => { + beforeEach(() => { + ({ axiosMock } = initializeMocks()); + }); + + it('shows a loading spinner while the taxonomy is being fetched', () => { + axiosMock.onGet(apiUrls.taxonomy(taxonomyId)).reply(() => new Promise(() => {})); + renderPage(); + expect(screen.getByRole('status')).toHaveTextContent('Loading...'); + }); + + it('shows a connection error alert when the taxonomy fails to load', async () => { + axiosMock.onGet(apiUrls.taxonomy(taxonomyId)).reply(500); + renderPage(); + expect(await screen.findByTestId('connectionErrorAlert')).toBeInTheDocument(); + }); + + it('renders the taxonomy name and links the breadcrumb to the right pages', async () => { + axiosMock.onGet(apiUrls.taxonomy(taxonomyId)).reply(200, { + id: taxonomyId, + name: 'Test taxonomy', + description: 'This is a description', + }); + axiosMock.onGet(tagListUrl).reply(200, emptyTagListResponse); + + renderPage(); + + // The page's own heading is the taxonomy's own name, matching the Figma + // design (there's no separate "Competencies" title anywhere on the page). + expect(await screen.findByRole('heading', { name: 'Test taxonomy' })).toBeInTheDocument(); + + const breadcrumbNav = screen.getByRole('navigation', { name: 'breadcrumb' }); + expect(within(breadcrumbNav).getByRole('link', { name: 'Taxonomies' })).toHaveAttribute('href', '/taxonomies/'); + // The active breadcrumb item is the taxonomy's own name, and isn't a link. + expect(within(breadcrumbNav).getByText('Test taxonomy')).toBeInTheDocument(); + expect(within(breadcrumbNav).queryByRole('link', { name: 'Test taxonomy' })).not.toBeInTheDocument(); + + // Wait for the tree's own (empty) data to settle so no async query is left + // pending. Even with zero tags, the tree still shows its synthetic + // taxonomy-root row (see `CompetencyTree`), so there's no separate empty + // state - the taxonomy name doubles as it. Scoped to the root row's own + // label, since the breadcrumb link above also reads "Test taxonomy". + expect(await screen.findByText('Test taxonomy', { selector: '.competency-row__label' })) + .toBeInTheDocument(); + expect(screen.queryByText('No results found')).not.toBeInTheDocument(); + }); +}); diff --git a/src/taxonomy/competency-management/CompetencyManagementPage.tsx b/src/taxonomy/competency-management/CompetencyManagementPage.tsx new file mode 100644 index 0000000000..32c8b3e1ec --- /dev/null +++ b/src/taxonomy/competency-management/CompetencyManagementPage.tsx @@ -0,0 +1,66 @@ +import { useIntl } from '@edx/frontend-platform/i18n'; +import { + Breadcrumb, + Container, +} from '@openedx/paragon'; +import { Helmet } from 'react-helmet'; +import { Link, useParams } from 'react-router-dom'; + +import ConnectionErrorAlert from '@src/generic/ConnectionErrorAlert'; +import Loading from '@src/generic/Loading'; +import getPageHeadTitle from '@src/generic/utils'; +import SubHeader from '@src/generic/sub-header/SubHeader'; +import taxonomyMessages from '@src/taxonomy/messages'; +import { useTaxonomyDetails } from '@src/taxonomy/data/apiHooks'; +import CompetencyTree from './CompetencyTree'; +import messages from './messages'; + +const CompetencyManagementPage = () => { + const intl = useIntl(); + const { taxonomyId: taxonomyIdString } = useParams(); + const taxonomyId = Number(taxonomyIdString); + + const { + data: taxonomy, + isError, + isFetched, + } = useTaxonomyDetails(taxonomyId); + + if (!isFetched) { + return ; + } + + if (isError || !taxonomy) { + return ; + } + + return ( + <> + + {getPageHeadTitle(intl.formatMessage(messages.headerTitle), taxonomy.name)} + +
    + + + + +
    +
    + + + +
    + + ); +}; + +export default CompetencyManagementPage; diff --git a/src/taxonomy/competency-management/CompetencyTree.scss b/src/taxonomy/competency-management/CompetencyTree.scss new file mode 100644 index 0000000000..f30d269ac6 --- /dev/null +++ b/src/taxonomy/competency-management/CompetencyTree.scss @@ -0,0 +1,198 @@ +// Scoped styling for the read-only Competency Management page's nested-list +// tree (a page-local `
      /
    • ` renderer, not a table). Scoped under +// `.competency-tree` so nothing here reaches the existing, unrelated +// `/taxonomy/:taxonomyId` page. +// +// This file is imported directly from `CompetencyTree.tsx`, so it compiles as +// its own root stylesheet rather than through `src/index.scss` (where +// `CourseSequence.scss` gets its custom-media names from, via that file's own +// `@import` chain). `postcss-custom-media` only resolves names declared in +// the same compiled output, so this file needs its own `@use` below. +@use "@openedx/paragon/styles/css/core/custom-media-breakpoints.css"; + +.competency-tree { + $line-color: rgb(0 0 0 / .08); + $shadow-color: rgb(0 0 0 / .12); + $border-radius: 8px; + $indent: 25px; + // Figma specifies 25px from a row's own left edge to its label text. + // Paragon's default `IconButton size="sm"` (36px hit area) doesn't fit + // that inset, so the chevron is shrunk to this size - see + // `.competency-row .btn-icon-sm` below. + $disclosure-icon-size: 16px; + // WCAG 2.2 AA's minimum interactive target is 24px. This pads the + // chevron's hit area back out to that minimum without changing its + // visible size - see `.competency-row .btn-icon-sm` below. + $disclosure-hit-area-inset: 4px; + // Group envelope fill/border, read directly off the Figma inspector for + // this frame - not the earlier `--pgn-color-light-400`/#E9E6E4 guess. + $group-bg-color: #F8F8F8; + $group-border-color: #DDDDDD; + + .competency-row__badge { + overflow-wrap: anywhere; + // Figma specifies a small, squared-off radius, not Paragon's full + // `pill` shape - reuses the custom property Paragon's own `.badge` + // already resolves to without the `pill` prop, rather than a literal. + border-radius: var(--pgn-size-badge-border-radius-base); + // Visible gap to the row's right edge - `ml-auto` (see + // `CompetencyTreeItem.tsx`) only pushes the badge right within the + // row, it doesn't add this gap. + margin-right: 5px; + // Padding and type read directly off Figma's inspector for this badge, + // overriding Paragon's own badge tokens (rather than the resolved + // properties) so anything else reading the same tokens stays in sync. + --pgn-spacing-badge-padding-y: 1.5px; + --pgn-spacing-badge-padding-x-base: 6px; + --pgn-typography-badge-font-weight: 400; + // `62.5%` renders as 10px at the default root font size (Figma's spec), + // but as a relative unit it keeps scaling with the user's font-size + // preference, unlike a fixed px value. + --pgn-typography-badge-font-size: 62.5%; + + line-height: 15.75px; + letter-spacing: normal; + } + + // Sits on this page's own light-grey background, not the white container + // below it - reuses the exact custom property `bg-light-400` resolves to. + &__toolbar { + background-color: var(--pgn-color-light-400); + // Bootstrap's `p-4` conflated two separate Figma measurements: a 24px + // vertical gap to `.competency-tree__container` below (kept, via + // top/bottom padding) and a 24px horizontal inset that pushed the + // button off the container's right edge below it - Figma has them + // flush, so left/right padding is zeroed instead. + padding: 24px 0; + } + + // The single outer boundary holding the root row and every group/leaf + // inside one container, matching Figma. A plain `
      ` rather than + // Paragon's `Card`, since `Card`'s own visible outline is entirely a + // `box-shadow` (its Bootstrap `.card` border is zeroed) - a plain `
      ` + // is simpler and gives full control over that same look. + &__container { + background-color: #FFFFFF; + border: 1px solid $line-color; + border-radius: $border-radius; + box-shadow: 0 1px 4px $shadow-color; + padding: 12px 16px; + } + + // Indentation comes from `
        ` nesting itself, not a depth number + // computed in JS. + ul { + list-style: none; + margin: 0; + padding-left: $indent; + + // Reduced indentation at narrow viewports so deep nesting doesn't crowd + // out the label and Competency ID badge. + @media (--pgn-size-breakpoint-max-width-xs) { + padding-left: 16px; + } + } + + // Base row look: white, bordered, rounded "pill". This is the leaf style + // and the default for every row; a row with children flattens back to + // its envelope's fill via `.competency-group > .competency-row` below. + .competency-row { + display: flex; + align-items: center; + background-color: #FFFFFF; + border: 1px solid $line-color; + border-radius: $border-radius; + // This plus the chevron's own size/margin below add up to Figma's 25px + // row inset (label start, measured from the row's own left edge). + padding-left: 5px; + + // The disclosure chevron (visible or invisible placeholder, rendered by + // `ExpandCollapseIconButton`) shrunk to fit the 25px inset above - see + // `$disclosure-icon-size`. + .btn-icon-sm { + // `content-box` (overriding the app-wide `border-box` reset for just + // this button) makes `width`/`height` define the glyph only, so the + // padding below grows the real hit box without shrinking the glyph. + box-sizing: content-box; + width: $disclosure-icon-size; + height: $disclosure-icon-size; + // Grows the hit area to the WCAG 24px minimum, then cancels that + // growth's effect on the row's layout with matching negative margins + // on three sides, so the glyph and the 25px label inset don't move. + // The right margin is left at 0, so the 4px right padding above stays + // uncancelled and provides the icon-to-label gap, replacing the `mr-1` + // utility this page's disclosure button doesn't apply + // (`ExpandCollapseIconButton`'s `!important` couldn't otherwise be + // overridden here). + padding: $disclosure-hit-area-inset; + margin: (-$disclosure-hit-area-inset) 0 (-$disclosure-hit-area-inset) (-$disclosure-hit-area-inset); // top right bottom left + + .btn-icon__icon-container, + .pgn__icon.btn-icon__icon { + width: $disclosure-icon-size; + height: $disclosure-icon-size; + } + } + } + + // The taxonomy-root row is the single top item of the outer container: + // plain/white, not its own box - it's not a real `Tag`, just this page's + // display-only entry point into the tree (see `CompetencyTree.tsx`). + .competency-row--root { + border: none; + // Clears the chevron's hover/focus circle from the group box below. + // Group header rows get equivalent room from `.competency-group`'s own + // `padding: 5px`; the root has no such wrapper, so it needs this + // instead. + margin-bottom: 12px + $disclosure-hit-area-inset; + } + + // Any node with children is a "group": one grey envelope around itself - + // its header row plus its whole rendered `
          ` of children, not just the + // header. Keyed off `.competency-group` (added by `CompetencyTreeItem` + // from the same `hasChildren` check that decides whether to render a + // nested `
            `), so this recurses to any depth: a child that's itself a + // group gets its own envelope inset inside this one. + .competency-group { + background-color: $group-bg-color; + border: 1px solid $group-border-color; + border-radius: $border-radius; + padding: 5px; + } + + // A group's own header row sits flat on its envelope's fill instead of + // the white/bordered leaf look - only true leaves keep that from the + // base `.competency-row` rule above. + .competency-group > .competency-row { + background-color: transparent; + border: none; + border-radius: 0; + } + + // Leaf rows keep the base rule's 1px border (groups/root override it to + // `none`), which shifts their inset over by that 1px - pulled back so + // every row's label lands at the same 25px inset regardless of variant. + li:not(.competency-group) > .competency-row { + padding-left: 4px; + } + + // Vertical gap between a group's own header row and its first child leaf + // (smaller than the leaf-to-leaf gap below - two distinct values). + // Keyed off `.competency-group` rather than a structural `> li >` selector, + // so this holds recursively at any nesting depth. + .competency-group > ul { + margin-top: 5px; + } + + // Vertical gap between adjacent sibling items within the same list (e.g. + // leaf-to-leaf within a group), recursively at any depth. + ul > li + li { + margin-top: 10px; + } + + // Keeps the toggle button from resizing when its label swaps between + // "Expand All" and "Collapse All" - sized for the longer label. + &__toggle-button { + min-width: 160px; + } +} diff --git a/src/taxonomy/competency-management/CompetencyTree.test.tsx b/src/taxonomy/competency-management/CompetencyTree.test.tsx new file mode 100644 index 0000000000..d71b67e032 --- /dev/null +++ b/src/taxonomy/competency-management/CompetencyTree.test.tsx @@ -0,0 +1,242 @@ +import { + fireEvent, + initializeMocks, + render, + screen, + within, +} from '@src/testUtils'; +import { apiUrls } from '@src/taxonomy/data/api'; +import { TAXONOMY_MAX_DEPTH } from '@src/taxonomy/taxonomy-detail/constants'; +import CompetencyTree from './CompetencyTree'; + +let axiosMock; + +const taxonomyId = 1; + +const tagListUrl = apiUrls.tagList(taxonomyId, { + pageIndex: 0, + pageSize: 50, + fullDepth: true, + disablePagination: true, +}); + +const tagDefaults = { depth: 0, parent_value: null, child_count: 0 }; + +// A root -> group -> leaf tree, plus two root tags exercising the three ways +// a Competency ID can be missing: `null`, `''`, and the field being absent +// (`undefined`) entirely. +const nestedTagsResponse = { + next: null, + previous: null, + count: 3, + num_pages: 1, + current_page: 1, + start: 0, + results: [ + { + ...tagDefaults, + id: 1, + value: 'Root A', + child_count: 1, + external_id: 'EXT-001', + }, + { + ...tagDefaults, + id: 2, + value: 'Group A1', + depth: 1, + parent_value: 'Root A', + child_count: 1, + external_id: null, + }, + { + ...tagDefaults, + id: 3, + value: 'Leaf A1a', + depth: 2, + parent_value: 'Group A1', + external_id: 'EXT-003', + }, + { + ...tagDefaults, + id: 4, + value: 'Root B', + external_id: '', + }, + { + ...tagDefaults, + id: 5, + value: 'Root C', + // no `external_id` field at all + }, + ], +}; + +const buildDuplicateValueResponse = () => ({ + ...nestedTagsResponse, + results: [ + { ...tagDefaults, id: 10, value: 'Dup Tag' }, + { ...tagDefaults, id: 11, value: 'DUP TAG' }, + ], +}); + +const taxonomyName = 'Test Taxonomy'; + +const renderTree = () => render(); + +describe('', () => { + beforeEach(() => { + ({ axiosMock } = initializeMocks()); + }); + + it('renders the taxonomy name as an expandable root, one level above the tag tree', async () => { + axiosMock.onGet(tagListUrl).reply(200, nestedTagsResponse); + renderTree(); + + // The taxonomy-root row starts expanded, so its immediate children (the + // real top-level tags) are visible without any click. + expect(await screen.findByText(taxonomyName)).toBeInTheDocument(); + expect(await screen.findByText('Root A')).toBeInTheDocument(); + + // Root A's own children are one level deeper than the root, so they + // still require an explicit expand of Root A itself. + expect(screen.queryByText('Group A1')).not.toBeInTheDocument(); + fireEvent.click(screen.getByRole('button', { name: 'Expand' })); + expect(await screen.findByText('Group A1')).toBeInTheDocument(); + + // It's a display-only row, not a real `Tag`: no Competency ID badge for + // itself, even though "Root A" (a real tag) has one. + const taxonomyRootRow = screen.getByText(taxonomyName).closest('.competency-row'); + expect(taxonomyRootRow).not.toHaveTextContent('EXT-001'); + }); + + it('renders a root -> group -> leaf tree, nested structurally rather than via a computed depth', async () => { + axiosMock.onGet(tagListUrl).reply(200, nestedTagsResponse); + renderTree(); + await screen.findByText(taxonomyName); + + fireEvent.click(screen.getByRole('button', { name: 'Expand All' })); + + const rootAItem = (await screen.findByText('Root A')).closest('li'); + const groupA1Item = (await screen.findByText('Group A1')).closest('li'); + const leafA1aItem = (await screen.findByText('Leaf A1a')).closest('li'); + expect(rootAItem).not.toBeNull(); + expect(groupA1Item).not.toBeNull(); + expect(leafA1aItem).not.toBeNull(); + + // The DOM itself encodes the hierarchy via nesting, rather than a depth + // number: "Group A1" lives inside "Root A"'s own `
          • ` subtree, and + // "Leaf A1a" lives inside "Group A1"'s (and therefore also "Root A"'s). + expect(within(rootAItem as HTMLElement).getByText('Group A1')).toBeInTheDocument(); + expect(within(groupA1Item as HTMLElement).getByText('Leaf A1a')).toBeInTheDocument(); + expect(within(rootAItem as HTMLElement).getByText('Leaf A1a')).toBeInTheDocument(); + }); + + it('renders every level of a maximum-depth chain when expanded', async () => { + // A single deep chain (each tag has exactly one child) from depth 0 to + // `TAXONOMY_MAX_DEPTH`, the worst case this page has to render: the + // synthetic taxonomy-root plus every legal real depth, 1 + (TAXONOMY_MAX_DEPTH + 1) + // nested `
              ` levels in total. + const depthChainLabels = Array.from( + { length: TAXONOMY_MAX_DEPTH + 1 }, + (_, depth) => `Depth ${depth} Tag`, + ); + const results = depthChainLabels.map((value, depth) => ({ + ...tagDefaults, + id: 100 + depth, + value, + depth, + parent_value: depth === 0 ? null : depthChainLabels[depth - 1], + child_count: depth < TAXONOMY_MAX_DEPTH ? 1 : 0, + })); + + axiosMock.onGet(tagListUrl).reply(200, { ...nestedTagsResponse, results }); + renderTree(); + await screen.findByText(taxonomyName); + + fireEvent.click(screen.getByRole('button', { name: 'Expand All' })); + await screen.findByText(depthChainLabels[depthChainLabels.length - 1]); + + // Assert every level individually, not just the deepest one: a bug that + // only manifested at the max depth wouldn't necessarily hide earlier + // levels, so this proves the whole chain rendered end-to-end. + depthChainLabels.forEach((label) => { + expect(screen.getByText(label)).toBeInTheDocument(); + }); + }); + + it('toggles nested rows with Expand All / Collapse All', async () => { + axiosMock.onGet(tagListUrl).reply(200, nestedTagsResponse); + renderTree(); + await screen.findByText(taxonomyName); + + expect(screen.queryByText('Group A1')).not.toBeInTheDocument(); + + fireEvent.click(screen.getByRole('button', { name: 'Expand All' })); + expect(await screen.findByText('Group A1')).toBeInTheDocument(); + expect(await screen.findByText('Leaf A1a')).toBeInTheDocument(); + + fireEvent.click(screen.getByRole('button', { name: 'Collapse All' })); + expect(screen.queryByText('Group A1')).not.toBeInTheDocument(); + expect(screen.queryByText('Leaf A1a')).not.toBeInTheDocument(); + }); + + it('renders the Competency ID with an accessible name when externalId is set, and nothing when it is falsy', async () => { + axiosMock.onGet(tagListUrl).reply(200, nestedTagsResponse); + renderTree(); + await screen.findByText(taxonomyName); + fireEvent.click(screen.getByRole('button', { name: 'Expand All' })); + await screen.findByText('Leaf A1a'); + + expect(screen.getByText('EXT-001')).toBeInTheDocument(); + // The badge has no `` to associate it with a column meaning anymore + // (there's no table), so it needs its own accessible name instead. Paragon's + // `Badge` renders a plain `` (implicit ARIA role "generic"), on which + // `aria-label` is prohibited, so the badge is `aria-hidden` and a + // visually-hidden sibling `` carries the accessible name instead. + expect(screen.getByText('EXT-001')).toHaveAttribute('aria-hidden', 'true'); + expect(screen.getByText('Competency ID: EXT-001')).toBeInTheDocument(); + expect(screen.getByText('EXT-003')).toBeInTheDocument(); + + // Root B (`externalId: ''`), Root C (`externalId` absent), Group A1 + // (`externalId: null`), and the taxonomy-root row itself (no `externalId` + // field at all) must not render the literal string "undefined" or + // "null", and must not render an empty badge either. + const rootBRow = screen.getByText('Root B').closest('.competency-row'); + const rootCRow = screen.getByText('Root C').closest('.competency-row'); + const groupRow = screen.getByText('Group A1').closest('.competency-row'); + const taxonomyRootRow = screen.getByText(taxonomyName).closest('.competency-row'); + [rootBRow, rootCRow, groupRow, taxonomyRootRow].forEach((row) => { + expect(row).not.toBeNull(); + expect(within(row as HTMLElement).queryByText('undefined')).not.toBeInTheDocument(); + expect(within(row as HTMLElement).queryByText('null')).not.toBeInTheDocument(); + }); + expect(screen.queryByText(/^undefined$/)).not.toBeInTheDocument(); + expect(screen.queryByText(/^null$/)).not.toBeInTheDocument(); + }); + + it('renders as a plain list, not a table', async () => { + axiosMock.onGet(tagListUrl).reply(200, nestedTagsResponse); + renderTree(); + await screen.findByText(taxonomyName); + fireEvent.click(screen.getByRole('button', { name: 'Expand All' })); + await screen.findByText('Leaf A1a'); + + // No table/column-header semantics at all - there's no `` to hide + // in the first place, since this is a `
                /
              • ` tree. + expect(screen.queryByRole('table')).not.toBeInTheDocument(); + expect(screen.queryAllByRole('columnheader')).toHaveLength(0); + + // Every real tag renders as its own list item (the synthetic + // taxonomy-root row is not itself a list item - see `CompetencyTree`). + expect(screen.getAllByRole('listitem')).toHaveLength(5); + }); + + it('renders a connection error alert instead of throwing when the tag data has duplicate values', async () => { + axiosMock.onGet(tagListUrl).reply(200, buildDuplicateValueResponse()); + renderTree(); + + expect(await screen.findByTestId('connectionErrorAlert')).toBeInTheDocument(); + expect(screen.queryByText('Dup Tag')).not.toBeInTheDocument(); + }); +}); diff --git a/src/taxonomy/competency-management/CompetencyTree.tsx b/src/taxonomy/competency-management/CompetencyTree.tsx new file mode 100644 index 0000000000..11955261ec --- /dev/null +++ b/src/taxonomy/competency-management/CompetencyTree.tsx @@ -0,0 +1,175 @@ +import { useMemo, useState } from 'react'; + +import { useIntl } from '@edx/frontend-platform/i18n'; +import { Button } from '@openedx/paragon'; +import { OpenInFull } from '@openedx/paragon/icons'; + +import ConnectionErrorAlert from '@src/generic/ConnectionErrorAlert'; +import ExpandCollapseIconButton from '@src/generic/ExpandCollapseIconButton'; +import { LoadingSpinner } from '@src/generic/Loading'; +import { useTagListData } from '@src/taxonomy/data/apiHooks'; +import { TagTree } from '@src/taxonomy/tag-list/tagTree'; +import CompetencyTreeItem from './CompetencyTreeItem'; +import messages from './messages'; +// @ts-ignore +import './CompetencyTree.scss'; + +interface CompetencyTreeProps { + taxonomyId: number; + taxonomyName: string; +} + +/** A single node of this page's tree: either a real tag (from `TagTree`) or + * the synthetic taxonomy-root node this component wraps it in. Narrower than + * `TagTreeNode`/`TagData` since this page only ever reads `id`, `value`, + * `externalId`, and `subRows`. + */ +export interface CompetencyTreeNode { + id: string | number; + value: string; + externalId?: string | null; + subRows?: CompetencyTreeNode[]; +} + +const ROOT_ID = 'taxonomy-root'; + +/** Either the successfully-built tree, or a marker that building it failed. */ +type TreeBuildResult = { treeData: CompetencyTreeNode[]; } | { treeBuildError: true; }; + +/** Collects the string ids of every node (including the root passed in) + * that has at least one child, i.e. every node that can be expanded. + */ +function collectExpandableIds(node: CompetencyTreeNode): string[] { + if (!node.subRows?.length) { + return []; + } + return [ + String(node.id), + ...node.subRows.flatMap(collectExpandableIds), + ]; +} + +const CompetencyTree = ({ taxonomyId, taxonomyName }: CompetencyTreeProps) => { + const intl = useIntl(); + const { isLoading, isError, data: tagList } = useTagListData(taxonomyId, { + pageIndex: 0, + disablePagination: true, + enabled: true, + }); + + // The taxonomy root starts expanded so top-level competencies are visible + // on first load, matching the existing tag-editing page's precedent of + // always showing top-level tags; deeper nodes still start collapsed. + const [expandedIds, setExpandedIds] = useState>(() => new Set([ROOT_ID])); + + const treeBuildResult = useMemo((): TreeBuildResult => { + if (!tagList?.results) { + return { treeData: [] }; + } + try { + return { treeData: new TagTree(tagList.results).getAllAsDeepCopy() as unknown as CompetencyTreeNode[] }; + } catch { + // `TagTree`'s constructor throws `TagTreeError` on duplicate tag values or a + // cycle in the data. `TaxonomyLayout` has no error boundary, so an uncaught + // throw here would break the whole page; render a connection-error state instead. + return { treeBuildError: true }; + } + }, [tagList?.results]); + + // The taxonomy itself is the tree's root row, one level above the tags + // returned by the API: the ticket's acceptance criteria calls for a nested + // tree of "root, groups, sub-competencies", where "root" is the taxonomy's + // own name, not just its starting point in the breadcrumb above the tree. + // This node is a display-only construct - it's never part of `TagData`/the + // API response, and nothing here writes it back. + const rootNode: CompetencyTreeNode | null = useMemo( + () => ('treeBuildError' in treeBuildResult + ? null + : { id: ROOT_ID, value: taxonomyName, subRows: treeBuildResult.treeData }), + [treeBuildResult, taxonomyName], + ); + + const allExpandableIds = useMemo( + () => (rootNode ? collectExpandableIds(rootNode) : []), + [rootNode], + ); + + const handleToggle = (id: string) => { + setExpandedIds((prev) => { + const next = new Set(prev); + if (next.has(id)) { + next.delete(id); + } else { + next.add(id); + } + return next; + }); + }; + + const isAllExpanded = allExpandableIds.length > 0 + && allExpandableIds.every((id) => expandedIds.has(id)); + + const handleToggleAll = () => { + setExpandedIds(isAllExpanded ? new Set() : new Set(allExpandableIds)); + }; + + if ('treeBuildError' in treeBuildResult || !rootNode || isError) { + return ; + } + + const rootHasChildren = !!rootNode.subRows?.length; + const rootIsExpanded = expandedIds.has(ROOT_ID); + + return ( +
                +
                + +
                + {isLoading ? + ( +
                + +
                + ) : + ( +
                +
                + handleToggle(ROOT_ID)} + expandLabel={intl.formatMessage(messages.expandRowButtonLabel)} + collapseLabel={intl.formatMessage(messages.collapseRowButtonLabel)} + /> + {taxonomyName} +
                + {rootIsExpanded && rootHasChildren && ( +
                  + {rootNode.subRows!.map((group) => ( + + ))} +
                + )} +
                + )} +
                + ); +}; + +export default CompetencyTree; diff --git a/src/taxonomy/competency-management/CompetencyTreeItem.tsx b/src/taxonomy/competency-management/CompetencyTreeItem.tsx new file mode 100644 index 0000000000..2f95150f8e --- /dev/null +++ b/src/taxonomy/competency-management/CompetencyTreeItem.tsx @@ -0,0 +1,84 @@ +import { useIntl } from '@edx/frontend-platform/i18n'; +import { Badge } from '@openedx/paragon'; + +import ExpandCollapseIconButton from '@src/generic/ExpandCollapseIconButton'; +import type { CompetencyTreeNode } from './CompetencyTree'; +import messages from './messages'; + +export interface CompetencyTreeItemProps { + node: CompetencyTreeNode; + expandedIds: Set; + onToggle: (id: string) => void; +} + +/** CompetencyTreeItem + * Recursively renders one node of the competency tree (a group or a + * sub-competency leaf, and anything nested deeper than that) as an `
              • `: + * a row (disclosure button + name + Competency ID badge) and, when expanded + * and the node has children, a nested `
                  ` of more `CompetencyTreeItem`s. + * + * Indentation comes from the `
                    ` nesting itself (see `CompetencyTree.scss`), + * not from a depth number computed here. Whether a node's `
                  • ` reads + * visually as a grey "group" envelope (wrapping its own header row and all + * of its rendered children) or as a plain white leaf "pill" is likewise + * driven by this same `hasChildren` check, via the `competency-group` class + * below - not a depth number or a CSS structural-position selector, so it + * holds recursively at any depth. + */ +const CompetencyTreeItem = ({ node, expandedIds, onToggle }: CompetencyTreeItemProps) => { + const intl = useIntl(); + const nodeId = String(node.id); + const hasChildren = !!node.subRows?.length; + const isExpanded = expandedIds.has(nodeId); + + // `externalId` can be `undefined`, `null`, or `''` depending on the data + // path (the tag-list tree's JSON deep-copy round-trip drops `undefined` + // keys but keeps `null`, and newly-created tags elsewhere are seeded with + // `externalId: ''`), so use a truthiness check rather than `!== null`. + const { externalId } = node; + + return ( +
                  • +
                    + onToggle(nodeId)} + expandLabel={intl.formatMessage(messages.expandRowButtonLabel)} + collapseLabel={intl.formatMessage(messages.collapseRowButtonLabel)} + /> + {node.value} + {externalId ? + ( + <> + + {intl.formatMessage(messages.competencyIdAccessibleLabel, { externalId })} + + + + ) : + null} +
                    + {isExpanded && hasChildren && ( +
                      + {node.subRows!.map((child) => ( + + ))} +
                    + )} +
                    • + ); +}; + +export default CompetencyTreeItem; diff --git a/src/taxonomy/competency-management/index.ts b/src/taxonomy/competency-management/index.ts new file mode 100644 index 0000000000..3819d813ab --- /dev/null +++ b/src/taxonomy/competency-management/index.ts @@ -0,0 +1 @@ +export { default as CompetencyManagementPage } from './CompetencyManagementPage'; diff --git a/src/taxonomy/competency-management/messages.ts b/src/taxonomy/competency-management/messages.ts new file mode 100644 index 0000000000..66be0b3a1e --- /dev/null +++ b/src/taxonomy/competency-management/messages.ts @@ -0,0 +1,36 @@ +import { defineMessages } from '@edx/frontend-platform/i18n'; + +const messages = defineMessages({ + headerTitle: { + id: 'course-authoring.competency-management.header.title', + defaultMessage: 'Competencies', + description: 'Document title suffix for the Competency Management page.', + }, + expandAll: { + id: 'course-authoring.competency-management.expand-all', + defaultMessage: 'Expand All', + description: 'Label for the button that expands every node of the competency tree.', + }, + collapseAll: { + id: 'course-authoring.competency-management.collapse-all', + defaultMessage: 'Collapse All', + description: 'Label for the button that collapses every node of the competency tree.', + }, + expandRowButtonLabel: { + id: 'course-authoring.competency-management.expand-row.button-label', + defaultMessage: 'Expand', + description: 'Accessible label for the disclosure button on a collapsed competency tree row.', + }, + collapseRowButtonLabel: { + id: 'course-authoring.competency-management.collapse-row.button-label', + defaultMessage: 'Collapse', + description: 'Accessible label for the disclosure button on an expanded competency tree row.', + }, + competencyIdAccessibleLabel: { + id: 'course-authoring.competency-management.competency-id.accessible-label', + defaultMessage: 'Competency ID: {externalId}', + description: 'Accessible name for the Competency ID badge, read by screen readers since there is no column header to associate it with.', + }, +}); + +export default messages; diff --git a/src/taxonomy/data/apiHooks.ts b/src/taxonomy/data/apiHooks.ts index fd40a10097..7a7d1ad565 100644 --- a/src/taxonomy/data/apiHooks.ts +++ b/src/taxonomy/data/apiHooks.ts @@ -192,7 +192,7 @@ export const useImportPlan = (taxonomyId: number, file: File | null) => * Use the list of tags in a taxonomy. */ export const useTagListData = (taxonomyId: number, options: QueryOptions) => { - const { pageIndex, pageSize, enabled = true, disablePagination = false } = options; // eslint-disable-line + const { pageIndex, pageSize = null, enabled = true, disablePagination = false } = options; // eslint-disable-line return useQuery({ // queryKey: taxonomyQueryKeys.taxonomyTagListPage(taxonomyId, pageIndex, pageSize), queryKey: taxonomyQueryKeys.taxonomyTagList(taxonomyId), // For now, ignore pagination in the query key. diff --git a/src/taxonomy/data/types.ts b/src/taxonomy/data/types.ts index 5a91cfdbf8..6bed6fdd1c 100644 --- a/src/taxonomy/data/types.ts +++ b/src/taxonomy/data/types.ts @@ -31,7 +31,9 @@ export interface TaxonomyListData { export interface QueryOptions { pageIndex: number; - pageSize: number; + // Unused (and thus omitted from the request URL) when `disablePagination` + // is true; see `apiUrls.tagList` in `./api.ts`. + pageSize?: number; enabled?: boolean; disablePagination?: boolean; } diff --git a/src/taxonomy/index.ts b/src/taxonomy/index.ts index 1a62229c51..495cc08a7f 100644 --- a/src/taxonomy/index.ts +++ b/src/taxonomy/index.ts @@ -1,3 +1,4 @@ +export { CompetencyManagementPage } from './competency-management'; export { TaxonomyDetailPage } from './taxonomy-detail'; export { TaxonomyLayout } from './TaxonomyLayout'; export { TaxonomyListPage } from './TaxonomyListPage'; diff --git a/src/taxonomy/tag-list/OptionalExpandLink.tsx b/src/taxonomy/tag-list/OptionalExpandLink.tsx index e42c198dcb..1a08cf81ef 100644 --- a/src/taxonomy/tag-list/OptionalExpandLink.tsx +++ b/src/taxonomy/tag-list/OptionalExpandLink.tsx @@ -1,9 +1,8 @@ import React from 'react'; -import { IconButton } from '@openedx/paragon'; -import { ExpandLess, ExpandMore } from '@openedx/paragon/icons'; import { Row } from '@tanstack/react-table'; import { useIntl } from '@edx/frontend-platform/i18n'; +import ExpandCollapseIconButton from '@src/generic/ExpandCollapseIconButton'; import type { TreeRowData } from '@src/taxonomy/tree-table/types'; import messages from './messages'; @@ -21,34 +20,15 @@ interface OptionalExpandLinkProps { const OptionalExpandLink = ({ row, forceHide = false }: OptionalExpandLinkProps) => { const intl = useIntl(); const canExpand = !!row?.getCanExpand() && !forceHide; - - if (!canExpand) { - return ( - - ); - } - const isExpanded = !!row?.getIsExpanded(); - const buttonLabel = isExpanded - ? intl.formatMessage(messages.hideSubtagsButtonLabel) - : intl.formatMessage(messages.showSubtagsButtonLabel); return ( - ); From faea5b70b1ce07a15b8c8cc623a7503595a03081 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 11 Sep 2026 14:23:44 -0700 Subject: [PATCH 11/16] chore(deps): bump colord from 2.9.3 to 2.10.0 (#3233) Bumps [colord](https://github.com/omgovich/colord) from 2.9.3 to 2.10.0. - [Release notes](https://github.com/omgovich/colord/releases) - [Changelog](https://github.com/omgovich/colord/blob/master/CHANGELOG.md) - [Commits](https://github.com/omgovich/colord/commits/v2.10) --- updated-dependencies: - dependency-name: colord dependency-version: 2.10.0 dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9d788ea393..0793aebbec 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10274,9 +10274,9 @@ "license": "MIT" }, "node_modules/colord": { - "version": "2.9.3", - "resolved": "https://registry.npmjs.org/colord/-/colord-2.9.3.tgz", - "integrity": "sha512-jeC1axXpnb0/2nn/Y1LPuLdgXBLH7aDcHu4KEKfqw3CUhX7ZpfBSlPKyqXE6btIgEzfWtrX3/tyBCaCvXvMkOw==", + "version": "2.10.0", + "resolved": "https://registry.npmjs.org/colord/-/colord-2.10.0.tgz", + "integrity": "sha512-AidJptpBJmjTclAp9BkLwJi0T93fo5epJnbaZslpg6QVzpHjAiveF55mE9AcUJiGMqRHgMDY8soMsQtuNYMHfw==", "license": "MIT" }, "node_modules/colorette": { From f71c40125408f8aa09b655108d65bf0696a1859f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 11 Sep 2026 14:25:06 -0700 Subject: [PATCH 12/16] chore(deps): bump svgo from 3.3.4 to 3.3.5 (#3236) Bumps [svgo](https://github.com/svg/svgo) from 3.3.4 to 3.3.5. - [Release notes](https://github.com/svg/svgo/releases) - [Commits](https://github.com/svg/svgo/compare/v3.3.4...v3.3.5) --- updated-dependencies: - dependency-name: svgo dependency-version: 3.3.5 dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 0793aebbec..888026d840 100644 --- a/package-lock.json +++ b/package-lock.json @@ -23289,9 +23289,9 @@ "dev": true }, "node_modules/svgo": { - "version": "3.3.4", - "resolved": "https://registry.npmjs.org/svgo/-/svgo-3.3.4.tgz", - "integrity": "sha512-GsNRis4e8jxn2Y9ENz/8lbJ93CstG8svtMnuRaHbiF2LTJ5tK0/q3t/URPq9Zc7zVWBJnNnJMIp6bevK7bSmNg==", + "version": "3.3.5", + "resolved": "https://registry.npmjs.org/svgo/-/svgo-3.3.5.tgz", + "integrity": "sha512-8SQMzdrvWaD8deUmrnYB+ASyxBVgWUOilg+A75nE/76WdLpj6LopCwiAVvkzkcqy/9b7t2Mg7faFLjg0ZRcZ3w==", "license": "MIT", "dependencies": { "commander": "^7.2.0", From 7e62e777b59ab688f8ca11c6c25077103f116e62 Mon Sep 17 00:00:00 2001 From: Jacobo Dominguez Date: Fri, 11 Sep 2026 15:25:55 -0600 Subject: [PATCH 13/16] feat: adding permissions checks related to course section and units (#3200) --- .../course-apps/proctoring/Settings.test.jsx | 12 +- src/course-outline/OutlineNode.tsx | 7 +- src/course-outline/outline-level.ts | 4 +- .../info-sidebar/InfoSidebar.test.tsx | 2 + .../info-sidebar/PublishButon.test.tsx | 56 ++++ .../info-sidebar/PublishButon.tsx | 35 ++- .../sharedSettings/VisibilitySection.test.tsx | 52 +++- .../sharedSettings/VisibilitySection.tsx | 5 + src/course-unit/CourseUnit.test.tsx | 113 ++++++- src/course-unit/CourseUnit.tsx | 7 +- .../SequenceNavigationDropdown.jsx | 27 +- .../SequenceNavigationTabs.jsx | 6 +- .../HeaderNavigations.test.tsx | 49 ++- .../header-navigations/HeaderNavigations.tsx | 8 +- .../header-title/HeaderTitle.test.tsx | 70 ++++- src/course-unit/header-title/HeaderTitle.tsx | 17 +- .../sidebar-footer/ActionButtons.test.jsx | 76 ++++- .../sidebar-footer/ActionButtons.tsx | 9 +- .../unit-sidebar/UnitAlignSidebar.test.tsx | 15 +- .../UnitSidebarPagesContext.test.tsx | 120 +++++++ .../unit-sidebar/UnitSidebarPagesContext.tsx | 11 +- .../unit-info/ComponentInfoSidebar.test.tsx | 2 + .../GenericUnitInfoSettings.test.tsx | 45 ++- .../unit-info/GenericUnitInfoSettings.tsx | 4 + .../unit-info/UnitInfoSidebar.test.tsx | 2 + .../unit-info/UnitVisibilityInfo.test.tsx | 114 +++++++ .../unit-info/UnitVisibilityInfo.tsx | 19 +- .../configure-modal/ConfigureModal.test.tsx | 292 ++++++++++-------- .../configure-modal/ConfigureModal.tsx | 30 +- src/generic/configure-modal/UnitTab.tsx | 30 +- src/generic/sidebar/SidebarTitle.test.tsx | 118 +++++++ src/generic/sidebar/SidebarTitle.tsx | 4 +- 32 files changed, 1092 insertions(+), 269 deletions(-) create mode 100644 src/course-outline/outline-sidebar/info-sidebar/PublishButon.test.tsx create mode 100644 src/course-unit/unit-sidebar/UnitSidebarPagesContext.test.tsx create mode 100644 src/course-unit/unit-sidebar/unit-info/UnitVisibilityInfo.test.tsx create mode 100644 src/generic/sidebar/SidebarTitle.test.tsx diff --git a/plugins/course-apps/proctoring/Settings.test.jsx b/plugins/course-apps/proctoring/Settings.test.jsx index 4ee6953dad..4d137efc98 100644 --- a/plugins/course-apps/proctoring/Settings.test.jsx +++ b/plugins/course-apps/proctoring/Settings.test.jsx @@ -470,11 +470,13 @@ describe('ProctoredExamSettings', () => { await waitFor(() => { screen.getByDisplayValue('mockproc'); }); - // (1) for studio settings - // (2) for course details - // (3) for user course permissions - expect(axiosMock.history.get.length).toBe(3); - expect(axiosMock.history.get[0].url.includes('proctored_exam_settings')).toEqual(true); + // With no exam service URL configured, no request should be made to the exams + // service for provider options. (Total GET count is not asserted because the + // CourseAuthoringProvider also fetches course details and waffle flags.) + const examProvidersRequested = axiosMock.history.get.some((req) => req.url.includes('/api/v1/providers')); + expect(examProvidersRequested).toBe(false); + const studioSettingsRequested = axiosMock.history.get.some((req) => req.url.includes('proctored_exam_settings')); + expect(studioSettingsRequested).toBe(true); }); it('Selected LTI proctoring provider is shown on page load', async () => { diff --git a/src/course-outline/OutlineNode.tsx b/src/course-outline/OutlineNode.tsx index 625233e705..934b0cdf0a 100644 --- a/src/course-outline/OutlineNode.tsx +++ b/src/course-outline/OutlineNode.tsx @@ -347,7 +347,12 @@ const OutlineNode = ({ readyToSync={blk.upstreamInfo?.readyToSync} />
                    onClickCard(e, false)} > diff --git a/src/course-outline/outline-level.ts b/src/course-outline/outline-level.ts index aea3ad9e0a..15ae1587fc 100644 --- a/src/course-outline/outline-level.ts +++ b/src/course-outline/outline-level.ts @@ -89,7 +89,7 @@ export const LEVEL_CONFIG: Record = { }, 1: { name: 'subsection', - contentClass: 'subsection-card__content item-children', + contentClass: 'subsection-card__content', contentTestId: 'subsection-card__content', childContainerClass: 'subsection-card__units', childContainerTestId: 'subsection-card__units', @@ -99,7 +99,7 @@ export const LEVEL_CONFIG: Record = { }, 2: { name: 'unit', - contentClass: 'unit-card__content item-children', + contentClass: 'unit-card__content', contentTestId: 'unit-card__content', iconSize: 'xs', background: { background: '#fdfdfd' }, diff --git a/src/course-outline/outline-sidebar/info-sidebar/InfoSidebar.test.tsx b/src/course-outline/outline-sidebar/info-sidebar/InfoSidebar.test.tsx index 9f79791c92..2db6461454 100644 --- a/src/course-outline/outline-sidebar/info-sidebar/InfoSidebar.test.tsx +++ b/src/course-outline/outline-sidebar/info-sidebar/InfoSidebar.test.tsx @@ -56,6 +56,8 @@ jest.mock('@src/CourseAuthoringContext', () => ({ courseId, openUnlinkModal, getUnitUrl: jest.fn(), + canEditCourseContent: true, + canPublishCourseContent: true, }), })); diff --git a/src/course-outline/outline-sidebar/info-sidebar/PublishButon.test.tsx b/src/course-outline/outline-sidebar/info-sidebar/PublishButon.test.tsx new file mode 100644 index 0000000000..d9f81857a8 --- /dev/null +++ b/src/course-outline/outline-sidebar/info-sidebar/PublishButon.test.tsx @@ -0,0 +1,56 @@ +import { + initializeMocks, + render, + screen, + userEvent, +} from '@src/testUtils'; + +import messages from '../messages'; +import { PublishButon } from './PublishButon'; + +const mockCourseAuthoringContext = { + canPublishCourseContent: true, +}; + +jest.mock('@src/CourseAuthoringContext', () => ({ + useCourseAuthoringContext: () => mockCourseAuthoringContext, +})); + +const onClickMock = jest.fn(); + +// The button's accessible name is the publish label plus the draft status, +// e.g. "Publish Changes (Draft)". +const publishButtonName = new RegExp(messages.publishContainerButton.defaultMessage, 'i'); + +describe('', () => { + beforeEach(() => { + initializeMocks(); + mockCourseAuthoringContext.canPublishCourseContent = true; + onClickMock.mockClear(); + }); + + it('renders the publish button when the user can publish course content', async () => { + render(); + + expect( + await screen.findByRole('button', { name: publishButtonName }), + ).toBeInTheDocument(); + }); + + it('calls onClick when the button is clicked', async () => { + const user = userEvent.setup(); + render(); + + await user.click(await screen.findByRole('button', { name: publishButtonName })); + expect(onClickMock).toHaveBeenCalledTimes(1); + }); + + it('does not render the button when the user cannot publish course content', () => { + mockCourseAuthoringContext.canPublishCourseContent = false; + render(); + + expect( + screen.queryByRole('button', { name: publishButtonName }), + ).not.toBeInTheDocument(); + }); +}); diff --git a/src/course-outline/outline-sidebar/info-sidebar/PublishButon.tsx b/src/course-outline/outline-sidebar/info-sidebar/PublishButon.tsx index c932a577d1..eaab37d8dc 100644 --- a/src/course-outline/outline-sidebar/info-sidebar/PublishButon.tsx +++ b/src/course-outline/outline-sidebar/info-sidebar/PublishButon.tsx @@ -1,22 +1,29 @@ import { FormattedMessage } from '@edx/frontend-platform/i18n'; import { Button } from '@openedx/paragon'; +import { useCourseAuthoringContext } from '@src/CourseAuthoringContext'; import messages from '../messages'; interface Props { onClick: () => void; } -export const PublishButon = ({ onClick }: Props) => ( - -); +export const PublishButon = ({ onClick }: Props) => { + const { canPublishCourseContent } = useCourseAuthoringContext(); + + if (!canPublishCourseContent) { + return null; + } + + return ( + + ); +}; diff --git a/src/course-outline/outline-sidebar/info-sidebar/sharedSettings/VisibilitySection.test.tsx b/src/course-outline/outline-sidebar/info-sidebar/sharedSettings/VisibilitySection.test.tsx index 3ec946a0cc..9607073fef 100644 --- a/src/course-outline/outline-sidebar/info-sidebar/sharedSettings/VisibilitySection.test.tsx +++ b/src/course-outline/outline-sidebar/info-sidebar/sharedSettings/VisibilitySection.test.tsx @@ -7,7 +7,9 @@ import { import userEvent from '@testing-library/user-event'; import { useCourseItemData } from '@src/course-outline/data/apiHooks'; import { VisibilityTypes } from '@src/data/constants'; +import { mockWaffleFlags } from '@src/data/apiHooks.mock'; import { VisibilitySection } from './VisibilitySection'; +import { CourseAuthoringProvider } from '@src/CourseAuthoringContext'; jest.mock('@src/course-outline/data/apiHooks', () => ({ ...jest.requireActual('@src/course-outline/data/apiHooks'), @@ -22,14 +24,30 @@ const defaultProps = { onChange: jest.fn(), }; +const WrapperProvider = ({ children }) => ( + {children} +); +const renderWithWrapper = (children) => { + render(children, { + extraWrapper: WrapperProvider, + }); +}; + +let validateUserPermissionsMock; + describe('VisibilitySection component', () => { beforeEach(() => { - initializeMocks(); + const mocks = initializeMocks(); mockUseCourseItemData.mockReturnValue({ data: undefined }); + mockWaffleFlags({ enableAuthzCourseAuthoring: true }); + validateUserPermissionsMock = mocks.validateUserPermissionsMock; + validateUserPermissionsMock.mockResolvedValue({ + canEditCourseContent: true, + }); }); it('renders title and buttons', async () => { - render(); + renderWithWrapper(); expect(await screen.findByText('Visibility')).toBeInTheDocument(); expect(await screen.findByRole('button', { name: 'Student Visible' })).toBeInTheDocument(); expect(await screen.findByRole('button', { name: 'Staff Only' })).toBeInTheDocument(); @@ -38,7 +56,7 @@ describe('VisibilitySection component', () => { it('clicking staff only calls onChange with staff and hideAfterDue false', async () => { const user = userEvent.setup(); const onChange = jest.fn(); - render(); + renderWithWrapper(); await user.click(await screen.findByRole('button', { name: 'Staff Only' })); await waitFor(async () => { @@ -50,7 +68,7 @@ describe('VisibilitySection component', () => { const user = userEvent.setup(); const onChange = jest.fn(); mockUseCourseItemData.mockReturnValue({ data: { visibilityState: VisibilityTypes.STAFF_ONLY } }); - render(); + renderWithWrapper(); await user.click(await screen.findByRole('button', { name: 'Student Visible' })); await waitFor(async () => { @@ -63,7 +81,7 @@ describe('VisibilitySection component', () => { const onChange = jest.fn(); // initial data not staff only mockUseCourseItemData.mockReturnValue({ data: { visibilityState: undefined, hideAfterDue: false } }); - render(); + renderWithWrapper(); const checkbox = await screen.findByRole('checkbox'); await user.click(checkbox); @@ -76,7 +94,7 @@ describe('VisibilitySection component', () => { const user = userEvent.setup(); const onChange = jest.fn(); mockUseCourseItemData.mockReturnValue({ data: { visibilityState: undefined, hideAfterDue: true } }); - render(); + renderWithWrapper(); await user.click(await screen.findByRole('button', { name: 'Staff Only' })); await waitFor(async () => { @@ -88,7 +106,27 @@ describe('VisibilitySection component', () => { const onChange = jest.fn(); // when item is staff only, checkbox should not be present mockUseCourseItemData.mockReturnValue({ data: { visibilityState: VisibilityTypes.STAFF_ONLY } }); - render(); + renderWithWrapper(); expect(screen.queryByRole('checkbox')).not.toBeInTheDocument(); }); + + it('disables both visibility buttons when the user cannot edit course content', async () => { + validateUserPermissionsMock.mockResolvedValue({ + canEditCourseContent: false, + }); + renderWithWrapper(); + + expect(await screen.findByRole('button', { name: 'Student Visible' })).toBeDisabled(); + expect(await screen.findByRole('button', { name: 'Staff Only' })).toBeDisabled(); + }); + + it('disables the hide-after-due checkbox when the user cannot edit course content', async () => { + validateUserPermissionsMock.mockResolvedValue({ + canEditCourseContent: false, + }); + mockUseCourseItemData.mockReturnValue({ data: { visibilityState: undefined, hideAfterDue: false } }); + renderWithWrapper(); + + expect(await screen.findByRole('checkbox')).toBeDisabled(); + }); }); diff --git a/src/course-outline/outline-sidebar/info-sidebar/sharedSettings/VisibilitySection.tsx b/src/course-outline/outline-sidebar/info-sidebar/sharedSettings/VisibilitySection.tsx index 75793104fb..fc69365bd8 100644 --- a/src/course-outline/outline-sidebar/info-sidebar/sharedSettings/VisibilitySection.tsx +++ b/src/course-outline/outline-sidebar/info-sidebar/sharedSettings/VisibilitySection.tsx @@ -7,6 +7,7 @@ import { SidebarSection } from '@src/generic/sidebar'; import { useFieldDraft } from '@src/hooks/useFieldDraft'; import { useMemo } from 'react'; import messages from '../messages'; +import { useCourseAuthoringContext } from '@src/CourseAuthoringContext'; interface Props> { itemId: string; @@ -22,6 +23,7 @@ interface State { export const VisibilitySection = ({ itemId, isSubsection, onChange }: Props) => { const intl = useIntl(); const { data: itemData } = useCourseItemData(itemId); + const { canEditCourseContent } = useCourseAuthoringContext(); const serverState = useMemo(() => ({ isVisibleToStaffOnly: itemData?.visibilityState === VisibilityTypes.STAFF_ONLY, @@ -42,12 +44,14 @@ export const VisibilitySection = ({ itemId, isSubsection, onChange }: Props) => > - {showPasteUnit && ( + {canEditCourseContent && + ( + + )} + {canEditCourseContent && showPasteUnit && ( )} diff --git a/src/course-unit/course-sequence/sequence-navigation/SequenceNavigationTabs.jsx b/src/course-unit/course-sequence/sequence-navigation/SequenceNavigationTabs.jsx index 0f4ca66bff..b57deff504 100644 --- a/src/course-unit/course-sequence/sequence-navigation/SequenceNavigationTabs.jsx +++ b/src/course-unit/course-sequence/sequence-navigation/SequenceNavigationTabs.jsx @@ -23,7 +23,7 @@ const SequenceNavigationTabs = ({ const dispatch = useDispatch(); const navigate = useNavigate(); const sequenceId = useSelector(getSequenceId); - const { courseId } = useCourseAuthoringContext(); + const { courseId, canEditCourseContent } = useCourseAuthoringContext(); const courseUnit = useSelector(getCourseUnitData); const sequenceChildAddable = courseUnit?.ancestorInfo?.ancestors?.[0]?.actions?.childAddable; @@ -67,7 +67,7 @@ const SequenceNavigationTabs = ({ isActive={unitId === buttonUnitId} /> ))} - {sequenceChildAddable && ( + {canEditCourseContent && sequenceChildAddable && ( - {!readOnly && ( + {canEditCourseContent && !readOnly && ( - - + {canEditCourseContent && ( + + + + {intl.formatMessage(messages.cancelButton)} + + + + + )} )} diff --git a/src/generic/configure-modal/UnitTab.tsx b/src/generic/configure-modal/UnitTab.tsx index 852e9c9b99..83b01004ea 100644 --- a/src/generic/configure-modal/UnitTab.tsx +++ b/src/generic/configure-modal/UnitTab.tsx @@ -8,6 +8,7 @@ import classNames from 'classnames'; import { COURSE_BLOCK_NAMES } from '../../constants'; import messages from './messages'; +import { useCourseAuthoringContext } from '@src/CourseAuthoringContext'; export type UserPartitionInfo = { selectablePartitions: { @@ -45,16 +46,19 @@ export const DiscussionEditComponent = ({ }: { discussionEnabled: boolean; handleDiscussionChange: (e: any) => void; -}) => ( - <> - - - -

                    - -

                    - -); +}) => { + const { canEditCourseContent } = useCourseAuthoringContext(); + return ( + <> + + + +

                    + +

                    + + ); +}; export interface AccessEditComponentProps { selectedPartitionIndex?: number; @@ -70,6 +74,7 @@ export const AccessEditComponent = ({ selectedGroups, }: AccessEditComponentProps) => { const intl = useIntl(); + const { canEditCourseContent } = useCourseAuthoringContext(); const checkIsDeletedGroup = (group) => { const isGroupSelected = selectedGroups.includes(group.id.toString()); @@ -92,6 +97,7 @@ export const AccessEditComponent = ({ value={selectedPartitionIndex} onChange={handleSelect} data-testid="group-type-select" + disabled={!canEditCourseContent} >