Andal.LND/frontend-app-authoring
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: differentiate between renamed and data edit in sync preview diff [FC-0112] (#2577)

Browse Source 
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
This commit is contained in:
Navin Karkera
2025-10-30 03:54:04 +05:30
committed by GitHub
parent 9b77a40284
commit 2dc087f87a
13 changed files with 168 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/container-comparison/CompareContainersWidget.tsx
View File

@@ -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) {

6
src/container-comparison/ContainerRow.test.tsx
View File

@@ -74,6 +74,12 @@ describe('<ContainerRow />', () => {
expect(await screen.findByText(messages.locallyContentUpdatedBeforeMessage.defaultMessage.replace('{blockType}', 'html'))).toBeInTheDocument();
});
test('renders with rename and local content update', async () => {
render(<ContainerRow title="Test title" containerType="html" side="Before" state="locallyRenamedAndContentUpdated" originalName="Modified name" />);
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(<ContainerRow title="Test title" containerType="subsection" side="After" state="moved" />);
expect(await screen.findByText(

67
src/container-comparison/ContainerRow.tsx
View File

@@ -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 = ({
>
<Stack direction="horizontal" gap={0}>
<div
className={`px-1 align-self-stretch align-content-center rounded-left ${stateContext[0]}`}
className={`px-1 align-self-stretch align-content-center rounded-left ${stateContext.className}`}
>
<Icon size="sm" src={stateContext[1]} />
<Icon size="sm" src={stateContext.icon} />
</div>
<ActionRow className="p-2">
<Stack direction="vertical" gap={2}>
@@ -80,16 +94,29 @@ const ContainerRow = ({
/>
<span className="small font-weight-bold">{title}</span>
</Stack>
{stateContext[2] ? (
<span className="micro">
<FormattedMessage
{...stateContext[2]}
values={{
blockType: containerType,
name: originalName,
}}
/>
</span>
{stateContext.message ? (
<div className="d-flex flex-column">
<span className="micro">
<FormattedMessage
{...stateContext.message}
values={{
blockType: containerType,
name: originalName,
}}
/>
</span>
{stateContext.message2 && (
<span className="micro">
<FormattedMessage
{...stateContext.message2}
values={{
blockType: containerType,
name: originalName,
}}
/>
</span>
)}
</div>
) : (
<span className="micro">&nbsp;</span>
)}

10
src/container-comparison/data/api.mock.ts
View File

@@ -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. */

2
src/container-comparison/types.ts
View File

@@ -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> = T & { state?: ContainerState, originalName?: string };
export type WithIndex<T> = T & { index: number };

102
src/container-comparison/utils.test.ts
View File

@@ -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);
});
});

23
src/container-comparison/utils.ts
View File

@@ -54,26 +54,29 @@ export function diffPreviewContainerChildren<A extends CourseContainerChildBase,
} else {
// It was present in previous version
let state: ContainerState | undefined;
const displayName = oldVersion.upstreamLink.isModified ? oldVersion.name : newVersion.displayName;
const displayName = oldVersion.upstreamLink.downstreamCustomized.includes('display_name') ? oldVersion.name : newVersion.displayName;
let originalName: string | undefined;
// FIXME: This logic doesn't work when the content is updated locally and the upstream display name is updated.
// `isRenamed` becomes true.
// We probably need to differentiate between `contentModified` and `rename` in the backend or
// send `downstream_customized` field to the frontend and use it here.
const isRenamed = displayName !== newVersion.displayName && displayName === oldVersion.name;
const isContentModified = oldVersion.upstreamLink.downstreamCustomized.includes('data');
if (index !== oldVersion.index) {
// has moved from its position
state = 'moved';
}
if (oldVersion.upstreamLink.isModified && !isRenamed) {
// The content is updated, not the name.
state = 'locallyContentUpdated';
} else if (isRenamed) {
// Has been renamed.
// TODO: At this point we can't know if the content is updated or not
// because `upstreamLink.isModified` is also true when renaming.
state = 'locallyRenamed';
originalName = newVersion.displayName;
if ((oldVersion.upstreamLink.downstreamCustomized.length || 0) > 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';

1
src/course-outline/section-card/SectionCard.test.tsx
View File

@@ -74,6 +74,7 @@ const section = {
versionAvailable: 2,
versionDeclined: null,
errorMessage: null,
downstreamCustomized: [] as string[],
},
} satisfies Partial<XBlock> as XBlock;

1
src/course-outline/subsection-card/SubsectionCard.test.tsx
View File

@@ -78,6 +78,7 @@ const subsection: XBlock = {
versionAvailable: 2,
versionDeclined: null,
errorMessage: null,
downstreamCustomized: [] as string[],
},
} satisfies Partial<XBlock> as XBlock;

1
src/course-outline/unit-card/UnitCard.test.tsx
View File

@@ -68,6 +68,7 @@ const unit = {
versionAvailable: 2,
versionDeclined: null,
errorMessage: null,
downstreamCustomized: [] as string[],
},
} satisfies Partial<XBlock> as XBlock;

2
src/course-unit/data/types.ts
View File

@@ -43,7 +43,7 @@ export interface UpstreamReadyToSyncChildrenInfo {
name: string;
upstream: string;
blockType: string;
isModified: boolean;
downstreamCustomized: string[];
}
export interface CourseContainerChildrenData {

2
src/data/types.ts
View File

@@ -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,

2
src/studio-home/tabs-section/messages.ts
View File

@@ -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: {