From 174dd6ed4d1581068e9bfb6d3544db8624eeb3cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20M=C3=BCller?= Date: Tue, 4 Dec 2018 15:18:41 +0100 Subject: [PATCH 1/2] mgr/dashboard: CdNotificationConfig MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This model will now be taken as single argument in the 'show' method of the notification service. Through using this model it's easy to define notifications before sending them out. Which can be helpful if you have to process an array of possible notification and the possibility that those notifications would look a like. This scenario doesn't exist in the current code base, but it's the case for new feature to show alerts from Prometheus. Fixes: https://tracker.ceph.com/issues/37469 Signed-off-by: Stephan Müller --- .../src/app/shared/models/cd-notification.ts | 23 ++++++++++------ .../services/notification.service.spec.ts | 5 ++-- .../shared/services/notification.service.ts | 27 +++++++++++++++---- 3 files changed, 40 insertions(+), 15 deletions(-) diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/models/cd-notification.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/cd-notification.ts index ba6a73a000e..575dc2d4056 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/models/cd-notification.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/cd-notification.ts @@ -1,16 +1,23 @@ +import { ToastOptions } from 'ng2-toastr'; import { NotificationType } from '../enum/notification-type.enum'; +export class CdNotificationConfig { + constructor( + public type: NotificationType, + public title: string, + public message?: string, // Use this for error notifications only + public options?: any | ToastOptions + ) {} +} + export class CdNotification { - message: string; timestamp: string; - title: string; - type: NotificationType; - - constructor(type: NotificationType = NotificationType.info, title?: string, message?: string) { - this.type = type; - this.title = title; - this.message = message; + constructor( + public type: NotificationType = NotificationType.info, + public title?: string, + public message?: string + ) { /* string representation of the Date object so it can be directly compared with the timestamps parsed from localStorage */ this.timestamp = new Date().toJSON(); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.spec.ts index f751d761794..a229c982ebe 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.spec.ts @@ -5,6 +5,7 @@ import { ToastsManager } from 'ng2-toastr'; import { configureTestBed, i18nProviders } from '../../../testing/unit-test-helper'; import { NotificationType } from '../enum/notification-type.enum'; +import { CdNotificationConfig } from '../models/cd-notification'; import { FinishedTask } from '../models/finished-task'; import { NotificationService } from './notification.service'; import { TaskMessageService } from './task-message.service'; @@ -57,7 +58,7 @@ describe('NotificationService', () => { })); it('should create a success notification and save it', fakeAsync(() => { - notificationService.show(NotificationType.success, 'Simple test'); + notificationService.show(new CdNotificationConfig(NotificationType.success, 'Simple test')); tick(100); expect(notificationService['dataSource'].getValue().length).toBe(1); expect(notificationService['dataSource'].getValue()[0].type).toBe(NotificationType.success); @@ -71,7 +72,7 @@ describe('NotificationService', () => { })); it('should create an info notification and save it', fakeAsync(() => { - notificationService.show(NotificationType.info, 'Simple test'); + notificationService.show(new CdNotificationConfig(NotificationType.info, 'Simple test')); tick(100); expect(notificationService['dataSource'].getValue().length).toBe(1); const notification = notificationService['dataSource'].getValue()[0]; diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.ts index 6c265014786..8874e272826 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.ts @@ -5,7 +5,7 @@ import { ToastsManager } from 'ng2-toastr'; import { BehaviorSubject } from 'rxjs'; import { NotificationType } from '../enum/notification-type.enum'; -import { CdNotification } from '../models/cd-notification'; +import { CdNotification, CdNotificationConfig } from '../models/cd-notification'; import { FinishedTask } from '../models/finished-task'; import { ServicesModule } from './services.module'; import { TaskMessageService } from './task-message.service'; @@ -70,10 +70,22 @@ export class NotificationService { * @param {string} [message] The message to be displayed. Note, use this field * for error notifications only. * @param {*} [options] toastr compatible options, used when creating a toastr - * @memberof NotificationService * @returns The timeout ID that is set to be able to cancel the notification. */ - show(type: NotificationType, title: string, message?: string, options?: any) { + show(type: NotificationType, title: string, message?: string, options?: any): number; + show(config: CdNotificationConfig): number; + show( + arg: NotificationType | CdNotificationConfig, + title?: string, + message?: string, + options?: any + ): number { + let type; + if (_.isObject(arg)) { + ({ message, type, title, options } = arg); + } else { + type = arg; + } return setTimeout(() => { this.save(type, title, message); if (!message) { @@ -94,15 +106,20 @@ export class NotificationService { } notifyTask(finishedTask: FinishedTask, success: boolean = true) { + let notification: CdNotificationConfig; if (finishedTask.success && success) { - this.show(NotificationType.success, this.taskMessageService.getSuccessTitle(finishedTask)); + notification = new CdNotificationConfig( + NotificationType.success, + this.taskMessageService.getSuccessTitle(finishedTask) + ); } else { - this.show( + notification = new CdNotificationConfig( NotificationType.error, this.taskMessageService.getErrorTitle(finishedTask), this.taskMessageService.getErrorMessage(finishedTask) ); } + this.show(notification); } /** From 0d3f312f57a92f9bfe1d04b916e0f5127600d52e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20M=C3=BCller?= Date: Mon, 26 Nov 2018 15:59:26 +0100 Subject: [PATCH 2/2] mgr/dashboard: Notification queue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's now possible to send an array of notification configs to the notification service. These array is cached for, by default 500ms, before this array is unified and than converted to full notifications that are seen by the user. Useful if different service can come to the same conclusions regarding the notification. This is the case for the prometheus notification service and the prometheus alert service. Fixes: https://tracker.ceph.com/issues/37469 Signed-off-by: Stephan Müller --- .../services/notification.service.spec.ts | 40 +++++++++++++++++++ .../shared/services/notification.service.ts | 21 +++++++++- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.spec.ts index a229c982ebe..b0b88df2736 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.spec.ts @@ -123,4 +123,44 @@ describe('NotificationService', () => { expect(notification.title).toBe(`Failed to create RBD 'somePool/someImage'`); expect(notification.message).toBe(`Name is already used by RBD 'somePool/someImage'.`); })); + + describe('notification queue', () => { + const n1 = new CdNotificationConfig(NotificationType.success, 'Some success'); + const n2 = new CdNotificationConfig(NotificationType.info, 'Some info'); + + beforeEach(() => { + spyOn(notificationService, 'show').and.stub(); + }); + + it('filters out duplicated notifications on single call', fakeAsync(() => { + notificationService.queueNotifications([n1, n1, n2, n2]); + tick(500); + expect(notificationService.show).toHaveBeenCalledTimes(2); + })); + + it('filters out duplicated notifications presented in different calls', fakeAsync(() => { + notificationService.queueNotifications([n1, n2]); + notificationService.queueNotifications([n1, n2]); + tick(500); + expect(notificationService.show).toHaveBeenCalledTimes(2); + })); + + it('will reset the timeout on every call', fakeAsync(() => { + notificationService.queueNotifications([n1, n2]); + tick(400); + notificationService.queueNotifications([n1, n2]); + tick(100); + expect(notificationService.show).toHaveBeenCalledTimes(0); + tick(400); + expect(notificationService.show).toHaveBeenCalledTimes(2); + })); + + it('wont filter out duplicated notifications if timeout was reached before', fakeAsync(() => { + notificationService.queueNotifications([n1, n2]); + tick(500); + notificationService.queueNotifications([n1, n2]); + tick(500); + expect(notificationService.show).toHaveBeenCalledTimes(4); + })); + }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.ts index 8874e272826..de1175b8e54 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/notification.service.ts @@ -16,10 +16,12 @@ import { TaskMessageService } from './task-message.service'; export class NotificationService { // Observable sources private dataSource = new BehaviorSubject([]); + private queuedNotifications: CdNotificationConfig[] = []; // Observable streams data$ = this.dataSource.asObservable(); + private queueTimeoutId: number; KEY = 'cdNotifications'; constructor(public toastr: ToastsManager, private taskMessageService: TaskMessageService) { @@ -63,6 +65,21 @@ export class NotificationService { localStorage.setItem(this.KEY, JSON.stringify(recent)); } + queueNotifications(notifications: CdNotificationConfig[]) { + this.queuedNotifications = this.queuedNotifications.concat(notifications); + this.cancel(this.queueTimeoutId); + this.queueTimeoutId = window.setTimeout(() => { + this.sendQueuedNotifications(); + }, 500); + } + + private sendQueuedNotifications() { + _.uniqWith(this.queuedNotifications, _.isEqual).forEach((notification) => { + this.show(notification); + }); + this.queuedNotifications = []; + } + /** * Method for showing a notification. * @param {NotificationType} type toastr type @@ -86,7 +103,7 @@ export class NotificationService { } else { type = arg; } - return setTimeout(() => { + return window.setTimeout(() => { this.save(type, title, message); if (!message) { message = ''; @@ -127,6 +144,6 @@ export class NotificationService { * @param {number} timeoutId A number representing the ID of the timeout to be canceled. */ cancel(timeoutId) { - clearTimeout(timeoutId); + window.clearTimeout(timeoutId); } }