[ZEPPELIN-6584] Fix reversed line diff direction in New UI revision comparator

zeppelin-web-angular/src/app/pages/workspace/notebook/revisions-comparator/revisions-comparator.component.spec.ts

@@ -0,0 +1,98 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { ChangeDetectorRef } from '@angular/core';
+import { DatePipe } from '@angular/common';
+import { describe, expect, it, vi } from 'vitest';
+
+import { NoteRevisionForCompareReceived } from '@zeppelin/sdk';
+import { MessageService } from '@zeppelin/services';
+
+import { NotebookRevisionsComparatorComponent } from './revisions-comparator.component';
+
+// The barrel pulls in monaco-editor; the comparator only needs the constructor token.
+vi.mock('@zeppelin/services', () => ({ MessageService: class {} }));
+
+interface ParagraphFixture {
+  id: string;
+  text: string;
+}
+
+const revision = (revisionId: string, paragraphs: ParagraphFixture[]): NoteRevisionForCompareReceived =>
+  ({
+    noteId: 'note',
+    revisionId,
+    position: '',
+    note: { paragraphs }
+  }) as unknown as NoteRevisionForCompareReceived;
+
+const compare = (first: NoteRevisionForCompareReceived, second: NoteRevisionForCompareReceived) => {
+  const component = new NotebookRevisionsComparatorComponent(
+    {} as MessageService,
+    {} as ChangeDetectorRef,
+    new DatePipe('en-US')
+  );
+  component.firstNoteRevisionForCompare = first;
+  component.secondNoteRevisionForCompare = second;
+  component.compareRevisions();
+  return component.mergeNoteRevisionsDiff;
+};
+
+const segmentTexts = (diff: ReturnType<typeof compare>[number], type: 'insert' | 'delete') =>
+  (diff.segments || []).filter(s => s.type === type).map(s => s.text);
+
+const older = revision('older', [{ id: 'p', text: 'line common\nold line' }]);
+const newer = revision('newer', [{ id: 'p', text: 'line common\nnew line' }]);
+
+describe('NotebookRevisionsComparatorComponent.compareRevisions', () => {
+  it('marks removed lines as deleted and added lines as inserted for older --> newer', () => {
+    const [diff] = compare(older, newer);
+
+    expect(diff.type).toBe('compared');
+    expect(diff.identical).toBe(false);
+    expect(segmentTexts(diff, 'delete')).toEqual(['old line']);
+    expect(segmentTexts(diff, 'insert')).toEqual(['new line']);
+  });
+
+  it('reverses the line diff when the revisions are selected in the opposite order', () => {
+    const [diff] = compare(newer, older);
+
+    expect(segmentTexts(diff, 'delete')).toEqual(['new line']);
+    expect(segmentTexts(diff, 'insert')).toEqual(['old line']);
+  });
+
+  it('marks a paragraph with unchanged text as identical', () => {
+    const [diff] = compare(older, revision('same', [{ id: 'p', text: 'line common\nold line' }]));
+
+    expect(diff.type).toBe('compared');
+    expect(diff.identical).toBe(true);
+    expect(diff.segments?.every(s => s.type === 'equal')).toBe(true);
+  });
+
+  it('classifies a paragraph only in the second revision as added', () => {
+    const diffs = compare(revision('first', []), revision('second', [{ id: 'new', text: 'added\nbody' }]));
+
+    expect(diffs).toHaveLength(1);
+    expect(diffs[0].type).toBe('added');
+    expect(diffs[0].paragraph.id).toBe('new');
+    expect(diffs[0].firstString).toBe('added');
+  });
+
+  it('classifies a paragraph only in the first revision as deleted', () => {
+    const diffs = compare(revision('first', [{ id: 'gone', text: 'removed\nbody' }]), revision('second', []));
+
+    expect(diffs).toHaveLength(1);
+    expect(diffs[0].type).toBe('deleted');
+    expect(diffs[0].paragraph.id).toBe('gone');
+    expect(diffs[0].firstString).toBe('removed');
+  });
+});

zeppelin-web-angular/src/app/pages/workspace/notebook/revisions-comparator/revisions-comparator.component.ts

@@ -12,7 +12,7 @@
 
 import { DatePipe } from '@angular/common';
 import { ChangeDetectionStrategy, ChangeDetectorRef, Component, Input, OnDestroy, OnInit } from '@angular/core';
-import * as DiffMatchPatch from 'diff-match-patch';
+import { diff_match_patch as DiffMatchPatch } from 'diff-match-patch';
 import { Subscription } from 'rxjs';
 
 import { NoteRevisionForCompareReceived, OP, ParagraphItem, RevisionListItem } from '@zeppelin/sdk';
@@ -127,38 +127,37 @@ export class NotebookRevisionsComparatorComponent implements OnInit, OnDestroy {
     if (!this.firstNoteRevisionForCompare || !this.secondNoteRevisionForCompare) {
       return;
     }
-    const baseParagraphs = this.secondNoteRevisionForCompare.note?.paragraphs || [];
-    const compareParagraphs = this.firstNoteRevisionForCompare.note?.paragraphs || [];
+    // The UI reads `first --> second`, so every diff runs from first (from) to second (to).
+    const fromParagraphs = this.firstNoteRevisionForCompare.note?.paragraphs || [];
+    const toParagraphs = this.secondNoteRevisionForCompare.note?.paragraphs || [];
     const paragraphDiffs: MergedParagraphDiff[] = [];
 
-    for (const p1 of baseParagraphs) {
-      const p2 = compareParagraphs.find((p: ParagraphItem) => p.id === p1.id) || null;
-      if (p2 === null) {
+    for (const toParagraph of toParagraphs) {
+      const fromParagraph = fromParagraphs.find((p: ParagraphItem) => p.id === toParagraph.id) || null;
+      if (fromParagraph === null) {
         paragraphDiffs.push({
-          paragraph: p1,
-          firstString: (p1.text || '').split('\n')[0],
+          paragraph: toParagraph,
+          firstString: (toParagraph.text || '').split('\n')[0],
           type: 'added'
         });
       } else {
-        const text1 = p1.text || '';
-        const text2 = p2.text || '';
-        const diffResult = this.buildLineDiff(text1, text2);
+        const diffResult = this.buildLineDiff(fromParagraph.text || '', toParagraph.text || '');
         paragraphDiffs.push({
-          paragraph: p1,
+          paragraph: toParagraph,
           segments: diffResult.segments,
           identical: diffResult.identical,
-          firstString: (p1.text || '').split('\n')[0],
+          firstString: (toParagraph.text || '').split('\n')[0],
           type: 'compared'
         });
       }
     }
 
-    for (const p2 of compareParagraphs) {
-      const p1 = baseParagraphs.find((p: ParagraphItem) => p.id === p2.id) || null;
-      if (p1 === null) {
+    for (const fromParagraph of fromParagraphs) {
+      const toParagraph = toParagraphs.find((p: ParagraphItem) => p.id === fromParagraph.id) || null;
+      if (toParagraph === null) {
         paragraphDiffs.push({
-          paragraph: p2,
-          firstString: (p2.text || '').split('\n')[0],
+          paragraph: fromParagraph,
+          firstString: (fromParagraph.text || '').split('\n')[0],
           type: 'deleted'
         });
       }
@@ -183,8 +182,8 @@ export class NotebookRevisionsComparatorComponent implements OnInit, OnDestroy {
     return this.datePipe.transform(time * 1000, 'MMMM d yyyy, h:mm:ss a') || '';
   }
 
-  private buildLineDiff(text1: string, text2: string): { segments: DiffSegment[]; identical: boolean } {
-    const { chars1, chars2, lineArray } = this.dmp.diff_linesToChars_(text1, text2);
+  private buildLineDiff(fromText: string, toText: string): { segments: DiffSegment[]; identical: boolean } {
+    const { chars1, chars2, lineArray } = this.dmp.diff_linesToChars_(fromText, toText);
     const diffs = this.dmp.diff_main(chars1, chars2, false);
     this.dmp.diff_charsToLines_(diffs, lineArray);