Skip to content

Commit

Permalink
[ACS-5311] Notification History Bug Fix (#9011)
Browse files Browse the repository at this point in the history 
* [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
  • Loading branch information
@SheenaMalhotra182
SheenaMalhotra182 committed Nov 3, 2023
1 parent 3687cc5 commit 94fb615
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 38 deletions.
6 changes: 3 additions & 3 deletions lib/core/src/lib/notifications/components/notification-history.component.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
id="adf-notification-history-open-button"
(menuOpened)="onMenuOpened()">
<mat-icon matBadge="&#8288;"
[matBadgeHidden]="!notifications.length"
[matBadgeHidden]="!unreadNotifications.length"
matBadgeColor="accent"
matBadgeSize="small">notifications</mat-icon>
</button>
Expand All @@ -21,7 +21,7 @@
(click)="$event.stopPropagation()">
<div mat-subheader role="menuitem">
<span>{{ 'NOTIFICATIONS.TITLE' | translate }}</span>
<button *ngIf="notifications.length"
<button *ngIf="unreadNotifications.length"
id="adf-notification-history-mark-as-read"
mat-icon-button
title="{{ 'NOTIFICATIONS.MARK_AS_READ' | translate }}"
Expand All @@ -33,7 +33,7 @@
<mat-divider></mat-divider>

<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"
class="adf-notification-history-menu-item"
(click)="onNotificationClick(notification)">
Expand Down
66 changes: 56 additions & 10 deletions lib/core/src/lib/notifications/components/notification-history.component.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { OverlayContainer } from '@angular/cdk/overlay';
import { NotificationService } from '../services/notification.service';
import { StorageService } from '../../common/services/storage.service';
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', () => {

Expand All @@ -32,11 +32,12 @@ describe('Notification History Component', () => {
let notificationService: NotificationService;
let overlayContainerElement: HTMLElement;
let storage: StorageService;
let testNotifications: NotificationModel[];

const openNotification = () => {
fixture.detectChanges();
const button = element.querySelector<HTMLButtonElement>('#adf-notification-history-open-button');
button.click();
button?.click();
fixture.detectChanges();
};

Expand All @@ -54,14 +55,34 @@ describe('Notification History Component', () => {
storage = TestBed.inject(StorageService);
notificationService = TestBed.inject(NotificationService);
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) => {
overlayContainerElement = oc.getContainerElement();
}));

afterEach(() => {
storage.removeItem(NotificationHistoryComponent.NOTIFICATION_STORAGE);
storage.removeItem(NOTIFICATION_STORAGE);
fixture.destroy();
});

Expand All @@ -83,12 +104,11 @@ describe('Notification History Component', () => {
fixture.detectChanges();
fixture.whenStable().then(() => {
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');
markAllAsRead.click();
markAllAsRead?.click();
fixture.detectChanges();
expect(storage.getItem(NotificationHistoryComponent.NOTIFICATION_STORAGE)).toBeNull();
expect(component.notifications.length).toBe(0);
expect(component.unreadNotifications).toEqual([]);
done();
});
});
Expand All @@ -101,7 +121,7 @@ describe('Notification History Component', () => {
fixture.whenStable().then(() => {
fixture.detectChanges();
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();
});
});
Expand All @@ -123,7 +143,7 @@ describe('Notification History Component', () => {
fixture.whenStable().then(() => {
fixture.detectChanges();
const notification = overlayContainerElement.querySelector<HTMLButtonElement>('.adf-notification-history-menu-item');
notification.click();
notification?.click();
expect(callBackSpy).toHaveBeenCalled();
done();
});
Expand All @@ -148,7 +168,7 @@ describe('Notification History Component', () => {
});

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'],
datetime: new Date(),
type: NOTIFICATION_TYPE.RECURSIVE
Expand Down Expand Up @@ -182,4 +202,30 @@ describe('Notification History Component', () => {
});
}, 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);
});
});
49 changes: 27 additions & 22 deletions lib/core/src/lib/notifications/components/notification-history.component.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

import { Component, Input, ViewChild, OnDestroy, OnInit, AfterViewInit, ChangeDetectorRef, ViewEncapsulation } from '@angular/core';
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 { takeUntil } from 'rxjs/operators';
import { Subject } from 'rxjs';
Expand All @@ -33,7 +33,6 @@ import { PaginationModel } from '../../models/pagination.model';
export class NotificationHistoryComponent implements OnDestroy, OnInit, AfterViewInit {

public static MAX_NOTIFICATION_STACK_LENGTH = 100;
public static NOTIFICATION_STORAGE = 'notification-history';

@ViewChild(MatMenuTrigger, { static: true })
trigger: MatMenuTrigger;
Expand All @@ -53,17 +52,17 @@ export class NotificationHistoryComponent implements OnDestroy, OnInit, AfterVie
onDestroy$ = new Subject<boolean>();
notifications: NotificationModel[] = [];
paginatedNotifications: NotificationModel[] = [];
unreadNotifications: NotificationModel[] = [];
pagination: PaginationModel;

constructor(
private notificationService: NotificationService,
public storageService: StorageService,
public cd: ChangeDetectorRef) {

}
constructor(private notificationService: NotificationService, public storageService: StorageService, public cd: ChangeDetectorRef) {}

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 {
Expand All @@ -81,20 +80,22 @@ export class NotificationHistoryComponent implements OnDestroy, OnInit, AfterVie
}

addNewNotification(notification: NotificationModel) {
this.notifications.unshift(notification);
this.unreadNotifications.unshift(notification);

if (this.notifications.length > NotificationHistoryComponent.MAX_NOTIFICATION_STACK_LENGTH) {
this.notifications.shift();
if (this.unreadNotifications.length > NotificationHistoryComponent.MAX_NOTIFICATION_STACK_LENGTH) {
this.unreadNotifications.shift();
}

this.saveNotifications();
this.createPagination();
}

saveNotifications() {
this.storageService.setItem(NotificationHistoryComponent.NOTIFICATION_STORAGE, JSON.stringify(this.notifications.filter((notification) =>
notification.type !== NOTIFICATION_TYPE.RECURSIVE
)));
this.unreadNotifications.forEach((notification: NotificationModel) => {
this.notifications.push(notification);
});

this.storageService.setItem(NOTIFICATION_STORAGE, JSON.stringify(this.notifications));
}

onMenuOpened() {
Expand All @@ -112,9 +113,13 @@ export class NotificationHistoryComponent implements OnDestroy, OnInit, AfterVie
}

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.storageService.removeItem(NotificationHistoryComponent.NOTIFICATION_STORAGE);
this.createPagination();
this.trigger.closeMenu();
}
Expand All @@ -123,16 +128,16 @@ export class NotificationHistoryComponent implements OnDestroy, OnInit, AfterVie
this.pagination = {
skipCount: this.maxNotifications,
maxItems: this.maxNotifications,
totalItems: this.notifications.length,
hasMoreItems: this.notifications.length > this.maxNotifications
totalItems: this.unreadNotifications.length,
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() {
this.pagination.skipCount = this.pagination.maxItems + this.pagination.skipCount;
this.pagination.hasMoreItems = this.notifications.length > this.pagination.skipCount;
this.paginatedNotifications = this.notifications.slice(0, this.pagination.skipCount);
this.pagination.hasMoreItems = this.unreadNotifications.length > this.pagination.skipCount;
this.paginatedNotifications = this.unreadNotifications.slice(0, this.pagination.skipCount);
}

hasMoreNotifications(): boolean {
Expand Down
9 changes: 6 additions & 3 deletions lib/core/src/lib/notifications/helpers/notification.factory.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,24 @@ export const info = (messages: string | string[], initiator: NotificationInitiat
icon: 'info',
datetime: new Date(),
initiator,
messages: [].concat(messages)
messages: [].concat(messages),
read: false
});

export const warning = (messages: string | string[], initiator: NotificationInitiator = rootInitiator): NotificationModel => ({
type: NOTIFICATION_TYPE.WARN,
icon: 'warning',
datetime: new Date(),
initiator,
messages: [].concat(messages)
messages: [].concat(messages),
read: false
});

export const error = (messages: string | string[], initiator: NotificationInitiator = rootInitiator): NotificationModel => ({
type: NOTIFICATION_TYPE.ERROR,
icon: 'error',
datetime: new Date(),
initiator,
messages: [].concat(messages)
messages: [].concat(messages),
read: false
});
3 changes: 3 additions & 0 deletions lib/core/src/lib/notifications/models/notification.model.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,7 @@ export interface NotificationModel {
icon?: string;
clickCallBack?: any;
args?: any;
read?: boolean;
}

export const NOTIFICATION_STORAGE = 'notification-history';

0 comments on commit 94fb615

Please sign in to comment.