feat(notificaciones): edit summary recipients in the UI
NOTIFICATION_ADMIN_EMAILS made "add Beto to the summaries" a redeploy — the wrong unit of work for a list that changes when office staff change. Adds `app_settings`, a key/value table for the configuration staff must be able to change without a deploy, and `SettingsService`, which resolves every key db -> env -> default and reports which of the three a value came from. That ladder is what makes the move safe: a deployment behaves exactly as before until somebody saves in the UI, and the screen can say "this is still coming from the deployment" rather than implying somebody chose it. - new ability `setting:manage` (ADMIN) — deliberately above `notification:send`, since redirecting the audit summaries is how someone would quietly stop them being read - GET/PUT /notifications/settings/admin-emails; read is open to any logged-in user so the UI can display the list, write is gated - resolved per job, not cached at boot, or we would reintroduce exactly the restart-to-apply behaviour being removed - a saved empty list means "nobody" and does NOT fall through to the env, or clearing the field would keep mailing the people just removed Credentials stay in env — see the model doc for where the line is drawn. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,14 @@
|
||||
import { Module } from "@nestjs/common";
|
||||
import { SettingsService } from "./settings.service";
|
||||
|
||||
/**
|
||||
* Operator-editable configuration. No controller of its own — each setting is
|
||||
* exposed by the feature that owns it (summary recipients live under
|
||||
* /notifications), so the validation and the permission live next to the
|
||||
* thing they protect rather than behind a generic key/value endpoint.
|
||||
*/
|
||||
@Module({
|
||||
providers: [SettingsService],
|
||||
exports: [SettingsService],
|
||||
})
|
||||
export class SettingsModule {}
|
||||
@@ -0,0 +1,89 @@
|
||||
import { SettingsService, invalidEmails, parseEmailList } from "./settings.service";
|
||||
|
||||
/**
|
||||
* The db → env → default ladder is the whole contract of this service: it is
|
||||
* what lets the setting move out of the environment without changing how any
|
||||
* existing deployment behaves.
|
||||
*/
|
||||
|
||||
function build(row: { value: string } | null, env?: string) {
|
||||
const prisma = {
|
||||
appSetting: {
|
||||
findUnique: jest.fn().mockResolvedValue(
|
||||
row ? { key: "k", updatedAt: new Date("2026-08-02"), updatedById: "u1", ...row } : null,
|
||||
),
|
||||
upsert: jest.fn().mockResolvedValue({}),
|
||||
},
|
||||
};
|
||||
const config = { get: jest.fn().mockReturnValue(env) };
|
||||
return {
|
||||
service: new SettingsService(prisma as never, config as never),
|
||||
prisma,
|
||||
};
|
||||
}
|
||||
|
||||
describe("notification admin emails resolve db > env > default", () => {
|
||||
it("prefers the stored row", async () => {
|
||||
const { service } = build({ value: "a@x.com,b@x.com" }, "env@x.com");
|
||||
|
||||
await expect(service.notificationAdminEmails()).resolves.toMatchObject({
|
||||
value: ["a@x.com", "b@x.com"],
|
||||
source: "db",
|
||||
updatedById: "u1",
|
||||
});
|
||||
});
|
||||
|
||||
it("falls back to the environment when nothing is stored", async () => {
|
||||
const { service } = build(null, "env@x.com, other@x.com");
|
||||
|
||||
await expect(service.notificationAdminEmails()).resolves.toMatchObject({
|
||||
value: ["env@x.com", "other@x.com"],
|
||||
source: "env",
|
||||
});
|
||||
});
|
||||
|
||||
it("falls back to the built-in defaults when neither is set", async () => {
|
||||
const { service } = build(null, undefined);
|
||||
|
||||
const resolved = await service.notificationAdminEmails();
|
||||
expect(resolved.source).toBe("default");
|
||||
expect(resolved.value).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("treats a stored empty list as 'nobody', not as unset", async () => {
|
||||
// The regression this guards: falling through to env/defaults here would
|
||||
// keep mailing people who were deliberately removed.
|
||||
const { service } = build({ value: "" }, "env@x.com");
|
||||
|
||||
await expect(service.notificationAdminEmails()).resolves.toMatchObject({
|
||||
value: [],
|
||||
source: "db",
|
||||
});
|
||||
});
|
||||
|
||||
it("writes the list back as CSV", async () => {
|
||||
const { service, prisma } = build({ value: "" });
|
||||
|
||||
await service.setNotificationAdminEmails(["a@x.com", "b@x.com"], "user-9");
|
||||
|
||||
expect(prisma.appSetting.upsert).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
create: expect.objectContaining({ value: "a@x.com,b@x.com", updatedById: "user-9" }),
|
||||
update: expect.objectContaining({ value: "a@x.com,b@x.com", updatedById: "user-9" }),
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("email list parsing", () => {
|
||||
it("trims and drops blanks", () => {
|
||||
expect(parseEmailList(" a@x.com , ,b@x.com ")).toEqual(["a@x.com", "b@x.com"]);
|
||||
});
|
||||
|
||||
it("rejects entries that are not addresses at all", () => {
|
||||
expect(invalidEmails(["ok@x.com", "nope", "also@bad"])).toEqual([
|
||||
"nope",
|
||||
"also@bad",
|
||||
]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,128 @@
|
||||
import { Injectable, Logger } from "@nestjs/common";
|
||||
import { ConfigService } from "@nestjs/config";
|
||||
import { PrismaService } from "../prisma/prisma.service";
|
||||
|
||||
/**
|
||||
* Reader/writer for `app_settings` — the configuration staff can change
|
||||
* without a redeploy.
|
||||
*
|
||||
* Every setting resolves through the same three-step ladder: the database row
|
||||
* if an operator has set one, else the environment variable it used to live
|
||||
* in, else a hardcoded default. That ordering is what makes this migration
|
||||
* safe — an existing deployment keeps behaving exactly as it did until
|
||||
* somebody edits the value in the UI, and `source` tells the UI which of the
|
||||
* three it is looking at so "this came from the env, editing it here will
|
||||
* take over" is visible rather than surprising.
|
||||
*/
|
||||
|
||||
export const SETTING_KEYS = {
|
||||
/** Comma-separated recipients of the per-job notification summary. */
|
||||
notificationAdminEmails: "notification.adminEmails",
|
||||
} as const;
|
||||
|
||||
/** Where a resolved value came from. Shown in the UI. */
|
||||
export type SettingSource = "db" | "env" | "default";
|
||||
|
||||
export interface ResolvedSetting<T> {
|
||||
value: T;
|
||||
source: SettingSource;
|
||||
updatedAt: Date | null;
|
||||
updatedById: string | null;
|
||||
}
|
||||
|
||||
/** Last resort when neither the database nor the environment says otherwise.
|
||||
* Matches what `NotificationsService` hardcoded before this table existed. */
|
||||
const DEFAULT_ADMIN_EMAILS = ["rmancinas@freakma.net", "mpulido@freakma.net"];
|
||||
|
||||
/** Deliberately permissive — this rejects "not an address at all", not
|
||||
* "not deliverable". Only SES can tell us the latter, and a validator strict
|
||||
* enough to argue with is a validator that blocks a legitimate address. */
|
||||
const EMAIL_RE = /^[^\s@,]+@[^\s@,]+\.[^\s@,]+$/;
|
||||
|
||||
export function parseEmailList(raw: string): string[] {
|
||||
return raw
|
||||
.split(",")
|
||||
.map((s) => s.trim())
|
||||
.filter(Boolean);
|
||||
}
|
||||
|
||||
export function invalidEmails(list: string[]): string[] {
|
||||
return list.filter((e) => !EMAIL_RE.test(e));
|
||||
}
|
||||
|
||||
@Injectable()
|
||||
export class SettingsService {
|
||||
private readonly logger = new Logger(SettingsService.name);
|
||||
|
||||
constructor(
|
||||
private readonly prisma: PrismaService,
|
||||
private readonly config: ConfigService,
|
||||
) {}
|
||||
|
||||
/**
|
||||
* Recipients of the per-job summary email.
|
||||
*
|
||||
* Read on every send rather than cached at boot: the point of moving this
|
||||
* out of the environment was that it changes while the app is running, and
|
||||
* a cache would reintroduce exactly the restart-to-apply behaviour we are
|
||||
* removing. It is one indexed primary-key lookup per sweep, not per email.
|
||||
*/
|
||||
async notificationAdminEmails(): Promise<ResolvedSetting<string[]>> {
|
||||
const row = await this.read(SETTING_KEYS.notificationAdminEmails);
|
||||
if (row) {
|
||||
const parsed = parseEmailList(row.value);
|
||||
// An empty stored value is a legitimate choice — "send no summaries" —
|
||||
// and must not silently fall through to the env or the defaults, or an
|
||||
// operator who cleared the field would keep receiving mail.
|
||||
return {
|
||||
value: parsed,
|
||||
source: "db",
|
||||
updatedAt: row.updatedAt,
|
||||
updatedById: row.updatedById,
|
||||
};
|
||||
}
|
||||
|
||||
const env = this.config.get<string>("NOTIFICATION_ADMIN_EMAILS");
|
||||
if (env && env.trim()) {
|
||||
return {
|
||||
value: parseEmailList(env),
|
||||
source: "env",
|
||||
updatedAt: null,
|
||||
updatedById: null,
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
value: [...DEFAULT_ADMIN_EMAILS],
|
||||
source: "default",
|
||||
updatedAt: null,
|
||||
updatedById: null,
|
||||
};
|
||||
}
|
||||
|
||||
/** Persist the summary recipients. An empty list is stored as an empty
|
||||
* string and means "nobody" — see the read path above. */
|
||||
async setNotificationAdminEmails(
|
||||
emails: string[],
|
||||
userId: string,
|
||||
): Promise<ResolvedSetting<string[]>> {
|
||||
await this.write(
|
||||
SETTING_KEYS.notificationAdminEmails,
|
||||
emails.join(","),
|
||||
userId,
|
||||
);
|
||||
return this.notificationAdminEmails();
|
||||
}
|
||||
|
||||
private read(key: string) {
|
||||
return this.prisma.appSetting.findUnique({ where: { key } });
|
||||
}
|
||||
|
||||
private async write(key: string, value: string, userId: string) {
|
||||
await this.prisma.appSetting.upsert({
|
||||
where: { key },
|
||||
create: { key, value, updatedById: userId },
|
||||
update: { value, updatedById: userId },
|
||||
});
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user