This is an automated email from the ASF dual-hosted git repository.
voidmatcha pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push:
new 532d86d3e2 [ZEPPELIN-6582] New UI: clearing a paragraph in
collaborative mode throws and does not sync the empty text
532d86d3e2 is described below
commit 532d86d3e2eb9dee37f791a1d49ab27e8582fcc8
Author: Lee SuJung <[email protected]>
AuthorDate: Fri Aug 21 00:58:48 2026 +0900
[ZEPPELIN-6582] New UI: clearing a paragraph in collaborative mode throws
and does not sync the empty text
### What is this PR for?
`ParagraphComponent.sendPatch()` guards `dirtyText` with a falsy check:
```typescript
sendPatch() {
if (!this.dirtyText) {
throw new Error('dirtyText is required');
}
```
An empty string is a valid paragraph state, but `!''` is `true`, so
deleting the last character of a paragraph throws before `patchParagraph(...)`
is reached. The empty edit never leaves the client, other sessions keep the
previous non-empty text, and `originalText` stays at the stale value — so even
later edits are diffed against text the paragraph no longer has.
The guard was introduced while migrating the frontend to `strict: true` in
ZEPPELIN-6252, which intended to preserve existing behaviour; the previous
runtime accepted empty paragraph text.
`dirtyText` is typed `dirtyText?: string`, so the only value the guard
needs to reject is `undefined`. This PR checks for that instead — the same
check `saveParagraph()` already performs on the same field a few lines below:
```typescript
if (this.dirtyText === undefined) {
```
### What type of PR is it?
Bug Fix
### Todos
* [x] Accept the empty string in `sendPatch()` while still rejecting an
unset `dirtyText`
* [x] Add frontend coverage for the non-empty-to-empty transition
### What is the Jira issue?
* [ZEPPELIN-6582](https://issues.apache.org/jira/browse/ZEPPELIN-6582)
### How should this be tested?
New spec `paragraph.component.spec.ts` covers both directions of the guard:
clearing a paragraph should send a patch turning `'abc'` into `''` and advance
`originalText` to `''`, and an unset `dirtyText` should still throw without
calling `patchParagraph`. It follows the existing
`react-mount.directive.spec.ts` style, constructing the component without its
constructor.
```
cd zeppelin-web-angular
npm run test:shell
npm run lint
```
**I could not run `test:shell` locally** — `vitest` and `jsdom` are missing
from this checkout and the npm registry returned `503` throughout, so please
treat CI as the authoritative check on the new spec. Verified instead: `tsc
--noEmit` passes, the pre-commit hook ran eslint and prettier clean, and
`patch_make('abc', '')` does apply back to `''`. Not verified: that
`Object.create(...prototype)` construction works under the vitest runtime.
Manual: open the same note in two sessions, type into a paragraph, then
delete all of it. Before this change the editing session logs `Error: dirtyText
is required` and the other session keeps the old text; after it, both end up
with an empty editor and no console error.
### Screenshots (if appropriate)
N/A
### Questions:
* Does the license files need to update? No
* Is there breaking changes for older versions? No — this only widens an
input guard, and non-collaborative saving goes through `saveParagraph()`, which
is unchanged
* Does this needs documentation? No
### Note for reviewers
ZEPPELIN-6563 also touches `sendPatch()` (it attaches checksums to each
patch). The two changes are independent — this one is about the guard at the
top of the method — but whichever merges second will need a trivial rebase.
Closes #5428 from xhaktm00/ZEPPELIN-6582.
Signed-off-by: YONGJAE LEE <[email protected]>
---
.../notebook/paragraph/paragraph-patch.spec.ts | 32 ++++++++++++++++++++++
.../notebook/paragraph/paragraph-patch.ts | 32 ++++++++++++++++++++++
.../notebook/paragraph/paragraph.component.ts | 9 ++----
3 files changed, 67 insertions(+), 6 deletions(-)
diff --git
a/zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/paragraph-patch.spec.ts
b/zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/paragraph-patch.spec.ts
new file mode 100644
index 0000000000..cb72ffe514
--- /dev/null
+++
b/zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/paragraph-patch.spec.ts
@@ -0,0 +1,32 @@
+/*
+ * 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 { diff_match_patch as DiffMatchPatch } from 'diff-match-patch';
+import { describe, expect, it } from 'vitest';
+
+import { makeParagraphPatch } from './paragraph-patch';
+
+describe('makeParagraphPatch', () => {
+ it('builds a patch that clears the paragraph', () => {
+ const dmp = new DiffMatchPatch();
+
+ const { patch, originalText } = makeParagraphPatch(dmp, 'abc', '');
+
+ // without this patch the collaborating client keeps the previous text
+ expect(dmp.patch_apply(dmp.patch_fromText(patch), 'abc')[0]).toBe('');
+ expect(originalText).toBe('');
+ });
+
+ it('rejects text that was never set', () => {
+ expect(() => makeParagraphPatch(new DiffMatchPatch(), 'abc',
undefined)).toThrow('dirtyText is required');
+ });
+});
diff --git
a/zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/paragraph-patch.ts
b/zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/paragraph-patch.ts
new file mode 100644
index 0000000000..0888a70763
--- /dev/null
+++
b/zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/paragraph-patch.ts
@@ -0,0 +1,32 @@
+/*
+ * 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 * as DiffMatchPatch from 'diff-match-patch';
+
+/**
+ * Builds the patch a collaborating client sends after an edit. An empty
string is a valid
+ * paragraph state, so only text that was never set is rejected.
+ */
+export function makeParagraphPatch(
+ diffMatchPatch: DiffMatchPatch,
+ originalText: string | undefined,
+ dirtyText: string | undefined
+): { patch: string; originalText: string } {
+ if (dirtyText === undefined) {
+ throw new Error('dirtyText is required');
+ }
+ const previousText = originalText ? originalText : '';
+ return {
+ patch: diffMatchPatch.patch_make(previousText, dirtyText).toString(),
+ originalText: dirtyText
+ };
+}
diff --git
a/zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/paragraph.component.ts
b/zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/paragraph.component.ts
index 186b4c595c..17c9a4d578 100644
---
a/zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/paragraph.component.ts
+++
b/zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/paragraph.component.ts
@@ -63,6 +63,7 @@ import {
import { NzResizeEvent } from 'ng-zorro-antd/resizable';
import { NotebookParagraphResultComponent } from
'../../share/result/result.component';
import { NotebookParagraphCodeEditorComponent } from
'./code-editor/code-editor.component';
+import { makeParagraphPatch } from './paragraph-patch';
type Mode = 'edit' | 'command';
@@ -197,12 +198,8 @@ export class NotebookParagraphComponent
}
sendPatch() {
- if (!this.dirtyText) {
- throw new Error('dirtyText is required');
- }
- this.originalText = this.originalText ? this.originalText : '';
- const patch = this.diffMatchPatch.patch_make(this.originalText,
this.dirtyText).toString();
- this.originalText = this.dirtyText;
+ const { patch, originalText } = makeParagraphPatch(this.diffMatchPatch,
this.originalText, this.dirtyText);
+ this.originalText = originalText;
this.messageService.patchParagraph(this.paragraph.id, this.note.id, patch);
}
