From d80d939bd8e9cb8805fba44d2dbd4898f9243a33 Mon Sep 17 00:00:00 2001 From: gorkem-bwl Date: Mon, 27 Apr 2026 19:26:16 -0400 Subject: [PATCH] test(status-page): cover theme-flag helpers and document remaining caveats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review follow-ups on PR #3568: - Extract the controller's withoutThemeFields and applyDefaultTheme helpers from a private class field into a standalone module (controllers/statusPageThemeFlag.ts) so they can be tested without a controller harness; the controller is now a one-line consumer - Add 9 unit tests covering both helpers (test/unit/controllers/statusPageThemeFlag.test.ts): no-mutation guarantees, partial inputs, all-fields-preserved verbatim, override-with-defaults semantics, and "page has no theme set yet" — the flag-off path is now exercised in both directions - Expand the comment over withoutThemeFields to spell out why the strip is silent rather than a 400: this is an ops kill switch, not API semantics; clients shouldn't need to know whether themes are disabled today Two caveats called out in review that this commit does NOT change: - Mongoose `default:` directives only inject during hydration, not on partial updates. Legacy documents updated with `findOneAndUpdate` still won't have `theme` / `themeMode` written to Mongo. They render as defaults on read because the schema injects them — functionally equivalent, but the DB will hold a mix of present and absent fields indefinitely. Acceptable per the original commit intent. - docs/plans/status-page-themes.md (304 lines) covers both backend and frontend work for the themes feature. It lives in this PR as the design artefact that drove both branches; moving it for purity would break the "where did this come from?" trail. Leaving it in. --- .../src/controllers/statusPageController.ts | 34 ++---- server/src/controllers/statusPageThemeFlag.ts | 33 ++++++ .../controllers/statusPageThemeFlag.test.ts | 106 ++++++++++++++++++ 3 files changed, 146 insertions(+), 27 deletions(-) create mode 100644 server/src/controllers/statusPageThemeFlag.ts create mode 100644 server/test/unit/controllers/statusPageThemeFlag.test.ts diff --git a/server/src/controllers/statusPageController.ts b/server/src/controllers/statusPageController.ts index 20fadbab6..547321d78 100644 --- a/server/src/controllers/statusPageController.ts +++ b/server/src/controllers/statusPageController.ts @@ -12,7 +12,7 @@ import { IStatusPageService } from "@/service/business/statusPageService.js"; import { IMonitorsRepository } from "@/repositories/index.js"; import { ISettingsService } from "@/service/system/settingsService.js"; import { NormalizeData } from "@/utils/dataUtils.js"; -import { DEFAULT_STATUS_PAGE_THEME, DEFAULT_STATUS_PAGE_THEME_MODE, type StatusPage } from "@/types/statusPage.js"; +import { applyDefaultTheme, withoutThemeFields } from "@/controllers/statusPageThemeFlag.js"; const SERVICE_NAME = "statusPageController"; @@ -40,26 +40,6 @@ class StatusPageController implements IStatusPageController { return StatusPageController.SERVICE_NAME; } - // Used when STATUS_PAGE_THEMES_ENABLED is false: drop incoming theme fields - // before persisting so an admin client can't accidentally store a value the - // public page won't honour. - private withoutThemeFields = (body: Record): Record => { - const rest = { ...body }; - delete rest.theme; - delete rest.themeMode; - return rest; - }; - - // Used when STATUS_PAGE_THEMES_ENABLED is false: overwrite the stored - // theme/themeMode with their defaults on read so callers see consistent - // values regardless of what's in the DB. Whatever a user previously chose - // is preserved in Mongo and resurfaces if the flag is flipped back on. - private applyDefaultTheme = (statusPage: StatusPage): StatusPage => ({ - ...statusPage, - theme: DEFAULT_STATUS_PAGE_THEME, - themeMode: DEFAULT_STATUS_PAGE_THEME_MODE, - }); - createStatusPage = async (req: Request, res: Response, next: NextFunction) => { try { createStatusPageBodyValidation.parse(req.body); @@ -68,7 +48,7 @@ class StatusPageController implements IStatusPageController { } const themesEnabled = this.settingsService.areStatusPageThemesEnabled(); - const body = themesEnabled ? req.body : this.withoutThemeFields(req.body); + const body = themesEnabled ? req.body : withoutThemeFields(req.body); const teamId = requireTeamId(req?.user?.teamId); const userId = requireUserId(req?.user?.id); const statusPage = await this.statusPageService.createStatusPage(userId, teamId, req.file, body); @@ -76,7 +56,7 @@ class StatusPageController implements IStatusPageController { return res.status(200).json({ success: true, msg: "Status page created successfully", - data: themesEnabled ? statusPage : this.applyDefaultTheme(statusPage), + data: themesEnabled ? statusPage : applyDefaultTheme(statusPage), }); } catch (error) { next(error); @@ -91,7 +71,7 @@ class StatusPageController implements IStatusPageController { } const themesEnabled = this.settingsService.areStatusPageThemesEnabled(); - const body = themesEnabled ? req.body : this.withoutThemeFields(req.body); + const body = themesEnabled ? req.body : withoutThemeFields(req.body); const teamId = requireTeamId(req?.user?.teamId); const statusPageId = req.params.id as string; if (!statusPageId) { @@ -104,7 +84,7 @@ class StatusPageController implements IStatusPageController { res.status(200).json({ success: true, msg: "Status page updated successfully", - data: themesEnabled ? statusPage : this.applyDefaultTheme(statusPage), + data: themesEnabled ? statusPage : applyDefaultTheme(statusPage), }); } catch (error) { next(error); @@ -155,7 +135,7 @@ class StatusPageController implements IStatusPageController { success: true, msg: "Status page retrieved successfully", data: { - statusPage: themesEnabled ? statusPage : this.applyDefaultTheme(statusPage), + statusPage: themesEnabled ? statusPage : applyDefaultTheme(statusPage), monitors: normalizedMonitors, }, }); @@ -172,7 +152,7 @@ class StatusPageController implements IStatusPageController { return res.status(200).json({ success: true, msg: "Status pages retrieved successfully", - data: themesEnabled ? statusPages : statusPages.map((sp) => this.applyDefaultTheme(sp)), + data: themesEnabled ? statusPages : statusPages.map((sp) => applyDefaultTheme(sp)), }); } catch (error) { next(error); diff --git a/server/src/controllers/statusPageThemeFlag.ts b/server/src/controllers/statusPageThemeFlag.ts new file mode 100644 index 000000000..bd65d5cb2 --- /dev/null +++ b/server/src/controllers/statusPageThemeFlag.ts @@ -0,0 +1,33 @@ +import { DEFAULT_STATUS_PAGE_THEME, DEFAULT_STATUS_PAGE_THEME_MODE, type StatusPage } from "@/types/statusPage.js"; + +/** + * When STATUS_PAGE_THEMES_ENABLED is false, the controller drops incoming + * theme fields before persisting so an admin client can't accidentally store + * a value the public page won't honour. Whatever a caller sends here for + * `theme` / `themeMode` is silently ignored — the rest of the body is + * preserved verbatim. + * + * The strip is silent (not a 400) because the kill switch is an ops-side + * concern: API clients should not have to know whether the operator has + * disabled themes today, only that their saved value won't take effect. + * If we later decide we'd rather fail loudly, change this to throw a + * 400 AppError; downstream consumers already handle it. + */ +export const withoutThemeFields = (body: Record): Record => { + const rest = { ...body }; + delete rest.theme; + delete rest.themeMode; + return rest; +}; + +/** + * When STATUS_PAGE_THEMES_ENABLED is false, callers reading a status page + * should see the canonical default theme / mode regardless of what's stored + * in Mongo. Whatever the user previously chose stays in the DB so flipping + * the flag back on resurfaces their selection. + */ +export const applyDefaultTheme = (statusPage: StatusPage): StatusPage => ({ + ...statusPage, + theme: DEFAULT_STATUS_PAGE_THEME, + themeMode: DEFAULT_STATUS_PAGE_THEME_MODE, +}); diff --git a/server/test/unit/controllers/statusPageThemeFlag.test.ts b/server/test/unit/controllers/statusPageThemeFlag.test.ts new file mode 100644 index 000000000..0ae10556a --- /dev/null +++ b/server/test/unit/controllers/statusPageThemeFlag.test.ts @@ -0,0 +1,106 @@ +import { describe, expect, it } from "@jest/globals"; +import { applyDefaultTheme, withoutThemeFields } from "../../../src/controllers/statusPageThemeFlag.ts"; +import { DEFAULT_STATUS_PAGE_THEME, DEFAULT_STATUS_PAGE_THEME_MODE } from "../../../src/types/statusPage.ts"; +import type { StatusPage } from "../../../src/types/index.ts"; + +const makeStatusPage = (overrides?: Partial): StatusPage => + ({ + id: "sp-1", + teamId: "team-1", + userId: "user-1", + url: "my-status-page", + companyName: "Test Co", + monitors: ["mon-1"], + theme: "modern", + themeMode: "dark", + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + ...overrides, + }) as StatusPage; + +describe("statusPageThemeFlag", () => { + describe("withoutThemeFields", () => { + it("removes theme and themeMode from a body that has them", () => { + const body = { companyName: "Co", theme: "modern", themeMode: "dark" }; + const result = withoutThemeFields(body); + expect(result).toEqual({ companyName: "Co" }); + }); + + it("is a no-op when neither field is present", () => { + const body = { companyName: "Co", url: "page" }; + const result = withoutThemeFields(body); + expect(result).toEqual({ companyName: "Co", url: "page" }); + }); + + it("removes only the theme fields when one is present", () => { + const body = { companyName: "Co", theme: "bold" }; + const result = withoutThemeFields(body); + expect(result).toEqual({ companyName: "Co" }); + }); + + it("does not mutate the input body", () => { + const body = { companyName: "Co", theme: "modern", themeMode: "dark" }; + withoutThemeFields(body); + expect(body).toEqual({ companyName: "Co", theme: "modern", themeMode: "dark" }); + }); + + it("preserves all other fields verbatim", () => { + const body = { + companyName: "Co", + monitors: ["m-1", "m-2"], + isPublished: true, + showCharts: false, + color: "#13715B", + theme: "editorial", + themeMode: "light", + }; + const result = withoutThemeFields(body); + expect(result).toEqual({ + companyName: "Co", + monitors: ["m-1", "m-2"], + isPublished: true, + showCharts: false, + color: "#13715B", + }); + }); + }); + + describe("applyDefaultTheme", () => { + it("overwrites theme and themeMode with the canonical defaults", () => { + const sp = makeStatusPage({ theme: "modern", themeMode: "dark" }); + const result = applyDefaultTheme(sp); + expect(result.theme).toBe(DEFAULT_STATUS_PAGE_THEME); + expect(result.themeMode).toBe(DEFAULT_STATUS_PAGE_THEME_MODE); + }); + + it("leaves all other fields untouched", () => { + const sp = makeStatusPage({ + companyName: "Test Co", + url: "my-page", + monitors: ["mon-1", "mon-2"], + isPublished: true, + color: "#13715B", + }); + const result = applyDefaultTheme(sp); + expect(result.companyName).toBe("Test Co"); + expect(result.url).toBe("my-page"); + expect(result.monitors).toEqual(["mon-1", "mon-2"]); + expect(result.isPublished).toBe(true); + expect(result.color).toBe("#13715B"); + }); + + it("does not mutate the input status page", () => { + const sp = makeStatusPage({ theme: "modern", themeMode: "dark" }); + applyDefaultTheme(sp); + expect(sp.theme).toBe("modern"); + expect(sp.themeMode).toBe("dark"); + }); + + it("works on a status page that has no theme set yet", () => { + const sp = makeStatusPage({ theme: undefined, themeMode: undefined }); + const result = applyDefaultTheme(sp); + expect(result.theme).toBe(DEFAULT_STATUS_PAGE_THEME); + expect(result.themeMode).toBe(DEFAULT_STATUS_PAGE_THEME_MODE); + }); + }); +});