From a9a6255cd2d2d61b48ccfe18af5b4751817832ba Mon Sep 17 00:00:00 2001 From: Afreen Misbah Date: Tue, 21 Jul 2026 18:01:36 +0530 Subject: [PATCH] mgr/dashboard: Sanitize toast HTML and optimize notification service - Escape user-controlled content before Carbon innerHTML rendering - Whitelist localStorage deserialization keys, prune stale read map - Replace deep equality with ID comparison in removeToast - Gate ngAfterViewChecked DOM queries behind a dirty flag - Deduplicate by title+type+message; show occurrences count - Fix bell icon unread dot clearing on new occurrences - Fix view more pre-selection via route queryParams observable - Save individual notifications instead of HTML-merged messages - Convert notifications bell component to signals with OnPush - Add OnPush to notification-item and toast components - Add e2e tests - Fix styling of notification panel Signed-off-by: Afreen Misbah --- .../cypress/e2e/ui/notification.e2e-spec.ts | 240 ++++++++++++++++++ .../cypress/e2e/ui/notification.po.ts | 60 +++++ .../notification-area.component.html | 1 + .../notification-area.component.scss | 12 + .../notification-item.component.html | 8 + .../notification-item.component.scss | 5 + .../notification-item.component.spec.ts | 1 - .../notification-item.component.ts | 12 +- .../notifications-page.component.html | 12 +- .../notifications-page.component.spec.ts | 48 ++-- .../notifications-page.component.ts | 27 +- .../notifications.component.html | 4 +- .../notifications.component.spec.ts | 59 +++-- .../notifications/notifications.component.ts | 47 +--- .../notification-toast.component.ts | 14 +- .../services/notification.service.spec.ts | 87 ++++++- .../shared/services/notification.service.ts | 111 +++++--- 17 files changed, 608 insertions(+), 140 deletions(-) diff --git a/src/pybind/mgr/dashboard/frontend/cypress/e2e/ui/notification.e2e-spec.ts b/src/pybind/mgr/dashboard/frontend/cypress/e2e/ui/notification.e2e-spec.ts index 17c2af07a2c..4039694e61a 100644 --- a/src/pybind/mgr/dashboard/frontend/cypress/e2e/ui/notification.e2e-spec.ts +++ b/src/pybind/mgr/dashboard/frontend/cypress/e2e/ui/notification.e2e-spec.ts @@ -54,3 +54,243 @@ describe('Notification page', () => { }); }); }); + +describe('Notification panel to page navigation', () => { + const notification = new NotificationSidebarPageHelper(); + + beforeEach(() => { + cy.login(); + cy.visit('#/'); + cy.wait(5000); + }); + + it('should navigate to notifications page via View all', () => { + notification.navigateToNotificationsPage(); + cy.url().should('include', '/notifications'); + notification.getNotificationsPage().should('exist'); + }); + + it('should navigate from panel notification click to page with pre-selected detail', () => { + notification.open(); + notification.getNotifications().then(($items) => { + if ($items.length === 0) { + cy.log('No notifications in panel — skipping'); + return; + } + notification.getNotifications().first().click(); + cy.url().should('include', '/notifications'); + cy.url().should('include', 'id='); + notification.getNotificationsPage().should('exist'); + notification.getNotificationsPageDetail().find('cd-notification-item').should('exist'); + notification + .getNotificationsPageListItems() + .first() + .should('have.class', 'notifications-page__item--active'); + }); + }); + + it('should show back button and navigate back', () => { + notification.navigateToNotificationsPage(); + cy.contains('Back').should('be.visible').click(); + cy.url().should('not.include', '/notifications'); + }); +}); + +describe('Notifications page actions', () => { + const notification = new NotificationSidebarPageHelper(); + + beforeEach(() => { + cy.login(); + cy.visit('#/'); + cy.wait(3000); + }); + + it('should show kebab menu with mark all as read and clear all', () => { + notification.navigateToNotificationsPage(); + notification.getKebabMenu().click(); + notification.getKebabMenuItems().should('have.length', 2); + cy.contains('.cds--overflow-menu-options__btn', 'Mark all as read').should('exist'); + cy.contains('.cds--overflow-menu-options__btn', 'Clear all').should('exist'); + }); + + it('should show empty state when no notifications', () => { + notification.open(); + notification.clearNotifications(); + notification.getViewAllBtn().click({ force: true }); + notification.getNotificationsPage().should('exist'); + cy.contains('No notifications available').should('be.visible'); + }); + + it('should disable kebab actions when no notifications', () => { + notification.open(); + notification.clearNotifications(); + notification.getViewAllBtn().click({ force: true }); + notification.getKebabMenu().click(); + cy.contains('.cds--overflow-menu-options__btn', 'Mark all as read').should('be.disabled'); + cy.contains('.cds--overflow-menu-options__btn', 'Clear all').should('be.disabled'); + }); +}); + +describe('Notification selection and detail', () => { + const notification = new NotificationSidebarPageHelper(); + + beforeEach(() => { + cy.login(); + cy.visit('#/'); + cy.wait(5000); + }); + + it('should select a notification and show detail in right panel', () => { + notification.navigateToNotificationsPage(); + notification.getNotificationsPageListItems().then(($items) => { + if ($items.length === 0) { + cy.log('No notifications to select — skipping'); + return; + } + cy.wrap($items.first()).click(); + cy.wrap($items.first()).should('have.class', 'notifications-page__item--active'); + notification.getNotificationsPageDetail().find('cd-notification-item').should('exist'); + notification + .getNotificationsPageDetail() + .find('.notifications-page__detail-text') + .should('exist'); + }); + }); + + it('should show occurrences count when notification has duplicates', () => { + notification.navigateToNotificationsPage(); + notification.getNotificationsPageListItems().then(($items) => { + if ($items.length === 0) { + cy.log('No notifications — skipping'); + return; + } + cy.wrap($items.first()).click(); + notification.getNotificationsPageDetail().then(($detail) => { + const hasOccurrences = $detail.find('.notifications-page__occurrences').length > 0; + if (hasOccurrences) { + cy.get('.notifications-page__occurrences') + .should('be.visible') + .and('contain', 'Occurrences:'); + } else { + cy.log('First notification has no duplicates — occurrences label correctly hidden'); + } + }); + }); + }); + + it('should clear all notifications via kebab menu', () => { + notification.navigateToNotificationsPage(); + notification.getNotificationsPageListItems().then(($items) => { + if ($items.length === 0) { + cy.log('No notifications to clear — skipping'); + return; + } + notification.getKebabMenu().click(); + cy.contains('.cds--overflow-menu-options__btn', 'Clear all').click(); + cy.contains('No notifications available').should('be.visible'); + }); + }); +}); + +describe('Notification toast deduplication', () => { + const notification = new NotificationSidebarPageHelper(); + + beforeEach(() => { + cy.login(); + cy.visit('#/'); + }); + + it('should show +N more count on duplicate toasts', () => { + cy.wait(8000); + cy.get('body').then(($body) => { + if ($body.find('.toast-duplicate-count').length > 0) { + notification + .getToastDuplicateCount() + .first() + .should('be.visible') + .invoke('text') + .should('match', /\(\+\d+ more\)/); + } else if ($body.find('cds-toast').length > 0) { + cy.log('Toasts present but no duplicates yet — only unique errors firing'); + } else { + cy.log('No toasts appeared — skipping'); + } + }); + }); + + it('should show view more link when toast text is truncated', () => { + cy.wait(5000); + cy.get('body').then(($body) => { + if ($body.find('.toast-view-more:visible').length > 0) { + notification + .getToastViewMoreLink() + .first() + .should('be.visible') + .and('contain', 'View more'); + } else { + cy.log('No truncated toasts with view more — skipping'); + } + }); + }); + + it('should navigate to notifications page and dismiss toasts on view more click', () => { + cy.wait(5000); + cy.get('body').then(($body) => { + if ($body.find('.toast-view-more:visible').length > 0) { + notification.getToastViewMoreLink().first().click({ force: true }); + cy.url().should('include', '/notifications'); + notification.getNotificationsPage().should('exist'); + notification.getToasts().should('not.exist'); + } else { + cy.log('No visible view more link — skipping'); + } + }); + }); +}); + +describe('Notification read state', () => { + const notification = new NotificationSidebarPageHelper(); + + beforeEach(() => { + cy.login(); + cy.visit('#/'); + cy.wait(5000); + }); + + it('should mark notification as read on selection', () => { + notification.navigateToNotificationsPage(); + notification.getNotificationsPageListItems().then(($items) => { + if ($items.length === 0) { + cy.log('No notifications — skipping'); + return; + } + cy.wrap($items.first()).click(); + cy.wrap($items.first()).should('not.have.class', 'notifications-page__item--unread'); + }); + }); + + it('should mark all as read via kebab menu', () => { + notification.navigateToNotificationsPage(); + notification.getNotificationsPageListItems().then(($items) => { + if ($items.length === 0) { + cy.log('No notifications — skipping'); + return; + } + notification.getKebabMenu().click(); + cy.contains('.cds--overflow-menu-options__btn', 'Mark all as read').click(); + notification.getNotificationsPageListItems().each(($item) => { + cy.wrap($item).should('not.have.class', 'notifications-page__item--unread'); + }); + }); + }); + + it('should show unread indicator on bell icon when unread notifications exist', () => { + cy.visit('#/'); + cy.wait(5000); + cy.get('[data-testid="header-notification-icon"]').then(($icon) => { + const iconType = $icon.find('cd-icon').attr('type'); + cy.log(`Bell icon type: ${iconType}`); + expect(['notification', 'notificationNew']).to.include(iconType); + }); + }); +}); diff --git a/src/pybind/mgr/dashboard/frontend/cypress/e2e/ui/notification.po.ts b/src/pybind/mgr/dashboard/frontend/cypress/e2e/ui/notification.po.ts index 897e11153f5..36b2b003c6c 100644 --- a/src/pybind/mgr/dashboard/frontend/cypress/e2e/ui/notification.po.ts +++ b/src/pybind/mgr/dashboard/frontend/cypress/e2e/ui/notification.po.ts @@ -1,6 +1,10 @@ import { PageHelper } from '../page-helper.po'; export class NotificationSidebarPageHelper extends PageHelper { + pages = { + index: { url: '#/notifications', id: 'cd-notifications-page' } + }; + getNotificationIcon() { return cy.get(`[data-testid='header-notification-icon']`); } @@ -31,6 +35,10 @@ export class NotificationSidebarPageHelper extends PageHelper { return cy.get('cd-notification-panel .notification-header__dismiss-btn'); } + getViewAllBtn() { + return cy.get('.notification-footer__view-all-button'); + } + open() { this.getNotificationIcon().click({ force: true }); this.getPanel().should('exist'); @@ -48,4 +56,56 @@ export class NotificationSidebarPageHelper extends PageHelper { this.clearNotifications(); }); } + + // Notifications page helpers + + getNotificationsPage() { + return cy.get('.notifications-page'); + } + + getNotificationsPageList() { + return cy.get('.notifications-page__list'); + } + + getNotificationsPageListItems() { + return cy.get('.notifications-page__item'); + } + + getNotificationsPageDetail() { + return cy.get('.notifications-page__detail'); + } + + getKebabMenu() { + return cy.get('.notifications-page__header-menu cds-overflow-menu'); + } + + getKebabMenuItems() { + return cy.get('.cds--overflow-menu-options__btn'); + } + + getOccurrencesLabel() { + return cy.get('.cd-notification-item__occurrences'); + } + + getToastContainer() { + return cy.get('.cds--toast-notification-container'); + } + + getToasts() { + return cy.get('cds-toast'); + } + + getToastViewMoreLink() { + return cy.get('.toast-view-more'); + } + + getToastDuplicateCount() { + return cy.get('.toast-duplicate-count'); + } + + navigateToNotificationsPage() { + this.open(); + this.getViewAllBtn().click({ force: true }); + this.getNotificationsPage().should('exist'); + } } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-area/notification-area.component.html b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-area/notification-area.component.html index 08a67d7e9f7..e85813bf05b 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-area/notification-area.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-area/notification-area.component.html @@ -61,6 +61,7 @@ [timestamp]="notification.timestamp" [message]="notification.message" [notificationId]="notification.id" + [occurrences]="notification.occurrences" > diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-area/notification-area.component.scss b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-area/notification-area.component.scss index 28569607d15..c10bc51c41f 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-area/notification-area.component.scss +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-area/notification-area.component.scss @@ -59,6 +59,18 @@ cd-notification-area { &:last-child { padding-bottom: var(--cds-spacing-05); } + + > .border-subtle-bottom { + margin: 0 calc(-1 * var(--cds-spacing-05)); + } + + &:has(.notification-item--clickable) { + cursor: pointer; + + &:hover { + background-color: var(--cds-layer-hover); + } + } } .notification-item { diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-item/notification-item.component.html b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-item/notification-item.component.html index 729361ed41d..5b0639b8ec1 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-item/notification-item.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-item/notification-item.component.html @@ -36,6 +36,14 @@

} } + @if (occurrences > 1) { + + Occurrences: {{ occurrences }} + + }
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-item/notification-item.component.scss b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-item/notification-item.component.scss index 94d7b75d64d..b359d84df1d 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-item/notification-item.component.scss +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-item/notification-item.component.scss @@ -83,6 +83,11 @@ cd-notification-item { height: 1rem; } +.cd-notification-item__occurrences { + color: var(--cds-text-secondary); + margin: 0; +} + .cd-notification-item__chevron svg { fill: var(--cds-icon-secondary); } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-item/notification-item.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-item/notification-item.component.spec.ts index cf749081f86..408199884f2 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-item/notification-item.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-item/notification-item.component.spec.ts @@ -9,7 +9,6 @@ import { } from '~/app/shared/enum/notification-type.enum'; import { NotificationService } from '~/app/shared/services/notification.service'; import { SharedModule } from '~/app/shared/shared.module'; -import { CdNotification, CdNotificationConfig } from '~/app/shared/models/cd-notification'; @Component({ template: ` diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-item/notification-item.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-item/notification-item.component.ts index 04aabf9e61f..a15184bb014 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-item/notification-item.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notification-item/notification-item.component.ts @@ -1,4 +1,11 @@ -import { Component, EventEmitter, Input, Output, ViewEncapsulation } from '@angular/core'; +import { + Component, + EventEmitter, + Input, + Output, + ViewEncapsulation, + ChangeDetectionStrategy +} from '@angular/core'; import { NotificationApplication, NotificationType @@ -10,6 +17,7 @@ import { NotificationService } from '~/app/shared/services/notification.service' templateUrl: './notification-item.component.html', styleUrls: ['./notification-item.component.scss'], encapsulation: ViewEncapsulation.None, + changeDetection: ChangeDetectionStrategy.OnPush, standalone: false }) export class NotificationItemComponent { @@ -29,6 +37,8 @@ export class NotificationItemComponent { @Input() showChevron: boolean = false; /* Replace severity icon with unread dot */ @Input() showUnread: boolean = false; + /* Number of deduplicated occurrences */ + @Input() occurrences: number = 1; /* Emitted after notification is deleted via the service */ @Output() deleted = new EventEmitter(); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notifications-page/notifications-page.component.html b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notifications-page/notifications-page.component.html index ad19191f339..68f8224e89e 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notifications-page/notifications-page.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notifications-page/notifications-page.component.html @@ -74,6 +74,7 @@ [timestamp]="notification.timestamp" [message]="notification.displayPreview" [notificationId]="notification.id" + [occurrences]="notification.occurrences" [showChevron]="true" [showUnread]="!readMap()[notification.id]" > @@ -92,6 +93,12 @@ > No notifications available

+

+ Notifications are stored locally and cleared on browser reset. +

} @@ -110,7 +117,10 @@
@if (selected.occurrences > 1) { -

+

Occurrences: {{ selected.occurrences }}

} diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notifications-page/notifications-page.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notifications-page/notifications-page.component.spec.ts index 22ae569fa38..1a5f450bca5 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notifications-page/notifications-page.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notifications-page/notifications-page.component.spec.ts @@ -22,6 +22,7 @@ describe('NotificationsPageComponent', () => { let readMapSubject: BehaviorSubject>; let notificationService: any; let mockLocation: any; + let queryParamsSubject: BehaviorSubject; const createMockNotificationService = () => { dataSourceSubject = new BehaviorSubject([]); @@ -50,7 +51,8 @@ describe('NotificationsPageComponent', () => { readMapSubject.next(updated); localStorage.setItem('cdNotificationsRead', JSON.stringify(updated)); } - }) + }), + getNotificationsSnapshot: () => dataSourceSubject.getValue() }; }; @@ -118,6 +120,7 @@ describe('NotificationsPageComponent', () => { mockLocation = { back: jasmine.createSpy('back') }; const mockNotificationService = createMockNotificationService(); notificationService = mockNotificationService; + queryParamsSubject = new BehaviorSubject({}); localStorage.removeItem('cdNotificationsRead'); @@ -130,7 +133,13 @@ describe('NotificationsPageComponent', () => { { provide: PrometheusNotificationService, useValue: mockPrometheusNotificationService }, { provide: AuthStorageService, useValue: mockAuthStorageService }, { provide: Location, useValue: mockLocation }, - { provide: ActivatedRoute, useValue: { snapshot: { queryParams: {} } } } + { + provide: ActivatedRoute, + useValue: { + snapshot: { queryParams: {} }, + queryParams: queryParamsSubject.asObservable() + } + } ], schemas: [CUSTOM_ELEMENTS_SCHEMA] }).compileComponents(); @@ -310,13 +319,8 @@ describe('NotificationsPageComponent', () => { }); describe('query param pre-selection', () => { - it('should pre-select notification from id query param', async () => { - const route = TestBed.inject(ActivatedRoute); - (route.snapshot.queryParams as any) = { id: '2' }; - - fixture = TestBed.createComponent(NotificationsPageComponent); - component = fixture.componentInstance; - dataSourceSubject.next(mockNotifications); + it('should pre-select notification from id query param', () => { + queryParamsSubject.next({ id: '2' }); fixture.detectChanges(); expect(component.selectedNotificationID()).toBe('2'); @@ -324,25 +328,16 @@ describe('NotificationsPageComponent', () => { }); it('should not pre-select if id does not match any notification', () => { - const route = TestBed.inject(ActivatedRoute); - (route.snapshot.queryParams as any) = { id: 'nonexistent' }; - - fixture = TestBed.createComponent(NotificationsPageComponent); - component = fixture.componentInstance; - dataSourceSubject.next(mockNotifications); + queryParamsSubject.next({ id: 'nonexistent' }); fixture.detectChanges(); expect(component.selectedNotificationID()).toBeNull(); }); it('should not override manual selection on subsequent data emissions', () => { - const route = TestBed.inject(ActivatedRoute); - (route.snapshot.queryParams as any) = { id: '2' }; - - fixture = TestBed.createComponent(NotificationsPageComponent); - component = fixture.componentInstance; - dataSourceSubject.next(mockNotifications); + queryParamsSubject.next({ id: '2' }); fixture.detectChanges(); + expect(component.selectedNotificationID()).toBe('2'); component.onNotificationSelect(component.notifications()[0]); expect(component.selectedNotificationID()).toBe('1'); @@ -351,6 +346,17 @@ describe('NotificationsPageComponent', () => { fixture.detectChanges(); expect(component.selectedNotificationID()).toBe('1'); }); + + it('should pre-select when navigating from toast view more link', () => { + dataSourceSubject.next(mockNotifications); + fixture.detectChanges(); + + queryParamsSubject.next({ id: '3' }); + fixture.detectChanges(); + + expect(component.selectedNotificationID()).toBe('3'); + expect(notificationService.markAsRead).toHaveBeenCalledWith('3'); + }); }); it('should set up interval for Prometheus alerts when permissions exist', () => { diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notifications-page/notifications-page.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notifications-page/notifications-page.component.ts index b9c517eea79..6d79b123d06 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notifications-page/notifications-page.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notification-panel/notifications-page/notifications-page.component.ts @@ -48,6 +48,7 @@ export class NotificationsPageComponent implements OnInit, OnDestroy { private sub: Subscription; private interval: number; + private _pendingId: string | null = null; constructor( private notificationService: NotificationService, @@ -81,15 +82,15 @@ export class NotificationsPageComponent implements OnInit, OnDestroy { ) ); - const id = this.route.snapshot.queryParams['id']; - if (id && !this.selectedNotificationID()) { - const match = notifications.find((n) => n.id === id); - if (match) { - this.selectedNotificationID.set(id); - this.notificationService.markAsRead(id); - } - } + this._tryPreselect(notifications); }); + + this.sub.add( + this.route.queryParams.subscribe((params) => { + this._pendingId = params['id'] || null; + this._tryPreselect(this.notificationService.getNotificationsSnapshot()); + }) + ); } ngOnDestroy(): void { @@ -132,6 +133,16 @@ export class NotificationsPageComponent implements OnInit, OnDestroy { } } + private _tryPreselect(notifications: CdNotification[]): void { + if (!this._pendingId || this.selectedNotificationID()) return; + const match = notifications.find((n) => n.id === this._pendingId); + if (match) { + this.selectedNotificationID.set(this._pendingId); + this.notificationService.markAsRead(this._pendingId); + this._pendingId = null; + } + } + private triggerPrometheusAlerts(): void { this.prometheusAlertService.refresh(); this.prometheusNotificationService.refresh(); 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 ee41505a363..9b10a25308d 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,10 +1,10 @@ -@if (isMuted) { +@if (isMuted()) { -} @else if (!isMuted && (hasRunningTasks || hasNotifications)) { +} @else if (hasRunningTasks() || hasNotifications()) { { let component: NotificationsComponent; let fixture: ComponentFixture; let summaryService: SummaryService; - let notificationService: NotificationService; + const hasUnreadSource = new BehaviorSubject(false); + const muteSource = new BehaviorSubject(false); const notificationServiceMock = { - dataSource: new BehaviorSubject([]), - hasUnreadSource, - get hasUnread$() { - return hasUnreadSource.asObservable(); - }, - muteState$: new BehaviorSubject(false) + hasUnread$: hasUnreadSource.asObservable(), + muteState$: muteSource.asObservable() }; configureTestBed({ - imports: [HttpClientTestingModule, SharedModule, RouterTestingModule], + imports: [HttpClientTestingModule], declarations: [NotificationsComponent], - providers: [{ provide: NotificationService, useValue: notificationServiceMock }] + providers: [{ provide: NotificationService, useValue: notificationServiceMock }], + schemas: [CUSTOM_ELEMENTS_SCHEMA] }); beforeEach(() => { + hasUnreadSource.next(false); + muteSource.next(false); fixture = TestBed.createComponent(NotificationsComponent); component = fixture.componentInstance; summaryService = TestBed.inject(SummaryService); - notificationService = TestBed.inject(NotificationService); - fixture.detectChanges(); }); @@ -46,24 +43,38 @@ describe('NotificationsComponent', () => { expect(component).toBeTruthy(); }); - it('should subscribe and check if there are running tasks', () => { - expect(component.hasRunningTasks).toBeFalsy(); + it('should reflect running tasks from summary service', () => { + expect(component.hasRunningTasks()).toBe(false); const task = new ExecutingTask('task', { name: 'name' }); summaryService['summaryDataSource'].next({ executing_tasks: [task] }); - expect(component.hasRunningTasks).toBeTruthy(); + expect(component.hasRunningTasks()).toBe(true); }); - it('should show notificationNew icon if there are notifications', () => { - const notification = new CdNotification(new CdNotificationConfig()); - notificationService['dataSource'].next([notification]); - notificationService['hasUnreadSource'].next(true); - + it('should show notificationNew icon when unread notifications exist', () => { + hasUnreadSource.next(true); fixture.detectChanges(); const icon = fixture.debugElement.nativeElement.querySelector('cd-icon'); expect(icon).toBeTruthy(); expect(icon.getAttribute('type')).toBe('notificationNew'); }); + + it('should show notification icon when no unread and no running tasks', () => { + fixture.detectChanges(); + + const icon = fixture.debugElement.nativeElement.querySelector('cd-icon'); + expect(icon).toBeTruthy(); + expect(icon.getAttribute('type')).toBe('notification'); + }); + + it('should show notificationOff icon when muted', () => { + muteSource.next(true); + fixture.detectChanges(); + + const icon = fixture.debugElement.nativeElement.querySelector('cd-icon'); + expect(icon).toBeTruthy(); + expect(icon.getAttribute('type')).toBe('notificationOff'); + }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notifications/notifications.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notifications/notifications.component.ts index f7d0d3bc46a..3c86778afe9 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notifications/notifications.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/navigation/notifications/notifications.component.ts @@ -1,6 +1,6 @@ -import { Component, OnDestroy, OnInit } from '@angular/core'; - -import { Subscription } from 'rxjs'; +import { Component, ChangeDetectionStrategy, inject, Signal } from '@angular/core'; +import { toSignal } from '@angular/core/rxjs-interop'; +import { map } from 'rxjs/operators'; import { ICON_TYPE, IconSize } from '~/app/shared/enum/icons.enum'; import { NotificationService } from '~/app/shared/services/notification.service'; @@ -10,42 +10,21 @@ import { SummaryService } from '~/app/shared/services/summary.service'; selector: 'cd-notifications', templateUrl: './notifications.component.html', styleUrls: ['./notifications.component.scss'], + changeDetection: ChangeDetectionStrategy.OnPush, standalone: false }) -export class NotificationsComponent implements OnInit, OnDestroy { +export class NotificationsComponent { icons = ICON_TYPE; iconSize = IconSize.size20; - hasRunningTasks = false; - hasNotifications = false; - isMuted = false; - private subs = new Subscription(); - constructor( - public notificationService: NotificationService, - private summaryService: SummaryService - ) {} + private notificationService = inject(NotificationService); + private summaryService = inject(SummaryService); - ngOnInit() { - this.subs.add( - this.summaryService.subscribe((summary) => { - this.hasRunningTasks = summary.executing_tasks.length > 0; - }) - ); + hasRunningTasks: Signal = toSignal( + this.summaryService.summaryData$.pipe(map((summary) => summary?.executing_tasks?.length > 0)), + { initialValue: false } + ); - this.subs.add( - this.notificationService.muteState$.subscribe((isMuted) => { - this.isMuted = isMuted; - }) - ); - - this.subs.add( - this.notificationService.hasUnread$.subscribe( - (hasUnread) => (this.hasNotifications = hasUnread) - ) - ); - } - - ngOnDestroy(): void { - this.subs.unsubscribe(); - } + isMuted = toSignal(this.notificationService.muteState$, { initialValue: false }); + hasNotifications = toSignal(this.notificationService.hasUnread$, { initialValue: false }); } 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 index 6c5446cfea2..889929cebfb 100644 --- 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 @@ -3,7 +3,8 @@ import { OnInit, AfterViewChecked, HostListener, - ElementRef + ElementRef, + ChangeDetectionStrategy } from '@angular/core'; import { animate, style, transition, trigger } from '@angular/animations'; import { Router } from '@angular/router'; @@ -15,6 +16,7 @@ import { NotificationService } from '../../services/notification.service'; selector: 'cd-toast', templateUrl: './notification-toast.component.html', styleUrls: ['./notification-toast.component.scss'], + changeDetection: ChangeDetectionStrategy.OnPush, animations: [ trigger('toastAnimation', [ transition( @@ -39,6 +41,7 @@ import { NotificationService } from '../../services/notification.service'; }) export class ToastComponent implements OnInit, AfterViewChecked { activeToasts$: Observable; + private toastsDirty = false; constructor( private notificationService: NotificationService, @@ -48,15 +51,22 @@ export class ToastComponent implements OnInit, AfterViewChecked { ngOnInit() { this.activeToasts$ = this.notificationService.activeToasts$; + this.notificationService.activeToasts$.subscribe(() => { + this.toastsDirty = true; + }); } ngAfterViewChecked() { + if (!this.toastsDirty) return; + this.toastsDirty = false; + const toasts = this.el.nativeElement.querySelectorAll('cds-toast'); toasts.forEach((toast: HTMLElement) => { const subtitle = toast.querySelector('.cds--toast-notification__subtitle'); const viewMore = toast.querySelector('.toast-view-more') as HTMLElement; if (!subtitle || !viewMore) return; - const isTruncated = subtitle.scrollHeight > subtitle.clientHeight; + const textEl = subtitle.querySelector('.toast-message') || subtitle; + const isTruncated = textEl.scrollHeight > textEl.clientHeight; viewMore.style.display = isTruncated ? '' : 'none'; }); } 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 0eb6a5313d5..6547c560b2e 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,7 +5,7 @@ import _ from 'lodash'; import { configureTestBed } from '~/testing/unit-test-helper'; import { RbdService } from '../api/rbd.service'; import { NotificationType } from '../enum/notification-type.enum'; -import { CdNotificationConfig } from '../models/cd-notification'; +import { CdNotification, CdNotificationConfig } from '../models/cd-notification'; import { FinishedTask } from '../models/finished-task'; import { CdDatePipe } from '../pipes/cd-date.pipe'; import { NotificationService } from './notification.service'; @@ -157,15 +157,25 @@ describe('NotificationService', () => { flush(); })); - it('combines different notifications with the same title', fakeAsync(() => { + it('deduplicates notifications with the same title, type, and message', fakeAsync(() => { service.show(NotificationType.error, '502 - Bad Gateway', 'Error occurred in path a'); - tick(60); + tick(600); + service.show(NotificationType.error, '502 - Bad Gateway', 'Error occurred in path a'); + tick(600); + const notifications = service['dataSource'].getValue(); + expect(notifications.length).toBe(1); + expect(notifications[0].title).toBe('502 - Bad Gateway'); + expect(notifications[0].occurrences).toBe(2); + flush(); + })); + + it('does not deduplicate notifications with different messages', fakeAsync(() => { + service.show(NotificationType.error, '502 - Bad Gateway', 'Error occurred in path a'); + tick(600); service.show(NotificationType.error, '502 - Bad Gateway', 'Error occurred in path b'); - expectSavedNotificationToHave({ - type: NotificationType.error, - title: '502 - Bad Gateway', - message: 'Error occurred in path a (+1 more)' - }); + tick(600); + const notifications = service['dataSource'].getValue(); + expect(notifications.length).toBe(2); flush(); })); @@ -286,4 +296,65 @@ describe('NotificationService', () => { flush(); })); }); + + describe('Active toast limit', () => { + it('should show only the latest 2 toasts', fakeAsync(() => { + for (let i = 0; i < 5; i++) { + service.show(NotificationType.error, `Error ${i}`, `Message ${i}`); + tick(600); + } + const toasts = service['activeToastsSource'].getValue(); + expect(toasts.length).toBe(2); + expect(toasts[0].title).toBe('Error 4'); + expect(toasts[1].title).toBe('Error 3'); + flush(); + })); + }); + + describe('Read state', () => { + it('should mark a notification as read', () => { + const config = new CdNotificationConfig(NotificationType.info, 'Test'); + const notification = new CdNotification(config); + service.save(notification); + + expect(service['hasUnreadSource'].getValue()).toBe(true); + service.markAsRead(notification.id); + expect(service['hasUnreadSource'].getValue()).toBe(false); + }); + + it('should mark all notifications as read', () => { + const n1 = new CdNotification(new CdNotificationConfig(NotificationType.info, 'A')); + const n2 = new CdNotification(new CdNotificationConfig(NotificationType.error, 'B')); + service.save(n1); + service.save(n2); + + expect(service['hasUnreadSource'].getValue()).toBe(true); + service.markAllAsRead(); + expect(service['hasUnreadSource'].getValue()).toBe(false); + }); + + it('should clear read state when a notification gets a new occurrence', () => { + const n1 = new CdNotification(new CdNotificationConfig(NotificationType.error, 'Err', 'msg')); + service.save(n1); + service.markAsRead(n1.id); + expect(service['hasUnreadSource'].getValue()).toBe(false); + + const n2 = new CdNotification(new CdNotificationConfig(NotificationType.error, 'Err', 'msg')); + service.save(n2); + expect(service['hasUnreadSource'].getValue()).toBe(true); + expect(service['dataSource'].getValue()[0].occurrences).toBe(2); + }); + + it('should remove by id', () => { + const n1 = new CdNotification(new CdNotificationConfig(NotificationType.info, 'A')); + service.save(n1); + expect(service['dataSource'].getValue().length).toBe(1); + expect(service.removeById(n1.id)).toBe(true); + expect(service['dataSource'].getValue().length).toBe(0); + }); + + it('should return false when removing non-existent id', () => { + expect(service.removeById('nonexistent')).toBe(false); + }); + }); }); 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 98dc131c118..0af14122d0a 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 @@ -13,6 +13,18 @@ import { FinishedTask } from '../models/finished-task'; import { CdDatePipe } from '../pipes/cd-date.pipe'; import { TaskMessageService } from './task-message.service'; +// Carbon's cds-toast renders title, subtitle, and caption via [innerHTML]. +// Angular's DomSanitizer only partially protects against XSS in innerHTML bindings, +// so we escape untrusted input at the source before passing it to Carbon. +function escapeHtml(str: string): string { + return str + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); +} + @Injectable({ providedIn: 'root' }) @@ -24,6 +36,7 @@ export class NotificationService { [NotificationType.warning]: 'warning' }; private readonly MAX_NOTIFICATIONS = 10; + private readonly MAX_ACTIVE_TOASTS = 2; private readonly SHOW_DELAY = 10; private readonly QUEUE_DELAY = 500; private readonly ERROR_TOAST_DURATION = 10000; @@ -66,9 +79,21 @@ export class NotificationService { let notifications: CdNotification[] = []; if (_.isString(stringNotifications)) { try { + const ALLOWED_KEYS = [ + 'id', + 'title', + 'message', + 'type', + 'timestamp', + 'application', + 'prometheusAlert', + 'isFinishedTask', + 'alertSilenced', + 'silenceId' + ]; notifications = JSON.parse(stringNotifications, (_key, value) => { if (_.isPlainObject(value)) { - return _.assign(new CdNotification(), value); + return _.assign(new CdNotification(), _.pick(value, ALLOWED_KEYS)); } return value; }); @@ -90,7 +115,17 @@ export class NotificationService { } private _persistReadMap(readMap: Record) { - localStorage.setItem(this.LOCAL_STORAGE_READ_KEY, JSON.stringify(readMap)); + const ids = new Set(this.dataSource.getValue().map((n) => n.id)); + const pruned: Record = {}; + for (const id of Object.keys(readMap)) { + if (ids.has(id)) pruned[id] = true; + } + try { + localStorage.setItem(this.LOCAL_STORAGE_READ_KEY, JSON.stringify(pruned)); + } catch { + // quota exceeded — clear stale read state + localStorage.removeItem(this.LOCAL_STORAGE_READ_KEY); + } } private _recomputeHasUnread(notifications: CdNotification[]) { @@ -130,18 +165,30 @@ export class NotificationService { /** * Saving a shown notification in local storage */ - save(notification: CdNotification) { + save(notification: CdNotification): string { const current = this.dataSource.getValue(); const existing = current.find( - (n) => n.title === notification.title && n.type === notification.type + (n) => + n.title === notification.title && + n.type === notification.type && + n.message === notification.message ); let notifications: CdNotification[]; + let storedId: string; if (existing) { existing.occurrences = (existing.occurrences || 1) + 1; existing.timestamp = notification.timestamp; + existing.message = notification.message; + storedId = existing.id; notifications = [...current]; + + const readMap = { ...this.readMapSource.getValue() }; + delete readMap[existing.id]; + this.readMapSource.next(readMap); + this._persistReadMap(readMap); } else { + storedId = notification.id; notifications = [notification, ...current]; } @@ -152,6 +199,7 @@ export class NotificationService { this.dataSource.next(limited); this._recomputeHasUnread(limited); this._persistNotifications(limited); + return storedId; } /** @@ -240,42 +288,20 @@ export class NotificationService { } private _showQueued() { - this._getUnifiedTitleQueue().forEach((config) => { + this.queued.forEach((config) => { const notification = new CdNotification(config); if (!notification.isFinishedTask) { - this.save(notification); + const storedId = this.save(notification); + notification.id = storedId; } this._showToasty(notification); }); this.queued = []; } - private _getUnifiedTitleQueue(): CdNotificationConfig[] { - return Object.values(this._queueShiftByTitle()).map((configs) => { - const config = configs[0]; - if (configs.length > 1) { - config.message = '
    ' + configs.map((c) => `
  • ${c.message}
    • `).join('') + '
'; - } - return config; - }); - } - - private _queueShiftByTitle(): { [key: string]: CdNotificationConfig[] } { - const byTitle: { [key: string]: CdNotificationConfig[] } = {}; - let config: CdNotificationConfig; - while ((config = this.queued.shift())) { - if (!byTitle[config.title]) { - byTitle[config.title] = []; - } - byTitle[config.title].push(config); - } - return byTitle; - } - private _showToasty(notification: CdNotification) { - // Exit immediately if no toasty should be displayed. - if (this.hideToasties) { + if (this.hideToasties || this.panelState.value) { return; } @@ -283,8 +309,11 @@ export class NotificationService { const carbonType = this.NOTIFICATION_TYPE_MAP[notification.type] || 'info'; const lowContrast = notification.options?.lowContrast || false; + const escapedTitle = escapeHtml(notification.title || ''); + const escapedMessage = escapeHtml(notification.message || ''); const existing = this.activeToasts.find( - (t) => t.title === notification.title && t.type === carbonType + (t) => + t.title === escapedTitle && t.type === carbonType && t.originalSubtitle === escapedMessage ); if (existing) { existing.duplicateCount = (existing.duplicateCount || 1) + 1; @@ -295,10 +324,9 @@ export class NotificationService { return; } - const subtitle = notification.message || ''; const toast: ToastContent = { - title: notification.title, - subtitle, + title: escapedTitle, + subtitle: escapedMessage, caption: this._renderTimeAndApplicationHtml(notification), type: carbonType, lowContrast: lowContrast, @@ -310,10 +338,13 @@ export class NotificationService { : this.DEFAULT_TOAST_DURATION), notificationId: notification.id, duplicateCount: 1, - originalSubtitle: subtitle + originalSubtitle: escapedMessage }; this.activeToasts.unshift(toast); + while (this.activeToasts.length > this.MAX_ACTIVE_TOASTS) { + this.activeToasts.pop(); + } this.activeToastsSource.next(this.activeToasts); if (toast.duration && toast.duration > 0) { @@ -328,9 +359,11 @@ export class NotificationService { } private _renderTimeAndApplicationHtml(notification: CdNotification): string { + const date = escapeHtml(this.cdDatePipe.transform(notification.timestamp) || ''); + const id = encodeURIComponent(notification.id); return `
- ${this.cdDatePipe.transform(notification.timestamp)} - View more + ${date} + View more
`; } @@ -350,7 +383,7 @@ export class NotificationService { } removeToast(toast: ToastContent) { - this.activeToasts = this.activeToasts.filter((t) => !_.isEqual(t, toast)); + this.activeToasts = this.activeToasts.filter((t) => t.notificationId !== toast.notificationId); this.activeToastsSource.next(this.activeToasts); } @@ -404,10 +437,12 @@ export class NotificationService { */ togglePanel(isOpen: boolean) { this.panelState.next(isOpen); + if (isOpen) this.clearAllToasts(); } setPanelState(isOpen: boolean) { this.panelState.next(isOpen); + if (isOpen) this.clearAllToasts(); } getPanelState(): boolean {