From: Stephan Müller Date: Thu, 17 Jan 2019 11:37:12 +0000 (+0100) Subject: mgr/dashboard: Application icons in notifications X-Git-Tag: v14.1.0~218^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=454f1caecf7c4e3231a13ed8ff182b16217b4c04;p=ceph.git mgr/dashboard: Application icons in notifications Now notifications and alerts show an application icon, that gives a hint about their origin. Fixes: https://tracker.ceph.com/issues/37950 Signed-off-by: Stephan Müller --- diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notifications/notifications.component.html b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notifications/notifications.component.html index 51ecd548ef36..fe9faa54cf45 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notifications/notifications.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notifications/notifications.component.html @@ -1,21 +1,18 @@ - +
-
- Recent Notifications -
- Remove all -
-
+
- - +
- + - + @@ -23,27 +20,32 @@
- {{ notification.message }} +
- {{ notification.timestamp | cdDate }} -
- + + +
There are no notifications.
- + + + + + { it('should test search manipulation', () => { let searchTerms = []; - spyOn(component, 'subSearch').and.callFake((d, search, c) => { + spyOn(component, 'subSearch').and.callFake((d, search) => { expect(search).toEqual(searchTerms); }); const searchTest = (s: string, st: string[]) => { @@ -252,7 +252,7 @@ describe('TableComponent', () => { beforeEach(() => { component.ngOnInit(); component.data = []; - component.updating = false; + component['updating'] = false; }); it('should call fetchData callback function', () => { @@ -269,7 +269,7 @@ describe('TableComponent', () => { expect(component.loadingError).toBeTruthy(); expect(component.data.length).toBe(0); expect(component.loadingIndicator).toBeFalsy(); - expect(component.updating).toBeFalsy(); + expect(component['updating']).toBeFalsy(); }); component.reloadData(); }); @@ -283,7 +283,7 @@ describe('TableComponent', () => { expect(component.loadingError).toBeFalsy(); expect(component.data.length).toBe(10); expect(component.loadingIndicator).toBeFalsy(); - expect(component.updating).toBeFalsy(); + expect(component['updating']).toBeFalsy(); }); component.reloadData(); }); @@ -339,17 +339,15 @@ describe('TableComponent', () => { }; }); - const expectUseCustomClass = (values: any[], expectation: string) => { - values.forEach((value) => expect(component.useCustomClass(value)).toBe(expectation)); - }; - it('should throw an error if custom classes are not set', () => { component.customCss = undefined; expect(() => component.useCustomClass('active')).toThrowError('Custom classes are not set!'); }); it('should not return any class', () => { - expectUseCustomClass(['', 'something', 123, { complex: 1 }, [1, 2, 3]], undefined); + ['', 'something', 123, { complex: 1 }, [1, 2, 3]].forEach((value) => + expect(component.useCustomClass(value)).toBe(undefined) + ); }); it('should match a string and return the corresponding class', () => { diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table/table.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table/table.component.ts index 655fd2c91d73..053b1fae7196 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table/table.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table/table.component.ts @@ -199,7 +199,7 @@ export class TableComponent implements AfterContentChecked, OnInit, OnChanges, O } if (_.isInteger(this.autoReload) && this.autoReload > 0) { this.ngZone.runOutsideAngular(() => { - this.reloadSubscriber = observableTimer(0, this.autoReload).subscribe((x) => { + this.reloadSubscriber = observableTimer(0, this.autoReload).subscribe(() => { this.ngZone.run(() => { return this.reloadData(); }); @@ -329,7 +329,7 @@ export class TableComponent implements AfterContentChecked, OnInit, OnChanges, O .map((v, i) => ((_.isFunction(v) && v(value)) || v === value) && classes[i]) .filter((x) => x) .join(' '); - return (!_.isEmpty(css) && css) || undefined; + return _.isEmpty(css) ? undefined : css; } ngOnChanges(changes) { diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/models/cd-notification.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/cd-notification.spec.ts new file mode 100644 index 000000000000..e0f65b9af039 --- /dev/null +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/cd-notification.spec.ts @@ -0,0 +1,95 @@ +import { NotificationType } from '../enum/notification-type.enum'; +import { CdNotification, CdNotificationConfig } from './cd-notification'; + +describe('cd-notification classes', () => { + const expectObject = (something: object, expected: object) => { + Object.keys(expected).forEach((key) => expect(something[key]).toBe(expected[key])); + }; + + // As these Models have a view methods they need to be tested + describe('CdNotificationConfig', () => { + it('should create a new config without any parameters', () => { + expectObject(new CdNotificationConfig(), { + application: 'Ceph', + applicationClass: 'ceph-icon', + message: undefined, + options: undefined, + title: undefined, + type: 1 + }); + }); + + it('should create a new config with parameters', () => { + expectObject( + new CdNotificationConfig( + NotificationType.error, + 'Some Alert', + 'Something failed', + undefined, + 'Prometheus' + ), + { + application: 'Prometheus', + applicationClass: 'prometheus-icon', + message: 'Something failed', + options: undefined, + title: 'Some Alert', + type: 0 + } + ); + }); + }); + + describe('CdNotification', () => { + beforeEach(() => { + const baseTime = new Date('2022-02-22'); + spyOn(global, 'Date').and.returnValue(baseTime); + }); + + it('should create a new config without any parameters', () => { + expectObject(new CdNotification(), { + application: 'Ceph', + applicationClass: 'ceph-icon', + iconClass: 'fa-info', + message: undefined, + options: undefined, + textClass: 'text-info', + timestamp: '2022-02-22T00:00:00.000Z', + title: undefined, + type: 1 + }); + }); + + it('should create a new config with parameters', () => { + expectObject( + new CdNotification( + new CdNotificationConfig( + NotificationType.error, + 'Some Alert', + 'Something failed', + undefined, + 'Prometheus' + ) + ), + { + application: 'Prometheus', + applicationClass: 'prometheus-icon', + iconClass: 'fa-exclamation-triangle', + message: 'Something failed', + options: undefined, + textClass: 'text-danger', + timestamp: '2022-02-22T00:00:00.000Z', + title: 'Some Alert', + type: 0 + } + ); + }); + + it('should expect the right success classes', () => { + expectObject(new CdNotification(new CdNotificationConfig(NotificationType.success)), { + iconClass: 'fa-check', + textClass: 'text-success' + }); + }); + }); +}); 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 575dc2d4056a..cc0bebfcf6f8 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 @@ -2,46 +2,39 @@ 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 - ) {} -} + applicationClass: string; -export class CdNotification { - timestamp: string; + private classes = { + Ceph: 'ceph-icon', + Prometheus: 'prometheus-icon' + }; constructor( public type: NotificationType = NotificationType.info, public title?: string, - public message?: string + public message?: string, // Use this for additional information only + public options?: any | ToastOptions, + public application: string = 'Ceph' ) { - /* string representation of the Date object so it can be directly compared - with the timestamps parsed from localStorage */ - this.timestamp = new Date().toJSON(); + this.applicationClass = this.classes[this.application]; } +} - textClass() { - switch (this.type) { - case NotificationType.error: - return 'text-danger'; - case NotificationType.info: - return 'text-info'; - case NotificationType.success: - return 'text-success'; - } - } +export class CdNotification extends CdNotificationConfig { + timestamp: string; + textClass: string; + iconClass: string; + + private textClasses = ['text-danger', 'text-info', 'text-success']; + private iconClasses = ['fa-exclamation-triangle', 'fa-info', 'fa-check']; - iconClass() { - switch (this.type) { - case NotificationType.error: - return 'fa-exclamation-triangle'; - case NotificationType.info: - return 'fa-info'; - case NotificationType.success: - return 'fa-check'; - } + constructor(private config: CdNotificationConfig = new CdNotificationConfig()) { + super(config.type, config.title, config.message, config.options, config.application); + delete this.config; + /* string representation of the Date object so it can be directly compared + with the timestamps parsed from localStorage */ + this.timestamp = new Date().toJSON(); + this.iconClass = this.iconClasses[this.type]; + this.textClass = this.textClasses[this.type]; } } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/api-interceptor.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/api-interceptor.service.spec.ts index 7d569ad6f7de..5ae427572a33 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/api-interceptor.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/api-interceptor.service.spec.ts @@ -1,10 +1,13 @@ import { HttpClient, HttpErrorResponse } from '@angular/common/http'; import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing'; -import { TestBed } from '@angular/core/testing'; +import { fakeAsync, TestBed, tick } from '@angular/core/testing'; import { Router } from '@angular/router'; +import { ToastsManager } from 'ng2-toastr'; + import { configureTestBed, i18nProviders } from '../../../testing/unit-test-helper'; import { AppModule } from '../../app.module'; +import { CdNotification, CdNotificationConfig } from '../models/cd-notification'; import { ApiInterceptorService } from './api-interceptor.service'; import { NotificationService } from './notification.service'; @@ -15,44 +18,60 @@ describe('ApiInterceptorService', () => { let router: Router; const url = 'api/xyz'; - const runRouterTest = (errorOpts, expectedCallParams, done) => { + const httpError = (error, errorOpts, done = (resp) => {}) => { httpClient.get(url).subscribe( () => {}, (resp) => { // Error must have been forwarded by the interceptor. expect(resp instanceof HttpErrorResponse).toBeTruthy(); - done(); + done(resp); } ); - httpTesting.expectOne(url).error(new ErrorEvent('abc'), errorOpts); + httpTesting.expectOne(url).error(error, errorOpts); + }; + + const runRouterTest = (errorOpts, expectedCallParams) => { + httpError(new ErrorEvent('abc'), errorOpts); httpTesting.verify(); expect(router.navigate).toHaveBeenCalledWith(...expectedCallParams); }; - const runNotificationTest = (error, errorOpts, expectedCallParams, done) => { - httpClient.get(url).subscribe( - () => {}, - (resp) => { - // Error must have been forwarded by the interceptor. - expect(resp instanceof HttpErrorResponse).toBeTruthy(); - done(); - } - ); - httpTesting.expectOne(url).error(error, errorOpts); + const runNotificationTest = (error, errorOpts, expectedCallParams) => { + httpError(error, errorOpts); httpTesting.verify(); - expect(notificationService.show).toHaveBeenCalledWith(...expectedCallParams); + expect(notificationService.show).toHaveBeenCalled(); + expect(notificationService.save).toHaveBeenCalledWith(expectedCallParams); + }; + + const createCdNotification = (type, title?, message?, options?, application?) => { + return new CdNotification(new CdNotificationConfig(type, title, message, options, application)); }; configureTestBed({ imports: [AppModule, HttpClientTestingModule], - providers: [NotificationService, i18nProviders] + providers: [ + NotificationService, + i18nProviders, + { + provide: ToastsManager, + useValue: { + error: () => true + } + } + ] }); beforeEach(() => { + const baseTime = new Date('2022-02-22'); + spyOn(global, 'Date').and.returnValue(baseTime); + httpClient = TestBed.get(HttpClient); httpTesting = TestBed.get(HttpTestingController); + notificationService = TestBed.get(NotificationService); - spyOn(notificationService, 'show'); + spyOn(notificationService, 'show').and.callThrough(); + spyOn(notificationService, 'save'); + router = TestBed.get(Router); spyOn(router, 'navigate'); }); @@ -62,97 +81,122 @@ describe('ApiInterceptorService', () => { expect(service).toBeTruthy(); }); - it('should redirect 401', (done) => { - runRouterTest( - { - status: 401 - }, - [['/login']], - done - ); - }); + describe('test different error behaviours', () => { + beforeEach(() => { + spyOn(window, 'setTimeout').and.callFake((fn) => fn()); + }); - it('should redirect 403', (done) => { - runRouterTest( - { - status: 403 - }, - [['/403']], - done - ); - }); + it('should redirect 401', () => { + runRouterTest( + { + status: 401 + }, + [['/login']] + ); + }); - it('should show notification (error string)', function(done) { - runNotificationTest( - 'foobar', - { - status: 500, - statusText: 'Foo Bar' - }, - [0, '500 - Foo Bar', 'foobar'], - done - ); - }); + it('should redirect 403', () => { + runRouterTest( + { + status: 403 + }, + [['/403']] + ); + }); - it('should show notification (error object, triggered from backend)', function(done) { - runNotificationTest( - { detail: 'abc' }, - { - status: 504, - statusText: 'AAA bbb CCC' - }, - [0, '504 - AAA bbb CCC', 'abc'], - done - ); - }); + it('should show notification (error string)', () => { + runNotificationTest( + 'foobar', + { + status: 500, + statusText: 'Foo Bar' + }, + createCdNotification(0, '500 - Foo Bar', 'foobar') + ); + }); - it('should show notification (error object with unknown keys)', function(done) { - runNotificationTest( - { type: 'error' }, - { - status: 0, - statusText: 'Unknown Error', - message: 'Http failure response for (unknown url): 0 Unknown Error', - name: 'HttpErrorResponse', - ok: false, - url: null - }, - [0, '0 - Unknown Error', 'Http failure response for api/xyz: 0 Unknown Error'], - done - ); - }); + it('should show notification (error object, triggered from backend)', () => { + runNotificationTest( + { detail: 'abc' }, + { + status: 504, + statusText: 'AAA bbb CCC' + }, + createCdNotification(0, '504 - AAA bbb CCC', 'abc') + ); + }); - it('should show notification (undefined error)', function(done) { - runNotificationTest( - undefined, - { - status: 502 - }, - [0, '502 - Unknown Error', 'Http failure response for api/xyz: 502 '], - done - ); - }); + it('should show notification (error object with unknown keys)', () => { + runNotificationTest( + { type: 'error' }, + { + status: 0, + statusText: 'Unknown Error', + message: 'Http failure response for (unknown url): 0 Unknown Error', + name: 'HttpErrorResponse', + ok: false, + url: null + }, + createCdNotification( + 0, + '0 - Unknown Error', + 'Http failure response for api/xyz: 0 Unknown Error' + ) + ); + }); - it('should show 400 notification', function(done) { - spyOn(notificationService, 'notifyTask'); - httpClient.get(url).subscribe( - () => {}, - (resp) => { - // Error must have been forwarded by the interceptor. - expect(resp instanceof HttpErrorResponse).toBeTruthy(); - done(); - } - ); - httpTesting - .expectOne(url) - .error({ task: { name: 'mytask', metadata: { component: 'foobar' } } }, { status: 400 }); - httpTesting.verify(); - expect(notificationService.show).toHaveBeenCalledTimes(0); - expect(notificationService.notifyTask).toHaveBeenCalledWith({ - exception: { task: { metadata: { component: 'foobar' }, name: 'mytask' } }, - metadata: { component: 'foobar' }, - name: 'mytask', - success: false + it('should show notification (undefined error)', () => { + runNotificationTest( + undefined, + { + status: 502 + }, + createCdNotification(0, '502 - Unknown Error', 'Http failure response for api/xyz: 502 ') + ); }); + + it('should show 400 notification', () => { + spyOn(notificationService, 'notifyTask'); + httpError({ task: { name: 'mytask', metadata: { component: 'foobar' } } }, { status: 400 }); + httpTesting.verify(); + expect(notificationService.show).toHaveBeenCalledTimes(0); + expect(notificationService.notifyTask).toHaveBeenCalledWith({ + exception: { task: { metadata: { component: 'foobar' }, name: 'mytask' } }, + metadata: { component: 'foobar' }, + name: 'mytask', + success: false + }); + }); + }); + + describe('interceptor error handling', () => { + it('should show default behaviour', fakeAsync(() => { + httpError(undefined, { status: 500 }); + tick(10); + expect(notificationService.save).toHaveBeenCalled(); + })); + + it('should prevent the default behaviour with preventDefault', fakeAsync(() => { + httpError(undefined, { status: 500 }, (resp) => resp.preventDefault()); + tick(10); + expect(notificationService.save).not.toHaveBeenCalled(); + })); + + it('should prevent the default behaviour by status code', fakeAsync(() => { + httpError(undefined, { status: 500 }, (resp) => resp.ignoreStatusCode(500)); + tick(10); + expect(notificationService.save).not.toHaveBeenCalled(); + })); + + it('should use different application icon (default Ceph) in error message', fakeAsync(() => { + const msg = 'Cannot connect to Alertmanager'; + httpError(undefined, { status: 500 }, (resp) => { + (resp.application = 'Prometheus'), (resp.message = msg); + }); + tick(10); + expect(notificationService.save).toHaveBeenCalledWith( + createCdNotification(0, '500 - Unknown Error', msg, undefined, 'Prometheus') + ); + })); }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/api-interceptor.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/api-interceptor.service.ts index 1a80b581b7df..27d41be95d20 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/api-interceptor.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/api-interceptor.service.ts @@ -13,6 +13,7 @@ import { Observable, throwError as observableThrowError } from 'rxjs'; import { catchError } from 'rxjs/operators'; import { NotificationType } from '../enum/notification-type.enum'; +import { CdNotificationConfig } from '../models/cd-notification'; import { FinishedTask } from '../models/finished-task'; import { AuthStorageService } from './auth-storage.service'; import { NotificationService } from './notification.service'; @@ -62,22 +63,7 @@ export class ApiInterceptorService implements HttpInterceptor { break; } - let timeoutId; - if (showNotification) { - let message = ''; - if (_.isPlainObject(resp.error) && _.isString(resp.error.detail)) { - message = resp.error.detail; // Error was triggered by the backend. - } else if (_.isString(resp.error)) { - message = resp.error; - } else if (_.isString(resp.message)) { - message = resp.message; - } - timeoutId = this.notificationService.show( - NotificationType.error, - `${resp.status} - ${resp.statusText}`, - message - ); - } + const timeoutId = showNotification ? this.prepareNotification(resp) : undefined; /** * Decorated preventDefault method (in case error previously had @@ -103,4 +89,24 @@ export class ApiInterceptorService implements HttpInterceptor { }) ); } + + private prepareNotification(resp): number { + return this.notificationService.show(() => { + let message = ''; + if (_.isPlainObject(resp.error) && _.isString(resp.error.detail)) { + message = resp.error.detail; // Error was triggered by the backend. + } else if (_.isString(resp.error)) { + message = resp.error; + } else if (_.isString(resp.message)) { + message = resp.message; + } + return new CdNotificationConfig( + NotificationType.error, + `${resp.status} - ${resp.statusText}`, + message, + undefined, + resp['application'] + ); + }); + } } 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 b0b88df27368..c0413f4404b2 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 @@ -1,3 +1,4 @@ +import { DatePipe } from '@angular/common'; import { fakeAsync, TestBed, tick } from '@angular/core/testing'; import * as _ from 'lodash'; @@ -7,11 +8,12 @@ import { configureTestBed, i18nProviders } from '../../../testing/unit-test-help import { NotificationType } from '../enum/notification-type.enum'; import { CdNotificationConfig } from '../models/cd-notification'; import { FinishedTask } from '../models/finished-task'; +import { CdDatePipe } from '../pipes/cd-date.pipe'; import { NotificationService } from './notification.service'; import { TaskMessageService } from './task-message.service'; describe('NotificationService', () => { - let notificationService: NotificationService; + let service: NotificationService; const toastFakeService = { error: () => true, info: () => true, @@ -20,25 +22,28 @@ describe('NotificationService', () => { configureTestBed({ providers: [ + CdDatePipe, + DatePipe, NotificationService, TaskMessageService, { provide: ToastsManager, useValue: toastFakeService }, + { provide: CdDatePipe, useValue: { transform: (d) => d } }, i18nProviders ] }); beforeEach(() => { - notificationService = TestBed.get(NotificationService); - notificationService.removeAll(); + service = TestBed.get(NotificationService); + service.removeAll(); }); it('should be created', () => { - expect(notificationService).toBeTruthy(); + expect(service).toBeTruthy(); }); it('should read empty notification list', () => { localStorage.setItem('cdNotifications', '[]'); - expect(notificationService['dataSource'].getValue()).toEqual([]); + expect(service['dataSource'].getValue()).toEqual([]); }); it('should read old notifications', fakeAsync(() => { @@ -46,36 +51,36 @@ describe('NotificationService', () => { 'cdNotifications', '[{"type":2,"message":"foobar","timestamp":"2018-05-24T09:41:32.726Z"}]' ); - const service = new NotificationService(null, null); + service = new NotificationService(null, null, null); expect(service['dataSource'].getValue().length).toBe(1); })); it('should cancel a notification', fakeAsync(() => { - const timeoutId = notificationService.show(NotificationType.error, 'Simple test'); - notificationService.cancel(timeoutId); + const timeoutId = service.show(NotificationType.error, 'Simple test'); + service.cancel(timeoutId); tick(5000); - expect(notificationService['dataSource'].getValue().length).toBe(0); + expect(service['dataSource'].getValue().length).toBe(0); })); it('should create a success notification and save it', fakeAsync(() => { - notificationService.show(new CdNotificationConfig(NotificationType.success, 'Simple test')); + service.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); + expect(service['dataSource'].getValue().length).toBe(1); + expect(service['dataSource'].getValue()[0].type).toBe(NotificationType.success); })); it('should create an error notification and save it', fakeAsync(() => { - notificationService.show(NotificationType.error, 'Simple test'); + service.show(NotificationType.error, 'Simple test'); tick(100); - expect(notificationService['dataSource'].getValue().length).toBe(1); - expect(notificationService['dataSource'].getValue()[0].type).toBe(NotificationType.error); + expect(service['dataSource'].getValue().length).toBe(1); + expect(service['dataSource'].getValue()[0].type).toBe(NotificationType.error); })); it('should create an info notification and save it', fakeAsync(() => { - notificationService.show(new CdNotificationConfig(NotificationType.info, 'Simple test')); + service.show(new CdNotificationConfig(NotificationType.info, 'Simple test')); tick(100); - expect(notificationService['dataSource'].getValue().length).toBe(1); - const notification = notificationService['dataSource'].getValue()[0]; + expect(service['dataSource'].getValue().length).toBe(1); + const notification = service['dataSource'].getValue()[0]; expect(notification.type).toBe(NotificationType.info); expect(notification.title).toBe('Simple test'); expect(notification.message).toBe(undefined); @@ -83,20 +88,20 @@ describe('NotificationService', () => { it('should never have more then 10 notifications', fakeAsync(() => { for (let index = 0; index < 15; index++) { - notificationService.show(NotificationType.info, 'Simple test'); + service.show(NotificationType.info, 'Simple test'); tick(100); } - expect(notificationService['dataSource'].getValue().length).toBe(10); + expect(service['dataSource'].getValue().length).toBe(10); })); it('should show a success task notification', fakeAsync(() => { const task = _.assign(new FinishedTask(), { success: true }); - notificationService.notifyTask(task, true); + service.notifyTask(task, true); tick(100); - expect(notificationService['dataSource'].getValue().length).toBe(1); - const notification = notificationService['dataSource'].getValue()[0]; + expect(service['dataSource'].getValue().length).toBe(1); + const notification = service['dataSource'].getValue()[0]; expect(notification.type).toBe(NotificationType.success); expect(notification.title).toBe('Executed unknown task'); expect(notification.message).toBe(undefined); @@ -115,10 +120,10 @@ describe('NotificationService', () => { } } ); - notificationService.notifyTask(task); + service.notifyTask(task); tick(100); - expect(notificationService['dataSource'].getValue().length).toBe(1); - const notification = notificationService['dataSource'].getValue()[0]; + expect(service['dataSource'].getValue().length).toBe(1); + const notification = service['dataSource'].getValue()[0]; expect(notification.type).toBe(NotificationType.error); expect(notification.title).toBe(`Failed to create RBD 'somePool/someImage'`); expect(notification.message).toBe(`Name is already used by RBD 'somePool/someImage'.`); @@ -129,38 +134,98 @@ describe('NotificationService', () => { const n2 = new CdNotificationConfig(NotificationType.info, 'Some info'); beforeEach(() => { - spyOn(notificationService, 'show').and.stub(); + spyOn(service, 'show').and.stub(); }); it('filters out duplicated notifications on single call', fakeAsync(() => { - notificationService.queueNotifications([n1, n1, n2, n2]); + service.queueNotifications([n1, n1, n2, n2]); tick(500); - expect(notificationService.show).toHaveBeenCalledTimes(2); + expect(service.show).toHaveBeenCalledTimes(2); })); it('filters out duplicated notifications presented in different calls', fakeAsync(() => { - notificationService.queueNotifications([n1, n2]); - notificationService.queueNotifications([n1, n2]); + service.queueNotifications([n1, n2]); + service.queueNotifications([n1, n2]); tick(500); - expect(notificationService.show).toHaveBeenCalledTimes(2); + expect(service.show).toHaveBeenCalledTimes(2); })); it('will reset the timeout on every call', fakeAsync(() => { - notificationService.queueNotifications([n1, n2]); + service.queueNotifications([n1, n2]); tick(400); - notificationService.queueNotifications([n1, n2]); + service.queueNotifications([n1, n2]); tick(100); - expect(notificationService.show).toHaveBeenCalledTimes(0); + expect(service.show).toHaveBeenCalledTimes(0); tick(400); - expect(notificationService.show).toHaveBeenCalledTimes(2); + expect(service.show).toHaveBeenCalledTimes(2); })); it('wont filter out duplicated notifications if timeout was reached before', fakeAsync(() => { - notificationService.queueNotifications([n1, n2]); + service.queueNotifications([n1, n2]); tick(500); - notificationService.queueNotifications([n1, n2]); + service.queueNotifications([n1, n2]); tick(500); - expect(notificationService.show).toHaveBeenCalledTimes(4); + expect(service.show).toHaveBeenCalledTimes(4); })); }); + + describe('showToasty', () => { + let toastr: ToastsManager; + const time = '2022-02-22T00:00:00.000Z'; + + beforeEach(() => { + const baseTime = new Date(time); + spyOn(global, 'Date').and.returnValue(baseTime); + spyOn(window, 'setTimeout').and.callFake((fn) => fn()); + + toastr = TestBed.get(ToastsManager); + // spyOn needs to know the methods before spying and can't read the array for clarification + ['error', 'info', 'success'].forEach((method: 'error' | 'info' | 'success') => + spyOn(toastr, method).and.stub() + ); + }); + + it('should show with only title defined', () => { + service.show(NotificationType.info, 'Some info'); + expect(toastr.info).toHaveBeenCalledWith( + `${time}` + + '', + 'Some info', + undefined + ); + }); + + it('should show with title and message defined', () => { + service.show( + () => + new CdNotificationConfig(NotificationType.error, 'Some error', 'Some operation failed') + ); + expect(toastr.error).toHaveBeenCalledWith( + 'Some operation failed
' + + `${time}` + + '', + 'Some error', + undefined + ); + }); + + it('should show with title, message and application defined', () => { + service.show( + new CdNotificationConfig( + NotificationType.success, + 'Alert resolved', + 'Some alert resolved', + undefined, + 'Prometheus' + ) + ); + expect(toastr.success).toHaveBeenCalledWith( + 'Some alert resolved
' + + `${time}` + + '', + 'Alert resolved', + undefined + ); + }); + }); }); 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 de1175b8e54d..18738f69350c 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 @@ -1,12 +1,13 @@ import { Injectable } from '@angular/core'; import * as _ from 'lodash'; -import { ToastsManager } from 'ng2-toastr'; +import { ToastOptions, ToastsManager } from 'ng2-toastr'; import { BehaviorSubject } from 'rxjs'; import { NotificationType } from '../enum/notification-type.enum'; import { CdNotification, CdNotificationConfig } from '../models/cd-notification'; import { FinishedTask } from '../models/finished-task'; +import { CdDatePipe } from '../pipes/cd-date.pipe'; import { ServicesModule } from './services.module'; import { TaskMessageService } from './task-message.service'; @@ -24,7 +25,11 @@ export class NotificationService { private queueTimeoutId: number; KEY = 'cdNotifications'; - constructor(public toastr: ToastsManager, private taskMessageService: TaskMessageService) { + constructor( + public toastr: ToastsManager, + private taskMessageService: TaskMessageService, + private cdDatePipe: CdDatePipe + ) { const stringNotifications = localStorage.getItem(this.KEY); let notifications: CdNotification[] = []; @@ -50,17 +55,13 @@ export class NotificationService { /** * Method used for saving a shown notification (check show() method). - * @param {Notification} notification */ - save(type: NotificationType, title: string, message?: string) { - const notification = new CdNotification(type, title, message); - + save(notification: CdNotification) { const recent = this.dataSource.getValue(); recent.push(notification); while (recent.length > 10) { recent.shift(); } - this.dataSource.next(recent); localStorage.setItem(this.KEY, JSON.stringify(recent)); } @@ -87,41 +88,62 @@ 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 + * @param {string} [application] Only needed if notification comes from an external application * @returns The timeout ID that is set to be able to cancel the notification. */ - show(type: NotificationType, title: string, message?: string, options?: any): number; - show(config: CdNotificationConfig): number; show( - arg: NotificationType | CdNotificationConfig, + type: NotificationType, + title: string, + message?: string, + options?: any | ToastOptions, + application?: string + ): number; + show(config: CdNotificationConfig | (() => CdNotificationConfig)): number; + show( + arg: NotificationType | CdNotificationConfig | (() => CdNotificationConfig), title?: string, message?: string, - options?: any + options?: any | ToastOptions, + application?: string ): number { - let type; - if (_.isObject(arg)) { - ({ message, type, title, options } = arg); - } else { - type = arg; - } return window.setTimeout(() => { - this.save(type, title, message); - if (!message) { - message = ''; - } - switch (type) { - case NotificationType.error: - this.toastr.error(message, title, options); - break; - case NotificationType.info: - this.toastr.info(message, title, options); - break; - case NotificationType.success: - this.toastr.success(message, title, options); - break; + let config: CdNotificationConfig; + if (_.isFunction(arg)) { + config = arg() as CdNotificationConfig; + } else if (_.isObject(arg)) { + config = arg as CdNotificationConfig; + } else { + config = new CdNotificationConfig( + arg as NotificationType, + title, + message, + options, + application + ); } + const notification = new CdNotification(config); + this.save(notification); + this.showToasty(notification); }, 10); } + private showToasty(notification: CdNotification) { + this.toastr[['error', 'info', 'success'][notification.type]]( + (notification.message ? notification.message + '
' : '') + + this.renderTimeAndApplicationHtml(notification), + notification.title, + notification.options + ); + } + + renderTimeAndApplicationHtml(notification: CdNotification): string { + return `${this.cdDatePipe.transform( + notification.timestamp + )}`; + } + notifyTask(finishedTask: FinishedTask, success: boolean = true) { let notification: CdNotificationConfig; if (finishedTask.success && success) { diff --git a/src/pybind/mgr/dashboard/frontend/src/assets/Ceph_Logo_Stacked_RGB_120411_fa_228x228.png b/src/pybind/mgr/dashboard/frontend/src/assets/Ceph_Logo_Stacked_RGB_120411_fa_228x228.png new file mode 100644 index 000000000000..79064dc05864 Binary files /dev/null and b/src/pybind/mgr/dashboard/frontend/src/assets/Ceph_Logo_Stacked_RGB_120411_fa_228x228.png differ diff --git a/src/pybind/mgr/dashboard/frontend/src/assets/prometheus_logo.png b/src/pybind/mgr/dashboard/frontend/src/assets/prometheus_logo.png new file mode 100644 index 000000000000..2ef61724233b Binary files /dev/null and b/src/pybind/mgr/dashboard/frontend/src/assets/prometheus_logo.png differ diff --git a/src/pybind/mgr/dashboard/frontend/src/styles.scss b/src/pybind/mgr/dashboard/frontend/src/styles.scss index 6e3f1bdab089..35bed38858d4 100644 --- a/src/pybind/mgr/dashboard/frontend/src/styles.scss +++ b/src/pybind/mgr/dashboard/frontend/src/styles.scss @@ -339,3 +339,17 @@ uib-accordion .panel-title, .nav-tabs { margin-bottom: 15px; } +/* Icons */ +.ceph-icon { + background: url('assets/Ceph_Logo_Stacked_RGB_120411_fa_228x228.png'); +} +.prometheus-icon { + background: url('assets/prometheus_logo.png'); +} +.custom-icon { + padding: 10px; + margin-right: 1em; + background-clip: padding-box; + background-size: contain; + background-repeat: no-repeat; +}