diff --git a/src/container-comparison/CompareContainersWidget.test.tsx b/src/container-comparison/CompareContainersWidget.test.tsx index 4a40b0ba36..348aff0546 100644 --- a/src/container-comparison/CompareContainersWidget.test.tsx +++ b/src/container-comparison/CompareContainersWidget.test.tsx @@ -31,6 +31,9 @@ describe('CompareContainersWidget', () => { expect((await screen.findAllByText('subsection block 2')).length).toEqual(1); expect((await screen.findAllByText('subsection block 11')).length).toEqual(1); expect((await screen.findAllByText('subsection block 22')).length).toEqual(1); + expect(screen.queryByText( + /the only change is to text block which has been edited in this course\. accepting will not remove local edits\./i, + )).not.toBeInTheDocument(); }); test('renders loading spinner when data is pending', async () => { @@ -90,4 +93,36 @@ describe('CompareContainersWidget', () => { expect(await screen.findByRole('button', { name: 'subsection block 00' })).toBeInTheDocument(); expect(await screen.findByRole('button', { name: 'subsection block 0' })).toBeInTheDocument(); }); + + test('should show alert if the only change is a single text component with local overrides', async () => { + const url = getLibraryContainerApiUrl(mockGetContainerMetadata.sectionId); + axiosMock.onGet(url).reply(200, { publishedDisplayName: 'Test Title' }); + render(); + + expect((await screen.findAllByText('Test Title')).length).toEqual(2); + + expect(screen.getByText( + /the only change is to text block which has been edited in this course\. accepting will not remove local edits\./i, + )).toBeInTheDocument(); + expect(screen.getByText(/Html block 11/i)).toBeInTheDocument(); + }); + + test('should show alert if the only changes is multiple text components with local overrides', async () => { + const url = getLibraryContainerApiUrl(mockGetContainerMetadata.sectionId); + axiosMock.onGet(url).reply(200, { publishedDisplayName: 'Test Title' }); + render(); + + expect((await screen.findAllByText('Test Title')).length).toEqual(2); + + expect(screen.getByText( + /the only change is to which have been edited in this course\. accepting will not remove local edits\./i, + )).toBeInTheDocument(); + expect(screen.getByText(/2 text blocks/i)).toBeInTheDocument(); + }); }); diff --git a/src/container-comparison/CompareContainersWidget.tsx b/src/container-comparison/CompareContainersWidget.tsx index ad83d495fd..96fec4a51a 100644 --- a/src/container-comparison/CompareContainersWidget.tsx +++ b/src/container-comparison/CompareContainersWidget.tsx @@ -1,13 +1,18 @@ +import { useCallback, useMemo, useState } from 'react'; + import { + Alert, Breadcrumb, Button, Card, Icon, Stack, } from '@openedx/paragon'; import { ArrowBack } from '@openedx/paragon/icons'; -import { useCallback, useMemo, useState } from 'react'; +import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; + import { ContainerType } from '@src/generic/key-utils'; import ErrorAlert from '@src/generic/alert-error'; import { LoadingSpinner } from '@src/generic/Loading'; import { useContainer, useContainerChildren } from '@src/library-authoring/data/apiHooks'; -import { useIntl } from '@edx/frontend-platform/i18n'; +import { BoldText } from '@src/utils'; + import ChildrenPreview from './ChildrenPreview'; import ContainerRow from './ContainerRow'; import { useCourseContainerChildren } from './data/apiHooks'; @@ -18,12 +23,17 @@ import messages from './messages'; interface ContainerInfoProps { upstreamBlockId: string; downstreamBlockId: string; + isReadyToSyncIndividually?: boolean; } interface Props extends ContainerInfoProps { parent: ContainerInfoProps[]; onRowClick: (row: WithState) => void; onBackBtnClick: () => void; + // This two props are used to show an alert for the changes to text components with local overrides. + // They may be removed in the future. + localUpdateAlertCount: number; + localUpdateAlertBlockName: string; } /** @@ -35,9 +45,11 @@ const CompareContainersWidgetInner = ({ parent, onRowClick, onBackBtnClick, + localUpdateAlertCount, + localUpdateAlertBlockName, }: Props) => { const intl = useIntl(); - const { data, isError, error } = useCourseContainerChildren(downstreamBlockId); + const { data, isError, error } = useCourseContainerChildren(downstreamBlockId, parent.length === 0); const { data: libData, isError: isLibError, @@ -127,7 +139,19 @@ const CompareContainersWidgetInner = ({ } return ( -
+
+ {localUpdateAlertCount > 0 && ( + + + + )}
@@ -151,7 +175,11 @@ const CompareContainersWidgetInner = ({ * and allows the user to select the container to view. This is a wrapper component that maintains current * source state. Actual implementation of the diff view is done by CompareContainersWidgetInner. */ -export const CompareContainersWidget = ({ upstreamBlockId, downstreamBlockId }: ContainerInfoProps) => { +export const CompareContainersWidget = ({ + upstreamBlockId, + downstreamBlockId, + isReadyToSyncIndividually = false, +}: ContainerInfoProps) => { const [currentContainerState, setCurrentContainerState] = useState({ @@ -160,6 +188,23 @@ export const CompareContainersWidget = ({ upstreamBlockId, downstreamBlockId }: parent: [], }); + const { data } = useCourseContainerChildren(downstreamBlockId, true); + let localUpdateAlertBlockName = ''; + let localUpdateAlertCount = 0; + + // Show this alert if the only change is text components with local overrides. + // We decided not to put this in `CompareContainersWidgetInner` because if you enter a child, + // the alert would disappear. By keeping this call in CompareContainersWidget, + // the alert remains in the modal regardless of whether you navigate within the children. + if (!isReadyToSyncIndividually && data?.upstreamReadyToSyncChildrenInfo + && data.upstreamReadyToSyncChildrenInfo.every(value => value.isModified && value.blockType === 'html') + ) { + localUpdateAlertCount = data.upstreamReadyToSyncChildrenInfo.length; + if (localUpdateAlertCount === 1) { + localUpdateAlertBlockName = data.upstreamReadyToSyncChildrenInfo[0].name; + } + } + const onRowClick = (row: WithState) => { if (!isRowClickable(row.state, row.blockType as ContainerType)) { return; @@ -197,6 +242,8 @@ export const CompareContainersWidget = ({ upstreamBlockId, downstreamBlockId }: parent={currentContainerState.parent} onRowClick={onRowClick} onBackBtnClick={onBackBtnClick} + localUpdateAlertCount={localUpdateAlertCount} + localUpdateAlertBlockName={localUpdateAlertBlockName} /> ); }; diff --git a/src/container-comparison/ContainerRow.test.tsx b/src/container-comparison/ContainerRow.test.tsx index aa80a888ec..dfd811a6f2 100644 --- a/src/container-comparison/ContainerRow.test.tsx +++ b/src/container-comparison/ContainerRow.test.tsx @@ -83,6 +83,11 @@ describe('', () => { expect(await screen.findByText(messages.renamedDiffBeforeMessage.defaultMessage.replace('{name}', 'Modified name'))).toBeInTheDocument(); }); + test('renders with local content update', async () => { + render(); + expect(await screen.findByText(messages.locallyContentUpdatedBeforeMessage.defaultMessage.replace('{blockType}', 'html'))).toBeInTheDocument(); + }); + test('renders with moved state', async () => { render(); expect(await screen.findByText( diff --git a/src/container-comparison/ContainerRow.tsx b/src/container-comparison/ContainerRow.tsx index 723001adee..3b215db141 100644 --- a/src/container-comparison/ContainerRow.tsx +++ b/src/container-comparison/ContainerRow.tsx @@ -40,7 +40,10 @@ const ContainerRow = ({ message = side === 'Before' ? messages.removedDiffBeforeMessage : messages.removedDiffAfterMessage; return ['text-white bg-danger-600', Delete, message]; case 'locallyRenamed': - message = side === 'Before' ? messages.renamedDiffBeforeMessage : messages.renamedDiffAfterMessage; + message = side === 'Before' ? messages.renamedDiffBeforeMessage : messages.renamedUpdatedDiffAfterMessage; + return ['bg-light-300 text-light-300 ', Done, message]; + case 'locallyContentUpdated': + message = side === 'Before' ? messages.locallyContentUpdatedBeforeMessage : messages.locallyContentUpdatedAfterMessage; return ['bg-light-300 text-light-300 ', Done, message]; case 'moved': message = side === 'Before' ? messages.movedDiffBeforeMessage : messages.movedDiffAfterMessage; diff --git a/src/container-comparison/data/api.mock.ts b/src/container-comparison/data/api.mock.ts index fe9201f082..e27e23681f 100644 --- a/src/container-comparison/data/api.mock.ts +++ b/src/container-comparison/data/api.mock.ts @@ -1,5 +1,5 @@ /* istanbul ignore file */ -import { CourseContainerChildrenData } from '@src/course-unit/data/types'; +import { CourseContainerChildrenData, type UpstreamReadyToSyncChildrenInfo } from '@src/course-unit/data/types'; import * as unitApi from '@src/course-unit/data/api'; /** @@ -11,6 +11,7 @@ export async function mockGetCourseContainerChildren(containerId: string): Promi const numChildren: number = 3; let blockType: string; let displayName: string; + let upstreamReadyToSyncChildrenInfo: UpstreamReadyToSyncChildrenInfo[] = []; switch (containerId) { case mockGetCourseContainerChildren.unitId: blockType = 'text'; @@ -24,6 +25,37 @@ export async function mockGetCourseContainerChildren(containerId: string): Promi blockType = 'unit'; displayName = 'subsection block 00'; break; + case mockGetCourseContainerChildren.sectionShowsAlertSingleText: + blockType = 'subsection'; + displayName = 'Test Title'; + upstreamReadyToSyncChildrenInfo = [{ + id: 'block-v1:UNIX+UX1+2025_T3+type@html+block@1', + name: 'Html block 11', + blockType: 'html', + isModified: true, + upstream: 'upstream-id', + }]; + break; + case mockGetCourseContainerChildren.sectionShowsAlertMultipleText: + blockType = 'subsection'; + displayName = 'Test Title'; + upstreamReadyToSyncChildrenInfo = [ + { + id: 'block-v1:UNIX+UX1+2025_T3+type@html+block@1', + name: 'Html block 11', + blockType: 'html', + isModified: true, + upstream: 'upstream-id', + }, + { + id: 'block-v1:UNIX+UX1+2025_T3+type@html+block@2', + name: 'Html block 22', + blockType: 'html', + isModified: true, + upstream: 'upstream-id', + }, + ]; + break; case mockGetCourseContainerChildren.unitIdLoading: case mockGetCourseContainerChildren.sectionIdLoading: case mockGetCourseContainerChildren.subsectionIdLoading: @@ -54,11 +86,14 @@ export async function mockGetCourseContainerChildren(containerId: string): Promi isPublished: false, children, displayName, + upstreamReadyToSyncChildrenInfo, }); } mockGetCourseContainerChildren.unitId = 'block-v1:UNIX+UX1+2025_T3+type@unit+block@0'; mockGetCourseContainerChildren.subsectionId = 'block-v1:UNIX+UX1+2025_T3+type@subsection+block@0'; mockGetCourseContainerChildren.sectionId = 'block-v1:UNIX+UX1+2025_T3+type@section+block@0'; +mockGetCourseContainerChildren.sectionShowsAlertSingleText = 'block-v1:UNIX+UX1+2025_T3+type@section2+block@0'; +mockGetCourseContainerChildren.sectionShowsAlertMultipleText = 'block-v1:UNIX+UX1+2025_T3+type@section3+block@0'; mockGetCourseContainerChildren.unitIdLoading = 'block-v1:UNIX+UX1+2025_T3+type@unit+block@loading'; mockGetCourseContainerChildren.subsectionIdLoading = 'block-v1:UNIX+UX1+2025_T3+type@subsection+block@loading'; mockGetCourseContainerChildren.sectionIdLoading = 'block-v1:UNIX+UX1+2025_T3+type@section+block@loading'; diff --git a/src/container-comparison/data/apiHooks.ts b/src/container-comparison/data/apiHooks.ts index 92ea4aecfe..053ee47fab 100644 --- a/src/container-comparison/data/apiHooks.ts +++ b/src/container-comparison/data/apiHooks.ts @@ -11,16 +11,16 @@ export const containerComparisonQueryKeys = { /** * Key for a single container */ - container: (usageKey: string) => { + container: (usageKey: string, getUpstreamInfo: boolean) => { const courseKey = getCourseKey(usageKey); - return [...containerComparisonQueryKeys.course(courseKey), usageKey]; + return [...containerComparisonQueryKeys.course(courseKey), usageKey, getUpstreamInfo.toString()]; }, }; -export const useCourseContainerChildren = (usageKey?: string) => ( +export const useCourseContainerChildren = (usageKey?: string, getUpstreamInfo?: boolean) => ( useQuery({ enabled: !!usageKey, - queryFn: () => getCourseContainerChildren(usageKey!), - queryKey: containerComparisonQueryKeys.container(usageKey!), + queryFn: () => getCourseContainerChildren(usageKey!, getUpstreamInfo), + queryKey: containerComparisonQueryKeys.container(usageKey!, getUpstreamInfo || false), }) ); diff --git a/src/container-comparison/messages.ts b/src/container-comparison/messages.ts index 29e9276942..1a8e61d2cb 100644 --- a/src/container-comparison/messages.ts +++ b/src/container-comparison/messages.ts @@ -37,14 +37,24 @@ const messages = defineMessages({ description: 'Description for added component in after section of diff preview', }, renamedDiffBeforeMessage: { - id: 'course-authoring.container-comparison.diff.before.renamed-message', - defaultMessage: 'Original Library Name: {name}', - description: 'Description for renamed component in before section of diff preview', + id: 'course-authoring.container-comparison.diff.before.locally-updated-message', + defaultMessage: 'Library Name: {name}', + description: 'Description for locally updated component in before section of diff preview', }, - renamedDiffAfterMessage: { - id: 'course-authoring.container-comparison.diff.after.renamed-message', - defaultMessage: 'This {blockType} will remain renamed', - description: 'Description for renamed component in after section of diff preview', + renamedUpdatedDiffAfterMessage: { + id: 'course-authoring.container-comparison.diff.after.locally-updated-message', + defaultMessage: 'Library name remains overwritten', + description: 'Description for locally updated component in after section of diff preview', + }, + locallyContentUpdatedBeforeMessage: { + id: 'course-authoring.container-comparison.diff.before.locally-content-updated-message', + defaultMessage: 'This {blockType} was edited locally', + description: 'Description for locally content updated component in before section of diff preview', + }, + locallyContentUpdatedAfterMessage: { + id: 'course-authoring.container-comparison.diff.after.locally-content-updated-message', + defaultMessage: 'Local edit will remain', + description: 'Description for locally content updated component in after section of diff preview', }, movedDiffBeforeMessage: { id: 'course-authoring.container-comparison.diff.before.moved-message', @@ -71,6 +81,11 @@ const messages = defineMessages({ defaultMessage: 'After', description: 'After section title text', }, + localChangeInTextAlert: { + id: 'course-authoring.container-comparison.text-with-local-change.alert', + defaultMessage: 'The only change is to {count, plural, one {text block {blockName} which has been edited} other {{count} text blocks which have been edited}} in this course. Accepting will not remove local edits.', + description: 'Alert to show if the only change is on text components with local overrides.', + }, }); export default messages; diff --git a/src/container-comparison/types.ts b/src/container-comparison/types.ts index 4b01770a70..c546e0c483 100644 --- a/src/container-comparison/types.ts +++ b/src/container-comparison/types.ts @@ -1,6 +1,6 @@ import { UpstreamInfo } from '@src/data/types'; -export type ContainerState = 'removed' | 'added' | 'modified' | 'childrenModified' | 'locallyRenamed' | 'moved'; +export type ContainerState = 'removed' | 'added' | 'modified' | 'childrenModified' | 'locallyContentUpdated' | 'locallyRenamed' | 'moved'; export type WithState = T & { state?: ContainerState, originalName?: string }; export type WithIndex = T & { index: number }; diff --git a/src/container-comparison/utils.ts b/src/container-comparison/utils.ts index 7a15155d75..ab0fa14439 100644 --- a/src/container-comparison/utils.ts +++ b/src/container-comparison/utils.ts @@ -36,6 +36,7 @@ export function diffPreviewContainerChildren getConfig().STUDIO_BASE_URL; export const getXBlockBaseApiUrl = (itemId: string) => `${getStudioBaseUrl()}/xblock/${itemId}`; export const getCourseSectionVerticalApiUrl = (itemId: string) => `${getStudioBaseUrl()}/api/contentstore/v1/container_handler/${itemId}`; -export const getCourseVerticalChildrenApiUrl = (itemId: string) => `${getStudioBaseUrl()}/api/contentstore/v1/container/${itemId}/children`; +export const getCourseVerticalChildrenApiUrl = (itemId: string, getUpstreamInfo: boolean = false) => `${getStudioBaseUrl()}/api/contentstore/v1/container/${itemId}/children?get_upstream_info=${getUpstreamInfo}`; export const getCourseOutlineInfoUrl = (courseId: string) => `${getStudioBaseUrl()}/course/${courseId}?format=concise`; export const postXBlockBaseApiUrl = () => `${getStudioBaseUrl()}/xblock/`; export const libraryBlockChangesUrl = (blockId: string) => `${getStudioBaseUrl()}/api/contentstore/v2/downstreams/${blockId}/sync`; @@ -108,9 +108,12 @@ export async function handleCourseUnitVisibilityAndData( /** * Get an object containing course vertical children data. */ -export async function getCourseContainerChildren(itemId: string): Promise { +export async function getCourseContainerChildren( + itemId: string, + getUpstreamInfo: boolean = false, +): Promise { const { data } = await getAuthenticatedHttpClient() - .get(getCourseVerticalChildrenApiUrl(itemId)); + .get(getCourseVerticalChildrenApiUrl(itemId, getUpstreamInfo)); const camelCaseData = camelCaseObject(data); return updateXBlockBlockIdToId(camelCaseData) as CourseContainerChildrenData; diff --git a/src/course-unit/data/types.ts b/src/course-unit/data/types.ts index 3a2b2fcc0d..99884cae54 100644 --- a/src/course-unit/data/types.ts +++ b/src/course-unit/data/types.ts @@ -38,9 +38,18 @@ export interface ContainerChildData { upstreamLink: UpstreamInfo; } +export interface UpstreamReadyToSyncChildrenInfo { + id: string; + name: string; + upstream: string; + blockType: string; + isModified: boolean; +} + export interface CourseContainerChildrenData { canPasteComponent: boolean; - children: ContainerChildData[], + children: ContainerChildData[]; isPublished: boolean; displayName: string; + upstreamReadyToSyncChildrenInfo: UpstreamReadyToSyncChildrenInfo[]; } diff --git a/src/course-unit/preview-changes/index.tsx b/src/course-unit/preview-changes/index.tsx index 4f74765bc2..efedfae37f 100644 --- a/src/course-unit/preview-changes/index.tsx +++ b/src/course-unit/preview-changes/index.tsx @@ -105,6 +105,7 @@ export interface LibraryChangesMessageData { isLocallyModified?: boolean, isContainer: boolean, blockType?: string | null, + isReadyToSyncIndividually?: boolean, } export interface PreviewLibraryXBlockChangesProps { @@ -143,6 +144,7 @@ export const PreviewLibraryXBlockChanges = ({ ); } diff --git a/src/data/types.ts b/src/data/types.ts index 111dc57852..86df4cb40d 100644 --- a/src/data/types.ts +++ b/src/data/types.ts @@ -64,6 +64,7 @@ export interface UpstreamInfo { isModified?: boolean, hasTopLevelParent?: boolean, readyToSyncChildren?: UpstreamChildrenInfo[], + isReadyToSyncIndividually?: boolean, } export interface XBlock {