refactor(system-notices): notice dismissals through a Nest provider and repository

server/src/systemNotices/conditions.ts

@@ -12,6 +12,18 @@ interface ConditionContext {
   addonEnabled: (addonId: string) => boolean;
   /** True when somebody other than this install's admin owns its configuration. */
   managed: boolean;
+  /**
+   * A custom predicate's pre-resolved answer to a question that would
+   * otherwise need a DB read — same shape as `addonEnabled`/`managed`: the
+   * caller (`SystemNoticesService.getActiveFor`, Plan 3f Task 6/R1) resolves
+   * every key a registered `case 'custom'` predicate needs BEFORE calling
+   * `evaluate()`, since `evaluateOne` below calls a predicate synchronously
+   * inside `SYSTEM_NOTICES.filter(...)`, which cannot `await` a repository
+   * read. `registry.ts`'s `whitespace-collision-detected` predicate is the
+   * only current reader (`ctx.settingFlag('whitespace_migration_collision')`);
+   * this module stays free of any DB import either way.
+   */
+  settingFlag: (key: string) => boolean;
 }
 
 // Custom predicate registry — extensible without modifying this file

server/src/systemNotices/registry.ts

@@ -1,11 +1,22 @@
 import type { SystemNotice } from './types.js';
 import { registerPredicate } from './conditions.js';
-import { db } from '../db/database.js';
 
-registerPredicate('whitespace-collision-detected', () => {
-  const row = db.prepare("SELECT value FROM app_settings WHERE key = 'whitespace_migration_collision'").get() as { value: string } | undefined;
-  return row?.value === 'true';
-});
+/**
+ * Registered at module load, unchanged from before this plan (Plan 3f Task
+ * 6/R1's adopted wiring): the closure used to read `app_settings` directly
+ * through the raw `db` handle here; now it only reads a value the caller
+ * (`SystemNoticesService.getActiveFor`) already resolved into `ctx` before
+ * `evaluate()` runs, the same shape `addonEnabled`/`managed` already use.
+ * `evaluateOne`'s `case 'custom'` (`conditions.ts`) calls this synchronously
+ * inside a `.filter()`, which cannot `await` — moving the DB read into this
+ * closure directly would either need to make the closure (and the whole
+ * evaluate/evaluateOne chain) async, or read an un-awaited repository
+ * result. Reading a pre-resolved value off `ctx` avoids both, stays
+ * race-free (the value flows through the per-call `ctx` object, not shared
+ * mutable state on a singleton), and needs no Nest lifecycle hook — this
+ * registration stays exactly where it always was.
+ */
+registerPredicate('whitespace-collision-detected', (ctx) => ctx.settingFlag('whitespace_migration_collision'));
 
 /**
  * SYSTEM NOTICE REGISTRY

server/src/systemNotices/service.ts

@@ -1,9 +1,6 @@
 import semver from 'semver';
 import { readEnv } from '../app-config';
-import { db } from '../db/database.js';
-import { SYSTEM_NOTICES } from './registry.js';
-import { evaluate } from './conditions.js';
-import type { SystemNotice, SystemNoticeDTO } from './types.js';
+import type { SystemNotice } from './types.js';
 
 export function getCurrentAppVersion(): string {
   const fromEnv = semver.valid(readEnv().app.appVersion ?? '');
@@ -31,91 +28,14 @@ export function isNoticeVersionActive(n: SystemNotice, currentAppVersion: string
   return true;
 }
 
-function severityWeight(s: string): number {
+/**
+ * Sort weight: critical > warn > info. Pure, no DB — kept here per Plan 3f
+ * Task 0's R1 wiring plan ("isNoticeVersionActive, getCurrentAppVersion,
+ * severityWeight are pure — stay as plain exported functions"), now exported
+ * for `SystemNoticesService.getActiveFor` (`nest/system-notices/`), which
+ * absorbed the DB-touching `getActiveNoticesFor`/`dismissNotice` that used to
+ * live in this file.
+ */
+export function severityWeight(s: string): number {
   return s === 'critical' ? 2 : s === 'warn' ? 1 : 0;
 }
-
-export async function getActiveNoticesFor(
-  userId: number,
-  addonEnabled: (addonId: string) => Promise<boolean>,
-  managed = false
-): Promise<SystemNoticeDTO[]> {
-  const user = db.prepare(
-    'SELECT login_count, first_seen_version, role FROM users WHERE id = ?'
-  ).get(userId) as { login_count: number; first_seen_version: string; role: string } | undefined;
-
-  if (!user) return [];
-
-  const { count: tripCount } = db.prepare(
-    'SELECT COUNT(*) AS count FROM trips WHERE user_id = ?'
-  ).get(userId) as { count: number };
-
-  // Dismissals mapped to the app version they were dismissed at (used by per-version notices).
-  const dismissals = new Map<string, string | null>(
-    (db.prepare('SELECT notice_id, dismissed_app_version FROM user_notice_dismissals WHERE user_id = ?')
-      .all(userId) as Array<{ notice_id: string; dismissed_app_version: string | null }>)
-      .map(r => [r.notice_id, r.dismissed_app_version])
-  );
-
-  const now = new Date();
-  const currentAppVersion = getCurrentAppVersion();
-  // `evaluate` runs inside a .filter(), which cannot await, so the addon flags
-  // the registry is able to ask about are resolved up front and handed to it as
-  // a lookup. The ids come from the notices themselves, so every question the
-  // conditions can ask has an answer here.
-  const addonFlags = new Map<string, boolean>();
-  for (const condition of SYSTEM_NOTICES.flatMap(n => n.conditions)) {
-    if (condition.kind === 'addonEnabled' && !addonFlags.has(condition.addonId)) {
-      addonFlags.set(condition.addonId, await addonEnabled(condition.addonId));
-    }
-  }
-  const ctx = {
-    user: { ...user, noTrips: tripCount },
-    currentAppVersion,
-    now,
-    addonEnabled: (addonId: string) => addonFlags.get(addonId),
-    managed,
-  };
-  const appVer = semver.coerce(currentAppVersion)?.version ?? '0.0.0';
-
-  const isStillDismissed = (n: SystemNotice): boolean => {
-    if (!dismissals.has(n.id)) return false;
-    if (n.recurring === 'per-version') {
-      // Re-show once the running app version moves past the version it was last dismissed at,
-      // so a per-version notice surfaces again on each install/upgrade.
-      const dismissedVer = semver.coerce(dismissals.get(n.id) ?? '0.0.0')?.version ?? '0.0.0';
-      return semver.gte(dismissedVer, appVer);
-    }
-    return true; // default: permanent one-time dismissal
-  };
-
-  return SYSTEM_NOTICES
-    .filter(n => {
-      if (isStillDismissed(n)) return false;
-      if (!isNoticeVersionActive(n, currentAppVersion)) return false;
-      return evaluate(n, ctx);
-    })
-    .sort((a, b) => {
-      const pw = (b.priority ?? 0) - (a.priority ?? 0);
-      if (pw !== 0) return pw;
-      const sw = severityWeight(b.severity) - severityWeight(a.severity);
-      if (sw !== 0) return sw;
-      return new Date(b.publishedAt).getTime() - new Date(a.publishedAt).getTime();
-    })
-    .map(({ conditions: _c, publishedAt: _p, minVersion: _mn, maxVersion: _mx, priority: _pr, recurring: _rc, ...dto }) => dto);
-}
-
-export async function dismissNotice(userId: number, noticeId: string): Promise<boolean> {
-  const exists = SYSTEM_NOTICES.some(n => n.id === noticeId);
-  if (!exists) return false;
-  // Record the app version at dismissal so per-version notices can re-appear on the next
-  // upgrade. Upsert (not INSERT OR IGNORE) so re-dismissing after a bump refreshes the version.
-  db.prepare(`
-    INSERT INTO user_notice_dismissals (user_id, notice_id, dismissed_at, dismissed_app_version)
-    VALUES (?, ?, ?, ?)
-    ON CONFLICT(user_id, notice_id) DO UPDATE SET
-      dismissed_at = excluded.dismissed_at,
-      dismissed_app_version = excluded.dismissed_app_version
-  `).run(userId, noticeId, Date.now(), getCurrentAppVersion());
-  return true;
-}