refactor(NotificationService): batchProcess util, gql Notifications->list instead of ->data to get notifications

api/src/graphql/generated/api/operations.ts

@@ -2,7 +2,7 @@
 import * as Types from '@app/graphql/generated/api/types';
 
 import { z } from 'zod'
-import { AccessUrl, AccessUrlInput, AllowedOriginInput, ApiKey, ApiKeyResponse, ArrayType, ArrayCapacity, ArrayDisk, ArrayDiskFsColor, ArrayDiskStatus, ArrayDiskType, ArrayPendingState, ArrayState, Baseboard, Capacity, Case, Cloud, CloudResponse, Config, ConfigErrorState, Connect, ConnectSignInInput, ConnectUserInfoInput, ContainerHostConfig, ContainerMount, ContainerPort, ContainerPortType, ContainerState, Devices, Disk, DiskFsType, DiskInterfaceType, DiskPartition, DiskSmartStatus, Display, Docker, DockerContainer, DockerNetwork, DynamicRemoteAccessStatus, DynamicRemoteAccessType, EnableDynamicRemoteAccessInput, Flash, Gpu, Importance, Info, InfoApps, InfoCpu, InfoMemory, KeyFile, Me, MemoryFormFactor, MemoryLayout, MemoryType, MinigraphStatus, MinigraphqlResponse, Mount, Network, Node, Notification, NotificationCounts, NotificationData, NotificationFilter, NotificationOverview, NotificationType, Notifications, NotificationsdataArgs, Os, Owner, ParityCheck, Partition, Pci, ProfileModel, Registration, RegistrationState, RelayResponse, RemoteAccess, Server, ServerStatus, Service, SetupRemoteAccessInput, Share, System, Temperature, Theme, URL_TYPE, UnassignedDevice, Uptime, Usb, User, UserAccount, Vars, Versions, VmDomain, VmState, Vms, WAN_ACCESS_TYPE, WAN_FORWARD_TYPE, Welcome, addApiKeyInput, addUserInput, arrayDiskInput, authenticateInput, deleteUserInput, mdState, registrationType, updateApikeyInput, usersInput } from '@app/graphql/generated/api/types'
+import { AccessUrl, AccessUrlInput, AllowedOriginInput, ApiKey, ApiKeyResponse, ArrayType, ArrayCapacity, ArrayDisk, ArrayDiskFsColor, ArrayDiskStatus, ArrayDiskType, ArrayPendingState, ArrayState, Baseboard, Capacity, Case, Cloud, CloudResponse, Config, ConfigErrorState, Connect, ConnectSignInInput, ConnectUserInfoInput, ContainerHostConfig, ContainerMount, ContainerPort, ContainerPortType, ContainerState, Devices, Disk, DiskFsType, DiskInterfaceType, DiskPartition, DiskSmartStatus, Display, Docker, DockerContainer, DockerNetwork, DynamicRemoteAccessStatus, DynamicRemoteAccessType, EnableDynamicRemoteAccessInput, Flash, Gpu, Importance, Info, InfoApps, InfoCpu, InfoMemory, KeyFile, Me, MemoryFormFactor, MemoryLayout, MemoryType, MinigraphStatus, MinigraphqlResponse, Mount, Network, Node, Notification, NotificationCounts, NotificationData, NotificationFilter, NotificationOverview, NotificationType, Notifications, NotificationslistArgs, Os, Owner, ParityCheck, Partition, Pci, ProfileModel, Registration, RegistrationState, RelayResponse, RemoteAccess, Server, ServerStatus, Service, SetupRemoteAccessInput, Share, System, Temperature, Theme, URL_TYPE, UnassignedDevice, Uptime, Usb, User, UserAccount, Vars, Versions, VmDomain, VmState, Vms, WAN_ACCESS_TYPE, WAN_FORWARD_TYPE, Welcome, addApiKeyInput, addUserInput, arrayDiskInput, authenticateInput, deleteUserInput, mdState, registrationType, updateApikeyInput, usersInput } from '@app/graphql/generated/api/types'
 import { TypedDocumentNode as DocumentNode } from '@graphql-typed-document-node/core';
 
 type Properties<T> = Required<{
@@ -641,13 +641,13 @@ export function NotificationOverviewSchema(): z.ZodObject<Properties<Notificatio
 export function NotificationsSchema(): z.ZodObject<Properties<Notifications>> {
   return z.object({
     __typename: z.literal('Notifications').optional(),
-    data: z.array(NotificationSchema()),
     id: z.string(),
+    list: z.array(NotificationSchema()),
     overview: NotificationOverviewSchema()
   })
 }
 
-export function NotificationsdataArgsSchema(): z.ZodObject<Properties<NotificationsdataArgs>> {
+export function NotificationslistArgsSchema(): z.ZodObject<Properties<NotificationslistArgs>> {
   return z.object({
     filter: NotificationFilterSchema()
   })

api/src/graphql/generated/api/types.ts

@@ -862,13 +862,13 @@ export enum NotificationType {
 
 export type Notifications = Node & {
   __typename?: 'Notifications';
-  data: Array<Notification>;
   id: Scalars['ID']['output'];
+  list: Array<Notification>;
   overview: NotificationOverview;
 };
 
 
-export type NotificationsdataArgs = {
+export type NotificationslistArgs = {
   filter: NotificationFilter;
 };
 
@@ -2424,8 +2424,8 @@ export type NotificationOverviewResolvers<ContextType = Context, ParentType exte
 }>;
 
 export type NotificationsResolvers<ContextType = Context, ParentType extends ResolversParentTypes['Notifications'] = ResolversParentTypes['Notifications']> = ResolversObject<{
-  data?: Resolver<Array<ResolversTypes['Notification']>, ParentType, ContextType, RequireFields<NotificationsdataArgs, 'filter'>>;
   id?: Resolver<ResolversTypes['ID'], ParentType, ContextType>;
+  list?: Resolver<Array<ResolversTypes['Notification']>, ParentType, ContextType, RequireFields<NotificationslistArgs, 'filter'>>;
   overview?: Resolver<ResolversTypes['NotificationOverview'], ParentType, ContextType>;
   __isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;
 }>;

api/src/graphql/schema/types/notifications/notifications.graphql

@@ -41,7 +41,7 @@ enum Importance {
 type Notifications implements Node {
     id: ID!
     overview: NotificationOverview!
-    data(filter: NotificationFilter!): [Notification!]!
+    list(filter: NotificationFilter!): [Notification!]!
 }
 
 type Notification implements Node {

api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts

@@ -8,7 +8,6 @@ import { Args, Mutation, Query, ResolveField, Resolver, Subscription } from '@ne
 import { UseRoles } from 'nest-access-control';
 import { createSubscription, PUBSUB_CHANNEL } from '@app/core/pubsub';
 import { NotificationsService } from './notifications.service';
-import { getServerIdentifier } from '@app/core/utils/server-identifier';
 import { Importance } from '@app/graphql/generated/client/graphql';
 
 @Resolver('Notifications')
@@ -37,7 +36,7 @@ export class NotificationsResolver {
     }
 
     @ResolveField()
-    public async data(
+    public async list(
         @Args('filter')
         filters: NotificationFilter
     ) {

api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts

@@ -12,6 +12,7 @@ import {
     type NotificationFilter,
 } from '@app/graphql/generated/api/types';
 import { NotificationSchema } from '@app/graphql/generated/api/operations';
+import { mkdir } from 'fs/promises';
 
 // defined outside `describe` so it's defined inside the `beforeAll`
 // needed to mock the dynamix import
@@ -33,6 +34,7 @@ describe.sequential('NotificationsService', () => {
      *------------------------------------------------------------------------**/
 
     beforeAll(async () => {
+        await mkdir(basePath, { recursive: true });
         // need to mock the dynamix import bc the file watcher is init'ed in the service constructor
         // i.e. before we can mock service.paths()
         vi.mock(import('../../../../store'), async (importOriginal) => {
@@ -229,6 +231,8 @@ describe.sequential('NotificationsService', () => {
             expect(storedNotification[key]).toEqual(value);
         });
 
+        expect(service.getOverview().unread.total).toEqual(1);
+
         // notification was deleted
         await service.deleteNotification({ id: notification.id, type: NotificationType.UNREAD });
         const deleted = await findById(notification.id);

api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts

@@ -15,7 +15,7 @@ import { Injectable } from '@nestjs/common';
 import { mkdir, readdir, rename, rm, unlink, writeFile } from 'fs/promises';
 import { basename, join } from 'path';
 import { Logger } from '@nestjs/common';
-import { isFulfilled, isRejected, unraidTimestamp } from '@app/utils';
+import { batchProcess, isFulfilled, isRejected, unraidTimestamp } from '@app/utils';
 import { FSWatcher, watch } from 'chokidar';
 import { FileLoadStatus } from '@app/store/types';
 import { pubsub, PUBSUB_CHANNEL } from '@app/core/pubsub';
@@ -384,7 +384,6 @@ export class NotificationsService {
         const { UNREAD } = this.paths();
 
         if (!importance) {
-            // use arrow function to preserve `this`
             await readdir(UNREAD).then((ids) => this.archiveIds(ids));
             return { overview: NotificationsService.overview };
         }
@@ -398,7 +397,7 @@ export class NotificationsService {
             snapshot: overviewSnapshot,
         });
 
-        const stats = await this.updateMany(notifications, archive);
+        const stats = await batchProcess(notifications, archive);
         return { ...stats, overview: overviewSnapshot };
     }
 
@@ -420,7 +419,7 @@ export class NotificationsService {
             snapshot: overviewSnapshot,
         });
 
-        const stats = await this.updateMany(notifications, unArchive);
+        const stats = await batchProcess(notifications, unArchive);
         return { ...stats, overview: overviewSnapshot };
     }
 
@@ -435,7 +434,7 @@ export class NotificationsService {
      * @returns
      */
     public archiveIds(ids: string[]) {
-        return this.updateMany(ids, (id) => this.archiveNotification({ id }));
+        return batchProcess(ids, (id) => this.archiveNotification({ id }));
     }
 
     /**
@@ -449,30 +448,7 @@ export class NotificationsService {
      * @returns
      */
     public unarchiveIds(ids: string[]) {
-        return this.updateMany(ids, (id) => this.markAsUnread({ id }));
-    }
-
-    /**
-     * Wrapper for Promise-handling of batch operations based on
-     * notification ids.
-     *
-     * @param notificationIds
-     * @param action
-     * @returns
-     */
-    private async updateMany<Input, T>(notificationIds: Input[], action: (id: Input) => Promise<T>) {
-        const processes = notificationIds.map(action);
-
-        const results = await Promise.allSettled(processes);
-        const successes = results.filter(isFulfilled);
-        const errors = results.filter(isRejected).map((result) => result.reason);
-
-        return {
-            data: successes,
-            successes: successes.length,
-            errors: errors,
-            errorOccured: errors.length > 0,
-        };
+        return batchProcess(ids, (id) => this.markAsUnread({ id }));
     }
 
     /**------------------------------------------------------------------------
@@ -583,7 +559,7 @@ export class NotificationsService {
      * Takes a NotificationIni (ini file data) and a few details of a notification,
      * and combines them into a Notification object.
      *
-     * Does not validate the returned Notification object or the input file data.
+     * Does *not* validate the returned Notification object or the input file data.
      * This simply encapsulates data transformation logic.
      *
      * @param details The 'id' and 'type' of the notification to be combined.
@@ -595,9 +571,11 @@ export class NotificationsService {
         fileData: NotificationIni
     ): Notification {
         const { importance, timestamp, event: title, description = '', ...passthroughData } = fileData;
+        const { type, id } = details;
         return {
-            ...details,
             ...passthroughData,
+            id,
+            type,
             title,
             description,
             importance: this.fileImportanceToGqlImportance(importance),

api/src/utils.ts

@@ -33,4 +33,31 @@ export const secondsSinceUnixEpoch = (): number => Math.floor(Date.now() / 1_000
  *
  * @returns the number of seconds since Unix Epoch
  */
-export const unraidTimestamp = secondsSinceUnixEpoch;
+export const unraidTimestamp = secondsSinceUnixEpoch;
+
+/**
+ * Wrapper for Promise-handling of batch operations based on
+ * a list of items.
+ *
+ * @param items a list of items to process
+ * @param action an async function operating on an item from the list
+ * @returns
+ *   - data: return values from each successful action
+ *   - errors: list of errors (Promise Failure Reasons)
+ *   - successes: # of successful actions
+ *   - errorOccured: true if at least one error occurred
+ */
+export async function batchProcess<Input, T>(items: Input[], action: (id: Input) => Promise<T>) {
+    const processes = items.map(action);
+
+    const results = await Promise.allSettled(processes);
+    const successes = results.filter(isFulfilled);
+    const errors = results.filter(isRejected).map((result) => result.reason);
+
+    return {
+        data: successes,
+        successes: successes.length,
+        errors: errors,
+        errorOccured: errors.length > 0,
+    };
+}