From c4b90ae9a248116e2d38b81aaa3d5893fd0742e9 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Wed, 18 Jun 2025 07:06:07 +0930 Subject: [PATCH 01/10] refactor: move draftChipText message to containers so we can use it in both containers and section-subsection --- src/library-authoring/containers/messages.ts | 5 +++++ .../section-subsections/LibraryContainerChildren.tsx | 2 +- src/library-authoring/section-subsections/messages.ts | 5 ----- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/library-authoring/containers/messages.ts b/src/library-authoring/containers/messages.ts index 1dec3cafc1..f9295c8df9 100644 --- a/src/library-authoring/containers/messages.ts +++ b/src/library-authoring/containers/messages.ts @@ -1,6 +1,11 @@ import { defineMessages } from '@edx/frontend-platform/i18n'; const messages = defineMessages({ + draftChipText: { + id: 'course-authoring.library-authoring.container-component.draft-chip.text', + defaultMessage: 'Draft', + description: 'Chip in children in section and subsection page that is shown when children has unpublished changes', + }, openButton: { id: 'course-authoring.library-authoring.container-sidebar.open-button', defaultMessage: 'Open', diff --git a/src/library-authoring/section-subsections/LibraryContainerChildren.tsx b/src/library-authoring/section-subsections/LibraryContainerChildren.tsx index cd29078c1c..257cf24f66 100644 --- a/src/library-authoring/section-subsections/LibraryContainerChildren.tsx +++ b/src/library-authoring/section-subsections/LibraryContainerChildren.tsx @@ -80,7 +80,7 @@ const ContainerRow = ({ container, readOnly }: ContainerRowProps) => { > - + )} diff --git a/src/library-authoring/section-subsections/messages.ts b/src/library-authoring/section-subsections/messages.ts index 896b4cbf6a..5b038dbc66 100644 --- a/src/library-authoring/section-subsections/messages.ts +++ b/src/library-authoring/section-subsections/messages.ts @@ -16,11 +16,6 @@ export const messages = defineMessages({ defaultMessage: 'Failed to update children order', description: 'Toast message displayed when reordering of children items in container fails', }, - draftChipText: { - id: 'course-authoring.library-authoring.container-component.draft-chip.text', - defaultMessage: 'Draft', - description: 'Chip in children in section and subsection page that is shown when children has unpublished changes', - }, }); export const sectionMessages = defineMessages({ From caef8dfd69d629b3a560ed64c5414371add27989 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Wed, 18 Jun 2025 07:06:38 +0930 Subject: [PATCH 02/10] refactor: make status widget styles extendable --- .../generic/status-widget/StatusWidget.scss | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/src/library-authoring/generic/status-widget/StatusWidget.scss b/src/library-authoring/generic/status-widget/StatusWidget.scss index cdb1c4cf61..642fcd429b 100644 --- a/src/library-authoring/generic/status-widget/StatusWidget.scss +++ b/src/library-authoring/generic/status-widget/StatusWidget.scss @@ -1,12 +1,26 @@ +%draft-status { + background-color: #FDF3E9; + border-color: #F4B57B !important; + color: #00262B; +} + +%published-status { + background-color: $info-100; + border-color: $info-400 !important; + color: $primary-500; +} + .status-widget { + border-top: 4px solid; + border-left: none; + border-right: none; + border-bottom: none; + &.draft-status { - background-color: #FDF3E9; - border-top: 4px solid #F4B57B; + @extend %draft-status; } &.published-status { - background-color: $info-100; - border-top: 4px solid $info-400; + @extend %published-status; } } - From 583a30ab517a4726c04802491ec11c7d937e0cb8 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Wed, 18 Jun 2025 07:18:19 +0930 Subject: [PATCH 03/10] feat: show container publish status and confirm publish box (text only) --- .../containers/ContainerInfo.test.tsx | 110 ++++++++++++-- .../containers/ContainerInfo.tsx | 30 +--- .../containers/ContainerPublishStatus.scss | 27 ++++ .../containers/ContainerPublishStatus.tsx | 143 ++++++++++++++++++ src/library-authoring/containers/index.scss | 1 + src/library-authoring/containers/messages.ts | 22 ++- src/library-authoring/index.scss | 1 + 7 files changed, 300 insertions(+), 34 deletions(-) create mode 100644 src/library-authoring/containers/ContainerPublishStatus.scss create mode 100644 src/library-authoring/containers/ContainerPublishStatus.tsx create mode 100644 src/library-authoring/containers/index.scss diff --git a/src/library-authoring/containers/ContainerInfo.test.tsx b/src/library-authoring/containers/ContainerInfo.test.tsx index 75c5cc91b3..387b10d8f7 100644 --- a/src/library-authoring/containers/ContainerInfo.test.tsx +++ b/src/library-authoring/containers/ContainerInfo.test.tsx @@ -6,15 +6,18 @@ import { fireEvent, } from '../../testUtils'; import { mockContentLibrary, mockGetContainerChildren, mockGetContainerMetadata } from '../data/api.mocks'; +import { mockContentSearchConfig, mockSearchResult } from '../../search-manager/data/api.mock'; import { LibraryProvider } from '../common/context/LibraryContext'; import ContainerInfo from './ContainerInfo'; import { getLibraryContainerApiUrl, getLibraryContainerPublishApiUrl } from '../data/api'; import { SidebarBodyItemId, SidebarProvider } from '../common/context/SidebarContext'; +import { PublishStatus } from '../../search-manager'; mockGetContainerMetadata.applyMock(); mockContentLibrary.applyMock(); mockGetContainerMetadata.applyMock(); mockGetContainerChildren.applyMock(); +mockContentSearchConfig.applyMock(); // TODO Remove this to un-skip section/subsection tests, when implemented const testIf = (condition) => (condition ? it : it.skip); @@ -22,10 +25,18 @@ const testIf = (condition) => (condition ? it : it.skip); const { libraryId } = mockContentLibrary; const { unitId, subsectionId, sectionId } = mockGetContainerMetadata; -const render = (containerId, showOnlyPublished: boolean = false) => { +const render = ( + containerId, + containerType: string = '', // renders container page + showOnlyPublished: boolean = false, +) => { const params: { libraryId: string, selectedItemId?: string } = { libraryId, selectedItemId: containerId }; + const path = containerType + ? `/library/:libraryId/${containerType.toLowerCase()}/:selectedItemId?` + : '/library/:libraryId/:selectedItemId?'; + return baseRender(, { - path: '/library/:libraryId/:selectedItemId?', + path, params, extraWrapper: ({ children }) => ( ', () => { beforeEach(() => { ({ axiosMock, mockShowToast } = initializeMocks()); + mockSearchResult({ + results: [ // @ts-ignore + { + hits: [], + }, + ], + }); }); [ @@ -90,14 +108,67 @@ describe('', () => { expect(mockShowToast).toHaveBeenCalled(); }); - it('can publish the container', async () => { + it(`shows Published if the ${containerType} has no draft changes`, async () => { axiosMock.onPost(getLibraryContainerPublishApiUrl(containerId)).reply(200); - render(containerId); + mockSearchResult({ + results: [ // @ts-ignore + { + hits: [ + { + type: 'library_container', + usageKey: containerId, + publishStatus: PublishStatus.Published, + }, + ], + }, + ], + }); + render(containerId, containerType); + + // "Published" status should be displayed + const publishedStatus = await screen.findByText('Published'); + expect(publishedStatus).toBeInTheDocument(); + }); + + it(`can publish the ${containerType} from the container page`, async () => { + axiosMock.onPost(getLibraryContainerPublishApiUrl(containerId)).reply(200); + mockSearchResult({ + results: [ // @ts-ignore + { + hits: [ + { + type: 'library_container', + usageKey: containerId, + publishStatus: PublishStatus.Modified, + }, + ], + }, + ], + }); + render(containerId, containerType); - // Click on Publish button - const publishButton = await screen.findByRole('button', { name: 'Publish' }); + // Click on Publish button to reveal the confirmation box + let publishButton = await screen.findByRole('button', { name: /publish changes/i }); expect(publishButton).toBeInTheDocument(); userEvent.click(publishButton); + expect(publishButton).not.toBeInTheDocument(); + + // Click on the confirm Cancel button + const publishCancel = await screen.findByRole('button', { name: 'Cancel' }); + expect(publishCancel).toBeInTheDocument(); + userEvent.click(publishCancel); + expect(axiosMock.history.post.length).toBe(0); + + // Click on Publish button again + publishButton = await screen.findByRole('button', { name: /publish changes/i }); + expect(publishButton).toBeInTheDocument(); + userEvent.click(publishButton); + expect(publishButton).not.toBeInTheDocument(); + + // Click on the confirm Publish button + const publishConfirm = await screen.findByRole('button', { name: 'Publish' }); + expect(publishConfirm).toBeInTheDocument(); + userEvent.click(publishConfirm); await waitFor(() => { expect(axiosMock.history.post.length).toBe(1); @@ -107,12 +178,31 @@ describe('', () => { it(`shows an error if publishing the ${containerType} fails`, async () => { axiosMock.onPost(getLibraryContainerPublishApiUrl(containerId)).reply(500); - render(containerId); + mockSearchResult({ + results: [ // @ts-ignore + { + hits: [ + { + type: 'library_container', + usageKey: containerId, + publishStatus: PublishStatus.Modified, + }, + ], + }, + ], + }); + render(containerId, containerType); - // Click on Publish button - const publishButton = await screen.findByRole('button', { name: 'Publish' }); + // Click on Publish button to reveal the confirmation box + const publishButton = await screen.findByRole('button', { name: /publish changes/i }); expect(publishButton).toBeInTheDocument(); userEvent.click(publishButton); + expect(publishButton).not.toBeInTheDocument(); + + // Click on the confirm Publish button + const publishConfirm = await screen.findByRole('button', { name: 'Publish' }); + expect(publishConfirm).toBeInTheDocument(); + userEvent.click(publishConfirm); await waitFor(() => { expect(axiosMock.history.post.length).toBe(1); @@ -121,7 +211,7 @@ describe('', () => { }); testIf(containerType === 'Unit')(`show only published ${containerType} content`, async () => { - render(containerId, true); + render(containerId, '', true); expect(await screen.findByTestId('container-info-menu-toggle')).toBeInTheDocument(); expect(screen.getByText(/text block published 1/i)).toBeInTheDocument(); }); diff --git a/src/library-authoring/containers/ContainerInfo.tsx b/src/library-authoring/containers/ContainerInfo.tsx index 8be15f53dd..298dabc471 100644 --- a/src/library-authoring/containers/ContainerInfo.tsx +++ b/src/library-authoring/containers/ContainerInfo.tsx @@ -28,9 +28,9 @@ import { LibraryContainerChildren } from '../section-subsections/LibraryContaine import messages from './messages'; import componentMessages from '../components/messages'; import ContainerDeleter from '../components/ContainerDeleter'; -import { useContainer, usePublishContainer } from '../data/apiHooks'; +import ContainerPublishStatus from './ContainerPublishStatus'; +import { useContainer } from '../data/apiHooks'; import { ContainerType, getBlockType } from '../../generic/key-utils'; -import { ToastContext } from '../../generic/toast-context'; type ContainerMenuProps = { containerId: string, @@ -85,9 +85,8 @@ const ContainerPreview = ({ containerId } : ContainerPreviewProps) => { const ContainerInfo = () => { const intl = useIntl(); - const { libraryId, readOnly } = useLibraryContext(); + const { libraryId } = useLibraryContext(); const { componentPickerMode } = useComponentPickerContext(); - const { showToast } = React.useContext(ToastContext); const { defaultTab, hiddenTabs, @@ -101,7 +100,6 @@ const ContainerInfo = () => { const containerId = sidebarItemInfo?.id; const containerType = containerId ? getBlockType(containerId) : undefined; const { data: container } = useContainer(containerId); - const publishContainer = usePublishContainer(containerId!); const defaultContainerTab = defaultTab.container; const tab: ContainerInfoTab = ( @@ -130,15 +128,6 @@ const ContainerInfo = () => { ); }, [hiddenTabs, defaultContainerTab, containerId]); - const handlePublish = useCallback(async () => { - try { - await publishContainer.mutateAsync(); - showToast(intl.formatMessage(messages.publishContainerSuccess)); - } catch (error) { - showToast(intl.formatMessage(messages.publishContainerFailed)); - } - }, [publishContainer]); - if (!container || !containerId || !containerType) { return null; } @@ -156,15 +145,10 @@ const ContainerInfo = () => { {intl.formatMessage(messages.openButton)} )} - {!componentPickerMode && !readOnly && ( - + {!showOpenButton && !componentPickerMode && ( + )} {showOpenButton && ( void; + container: ContainerHit; +}; + +const ContainerPublisher = ({ + close, + container, +}: ContainerPublisherProps) => { + const intl = useIntl(); + const publishContainer = usePublishContainer(container.usageKey); + const { showToast } = useContext(ToastContext); + + const handlePublish = useCallback(async () => { + try { + await publishContainer.mutateAsync(); + showToast(intl.formatMessage(messages.publishContainerSuccess)); + } catch (error) { + showToast(intl.formatMessage(messages.publishContainerFailed)); + } + close(); + }, [publishContainer, showToast]); + + return ( + +

{intl.formatMessage(messages.publishContainerConfirmHeading)}

+

+ { + // TODO show specific message for this container and children + 'Are you sure you want to publish this container?' + } +

+ + + { + e.preventDefault(); + e.stopPropagation(); + await handlePublish(); + }} + variant="primary rounded-0" + label={intl.formatMessage(messages.publishContainerConfirm)} + /> + + + ); +}; + +type ContainerPublishStatusProps = { + containerId: string; +}; + +const ContainerPublishStatus = ({ + containerId, +}: ContainerPublishStatusProps) => { + const intl = useIntl(); + const { readOnly } = useLibraryContext(); + const [isConfirmingPublish, confirmPublish, cancelPublish] = useToggle(false); + const { hits, isLoading, isError } = useContentFromSearchIndex([containerId]); + + if (isLoading) { + return ; + } + + // istanbul ignore if: this should never happen + if (isError || !hits) { + return undefined; + } + + // TODO -- why isn't this auto-updating when the container gets modified or published? + const container = (hits as ContainerHit[])[0]; + if (container.publishStatus === PublishStatus.Published) { + return ( + + {intl.formatMessage(messages.publishedChipText)} + + ); + } + + return ( + (isConfirmingPublish + ? ( + + ) : ( + + ) + ) + ); +}; + +export default ContainerPublishStatus; diff --git a/src/library-authoring/containers/index.scss b/src/library-authoring/containers/index.scss new file mode 100644 index 0000000000..46bba7d588 --- /dev/null +++ b/src/library-authoring/containers/index.scss @@ -0,0 +1 @@ +@import "./ContainerPublishStatus.scss"; diff --git a/src/library-authoring/containers/messages.ts b/src/library-authoring/containers/messages.ts index f9295c8df9..99f5029a44 100644 --- a/src/library-authoring/containers/messages.ts +++ b/src/library-authoring/containers/messages.ts @@ -6,6 +6,11 @@ const messages = defineMessages({ defaultMessage: 'Draft', description: 'Chip in children in section and subsection page that is shown when children has unpublished changes', }, + publishedChipText: { + id: 'course-authoring.library-authoring.container-component.published-chip.text', + defaultMessage: 'Published', + description: 'Text shown when a unit/section/subsection is published.', + }, openButton: { id: 'course-authoring.library-authoring.container-sidebar.open-button', defaultMessage: 'Open', @@ -33,8 +38,23 @@ const messages = defineMessages({ }, publishContainerButton: { id: 'course-authoring.library-authoring.container-sidebar.publish-button', + defaultMessage: 'Publish Changes {publishStatus}', + description: 'Button text to initiate publish the unit/subsection/section, showing current publish status', + }, + publishContainerConfirmHeading: { + id: 'course-authoring.library-authoring.container-sidebar.publish-confirm-heading', + defaultMessage: 'Confirm Publish', + description: 'Header text shown while confirming publish of a unit/subsection/section', + }, + publishContainerConfirm: { + id: 'course-authoring.library-authoring.container-sidebar.publish-confirm-button', defaultMessage: 'Publish', - description: 'Button text to publish the unit/subsection/section', + description: 'Button text shown to confirm publish of a unit/subsection/section', + }, + publishContainerCancel: { + id: 'course-authoring.library-authoring.container-sidebar.publish-cancel', + defaultMessage: 'Cancel', + description: 'Button text shown to cancel publish of a unit/subsection/section', }, publishContainerSuccess: { id: 'course-authoring.library-authoring.container-sidebar.publish-success', diff --git a/src/library-authoring/index.scss b/src/library-authoring/index.scss index 1de9533738..a93dd477bd 100644 --- a/src/library-authoring/index.scss +++ b/src/library-authoring/index.scss @@ -4,6 +4,7 @@ @import "./LibraryAuthoringPage"; @import "./units"; @import "./section-subsections"; +@import "./containers"; .library-cards-grid { display: grid; From 1f096b754a1eca06905008174ee297f43d18f727 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Thu, 19 Jun 2025 04:17:30 +0930 Subject: [PATCH 04/10] feat: add custom warning message to container publish box --- .../containers/ContainerInfo.test.tsx | 85 ++++++++++++++++++- .../containers/ContainerPublishStatus.tsx | 37 ++++++-- src/library-authoring/containers/messages.ts | 30 +++++++ 3 files changed, 142 insertions(+), 10 deletions(-) diff --git a/src/library-authoring/containers/ContainerInfo.test.tsx b/src/library-authoring/containers/ContainerInfo.test.tsx index 387b10d8f7..cfee295508 100644 --- a/src/library-authoring/containers/ContainerInfo.test.tsx +++ b/src/library-authoring/containers/ContainerInfo.test.tsx @@ -74,16 +74,19 @@ describe('', () => { { containerType: 'Unit', containerId: unitId, + childType: 'component', }, { containerType: 'Subsection', containerId: subsectionId, + childType: 'unit', }, { containerType: 'Section', containerId: sectionId, + childType: 'subsection', }, - ].forEach(({ containerId, containerType }) => { + ].forEach(({ containerId, containerType, childType }) => { testIf(containerType === 'Unit')(`should delete the ${containerType} using the menu`, async () => { axiosMock.onDelete(getLibraryContainerApiUrl(containerId)).reply(200); render(containerId); @@ -117,6 +120,7 @@ describe('', () => { { type: 'library_container', usageKey: containerId, + blockType: containerType.toLowerCase(), publishStatus: PublishStatus.Published, }, ], @@ -139,6 +143,7 @@ describe('', () => { { type: 'library_container', usageKey: containerId, + blockType: containerType.toLowerCase(), publishStatus: PublishStatus.Modified, }, ], @@ -147,12 +152,17 @@ describe('', () => { }); render(containerId, containerType); - // Click on Publish button to reveal the confirmation box + // Click on Publish button let publishButton = await screen.findByRole('button', { name: /publish changes/i }); expect(publishButton).toBeInTheDocument(); userEvent.click(publishButton); expect(publishButton).not.toBeInTheDocument(); + // Reveals the confirmation box with warning text + expect(await screen.findByText( + `Are you sure you want to publish this ${containerType.toLowerCase()}?`, + )).toBeInTheDocument(); + // Click on the confirm Cancel button const publishCancel = await screen.findByRole('button', { name: 'Cancel' }); expect(publishCancel).toBeInTheDocument(); @@ -185,6 +195,7 @@ describe('', () => { { type: 'library_container', usageKey: containerId, + blockType: containerType.toLowerCase(), publishStatus: PublishStatus.Modified, }, ], @@ -210,6 +221,76 @@ describe('', () => { expect(mockShowToast).toHaveBeenCalledWith('Failed to publish changes'); }); + it(`shows single child before publishing the ${containerType}`, async () => { + axiosMock.onPost(getLibraryContainerPublishApiUrl(containerId)).reply(200); + mockSearchResult({ + results: [ // @ts-ignore + { + hits: [ + { + type: 'library_container', + usageKey: containerId, + blockType: containerType.toLowerCase(), + publishStatus: PublishStatus.Modified, + content: { + childDisplayNames: [ + 'one', + ], + }, + }, + ], + }, + ], + }); + render(containerId, containerType); + + // Click on Publish button + const publishButton = await screen.findByRole('button', { name: /publish changes/i }); + expect(publishButton).toBeInTheDocument(); + userEvent.click(publishButton); + expect(publishButton).not.toBeInTheDocument(); + + // Check warning text in the confirmation box + expect(await screen.findByText( + `This ${containerType.toLowerCase()} and its 1 ${childType} will all be published.`, + )).toBeInTheDocument(); + }); + + it(`shows child count before publishing the ${containerType}`, async () => { + axiosMock.onPost(getLibraryContainerPublishApiUrl(containerId)).reply(200); + mockSearchResult({ + results: [ // @ts-ignore + { + hits: [ + { + type: 'library_container', + usageKey: containerId, + blockType: containerType.toLowerCase(), + publishStatus: PublishStatus.Modified, + content: { + childDisplayNames: [ + 'one', 'two', + ], + }, + }, + ], + }, + ], + }); + render(containerId, containerType); + + // Click on Publish button + const publishButton = await screen.findByRole('button', { name: /publish changes/i }); + expect(publishButton).toBeInTheDocument(); + userEvent.click(publishButton); + expect(publishButton).not.toBeInTheDocument(); + + // Check warning text in the confirmation box + expect(await screen.findByText( + `This ${containerType.toLowerCase()} and its 2 ${childType}s will all be published.`, + )).toBeInTheDocument(); + }); + testIf(containerType === 'Unit')(`show only published ${containerType} content`, async () => { render(containerId, '', true); expect(await screen.findByTestId('container-info-menu-toggle')).toBeInTheDocument(); diff --git a/src/library-authoring/containers/ContainerPublishStatus.tsx b/src/library-authoring/containers/ContainerPublishStatus.tsx index e6889ec3d7..af1c8b095c 100644 --- a/src/library-authoring/containers/ContainerPublishStatus.tsx +++ b/src/library-authoring/containers/ContainerPublishStatus.tsx @@ -2,8 +2,9 @@ * Shows the Container's publish status, * and enables publishing any unpublished changes. */ -import { useContext, useCallback } from 'react'; +import { useContext, useCallback, useMemo } from 'react'; import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; +import type { MessageDescriptor } from 'react-intl'; import { ActionRow, Button, @@ -13,6 +14,7 @@ import { import Loading from '../../generic/Loading'; import LoadingButton from '../../generic/loading-button'; import { ToastContext } from '../../generic/toast-context'; +import { ContainerType } from '../../generic/key-utils'; import { ContainerHit, PublishStatus } from '../../search-manager'; import { useLibraryContext } from '../common/context/LibraryContext'; import { useContentFromSearchIndex, usePublishContainer } from '../data/apiHooks'; @@ -41,17 +43,36 @@ const ContainerPublisher = ({ close(); }, [publishContainer, showToast]); + const warningMessage = useMemo(() => { + const childCount = container.content?.childDisplayNames?.length || 0; + let childMessage: MessageDescriptor; + let noChildMessage: MessageDescriptor; + + switch (container.blockType) { + case ContainerType.Section: + childMessage = messages.publishSectionWithChildrenWarning; + noChildMessage = messages.publishSectionWarning; + break; + case ContainerType.Subsection: + childMessage = messages.publishSubsectionWithChildrenWarning; + noChildMessage = messages.publishSubsectionWarning; + break; + default: // ContainerType.Unit + childMessage = messages.publishUnitWithChildrenWarning; + noChildMessage = messages.publishUnitWarning; + } + return intl.formatMessage( + childCount ? childMessage : noChildMessage, + { childCount }, + ); + }, [container]); + return (

{intl.formatMessage(messages.publishContainerConfirmHeading)}

-

- { - // TODO show specific message for this container and children - 'Are you sure you want to publish this container?' - } -

+

{warningMessage}