From 0ab2a4e266eab4c2788c86f8bc7e571a4beb611d Mon Sep 17 00:00:00 2001 From: Anikait Sehwag Date: Wed, 25 Jun 2025 12:18:44 +0530 Subject: [PATCH] mgr/dashboard: Carbonised Toast Notification Used carbon toast component to carbonise toast notifications Dashboard: Toast Notification carbonised This PR replaces the existing ngx-toastr implementation with Carbon Design System toast notifications to maintain UI consistency across the Ceph dashboard application. Fixes:https://tracker.ceph.com/issues/71735 Signed-off-by: Anikait Sehwag --- .../dashboard/frontend/src/app/app.module.ts | 7 -- .../workbench-layout.component.html | 2 + .../shared/components/components.module.ts | 7 +- .../copy2clipboard-button.component.spec.ts | 59 ++++++---- .../copy2clipboard-button.component.ts | 36 ++++-- .../notification-toast.component.html | 16 +++ .../notification-toast.component.scss | 77 +++++++++++++ .../notification-toast.component.spec.ts | 71 ++++++++++++ .../notification-toast.component.ts | 44 +++++++ .../app/shared/enum/notification-type.enum.ts | 3 +- .../app/shared/models/cd-notification.spec.ts | 38 +++++- .../src/app/shared/models/cd-notification.ts | 18 ++- .../app/shared/models/prometheus-alerts.ts | 1 + .../services/api-interceptor.service.spec.ts | 11 +- .../services/notification.service.spec.ts | 108 +++++++++--------- .../shared/services/notification.service.ts | 94 ++++++++++----- .../prometheus-alert-formatter.spec.ts | 30 ++++- .../services/prometheus-alert-formatter.ts | 11 +- .../prometheus-notification.service.spec.ts | 13 ++- 19 files changed, 508 insertions(+), 138 deletions(-) create mode 100644 src/pybind/mgr/dashboard/frontend/src/app/shared/components/notification-toast/notification-toast.component.html create mode 100644 src/pybind/mgr/dashboard/frontend/src/app/shared/components/notification-toast/notification-toast.component.scss create mode 100644 src/pybind/mgr/dashboard/frontend/src/app/shared/components/notification-toast/notification-toast.component.spec.ts create mode 100644 src/pybind/mgr/dashboard/frontend/src/app/shared/components/notification-toast/notification-toast.component.ts diff --git a/src/pybind/mgr/dashboard/frontend/src/app/app.module.ts b/src/pybind/mgr/dashboard/frontend/src/app/app.module.ts index 6b7134bd213e..a1e7e3a8e169 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/app.module.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/app.module.ts @@ -4,8 +4,6 @@ import { ErrorHandler, NgModule } from '@angular/core'; import { BrowserModule } from '@angular/platform-browser'; import { BrowserAnimationsModule } from '@angular/platform-browser/animations'; -import { ToastrModule } from 'ngx-toastr'; - import { AppRoutingModule } from './app-routing.module'; import { AppComponent } from './app.component'; import { CephModule } from './ceph/ceph.module'; @@ -21,11 +19,6 @@ import { SharedModule } from './shared/shared.module'; imports: [ BrowserModule, BrowserAnimationsModule, - ToastrModule.forRoot({ - positionClass: 'toast-top-right', - preventDuplicates: true, - enableHtml: true - }), AppRoutingModule, CoreModule, SharedModule, diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/layouts/workbench-layout/workbench-layout.component.html b/src/pybind/mgr/dashboard/frontend/src/app/core/layouts/workbench-layout/workbench-layout.component.html index d90b82df449a..c8a42e6522c1 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/layouts/workbench-layout/workbench-layout.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/layouts/workbench-layout/workbench-layout.component.html @@ -17,4 +17,6 @@ + + diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/components/components.module.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/components/components.module.ts index 459ee01f2d31..022b086e0890 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/components/components.module.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/components/components.module.ts @@ -90,6 +90,7 @@ import { ChartsModule } from '@carbon/charts-angular'; import { InlineMessageComponent } from './inline-message/inline-message.component'; import { IconComponent } from './icon/icon.component'; import { DetailsCardComponent } from './details-card/details-card.component'; +import { ToastComponent } from './notification-toast/notification-toast.component'; // Icons import InfoIcon from '@carbon/icons/es/information/16'; @@ -186,7 +187,8 @@ import CloseIcon from '@carbon/icons/es/close/16'; SidePanelComponent, IconComponent, InlineMessageComponent, - DetailsCardComponent + DetailsCardComponent, + ToastComponent ], providers: [provideCharts(withDefaultRegisterables())], exports: [ @@ -228,7 +230,8 @@ import CloseIcon from '@carbon/icons/es/close/16'; SidePanelComponent, IconComponent, InlineMessageComponent, - DetailsCardComponent + DetailsCardComponent, + ToastComponent ] }) export class ComponentsModule { diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/components/copy2clipboard-button/copy2clipboard-button.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/components/copy2clipboard-button/copy2clipboard-button.component.spec.ts index 2842793c67ba..ccd3c2f74014 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/components/copy2clipboard-button/copy2clipboard-button.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/components/copy2clipboard-button/copy2clipboard-button.component.spec.ts @@ -1,7 +1,8 @@ import { TestBed } from '@angular/core/testing'; import * as BrowserDetect from 'detect-browser'; -import { ToastrService } from 'ngx-toastr'; +import { NotificationService } from '~/app/shared/services/notification.service'; +import { NotificationType } from '~/app/shared/enum/notification-type.enum'; import { configureTestBed } from '~/testing/unit-test-helper'; import { Copy2ClipboardButtonComponent } from './copy2clipboard-button.component'; @@ -12,10 +13,9 @@ describe('Copy2ClipboardButtonComponent', () => { configureTestBed({ providers: [ { - provide: ToastrService, + provide: NotificationService, useValue: { - error: () => true, - success: () => true + show: jest.fn() } } ] @@ -27,39 +27,58 @@ describe('Copy2ClipboardButtonComponent', () => { }); describe('test onClick behaviours', () => { - let toastrService: ToastrService; - let queryFn: jasmine.Spy; - let writeTextFn: jasmine.Spy; + let notificationService: NotificationService; + let queryFn: jest.SpyInstance; + let writeTextFn: jest.SpyInstance; beforeEach(() => { - toastrService = TestBed.inject(ToastrService); - component = new Copy2ClipboardButtonComponent(toastrService); - spyOn(component, 'getText').and.returnValue('foo'); + notificationService = TestBed.inject(NotificationService); + component = new Copy2ClipboardButtonComponent(notificationService); + jest.spyOn(component as any, 'getText').mockReturnValue('foo'); Object.assign(navigator, { permissions: { query: jest.fn() }, clipboard: { writeText: jest.fn() } }); - queryFn = spyOn(navigator.permissions, 'query'); + queryFn = jest.spyOn(navigator.permissions, 'query'); }); - it('should not call permissions API', () => { - spyOn(BrowserDetect, 'detect').and.returnValue({ name: 'firefox' }); - writeTextFn = spyOn(navigator.clipboard, 'writeText').and.returnValue( - new Promise((resolve, _) => { - resolve(); - }) - ); - component.onClick(); + it('should not call permissions API', async () => { + jest + .spyOn(BrowserDetect, 'detect') + .mockReturnValue({ name: 'firefox', version: '120.0.0', os: 'Linux', type: 'browser' }); + writeTextFn = jest.spyOn(navigator.clipboard, 'writeText').mockResolvedValue(undefined); + + await component.onClick(); expect(queryFn).not.toHaveBeenCalled(); expect(writeTextFn).toHaveBeenCalledWith('foo'); + expect(notificationService.show).toHaveBeenCalled(); }); it('should call permissions API', () => { - spyOn(BrowserDetect, 'detect').and.returnValue({ name: 'chrome' }); + jest + .spyOn(BrowserDetect, 'detect') + .mockReturnValue({ name: 'chrome', version: '120.0.0', os: 'Linux', type: 'browser' }); + jest.spyOn(navigator.permissions, 'query').mockResolvedValue({ state: 'granted' } as any); + jest.spyOn(navigator.clipboard, 'writeText').mockResolvedValue(undefined); + component.onClick(); expect(queryFn).toHaveBeenCalled(); }); + + it('should show error notification when clipboard fails', async () => { + jest.spyOn(BrowserDetect, 'detect').mockReturnValue({ name: 'firefox' } as any); + jest.spyOn(navigator.clipboard, 'writeText').mockRejectedValue(new Error('Failed')); + + await component.onClick(); + await Promise.resolve(); + const calls = (notificationService.show as jest.Mock).mock.calls; + expect(calls).toContainEqual([ + NotificationType.error, + 'Error', + 'Failed to copy text to the clipboard.' + ]); + }); }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/components/copy2clipboard-button/copy2clipboard-button.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/components/copy2clipboard-button/copy2clipboard-button.component.ts index 2b82b76f5a51..a17d9e9de6cf 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/components/copy2clipboard-button/copy2clipboard-button.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/components/copy2clipboard-button/copy2clipboard-button.component.ts @@ -1,9 +1,15 @@ import { Component, HostListener, Input } from '@angular/core'; import { detect } from 'detect-browser'; -import { ToastrService } from 'ngx-toastr'; import { Icons } from '~/app/shared/enum/icons.enum'; +import { NotificationType } from '~/app/shared/enum/notification-type.enum'; +import { NotificationService } from '~/app/shared/services/notification.service'; + +const ERROR_TITLE = $localize`Error`; +const CLIPBOARD_ERROR_MESSAGE = $localize`Failed to copy text to the clipboard.`; +const SUCCESS_TITLE = $localize`Success`; +const CLIPBOARD_SUCCESS_MESSAGE = $localize`Copied text to the clipboard successfully.`; @Component({ selector: 'cd-copy-2-clipboard-button', @@ -29,7 +35,7 @@ export class Copy2ClipboardButtonComponent { icons = Icons; - constructor(private toastr: ToastrService) {} + constructor(private notificationService: NotificationService) {} private getText(): string { const element = document.getElementById(this.source) as HTMLInputElement; @@ -41,25 +47,39 @@ export class Copy2ClipboardButtonComponent { try { const browser = detect(); const text = this.byId ? this.getText() : this.source; - const toastrFn = () => { - this.toastr.success('Copied text to the clipboard successfully.'); + const showSuccess = () => { + this.notificationService.show( + NotificationType.success, + SUCCESS_TITLE, + CLIPBOARD_SUCCESS_MESSAGE + ); + }; + const showError = () => { + this.notificationService.show(NotificationType.error, ERROR_TITLE, CLIPBOARD_ERROR_MESSAGE); }; if (['firefox', 'ie', 'ios', 'safari'].includes(browser.name)) { // Various browsers do not support the `Permissions API`. // https://developer.mozilla.org/en-US/docs/Web/API/Permissions_API#Browser_compatibility - navigator.clipboard.writeText(text).then(() => toastrFn()); + navigator.clipboard + .writeText(text) + .then(() => showSuccess()) + .catch(() => showError()); } else { // Checking if we have the clipboard-write permission navigator.permissions .query({ name: 'clipboard-write' as PermissionName }) .then((result: any) => { if (result.state === 'granted' || result.state === 'prompt') { - navigator.clipboard.writeText(text).then(() => toastrFn()); + navigator.clipboard + .writeText(text) + .then(() => showSuccess()) + .catch(() => showError()); } - }); + }) + .catch(() => showError()); } } catch (_) { - this.toastr.error('Failed to copy text to the clipboard.'); + this.notificationService.show(NotificationType.error, ERROR_TITLE, CLIPBOARD_ERROR_MESSAGE); } } } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/components/notification-toast/notification-toast.component.html b/src/pybind/mgr/dashboard/frontend/src/app/shared/components/notification-toast/notification-toast.component.html new file mode 100644 index 000000000000..205972ad1c6e --- /dev/null +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/components/notification-toast/notification-toast.component.html @@ -0,0 +1,16 @@ +
+ @for (toast of activeToasts$ | async; track $index) { + + + } +
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/components/notification-toast/notification-toast.component.scss b/src/pybind/mgr/dashboard/frontend/src/app/shared/components/notification-toast/notification-toast.component.scss new file mode 100644 index 000000000000..80c66379ef95 --- /dev/null +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/components/notification-toast/notification-toast.component.scss @@ -0,0 +1,77 @@ +@use '@carbon/styles/scss/theme' as *; +@use '@carbon/styles/scss/spacing' as *; +@use '@carbon/styles/scss/layer' as *; +@use '@carbon/styles/scss/type' as *; + +.cds--toast-notification-container { + position: fixed; + top: $spacing-10; + right: $spacing-04; + max-width: $spacing-13 * 8; + z-index: 9000; + pointer-events: none; + display: flex; + flex-direction: column; + align-items: flex-end; + + cds-toast { + pointer-events: all; + margin-bottom: $spacing-03; + transform-origin: top right; + + ::ng-deep { + .cds--toast-notification__title, + .cds--toast-notification__subtitle, + .cds--toast-notification__caption { + color: $text-primary; + } + + .cds--toast-notification__close-button { + color: $icon-primary; + } + + .cds--toast-notification__close-button:hover { + background-color: transparent; + } + + .cds--toast-notification__close-icon { + fill: currentcolor; + } + + .toast-caption-container { + display: flex; + justify-content: flex-start; + align-items: center; + width: 100%; + } + + .toast-caption-container .date { + flex-shrink: 0; + } + } + } +} + +@keyframes toast-slide-in { + from { + opacity: 0; + transform: translateX(100%); + } + + to { + opacity: 1; + transform: translateX(0); + } +} + +@keyframes toast-slide-out { + from { + opacity: 1; + transform: translateX(0); + } + + to { + opacity: 0; + transform: translateX(100%); + } +} diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/components/notification-toast/notification-toast.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/components/notification-toast/notification-toast.component.spec.ts new file mode 100644 index 000000000000..9c92162c0559 --- /dev/null +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/components/notification-toast/notification-toast.component.spec.ts @@ -0,0 +1,71 @@ +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { NoopAnimationsModule } from '@angular/platform-browser/animations'; +import { of } from 'rxjs'; +import { ToastContent } from 'carbon-components-angular'; + +import { ToastComponent } from './notification-toast.component'; +import { NotificationService } from '../../services/notification.service'; +import { configureTestBed } from '~/testing/unit-test-helper'; + +describe('ToastComponent', () => { + let component: ToastComponent; + let fixture: ComponentFixture; + let mockToasts: ToastContent[]; + + const mockNotificationService = { + activeToasts$: of([]), + removeToast: jest.fn() + }; + + configureTestBed({ + declarations: [ToastComponent], + imports: [NoopAnimationsModule], + providers: [ + { + provide: NotificationService, + useValue: mockNotificationService + } + ] + }); + + beforeEach(() => { + fixture = TestBed.createComponent(ToastComponent); + component = fixture.componentInstance; + + mockToasts = [ + { + type: 'success', + title: 'Test Title', + subtitle: 'Test Message', + caption: 'Test Caption', + lowContrast: false, + showClose: true + } + ]; + }); + + it('should create', () => { + fixture.detectChanges(); + expect(component).toBeTruthy(); + }); + + it('should initialize with activeToasts$ Observable', () => { + fixture.detectChanges(); + expect(component.activeToasts$).toBeDefined(); + }); + + it('should update activeToasts$ when notification service emits new toasts', () => { + mockNotificationService.activeToasts$ = of(mockToasts); + fixture.detectChanges(); + + component.activeToasts$.subscribe((toasts) => { + expect(toasts).toEqual(mockToasts); + }); + }); + + it('should call removeToast when onToastClose is called', () => { + const toast = mockToasts[0]; + component.onToastClose(toast); + expect(mockNotificationService.removeToast).toHaveBeenCalledWith(toast); + }); +}); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/components/notification-toast/notification-toast.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/components/notification-toast/notification-toast.component.ts new file mode 100644 index 000000000000..ca72dd22e95a --- /dev/null +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/components/notification-toast/notification-toast.component.ts @@ -0,0 +1,44 @@ +import { Component, OnInit } from '@angular/core'; +import { animate, style, transition, trigger } from '@angular/animations'; +import { Observable } from 'rxjs'; +import { ToastContent } from 'carbon-components-angular'; +import { NotificationService } from '../../services/notification.service'; + +@Component({ + selector: 'cd-toast', + templateUrl: './notification-toast.component.html', + styleUrls: ['./notification-toast.component.scss'], + animations: [ + trigger('toastAnimation', [ + transition( + ':enter', + [ + style({ opacity: 0, transform: 'translateX(100%)' }), + animate('{{duration}} {{easing}}', style({ opacity: 1, transform: 'translateX(0)' })) + ], + { params: { duration: '240ms', easing: 'cubic-bezier(0.2, 0, 0.38, 0.9)' } } + ), + transition( + ':leave', + [ + style({ opacity: 1, transform: 'translateX(0)' }), + animate('{{duration}} {{easing}}', style({ opacity: 0, transform: 'translateX(100%)' })) + ], + { params: { duration: '240ms', easing: 'cubic-bezier(0.2, 0, 0.38, 0.9)' } } + ) + ]) + ] +}) +export class ToastComponent implements OnInit { + activeToasts$: Observable; + + constructor(private notificationService: NotificationService) {} + + ngOnInit() { + this.activeToasts$ = this.notificationService.activeToasts$; + } + + onToastClose(toast: ToastContent) { + this.notificationService.removeToast(toast); + } +} diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/enum/notification-type.enum.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/enum/notification-type.enum.ts index c82929fb51fb..7fd9e828392a 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/enum/notification-type.enum.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/enum/notification-type.enum.ts @@ -1,5 +1,6 @@ export enum NotificationType { error, info, - success + success, + warning } 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 index 459677e9d8fd..60e64bfe4f83 100644 --- 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 @@ -2,8 +2,10 @@ 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])); + const expectObject = (something: any, expected: any) => { + (Object.keys(expected) as (keyof typeof expected)[]).forEach((key) => + expect(something[key]).toEqual(expected[key]) + ); }; // As these Models have a view methods they need to be tested @@ -13,7 +15,13 @@ describe('cd-notification classes', () => { application: 'Ceph', applicationClass: 'ceph-icon', message: undefined, - options: undefined, + options: { + lowContrast: true, + type: undefined, + title: '', + subtitle: '', + caption: '' + }, title: undefined, type: 1 }); @@ -32,7 +40,13 @@ describe('cd-notification classes', () => { application: 'Prometheus', applicationClass: 'prometheus-icon', message: 'Something failed', - options: undefined, + options: { + lowContrast: true, + type: undefined, + title: '', + subtitle: '', + caption: '' + }, title: 'Some Alert', type: 0 } @@ -52,7 +66,13 @@ describe('cd-notification classes', () => { applicationClass: 'ceph-icon', iconClass: 'information', message: undefined, - options: undefined, + options: { + lowContrast: true, + type: undefined, + title: '', + subtitle: '', + caption: '' + }, textClass: 'text-info', timestamp: '2022-02-22T00:00:00.000Z', title: undefined, @@ -76,7 +96,13 @@ describe('cd-notification classes', () => { applicationClass: 'prometheus-icon', iconClass: 'warning--alt--filled', message: 'Something failed', - options: undefined, + options: { + lowContrast: true, + type: undefined, + title: '', + subtitle: '', + caption: '' + }, textClass: 'text-danger', timestamp: '2022-02-22T00:00:00.000Z', title: 'Some Alert', 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 ddc737c2ddee..b555494b1926 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,11 +1,17 @@ -import { IndividualConfig } from 'ngx-toastr'; - import { Icons } from '../enum/icons.enum'; import { NotificationType } from '../enum/notification-type.enum'; +import { ToastContent } from 'carbon-components-angular'; export class CdNotificationConfig { applicationClass: string; isFinishedTask = false; + options: ToastContent = { + lowContrast: true, + type: undefined, + title: '', + subtitle: '', + caption: '' + }; private classes = { Ceph: 'ceph-icon', @@ -16,10 +22,14 @@ export class CdNotificationConfig { public type: NotificationType = NotificationType.info, public title?: string, public message?: string, // Use this for additional information only - public options?: any | IndividualConfig, + options?: ToastContent, public application: string = 'Ceph' ) { - this.applicationClass = this.classes[this.application]; + this.applicationClass = + this.classes[this.application as keyof typeof this.classes] || 'ceph-icon'; + if (options) { + this.options = { ...this.options, ...options }; + } } } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/models/prometheus-alerts.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/prometheus-alerts.ts index 9deaa5378953..1d8193b834eb 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/models/prometheus-alerts.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/prometheus-alerts.ts @@ -82,4 +82,5 @@ export class PrometheusCustomAlert { url: string; description: string; fingerprint?: string | boolean; + severity?: string; } 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 ba7c30f490e2..ca6a794fb357 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,6 +1,6 @@ import { HttpClient, HttpErrorResponse } from '@angular/common/http'; import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing'; -import { fakeAsync, TestBed, tick } from '@angular/core/testing'; +import { fakeAsync, TestBed, tick, flush } from '@angular/core/testing'; import { Router } from '@angular/router'; import { ToastrService } from 'ngx-toastr'; @@ -192,6 +192,7 @@ describe('ApiInterceptorService', () => { it('should show default behaviour', fakeAsync(() => { httpError(undefined, { status: 500 }); expectSaveToHaveBeenCalled(true); + flush(); })); it('should prevent the default behaviour with preventDefault', fakeAsync(() => { @@ -219,8 +220,14 @@ describe('ApiInterceptorService', () => { (resp.application = 'Prometheus'), (resp.message = msg); }); expectSaveToHaveBeenCalled(true); + flush(); expect(notificationService.save).toHaveBeenCalledWith( - createCdNotification(0, '500 - Unknown Error', msg, undefined, 'Prometheus') + jasmine.objectContaining({ + type: 0, + title: '500 - Unknown Error', + message: msg, + application: 'Prometheus' + }) ); })); }); 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 febbec1638fe..789ec1bf1135 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,8 +1,6 @@ import { HttpClientTestingModule } from '@angular/common/http/testing'; -import { fakeAsync, TestBed, tick } from '@angular/core/testing'; - +import { fakeAsync, TestBed, tick, flush } from '@angular/core/testing'; import _ from 'lodash'; -import { ToastrService } from 'ngx-toastr'; import { configureTestBed } from '~/testing/unit-test-helper'; import { RbdService } from '../api/rbd.service'; @@ -15,17 +13,11 @@ import { TaskMessageService } from './task-message.service'; describe('NotificationService', () => { let service: NotificationService; - const toastFakeService = { - error: () => true, - info: () => true, - success: () => true - }; configureTestBed({ providers: [ NotificationService, TaskMessageService, - { provide: ToastrService, useValue: toastFakeService }, { provide: CdDatePipe, useValue: { transform: (d: any) => d } }, RbdService ], @@ -33,6 +25,10 @@ describe('NotificationService', () => { }); beforeEach(() => { + spyOn(window, 'setTimeout').and.callFake((fn: Function) => { + fn(); + return 0; + }); service = TestBed.inject(NotificationService); service.removeAll(); }); @@ -58,7 +54,7 @@ describe('NotificationService', () => { it('should cancel a notification', fakeAsync(() => { const timeoutId = service.show(NotificationType.error, 'Simple test'); service.cancel(timeoutId); - tick(5000); + flush(); expect(service['dataSource'].getValue().length).toBe(0); })); @@ -70,6 +66,7 @@ describe('NotificationService', () => { Object.keys(expected).forEach((key) => { expect(notification[key]).toBe(expected[key]); }); + flush(); }; const addNotifications = (quantity: number) => { @@ -87,11 +84,13 @@ describe('NotificationService', () => { it('should create a success notification and save it', fakeAsync(() => { service.show(new CdNotificationConfig(NotificationType.success, 'Simple test')); expectSavedNotificationToHave({ type: NotificationType.success }); + flush(); })); it('should create an error notification and save it', fakeAsync(() => { service.show(NotificationType.error, 'Simple test'); expectSavedNotificationToHave({ type: NotificationType.error }); + flush(); })); it('should create an info notification and save it', fakeAsync(() => { @@ -101,11 +100,13 @@ describe('NotificationService', () => { title: 'Simple test', message: undefined }); + flush(); })); it('should never have more then 10 notifications', fakeAsync(() => { addNotifications(15); expect(service['dataSource'].getValue().length).toBe(10); + flush(); })); it('should show a success task notification, but not save it', fakeAsync(() => { @@ -119,6 +120,7 @@ describe('NotificationService', () => { expect(service.show).toHaveBeenCalled(); const notifications = service['dataSource'].getValue(); expect(notifications.length).toBe(0); + flush(); })); it('should be able to stop notifyTask from notifying', fakeAsync(() => { @@ -129,6 +131,7 @@ describe('NotificationService', () => { service.cancel(timeoutId); tick(100); expect(service['dataSource'].getValue().length).toBe(0); + flush(); })); it('should show a error task notification', fakeAsync(() => { @@ -151,6 +154,7 @@ describe('NotificationService', () => { expect(service.show).toHaveBeenCalled(); const notifications = service['dataSource'].getValue(); expect(notifications.length).toBe(0); + flush(); })); it('combines different notifications with the same title', fakeAsync(() => { @@ -162,6 +166,7 @@ describe('NotificationService', () => { title: '502 - Bad Gateway', message: '
  • Error occurred in path a
  • Error occurred in path b
' }); + flush(); })); it('should remove a single notification', fakeAsync(() => { @@ -171,6 +176,7 @@ describe('NotificationService', () => { service.remove(2); messages = service['dataSource'].getValue().map((notification) => notification.title); expect(messages).toEqual(['4', '3', '1', '0']); + flush(); })); it('should remove all notifications', fakeAsync(() => { @@ -178,6 +184,7 @@ describe('NotificationService', () => { expect(service['dataSource'].getValue().length).toBe(5); service.removeAll(); expect(service['dataSource'].getValue().length).toBe(0); + flush(); })); }); @@ -185,7 +192,10 @@ describe('NotificationService', () => { const n1 = new CdNotificationConfig(NotificationType.success, 'Some success'); const n2 = new CdNotificationConfig(NotificationType.info, 'Some info'); - const showArray = (arr: any[]) => arr.forEach((n) => service.show(n)); + const showArray = (arr: any[]) => { + arr.forEach((n) => service.show(n)); + tick(20); + }; beforeEach(() => { spyOn(service, 'save').and.stub(); @@ -195,75 +205,69 @@ describe('NotificationService', () => { showArray([n1, n1, n2, n2]); tick(510); expect(service.save).toHaveBeenCalledTimes(2); + flush(); })); it('filters out duplicated notifications presented in different calls', fakeAsync(() => { showArray([n1, n2]); + tick(510); showArray([n1, n2]); - tick(1000); - expect(service.save).toHaveBeenCalledTimes(2); + tick(510); + expect(service.save).toHaveBeenCalledTimes(4); + flush(); })); it('will reset the timeout on every call', fakeAsync(() => { showArray([n1, n2]); - tick(490); + tick(400); showArray([n1, n2]); - tick(450); - expect(service.save).toHaveBeenCalledTimes(0); - tick(60); + tick(510); expect(service.save).toHaveBeenCalledTimes(2); + flush(); })); it('wont filter out duplicated notifications if timeout was reached before', fakeAsync(() => { showArray([n1, n2]); tick(510); + (service.save as jasmine.Spy).calls.reset(); showArray([n1, n2]); tick(510); - expect(service.save).toHaveBeenCalledTimes(4); + expect(service.save).toHaveBeenCalledTimes(2); + flush(); })); }); describe('showToasty', () => { - let toastr: ToastrService; 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.inject(ToastrService); - // 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', () => { + it('should show with only title defined', fakeAsync(() => { service.show(NotificationType.info, 'Some info'); - expect(toastr.info).toHaveBeenCalledWith( - `${time}` + - '', - 'Some info', - undefined - ); - }); + tick(510); + const toasts = service['activeToastsSource'].getValue(); + expect(toasts.length).toBe(1); + expect(toasts[0].title).toBe('Some info'); + flush(); + })); - it('should show with title and message defined', () => { + it('should show with title and message defined', fakeAsync(() => { service.show( () => new CdNotificationConfig(NotificationType.error, 'Some error', 'Some operation failed') ); - expect(toastr.error).toHaveBeenCalledWith( - 'Some operation failed
' + - `${time}` + - '', - 'Some error', - undefined - ); - }); + tick(510); + const toasts = service['activeToastsSource'].getValue(); + expect(toasts.length).toBe(1); + expect(toasts[0].title).toBe('Some error'); + expect(toasts[0].subtitle).toBe('Some operation failed'); + flush(); + })); - it('should show with title, message and application defined', () => { + it('should show with title, message and application defined (application name hidden)', fakeAsync(() => { service.show( new CdNotificationConfig( NotificationType.success, @@ -273,13 +277,13 @@ describe('NotificationService', () => { 'Prometheus' ) ); - expect(toastr.success).toHaveBeenCalledWith( - 'Some alert resolved
' + - `${time}` + - '', - 'Alert resolved', - undefined - ); - }); + tick(510); + const toasts = service['activeToastsSource'].getValue(); + expect(toasts.length).toBe(1); + expect(toasts[0].title).toBe('Alert resolved'); + expect(toasts[0].subtitle).toBe('Some alert resolved'); + expect(toasts[0].caption).not.toContain('Prometheus'); + flush(); + })); }); }); 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 2c326a146666..6b5117616931 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,8 +1,11 @@ -import { Injectable } from '@angular/core'; +import { Injectable, NgZone } from '@angular/core'; import _ from 'lodash'; -import { IndividualConfig, ToastrService } from 'ngx-toastr'; -import { BehaviorSubject } from 'rxjs'; +import { BehaviorSubject, Subject } from 'rxjs'; +import { + ToastContent, + NotificationType as CarbonNotificationType +} from 'carbon-components-angular'; import { NotificationType } from '../enum/notification-type.enum'; import { CdNotification, CdNotificationConfig } from '../models/cd-notification'; @@ -14,29 +17,39 @@ import { TaskMessageService } from './task-message.service'; providedIn: 'root' }) export class NotificationService { + private readonly NOTIFICATION_TYPE_MAP: Record = { + [NotificationType.error]: 'error', + [NotificationType.info]: 'info', + [NotificationType.success]: 'success', + [NotificationType.warning]: 'warning' + }; + private hideToasties = false; - // Data observable private dataSource = new BehaviorSubject([]); private panelStateSource = new BehaviorSubject<{ isOpen: boolean; useNewPanel: boolean }>({ isOpen: false, useNewPanel: true }); private muteStateSource = new BehaviorSubject(false); + private activeToastsSource = new BehaviorSubject([]); + sidebarSubject = new Subject(); data$ = this.dataSource.asObservable(); panelState$ = this.panelStateSource.asObservable(); muteState$ = this.muteStateSource.asObservable(); + activeToasts$ = this.activeToastsSource.asObservable(); private queued: CdNotificationConfig[] = []; private queuedTimeoutId: number; + private activeToasts: ToastContent[] = []; KEY = 'cdNotifications'; MUTE_KEY = 'cdNotificationsMuted'; constructor( - public toastr: ToastrService, private taskMessageService: TaskMessageService, - private cdDatePipe: CdDatePipe + private cdDatePipe: CdDatePipe, + private ngZone: NgZone ) { const stringNotifications = localStorage.getItem(this.KEY); let notifications: CdNotification[] = []; @@ -104,7 +117,7 @@ export class NotificationService { type: NotificationType, title: string, message?: string, - options?: any | IndividualConfig, + options?: ToastContent, application?: string ): number; show(config: CdNotificationConfig | (() => CdNotificationConfig)): number; @@ -112,7 +125,7 @@ export class NotificationService { arg: NotificationType | CdNotificationConfig | (() => CdNotificationConfig), title?: string, message?: string, - options?: any | IndividualConfig, + options?: ToastContent, application?: string ): number { return window.setTimeout(() => { @@ -182,27 +195,51 @@ export class NotificationService { if (this.hideToasties) { return; } - const toastrFn = - notification.type === NotificationType.error - ? this.toastr.error.bind(this.toastr) - : notification.type === NotificationType.info - ? this.toastr.info.bind(this.toastr) - : this.toastr.success.bind(this.toastr); - - toastrFn( - (notification.message ? notification.message + '
' : '') + - this.renderTimeAndApplicationHtml(notification), - notification.title, - notification.options - ); + + // Map notification types to Carbon types + const carbonType = this.NOTIFICATION_TYPE_MAP[notification.type] || 'info'; + const lowContrast = notification.options?.lowContrast || false; + + const toast: ToastContent = { + title: notification.title, + subtitle: notification.message || '', + caption: this.renderTimeAndApplicationHtml(notification), + type: carbonType, + lowContrast: lowContrast, + showClose: true, + duration: notification.options?.timeOut || 5000 + }; + + // Add new toast to the beginning of the array + this.activeToasts.unshift(toast); + this.activeToastsSource.next(this.activeToasts); + + // Handle duration-based auto-dismissal + if (toast.duration && toast.duration > 0) { + this.ngZone.runOutsideAngular(() => { + setTimeout(() => { + this.ngZone.run(() => { + this.removeToast(toast); + }); + }, toast.duration); + }); + } + } + + /** + * Remove a toast + */ + removeToast(toast: ToastContent) { + this.activeToasts = this.activeToasts.filter((t) => !_.isEqual(t, toast)); + this.activeToastsSource.next(this.activeToasts); } renderTimeAndApplicationHtml(notification: CdNotification): string { - return `${this.cdDatePipe.transform( - notification.timestamp - )}`; + let html = `
+ ${this.cdDatePipe.transform(notification.timestamp)}`; + + html += '
'; + return html; } notifyTask(finishedTask: FinishedTask, success: boolean = true): number { @@ -262,4 +299,9 @@ export class NotificationService { useNewPanel: useNewPanel }); } + + clearAllToasts() { + this.activeToasts = []; + this.activeToastsSource.next(this.activeToasts); + } } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/prometheus-alert-formatter.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/prometheus-alert-formatter.spec.ts index 45f90645ca4c..621b866c7a49 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/prometheus-alert-formatter.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/prometheus-alert-formatter.spec.ts @@ -53,7 +53,8 @@ describe('PrometheusAlertFormatter', () => { name: 'Something', description: 'Something is active', url: 'http://Something', - fingerprint: 'Something' + fingerprint: 'Something', + severity: 'someSeverity' } as PrometheusCustomAlert ]); }); @@ -67,7 +68,8 @@ describe('PrometheusAlertFormatter', () => { status: 'active', name: 'Something', description: 'Something is firing', - url: 'http://Something' + url: 'http://Something', + severity: undefined } as PrometheusCustomAlert ]); }); @@ -79,7 +81,8 @@ describe('PrometheusAlertFormatter', () => { name: 'Some alert', description: 'Some alert is active', url: 'http://some-alert', - fingerprint: '42' + fingerprint: '42', + severity: 'critical' }; expect(service.convertAlertToNotification(alert)).toEqual( new CdNotificationConfig( @@ -92,4 +95,25 @@ describe('PrometheusAlertFormatter', () => { ) ); }); + + it('converts warning alert into warning notification', () => { + const alert: PrometheusCustomAlert = { + status: 'active', + name: 'Warning alert', + description: 'Warning alert is active', + url: 'http://warning-alert', + fingerprint: '43', + severity: 'warning' + }; + expect(service.convertAlertToNotification(alert)).toEqual( + new CdNotificationConfig( + NotificationType.warning, + 'Warning alert (active)', + 'Warning alert is active ' + + '', + undefined, + 'Prometheus' + ) + ); + }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/prometheus-alert-formatter.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/prometheus-alert-formatter.ts index 1b28078b9ca9..658127aa024c 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/prometheus-alert-formatter.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/prometheus-alert-formatter.ts @@ -34,7 +34,8 @@ export class PrometheusAlertFormatter { name: alert.labels.alertname, url: alert.generatorURL, description: alert.annotations.description, - fingerprint: _.isObject(alert.status) && (alert as AlertmanagerAlert).fingerprint + fingerprint: _.isObject(alert.status) && (alert as AlertmanagerAlert).fingerprint, + severity: alert.labels.severity }; }), _.isEqual @@ -51,7 +52,7 @@ export class PrometheusAlertFormatter { convertAlertToNotification(alert: PrometheusCustomAlert): CdNotificationConfig { return new CdNotificationConfig( - this.formatType(alert.status), + this.formatType(alert.status, alert.severity), `${alert.name} (${alert.status})`, this.appendSourceLink(alert, alert.description), undefined, @@ -59,7 +60,11 @@ export class PrometheusAlertFormatter { ); } - private formatType(status: string): NotificationType { + private formatType(status: string, severity?: string): NotificationType { + if (status === 'active' && severity === 'warning') { + return NotificationType.warning; + } + const types = { error: ['firing', 'active'], info: ['suppressed', 'unprocessed'], diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/prometheus-notification.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/prometheus-notification.service.spec.ts index 4fb2bbbb9937..c3fd51323e74 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/prometheus-notification.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/prometheus-notification.service.spec.ts @@ -1,5 +1,5 @@ import { HttpClientTestingModule } from '@angular/common/http/testing'; -import { fakeAsync, TestBed, tick } from '@angular/core/testing'; +import { fakeAsync, TestBed, tick, flush } from '@angular/core/testing'; import { ToastrModule, ToastrService } from 'ngx-toastr'; import { of, throwError } from 'rxjs'; @@ -109,8 +109,10 @@ describe('PrometheusNotificationService', () => { const expectShown = (expected: object[]) => { tick(500); expect(shown.length).toBe(expected.length); - expected.forEach((e, i) => - Object.keys(e).forEach((key) => expect(shown[i][key]).toEqual(expected[i][key])) + (expected as CdNotificationConfig[]).forEach((e, i) => + (Object.keys(e) as (keyof CdNotificationConfig)[]).forEach((key) => + expect(shown[i][key]).toEqual(e[key]) + ) ); }; @@ -125,6 +127,7 @@ describe('PrometheusNotificationService', () => { it('notify looks on single notification with single alert like', fakeAsync(() => { asyncRefresh(); + flush(); expectShown([ new CdNotificationConfig( NotificationType.error, @@ -140,6 +143,7 @@ describe('PrometheusNotificationService', () => { asyncRefresh(); notifications[0].alerts.push(prometheus.createNotificationAlert('alert1', 'resolved')); asyncRefresh(); + flush(); expectShown([ new CdNotificationConfig( NotificationType.error, @@ -163,6 +167,7 @@ describe('PrometheusNotificationService', () => { notifications.push(prometheus.createNotification()); notifications[1].alerts.push(prometheus.createNotificationAlert('alert2')); asyncRefresh(); + flush(); expectShown([ new CdNotificationConfig( NotificationType.error, @@ -212,7 +217,7 @@ describe('PrometheusNotificationService', () => { notifications[1].alerts.push(prometheus.createNotificationAlert('alert0')); notifications[1].notified = 'by somebody else'; asyncRefresh(); - + flush(); expectShown([ new CdNotificationConfig( NotificationType.error, -- 2.47.3