feat: Update Notifications front-end

Signed-off-by: Marek Libra <marek.libra@gmail.com>
backstage · Feb 21, 2024 · 985e5e2 · 985e5e2
1 parent 42ee6b9
commit 985e5e2
Show file tree

Hide file tree

Showing 16 changed files with 537 additions and 403 deletions.
diff --git a/.changeset/tender-carrots-care.md b/.changeset/tender-carrots-care.md
@@ -0,0 +1,7 @@
+---
+'@backstage/plugin-notifications-backend': minor
+'@backstage/plugin-notifications': minor
+'@backstage/plugin-notifications-common': patch
+---
+
+Notifications frontend has been updated towards expectations of the BEP 001
diff --git a/plugins/notifications-backend/migrations/20231215_init.js b/plugins/notifications-backend/migrations/20231215_init.js
@@ -21,7 +21,7 @@ exports.up = async function up(knex) {
     table.string('title').notNullable();
     table.text('description').nullable();
     table.string('severity', 8).notNullable();
-    table.text('link').notNullable();
+    table.text('link').nullable();
     table.string('origin', 255).notNullable();
     table.string('scope', 255).nullable();
     table.string('topic', 255).nullable();

diff --git a/plugins/notifications-backend/migrations/20240221_removeDone.js b/plugins/notifications-backend/migrations/20240221_removeDone.js
@@ -0,0 +1,25 @@
+/*
+ * Copyright 2024 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+exports.up = async function up(knex) {
+  await knex.schema.alterTable('notification', table => {
+    table.dropColumn('done');
+  });
+};
+
+exports.down = async function down(knex) {
+  await knex.schema.dropTable('notification');
+};
diff --git a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts
@@ -62,7 +62,6 @@ describe.each(databases.eachSupportedId())(
     const insertNotification = async (
       notification: Partial<Notification> & {
         id: string;
-        done?: Date;
         saved?: Date;
         read?: Date;
       },
@@ -78,7 +77,6 @@ describe.each(databases.eachSupportedId())(
             title: notification.payload?.title,
             severity: notification.payload?.severity,
             scope: notification.payload?.scope,
-            done: notification.done,
             saved: notification.saved,
             read: notification.read,
           })
@@ -106,42 +104,64 @@ describe.each(databases.eachSupportedId())(
         expect(notifications.length).toBe(2);
       });
 
-      it('should return undone notifications for user', async () => {
+      it('should return read notifications for user', async () => {
         const id1 = uuid();
         const id2 = uuid();
-        await insertNotification({
-          id: id1,
-          ...testNotification,
-          done: new Date(),
+        const id3 = uuid();
+        await insertNotification({ id: id1, ...testNotification });
+        await insertNotification({ id: id2, ...testNotification });
+        await insertNotification({ id: id3, ...testNotification });
+        await insertNotification({ id: uuid(), ...otherUserNotification });
+
+        await storage.markRead({ ids: [id1, id3], user });
+
+        const notifications = await storage.getNotifications({
+          user,
+          read: true,
         });
+        expect(notifications.length).toBe(2);
+        expect(notifications.at(0)?.id).toEqual(id1);
+        expect(notifications.at(1)?.id).toEqual(id3);
+      });
+
+      it('should return unread notifications for user', async () => {
+        const id1 = uuid();
+        const id2 = uuid();
+        const id3 = uuid();
+        await insertNotification({ id: id1, ...testNotification });
         await insertNotification({ id: id2, ...testNotification });
+        await insertNotification({ id: id3, ...testNotification });
         await insertNotification({ id: uuid(), ...otherUserNotification });
 
+        await storage.markRead({ ids: [id1, id3], user });
+
         const notifications = await storage.getNotifications({
           user,
-          type: 'undone',
+          read: false,
         });
         expect(notifications.length).toBe(1);
         expect(notifications.at(0)?.id).toEqual(id2);
       });
 
-      it('should return done notifications for user', async () => {
+      it('should return both read and unread notifications for user', async () => {
         const id1 = uuid();
         const id2 = uuid();
-        await insertNotification({
-          id: id1,
-          ...testNotification,
-          done: new Date(),
-        });
+        const id3 = uuid();
+        await insertNotification({ id: id1, ...testNotification });
         await insertNotification({ id: id2, ...testNotification });
+        await insertNotification({ id: id3, ...testNotification });
         await insertNotification({ id: uuid(), ...otherUserNotification });
 
+        await storage.markRead({ ids: [id1, id3], user });
+
         const notifications = await storage.getNotifications({
           user,
-          type: 'done',
+          read: undefined,
         });
-        expect(notifications.length).toBe(1);
+        expect(notifications.length).toBe(3);
         expect(notifications.at(0)?.id).toEqual(id1);
+        expect(notifications.at(1)?.id).toEqual(id2);
+        expect(notifications.at(2)?.id).toEqual(id3);
       });
 
       it('should allow searching for notifications', async () => {
@@ -218,7 +238,6 @@ describe.each(databases.eachSupportedId())(
           ...testNotification,
           id: id1,
           read: new Date(),
-          done: new Date(),
           payload: {
             title: 'Notification',
             link: '/scaffolder/task/1234',
@@ -242,7 +261,6 @@ describe.each(databases.eachSupportedId())(
         expect(existing).not.toBeNull();
         expect(existing?.id).toEqual(id1);
         expect(existing?.payload.title).toEqual('New notification');
-        expect(existing?.done).toBeNull();
         expect(existing?.read).toBeNull();
       });
     });
@@ -283,32 +301,6 @@ describe.each(databases.eachSupportedId())(
       });
     });
 
-    describe('markDone', () => {
-      it('should mark notification done', async () => {
-        const id1 = uuid();
-        await insertNotification({ id: id1, ...testNotification });
-
-        await storage.markDone({ ids: [id1], user });
-        const notification = await storage.getNotification({ id: id1 });
-        expect(notification?.done).not.toBeNull();
-      });
-    });
-
-    describe('markUndone', () => {
-      it('should mark notification undone', async () => {
-        const id1 = uuid();
-        await insertNotification({
-          id: id1,
-          ...testNotification,
-          done: new Date(),
-        });
-
-        await storage.markUndone({ ids: [id1], user });
-        const notification = await storage.getNotification({ id: id1 });
-        expect(notification?.done).toBeNull();
-      });
-    });
-
     describe('markSaved', () => {
       it('should mark notification saved', async () => {
         const id1 = uuid();
@@ -334,5 +326,26 @@ describe.each(databases.eachSupportedId())(
         expect(notification?.saved).toBeNull();
       });
     });
+
+    describe('saveNotification', () => {
+      it('should store a notification', async () => {
+        const id1 = uuid();
+        await storage.saveNotification({
+          id: id1,
+          user,
+          created: new Date(),
+          origin: 'my-origin',
+          payload: {
+            title: 'My title One',
+            description: 'a description of the notification',
+            link: 'http://foo.bar',
+            severity: 'normal',
+            topic: 'my-topic',
+          },
+        });
+        const notification = await storage.getNotification({ id: id1 });
+        expect(notification?.payload?.title).toBe('My title One');
+      });
+    });
   },
 );
diff --git a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts
@@ -78,10 +78,27 @@ export class DatabaseNotificationsStore implements NotificationsStore {
     }));
   };
 
+  private mapNotificationToDbRow = (notification: Notification) => {
+    return {
+      id: notification.id,
+      user: notification.user,
+      origin: notification.origin,
+      created: notification.created,
+      topic: notification.payload?.topic,
+      link: notification.payload?.link,
+      title: notification.payload?.title,
+      description: notification.payload?.description,
+      severity: notification.payload?.severity,
+      scope: notification.payload?.scope,
+      saved: notification.saved,
+      read: notification.read,
+    };
+  };
+
   private getNotificationsBaseQuery = (
     options: NotificationGetOptions | NotificationModifyOptions,
   ) => {
-    const { user, type } = options;
+    const { user } = options;
     const query = this.db('notification').where('user', user);
 
     if (options.sort !== undefined && options.sort !== null) {
@@ -90,14 +107,6 @@ export class DatabaseNotificationsStore implements NotificationsStore {
       query.orderBy('created', options.sortOrder ?? 'desc');
     }
 
-    if (type === 'undone') {
-      query.whereNull('done');
-    } else if (type === 'done') {
-      query.whereNotNull('done');
-    } else if (type === 'saved') {
-      query.whereNotNull('saved');
-    }
-
     if (options.limit) {
       query.limit(options.limit);
     }
@@ -117,6 +126,18 @@ export class DatabaseNotificationsStore implements NotificationsStore {
       query.whereIn('notification.id', options.ids);
     }
 
+    if (options.read) {
+      query.whereNotNull('notification.read');
+    } else if (options.read === false) {
+      query.whereNull('notification.read');
+    } // or match both if undefined
+
+    if (options.saved) {
+      query.whereNotNull('notification.saved');
+    } else if (options.saved === false) {
+      query.whereNull('notification.saved');
+    } // or match both if undefined
+
     return query;
   };
 
@@ -127,7 +148,9 @@ export class DatabaseNotificationsStore implements NotificationsStore {
   }
 
   async saveNotification(notification: Notification) {
-    await this.db.insert(notification).into('notification');
+    await this.db
+      .insert(this.mapNotificationToDbRow(notification))
+      .into('notification');
   }
 
   async getStatus(options: NotificationGetOptions) {
@@ -188,10 +211,9 @@ export class DatabaseNotificationsStore implements NotificationsStore {
       description: options.notification.payload.description,
       link: options.notification.payload.link,
       topic: options.notification.payload.topic,
-      updated: options.notification.created,
+      updated: new Date(),
       severity: options.notification.payload.severity,
       read: null,
-      done: null,
     });
 
     return await this.getNotification(options);
@@ -218,16 +240,6 @@ export class DatabaseNotificationsStore implements NotificationsStore {
     await notificationQuery.update({ read: null });
   }
 
-  async markDone(options: NotificationModifyOptions): Promise<void> {
-    const notificationQuery = this.getNotificationsBaseQuery(options);
-    await notificationQuery.update({ done: new Date(), read: new Date() });
-  }
-
-  async markUndone(options: NotificationModifyOptions): Promise<void> {
-    const notificationQuery = this.getNotificationsBaseQuery(options);
-    await notificationQuery.update({ done: null, read: null });
-  }
-
   async markSaved(options: NotificationModifyOptions): Promise<void> {
     const notificationQuery = this.getNotificationsBaseQuery(options);
     await notificationQuery.update({ saved: new Date() });

diff --git a/plugins/notifications-backend/src/database/NotificationsStore.ts b/plugins/notifications-backend/src/database/NotificationsStore.ts
@@ -17,19 +17,20 @@
 import {
   Notification,
   NotificationStatus,
-  NotificationType,
 } from '@backstage/plugin-notifications-common';
 
+// TODO: reuse the common part of the type with front-end
 /** @internal */
 export type NotificationGetOptions = {
   user: string;
   ids?: string[];
-  type?: NotificationType;
   offset?: number;
   limit?: number;
   search?: string;
   sort?: 'created' | 'read' | 'updated' | null;
   sortOrder?: 'asc' | 'desc';
+  read?: boolean;
+  saved?: boolean;
 };
 
 /** @internal */
@@ -62,10 +63,6 @@ export interface NotificationsStore {
 
   markUnread(options: NotificationModifyOptions): Promise<void>;
 
-  markDone(options: NotificationModifyOptions): Promise<void>;
-
-  markUndone(options: NotificationModifyOptions): Promise<void>;
-
   markSaved(options: NotificationModifyOptions): Promise<void>;
 
   markUnsaved(options: NotificationModifyOptions): Promise<void>;

diff --git a/plugins/notifications-backend/src/service/router.ts b/plugins/notifications-backend/src/service/router.ts
@@ -45,7 +45,6 @@ import {
   NewNotificationSignal,
   Notification,
   NotificationReadSignal,
-  NotificationType,
 } from '@backstage/plugin-notifications-common';
 
 /** @internal */
@@ -191,9 +190,6 @@ export async function createRouter(
     const opts: NotificationGetOptions = {
       user: user,
     };
-    if (req.query.type) {
-      opts.type = req.query.type.toString() as NotificationType;
-    }
     if (req.query.offset) {
       opts.offset = Number.parseInt(req.query.offset.toString(), 10);
     }
@@ -203,6 +199,12 @@ export async function createRouter(
     if (req.query.search) {
       opts.search = req.query.search.toString();
     }
+    if (req.query.read === 'true') {
+      opts.read = true;
+    } else if (req.query.read === 'false') {
+      opts.read = false;
+      // or keep undefined
+    }
 
     const notifications = await store.getNotifications(opts);
     res.send(notifications);
@@ -225,7 +227,7 @@ export async function createRouter(
 
   router.get('/status', async (req, res) => {
     const user = await getUser(req);
-    const status = await store.getStatus({ user, type: 'undone' });
+    const status = await store.getStatus({ user });
     res.send(status);
   });