Skip to content

Commit 3ad1a2c

Browse files
authored
[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 <[email protected]>
1 parent 9aac655 commit 3ad1a2c

File tree

9 files changed

+32
-13
lines changed

9 files changed

+32
-13
lines changed

zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1200,7 +1200,7 @@ protected String importNote(NotebookSocket conn, ServiceContext context, Message
12001200
public void onSuccess(Note note, ServiceContext context) throws IOException {
12011201
super.onSuccess(note, context);
12021202
try {
1203-
broadcastNote(note);
1203+
conn.send(serializeMessage(new Message(OP.IMPORT_NOTE).put("note", note)));
12041204
broadcastNoteList(context.getAutheInfo(), context.getUserAndRoles());
12051205
} catch (NullPointerException e) {
12061206
// TODO(zjffdu) remove this try catch. This is only for test of

zeppelin-web-angular/projects/zeppelin-sdk/src/interfaces/message-data-type-map.interface.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,14 @@ import {
2121
FolderRename,
2222
GetInterpreterBindings,
2323
GetNode,
24+
ImportNote,
25+
ImportNoteReceived,
2426
ListRevision,
2527
ListRevisionHistory,
2628
MoveFolderToTrash,
2729
MoveNoteToTrash,
2830
NewNote,
31+
NewNoteReceived,
2932
Note,
3033
NotesInfo,
3134
NoteRename,
@@ -100,7 +103,8 @@ export interface MessageReceiveDataTypeMap {
100103
[OP.SET_NOTE_REVISION]: SetNoteRevisionStatus;
101104
[OP.PARAGRAPH_ADDED]: ParagraphAdded;
102105
[OP.NOTE_RUNNING_STATUS]: NoteRunningStatus;
103-
[OP.NEW_NOTE]: NoteRevision;
106+
[OP.NEW_NOTE]: NewNoteReceived;
107+
[OP.IMPORT_NOTE]: ImportNoteReceived;
104108
[OP.SAVE_NOTE_FORMS]: SaveNoteFormsSend;
105109
[OP.PARAGRAPH]: UpdateParagraph;
106110
[OP.PATCH_PARAGRAPH]: PatchParagraphSend;
@@ -154,7 +158,7 @@ export interface MessageSendDataTypeMap {
154158
[OP.COMPLETION]: Completion;
155159
[OP.COMMIT_PARAGRAPH]: CommitParagraph;
156160
[OP.PATCH_PARAGRAPH]: PatchParagraphReceived;
157-
[OP.IMPORT_NOTE]: {}; // TODO(hsuanxyz)
161+
[OP.IMPORT_NOTE]: ImportNote;
158162
[OP.CHECKPOINT_NOTE]: CheckpointNote;
159163
[OP.SET_NOTE_REVISION]: SetNoteRevision;
160164
[OP.LIST_REVISION_HISTORY]: ListRevisionHistory;

zeppelin-web-angular/projects/zeppelin-sdk/src/interfaces/message-notebook.interface.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,10 @@ export interface Note {
6262
};
6363
}
6464

65+
export interface ImportNote {
66+
note: Exclude<Required<Note>['note'], 'path'>;
67+
}
68+
6569
export interface NoteAngularObjects {
6670
// tslint:disable-next-line no-any
6771
[key: string]: any;
@@ -146,6 +150,14 @@ export interface NoteRunningStatus {
146150
status: boolean;
147151
}
148152

153+
export interface NewNoteReceived {
154+
note: Required<Note>['note'];
155+
}
156+
157+
export interface ImportNoteReceived {
158+
note: Required<Note>['note'];
159+
}
160+
149161
export interface ParagraphAdded {
150162
index: number;
151163
paragraph: ParagraphItem;

zeppelin-web-angular/projects/zeppelin-sdk/src/message.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {
2020
MessageSendDataTypeMap,
2121
MixMessageDataTypeMap
2222
} from './interfaces/message-data-type-map.interface';
23-
import { Note, NoteConfig, PersonalizedMode, SendNote } from './interfaces/message-notebook.interface';
23+
import { ImportNote, Note, NoteConfig, PersonalizedMode, SendNote } from './interfaces/message-notebook.interface';
2424
import { OP } from './interfaces/message-operator.interface';
2525
import {
2626
DynamicFormParams,
@@ -465,7 +465,7 @@ export class Message {
465465
});
466466
}
467467

468-
importNote(note: SendNote): void {
468+
importNote(note: ImportNote['note']): void {
469469
this.send<OP.IMPORT_NOTE>(OP.IMPORT_NOTE, {
470470
note: note
471471
});

zeppelin-web-angular/src/app/services/message.service.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { Observable } from 'rxjs';
1616
import { MessageInterceptor, MESSAGE_INTERCEPTOR } from '@zeppelin/interfaces';
1717
import {
1818
DynamicFormParams,
19+
ImportNote,
1920
Message,
2021
MessageReceiveDataTypeMap,
2122
MessageSendDataTypeMap,
@@ -275,7 +276,7 @@ export class MessageService extends Message implements OnDestroy {
275276
super.patchParagraph(paragraphId, noteId, patch);
276277
}
277278

278-
importNote(note: SendNote): void {
279+
importNote(note: ImportNote['note']): void {
279280
super.importNote(note);
280281
}
281282

zeppelin-web-angular/src/app/share/note-create/note-create.component.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ export class NoteCreateComponent extends MessageListenersManager implements OnIn
3838
this.cdr.markForCheck();
3939
}
4040

41-
@MessageListener(OP.NOTES_INFO)
42-
getNotes(data: MessageReceiveDataTypeMap[OP.NOTES_INFO]) {
41+
@MessageListener(OP.NEW_NOTE)
42+
newNoteCreated(_: MessageReceiveDataTypeMap[OP.NEW_NOTE]) {
4343
this.nzModalRef.destroy();
4444
}
4545

zeppelin-web-angular/src/app/share/note-import/note-import.component.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { NzModalRef } from 'ng-zorro-antd/modal';
1919
import { NzUploadFile } from 'ng-zorro-antd/upload';
2020

2121
import { MessageListener, MessageListenersManager } from '@zeppelin/core';
22-
import { MessageReceiveDataTypeMap, OP, SendNote } from '@zeppelin/sdk';
22+
import { ImportNote, MessageReceiveDataTypeMap, OP } from '@zeppelin/sdk';
2323

2424
@Component({
2525
selector: 'zeppelin-note-import',
@@ -34,8 +34,8 @@ export class NoteImportComponent extends MessageListenersManager implements OnIn
3434
importLoading = false;
3535
maxLimit = get(this.ticketService.configuration, ['zeppelin.websocket.max.text.message.size'], null);
3636

37-
@MessageListener(OP.NOTES_INFO)
38-
getNotes(data: MessageReceiveDataTypeMap[OP.NOTES_INFO]) {
37+
@MessageListener(OP.IMPORT_NOTE)
38+
noteImported(_: MessageReceiveDataTypeMap[OP.IMPORT_NOTE]) {
3939
this.nzModalRef.destroy();
4040
}
4141

@@ -92,7 +92,7 @@ export class NoteImportComponent extends MessageListenersManager implements OnIn
9292
// @ts-ignore
9393
result.name = this.noteImportName;
9494
}
95-
this.messageService.importNote(result as SendNote);
95+
this.messageService.importNote(result as ImportNote['note']);
9696
} else {
9797
this.errorText = 'Invalid JSON';
9898
}

zeppelin-web/src/components/note-import/note-import.controller.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ function NoteImportCtrl($scope, $timeout, websocketMsgSrv) {
144144
** $scope.$on functions below
145145
*/
146146

147-
$scope.$on('setNoteMenu', function(event, notes) {
147+
$scope.$on('noteImported', function(event, note) {
148148
vm.resetFlags();
149149
angular.element('#noteImportModal').modal('hide');
150150
});

zeppelin-web/src/components/websocket/websocket-event.factory.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,8 @@ function WebsocketEventFactory($rootScope, $websocket, $location, baseUrlSrv, sa
7272
$rootScope.$broadcast('setNoteContent', data.note);
7373
} else if (op === 'NEW_NOTE') {
7474
$location.path('/notebook/' + data.note.id);
75+
} else if (op === 'IMPORT_NOTE') {
76+
$rootScope.$broadcast('noteImported', data.note);
7577
} else if (op === 'NOTES_INFO') {
7678
$rootScope.$broadcast('setNoteMenu', data.notes);
7779
} else if (op === 'NOTE_RUNNING_STATUS') {

0 commit comments

Comments
 (0)