[ACS-5311] Notification History Bug Fix (#9011)

* [ACS-5311] implemented read state for notifications and showing only unread notifications in history menu

* [ACS-5311] linting fixes

* [ACS-5311] linting fixes

* [ACS-5311] fixed code smell

* [ACS-5311] addressed review comments

* [ACS-5311] addressed review comments

* [ACS-5311] updated unit tests

* [ACS-5311] addressed review comments and optimized unit tests

* [ACS-5311] linting fix

* [ACS-5311] addressed review comments

* [ACS-5311] fixed typos

* [ACS-5311] moved NOTIFICATION_STORAGE to notification.model.ts to reuse across repos

* [ACS-5311] addressed review comments
This commit is contained in:
SheenaMalhotra182
2023-11-03 20:11:40 +05:30
committed by GitHub
parent 3687cc58e0
commit 94fb61541c
5 changed files with 95 additions and 38 deletions

View File

@@ -7,7 +7,7 @@
id="adf-notification-history-open-button" id="adf-notification-history-open-button"
(menuOpened)="onMenuOpened()"> (menuOpened)="onMenuOpened()">
<mat-icon matBadge="&#8288;" <mat-icon matBadge="&#8288;"
[matBadgeHidden]="!notifications.length" [matBadgeHidden]="!unreadNotifications.length"
matBadgeColor="accent" matBadgeColor="accent"
matBadgeSize="small">notifications</mat-icon> matBadgeSize="small">notifications</mat-icon>
</button> </button>
@@ -21,7 +21,7 @@
(click)="$event.stopPropagation()"> (click)="$event.stopPropagation()">
<div mat-subheader role="menuitem"> <div mat-subheader role="menuitem">
<span>{{ 'NOTIFICATIONS.TITLE' | translate }}</span> <span>{{ 'NOTIFICATIONS.TITLE' | translate }}</span>
<button *ngIf="notifications.length" <button *ngIf="unreadNotifications.length"
id="adf-notification-history-mark-as-read" id="adf-notification-history-mark-as-read"
mat-icon-button mat-icon-button
title="{{ 'NOTIFICATIONS.MARK_AS_READ' | translate }}" title="{{ 'NOTIFICATIONS.MARK_AS_READ' | translate }}"
@@ -33,7 +33,7 @@
<mat-divider></mat-divider> <mat-divider></mat-divider>
<mat-list role="menuitem"> <mat-list role="menuitem">
<ng-container *ngIf="notifications.length; else empty_list_template"> <ng-container *ngIf="unreadNotifications.length; else empty_list_template">
<mat-list-item *ngFor="let notification of paginatedNotifications" <mat-list-item *ngFor="let notification of paginatedNotifications"
class="adf-notification-history-menu-item" class="adf-notification-history-menu-item"
(click)="onNotificationClick(notification)"> (click)="onNotificationClick(notification)">

View File

@@ -22,7 +22,7 @@ import { OverlayContainer } from '@angular/cdk/overlay';
import { NotificationService } from '../services/notification.service'; import { NotificationService } from '../services/notification.service';
import { StorageService } from '../../common/services/storage.service'; import { StorageService } from '../../common/services/storage.service';
import { TranslateModule } from '@ngx-translate/core'; import { TranslateModule } from '@ngx-translate/core';
import { NotificationModel, NOTIFICATION_TYPE } from '../models/notification.model'; import { NotificationModel, NOTIFICATION_TYPE, NOTIFICATION_STORAGE } from '../models/notification.model';
describe('Notification History Component', () => { describe('Notification History Component', () => {
@@ -32,11 +32,12 @@ describe('Notification History Component', () => {
let notificationService: NotificationService; let notificationService: NotificationService;
let overlayContainerElement: HTMLElement; let overlayContainerElement: HTMLElement;
let storage: StorageService; let storage: StorageService;
let testNotifications: NotificationModel[];
const openNotification = () => { const openNotification = () => {
fixture.detectChanges(); fixture.detectChanges();
const button = element.querySelector<HTMLButtonElement>('#adf-notification-history-open-button'); const button = element.querySelector<HTMLButtonElement>('#adf-notification-history-open-button');
button.click(); button?.click();
fixture.detectChanges(); fixture.detectChanges();
}; };
@@ -54,6 +55,26 @@ describe('Notification History Component', () => {
storage = TestBed.inject(StorageService); storage = TestBed.inject(StorageService);
notificationService = TestBed.inject(NotificationService); notificationService = TestBed.inject(NotificationService);
component.notifications = []; component.notifications = [];
component.unreadNotifications = [];
testNotifications = [
{
type: NOTIFICATION_TYPE.INFO,
icon: 'info',
datetime: new Date(),
initiator: { key: '*', displayName: 'SYSTEM' },
messages: ['Moved 1 item.'],
read: false
},
{
type: NOTIFICATION_TYPE.INFO,
icon: 'info',
datetime: new Date(),
initiator: { key: '*', displayName: 'SYSTEM' },
messages: ['Copied 1 item.'],
read: false
}
];
}); });
beforeEach(inject([OverlayContainer], (oc: OverlayContainer) => { beforeEach(inject([OverlayContainer], (oc: OverlayContainer) => {
@@ -61,7 +82,7 @@ describe('Notification History Component', () => {
})); }));
afterEach(() => { afterEach(() => {
storage.removeItem(NotificationHistoryComponent.NOTIFICATION_STORAGE); storage.removeItem(NOTIFICATION_STORAGE);
fixture.destroy(); fixture.destroy();
}); });
@@ -83,12 +104,11 @@ describe('Notification History Component', () => {
fixture.detectChanges(); fixture.detectChanges();
fixture.whenStable().then(() => { fixture.whenStable().then(() => {
fixture.detectChanges(); fixture.detectChanges();
expect(component.notifications.length).toBe(1); expect(component.unreadNotifications.length).toBe(1);
const markAllAsRead = overlayContainerElement.querySelector<HTMLButtonElement>('#adf-notification-history-mark-as-read'); const markAllAsRead = overlayContainerElement.querySelector<HTMLButtonElement>('#adf-notification-history-mark-as-read');
markAllAsRead.click(); markAllAsRead?.click();
fixture.detectChanges(); fixture.detectChanges();
expect(storage.getItem(NotificationHistoryComponent.NOTIFICATION_STORAGE)).toBeNull(); expect(component.unreadNotifications).toEqual([]);
expect(component.notifications.length).toBe(0);
done(); done();
}); });
}); });
@@ -101,7 +121,7 @@ describe('Notification History Component', () => {
fixture.whenStable().then(() => { fixture.whenStable().then(() => {
fixture.detectChanges(); fixture.detectChanges();
expect(overlayContainerElement.querySelector('#adf-notification-history-component-no-message')).toBeNull(); expect(overlayContainerElement.querySelector('#adf-notification-history-component-no-message')).toBeNull();
expect(overlayContainerElement.querySelector('.adf-notification-history-list').innerHTML).toContain('Example Message'); expect(overlayContainerElement.querySelector('.adf-notification-history-list')?.innerHTML).toContain('Example Message');
done(); done();
}); });
}); });
@@ -123,7 +143,7 @@ describe('Notification History Component', () => {
fixture.whenStable().then(() => { fixture.whenStable().then(() => {
fixture.detectChanges(); fixture.detectChanges();
const notification = overlayContainerElement.querySelector<HTMLButtonElement>('.adf-notification-history-menu-item'); const notification = overlayContainerElement.querySelector<HTMLButtonElement>('.adf-notification-history-menu-item');
notification.click(); notification?.click();
expect(callBackSpy).toHaveBeenCalled(); expect(callBackSpy).toHaveBeenCalled();
done(); done();
}); });
@@ -148,7 +168,7 @@ describe('Notification History Component', () => {
}); });
it('should read notifications from local storage', (done) => { it('should read notifications from local storage', (done) => {
storage.setItem(NotificationHistoryComponent.NOTIFICATION_STORAGE, JSON.stringify([{ storage.setItem(NOTIFICATION_STORAGE, JSON.stringify([{
messages: ['My new message'], messages: ['My new message'],
datetime: new Date(), datetime: new Date(),
type: NOTIFICATION_TYPE.RECURSIVE type: NOTIFICATION_TYPE.RECURSIVE
@@ -182,4 +202,30 @@ describe('Notification History Component', () => {
}); });
}, 45000); }, 45000);
}); });
it('should set unreadNotifications to an empty array when there are no notifications', () => {
component.notifications = [];
fixture.detectChanges();
expect(component.unreadNotifications).toEqual([]);
});
it('should set unreadNotifications to an empty array when all notifications are read', () => {
testNotifications.forEach((notification: NotificationModel) => {
notification.read = true;
});
storage.setItem(NOTIFICATION_STORAGE, JSON.stringify(testNotifications));
fixture.detectChanges();
expect(component.unreadNotifications).toEqual([]);
});
it('should set unreadNotifications by filtering notifications where read is false', () => {
testNotifications[0].read = true;
storage.setItem(NOTIFICATION_STORAGE, JSON.stringify(testNotifications));
fixture.detectChanges();
expect(component.unreadNotifications.length).toEqual(1);
expect(component.unreadNotifications[0].read).toEqual(false);
});
}); });

View File

@@ -17,7 +17,7 @@
import { Component, Input, ViewChild, OnDestroy, OnInit, AfterViewInit, ChangeDetectorRef, ViewEncapsulation } from '@angular/core'; import { Component, Input, ViewChild, OnDestroy, OnInit, AfterViewInit, ChangeDetectorRef, ViewEncapsulation } from '@angular/core';
import { NotificationService } from '../services/notification.service'; import { NotificationService } from '../services/notification.service';
import { NotificationModel, NOTIFICATION_TYPE } from '../models/notification.model'; import { NOTIFICATION_STORAGE, NotificationModel } from '../models/notification.model';
import { MatMenuTrigger, MenuPositionX, MenuPositionY } from '@angular/material/menu'; import { MatMenuTrigger, MenuPositionX, MenuPositionY } from '@angular/material/menu';
import { takeUntil } from 'rxjs/operators'; import { takeUntil } from 'rxjs/operators';
import { Subject } from 'rxjs'; import { Subject } from 'rxjs';
@@ -33,7 +33,6 @@ import { PaginationModel } from '../../models/pagination.model';
export class NotificationHistoryComponent implements OnDestroy, OnInit, AfterViewInit { export class NotificationHistoryComponent implements OnDestroy, OnInit, AfterViewInit {
public static MAX_NOTIFICATION_STACK_LENGTH = 100; public static MAX_NOTIFICATION_STACK_LENGTH = 100;
public static NOTIFICATION_STORAGE = 'notification-history';
@ViewChild(MatMenuTrigger, { static: true }) @ViewChild(MatMenuTrigger, { static: true })
trigger: MatMenuTrigger; trigger: MatMenuTrigger;
@@ -53,17 +52,17 @@ export class NotificationHistoryComponent implements OnDestroy, OnInit, AfterVie
onDestroy$ = new Subject<boolean>(); onDestroy$ = new Subject<boolean>();
notifications: NotificationModel[] = []; notifications: NotificationModel[] = [];
paginatedNotifications: NotificationModel[] = []; paginatedNotifications: NotificationModel[] = [];
unreadNotifications: NotificationModel[] = [];
pagination: PaginationModel; pagination: PaginationModel;
constructor( constructor(private notificationService: NotificationService, public storageService: StorageService, public cd: ChangeDetectorRef) {}
private notificationService: NotificationService,
public storageService: StorageService,
public cd: ChangeDetectorRef) {
}
ngOnInit() { ngOnInit() {
this.notifications = JSON.parse(this.storageService.getItem(NotificationHistoryComponent.NOTIFICATION_STORAGE)) || []; this.notifications = JSON.parse(this.storageService.getItem(NOTIFICATION_STORAGE)) || [];
this.unreadNotifications =
JSON.parse(this.storageService.getItem(NOTIFICATION_STORAGE))?.filter(
(notification: NotificationModel) => !notification.read
) || [];
} }
ngAfterViewInit(): void { ngAfterViewInit(): void {
@@ -81,10 +80,10 @@ export class NotificationHistoryComponent implements OnDestroy, OnInit, AfterVie
} }
addNewNotification(notification: NotificationModel) { addNewNotification(notification: NotificationModel) {
this.notifications.unshift(notification); this.unreadNotifications.unshift(notification);
if (this.notifications.length > NotificationHistoryComponent.MAX_NOTIFICATION_STACK_LENGTH) { if (this.unreadNotifications.length > NotificationHistoryComponent.MAX_NOTIFICATION_STACK_LENGTH) {
this.notifications.shift(); this.unreadNotifications.shift();
} }
this.saveNotifications(); this.saveNotifications();
@@ -92,9 +91,11 @@ export class NotificationHistoryComponent implements OnDestroy, OnInit, AfterVie
} }
saveNotifications() { saveNotifications() {
this.storageService.setItem(NotificationHistoryComponent.NOTIFICATION_STORAGE, JSON.stringify(this.notifications.filter((notification) => this.unreadNotifications.forEach((notification: NotificationModel) => {
notification.type !== NOTIFICATION_TYPE.RECURSIVE this.notifications.push(notification);
))); });
this.storageService.setItem(NOTIFICATION_STORAGE, JSON.stringify(this.notifications));
} }
onMenuOpened() { onMenuOpened() {
@@ -112,9 +113,13 @@ export class NotificationHistoryComponent implements OnDestroy, OnInit, AfterVie
} }
markAsRead() { markAsRead() {
this.notifications = []; this.unreadNotifications = [];
this.notifications.forEach((notification: NotificationModel) => {
notification.read = true;
});
this.storageService.setItem(NOTIFICATION_STORAGE, JSON.stringify(this.notifications));
this.paginatedNotifications = []; this.paginatedNotifications = [];
this.storageService.removeItem(NotificationHistoryComponent.NOTIFICATION_STORAGE);
this.createPagination(); this.createPagination();
this.trigger.closeMenu(); this.trigger.closeMenu();
} }
@@ -123,16 +128,16 @@ export class NotificationHistoryComponent implements OnDestroy, OnInit, AfterVie
this.pagination = { this.pagination = {
skipCount: this.maxNotifications, skipCount: this.maxNotifications,
maxItems: this.maxNotifications, maxItems: this.maxNotifications,
totalItems: this.notifications.length, totalItems: this.unreadNotifications.length,
hasMoreItems: this.notifications.length > this.maxNotifications hasMoreItems: this.unreadNotifications.length > this.maxNotifications
}; };
this.paginatedNotifications = this.notifications.slice(0, this.pagination.skipCount); this.paginatedNotifications = this.unreadNotifications.slice(0, this.pagination.skipCount);
} }
loadMore() { loadMore() {
this.pagination.skipCount = this.pagination.maxItems + this.pagination.skipCount; this.pagination.skipCount = this.pagination.maxItems + this.pagination.skipCount;
this.pagination.hasMoreItems = this.notifications.length > this.pagination.skipCount; this.pagination.hasMoreItems = this.unreadNotifications.length > this.pagination.skipCount;
this.paginatedNotifications = this.notifications.slice(0, this.pagination.skipCount); this.paginatedNotifications = this.unreadNotifications.slice(0, this.pagination.skipCount);
} }
hasMoreNotifications(): boolean { hasMoreNotifications(): boolean {

View File

@@ -31,7 +31,8 @@ export const info = (messages: string | string[], initiator: NotificationInitiat
icon: 'info', icon: 'info',
datetime: new Date(), datetime: new Date(),
initiator, initiator,
messages: [].concat(messages) messages: [].concat(messages),
read: false
}); });
export const warning = (messages: string | string[], initiator: NotificationInitiator = rootInitiator): NotificationModel => ({ export const warning = (messages: string | string[], initiator: NotificationInitiator = rootInitiator): NotificationModel => ({
@@ -39,7 +40,8 @@ export const warning = (messages: string | string[], initiator: NotificationInit
icon: 'warning', icon: 'warning',
datetime: new Date(), datetime: new Date(),
initiator, initiator,
messages: [].concat(messages) messages: [].concat(messages),
read: false
}); });
export const error = (messages: string | string[], initiator: NotificationInitiator = rootInitiator): NotificationModel => ({ export const error = (messages: string | string[], initiator: NotificationInitiator = rootInitiator): NotificationModel => ({
@@ -47,5 +49,6 @@ export const error = (messages: string | string[], initiator: NotificationInitia
icon: 'error', icon: 'error',
datetime: new Date(), datetime: new Date(),
initiator, initiator,
messages: [].concat(messages) messages: [].concat(messages),
read: false
}); });

View File

@@ -39,4 +39,7 @@ export interface NotificationModel {
icon?: string; icon?: string;
clickCallBack?: any; clickCallBack?: any;
args?: any; args?: any;
read?: boolean;
} }
export const NOTIFICATION_STORAGE = 'notification-history';