diff --git a/api/generated-schema.graphql b/api/generated-schema.graphql index c697f98867..a04250969f 100644 --- a/api/generated-schema.graphql +++ b/api/generated-schema.graphql @@ -2138,6 +2138,9 @@ type NotificationCounts { type NotificationOverview { unread: NotificationCounts! archive: NotificationCounts! + + """Counts for persistent ("Active") condition-style notifications.""" + active: NotificationCounts! } type Notification implements Node { @@ -2149,6 +2152,14 @@ type Notification implements Node { description: String! importance: NotificationImportance! link: String + + """Stable key for condition-style notifications (idempotent raise / clear-by-key).""" + key: String + + """ + Whether this notification persists until its condition is resolved. Persistent notifications are not user-archivable. + """ + persistent: Boolean! type: NotificationType! """ISO Timestamp for when the notification occurred""" @@ -2165,6 +2176,7 @@ enum NotificationImportance { enum NotificationType { UNREAD ARCHIVE + ACTIVE } type Notifications implements Node { @@ -3401,6 +3413,16 @@ type Mutation { notifyIfUnique(input: NotificationData!): Notification archiveAll(importance: NotificationImportance): NotificationOverview! + """ + Clears all unread notifications that share a stable producer key. Used to resolve condition-style (persistent) notifications when their condition no longer holds. + """ + clearNotificationByKey(key: String!): NotificationOverview! + + """ + Reconciles JS-sourced banner notifications: clears any unread "banner-" keyed notification not stamped with the supplied current page-load generation (i.e. a banner the producer stopped rendering). + """ + reconcileBannerNotifications(currentGeneration: String!): NotificationOverview! + """Marks a notification as unread.""" unreadNotification(id: PrefixedID!): Notification! unarchiveNotifications(ids: [PrefixedID!]!): NotificationOverview! @@ -3468,6 +3490,16 @@ input NotificationData { description: String! importance: NotificationImportance! link: String + + """ + Stable key for a condition-style notification. Raising again with the same key replaces the existing one; clear it with clearNotificationByKey when the condition resolves. + """ + key: String + + """ + Persistent notifications cannot be archived by the user; they stay until cleared programmatically (typically via their key) when the underlying condition resolves. + """ + persistent: Boolean = false } input UpdateSshInput { diff --git a/api/src/core/types/states/notification.ts b/api/src/core/types/states/notification.ts index b773ec31b7..77153e2ab6 100644 --- a/api/src/core/types/states/notification.ts +++ b/api/src/core/types/states/notification.ts @@ -5,4 +5,15 @@ export interface NotificationIni { description: string; importance: 'normal' | 'alert' | 'warning'; link?: string; + /** Stable producer key for condition-style notifications (idempotent raise / clear-by-key). */ + key?: string; + /** 'true' when the notification is persistent (not user-archivable). Stored as a string in the ini file. */ + persistent?: string; + /** + * Per-page-load generation stamp for JS-sourced banner notifications (keys prefixed + * 'banner-'). The page re-raises active banners on every load with the current + * generation; reconcileBannerNotifications clears any 'banner-' entry whose gen no + * longer matches, i.e. one the producer stopped rendering. + */ + gen?: string; } diff --git a/api/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.ts b/api/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.ts index 09b3663e45..69d994933d 100644 --- a/api/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.ts +++ b/api/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.ts @@ -121,8 +121,10 @@ describe('NotificationsService - loadNotificationFile (minimal mocks)', () => { }); it('creates the notifications watcher without replaying existing files', () => { + // The configured path (testNotificationsDir) is not the tmpfs default, so the + // always-off-disk active dir falls outside it and is watched as a second path. expect(mockWatch).toHaveBeenCalledWith( - testNotificationsDir, + [testNotificationsDir, '/tmp/notifications/active'], expect.objectContaining({ ignoreInitial: true, }) @@ -177,6 +179,32 @@ describe('NotificationsService - loadNotificationFile (minimal mocks)', () => { expect(processNotificationAdd).toHaveBeenCalledWith(bufferedPath); }); + it('debounces a single authoritative recalc when notification files are unlinked', async () => { + vi.useFakeTimers(); + try { + const recalc = vi + .spyOn(service, 'recalculateOverview') + .mockResolvedValue({ error: false, overview: service.getOverview() }); + Reflect.set(service, 'publishWarningsAndAlerts', vi.fn().mockResolvedValue(undefined)); + const handleUnlink = ( + Reflect.get(service, 'handleNotificationUnlink') as (path: string) => void + ).bind(service); + + // Non-notification files are ignored entirely. + handleUnlink(`${testNotificationsDir}/active/not-a-notification.txt`); + // A burst of real removals collapses into one rebuild. + handleUnlink(`${testNotificationsDir}/active/a.notify`); + handleUnlink(`${testNotificationsDir}/active/b.notify`); + handleUnlink(`${testNotificationsDir}/unread/c.notify`); + + expect(recalc).not.toHaveBeenCalled(); // still within the debounce window + await vi.advanceTimersByTimeAsync(200); + expect(recalc).toHaveBeenCalledTimes(1); + } finally { + vi.useRealTimers(); + } + }); + it('should load and validate a valid notification file', async () => { const mockFileContent = `timestamp=1609459200 event=Test Event @@ -242,7 +270,7 @@ importance=not-a-valid-enum`; expect(result.description).toBe('Test Description'); }); - it('should handle missing description field (should return masked warning notification)', async () => { + it('should allow a missing description field (empty is valid, not masked)', async () => { const mockFileContent = `timestamp=1609459200 event=Test Event subject=Test Subject @@ -254,9 +282,13 @@ importance=normal`; '/test/path/test.notify', NotificationType.UNREAD ); - // Should be a masked warning notification - expect(result.description).toContain('invalid and cannot be displayed'); - expect(result.importance).toBe(NotificationImportance.WARNING); + // A missing description is allowed: condition/banner notifications carry their + // meaning in the title + Active badge, and the UI hides the empty line. It must + // not be masked as invalid. + expect(result.id).toBe('test.notify'); + expect(result.description).toBe(''); + expect(result.description).not.toContain('invalid and cannot be displayed'); + expect(result.importance).toBe(NotificationImportance.INFO); }); it('should preserve passthrough data from notification file (only known fields)', async () => { diff --git a/api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts b/api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts index 64dd0893a1..1fe59fd9f5 100644 --- a/api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts +++ b/api/src/unraid-api/graph/resolvers/notifications/notifications.model.ts @@ -1,11 +1,14 @@ import { Field, InputType, Int, ObjectType, registerEnumType } from '@nestjs/graphql'; import { Node } from '@unraid/shared/graphql.model.js'; -import { IsEnum, IsInt, IsNotEmpty, IsOptional, IsString, Min } from 'class-validator'; +import { IsBoolean, IsEnum, IsInt, IsNotEmpty, IsOptional, IsString, Min } from 'class-validator'; export enum NotificationType { UNREAD = 'UNREAD', ARCHIVE = 'ARCHIVE', + // Persistent ("Active") condition-style notifications. Stored separately; they + // stay until their producer clears them and are never archived by the user. + ACTIVE = 'ACTIVE', } export enum NotificationImportance { @@ -60,9 +63,11 @@ export class NotificationData { @IsNotEmpty() subject!: string; + // Description is optional in practice (e.g. condition/banner notifications carry + // their meaning in the title + Active badge). Allow empty so they aren't masked + // as invalid; the UI hides the line when empty. @Field() @IsString() - @IsNotEmpty() description!: string; @Field(() => NotificationImportance) @@ -74,6 +79,25 @@ export class NotificationData { @IsString() @IsOptional() link?: string; + + @Field({ + nullable: true, + description: + 'Stable key for a condition-style notification. Raising again with the same key replaces the existing one; clear it with clearNotificationByKey when the condition resolves.', + }) + @IsString() + @IsOptional() + key?: string; + + @Field({ + nullable: true, + defaultValue: false, + description: + 'Persistent notifications cannot be archived by the user; they stay until cleared programmatically (typically via their key) when the underlying condition resolves.', + }) + @IsBoolean() + @IsOptional() + persistent?: boolean; } @ObjectType('NotificationCounts') @@ -108,6 +132,12 @@ export class NotificationOverview { @Field(() => NotificationCounts) @IsNotEmpty() archive!: NotificationCounts; + + @Field(() => NotificationCounts, { + description: 'Counts for persistent ("Active") condition-style notifications.', + }) + @IsNotEmpty() + active!: NotificationCounts; } @ObjectType({ implements: () => Node }) @@ -122,9 +152,11 @@ export class Notification extends Node { @IsNotEmpty() subject!: string; + // Description is optional in practice (e.g. condition/banner notifications carry + // their meaning in the title + Active badge). Allow empty so they aren't masked + // as invalid; the UI hides the line when empty. @Field() @IsString() - @IsNotEmpty() description!: string; @Field(() => NotificationImportance) @@ -137,6 +169,21 @@ export class Notification extends Node { @IsOptional() link?: string; + @Field({ + nullable: true, + description: 'Stable key for condition-style notifications (idempotent raise / clear-by-key).', + }) + @IsString() + @IsOptional() + key?: string; + + @Field({ + description: + 'Whether this notification persists until its condition is resolved. Persistent notifications are not user-archivable.', + }) + @IsBoolean() + persistent!: boolean; + @Field(() => NotificationType) @IsEnum(NotificationType) @IsNotEmpty() 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 d3e0c6797b..45646100d0 100644 --- a/api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts +++ b/api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts @@ -124,6 +124,28 @@ export class NotificationsResolver { return overview; } + @Mutation(() => NotificationOverview, { + description: + 'Clears all unread notifications that share a stable producer key. Used to resolve condition-style (persistent) notifications when their condition no longer holds.', + }) + public clearNotificationByKey( + @Args('key', { type: () => String }) + key: string + ): Promise { + return this.notificationsService.clearNotificationsByKey(key); + } + + @Mutation(() => NotificationOverview, { + description: + 'Reconciles JS-sourced banner notifications: clears any unread "banner-" keyed notification not stamped with the supplied current page-load generation (i.e. a banner the producer stopped rendering).', + }) + public reconcileBannerNotifications( + @Args('currentGeneration', { type: () => String }) + currentGeneration: string + ): Promise { + return this.notificationsService.reconcileBannerNotifications(currentGeneration); + } + @Mutation(() => Notification, { description: 'Marks a notification as unread.' }) public unreadNotification( @Args('id', { type: () => PrefixedID }) 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 dcf624a278..9f31c194b5 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 @@ -10,9 +10,10 @@ import type { TestingModule } from '@nestjs/testing'; import { ConfigService } from '@nestjs/config'; import { Test } from '@nestjs/testing'; import { existsSync } from 'fs'; -import { mkdir } from 'fs/promises'; +import { mkdir, writeFile } from 'fs/promises'; import { execa } from 'execa'; +import { emptyDir } from 'fs-extra'; import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; import { NotificationIni } from '@app/core/types/states/notification.js'; @@ -44,6 +45,12 @@ const zeroOverview = (): NotificationOverview => ({ warning: 0, total: 0, }, + active: { + alert: 0, + info: 0, + warning: 0, + total: 0, + }, }); async function disableNotificationsWatcher() { @@ -64,6 +71,8 @@ describe.sequential('NotificationsService', () => { basePath, UNREAD: `${basePath}/unread`, ARCHIVE: `${basePath}/archive`, + ACTIVE: `${basePath}/active`, + active: `${basePath}/active`, }; /**------------------------------------------------------------------------ @@ -88,13 +97,23 @@ describe.sequential('NotificationsService', () => { await disableNotificationsWatcher(); vi.spyOn(service, 'paths').mockImplementation(() => testPaths); - await service.deleteAllNotifications(); + // Hard-wipe the test dirs directly: deleteAllNotifications intentionally + // preserves persistent notifications, which would otherwise leak between tests. + await Promise.all([ + emptyDir(testPaths.UNREAD), + emptyDir(testPaths.ARCHIVE), + emptyDir(testPaths.active), + ]); }); // make sure each test is isolated (as much as possible) afterEach(async () => { Reflect.set(NotificationsService, 'overview', zeroOverview()); - await service.deleteAllNotifications(); + await Promise.all([ + emptyDir(testPaths.UNREAD), + emptyDir(testPaths.ARCHIVE), + emptyDir(testPaths.active), + ]); }); /**------------------------------------------------------------------------ @@ -107,8 +126,10 @@ describe.sequential('NotificationsService', () => { subject = 'Test Subject', description = 'Test Description', importance = NotificationImportance.INFO, + persistent, + key, } = data; - return service.createNotification({ title, subject, description, importance }); + return service.createNotification({ title, subject, description, importance, persistent, key }); } async function findById(id: string, type: NotificationType = NotificationType.UNREAD) { @@ -509,6 +530,132 @@ describe.sequential('NotificationsService', () => { expect.soft(overview.unread.total).toEqual(6); expect.soft(overview.archive.total).toEqual(3); }); + + it('archiveAll never archives persistent notifications', async () => { + const expectIn = makeExpectIn(expect); + await Promise.all([ + createNotification({ importance: NotificationImportance.WARNING }), + createNotification({ importance: NotificationImportance.INFO }), + createNotification({ + importance: NotificationImportance.WARNING, + persistent: true, + key: 'persistent-condition', + }), + ]); + + // 2 transient land in UNREAD; the persistent one lands in ACTIVE. + await expectIn({ type: NotificationType.UNREAD }, 2); + await expectIn({ type: NotificationType.ACTIVE }, 1); + + // Unfiltered "Archive all" archives the transient unread and leaves ACTIVE alone. + await service.archiveAll(); + await expectIn({ type: NotificationType.UNREAD }, 0); + await expectIn({ type: NotificationType.ARCHIVE }, 2); + await expectIn({ type: NotificationType.ACTIVE }, 1); + + // The per-importance variant also never touches ACTIVE. + await service.archiveAll(NotificationImportance.WARNING); + await expectIn({ type: NotificationType.ACTIVE }, 1); + }); + + it('reconcileBannerNotifications clears only stale banner-* notifications', async () => { + // Persistent (banner/keyed) notifications live in the 'active' store; the `gen` + // stamp is written by the legacy notify script, so write files directly there to + // exercise the reconcile read path. + const writeNotify = (id: string, key: string, gen: string) => + writeFile( + `${testPaths.active}/${id}.notify`, + [ + 'timestamp=1700000000', + 'event="Test"', + 'subject="Test"', + 'description="Test"', + 'importance="warning"', + `key="${key}"`, + 'persistent="true"', + `gen="${gen}"`, + ].join('\n') + '\n' + ); + + // Two banners stamped with an old generation, one re-raised with the current + // generation, plus a non-banner keyed (lifecycle-managed) notification. + await writeNotify('banner_aaa', 'banner-aaa', '100'); + await writeNotify('banner_bbb', 'banner-bbb', '100'); + await writeNotify('banner_ccc', 'banner-ccc', '200'); + await writeNotify('webgui_pr_1', 'webgui-pr-1', '100'); + + await service.reconcileBannerNotifications('200'); + + // Stale banners cleared; current-gen banner and non-banner key survive (in ACTIVE). + const remaining = await service.getNotifications({ + type: NotificationType.ACTIVE, + offset: 0, + limit: 50, + }); + const keys = remaining.map((n) => n.key).sort(); + expect(keys).toEqual(['banner-ccc', 'webgui-pr-1']); + }); + + it('serves persistent under ACTIVE and transient under UNREAD', async () => { + await createNotification({ + key: 'webgui-pr-1', + persistent: true, + importance: NotificationImportance.WARNING, + }); + for (let i = 0; i < 3; i++) { + await createNotification({ importance: NotificationImportance.INFO }); + } + + const active = await service.getNotifications({ + type: NotificationType.ACTIVE, + offset: 0, + limit: 50, + }); + const unread = await service.getNotifications({ + type: NotificationType.UNREAD, + offset: 0, + limit: 50, + }); + + expect(active.map((n) => n.key)).toEqual(['webgui-pr-1']); + expect(active.every((n) => n.persistent)).toBe(true); + expect(unread).toHaveLength(3); + expect(unread.some((n) => n.persistent)).toBe(false); + }); + + it('Delete all / Archive all never touch persistent notifications', async () => { + const expectIn = makeExpectIn(expect); + + // Persistent notifications live in the 'active' store, so bulk archive/delete over + // unread/archive structurally cannot reach them. + await writeFile( + `${testPaths.active}/persistent-condition.notify`, + [ + 'timestamp=1700000000', + 'event="Active"', + 'subject="System notice"', + 'importance="warning"', + 'key="persistent-condition"', + 'persistent="true"', + ].join('\n') + '\n' + ); + await createNotification({ importance: NotificationImportance.INFO }); + await createNotification({ importance: NotificationImportance.INFO }); + + // Archive all (unread -> archive), then Delete all archived. + await service.archiveAll(); + await service.deleteNotifications(NotificationType.ARCHIVE); + await service.deleteNotifications(NotificationType.UNREAD); + + // The persistent one survives and is still served under ACTIVE (from active/). + const persistent = await service.getNotifications({ + type: NotificationType.ACTIVE, + offset: 0, + limit: 50, + }); + expect(persistent.map((n) => n.key)).toEqual(['persistent-condition']); + await expectIn({ type: NotificationType.ACTIVE }, 1); + }); }); describe.concurrent('NotificationsService legacy script compatibility', () => { @@ -567,3 +714,37 @@ describe.concurrent('NotificationsService legacy script compatibility', () => { } ); }); + +describe.concurrent('NotificationsService active path is always off-disk', () => { + // Simulate "Store notifications to boot drive" = Yes: the configured notifier path is + // somewhere other than the default /tmp/notifications (on real systems, the flash + // device). Active notifications must NOT follow it. We use a writable temp dir instead + // of a literal /boot path so the service's startup mkdir succeeds in CI. + const flashPath = '/tmp/test/flash-notifications'; + const buildService = async () => { + const module: TestingModule = await Test.createTestingModule({ + providers: [ + NotificationsService, + { + provide: ConfigService, + useValue: { get: vi.fn().mockReturnValue(flashPath) }, + }, + ], + }).compile(); + const service = module.get(NotificationsService); + await disableNotificationsWatcher(); + return service; + }; + + it('keeps active in tmpfs while event stores follow the flash path', async ({ expect }) => { + const service = await buildService(); + const paths = service.paths(); + // Condition-style notifications are derived state and must never persist to flash. + expect(paths.active).toBe('/tmp/notifications/active'); + expect(paths[NotificationType.ACTIVE]).toBe('/tmp/notifications/active'); + // Event notifications still honor the user's flash preference. + expect(paths.basePath).toBe(flashPath); + expect(paths[NotificationType.UNREAD]).toBe(`${flashPath}/unread`); + expect(paths[NotificationType.ARCHIVE]).toBe(`${flashPath}/archive`); + }); +}); 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 7a32c7b15f..bd0a41d179 100644 --- a/api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts +++ b/api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts @@ -1,13 +1,12 @@ import { Injectable, Logger } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { mkdir, readdir, readFile, rename, stat, unlink, writeFile } from 'fs/promises'; -import { basename, join } from 'path'; +import { basename, join, relative } from 'path'; import type { Stats } from 'fs'; import { FSWatcher, watch } from 'chokidar'; import { ValidationError } from 'class-validator'; import { execa } from 'execa'; -import { emptyDir } from 'fs-extra'; import { decode } from 'html-entities'; import { encode as encodeIni } from 'ini'; import { v7 as uuidv7 } from 'uuid'; @@ -32,11 +31,27 @@ import { validateObject } from '@app/unraid-api/graph/resolvers/validation.utils import { SortFn } from '@app/unraid-api/types/util.js'; import { batchProcess, formatDatetime, isFulfilled, isRejected, unraidTimestamp } from '@app/utils.js'; +/** + * Condition-style ("active") notifications are derived state: true only while their + * condition holds, and re-raised by the webgui on every page load. They are ALWAYS stored + * off-disk in tmpfs, never under the user-configurable notifier path (which may point at the + * flash boot device when "Store notifications to boot drive" is set). Persisting a derived + * condition across reboots strands it forever once the condition is gone (e.g. the plugin + * that raised it was removed); keeping it in tmpfs makes a reboot free GC. + * + * This MUST match the webgui `notify` script's hardcoded $active path. + */ +const ACTIVE_NOTIFICATIONS_PATH = '/tmp/notifications/active'; + @Injectable() export class NotificationsService { private static readonly FILE_IO_BATCH_SIZE = 32; + // Coalesce a burst of external file removals (e.g. a bulk `notify clear`) into a single + // authoritative overview rebuild. + private static readonly UNLINK_RECALC_DEBOUNCE_MS = 150; private logger = new Logger(NotificationsService.name); private static watcher: FSWatcher | null = null; + private unlinkRecalcTimer: NodeJS.Timeout | null = null; /** * The path to the notification directory - will be updated if the user changes the notifier path */ @@ -58,6 +73,12 @@ export class NotificationsService { warning: 0, total: 0, }, + active: { + alert: 0, + info: 0, + warning: 0, + total: 0, + }, }; constructor(private readonly configService: ConfigService) { @@ -77,7 +98,7 @@ export class NotificationsService { * - path to the unread notifications * - path to the archived notifications */ - public paths(): Record<'basePath' | NotificationType, string> { + public paths(): Record<'basePath' | NotificationType, string> & { active: string } { const basePath = this.getConfiguredPath(); if (this.path !== basePath) { @@ -86,10 +107,17 @@ export class NotificationsService { } const makePath = (type: NotificationType) => join(basePath, type.toLowerCase()); + // Active notifications always live in tmpfs, decoupled from the configurable + // basePath. See ACTIVE_NOTIFICATIONS_PATH. + const active = ACTIVE_NOTIFICATIONS_PATH; return { basePath, + // Persistent (condition-style) notifications are stored separately so they + // can be read cheaply and are never touched by bulk archive/delete. + active, [NotificationType.UNREAD]: makePath(NotificationType.UNREAD), [NotificationType.ARCHIVE]: makePath(NotificationType.ARCHIVE), + [NotificationType.ACTIVE]: active, }; } @@ -138,6 +166,8 @@ export class NotificationsService { mkdir(join(basePath, NotificationType.ARCHIVE.toLowerCase()), { recursive: true, }), + // Active notifications live in tmpfs, not under basePath. See ACTIVE_NOTIFICATIONS_PATH. + mkdir(ACTIVE_NOTIFICATIONS_PATH, { recursive: true }), ]); } @@ -154,12 +184,24 @@ export class NotificationsService { } await NotificationsService.watcher?.close().catch((e) => this.logger.error(e)); - NotificationsService.watcher = watch(basePath, { + // Active notifications live in tmpfs (ACTIVE_NOTIFICATIONS_PATH), which is outside + // basePath whenever the user stores event notifications on the flash device. Watch it + // separately in that case; when it is already nested under basePath (the default + // /tmp/notifications), basePath's recursive watch already covers it, so watching both + // would double-fire 'add' events. + const activeNested = relative(basePath, ACTIVE_NOTIFICATIONS_PATH) === 'active'; + const watchPaths = activeNested ? [basePath] : [basePath, ACTIVE_NOTIFICATIONS_PATH]; + + NotificationsService.watcher = watch(watchPaths, { usePolling: CHOKIDAR_USEPOLLING, ignoreInitial: true, - }).on('add', (path) => { - void this.handleNotificationAdd(path).catch((e) => this.logger.error(e)); - }); + }) + .on('add', (path) => { + void this.handleNotificationAdd(path).catch((e) => this.logger.error(e)); + }) + .on('unlink', (path) => { + this.handleNotificationUnlink(path); + }); return NotificationsService.watcher; } @@ -177,9 +219,37 @@ export class NotificationsService { await this.processNotificationAdd(path); } + /** + * A notification file vanished. Our own delete/clear/archive mutations adjust the overview + * synchronously (and this fires for those too), but external removals — the webgui + * `notify clear`, a producer resolving a keyed condition, tmpfs being wiped — bypass them + * and would leave the overview inflated. + * + * We rebuild from disk rather than decrement: the file is already gone (no importance to + * read), and an absolute snapshot is idempotent with any synchronous decrement that already + * happened, so it can't double-count. The rebuild is debounced so a bulk removal collapses + * into one pass. + */ + private handleNotificationUnlink(path: string) { + if (!path.endsWith('.notify')) return; + // The in-flight hydration snapshot will already reflect this removal. + if (this.isHydratingOverview) return; + if (this.unlinkRecalcTimer) clearTimeout(this.unlinkRecalcTimer); + this.unlinkRecalcTimer = setTimeout(() => { + this.unlinkRecalcTimer = null; + void this.recalculateOverview() + .then(() => this.publishWarningsAndAlerts()) + .catch((e) => this.logger.error(e)); + }, NotificationsService.UNLINK_RECALC_DEBOUNCE_MS); + } + private async processNotificationAdd(path: string) { - // The path looks like /{notification base path}/{type}/{notification id} - const type = path.includes('/unread/') ? NotificationType.UNREAD : NotificationType.ARCHIVE; + // The path looks like /{notification base path}/{store}/{notification id}. + const type = path.includes('/active/') + ? NotificationType.ACTIVE + : path.includes('/unread/') + ? NotificationType.UNREAD + : NotificationType.ARCHIVE; // this.logger.debug(`Adding ${type} Notification: ${path}`); let notification: Notification; @@ -197,7 +267,8 @@ export class NotificationsService { this.increment(notification.importance, NotificationsService.overview[type.toLowerCase()]); - if (type === NotificationType.UNREAD) { + // Both unread and active count toward the bell badge and live UI updates. + if (type === NotificationType.UNREAD || type === NotificationType.ACTIVE) { this.publishOverview(); pubsub.publish(PUBSUB_CHANNEL.NOTIFICATION_ADDED, { notificationAdded: notification, @@ -269,23 +340,34 @@ export class NotificationsService { warning: 0, total: 0, }, + active: { + alert: 0, + info: 0, + warning: 0, + total: 0, + }, }; const seenPaths = new Set(); // 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]); + // recalculates stats for a particular store (unread / archive / active). + const recalculate = async (folder: string, bucket: 'unread' | 'archive' | 'active') => { + const ids = await this.listFilesInFolder(folder); ids.forEach((id) => seenPaths.add(id)); const [notifications] = await this.loadNotificationsFromPaths(ids, {}); - notifications.forEach((n) => this.increment(n.importance, overview[type.toLowerCase()])); + notifications.forEach((n) => this.increment(n.importance, overview[bucket])); }; + const paths = this.paths(); const results = await batchProcess( - [NotificationType.ARCHIVE, NotificationType.UNREAD], - recalculate + [ + [paths[NotificationType.ARCHIVE], 'archive'] as const, + [paths[NotificationType.UNREAD], 'unread'] as const, + [paths.active, 'active'] as const, + ], + ([folder, bucket]) => recalculate(folder, bucket) ); if (results.errorOccurred) { @@ -300,6 +382,12 @@ export class NotificationsService { *------------------------------------------------------------------------**/ public async createNotification(data: NotificationData): Promise { + // Condition-style notifications are keyed and idempotent: raising one with an + // existing key replaces the prior instance (latest wins) instead of stacking dupes. + if (data.key) { + await this.clearNotificationsByKey(data.key); + } + const id: string = await this.makeNotificationId(data.title); const fileData = this.makeNotificationFileData(data); @@ -311,7 +399,10 @@ export class NotificationsService { this.logger.debug(`[createNotification] legacy notifier failed: ${error}`); this.logger.verbose(`[createNotification] Writing: ${JSON.stringify(fileData, null, 4)}`); - const path = join(this.paths().UNREAD, id); + // Persistent notifications belong in the 'active' store, matching the legacy + // notifier's write path. + const dir = data.persistent ? this.paths().active : this.paths().UNREAD; + const path = join(dir, id); const ini = encodeIni(fileData); // this.logger.debug(`[createNotification] INI: ${ini}`); await writeFile(path, ini); @@ -319,7 +410,10 @@ export class NotificationsService { void this.publishWarningsAndAlerts(); - return this.notificationFileToGqlNotification({ id, type: NotificationType.UNREAD }, fileData); + return this.notificationFileToGqlNotification( + { id, type: data.persistent ? NotificationType.ACTIVE : NotificationType.UNREAD }, + fileData + ); } /** @@ -332,7 +426,7 @@ export class NotificationsService { * @returns A 2-element tuple containing the legacy notifier command and arguments. */ public getLegacyScriptArgs(notification: NotificationIni): [string, string[]] { - const { event, subject, description, link, importance } = notification; + const { event, subject, description, link, importance, key, persistent } = notification; const args = [ ['-i', importance], ['-e', event], @@ -342,6 +436,12 @@ export class NotificationsService { if (link) { args.push(['-l', link]); } + if (key) { + args.push(['-k', key]); + } + if (persistent === 'true') { + args.push(['-p']); + } return ['/usr/local/emhttp/webGui/scripts/notify', args.flat()]; } @@ -370,7 +470,7 @@ export class NotificationsService { /** transforms gql compliant NotificationData to .notify compliant data*/ private makeNotificationFileData(notification: NotificationData): NotificationIni { - const { title, subject, description, link, importance } = notification; + const { title, subject, description, link, importance, key, persistent } = notification; const data: NotificationIni = { timestamp: unraidTimestamp().toString(), @@ -382,11 +482,17 @@ export class NotificationsService { // HACK - the ini encoder stringifies all fields defined on the object, even if they're undefined. // this results in a field like "link=undefined" in the resulting ini string. - // So, we only add a link if it's defined + // So, we only add optional fields if they're defined/truthy. if (link) { data.link = link; } + if (key) { + data.key = key; + } + if (persistent) { + data.persistent = 'true'; + } return data; } @@ -421,14 +527,12 @@ export class NotificationsService { * @remarks Ensures the notifications directory exists before emptying it */ public async deleteNotifications(type: NotificationType) { - await emptyDir(this.paths()[type]); - NotificationsService.overview[type.toLowerCase()] = { - alert: 0, - info: 0, - warning: 0, - total: 0, - }; - await this.publishOverview(); + const dir = this.paths()[type]; + // Persistent notifications live in 'active', never in unread/archive, so a bulk + // delete here structurally can't touch them — just remove everything in the store. + const paths = await this.listFilesInFolder(dir); + await batchProcess(paths, (path) => unlink(path)); + await this.recalculateOverview(); if (type === NotificationType.UNREAD) { void this.publishWarningsAndAlerts(); } @@ -447,6 +551,114 @@ export class NotificationsService { return this.getOverview(); } + /** + * Clears all unread notifications that share a stable producer `key`. + * + * This is how condition-style (typically persistent) notifications are resolved: a + * producer raises one with a key (e.g. "reboot-required") and clears it with the same + * key once the condition no longer holds. Also used to make keyed creates idempotent. + * + * @param key The stable producer key to clear. + * @returns The updated notification overview. + */ + public async clearNotificationsByKey(key: string): Promise { + // Keyed conditions normally live in 'active' (persistent), but sweep every store + // so a resolve / idempotent re-raise leaves no stale twin behind — e.g. a copy in + // unread/archive from before the active/ split, or one the user archived. + const paths = this.paths(); + const stores: Array<{ dir: string; bucket: 'unread' | 'archive' | 'active' }> = [ + { dir: paths.active, bucket: 'active' }, + { dir: paths[NotificationType.UNREAD], bucket: 'unread' }, + { dir: paths[NotificationType.ARCHIVE], bucket: 'archive' }, + ]; + let removed = false; + for (const { dir, bucket } of stores) { + for (const path of await this.listFilesInFolder(dir)) { + let ini: NotificationIni; + try { + ini = parseConfig({ + file: decode(await readFile(path, 'utf-8')), + type: 'ini', + }); + } catch { + continue; + } + if (ini.key !== key) continue; + await this.unlinkAndDecrement( + path, + this.fileImportanceToGqlImportance(ini.importance), + bucket + ); + removed = true; + } + } + if (removed) { + await this.publishOverview(); + void this.publishWarningsAndAlerts(); + } + return this.getOverview(); + } + + /** Unlinks a notification file and decrements the given overview bucket. */ + private async unlinkAndDecrement( + path: string, + importance: NotificationImportance, + bucket: 'unread' | 'archive' | 'active' + ): Promise { + await unlink(path).catch((e) => { + if (!this.isMissingFileError(e)) throw e; + }); + this.decrement(importance, NotificationsService.overview[bucket]); + } + + /** + * Reconciles JS-sourced banner notifications (keys prefixed `banner-`). + * + * Legacy webgui banners (CA's "Action Centre Enabled", boot checks, ...) have no + * explicit clear: they re-render on every page load while active and simply stop + * when resolved. The page stamps each banner -> bell notification with the current + * page-load generation; this clears any unread `banner-` notification NOT stamped + * with the supplied (current) generation, i.e. one whose producer stopped rendering + * it. Non-banner keys (PR plugins, reboot-required, ...) own their own lifecycle and + * are left untouched. + * + * Called by the notification drawer when it loads, so the served list and overview + * are authoritative without relying on a page-side timer. + * + * @param currentGeneration The page's current banner generation stamp. + * @returns The updated notification overview. + */ + public async reconcileBannerNotifications(currentGeneration: string): Promise { + // Banners are persistent, so they live in the 'active' store — read only that + // (small) directory rather than scanning all of unread. + const files = await this.listFilesInFolder(this.paths().active); + let removed = false; + for (const path of files) { + let ini: NotificationIni; + try { + ini = parseConfig({ + file: decode(await readFile(path, 'utf-8')), + type: 'ini', + }); + } catch { + continue; // skip unreadable/corrupt files; they surface elsewhere + } + if (!(ini.key ?? '').startsWith('banner-')) continue; + if (String(ini.gen ?? '') === currentGeneration) continue; + await this.unlinkAndDecrement( + path, + this.fileImportanceToGqlImportance(ini.importance), + 'active' + ); + removed = true; + } + if (removed) { + await this.publishOverview(); + void this.publishWarningsAndAlerts(); + } + return this.getOverview(); + } + /**------------------------------------------------------------------------ * CRUD: Updating Notifications *------------------------------------------------------------------------**/ @@ -538,6 +750,12 @@ export class NotificationsService { *------------------------**/ const snapshot = this.getOverview(); const notification = await this.loadNotificationFile(unreadPath, NotificationType.UNREAD); + + // Persistent notifications represent an ongoing condition. They are not + // dismissed casually (the UI gates this behind a warning, and bulk archiveAll + // skips them), but an explicit, confirmed single dismiss is allowed: it + // acknowledges/hides the reminder. The condition itself is unaffected, so it + // re-pins if the producer raises it again (idempotent key). const moveToArchive = this.moveNotification({ from: NotificationType.UNREAD, to: NotificationType.ARCHIVE, @@ -582,14 +800,14 @@ export class NotificationsService { public async archiveAll(importance?: NotificationImportance) { const { UNREAD } = this.paths(); - if (!importance) { - await readdir(UNREAD).then((ids) => this.archiveIds(ids)); - return { overview: NotificationsService.overview }; - } - const overviewSnapshot = this.getOverview(); + // Persistent notifications live in 'active', not 'unread', so "Archive all" over the + // unread store can't touch them — no persistent filtering needed. const unreads = await this.listFilesInFolder(UNREAD); - const [notifications] = await this.loadNotificationsFromPaths(unreads, { importance }); + const [notifications] = await this.loadNotificationsFromPaths( + unreads, + importance ? { importance } : {} + ); const archive = this.moveNotification({ from: NotificationType.UNREAD, to: NotificationType.ARCHIVE, @@ -666,20 +884,22 @@ export class NotificationsService { const { type = NotificationType.UNREAD } = filters; const { ARCHIVE, UNREAD } = this.paths(); - let files: string[]; - if (type === NotificationType.UNREAD) { - files = await this.listFilesInFolder(UNREAD); - } else { + if (type === NotificationType.ARCHIVE) { // Exclude notifications present in both unread & archive from archive. //* this is necessary because the legacy script writes new notifications to both places. //* this should be a temporary measure. const unreads = new Set(await readdir(UNREAD)); - files = await this.listFilesInFolder(ARCHIVE, (archives) => { + const files = await this.listFilesInFolder(ARCHIVE, (archives) => { return archives.filter((file) => !unreads.has(file)); }); + const [archived] = await this.loadNotificationsFromPaths(files, filters); + return archived; } + // UNREAD -> the transient 'unread' store; ACTIVE -> the persistent 'active' store. + // Each tab reads only its own directory, so pagination is clean and independent. + const files = await this.listFilesInFolder(this.paths()[type]); const [notifications] = await this.loadNotificationsFromPaths(files, filters); return notifications; } @@ -883,6 +1103,7 @@ export class NotificationsService { subject: nameMask, description: `This notification is invalid and cannot be displayed! For details, see the logs and the notification file at ${path}`, importance: NotificationImportance.WARNING, + persistent: false, timestamp: dateMask.toISOString(), formattedTimestamp: this.formatDatetime(dateMask), }; @@ -908,7 +1129,14 @@ export class NotificationsService { details: Pick, fileData: NotificationIni ): Notification { - const { importance, timestamp, event: title, description = '', ...passthroughData } = fileData; + const { + importance, + timestamp, + event: title, + description = '', + persistent, + ...passthroughData + } = fileData; const { type, id } = details; return { ...passthroughData, @@ -917,6 +1145,9 @@ export class NotificationsService { title, description, importance: this.fileImportanceToGqlImportance(importance), + // The ini parser may coerce `persistent="true"` to a boolean, so normalize + // via String() rather than a strict string compare. + persistent: String(persistent) === 'true', timestamp: this.parseNotificationDateToIsoDate(timestamp)?.toISOString(), formattedTimestamp: this.formatTimestamp(timestamp), }; diff --git a/web/__test__/store/notifications.test.ts b/web/__test__/store/notifications.test.ts index df94a4847a..f6aea0140c 100644 --- a/web/__test__/store/notifications.test.ts +++ b/web/__test__/store/notifications.test.ts @@ -70,6 +70,7 @@ describe('Notifications Store', () => { subject: 'Test Subject 1', description: 'This is a test notification 1', importance: 'NORMAL' as NotificationImportance, + persistent: false, type: 'SYSTEM' as NotificationType, timestamp: '2023-01-01T12:00:00Z', formattedTimestamp: 'Jan 1, 2023', @@ -81,6 +82,7 @@ describe('Notifications Store', () => { subject: 'Test Subject 2', description: 'This is a test notification 2', importance: 'HIGH' as NotificationImportance, + persistent: false, type: 'UPDATE' as NotificationType, timestamp: '2023-01-02T12:00:00Z', formattedTimestamp: 'Jan 2, 2023', diff --git a/web/src/components/Notifications/CriticalNotifications.standalone.vue b/web/src/components/Notifications/CriticalNotifications.standalone.vue index 304ad430d0..cd1c173b78 100644 --- a/web/src/components/Notifications/CriticalNotifications.standalone.vue +++ b/web/src/components/Notifications/CriticalNotifications.standalone.vue @@ -256,6 +256,7 @@ onNotificationAdded(({ data }) => { View Details + + Clears automatically when resolved + diff --git a/web/src/components/Notifications/Indicator.vue b/web/src/components/Notifications/Indicator.vue index 95330461a8..507606227f 100644 --- a/web/src/components/Notifications/Indicator.vue +++ b/web/src/components/Notifications/Indicator.vue @@ -11,22 +11,43 @@ import { NotificationImportance as Importance } from '~/composables/gql/graphql' const props = defineProps<{ overview?: OverviewQuery['notifications']['overview']; seen?: boolean }>(); +// The bell reflects unread + active: new transient notifications AND ongoing +// ("Active") conditions both light it. +const counts = computed(() => { + const o = props.overview; + if (!o) return undefined; + return { + alert: o.unread.alert + o.active.alert, + warning: o.unread.warning + o.active.warning, + total: o.unread.total + o.active.total, + activeTotal: o.active.total, + unreadTotal: o.unread.total, + }; +}); + const indicatorLevel = computed(() => { - if (!props.overview?.unread) { - return undefined; - } + const c = counts.value; + if (!c) return undefined; switch (true) { - case props.overview.unread.alert > 0: + case c.alert > 0: return Importance.ALERT; - case props.overview.unread.warning > 0: + case c.warning > 0: return Importance.WARNING; - case props.overview.unread.total > 0: + case c.total > 0: return 'UNREAD'; default: return undefined; } }); +// Active conditions are ongoing, so they keep the bell lit regardless of "seen"; +// transient unread only lights it until the user has seen them. +const shouldShow = computed(() => { + const c = counts.value; + if (!c) return false; + return c.activeTotal > 0 || (!props.seen && c.unreadTotal > 0); +}); + const icon = computed<{ component: Component; color: string } | null>(() => { switch (indicatorLevel.value) { case Importance.WARNING: @@ -48,12 +69,12 @@ const icon = computed<{ component: Component; color: string } | null>(() => {
diff --git a/web/src/components/Notifications/Item.vue b/web/src/components/Notifications/Item.vue index ede4b1afe3..ccf8612be9 100644 --- a/web/src/components/Notifications/Item.vue +++ b/web/src/components/Notifications/Item.vue @@ -91,10 +91,17 @@ const reformattedTimestamp = computed(() => {