Skip to content

[ZEPPELIN-6661] Cover notebook editor save timing - #5483

Open
voidmatcha wants to merge 2 commits into
apache:masterfrom
voidmatcha:ZEPPELIN-6661-editor-save-timing
Open

voidmatcha wants to merge 2 commits into
apache:masterfrom
voidmatcha:ZEPPELIN-6661-editor-save-timing

Conversation

@voidmatcha

@voidmatcha voidmatcha commented Sep 14, 2026

Copy link
Copy Markdown
Member

What is this PR for?

When a paragraph save response arrives late, ParagraphBase treats it as a remote update and replaces the editor text, losing edits typed in the meantime.

This PR tracks pending saves by msgId. A response to one of the user's own saves no longer replaces the editor text when a newer edit or save followed it; otherwise the response is the server copy and is applied as before. Other paragraph fields in that response always apply.

Related behavior changes in ParagraphBase:

  • A remote edit that differs from the local unsaved text now replaces it and resets the saved baseline, so the discarded local text is not re-committed later.
  • An empty local edit (all text deleted) is now treated as a local edit and protected like any other.
  • A broadcast that does not match a pending local save and repeats the saved text no longer rewinds the editor while the user is typing.

This PR adds the E2E scenarios for ZEPPELIN-6661 (idle autosave, edit during an in-flight save). The in-flight scenario fails without this fix, so the fix is included here.

What type of PR is it?

Bug Fix

Todos

  • Match save responses to pending saves by msgId
  • Add unit tests and browser scenarios for save timing
  • Register the scenarios in the notebook parity inventory

What is the Jira issue?

ZEPPELIN-6661

How should this be tested?

cd zeppelin-web-angular
npx vitest run --config vitest.shell.config.mts src/app/core/paragraph-base/paragraph-base.spec.ts
npm run check:notebook-parity-scenarios
CI=true PLAYWRIGHT_HTML_OPEN=never npm run e2e -- --reporter=list tests/notebook/persistence/notebook-save-timing.spec.ts

Screenshots (if appropriate)

N/A

Questions:

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

@voidmatcha
voidmatcha force-pushed the ZEPPELIN-6661-editor-save-timing branch 4 times, most recently from 532b535 to 68184b0 Compare September 15, 2026 16:36
@voidmatcha
voidmatcha marked this pull request as ready for review September 16, 2026 13:17
@voidmatcha
voidmatcha force-pushed the ZEPPELIN-6661-editor-save-timing branch 7 times, most recently from ee99efb to 6d323f2 Compare September 17, 2026 18:17
@voidmatcha

Copy link
Copy Markdown
Member Author

This is why the PR changes Angular code. On master, an edit typed while a save is in flight is overwritten when the delayed save response arrives. This branch puts only this PR's E2E files on master, without the fix: https://github.com/voidmatcha/zeppelin/tree/ZEPPELIN-6661-repro-on-master

CI results (Playwright, anonymous mode):

App code NB-PARITY-050 NB-PARITY-051
master (run) pass fails on Chromium, Firefox and WebKit: the editor rewinds to %md First pending save (expected %md First pending save; latest edit wins)
this PR pass pass

Angular changes:

  • ParagraphBase: tracks sent saves by msgId; a response to the user's own save no longer changes the editor text.
  • NotebookParagraphComponent: records the msgId when it commits a paragraph.
  • MessageService: passes the msgId of PARAGRAPH responses to the handler.
  • SDK Message.send() and commitParagraph(): return the sent msgId.

@voidmatcha
voidmatcha force-pushed the ZEPPELIN-6661-editor-save-timing branch from 5697900 to f350173 Compare September 20, 2026 02:12
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.

1 participant