diff --git a/api/src/graphql/generated/api/types.ts b/api/src/graphql/generated/api/types.ts index dad07f841..d32265b02 100644 --- a/api/src/graphql/generated/api/types.ts +++ b/api/src/graphql/generated/api/types.ts @@ -643,6 +643,8 @@ export type Mutation = { /** Pause parity check */ pauseParityCheck?: Maybe; reboot?: Maybe; + /** Reads each notification to recompute & update the overview. */ + recalculateOverview: NotificationOverview; /** Remove existing disk from array. NOTE: The array must be stopped before running this otherwise it'll throw an error. */ removeDiskFromArray?: Maybe; /** Resume parity check */ @@ -864,6 +866,7 @@ export type Notifications = Node & { __typename?: 'Notifications'; id: Scalars['ID']['output']; list: Array; + /** A cached overview of the notifications in the system & their severity. */ overview: NotificationOverview; }; @@ -2359,6 +2362,7 @@ export type MutationResolvers, ParentType, ContextType, RequireFields>; pauseParityCheck?: Resolver, ParentType, ContextType>; reboot?: Resolver, ParentType, ContextType>; + recalculateOverview?: Resolver; removeDiskFromArray?: Resolver, ParentType, ContextType, Partial>; resumeParityCheck?: Resolver, ParentType, ContextType>; setAdditionalAllowedOrigins?: Resolver, ParentType, ContextType, RequireFields>; diff --git a/api/src/graphql/schema/types/notifications/notifications.graphql b/api/src/graphql/schema/types/notifications/notifications.graphql index f6f6ae893..f0d0cb4e1 100644 --- a/api/src/graphql/schema/types/notifications/notifications.graphql +++ b/api/src/graphql/schema/types/notifications/notifications.graphql @@ -25,6 +25,8 @@ type Mutation { unarchiveNotifications(ids: [String!]): NotificationOverview! archiveAll(importance: Importance): NotificationOverview! unarchiveAll(importance: Importance): NotificationOverview! + """Reads each notification to recompute & update the overview.""" + recalculateOverview: NotificationOverview! } type Subscription { @@ -40,6 +42,7 @@ enum Importance { type Notifications implements Node { id: ID! + """A cached overview of the notifications in the system & their severity.""" overview: NotificationOverview! list(filter: NotificationFilter!): [Notification!]! } diff --git a/api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts b/api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts index 7d251c62c..30aa9e146 100644 --- a/api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts +++ b/api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts @@ -9,6 +9,7 @@ import { UseRoles } from 'nest-access-control'; import { createSubscription, PUBSUB_CHANNEL } from '@app/core/pubsub'; import { NotificationsService } from './notifications.service'; import { Importance } from '@app/graphql/generated/client/graphql'; +import { AppError } from '@app/core/errors/app-error'; @Resolver('Notifications') export class NotificationsResolver { @@ -101,6 +102,15 @@ export class NotificationsResolver { return overview; } + @Mutation() + public async recalculateOverview() { + const { overview, error } = await this.notificationsService.recalculateOverview(); + if (error) { + throw new AppError("Failed to refresh overview", 500); + } + return overview; + } + /**============================================ * Subscriptions *=============================================**/ diff --git a/api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts b/api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts index 90afbea1b..370eea667 100644 --- a/api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts +++ b/api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts @@ -1,6 +1,6 @@ import { Test, TestingModule } from '@nestjs/testing'; import { NotificationsService } from './notifications.service'; -import { describe, it, expect, vi, beforeAll, afterEach } from 'vitest'; +import { describe, it, expect, vi, beforeAll, afterEach, assert } from 'vitest'; import { existsSync } from 'fs'; import { Importance, @@ -216,6 +216,11 @@ describe.sequential('NotificationsService', () => { }; const notification = await createNotification(notificationData); + // HACK: we brute-force re-calculate instead of using service.getOverview() + // because the file-system-watcher's test setup isn't working rn. + let { overview } = await service.recalculateOverview(); + expect.soft(overview.unread.total).toEqual(1); + // data in returned notification (from createNotification) matches? Object.entries(notificationData).forEach(([key, value]) => { expect(notification[key]).toEqual(value); @@ -231,12 +236,13 @@ 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); expect(deleted).toBeUndefined(); + + ({ overview } = await service.recalculateOverview()); + expect.soft(overview.unread.total).toEqual(0); }); it.each(notificationImportance)('loadNotifications respects %s filter', async (importance) => { @@ -251,7 +257,9 @@ describe.sequential('NotificationsService', () => { createNotification({ importance: Importance.WARNING }), createNotification({ importance: Importance.WARNING }), ]); + const { overview } = await service.recalculateOverview(); expect(notifications.length).toEqual(9); + expect.soft(overview.unread.total).toEqual(9); // don't use the `expectIn` helper, just in case it changes const loaded = await service.getNotifications({ @@ -267,18 +275,29 @@ describe.sequential('NotificationsService', () => { * CRUD: Update Tests *---------------------------------------------**/ - it('can correctly archive and unarchive a notification', async ({ expect }) => { - await forEachImportance(async (importance) => { + it.for(notificationImportance.map((i) => [i]))( + 'can correctly archive and unarchive a %s notification', + async ([importance], { expect }) => { const notification = await createNotification({ importance }); + let { overview } = await service.recalculateOverview(); + expect.soft(overview.unread.total).toEqual(1); + expect.soft(overview.archive.total).toEqual(0); await service.archiveNotification(notification); let exists = await doesExist(expect)(notification, NotificationType.ARCHIVE); if (!exists) return; + ({ overview } = await service.recalculateOverview()); + expect.soft(overview.unread.total).toEqual(0); + expect.soft(overview.archive.total).toEqual(1); + await service.markAsUnread(notification); exists = await doesExist(expect)(notification, NotificationType.UNREAD); - }); - }); + ({ overview } = await service.recalculateOverview()); + expect.soft(overview.unread.total).toEqual(1); + expect.soft(overview.archive.total).toEqual(0); + } + ); it.each(notificationImportance)('can archiveAll & unarchiveAll %s', async (importance) => { const expectIn = makeExpectIn(expect); @@ -293,19 +312,35 @@ describe.sequential('NotificationsService', () => { createNotification({ importance: Importance.WARNING }), createNotification({ importance: Importance.WARNING }), ]); + expect(notifications.length).toEqual(9); await expectIn({ type: NotificationType.UNREAD }, 9); + let { overview } = await service.recalculateOverview(); + expect.soft(overview.unread.total).toEqual(9); + expect.soft(overview.archive.total).toEqual(0); await service.archiveAll(); await expectIn({ type: NotificationType.ARCHIVE }, 9); + ({ overview } = await service.recalculateOverview()); + expect.soft(overview.unread.total).toEqual(0); + expect.soft(overview.archive.total).toEqual(9); + await service.unarchiveAll(); await expectIn({ type: NotificationType.UNREAD }, 9); + ({ overview } = await service.recalculateOverview()); + expect.soft(overview.unread.total).toEqual(9); + expect.soft(overview.archive.total).toEqual(0); + await service.archiveAll(importance); await expectIn({ type: NotificationType.ARCHIVE }, 3); await expectIn({ type: NotificationType.UNREAD }, 6); + ({ overview } = await service.recalculateOverview()); + expect.soft(overview.unread.total).toEqual(6); + expect.soft(overview.archive.total).toEqual(3); + // archive another importance set, just to make sure unarchiveAll // isn't just ignoring the filter, which would be possible if it only // contained the stuff it was supposed to unarchive. @@ -315,16 +350,16 @@ describe.sequential('NotificationsService', () => { await expectIn({ type: NotificationType.ARCHIVE }, 6); await expectIn({ type: NotificationType.UNREAD }, 3); + ({ overview } = await service.recalculateOverview()); + expect.soft(overview.unread.total).toEqual(3); + expect.soft(overview.archive.total).toEqual(6); + await service.unarchiveAll(importance); await expectIn({ type: NotificationType.ARCHIVE }, 3); await expectIn({ type: NotificationType.UNREAD }, 6); - }); - /**======================================================================== - * Overview (Notification Stats) State - *========================================================================**/ - - it.skip('calculates stats correctly', async () => { - // todo implement + ({ overview } = await service.recalculateOverview()); + expect.soft(overview.unread.total).toEqual(6); + expect.soft(overview.archive.total).toEqual(3); }); }); diff --git a/api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts b/api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts index 34d9fdfa0..3d950ab56 100644 --- a/api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts +++ b/api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts @@ -132,6 +132,46 @@ export class NotificationsService { collector['total'] -= 1; } + public async recalculateOverview() { + const overview: NotificationOverview = { + unread: { + alert: 0, + info: 0, + warning: 0, + total: 0, + }, + archive: { + alert: 0, + info: 0, + warning: 0, + total: 0, + }, + }; + + // todo - refactor this to be more memory efficient + // i.e. by using a lazy generator vs the current eager implementation + // + // recalculates stats for a particular notification type + const recalculate = async (type: NotificationType) => { + const ids = await this.listFilesInFolder(this.paths()[type]); + const [notifications] = await this.loadNotificationsFromPaths(ids, {}); + notifications.forEach((n) => this.increment(n.importance, overview[type.toLowerCase()])); + }; + + const results = await batchProcess( + [NotificationType.ARCHIVE, NotificationType.UNREAD], + recalculate + ); + + if (results.errorOccured) { + results.errors.forEach((e) => this.logger.error('[recalculateOverview] ' + e)); + } + + NotificationsService.overview = overview; + void this.publishOverview(); + return { error: results.errorOccured, overview: this.getOverview() }; + } + /**------------------------------------------------------------------------ * CRUD: Creating Notifications *------------------------------------------------------------------------**/ @@ -538,7 +578,7 @@ export class NotificationsService { type: 'ini', }); - this.logger.debug(`Loaded notification ini file from ${path}}`); + this.logger.verbose(`Loaded notification ini file from ${path}}`); const notification: Notification = this.notificationFileToGqlNotification( { id: this.getIdFromPath(path), type },