[ZEPPELIN-6351] Close modals only on their own user actions
### What is this PR for?
On the home page, an open modal (e.g., `Import Note`, `Create New Note`) closes when a `NOTES_INFO` message is received.
This is unexpected and makes running parallel E2E tests that create new notes unreliable.
#### Root cause
Both modals treated any `NOTES_INFO` message as the result of *their* own submit action.
Since `NOTES_INFO` message is broadcast for various events, this led to false positives.
#### Fix
Close modals only in response to messages addressed to the submitting client: `IMPORT_NOTE` and `NEW_NOTE`. The server already sends `NEW_NOTE` (used by the old UI), but not `IMPORT_NOTE`. I added a server-sent `IMPORT_NOTE` and replaced `broadcastNote(note)` with sending `IMPORT_NOTE` only to the caller. The previously broadcast `NOTE` is mainly useful to users already on that note page; for a newly imported note, no user is on that page yet, so the broadcast is unnecessary.
### What type of PR is it?
Bug Fix
### What is the Jira issue?
[[ZEPPELIN-6351]](https://issues.apache.org/jira/browse/ZEPPELIN-6351)
### How should this be tested?
- Open two browser windows (A and B).
- In A, open a modal (e.g., Import Note or Create New Note).
- In B, perform a submit that triggers a note update. Verify the modal in A does not close.
- In A, submit the modal. Verify it closes and the note list updates.
### Screenshots (if appropriate)
#### [AS-IS]
https://github.com/user-attachments/assets/a38a8334-81d3-45ae-9264-755143df9041
#### [TO-BE]
https://github.com/user-attachments/assets/b5e9148a-2a2a-441c-a389-aa1f558a025d
### Questions:
* Does the license files need to update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
Closes #5092 from tbonelee/fix-modal-close.
Signed-off-by: ChanHo Lee <chanholee@apache.org>
diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
index d0d0af6..5e4cccd 100644
--- a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
+++ b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
@@ -1200,7 +1200,7 @@
public void onSuccess(Note note, ServiceContext context) throws IOException {
super.onSuccess(note, context);
try {
- broadcastNote(note);
+ conn.send(serializeMessage(new Message(OP.IMPORT_NOTE).put("note", note)));
broadcastNoteList(context.getAutheInfo(), context.getUserAndRoles());
} catch (NullPointerException e) {
// TODO(zjffdu) remove this try catch. This is only for test of
diff --git a/zeppelin-web-angular/projects/zeppelin-sdk/src/interfaces/message-data-type-map.interface.ts b/zeppelin-web-angular/projects/zeppelin-sdk/src/interfaces/message-data-type-map.interface.ts
index e832a1e..1dd1f58 100644
--- a/zeppelin-web-angular/projects/zeppelin-sdk/src/interfaces/message-data-type-map.interface.ts
+++ b/zeppelin-web-angular/projects/zeppelin-sdk/src/interfaces/message-data-type-map.interface.ts
@@ -21,11 +21,14 @@
FolderRename,
GetInterpreterBindings,
GetNode,
+ ImportNote,
+ ImportNoteReceived,
ListRevision,
ListRevisionHistory,
MoveFolderToTrash,
MoveNoteToTrash,
NewNote,
+ NewNoteReceived,
Note,
NotesInfo,
NoteRename,
@@ -100,7 +103,8 @@
[OP.SET_NOTE_REVISION]: SetNoteRevisionStatus;
[OP.PARAGRAPH_ADDED]: ParagraphAdded;
[OP.NOTE_RUNNING_STATUS]: NoteRunningStatus;
- [OP.NEW_NOTE]: NoteRevision;
+ [OP.NEW_NOTE]: NewNoteReceived;
+ [OP.IMPORT_NOTE]: ImportNoteReceived;
[OP.SAVE_NOTE_FORMS]: SaveNoteFormsSend;
[OP.PARAGRAPH]: UpdateParagraph;
[OP.PATCH_PARAGRAPH]: PatchParagraphSend;
@@ -154,7 +158,7 @@
[OP.COMPLETION]: Completion;
[OP.COMMIT_PARAGRAPH]: CommitParagraph;
[OP.PATCH_PARAGRAPH]: PatchParagraphReceived;
- [OP.IMPORT_NOTE]: {}; // TODO(hsuanxyz)
+ [OP.IMPORT_NOTE]: ImportNote;
[OP.CHECKPOINT_NOTE]: CheckpointNote;
[OP.SET_NOTE_REVISION]: SetNoteRevision;
[OP.LIST_REVISION_HISTORY]: ListRevisionHistory;
diff --git a/zeppelin-web-angular/projects/zeppelin-sdk/src/interfaces/message-notebook.interface.ts b/zeppelin-web-angular/projects/zeppelin-sdk/src/interfaces/message-notebook.interface.ts
index 24f55fe..f81e5c5 100644
--- a/zeppelin-web-angular/projects/zeppelin-sdk/src/interfaces/message-notebook.interface.ts
+++ b/zeppelin-web-angular/projects/zeppelin-sdk/src/interfaces/message-notebook.interface.ts
@@ -62,6 +62,10 @@
};
}
+export interface ImportNote {
+ note: Exclude<Required<Note>['note'], 'path'>;
+}
+
export interface NoteAngularObjects {
// tslint:disable-next-line no-any
[key: string]: any;
@@ -146,6 +150,14 @@
status: boolean;
}
+export interface NewNoteReceived {
+ note: Required<Note>['note'];
+}
+
+export interface ImportNoteReceived {
+ note: Required<Note>['note'];
+}
+
export interface ParagraphAdded {
index: number;
paragraph: ParagraphItem;
diff --git a/zeppelin-web-angular/projects/zeppelin-sdk/src/message.ts b/zeppelin-web-angular/projects/zeppelin-sdk/src/message.ts
index 951e140..0e4e8bd 100644
--- a/zeppelin-web-angular/projects/zeppelin-sdk/src/message.ts
+++ b/zeppelin-web-angular/projects/zeppelin-sdk/src/message.ts
@@ -20,7 +20,7 @@
MessageSendDataTypeMap,
MixMessageDataTypeMap
} from './interfaces/message-data-type-map.interface';
-import { Note, NoteConfig, PersonalizedMode, SendNote } from './interfaces/message-notebook.interface';
+import { ImportNote, Note, NoteConfig, PersonalizedMode, SendNote } from './interfaces/message-notebook.interface';
import { OP } from './interfaces/message-operator.interface';
import {
DynamicFormParams,
@@ -465,7 +465,7 @@
});
}
- importNote(note: SendNote): void {
+ importNote(note: ImportNote['note']): void {
this.send<OP.IMPORT_NOTE>(OP.IMPORT_NOTE, {
note: note
});
diff --git a/zeppelin-web-angular/src/app/services/message.service.ts b/zeppelin-web-angular/src/app/services/message.service.ts
index 675fc7a..f62697b 100644
--- a/zeppelin-web-angular/src/app/services/message.service.ts
+++ b/zeppelin-web-angular/src/app/services/message.service.ts
@@ -16,6 +16,7 @@
import { MessageInterceptor, MESSAGE_INTERCEPTOR } from '@zeppelin/interfaces';
import {
DynamicFormParams,
+ ImportNote,
Message,
MessageReceiveDataTypeMap,
MessageSendDataTypeMap,
@@ -275,7 +276,7 @@
super.patchParagraph(paragraphId, noteId, patch);
}
- importNote(note: SendNote): void {
+ importNote(note: ImportNote['note']): void {
super.importNote(note);
}
diff --git a/zeppelin-web-angular/src/app/share/note-create/note-create.component.ts b/zeppelin-web-angular/src/app/share/note-create/note-create.component.ts
index 13e1b89..173db41 100644
--- a/zeppelin-web-angular/src/app/share/note-create/note-create.component.ts
+++ b/zeppelin-web-angular/src/app/share/note-create/note-create.component.ts
@@ -38,8 +38,8 @@
this.cdr.markForCheck();
}
- @MessageListener(OP.NOTES_INFO)
- getNotes(data: MessageReceiveDataTypeMap[OP.NOTES_INFO]) {
+ @MessageListener(OP.NEW_NOTE)
+ newNoteCreated(_: MessageReceiveDataTypeMap[OP.NEW_NOTE]) {
this.nzModalRef.destroy();
}
diff --git a/zeppelin-web-angular/src/app/share/note-import/note-import.component.ts b/zeppelin-web-angular/src/app/share/note-import/note-import.component.ts
index f73fe7f..05dc9e3 100644
--- a/zeppelin-web-angular/src/app/share/note-import/note-import.component.ts
+++ b/zeppelin-web-angular/src/app/share/note-import/note-import.component.ts
@@ -19,7 +19,7 @@
import { NzUploadFile } from 'ng-zorro-antd/upload';
import { MessageListener, MessageListenersManager } from '@zeppelin/core';
-import { MessageReceiveDataTypeMap, OP, SendNote } from '@zeppelin/sdk';
+import { ImportNote, MessageReceiveDataTypeMap, OP } from '@zeppelin/sdk';
@Component({
selector: 'zeppelin-note-import',
@@ -34,8 +34,8 @@
importLoading = false;
maxLimit = get(this.ticketService.configuration, ['zeppelin.websocket.max.text.message.size'], null);
- @MessageListener(OP.NOTES_INFO)
- getNotes(data: MessageReceiveDataTypeMap[OP.NOTES_INFO]) {
+ @MessageListener(OP.IMPORT_NOTE)
+ noteImported(_: MessageReceiveDataTypeMap[OP.IMPORT_NOTE]) {
this.nzModalRef.destroy();
}
@@ -92,7 +92,7 @@
// @ts-ignore
result.name = this.noteImportName;
}
- this.messageService.importNote(result as SendNote);
+ this.messageService.importNote(result as ImportNote['note']);
} else {
this.errorText = 'Invalid JSON';
}
diff --git a/zeppelin-web/src/components/note-import/note-import.controller.js b/zeppelin-web/src/components/note-import/note-import.controller.js
index 61e21c8..fb14ac2 100644
--- a/zeppelin-web/src/components/note-import/note-import.controller.js
+++ b/zeppelin-web/src/components/note-import/note-import.controller.js
@@ -144,7 +144,7 @@
** $scope.$on functions below
*/
- $scope.$on('setNoteMenu', function(event, notes) {
+ $scope.$on('noteImported', function(event, note) {
vm.resetFlags();
angular.element('#noteImportModal').modal('hide');
});
diff --git a/zeppelin-web/src/components/websocket/websocket-event.factory.js b/zeppelin-web/src/components/websocket/websocket-event.factory.js
index ccf941b..36e9423 100644
--- a/zeppelin-web/src/components/websocket/websocket-event.factory.js
+++ b/zeppelin-web/src/components/websocket/websocket-event.factory.js
@@ -72,6 +72,8 @@
$rootScope.$broadcast('setNoteContent', data.note);
} else if (op === 'NEW_NOTE') {
$location.path('/notebook/' + data.note.id);
+ } else if (op === 'IMPORT_NOTE') {
+ $rootScope.$broadcast('noteImported', data.note);
} else if (op === 'NOTES_INFO') {
$rootScope.$broadcast('setNoteMenu', data.notes);
} else if (op === 'NOTE_RUNNING_STATUS') {
