diff --git a/zeppelin-web-angular/src/app/pages/workspace/notebook/revisions-comparator/revisions-comparator.component.spec.ts b/zeppelin-web-angular/src/app/pages/workspace/notebook/revisions-comparator/revisions-comparator.component.spec.ts new file mode 100644 index 00000000000..7cd14e8233d --- /dev/null +++ b/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[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'); + }); +}); diff --git a/zeppelin-web-angular/src/app/pages/workspace/notebook/revisions-comparator/revisions-comparator.component.ts b/zeppelin-web-angular/src/app/pages/workspace/notebook/revisions-comparator/revisions-comparator.component.ts index b29f42dad5e..2bd19ef90fb 100644 --- a/zeppelin-web-angular/src/app/pages/workspace/notebook/revisions-comparator/revisions-comparator.component.ts +++ b/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);