[ZEPPELIN-6697] Prevent live NOTE_UPDATED from mutating revision snapshots

[xhaktm00]

What is this PR for?

NotebookServer.updateNote() broadcasts NOTE_UPDATED to every socket associated with the note, and a socket that navigates to a saved revision keeps that association. The Angular handler applied the payload unconditionally:

@MessageListener(OP.NOTE_UPDATED)
noteUpdated(data) {
  if (!this.note) {
    return;
  }
  this.note.config = data.config;   // overwrites the historical snapshot

So changing the live note's look and feel from another browser rewrote the snapshot the revision view was showing.

The guard already exists elsewhere in the same component — removeParagraph, addParagraph and moveParagraph all check revisionView, and ZEPPELIN-2452 added the same check to the classic UI. It was only partly carried over during the migration, and noteUpdated was the one left out. This adds it there.

Per the issue, this stays out of noteId, request correlation and socket generation: the reproduction only proves a revision-isolation defect, and nothing here required a wire change. The Shared Notebook Core revision adapter is not covered yet, since that slice has not landed.

What type of PR is it?

Bug Fix

Todos

• Guard noteUpdated with revisionView
• Add a retained Playwright regression for the same-tab live-to-revision route

What is the Jira issue?

• ZEPPELIN-6697

How should this be tested?

New spec e2e/tests/notebook/revision/revision-isolation.spec.ts opens a note, saves a revision, enters it on the same socket, then changes look and feel from an independent browser context. It asserts the live follower applies the update, and that the revision view still shows the value it was saved with.

cd zeppelin-web-angular && npx playwright test e2e/tests/notebook/revision/revision-isolation.spec.ts --project=chromium --reporter=list

Result: 2 passed (22.5s). --reporter=list is worth passing locally, since the configured HTML reporter opens a report server afterwards and the command does not return on its own.

Reverting only the guard makes it fail, so it does catch the defect rather than passing by construction:

Error: expect(locator).toContainText(expected) failed
Expected string: "default"
Received string: " simple "

Two details the spec depends on, both found the hard way:

• Look and feel, not rename. Renaming goes through renameNote, which answers with broadcastNote() (OP.NOTE) and never emits NOTE_UPDATED. setLookAndFeel() is what calls updateNote(), which is why the issue names it in the reproduction.
• Waiting for the event. Asserting the snapshot straight after the live change passes trivially, because the assertion resolves before the broadcast arrives. The spec first polls the revision page's console for Receive: NOTE_UPDATED, then asserts.

Screenshots (if appropriate)

N/A

Questions:

• Does the license files need to update? No
• Is there breaking changes for older versions? No — a live follower still applies the same update exactly once; only a revision view ignores it
• Does this needs documentation? No

[voidmatcha]

Ignoring NOTE_UPDATED while viewing a revision looks reasonable to me.

I noticed two things in the test. The current CI uses VFS storage, which does not support revisions, so the test fails while waiting for the commit button. Also, livePage is the page making the change, so checking its value alone does not verify that a live viewer received the broadcast.

I prepared a follow-up commit that runs this test separately with Git storage and checks the update in another live viewer that makes no local edits. Feel free to use it as a reference or cherry-pick it if helpful.

[voidmatcha]

LGTM 👍

One minor thing: skipWhenAuthenticationIsStillRequired is missing from this spec's guard (still present in collaborative-mode.spec.ts etc.). Not blocking, worth cleaning up later.

[voidmatcha]

Merged into master (5e36932).