1
0
mirror of https://github.com/bitwarden/browser synced 2026-03-01 11:01:17 +00:00

fix(notification-processing): [PM-19877] System Notification Implementation - Addressed more feedback.

This commit is contained in:
Patrick Pimentel
2025-08-08 13:10:28 -04:00
parent 81281a2b6a
commit 49bef8ee3c
49 changed files with 112 additions and 140 deletions

View File

@@ -5,17 +5,17 @@ import { NotificationId, UserId } from "@bitwarden/common/types/guid";
import { NotificationView } from "../models";
/**
* A service for retrieving and managing notifications for end users.
* A service for retrieving and managing server notifications for end users.
*/
export abstract class EndUserNotificationService {
/**
* Observable of all notifications for the given user.
* Observable of all server notifications for the given user.
* @param userId
*/
abstract notifications$(userId: UserId): Observable<NotificationView[]>;
/**
* Observable of all unread notifications for the given user.
* Observable of all unread server notifications for the given user.
* @param userId
*/
abstract unreadNotifications$(userId: UserId): Observable<NotificationView[]>;
@@ -35,13 +35,13 @@ export abstract class EndUserNotificationService {
abstract markAsDeleted(notificationId: NotificationId, userId: UserId): Promise<void>;
/**
* Clear all notifications from state for the given user.
* Clear all server notifications from state for the given user.
* @param userId
*/
abstract clearState(userId: UserId): Promise<void>;
/**
* Creates a subscription to listen for end user push notifications and notification status updates.
* Creates a subscription to listen for end user push server notifications and notification status updates.
*/
abstract listenForEndUserNotifications(): Subscription;
}

View File

@@ -4,7 +4,7 @@ import { firstValueFrom, of } from "rxjs";
import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { ServerNotificationsService } from "@bitwarden/common/platform/notifications";
import { ServerNotificationsService } from "@bitwarden/common/platform/server-notifications";
import { StateProvider } from "@bitwarden/common/platform/state";
import { NotificationId, UserId } from "@bitwarden/common/types/guid";
@@ -48,7 +48,7 @@ describe("End User Notification Center Service", () => {
});
describe("notifications$", () => {
it("should return notifications from state when not null", async () => {
it("should return server notifications from state when not null", async () => {
fakeStateProvider.singleUser.mockFor("user-id" as UserId, NOTIFICATIONS, [
{
id: "notification-id" as NotificationId,
@@ -62,7 +62,7 @@ describe("End User Notification Center Service", () => {
expect(mockLogService.warning).not.toHaveBeenCalled();
});
it("should return notifications API when state is null", async () => {
it("should return server notifications API when state is null", async () => {
mockApiService.send.mockResolvedValue({
data: [
{
@@ -86,7 +86,7 @@ describe("End User Notification Center Service", () => {
expect(mockLogService.warning).not.toHaveBeenCalled();
});
it("should log a warning if there are more notifications available", async () => {
it("should log a warning if there are more server notifications available", async () => {
mockApiService.send.mockResolvedValue({
data: [
...new Array(DEFAULT_NOTIFICATION_PAGE_SIZE + 1).fill({ id: "notification-id" }),
@@ -120,7 +120,7 @@ describe("End User Notification Center Service", () => {
});
describe("unreadNotifications$", () => {
it("should return unread notifications from state when read value is null", async () => {
it("should return unread server notifications from state when read value is null", async () => {
fakeStateProvider.singleUser.mockFor("user-id" as UserId, NOTIFICATIONS, [
{
id: "notification-id" as NotificationId,
@@ -136,7 +136,7 @@ describe("End User Notification Center Service", () => {
});
describe("getNotifications", () => {
it("should call getNotifications returning notifications from API", async () => {
it("should call getNotifications returning server notifications from API", async () => {
mockApiService.send.mockResolvedValue({
data: [
{
@@ -156,7 +156,7 @@ describe("End User Notification Center Service", () => {
);
});
it("should update local state when notifications are updated", async () => {
it("should update local state when server notifications are updated", async () => {
mockApiService.send.mockResolvedValue({
data: [
{

View File

@@ -6,7 +6,7 @@ import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authenticatio
import { NotificationType } from "@bitwarden/common/enums";
import { ListResponse } from "@bitwarden/common/models/response/list.response";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { ServerNotificationsService } from "@bitwarden/common/platform/notifications";
import { ServerNotificationsService } from "@bitwarden/common/platform/server-notifications";
import { StateProvider } from "@bitwarden/common/platform/state";
import { NotificationId, UserId } from "@bitwarden/common/types/guid";
import {
@@ -19,7 +19,7 @@ import { NotificationView, NotificationViewData, NotificationViewResponse } from
import { NOTIFICATIONS } from "../state/end-user-notification.state";
/**
* The default number of notifications to fetch from the API.
* The default number of server notifications to fetch from the API.
*/
export const DEFAULT_NOTIFICATION_PAGE_SIZE = 50;
@@ -30,7 +30,7 @@ const getLoggedInUserIds = map<Record<UserId, AuthenticationStatus>, UserId[]>((
);
/**
* A service for retrieving and managing notifications for end users.
* A service for retrieving and managing server notifications for end users.
*/
export class DefaultEndUserNotificationService implements EndUserNotificationService {
constructor(
@@ -100,7 +100,7 @@ export class DefaultEndUserNotificationService implements EndUserNotificationSer
}
/**
* Helper observable to filter notifications by the notification type and user ids
* Helper observable to filter server notifications by the notification type and user ids
* Returns EMPTY if no user ids are provided
* @param userIds
* @private
@@ -121,7 +121,7 @@ export class DefaultEndUserNotificationService implements EndUserNotificationSer
}
/**
* Creates a subscription to listen for end user push notifications and notification status updates.
* Creates a subscription to listen for end user push server notifications and notification status updates.
*/
listenForEndUserNotifications(): Subscription {
return this.authService.authStatuses$
@@ -139,7 +139,7 @@ export class DefaultEndUserNotificationService implements EndUserNotificationSer
}
/**
* Fetches the notifications from the API and updates the local state
* Fetches the server notifications from the API and updates the local state
* @param userId
* @private
*/
@@ -164,7 +164,7 @@ export class DefaultEndUserNotificationService implements EndUserNotificationSer
}
/**
* Replaces the local state with notifications and returns the updated state
* Replaces the local state with server notifications and returns the updated state
* @param userId
* @param notifications
* @private
@@ -178,7 +178,7 @@ export class DefaultEndUserNotificationService implements EndUserNotificationSer
/**
* Updates the local state adding the new notification or updates an existing one with the same id
* Returns the entire updated notifications state
* Returns the entire updated server notifications state
* @param userId
* @param notification
* @private
@@ -203,7 +203,7 @@ export class DefaultEndUserNotificationService implements EndUserNotificationSer
}
/**
* Returns the local state for notifications
* Returns the local state for server notifications
* @param userId
* @private
*/

View File

@@ -45,7 +45,7 @@ export abstract class TaskService {
abstract markAsComplete(taskId: SecurityTaskId, userId: UserId): Promise<void>;
/**
* Creates a subscription for pending security task notifications or completed syncs for unlocked users.
* Creates a subscription for pending security task server notifications or completed syncs for unlocked users.
*/
abstract listenForTaskNotifications(): Subscription;
}

View File

@@ -8,7 +8,7 @@ import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authenticatio
import { NotificationType } from "@bitwarden/common/enums";
import { NotificationResponse } from "@bitwarden/common/models/response/notification.response";
import { Message, MessageListener } from "@bitwarden/common/platform/messaging";
import { ServerNotificationsService } from "@bitwarden/common/platform/notifications";
import { ServerNotificationsService } from "@bitwarden/common/platform/server-notifications";
import { SecurityTaskId, UserId } from "@bitwarden/common/types/guid";
import { FakeStateProvider, mockAccountServiceWith } from "../../../../spec";
@@ -304,7 +304,7 @@ describe("Default task service", () => {
});
describe("listenForTaskNotifications()", () => {
it("should not subscribe to notifications when there are no unlocked users", () => {
it("should not subscribe to server notifications when there are no unlocked users", () => {
mockAuthStatuses$.next({
[userId]: AuthenticationStatus.Locked,
});
@@ -320,7 +320,7 @@ describe("Default task service", () => {
subscription.unsubscribe();
});
it("should not subscribe to notifications when no users have tasks enabled", () => {
it("should not subscribe to server notifications when no users have tasks enabled", () => {
mockAuthStatuses$.next({
[userId]: AuthenticationStatus.Unlocked,
});
@@ -336,7 +336,7 @@ describe("Default task service", () => {
subscription.unsubscribe();
});
it("should subscribe to notifications when there are unlocked users with tasks enabled", () => {
it("should subscribe to server notifications when there are unlocked users with tasks enabled", () => {
mockAuthStatuses$.next({
[userId]: AuthenticationStatus.Unlocked,
});
@@ -378,7 +378,7 @@ describe("Default task service", () => {
subscription.unsubscribe();
});
it("should ignore notifications for other users", async () => {
it("should ignore server notifications for other users", async () => {
mockAuthStatuses$.next({
[userId]: AuthenticationStatus.Unlocked,
});
@@ -403,7 +403,7 @@ describe("Default task service", () => {
subscription.unsubscribe();
});
it("should ignore other notifications types", async () => {
it("should ignore other server notifications types", async () => {
mockAuthStatuses$.next({
[userId]: AuthenticationStatus.Unlocked,
});

View File

@@ -17,7 +17,7 @@ import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authenticatio
import { NotificationType } from "@bitwarden/common/enums";
import { ListResponse } from "@bitwarden/common/models/response/list.response";
import { MessageListener } from "@bitwarden/common/platform/messaging";
import { ServerNotificationsService } from "@bitwarden/common/platform/notifications";
import { ServerNotificationsService } from "@bitwarden/common/platform/server-notifications";
import { StateProvider } from "@bitwarden/common/platform/state";
import { SecurityTaskId, UserId } from "@bitwarden/common/types/guid";
import {
@@ -171,7 +171,7 @@ export class DefaultTaskService implements TaskService {
}
/**
* Creates a subscription for pending security task notifications or completed syncs for unlocked users.
* Creates a subscription for pending security task server notifications or completed syncs for unlocked users.
*/
listenForTaskNotifications(): Subscription {
return this.authService.authStatuses$