From 4dbad48d89b254f91092236f0b30b6e495378929 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 3 Sep 2026 02:02:22 +0200 Subject: [PATCH] fix(admin): persist the assistant flags sent to the settings route MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `adminSettingsSchema` validates five `assistant*` booleans and the strict schema refuses anything else, but the route's `booleanFields` list never carried them. Sending one alone answered 422 "No valid fields"; sending one alongside a real field answered 200 and dropped it. The schema comment says the entry exists so an operator scripting their settings over the single route can carry the values, so the route is brought up to the contract rather than the contract cut down to the route. The flags are echoed in the GET and PUT payloads as raw column values — the master-over-sub resolution stays with `/settings/assistant-flags`, which owns that shape and was never affected. --- .../admin/settings/__tests__/route.test.ts | 131 ++++++++++++++++++ src/app/api/admin/settings/route.ts | 28 +++- 2 files changed, 158 insertions(+), 1 deletion(-) diff --git a/src/app/api/admin/settings/__tests__/route.test.ts b/src/app/api/admin/settings/__tests__/route.test.ts index deaae0f41..56d33df68 100644 --- a/src/app/api/admin/settings/__tests__/route.test.ts +++ b/src/app/api/admin/settings/__tests__/route.test.ts @@ -482,3 +482,134 @@ describe("PUT /api/admin/settings", () => { }); }); }); + +/** + * The five assistant flags were accepted by `adminSettingsSchema` and then + * dropped on the floor: sending one alone answered 422 ("No valid fields"), + * sending it alongside a real field answered 200 and stored nothing. The + * schema comment promised the opposite, so the route was brought up to the + * contract rather than the contract down to the route. + */ +describe("PUT /api/admin/settings — assistant flags", () => { + const STORED = { + registrationEnabled: true, + mfaRequired: false, + defaultLocale: "de", + telegramGlobal: true, + ntfyGlobal: true, + webPushGlobal: true, + webPushVapidPublicKey: null, + webPushVapidPrivateKeyEncrypted: null, + webPushVapidSubject: null, + apiGlobal: true, + umamiEnabled: false, + umamiScriptUrl: null, + umamiWebsiteId: null, + glitchtipEnabled: false, + glitchtipDsn: null, + glitchtipEnvironment: null, + reminderLateMinutes: 120, + reminderMissedMinutes: 240, + documentMaxFileBytes: 26_214_400, + documentQuotaBytes: BigInt(1_073_741_824), + defaultUserTimezone: null, + assistantEnabled: true, + assistantCoachEnabled: false, + assistantBriefingEnabled: true, + assistantInsightStatusEnabled: true, + assistantCorrelationsEnabled: true, + }; + + beforeEach(() => { + vi.mocked(prisma.appSettings.upsert).mockResolvedValue(STORED as never); + }); + + it("persists a lone assistant flag instead of answering 'No valid fields'", async () => { + const res = await PUT(jsonReq({ assistantCoachEnabled: false })); + + expect(res.status).toBe(200); + expect(prisma.appSettings.upsert).toHaveBeenCalledTimes(1); + const args = vi.mocked(prisma.appSettings.upsert).mock.calls[0]?.[0] as { + update: Record; + }; + expect(args.update).toEqual({ assistantCoachEnabled: false }); + }); + + it("persists an assistant flag sent alongside an unrelated field", async () => { + const res = await PUT( + jsonReq({ registrationEnabled: false, assistantCoachEnabled: false }), + ); + + expect(res.status).toBe(200); + const args = vi.mocked(prisma.appSettings.upsert).mock.calls[0]?.[0] as { + update: Record; + }; + expect(args.update).toEqual({ + registrationEnabled: false, + assistantCoachEnabled: false, + }); + }); + + it("echoes the stored flags so a scripted write reads back", async () => { + const res = await PUT(jsonReq({ assistantCoachEnabled: false })); + const body = (await res.json()) as { data: Record }; + + expect(body.data).toMatchObject({ + assistantEnabled: true, + assistantCoachEnabled: false, + assistantBriefingEnabled: true, + assistantInsightStatusEnabled: true, + assistantCorrelationsEnabled: true, + }); + }); + + it("audits the flag it changed", async () => { + await PUT(jsonReq({ assistantCorrelationsEnabled: false })); + + expect(auditLog).toHaveBeenCalledWith( + "admin.settings.update", + expect.objectContaining({ + details: expect.objectContaining({ + assistantCorrelationsEnabled: false, + }), + }), + ); + }); + + it("still refuses an unknown flag-shaped key", async () => { + const res = await PUT(jsonReq({ assistantNotAThing: false })); + expect(res.status).toBe(422); + expect(prisma.appSettings.upsert).not.toHaveBeenCalled(); + }); +}); + +describe("GET /api/admin/settings — assistant flags", () => { + it("defaults every flag to enabled when the row is absent", async () => { + vi.mocked(prisma.appSettings.findUnique).mockResolvedValue(null); + const res = await GET(); + const body = (await res.json()) as { data: Record }; + + expect(body.data).toMatchObject({ + assistantEnabled: true, + assistantCoachEnabled: true, + assistantBriefingEnabled: true, + assistantInsightStatusEnabled: true, + assistantCorrelationsEnabled: true, + }); + }); + + it("surfaces the stored flags", async () => { + vi.mocked(prisma.appSettings.findUnique).mockResolvedValue({ + assistantEnabled: false, + assistantCoachEnabled: false, + } as never); + const res = await GET(); + const body = (await res.json()) as { data: Record }; + + expect(body.data).toMatchObject({ + assistantEnabled: false, + assistantCoachEnabled: false, + assistantBriefingEnabled: true, + }); + }); +}); diff --git a/src/app/api/admin/settings/route.ts b/src/app/api/admin/settings/route.ts index cfb4d787b..bfbaf9b89 100644 --- a/src/app/api/admin/settings/route.ts +++ b/src/app/api/admin/settings/route.ts @@ -61,6 +61,16 @@ export const GET = apiHandler(async () => { // resolver"; surfacing the raw value lets the admin UI render // an empty picker placeholder until they opt in. defaultUserTimezone: settings?.defaultUserTimezone ?? null, + // Raw column values, not the resolved matrix — the master flag is + // applied to the sub-flags by `/settings/assistant-flags`, which owns + // that shape. Echoed here so a write over this route reads back. + assistantEnabled: settings?.assistantEnabled ?? true, + assistantCoachEnabled: settings?.assistantCoachEnabled ?? true, + assistantBriefingEnabled: settings?.assistantBriefingEnabled ?? true, + assistantInsightStatusEnabled: + settings?.assistantInsightStatusEnabled ?? true, + assistantCorrelationsEnabled: + settings?.assistantCorrelationsEnabled ?? true, }); }); @@ -83,7 +93,13 @@ export const PUT = apiHandler(async (request: NextRequest) => { const updates: Record = {}; const auditDetails: Record = {}; - // Boolean fields — direct mapping + // Boolean fields — direct mapping. + // + // The five assistant flags belong here and not only on the dedicated + // `/settings/assistant-flags` endpoint: `adminSettingsSchema` accepts them + // so an operator scripting their settings over a single route can carry + // them, and a schema that accepts a field and then drops it is worse than + // one that refuses it. const booleanFields = [ "registrationEnabled", "mfaRequired", @@ -93,6 +109,11 @@ export const PUT = apiHandler(async (request: NextRequest) => { "apiGlobal", "umamiEnabled", "glitchtipEnabled", + "assistantEnabled", + "assistantCoachEnabled", + "assistantBriefingEnabled", + "assistantInsightStatusEnabled", + "assistantCorrelationsEnabled", ] as const; for (const field of booleanFields) { if (data[field] !== undefined) { @@ -258,5 +279,10 @@ export const PUT = apiHandler(async (request: NextRequest) => { documentMaxFileBytes: settings.documentMaxFileBytes, documentQuotaBytes: Number(settings.documentQuotaBytes), defaultUserTimezone: settings.defaultUserTimezone, + assistantEnabled: settings.assistantEnabled, + assistantCoachEnabled: settings.assistantCoachEnabled, + assistantBriefingEnabled: settings.assistantBriefingEnabled, + assistantInsightStatusEnabled: settings.assistantInsightStatusEnabled, + assistantCorrelationsEnabled: settings.assistantCorrelationsEnabled, }); });