From c7329febb41c9841c45282ec19b427044e980c49 Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 19 Oct 2026 11:20:56 -0600 Subject: [PATCH 1/3] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Load=20the=20email=20s?= =?UTF-8?q?ettings=20in=20the=20dialog=20instead=20of=20keeping=20them=20i?= =?UTF-8?q?n=20the=20store?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The store now keeps only page-level state: whether email notifications are enabled, and the permissions. The configuration dialog loads the settings it edits each time it opens, and they are dropped when it closes. The SMTP password is never copied from the response. ServiceControl versions before 6.21.2 replace the stored password with an empty one. When the password field is left empty, the stored password is read and sent back with the save request, so it is kept. If it cannot be read, the save fails instead of erasing it. If the settings cannot be loaded, the dialog says so and cannot be saved. The on/off switch is no longer changed by opening the dialog. --- .../components/configuration/EmailSettings.ts | 3 +- .../HealthCheckNotifications.spec.ts | 188 +++++++------ .../HealthCheckNotifications.vue | 12 +- ...ealthCheckNotifications_ConfigureEmail.vue | 47 +++- .../src/resources/EmailNotifications.ts | 1 + .../src/stores/HealthChecksStore.spec.ts | 249 ++++++++++++++---- src/Frontend/src/stores/HealthChecksStore.ts | 79 +++--- 7 files changed, 365 insertions(+), 214 deletions(-) diff --git a/src/Frontend/src/components/configuration/EmailSettings.ts b/src/Frontend/src/components/configuration/EmailSettings.ts index 725b07323a..30c00e9008 100644 --- a/src/Frontend/src/components/configuration/EmailSettings.ts +++ b/src/Frontend/src/components/configuration/EmailSettings.ts @@ -1,10 +1,9 @@ +// The settings the email configuration dialog edits. The SMTP password is write-only, so it is not part of them. export default interface EmailSettings { - enabled: boolean | null; enable_tls: boolean | null; smtp_server: string; smtp_port: number | null; authentication_account: string; - authentication_password: string; from: string; to: string; } diff --git a/src/Frontend/src/components/configuration/HealthCheckNotifications.spec.ts b/src/Frontend/src/components/configuration/HealthCheckNotifications.spec.ts index 494970b371..6ef8fe9397 100644 --- a/src/Frontend/src/components/configuration/HealthCheckNotifications.spec.ts +++ b/src/Frontend/src/components/configuration/HealthCheckNotifications.spec.ts @@ -1,8 +1,9 @@ -import { render, describe, test, screen, expect, userEvent } from "@component-test-utils"; +import { render, describe, test, screen, within, expect, userEvent } from "@component-test-utils"; +import { waitFor } from "@testing-library/vue"; import { vi, beforeEach, afterEach } from "vitest"; import sut from "@/components/configuration/HealthCheckNotifications.vue"; import { createTestingPinia } from "@pinia/testing"; -import { useHealthChecksStore } from "@/stores/HealthChecksStore"; +import { useHealthChecksStore, type HealthChecksStore } from "@/stores/HealthChecksStore"; import type EmailSettings from "@/components/configuration/EmailSettings"; // ─── DSL ───────────────────────────────────────────────────────────────────── @@ -16,6 +17,7 @@ interface ComponentDSL { requestTestNotification(): Promise; provideValidEmailConfiguration(): Promise; settingsChangeInServiceControl(changes: Partial): void; + serviceControlBecomesUnreachable(): void; }; verify: { emailNotificationsAreEnabled(): void; @@ -33,9 +35,16 @@ interface ComponentDSL { emailConfigurationMatches(config: Partial): void; storedPasswordIsNotShownAndCanBeKept(): void; limitingTheSmtpAccountIsRecommended(): void; + emailSettingsCouldNotBeLoaded(): void; }; } +// The store state a test can start from. Only page-level state lives in the store; the settings the dialog +// edits are provided with settingsInServiceControl. +interface InitialState { + HealthChecksStore?: Partial>; +} + // ─── Test suite ────────────────────────────────────────────────────────────── // The email configuration modal is rendered via . @@ -55,8 +64,8 @@ afterEach(() => { }); describe("FEATURE: Health check notifications", () => { - describe("RULE: Email notifications can be enabled and disabled", () => { - test("EXAMPLE: Email notifications are currently disabled", async () => { + describe("RULE: Must let email notifications be turned on and off", () => { + test("EXAMPLE: The one where email notifications are off and get switched on", async () => { /* * Given Email notifications are OFF * When the toggle button is clicked @@ -65,7 +74,7 @@ describe("FEATURE: Health check notifications", () => { const componentDriver = renderComponent({ initialState: { HealthChecksStore: { - emailNotifications: emailSettingsWithEnabled(false), + emailNotificationsEnabled: false, }, }, }); @@ -75,7 +84,7 @@ describe("FEATURE: Health check notifications", () => { componentDriver.verify.emailNotificationsWereToggled(); }); - test("EXAMPLE: Email notifications are currently enabled", async () => { + test("EXAMPLE: The one where email notifications are on and get switched off", async () => { /* * Given Email notifications are ON * When the toggle button is clicked @@ -84,7 +93,7 @@ describe("FEATURE: Health check notifications", () => { const componentDriver = renderComponent({ initialState: { HealthChecksStore: { - emailNotifications: emailSettingsWithEnabled(true), + emailNotificationsEnabled: true, }, }, }); @@ -95,8 +104,8 @@ describe("FEATURE: Health check notifications", () => { }); }); - describe("RULE: The email configuration dialog can be opened and closed", () => { - test("EXAMPLE: The configuration dialog is not currently visible", async () => { + describe("RULE: Must let the email configuration dialog be opened and closed without saving", () => { + test("EXAMPLE: The one where Configure is clicked, and the dialog opens", async () => { /* * Given the Email configuration popup is not visible * When the "Configure" button is clicked @@ -109,7 +118,7 @@ describe("FEATURE: Health check notifications", () => { componentDriver.verify.emailConfigurationIsAccessible(); }); - test("EXAMPLE: Edits have been made in the configuration dialog", async () => { + test("EXAMPLE: The one where edits are made and then cancelled, and nothing is saved", async () => { /* * Given the Email configuration popup is visible * And edits have been made to the email configuration @@ -128,49 +137,37 @@ describe("FEATURE: Health check notifications", () => { }); }); - describe("RULE: Email configuration can only be saved when the form is valid", () => { - test("EXAMPLE: Required fields are missing or contain invalid values", async () => { + describe("RULE: Must allow saving the email configuration only when the form is valid", () => { + test("EXAMPLE: The one where every required field is valid, and the configuration can be saved", async () => { /* * Given the Email configuration popup is visible - * When invalid or incomplete data is entered into the form - * Then the Save button is not enabled + * When valid data is entered for all required fields + * Then the Save button is enabled */ - const componentDriver = renderComponent({ - initialState: { - HealthChecksStore: { - // All fields empty – form is invalid by default - emailNotifications: emptyEmailSettings(), - }, - }, - }); + const componentDriver = renderComponent({ settingsInServiceControl: emptyEmailSettings() }); await componentDriver.actions.openEmailConfiguration(); componentDriver.verify.userCannotSaveConfiguration(); + await componentDriver.actions.provideValidEmailConfiguration(); + componentDriver.verify.userCanSaveConfiguration(); }); - test("EXAMPLE: All required fields are present and valid", async () => { + test("COUNTER-EXAMPLE: The one where required fields are missing or invalid, and the configuration cannot be saved", async () => { /* * Given the Email configuration popup is visible - * When valid data is entered for all required fields - * Then the Save button is enabled + * When invalid or incomplete data is entered into the form + * Then the Save button is not enabled */ - const componentDriver = renderComponent({ - initialState: { - HealthChecksStore: { - emailNotifications: emptyEmailSettings(), - }, - }, - }); + // All fields empty – form is invalid by default + const componentDriver = renderComponent({ settingsInServiceControl: emptyEmailSettings() }); await componentDriver.actions.openEmailConfiguration(); componentDriver.verify.userCannotSaveConfiguration(); - await componentDriver.actions.provideValidEmailConfiguration(); - componentDriver.verify.userCanSaveConfiguration(); }); }); - describe("RULE: Saving email configuration updates the stored settings and closes the dialog", () => { - test("EXAMPLE: A valid configuration has been entered", async () => { + describe("RULE: Must close the dialog once a valid email configuration is saved", () => { + test("EXAMPLE: The one where a valid configuration is saved, and the dialog closes", async () => { /* * Given the Email configuration popup is visible * And a valid configuration has been entered @@ -178,13 +175,7 @@ describe("FEATURE: Health check notifications", () => { * Then the email configuration is updated * And the Email configuration popup is closed */ - const componentDriver = renderComponent({ - initialState: { - HealthChecksStore: { - emailNotifications: emptyEmailSettings(), - }, - }, - }); + const componentDriver = renderComponent({ settingsInServiceControl: emptyEmailSettings() }); await componentDriver.actions.openEmailConfiguration(); await componentDriver.actions.provideValidEmailConfiguration(); @@ -195,40 +186,32 @@ describe("FEATURE: Health check notifications", () => { }); }); - describe("RULE: Health check notification settings persist after a page refresh", () => { - test("EXAMPLE: Email configuration has been updated", async () => { + describe("RULE: Must show the saved settings after a page refresh", () => { + test("EXAMPLE: The one where the configuration was changed before the refresh, and the dialog shows what was last saved", async () => { /* * When the email configuration has been changed * And the screen is refreshed * Then the email notification configuration matches what was last saved * - * The store's refresh() action is stubbed, so the initial state we set - * represents what the store holds after loading previously-saved settings. + * The store's loadEmailSettings() action is stubbed, so the settings we + * provide represent what ServiceControl returns for previously-saved settings. */ const savedConfig: EmailSettings = { - enabled: true, enable_tls: true, smtp_server: "mail.example.com", smtp_port: 465, authentication_account: "user@example.com", - authentication_password: "secret", from: "noreply@example.com", to: "admin@example.com", }; - const componentDriver = renderComponent({ - initialState: { - HealthChecksStore: { - emailNotifications: savedConfig, - }, - }, - }); + const componentDriver = renderComponent({ settingsInServiceControl: savedConfig }); await componentDriver.actions.openEmailConfiguration(); componentDriver.verify.emailConfigurationMatches(savedConfig); }); - test("EXAMPLE: Email notifications were enabled before refresh", () => { + test("EXAMPLE: The one where email notifications were on before the refresh, and they are still shown as on", () => { /* * Given Email notifications are ON * When the page is refreshed @@ -237,7 +220,7 @@ describe("FEATURE: Health check notifications", () => { const componentDriver = renderComponent({ initialState: { HealthChecksStore: { - emailNotifications: emailSettingsWithEnabled(true), + emailNotificationsEnabled: true, }, }, }); @@ -245,7 +228,7 @@ describe("FEATURE: Health check notifications", () => { componentDriver.verify.emailNotificationsAreEnabled(); }); - test("EXAMPLE: Email notifications were disabled before refresh", () => { + test("EXAMPLE: The one where email notifications were off before the refresh, and they are still shown as off", () => { /* * Given Email notifications are OFF * When the page is refreshed @@ -254,7 +237,7 @@ describe("FEATURE: Health check notifications", () => { const componentDriver = renderComponent({ initialState: { HealthChecksStore: { - emailNotifications: emailSettingsWithEnabled(false), + emailNotificationsEnabled: false, }, }, }); @@ -263,30 +246,48 @@ describe("FEATURE: Health check notifications", () => { }); }); - describe("RULE: The email configuration dialog shows the settings that ServiceControl has now", () => { - test("EXAMPLE: Another user changed the settings after the page was loaded", async () => { + describe("RULE: Must show the settings ServiceControl has now when the dialog opens", () => { + test("EXAMPLE: The one where another user changed the settings after the page loaded, and the dialog shows their change", async () => { /* * Given the page was loaded with the To address old@example.com * And another user changed the To address in ServiceControl to new@example.com * When the Email configuration popup is opened * Then the To address is new@example.com */ + const componentDriver = renderComponent({ settingsInServiceControl: { ...emptyEmailSettings(), to: "old@example.com" } }); + + componentDriver.actions.settingsChangeInServiceControl({ to: "new@example.com" }); + await componentDriver.actions.openEmailConfiguration(); + componentDriver.verify.emailConfigurationMatches({ to: "new@example.com" }); + }); + + test("COUNTER-EXAMPLE: The one where ServiceControl cannot be reached when the dialog opens, and it says the settings could not be loaded", async () => { + /* + * Given Email notifications are ON + * And ServiceControl cannot be reached + * When the Email configuration popup is opened + * Then it says that the settings could not be loaded + * And the configuration cannot be saved + * And Email notifications are still shown as ON + */ const componentDriver = renderComponent({ initialState: { HealthChecksStore: { - emailNotifications: { ...emptyEmailSettings(), to: "old@example.com" }, + emailNotificationsEnabled: true, }, }, }); - componentDriver.actions.settingsChangeInServiceControl({ to: "new@example.com" }); + componentDriver.actions.serviceControlBecomesUnreachable(); await componentDriver.actions.openEmailConfiguration(); - componentDriver.verify.emailConfigurationMatches({ to: "new@example.com" }); + componentDriver.verify.emailSettingsCouldNotBeLoaded(); + componentDriver.verify.userCannotSaveConfiguration(); + componentDriver.verify.emailNotificationsAreEnabled(); }); }); - describe("RULE: The stored SMTP password is never shown", () => { - test("EXAMPLE: ServiceControl has a stored password for the SMTP account", async () => { + describe("RULE: Must never show the stored SMTP password", () => { + test("EXAMPLE: The one where ServiceControl has a stored password, and the password field stays empty", async () => { /* * Given ServiceControl has a stored SMTP password * And ServiceControl does not return it @@ -294,41 +295,29 @@ describe("FEATURE: Health check notifications", () => { * Then the password field is empty * And the field explains that an empty password keeps the stored password */ - const componentDriver = renderComponent({ - initialState: { - HealthChecksStore: { - emailNotifications: { ...emptyEmailSettings(), authentication_account: "user@example.com" }, - }, - }, - }); + const componentDriver = renderComponent({ settingsInServiceControl: { ...emptyEmailSettings(), authentication_account: "user@example.com" } }); await componentDriver.actions.openEmailConfiguration(); componentDriver.verify.storedPasswordIsNotShownAndCanBeKept(); }); }); - describe("RULE: The email configuration recommends limiting what the SMTP account can send", () => { - test("EXAMPLE: The Email configuration popup is opened", async () => { + describe("RULE: Should recommend limiting what the SMTP account can send", () => { + test("EXAMPLE: The one where the dialog opens, and it recommends a dedicated SMTP account with limited send permissions", async () => { /* * When the Email configuration popup is opened * Then it recommends a dedicated SMTP account with limited send permissions * And it links to the documentation */ - const componentDriver = renderComponent({ - initialState: { - HealthChecksStore: { - emailNotifications: emptyEmailSettings(), - }, - }, - }); + const componentDriver = renderComponent({ settingsInServiceControl: emptyEmailSettings() }); await componentDriver.actions.openEmailConfiguration(); componentDriver.verify.limitingTheSmtpAccountIsRecommended(); }); }); - describe("RULE: Sending a test notification indicates whether delivery succeeded or failed", () => { - test("EXAMPLE: The email configuration is invalid", async () => { + describe("RULE: Must show whether a test notification was delivered", () => { + test("EXAMPLE: The one where the configuration is invalid, and the test notification is shown as failed", async () => { /* * Given an invalid configuration * When "Send test notification" is clicked @@ -345,7 +334,7 @@ describe("FEATURE: Health check notifications", () => { await componentDriver.verify.testNotificationFailed(); }); - test("EXAMPLE: The email configuration is valid", async () => { + test("EXAMPLE: The one where the configuration is valid, and the test notification is shown as sent", async () => { /* * Given a valid configuration * When "Send test notification" is clicked @@ -366,7 +355,7 @@ describe("FEATURE: Health check notifications", () => { // ─── Render helper ─────────────────────────────────────────────────────────── -function renderComponent({ initialState = {} }: { initialState?: Record } = {}): ComponentDSL { +function renderComponent({ initialState = {}, settingsInServiceControl = emptyEmailSettings() }: { initialState?: InitialState; settingsInServiceControl?: EmailSettings } = {}): ComponentDSL { const user = userEvent.setup(); render(sut, { @@ -391,6 +380,8 @@ function renderComponent({ initialState = {} }: { initialState?: Record expect(within(dialog).queryByRole("status")).not.toBeInTheDocument()); }, async confirmEmailConfigurationChanges() { await user.click(screen.getByRole("button", { name: /save/i })); @@ -410,10 +403,10 @@ function renderComponent({ initialState = {} }: { initialState?: Record) { - vi.mocked(store.refresh).mockImplementation(() => { - store.emailNotifications = { ...store.emailNotifications, ...changes }; - return Promise.resolve(); - }); + vi.mocked(store.loadEmailSettings).mockResolvedValue({ ...settingsInServiceControl, ...changes }); + }, + serviceControlBecomesUnreachable() { + vi.mocked(store.loadEmailSettings).mockResolvedValue(null); }, async provideValidEmailConfiguration() { await user.type(screen.getByLabelText(/smtp server address/i), "smtp.example.com"); @@ -465,6 +458,10 @@ function renderComponent({ initialState = {} }: { initialState?: Record {
- +
@@ -139,8 +137,8 @@ onMounted(async () => { - - + + diff --git a/src/Frontend/src/components/configuration/HealthCheckNotifications_ConfigureEmail.vue b/src/Frontend/src/components/configuration/HealthCheckNotifications_ConfigureEmail.vue index d98fb8315e..6a3586be43 100644 --- a/src/Frontend/src/components/configuration/HealthCheckNotifications_ConfigureEmail.vue +++ b/src/Frontend/src/components/configuration/HealthCheckNotifications_ConfigureEmail.vue @@ -1,25 +1,46 @@