Skip to content

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

Merged
voidmatcha merged 1 commit into
apache:masterfrom
JangAyeon:ZEPPELIN-6584
Sep 30, 2026
Merged

voidmatcha merged 1 commit into
apache:masterfrom
JangAyeon:ZEPPELIN-6584

Conversation

@JangAyeon

Copy link
Copy Markdown
Contributor

What is this PR for?

The New UI revision comparator labels a comparison as first --> second (e.g. older --> newer), but compareRevisions() built the line-level diff from the second revision back to the first. As a result, added lines were rendered as red deletions and removed lines as green insertions for paragraphs present in both revisions.

This PR computes the line diff in the same first --> second direction shown in the UI, matching the legacy AngularJS comparator (diffLines(firstText, secondText)). The whole-paragraph added/deleted classification, which was already correct, is unchanged.

What type of PR is it?

Bug Fix

Todos

  • - Fix line diff direction
  • - Add focused comparator unit tests

What is the Jira issue?

ZEPPELIN-6584

How should this be tested?

  • Unit tests:
  cd zeppelin-web-angular
  npm run test:shell -- revisions-comparator.component.spec.ts
  npm run lint

Screenshots (if appropriate)

Questions:

  • Does the license files need to update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

@jongyoul jongyoul left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed fa674b0. The first/from -> second/to direction now matches the selectors and insert/delete rendering, while whole-paragraph classification is preserved. The five focused comparator tests passed in both exact-head CI jobs; an isolated exact-source probe also distinguished the fix from the reversed-direction baseline. No actionable issue found in this bounded change.

CI is not fully green: the auth keyboard E2E failed while waiting on PENDING, the anonymous classic phase was cancelled, and Selenium setup hit a Maven Central download timeout. This is a code-review approval, not confirmation that all merge checks have passed.

@voidmatcha
voidmatcha merged commit 45f305a into apache:master Sep 30, 2026
27 of 32 checks passed
@voidmatcha

Copy link
Copy Markdown
Member

Merged into master (45f305a).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants