From a2786a77ec3f401d5848d4e16080d95293a09dde Mon Sep 17 00:00:00 2001 From: gorkem-bwl Date: Mon, 27 Apr 2026 19:33:03 -0400 Subject: [PATCH] refactor(status-page): tighten the theme-fields backend after second review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apply review feedback: - Extract makeStatusPage from the two test files that duplicated it (test/unit/services/statusPageService.test.ts and test/unit/controllers/statusPageThemeFlag.test.ts) into test/helpers/makeStatusPage.ts. Both files now import the shared helper, matching the existing test/helpers/ convention used for repository fakes and provider contracts - Add a blank line in statusPageController.getStatusPageByUrl between the normalizedMonitors map and the themesEnabled lookup so the response-shaping section breathes a bit - Drop the obsolete localStorage references from the plan doc that survived from an earlier draft of the design — the public page never persisted a per-visitor mode choice in localStorage, so the test bullet under 2.11 and the risk row both pointed at functionality that does not exist --- docs/plans/status-page-themes.md | 3 +-- server/src/controllers/statusPageController.ts | 1 + server/test/helpers/makeStatusPage.ts | 14 ++++++++++++++ .../controllers/statusPageThemeFlag.test.ts | 17 +---------------- .../unit/services/statusPageService.test.ts | 17 +---------------- 5 files changed, 18 insertions(+), 34 deletions(-) create mode 100644 server/test/helpers/makeStatusPage.ts diff --git a/docs/plans/status-page-themes.md b/docs/plans/status-page-themes.md index eb460766e..f7fb97331 100644 --- a/docs/plans/status-page-themes.md +++ b/docs/plans/status-page-themes.md @@ -232,7 +232,7 @@ Keys to add: **Component tests (existing test setup):** - Admin form: selecting a theme card updates form state and survives submit - Admin form: `themeMode` dropdown changes payload -- `StatusPageThemeProvider`: `auto` mode resolves to `prefers-color-scheme` when no localStorage; localStorage overrides when set; admin-forced mode wins over both +- `StatusPageThemeProvider`: `auto` mode resolves to `prefers-color-scheme`; admin-forced `light`/`dark` modes win over the OS setting - Mode resolution: `themeMode="auto"` picks up `prefers-color-scheme`; `themeMode="light"` and `"dark"` lock the mode regardless of OS setting - Admin picker shows both light + dark preview SVGs for every theme, regardless of admin UI mode - Jest/Vitest snapshot tests (plain, no visual-regression tool): `Status/index.tsx` rendered under each of `refined | modern | bold | editorial` × `light | dark`. 8 snapshots via a single parametrised test. @@ -263,7 +263,6 @@ Two PRs, small-enough individually to review carefully. | Four themes = 4× visual-regression surface going forward | Parametrised snapshot tests in PR 2 cover all 4 themes × both modes × key status states. Regressions caught at PR time. | | Editorial hairline dividers fail WCAG 3:1 for UI borders | Axe pass in 2.12; darken divider token until it clears. | | Older clients posting create/update without `theme` break | Validator makes fields optional; model default handles it. Covered by backend test 1.6. | -| Per-page localStorage theme mode leaks across status pages | Storage key namespaced by `statusPage.url`. Covered in 2.3 and 2.11. | | `customCSS` collides with theme tokens | Themes set CSS variables on a wrapper; `customCSS` is injected inline after and can override any `--sp-*` variable. No code change. Document in settings help text. | | Shared chart components look off-brand on bold/modern dark | Decided in 2.6: pass an optional `paletteOverride` prop from the status page subtree. | | Rollback needed if feature misbehaves in prod | Feature flag `STATUS_PAGE_THEMES_ENABLED` in 1.5 disables writes and forces `refined` on reads. | diff --git a/server/src/controllers/statusPageController.ts b/server/src/controllers/statusPageController.ts index 547321d78..706ee5ebd 100644 --- a/server/src/controllers/statusPageController.ts +++ b/server/src/controllers/statusPageController.ts @@ -130,6 +130,7 @@ class StatusPageController implements IStatusPageController { } return { ...monitor, checks: normalizedChecks }; }); + const themesEnabled = this.settingsService.areStatusPageThemesEnabled(); return res.status(200).json({ success: true, diff --git a/server/test/helpers/makeStatusPage.ts b/server/test/helpers/makeStatusPage.ts new file mode 100644 index 000000000..23c13bca1 --- /dev/null +++ b/server/test/helpers/makeStatusPage.ts @@ -0,0 +1,14 @@ +import type { StatusPage } from "../../src/types/index.ts"; + +export const makeStatusPage = (overrides?: Partial): StatusPage => + ({ + id: "sp-1", + teamId: "team-1", + userId: "user-1", + url: "my-status-page", + companyName: "Test Co", + monitors: ["mon-1"], + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + ...overrides, + }) as StatusPage; diff --git a/server/test/unit/controllers/statusPageThemeFlag.test.ts b/server/test/unit/controllers/statusPageThemeFlag.test.ts index 0ae10556a..b4f9435a9 100644 --- a/server/test/unit/controllers/statusPageThemeFlag.test.ts +++ b/server/test/unit/controllers/statusPageThemeFlag.test.ts @@ -1,22 +1,7 @@ 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; +import { makeStatusPage } from "../../helpers/makeStatusPage.ts"; describe("statusPageThemeFlag", () => { describe("withoutThemeFields", () => { diff --git a/server/test/unit/services/statusPageService.test.ts b/server/test/unit/services/statusPageService.test.ts index ea3d1ce82..d50fd92f7 100644 --- a/server/test/unit/services/statusPageService.test.ts +++ b/server/test/unit/services/statusPageService.test.ts @@ -1,22 +1,7 @@ import { describe, expect, it, jest } from "@jest/globals"; import { StatusPageService } from "../../../src/service/business/statusPageService.ts"; import type { IStatusPagesRepository } from "../../../src/repositories/status-pages/IStatusPagesRepository.ts"; -import type { StatusPage } from "../../../src/types/index.ts"; - -// ── Helpers ────────────────────────────────────────────────────────────────── - -const makeStatusPage = (overrides?: Partial): StatusPage => - ({ - id: "sp-1", - teamId: "team-1", - userId: "user-1", - url: "my-status-page", - companyName: "Test Co", - monitors: ["mon-1"], - createdAt: "2026-01-01T00:00:00Z", - updatedAt: "2026-01-01T00:00:00Z", - ...overrides, - }) as StatusPage; +import { makeStatusPage } from "../../helpers/makeStatusPage.ts"; const createRepo = () => ({