From 2dc087f87a84ca6951be5e788e35675989f32311 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Thu, 30 Oct 2025 03:54:04 +0530 Subject: [PATCH] feat: differentiate between renamed and data edit in sync preview diff [FC-0112] (#2577) Use `downstream_customized` field from upstream link to determine whether the text component was locally renamed or content updated or both and display correct notes in preview diff. Also update libraries v2 alert wording as per https://github.com/openedx/frontend-app-authoring/issues/2169#issuecomment-3434735901 --- .../CompareContainersWidget.tsx | 2 +- .../ContainerRow.test.tsx | 6 ++ src/container-comparison/ContainerRow.tsx | 67 ++++++++---- src/container-comparison/data/api.mock.ts | 10 +- src/container-comparison/types.ts | 2 +- src/container-comparison/utils.test.ts | 102 +++++++++++++++--- src/container-comparison/utils.ts | 23 ++-- .../section-card/SectionCard.test.tsx | 1 + .../subsection-card/SubsectionCard.test.tsx | 1 + .../unit-card/UnitCard.test.tsx | 1 + src/course-unit/data/types.ts | 2 +- src/data/types.ts | 2 +- src/studio-home/tabs-section/messages.ts | 2 +- 13 files changed, 168 insertions(+), 53 deletions(-) diff --git a/src/container-comparison/CompareContainersWidget.tsx b/src/container-comparison/CompareContainersWidget.tsx index 90f58e5f2..cb1b318bf 100644 --- a/src/container-comparison/CompareContainersWidget.tsx +++ b/src/container-comparison/CompareContainersWidget.tsx @@ -243,7 +243,7 @@ export const CompareContainersWidget = ({ // 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') + && data.upstreamReadyToSyncChildrenInfo.every(value => value.downstreamCustomized.length > 0 && value.blockType === 'html') ) { localUpdateAlertCount = data.upstreamReadyToSyncChildrenInfo.length; if (localUpdateAlertCount === 1) { diff --git a/src/container-comparison/ContainerRow.test.tsx b/src/container-comparison/ContainerRow.test.tsx index 669c9a7c7..2826d41fc 100644 --- a/src/container-comparison/ContainerRow.test.tsx +++ b/src/container-comparison/ContainerRow.test.tsx @@ -74,6 +74,12 @@ describe('', () => { expect(await screen.findByText(messages.locallyContentUpdatedBeforeMessage.defaultMessage.replace('{blockType}', 'html'))).toBeInTheDocument(); }); + test('renders with rename and local content update', async () => { + render(); + expect(await screen.findByText(messages.renamedDiffBeforeMessage.defaultMessage.replace('{name}', 'Modified name'))).toBeInTheDocument(); + 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 3b215db14..ef242ee3e 100644 --- a/src/container-comparison/ContainerRow.tsx +++ b/src/container-comparison/ContainerRow.tsx @@ -23,33 +23,47 @@ export interface ContainerRowProps { onClick?: () => void; } +interface StateContext { + className: string; + icon: React.ComponentType; + message?: MessageDescriptor; + message2?: MessageDescriptor; +} + const ContainerRow = ({ title, containerType, state, side, originalName, onClick, }: ContainerRowProps) => { const isClickable = isRowClickable(state, containerType as ContainerType); - const stateContext = useMemo(() => { + const stateContext: StateContext = useMemo(() => { let message: MessageDescriptor | undefined; + let message2: MessageDescriptor | undefined; switch (state) { case 'added': message = side === 'Before' ? messages.addedDiffBeforeMessage : messages.addedDiffAfterMessage; - return ['text-white bg-success-500', Plus, message]; + return { className: 'text-white bg-success-500', icon: Plus, message }; case 'modified': message = side === 'Before' ? messages.modifiedDiffBeforeMessage : messages.modifiedDiffAfterMessage; - return ['text-white bg-warning-900', Cached, message]; + return { className: 'text-white bg-warning-900', icon: Cached, message }; case 'removed': message = side === 'Before' ? messages.removedDiffBeforeMessage : messages.removedDiffAfterMessage; - return ['text-white bg-danger-600', Delete, message]; + return { className: 'text-white bg-danger-600', icon: Delete, message }; case 'locallyRenamed': message = side === 'Before' ? messages.renamedDiffBeforeMessage : messages.renamedUpdatedDiffAfterMessage; - return ['bg-light-300 text-light-300 ', Done, message]; + return { className: 'bg-light-300 text-light-300 ', icon: Done, message }; case 'locallyContentUpdated': message = side === 'Before' ? messages.locallyContentUpdatedBeforeMessage : messages.locallyContentUpdatedAfterMessage; - return ['bg-light-300 text-light-300 ', Done, message]; + return { className: 'bg-light-300 text-light-300 ', icon: Done, message }; + case 'locallyRenamedAndContentUpdated': + message = side === 'Before' ? messages.renamedDiffBeforeMessage : messages.renamedUpdatedDiffAfterMessage; + message2 = side === 'Before' ? messages.locallyContentUpdatedBeforeMessage : messages.locallyContentUpdatedAfterMessage; + return { + className: 'bg-light-300 text-light-300 ', icon: Done, message, message2, + }; case 'moved': message = side === 'Before' ? messages.movedDiffBeforeMessage : messages.movedDiffAfterMessage; - return ['bg-light-300 text-light-300', Done, message]; + return { className: 'bg-light-300 text-light-300', icon: Done, message }; default: - return ['bg-light-300 text-light-300', Done, message]; + return { className: 'bg-light-300 text-light-300', icon: Done, message }; } }, [state, side]); @@ -66,9 +80,9 @@ const ContainerRow = ({ >
- +
@@ -80,16 +94,29 @@ const ContainerRow = ({ /> {title} - {stateContext[2] ? ( - - - + {stateContext.message ? ( +
+ + + + {stateContext.message2 && ( + + + + )} +
) : (   )} diff --git a/src/container-comparison/data/api.mock.ts b/src/container-comparison/data/api.mock.ts index 49dccb95f..d68b564f8 100644 --- a/src/container-comparison/data/api.mock.ts +++ b/src/container-comparison/data/api.mock.ts @@ -32,7 +32,7 @@ export async function mockGetCourseContainerChildren(containerId: string): Promi id: 'block-v1:UNIX+UX1+2025_T3+type@html+block@1', name: 'Html block 11', blockType: 'html', - isModified: true, + downstreamCustomized: ['display_name'], upstream: 'upstream-id', }]; break; @@ -44,14 +44,14 @@ export async function mockGetCourseContainerChildren(containerId: string): Promi id: 'block-v1:UNIX+UX1+2025_T3+type@html+block@1', name: 'Html block 11', blockType: 'html', - isModified: true, + downstreamCustomized: ['display_name'], upstream: 'upstream-id', }, { id: 'block-v1:UNIX+UX1+2025_T3+type@html+block@2', name: 'Html block 22', blockType: 'html', - isModified: true, + downstreamCustomized: ['display_name'], upstream: 'upstream-id', }, ]; @@ -78,7 +78,7 @@ export async function mockGetCourseContainerChildren(containerId: string): Promi versionSynced: 1, versionAvailable: 2, versionDeclined: null, - isModified: false, + downstreamCustomized: [], }, } )); @@ -107,7 +107,7 @@ mockGetCourseContainerChildren.childTemplate = { versionSynced: 1, versionAvailable: 2, versionDeclined: null, - isModified: false, + downstreamCustomized: [], }, }; /** Apply this mock. Returns a spy object that can tell you if it's been called. */ diff --git a/src/container-comparison/types.ts b/src/container-comparison/types.ts index c546e0c48..e980279ed 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' | 'locallyContentUpdated' | 'locallyRenamed' | 'moved'; +export type ContainerState = 'removed' | 'added' | 'modified' | 'childrenModified' | 'locallyContentUpdated' | 'locallyRenamed' | 'locallyRenamedAndContentUpdated' | 'moved'; export type WithState = T & { state?: ContainerState, originalName?: string }; export type WithIndex = T & { index: number }; diff --git a/src/container-comparison/utils.test.ts b/src/container-comparison/utils.test.ts index 2b732b41e..42a36547c 100644 --- a/src/container-comparison/utils.test.ts +++ b/src/container-comparison/utils.test.ts @@ -2,7 +2,7 @@ import { ContainerChildBase, CourseContainerChildBase } from './types'; import { diffPreviewContainerChildren } from './utils'; export const getMockCourseContainerData = ( - type: 'added|deleted' | 'moved|deleted' | 'all', + type: 'added|deleted' | 'moved|deleted' | 'all' | 'locallyEdited', ): [CourseContainerChildBase[], ContainerChildBase[]] => { switch (type) { case 'moved|deleted': @@ -17,7 +17,7 @@ export const getMockCourseContainerData = ( versionSynced: 11, versionAvailable: 11, versionDeclined: null, - isModified: true, + downstreamCustomized: ['display_name'], }, }, { @@ -29,7 +29,7 @@ export const getMockCourseContainerData = ( versionSynced: 7, versionAvailable: 7, versionDeclined: null, - isModified: false, + downstreamCustomized: [], }, }, { @@ -41,7 +41,7 @@ export const getMockCourseContainerData = ( versionSynced: 2, versionAvailable: 2, versionDeclined: null, - isModified: false, + downstreamCustomized: [], }, }, { @@ -53,7 +53,7 @@ export const getMockCourseContainerData = ( versionSynced: 1, versionAvailable: 1, versionDeclined: null, - isModified: false, + downstreamCustomized: [], }, }, ], @@ -87,7 +87,7 @@ export const getMockCourseContainerData = ( versionSynced: 11, versionAvailable: 11, versionDeclined: null, - isModified: true, + downstreamCustomized: ['display_name'], }, }, { @@ -99,7 +99,7 @@ export const getMockCourseContainerData = ( versionSynced: 7, versionAvailable: 7, versionDeclined: null, - isModified: false, + downstreamCustomized: [], }, }, { @@ -111,7 +111,7 @@ export const getMockCourseContainerData = ( versionSynced: 2, versionAvailable: 2, versionDeclined: null, - isModified: false, + downstreamCustomized: [], }, }, { @@ -123,7 +123,7 @@ export const getMockCourseContainerData = ( versionSynced: 1, versionAvailable: 1, versionDeclined: null, - isModified: false, + downstreamCustomized: [], }, }, ], @@ -162,7 +162,7 @@ export const getMockCourseContainerData = ( versionSynced: 11, versionAvailable: 11, versionDeclined: null, - isModified: true, + downstreamCustomized: ['display_name'], }, }, { @@ -174,7 +174,7 @@ export const getMockCourseContainerData = ( versionSynced: 7, versionAvailable: 7, versionDeclined: null, - isModified: false, + downstreamCustomized: [], }, }, { @@ -186,7 +186,7 @@ export const getMockCourseContainerData = ( versionSynced: 2, versionAvailable: 2, versionDeclined: null, - isModified: false, + downstreamCustomized: [], }, }, { @@ -198,7 +198,7 @@ export const getMockCourseContainerData = ( versionSynced: 1, versionAvailable: 1, versionDeclined: null, - isModified: false, + downstreamCustomized: [], }, }, ], @@ -225,6 +225,64 @@ export const getMockCourseContainerData = ( }, ], ] as [CourseContainerChildBase[], ContainerChildBase[]]; + case 'locallyEdited': + return [ + [ + { + id: 'block-v1:UNIX+UX1+2025_T3+type@vertical+block@1', + name: 'Unit 1 remote edit - local edit', + blockType: 'vertical', + upstreamLink: { + upstreamRef: 'lct:UNIX:CS1:unit:unit-1-2a1741', + versionSynced: 11, + versionAvailable: 11, + versionDeclined: null, + downstreamCustomized: ['display_name'], + }, + }, + { + id: 'block-v1:UNIX+UX1+2025_T3+type@vertical+block@2', + name: 'New unit remote edit', + blockType: 'vertical', + upstreamLink: { + upstreamRef: 'lct:UNIX:CS1:unit:new-unit-remote-7eb9d1', + versionSynced: 7, + versionAvailable: 7, + versionDeclined: null, + downstreamCustomized: ['data'], + }, + }, + { + id: 'block-v1:UNIX+UX1+2025_T3+type@vertical+block@3', + name: 'Unit with tags - local edit', + blockType: 'vertical', + upstreamLink: { + upstreamRef: 'lct:UNIX:CS1:unit:unit-with-tags-bec5f9', + versionSynced: 2, + versionAvailable: 2, + versionDeclined: null, + downstreamCustomized: ['display_name', 'data'], + }, + }, + ], + [ + { + id: 'lct:UNIX:CS1:unit:unit-1-2a1741', + displayName: 'Unit 1 remote edit - remote edit', + containerType: 'unit', + }, + { + id: 'lct:UNIX:CS1:unit:new-unit-remote-7eb9d1', + displayName: 'New unit remote edit', + containerType: 'unit', + }, + { + id: 'lct:UNIX:CS1:unit:unit-with-tags-bec5f9', + displayName: 'Unit with tags - remote edit', + containerType: 'unit', + }, + ], + ] as [CourseContainerChildBase[], ContainerChildBase[]]; default: throw new Error(); } @@ -280,4 +338,22 @@ describe('diffPreviewContainerChildren', () => { expect(result[1][2].state).toEqual('added'); expect(result[1][2].id).toEqual(result[0][2].id); }); + + it('should handle locally edited content', () => { + const [a, b] = getMockCourseContainerData('locallyEdited'); + const result = diffPreviewContainerChildren(a as CourseContainerChildBase[], b); + expect(result[0].length).toEqual(result[1].length); + // renamed + expect(result[0][0].state).toEqual('locallyRenamed'); + expect(result[1][0].state).toEqual('locallyRenamed'); + expect(result[1][0].id).toEqual(result[0][0].id); + // content updated + expect(result[0][1].state).toEqual('locallyContentUpdated'); + expect(result[1][1].state).toEqual('locallyContentUpdated'); + expect(result[1][1].id).toEqual(result[0][1].id); + // renamed and content updated + expect(result[0][2].state).toEqual('locallyRenamedAndContentUpdated'); + expect(result[1][2].state).toEqual('locallyRenamedAndContentUpdated'); + expect(result[1][2].id).toEqual(result[0][2].id); + }); }); diff --git a/src/container-comparison/utils.ts b/src/container-comparison/utils.ts index 2d4ddb4d2..03382418b 100644 --- a/src/container-comparison/utils.ts +++ b/src/container-comparison/utils.ts @@ -54,26 +54,29 @@ export function diffPreviewContainerChildren 0) { + if (isRenamed) { + state = 'locallyRenamed'; + originalName = newVersion.displayName; + } + if (isContentModified) { + state = 'locallyContentUpdated'; + } + if (isRenamed && isContentModified) { + state = 'locallyRenamedAndContentUpdated'; + } } else if (checkIsReadyToSync(oldVersion.upstreamLink)) { // has a new version ready to sync state = 'modified'; diff --git a/src/course-outline/section-card/SectionCard.test.tsx b/src/course-outline/section-card/SectionCard.test.tsx index 35e83057f..f2c37de59 100644 --- a/src/course-outline/section-card/SectionCard.test.tsx +++ b/src/course-outline/section-card/SectionCard.test.tsx @@ -74,6 +74,7 @@ const section = { versionAvailable: 2, versionDeclined: null, errorMessage: null, + downstreamCustomized: [] as string[], }, } satisfies Partial as XBlock; diff --git a/src/course-outline/subsection-card/SubsectionCard.test.tsx b/src/course-outline/subsection-card/SubsectionCard.test.tsx index 969852070..159f8bd5a 100644 --- a/src/course-outline/subsection-card/SubsectionCard.test.tsx +++ b/src/course-outline/subsection-card/SubsectionCard.test.tsx @@ -78,6 +78,7 @@ const subsection: XBlock = { versionAvailable: 2, versionDeclined: null, errorMessage: null, + downstreamCustomized: [] as string[], }, } satisfies Partial as XBlock; diff --git a/src/course-outline/unit-card/UnitCard.test.tsx b/src/course-outline/unit-card/UnitCard.test.tsx index e6f8c605c..c8282a273 100644 --- a/src/course-outline/unit-card/UnitCard.test.tsx +++ b/src/course-outline/unit-card/UnitCard.test.tsx @@ -68,6 +68,7 @@ const unit = { versionAvailable: 2, versionDeclined: null, errorMessage: null, + downstreamCustomized: [] as string[], }, } satisfies Partial as XBlock; diff --git a/src/course-unit/data/types.ts b/src/course-unit/data/types.ts index 99884cae5..6624dac89 100644 --- a/src/course-unit/data/types.ts +++ b/src/course-unit/data/types.ts @@ -43,7 +43,7 @@ export interface UpstreamReadyToSyncChildrenInfo { name: string; upstream: string; blockType: string; - isModified: boolean; + downstreamCustomized: string[]; } export interface CourseContainerChildrenData { diff --git a/src/data/types.ts b/src/data/types.ts index 86df4cb40..4906c15c7 100644 --- a/src/data/types.ts +++ b/src/data/types.ts @@ -61,7 +61,7 @@ export interface UpstreamInfo { versionAvailable: number | null, versionDeclined: number | null, errorMessage: string | null, - isModified?: boolean, + downstreamCustomized: string[], hasTopLevelParent?: boolean, readyToSyncChildren?: UpstreamChildrenInfo[], isReadyToSyncIndividually?: boolean, diff --git a/src/studio-home/tabs-section/messages.ts b/src/studio-home/tabs-section/messages.ts index efb2cc7f1..d69b196eb 100644 --- a/src/studio-home/tabs-section/messages.ts +++ b/src/studio-home/tabs-section/messages.ts @@ -118,7 +118,7 @@ const messages = defineMessages({ defaultMessage: 'Welcome to the new Content Libraries experience! Libraries have been redesigned' + ' from the ground up, making it much easier to reuse content. You can create, organize and manage' + ' new content, reuse your content in as many courses as you\'d like, publish updates, and create/randomize' - + ' problem sets. See {link} for details.', + + ' Problem Banks. See {link} for details.', description: 'Description for the alert message while there are no libraries pending migration on v2 tab.', }, alertDescriptionV2MigrationPending: {