Skip to content

Commit 7f4133a

Browse files
chittolinagchittolinacaio-pizzol
authored
SD-2277 - fix: tracked change doesn't undo correctly (#2658)
* fix: tracked change doesn't undo correctly * chore: small code tweaks * chore: small test tweaks * test: added old assertions back * test: added old assertions back * chore: update packages/superdoc/src/stores/comments-store.js Co-authored-by: Caio Pizzol <97641911+caio-pizzol@users.noreply.github.com> * fix: redundant check * test: add behavior test for tracked-change bubble text after partial undo (SD-2277) --------- Co-authored-by: Gabriel Chittolina <gabrielchittolina1@gmail.com> Co-authored-by: Caio Pizzol <97641911+caio-pizzol@users.noreply.github.com> Co-authored-by: Caio Pizzol <caio@harbourshare.com>
1 parent fb59ef8 commit 7f4133a

3 files changed

Lines changed: 114 additions & 29 deletions

File tree

‎packages/superdoc/src/stores/comments-store.js‎

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -544,7 +544,7 @@ export const useCommentsStore = defineStore('comments', () => {
544544

545545
const emitData = {
546546
type: COMMENT_EVENTS.UPDATE,
547-
comment: existing.getValues(),
547+
comment: getCommentEventPayload(existing),
548548
};
549549

550550
syncCommentsToClients(superdoc, emitData);
@@ -567,7 +567,7 @@ export const useCommentsStore = defineStore('comments', () => {
567567

568568
const emitData = {
569569
type: COMMENT_EVENTS.UPDATE,
570-
comment: existingTrackedChange.getValues(),
570+
comment: getCommentEventPayload(existingTrackedChange),
571571
};
572572

573573
syncCommentsToClients(superdoc, emitData);
@@ -990,30 +990,39 @@ export const useCommentsStore = defineStore('comments', () => {
990990
};
991991

992992
const createCommentForTrackChanges = (editor, superdoc, trackedChangesOverride = null, options = {}) => {
993-
const { reopenResolved = false } = options;
993+
const { reopenResolved = false, refreshExisting = false } = options;
994994
const trackedChanges = trackedChangesOverride ?? trackChangesHelpers.getTrackChanges(editor.state);
995995
const groupedChanges = groupChanges(trackedChanges);
996996
const activeDocumentId = editor?.options?.documentId != null ? String(editor.options.documentId) : null;
997997
if (!activeDocumentId) return;
998998

999-
// Build a Set of existing unresolved tracked-change IDs for O(1) lookup.
1000-
// Include both runtime and imported IDs to avoid duplicate threads when
1001-
// replay/import flows remap commentId but marks still reference importedId.
999+
// Build a Set of existing unresolved tracked-change IDs for O(1) lookup
1000+
// and a map of id -> comment so we can refresh existing text when needed.
10021001
// History replay can opt in to excluding resolved tracked-change threads so
10031002
// undo/redo reopens them when their marks reappear. Initial import rebuilds
1004-
// keep resolved IDs in the set so resolved DOCX threads do not reopen on load.
1003+
// keep resolved DOCX threads in the set so resolved threads do not reopen.
10051004
const skipIds = new Set();
1005+
const existingTrackedChangeById = new Map();
10061006
commentsList.value.forEach((comment) => {
10071007
if (!comment?.trackedChange) return;
10081008
if (!belongsToDocument(comment, activeDocumentId)) return;
1009-
if (comment.resolvedTime && !reopenResolved) {
1010-
if (comment.commentId != null) skipIds.add(String(comment.commentId));
1011-
if (comment.importedId != null) skipIds.add(String(comment.importedId));
1009+
const commentIds = [comment.commentId, comment.importedId]
1010+
.map((id) => (id != null ? String(id) : null))
1011+
.filter(Boolean);
1012+
1013+
if (comment.resolvedTime) {
1014+
if (!reopenResolved) {
1015+
commentIds.forEach((id) => skipIds.add(id));
1016+
}
10121017
return;
10131018
}
1014-
if (comment.resolvedTime) return;
1015-
if (comment.commentId != null) skipIds.add(String(comment.commentId));
1016-
if (comment.importedId != null) skipIds.add(String(comment.importedId));
1019+
1020+
commentIds.forEach((id) => {
1021+
existingTrackedChangeById.set(id, comment);
1022+
if (!refreshExisting) {
1023+
skipIds.add(id);
1024+
}
1025+
});
10171026
});
10181027

10191028
// Build a Map of change ID → tracked change entries for O(1) lookup per group.
@@ -1028,9 +1037,16 @@ export const useCommentsStore = defineStore('comments', () => {
10281037
const documentId = activeDocumentId;
10291038

10301039
// Build comment params directly from grouped changes — no PM dispatch needed
1040+
const processedIds = new Set();
10311041
groupedChanges.forEach(({ insertedMark, deletionMark, formatMark }) => {
10321042
const id = insertedMark?.mark.attrs.id || deletionMark?.mark.attrs.id || formatMark?.mark.attrs.id;
1033-
if (!id || skipIds.has(id)) return;
1043+
if (id == null) return;
1044+
const normalizedId = String(id);
1045+
if (processedIds.has(normalizedId)) return;
1046+
processedIds.add(normalizedId);
1047+
1048+
if (!refreshExisting && skipIds.has(normalizedId)) return;
1049+
const existingTrackedChange = existingTrackedChangeById.get(normalizedId);
10341050

10351051
const marks = {
10361052
...(insertedMark && { insertedMark: insertedMark.mark }),
@@ -1041,7 +1057,7 @@ export const useCommentsStore = defineStore('comments', () => {
10411057
// nodes/deletionNodes are unused here — the function resolves them from
10421058
// trackedChangesForId which already contains all document positions for this ID.
10431059
const params = createOrUpdateTrackedChangeComment({
1044-
event: 'add',
1060+
event: existingTrackedChange ? 'update' : 'add',
10451061
marks,
10461062
nodes: [],
10471063
newEditorState: editor.state,
@@ -1051,9 +1067,11 @@ export const useCommentsStore = defineStore('comments', () => {
10511067

10521068
if (params) {
10531069
handleTrackedChangeUpdate({ superdoc, params });
1054-
skipIds.add(String(id));
1055-
if (params.changeId != null) skipIds.add(String(params.changeId));
1056-
if (params.importedId != null) skipIds.add(String(params.importedId));
1070+
if (!existingTrackedChange) {
1071+
skipIds.add(normalizedId);
1072+
if (params.changeId != null) skipIds.add(String(params.changeId));
1073+
if (params.importedId != null) skipIds.add(String(params.importedId));
1074+
}
10571075
}
10581076
});
10591077

@@ -1227,7 +1245,7 @@ export const useCommentsStore = defineStore('comments', () => {
12271245
});
12281246

12291247
pruneStaleTrackedChangeComments(liveTrackedChangeIds, activeDocumentId, superdoc);
1230-
createCommentForTrackChanges(editor, superdoc, trackedChanges, { reopenResolved: true });
1248+
createCommentForTrackChanges(editor, superdoc, trackedChanges, { reopenResolved: true, refreshExisting: true });
12311249
};
12321250

12331251
const normalizeDocxSchemaForExport = (value) => {

‎packages/superdoc/src/stores/comments-store.test.js‎

Lines changed: 50 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -895,14 +895,52 @@ describe('comments-store', () => {
895895

896896
store.syncTrackedChangeComments({ superdoc: {}, editor });
897897

898+
expect(store.commentsList).toHaveLength(2);
898899
expect(store.commentsList).toEqual([
899-
{ commentId: 'tc-live', trackedChange: true, trackedChangeText: 'Existing', fileId: 'doc-1' },
900-
{ commentId: 'normal-1', commentText: 'Regular comment', fileId: 'doc-1' },
900+
expect.objectContaining({
901+
commentId: 'tc-live',
902+
trackedChange: true,
903+
trackedChangeText: 'tracked-tc-live',
904+
trackedChangeType: 'insert',
905+
}),
906+
expect.objectContaining({ commentId: 'normal-1', commentText: 'Regular comment', fileId: 'doc-1' }),
901907
]);
902908
expect(tr.setMeta).toHaveBeenCalledWith('CommentsPluginKey', { type: 'force' });
903909
expect(editorDispatch).toHaveBeenCalledWith(tr);
904910
});
905911

912+
it('refreshes tracked-change text from the document during replay sync', () => {
913+
const editorDispatch = vi.fn();
914+
const tr = { setMeta: vi.fn() };
915+
const superdoc = { emit: vi.fn() };
916+
const editor = {
917+
state: {},
918+
view: { state: { tr }, dispatch: editorDispatch },
919+
options: { documentId: 'doc-1' },
920+
};
921+
922+
trackChangesHelpersMock.getTrackChanges.mockReturnValue([{ mark: { attrs: { id: 'tc-live' } }, from: 0, to: 4 }]);
923+
groupChangesMock.mockReturnValue([{ insertedMark: { mark: { attrs: { id: 'tc-live' } } } }]);
924+
925+
const existingComment = {
926+
commentId: 'tc-live',
927+
trackedChange: true,
928+
trackedChangeText: 'Old text',
929+
fileId: 'doc-1',
930+
getValues: vi.fn(() => ({ commentId: 'tc-live' })),
931+
};
932+
933+
store.commentsList = [existingComment];
934+
935+
store.syncTrackedChangeComments({ superdoc, editor });
936+
937+
const createCall = createOrUpdateTrackedChangeCommentMock.mock.calls[0]?.[0];
938+
expect(createCall?.event).toBe('update');
939+
expect(existingComment.trackedChangeText).toBe('tracked-tc-live');
940+
expect(tr.setMeta).toHaveBeenCalledWith('CommentsPluginKey', { type: 'force' });
941+
expect(editorDispatch).toHaveBeenCalledWith(tr);
942+
});
943+
906944
it('keeps imported resolved tracked-change comments resolved during initial tracked-change rebuild', async () => {
907945
const editorDispatch = vi.fn();
908946
const tr = { setMeta: vi.fn() };
@@ -1150,15 +1188,16 @@ describe('comments-store', () => {
11501188

11511189
store.syncTrackedChangeComments({ superdoc: {}, editor });
11521190

1191+
expect(store.commentsList).toHaveLength(2);
11531192
expect(store.commentsList).toEqual([
1154-
{
1193+
expect.objectContaining({
11551194
commentId: 'runtime-id-123',
11561195
importedId: 'tc-live-imported',
1157-
trackedChange: true,
11581196
trackedChangeText: 'Existing',
1197+
trackedChange: true,
11591198
fileId: 'doc-1',
1160-
},
1161-
{ commentId: 'normal-1', commentText: 'Regular comment', fileId: 'doc-1' },
1199+
}),
1200+
expect.objectContaining({ commentId: 'normal-1', commentText: 'Regular comment', fileId: 'doc-1' }),
11621201
]);
11631202
expect(tr.setMeta).toHaveBeenCalledWith('CommentsPluginKey', { type: 'force' });
11641203
expect(editorDispatch).toHaveBeenCalledWith(tr);
@@ -1189,15 +1228,16 @@ describe('comments-store', () => {
11891228

11901229
store.syncTrackedChangeComments({ superdoc: {}, editor });
11911230

1231+
expect(store.commentsList).toHaveLength(2);
11921232
expect(store.commentsList).toEqual([
1193-
{
1233+
expect.objectContaining({
11941234
commentId: 'runtime-id-123',
11951235
importedId: 'tc-live-imported',
11961236
trackedChange: true,
1197-
trackedChangeText: 'Existing',
1237+
trackedChangeText: 'tracked-tc-live-imported',
11981238
fileId: 'doc-1',
1199-
},
1200-
{ commentId: 'normal-1', commentText: 'Regular comment', fileId: 'doc-1' },
1239+
}),
1240+
expect.objectContaining({ commentId: 'normal-1', commentText: 'Regular comment', fileId: 'doc-1' }),
12011241
]);
12021242
expect(tr.setMeta).toHaveBeenCalledWith('CommentsPluginKey', { type: 'force' });
12031243
expect(editorDispatch).toHaveBeenCalledWith(tr);

‎tests/behavior/tests/comments/undo-redo-tracked-change-sidebar.spec.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,33 @@ for (const changeType of CHANGE_TYPES) {
133133
});
134134
}
135135

136+
test('partial undo updates tracked-change bubble text to match the document (SD-2277)', async ({ superdoc }) => {
137+
const sidebar = superdoc.page.locator('.superdoc__right-sidebar');
138+
const bubbleText = sidebar.locator('.tracked-change-text.is-inserted');
139+
140+
await assertDocumentApiReady(superdoc.page);
141+
await superdoc.setDocumentMode('suggesting');
142+
await superdoc.waitForStable();
143+
144+
await superdoc.type('hello');
145+
await superdoc.waitForStable();
146+
await superdoc.page.waitForTimeout(500);
147+
await superdoc.type(' world');
148+
await superdoc.waitForStable();
149+
150+
await expectTrackedState(superdoc.page, { changes: 1, bubbles: 1 });
151+
await expect(bubbleText).toContainText('hello world');
152+
153+
await superdoc.undo();
154+
await superdoc.waitForStable();
155+
await expect(bubbleText).toContainText('hello');
156+
await expect(bubbleText).not.toContainText('world');
157+
158+
await superdoc.redo();
159+
await superdoc.waitForStable();
160+
await expect(bubbleText).toContainText('hello world');
161+
});
162+
136163
test('accepting from the tracked-change bubble can be undone immediately with the keyboard shortcut', async ({
137164
superdoc,
138165
}) => {

0 commit comments

Comments
 (0)