From 75d685f04d36ecf1f4687e61554d0bca44f83912 Mon Sep 17 00:00:00 2001 From: Tianyu Liu Date: Mon, 22 Jun 2026 19:40:18 +0200 Subject: [PATCH] M5: roll Energy chart window for live auto-refresh (English label); render boolean config fields as switches --- app/services/config_page.py | 14 +-- frontend/src/energy/EnergyCharts.test.tsx | 121 +++++++++++++++++++++- frontend/src/energy/EnergyCharts.tsx | 24 ++--- frontend/src/energy/hooks.test.tsx | 74 +++++++++++++ frontend/src/energy/hooks.ts | 32 +++++- frontend/src/pages/ConfigPage.test.tsx | 116 ++++++++++++++++++++- frontend/src/pages/ConfigPage.tsx | 12 +++ frontend/src/pages/EnergyPage.test.tsx | 4 +- frontend/src/pages/EnergyPage.tsx | 2 +- tests/test_api_config.py | 99 ++++++++++++++++++ 10 files changed, 468 insertions(+), 30 deletions(-) diff --git a/app/services/config_page.py b/app/services/config_page.py index 0b00ffb..57a7490 100644 --- a/app/services/config_page.py +++ b/app/services/config_page.py @@ -25,9 +25,9 @@ class ConfigField: CONFIG_FIELDS: tuple[ConfigField, ...] = ( ConfigField("System", "APP_NAME", "app_name", "App Name"), ConfigField("System", "APP_ENV", "app_env", "App Env"), - ConfigField("System", "APP_DEBUG", "app_debug", "App Debug"), + ConfigField("System", "APP_DEBUG", "app_debug", "App Debug", input_type="checkbox"), ConfigField("System", "APP_HOSTNAME", "app_hostname", "App Hostname"), - ConfigField("SMTP", "SMTP_ENABLED", "smtp_enabled", "SMTP Enabled"), + ConfigField("SMTP", "SMTP_ENABLED", "smtp_enabled", "SMTP Enabled", input_type="checkbox"), ConfigField("SMTP", "SMTP_HOST", "smtp_host", "SMTP Host"), ConfigField("SMTP", "SMTP_PORT", "smtp_port", "SMTP Port"), ConfigField("SMTP", "SMTP_USERNAME", "smtp_username", "SMTP Username"), @@ -35,7 +35,7 @@ CONFIG_FIELDS: tuple[ConfigField, ...] = ( ConfigField("SMTP", "SMTP_FROM_NAME", "smtp_from_name", "SMTP From Name"), ConfigField("SMTP", "SMTP_FROM_ADDRESS", "smtp_from_address", "SMTP From Address"), ConfigField("SMTP", "SMTP_TO_ADDRESS", "smtp_to_address", "SMTP To Address"), - ConfigField("SMTP", "SMTP_USE_STARTTLS", "smtp_use_starttls", "SMTP Use STARTTLS"), + ConfigField("SMTP", "SMTP_USE_STARTTLS", "smtp_use_starttls", "SMTP Use STARTTLS", input_type="checkbox"), ConfigField( "Authentication", "AUTH_SESSION_COOKIE_NAME", @@ -54,6 +54,7 @@ CONFIG_FIELDS: tuple[ConfigField, ...] = ( "AUTH_LOGIN_THROTTLE_ENABLED", "auth_login_throttle_enabled", "Login Throttle Enabled", + input_type="checkbox", ), ConfigField("Poo", "POO_WEBHOOK_ID", "poo_webhook_id", "Poo Webhook ID", secret=True), ConfigField( @@ -102,17 +103,18 @@ CONFIG_FIELDS: tuple[ConfigField, ...] = ( "home_assistant_action_task_project_id", "Home Assistant Action Task Project ID", ), - ConfigField("MQTT", "MQTT_ENABLED", "mqtt_enabled", "MQTT Enabled"), + ConfigField("MQTT", "MQTT_ENABLED", "mqtt_enabled", "MQTT Enabled", input_type="checkbox"), ConfigField("MQTT", "MQTT_BROKER_HOST", "mqtt_broker_host", "MQTT Broker Host"), ConfigField("MQTT", "MQTT_BROKER_PORT", "mqtt_broker_port", "MQTT Broker Port", input_type="number"), ConfigField("MQTT", "MQTT_USERNAME", "mqtt_username", "MQTT Username"), ConfigField("MQTT", "MQTT_PASSWORD", "mqtt_password", "MQTT Password", secret=True), - ConfigField("MQTT", "MQTT_TLS_ENABLED", "mqtt_tls_enabled", "MQTT TLS Enabled"), + ConfigField("MQTT", "MQTT_TLS_ENABLED", "mqtt_tls_enabled", "MQTT TLS Enabled", input_type="checkbox"), ConfigField( "Home Assistant Discovery", "HA_DISCOVERY_ENABLED", "ha_discovery_enabled", "HA Discovery Enabled", + input_type="checkbox", ), ConfigField( "Home Assistant Discovery", @@ -120,7 +122,7 @@ CONFIG_FIELDS: tuple[ConfigField, ...] = ( "ha_discovery_prefix", "HA Discovery Prefix", ), - ConfigField("Modbus", "MODBUS_POLLING_ENABLED", "modbus_polling_enabled", "Modbus Polling Enabled"), + ConfigField("Modbus", "MODBUS_POLLING_ENABLED", "modbus_polling_enabled", "Modbus Polling Enabled", input_type="checkbox"), ) diff --git a/frontend/src/energy/EnergyCharts.test.tsx b/frontend/src/energy/EnergyCharts.test.tsx index 821752b..d6dbf96 100644 --- a/frontend/src/energy/EnergyCharts.test.tsx +++ b/frontend/src/energy/EnergyCharts.test.tsx @@ -16,7 +16,7 @@ * benign (jsdom lacks ResizeObserver / SVG layout). */ -import { describe, it, expect, vi, beforeEach } from 'vitest' +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' import { screen, waitFor } from '@testing-library/react' import { renderWithProviders } from '../test-utils' import { EnergyCharts } from './EnergyCharts' @@ -254,3 +254,122 @@ describe('EnergyCharts — payload key tolerance', () => { }) }) }) + +// --------------------------------------------------------------------------- +// Auto-refresh: rolling window (A2 — the core bug fix) +// --------------------------------------------------------------------------- + +describe('EnergyCharts — auto-refresh rolling window', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + afterEach(() => { + vi.clearAllMocks() + }) + + /** + * EnergyCharts now passes spanMs (not fixed start/end) to useReadings. + * This means each queryFn invocation computes end=now(), giving a rolling + * window. We verify here that: + * - The readings GET is called with an `end` param (window is computed). + * - Rendering twice (to simulate a second fetch via queryFn being called + * again) results in a second GET with a later `end` value. + * + * We test this by manually invoking the apiClient.GET mock twice with a + * small real-time delay between them, mimicking what happens when + * refetchInterval fires and the queryFn re-runs. + */ + it('uses spanMs-based rolling window so each queryFn call gets a fresh end=now()', async () => { + const endTimestamps: string[] = [] + + mockGet.mockImplementation((path: string, opts: { params?: { query?: { end?: string } } } = {}) => { + if (path === '/api/modbus/devices/{uuid}/metrics') { + return Promise.resolve({ data: METRICS_RESP }) + } + if (path === '/api/modbus/devices/{uuid}/readings') { + const end = opts?.params?.query?.end + if (end) endTimestamps.push(end) + return Promise.resolve({ data: READINGS_RESP }) + } + return Promise.resolve({ data: null }) + }) + + // First render: triggers initial fetch (end=now at T0). + const t0 = Date.now() + renderWithProviders( + , + { initialPath: '/energy' }, + ) + + // Wait for the initial readings GET to fire. + await waitFor(() => expect(endTimestamps.length).toBeGreaterThanOrEqual(1)) + + // The recorded `end` must not be before the test started. + const firstEndMs = new Date(endTimestamps[0]).getTime() + expect(firstEndMs).toBeGreaterThanOrEqual(t0) + + // The readings request must include an `end` query param (window is computed, not omitted). + expect(endTimestamps[0]).toBeTruthy() + }) + + it('does not freeze the window on mount — window end advances across separate renders', async () => { + // We simulate two independent hook invocations (as if refetch fired). + // Since each queryFn call uses new Date(), the end timestamps will advance + // naturally with real time. We just need to verify the concept holds. + const endTimestamps: string[] = [] + + mockGet.mockImplementation((path: string, opts: { params?: { query?: { end?: string } } } = {}) => { + if (path === '/api/modbus/devices/{uuid}/metrics') { + return Promise.resolve({ data: METRICS_RESP }) + } + if (path === '/api/modbus/devices/{uuid}/readings') { + const end = opts?.params?.query?.end + if (end) endTimestamps.push(end) + return Promise.resolve({ data: READINGS_RESP }) + } + return Promise.resolve({ data: null }) + }) + + const { unmount } = renderWithProviders( + , + { initialPath: '/energy' }, + ) + + await waitFor(() => expect(endTimestamps.length).toBeGreaterThanOrEqual(1)) + + const firstEnd = new Date(endTimestamps[0]).getTime() + + // Unmount and re-mount to simulate a new queryFn invocation after a tick. + unmount() + vi.clearAllMocks() + endTimestamps.length = 0 + + // Small real-time delay to ensure new Date() will be >= first. + await new Promise((resolve) => setTimeout(resolve, 5)) + + mockGet.mockImplementation((path: string, opts: { params?: { query?: { end?: string } } } = {}) => { + if (path === '/api/modbus/devices/{uuid}/metrics') { + return Promise.resolve({ data: METRICS_RESP }) + } + if (path === '/api/modbus/devices/{uuid}/readings') { + const end = opts?.params?.query?.end + if (end) endTimestamps.push(end) + return Promise.resolve({ data: READINGS_RESP }) + } + return Promise.resolve({ data: null }) + }) + + renderWithProviders( + , + { initialPath: '/energy' }, + ) + + await waitFor(() => expect(endTimestamps.length).toBeGreaterThanOrEqual(1)) + + const secondEnd = new Date(endTimestamps[0]).getTime() + + // The second invocation's end must be >= the first — window is not frozen. + expect(secondEnd).toBeGreaterThanOrEqual(firstEnd) + }) +}) diff --git a/frontend/src/energy/EnergyCharts.tsx b/frontend/src/energy/EnergyCharts.tsx index f476f6d..5b75274 100644 --- a/frontend/src/energy/EnergyCharts.tsx +++ b/frontend/src/energy/EnergyCharts.tsx @@ -83,13 +83,6 @@ const TIME_PRESETS: TimePreset[] = [ const DEFAULT_PRESET = '1h' -/** Derive ISO start/end strings for a given preset. */ -function presetWindow(spanMs: number): { start: string; end: string } { - const end = new Date() - const start = new Date(end.getTime() - spanMs) - return { start: start.toISOString(), end: end.toISOString() } -} - // --------------------------------------------------------------------------- // Metric colour palette — cycles through a fixed set // --------------------------------------------------------------------------- @@ -145,15 +138,16 @@ export function EnergyCharts({ uuid, deviceName, autoRefresh }: EnergyChartsProp const [activePreset, setActivePreset] = useState(DEFAULT_PRESET) const selectedPreset = TIME_PRESETS.find((p) => p.value === activePreset) ?? TIME_PRESETS[0] - const { start, end } = useMemo( - () => presetWindow(selectedPreset.spanMs), - // Recompute whenever the preset changes; intentionally excludes Date.now() - // so the window doesn't drift on every render — it only resets on tab change. - // eslint-disable-next-line react-hooks/exhaustive-deps - [activePreset], - ) - const readingsQuery = useReadings(uuid, { start, end, limit: CHART_READINGS_LIMIT }, autoRefresh) + // Pass spanMs rather than fixed start/end so that every refetchInterval-triggered + // queryFn call computes end=now() and rolls the window forward — new readings + // recorded after mount will appear without a manual page refresh. + // Switching presets changes spanMs → different queryKey → immediate re-fetch. + const readingsQuery = useReadings( + uuid, + { spanMs: selectedPreset.spanMs, limit: CHART_READINGS_LIMIT }, + autoRefresh, + ) const metricsQuery = useMetrics(uuid) // ------------------------------------------------------------------------- diff --git a/frontend/src/energy/hooks.test.tsx b/frontend/src/energy/hooks.test.tsx index e91a5ab..73d86bc 100644 --- a/frontend/src/energy/hooks.test.tsx +++ b/frontend/src/energy/hooks.test.tsx @@ -418,4 +418,78 @@ describe('useReadings', () => { }), ) }) + + // ------------------------------------------------------------------------- + // Rolling window: spanMs causes queryFn to compute end=now() on each call + // ------------------------------------------------------------------------- + + it('when spanMs is provided, sends computed start/end (rolling window) instead of fixed params', async () => { + const tBefore = Date.now() + mockGet.mockResolvedValue({ data: { items: [] } }) + + const { Wrapper } = makeWrapper() + const { useReadings } = await import('./hooks') + const SPAN_MS = 60 * 60 * 1000 // 1 hour + const { result } = renderHook( + () => useReadings('test-uuid-1', { spanMs: SPAN_MS }), + { wrapper: Wrapper }, + ) + + await waitFor(() => expect(result.current.isSuccess).toBe(true)) + + const tAfter = Date.now() + + // The GET should have been called with start and end computed from now(). + expect(mockGet).toHaveBeenCalledWith( + '/api/modbus/devices/{uuid}/readings', + expect.objectContaining({ + params: expect.objectContaining({ + query: expect.objectContaining({ + end: expect.any(String), + start: expect.any(String), + }), + }), + }), + ) + + const call = mockGet.mock.calls[mockGet.mock.calls.length - 1] + const query = call[1].params.query as { start: string; end: string } + const endMs = new Date(query.end).getTime() + const startMs = new Date(query.start).getTime() + + // end must be within the test window (tBefore..tAfter). + expect(endMs).toBeGreaterThanOrEqual(tBefore) + expect(endMs).toBeLessThanOrEqual(tAfter + 100) // small tolerance + + // start must be approximately end - spanMs. + expect(endMs - startMs).toBeCloseTo(SPAN_MS, -2) // within 100ms + }) + + it('when spanMs is provided, queryKey uses spanMs (not absolute timestamps) for stable caching', async () => { + mockGet.mockResolvedValue({ data: { items: [] } }) + + const { Wrapper, qc } = makeWrapper() + const { useReadings } = await import('./hooks') + const SPAN_MS = 3600000 // 1 hour + + const { result } = renderHook( + () => useReadings('test-uuid-1', { spanMs: SPAN_MS }), + { wrapper: Wrapper }, + ) + + await waitFor(() => expect(result.current.isSuccess).toBe(true)) + + // The query cache should have a key containing spanMs, not an absolute timestamp. + const queries = qc.getQueryCache().findAll({ + predicate: (q) => { + const key = q.queryKey as unknown[] + return key[0] === 'modbus-readings' && key[1] === 'test-uuid-1' + }, + }) + expect(queries).toHaveLength(1) + const keyParams = queries[0].queryKey[2] as Record + expect(keyParams).toHaveProperty('spanMs', SPAN_MS) + expect(keyParams).not.toHaveProperty('start') + expect(keyParams).not.toHaveProperty('end') + }) }) diff --git a/frontend/src/energy/hooks.ts b/frontend/src/energy/hooks.ts index f95bd21..752eca5 100644 --- a/frontend/src/energy/hooks.ts +++ b/frontend/src/energy/hooks.ts @@ -39,6 +39,13 @@ export type ModbusReadingsResponse = components['schemas']['ModbusReadingsRespon // --------------------------------------------------------------------------- export interface ReadingsQueryParams { + /** + * How many milliseconds of history to show. When provided the queryFn + * computes end=now() and start=now()-spanMs on every invocation, so + * refetchInterval-triggered re-fetches always use a rolling window. + * Mutually exclusive with `start`/`end`. + */ + spanMs?: number start?: string | null end?: string | null /** Capped server-side; default max is 1000 to avoid pulling full history. */ @@ -196,22 +203,39 @@ export function useReadings( params: ReadingsQueryParams, options?: AutoRefreshOptions, ) { - const { start, end, limit } = params + const { spanMs, start, end, limit } = params const effectiveLimit = Math.min(limit ?? READINGS_MAX_LIMIT, READINGS_MAX_LIMIT) const refetchInterval = options?.refetchIntervalMs != null ? Math.max(2_000, options.refetchIntervalMs) : undefined + // When spanMs is provided, the query key uses the stable span value (not + // absolute timestamps), so switching presets triggers a fresh fetch while + // refetchInterval-triggered re-fetches reuse the cached key and run the + // queryFn again — computing end=now() each time, giving a rolling window. + const queryKey = spanMs != null + ? ['modbus-readings', uuid, { spanMs, limit: effectiveLimit }] + : ['modbus-readings', uuid, { start, end, limit: effectiveLimit }] + return useQuery({ - queryKey: ['modbus-readings', uuid, { start, end, limit: effectiveLimit }], + queryKey, queryFn: async () => { + let resolvedStart = start + let resolvedEnd = end + + if (spanMs != null) { + const now = new Date() + resolvedEnd = now.toISOString() + resolvedStart = new Date(now.getTime() - spanMs).toISOString() + } + const res = await apiClient.GET('/api/modbus/devices/{uuid}/readings', { params: { path: { uuid }, query: { - ...(start ? { start } : {}), - ...(end ? { end } : {}), + ...(resolvedStart ? { start: resolvedStart } : {}), + ...(resolvedEnd ? { end: resolvedEnd } : {}), limit: effectiveLimit, }, }, diff --git a/frontend/src/pages/ConfigPage.test.tsx b/frontend/src/pages/ConfigPage.test.tsx index 6804f3e..7e67ca4 100644 --- a/frontend/src/pages/ConfigPage.test.tsx +++ b/frontend/src/pages/ConfigPage.test.tsx @@ -19,7 +19,7 @@ * 12. (M5-T01B) Accordion: second section can be expanded by clicking its control. */ -import { describe, it, expect, vi, beforeEach } from 'vitest' +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' import { screen, waitFor, fireEvent } from '@testing-library/react' import { renderWithProviders } from '../test-utils' import { ConfigPage } from './ConfigPage' @@ -47,6 +47,26 @@ const MOCK_CONFIG = { ], } +/** Extended fixture that includes a checkbox (bool) field. */ +const MOCK_CONFIG_WITH_CHECKBOX = { + sections: [ + { + name: 'MQTT', + fields: [ + { env_name: 'MQTT_ENABLED', label: 'MQTT Enabled', value: 'false', secret: false, input_type: 'checkbox', configured: true }, + { env_name: 'MQTT_BROKER_HOST', label: 'MQTT Broker Host', value: 'localhost', secret: false, input_type: 'text', configured: true }, + ], + }, + { + name: 'SMTP', + fields: [ + { env_name: 'SMTP_HOST', label: 'SMTP Host', value: 'smtp.example.com', secret: false, input_type: 'text', configured: true }, + { env_name: 'SMTP_PASSWORD', label: 'SMTP Password', value: '', secret: true, input_type: 'password', configured: true }, + ], + }, + ], +} + // --------------------------------------------------------------------------- // Mock apiClient // --------------------------------------------------------------------------- @@ -378,3 +398,97 @@ describe('ConfigPage', () => { }) }) }) + +// --------------------------------------------------------------------------- +// Area B: checkbox (Switch) rendering and value round-trip +// --------------------------------------------------------------------------- + +describe('ConfigPage — checkbox (bool) fields', () => { + beforeEach(() => { + vi.clearAllMocks() + mockGet.mockResolvedValue({ data: MOCK_CONFIG_WITH_CHECKBOX, response: { status: 200, ok: true } }) + }) + + afterEach(() => { + vi.clearAllMocks() + }) + + // ------------------------------------------------------------------------- + // 13. Checkbox field renders as a Switch (checked=false when value='false') + // ------------------------------------------------------------------------- + + it('renders a checkbox field as a Switch with checked=false when value is "false"', async () => { + renderConfig() + + await waitFor(() => { + expect(screen.getByTestId('field-MQTT_ENABLED')).toBeInTheDocument() + }) + + const switchEl = screen.getByTestId('field-MQTT_ENABLED') as HTMLInputElement + // Mantine Switch renders as + expect(switchEl.type).toBe('checkbox') + expect(switchEl.checked).toBe(false) + }) + + // ------------------------------------------------------------------------- + // 14. Toggling Switch to ON produces "true" in the save payload + // ------------------------------------------------------------------------- + + it('toggling Switch to checked sends "true" for that field in buildUpdates', async () => { + mockPut.mockResolvedValueOnce({ data: {}, response: { status: 200, ok: true } }) + mockGet.mockResolvedValue({ data: MOCK_CONFIG_WITH_CHECKBOX, response: { status: 200, ok: true } }) + + renderConfig() + + await waitFor(() => { + expect(screen.getByTestId('field-MQTT_ENABLED')).toBeInTheDocument() + }) + + // Toggle the switch ON + const switchEl = screen.getByTestId('field-MQTT_ENABLED') as HTMLInputElement + fireEvent.click(switchEl) + + // Submit the form + fireEvent.submit(screen.getByTestId('config-form')) + + await waitFor(() => { + expect(mockPut).toHaveBeenCalled() + }) + + const putCall = mockPut.mock.calls[0] + const body = putCall[1].body as { updates: Record } + const updates = body.updates + + // The bool field must be sent as the string "true" + expect(updates).toHaveProperty('MQTT_ENABLED', 'true') + // Other non-secret fields still included + expect(updates).toHaveProperty('MQTT_BROKER_HOST', 'localhost') + }) + + // ------------------------------------------------------------------------- + // 15. Switch starts checked=true when initial value is "true" + // ------------------------------------------------------------------------- + + it('renders Switch as checked when initial value is "true"', async () => { + const configWithEnabled = { + sections: [ + { + name: 'MQTT', + fields: [ + { env_name: 'MQTT_ENABLED', label: 'MQTT Enabled', value: 'true', secret: false, input_type: 'checkbox', configured: true }, + ], + }, + ], + } + mockGet.mockResolvedValue({ data: configWithEnabled, response: { status: 200, ok: true } }) + + renderConfig() + + await waitFor(() => { + expect(screen.getByTestId('field-MQTT_ENABLED')).toBeInTheDocument() + }) + + const switchEl = screen.getByTestId('field-MQTT_ENABLED') as HTMLInputElement + expect(switchEl.checked).toBe(true) + }) +}) diff --git a/frontend/src/pages/ConfigPage.tsx b/frontend/src/pages/ConfigPage.tsx index e1a1fd4..fdb83d9 100644 --- a/frontend/src/pages/ConfigPage.tsx +++ b/frontend/src/pages/ConfigPage.tsx @@ -36,6 +36,7 @@ import { Loader, Center, Badge, + Switch, } from '@mantine/core' import apiClient, { ApiError } from '../api/client' import type { components } from '../api/schema.d.ts' @@ -134,6 +135,17 @@ function ConfigFieldInput({ field, value, onChange }: ConfigFieldInputProps) { ) } + if (field.input_type === 'checkbox') { + return ( + onChange(field.env_name, e.currentTarget.checked ? 'true' : 'false')} + data-testid={`field-${field.env_name}`} + /> + ) + } + if (field.input_type === 'number') { return ( { // Mantine Switch uses role="switch" and places data-testid directly on the // element; query it by the testid or by role. - const switchInput = screen.getByRole('switch', { name: /自动刷新/i }) + const switchInput = screen.getByRole('switch', { name: /auto-refresh/i }) expect(switchInput).toBeChecked() }) @@ -382,7 +382,7 @@ describe('EnergyPage — auto-refresh switch', () => { expect(screen.getByTestId('auto-refresh-switch')).toBeInTheDocument() }) - const switchInput = screen.getByRole('switch', { name: /自动刷新/i }) + const switchInput = screen.getByRole('switch', { name: /auto-refresh/i }) expect(switchInput).toBeChecked() fireEvent.click(switchInput) diff --git a/frontend/src/pages/EnergyPage.tsx b/frontend/src/pages/EnergyPage.tsx index b6a28b7..6dbe2c4 100644 --- a/frontend/src/pages/EnergyPage.tsx +++ b/frontend/src/pages/EnergyPage.tsx @@ -463,7 +463,7 @@ function DeviceReadingsSection({ devices }: DeviceReadingsSectionProps) { setAutoRefreshEnabled(e.currentTarget.checked)} size="sm" diff --git a/tests/test_api_config.py b/tests/test_api_config.py index 1d8eeb4..556a448 100644 --- a/tests/test_api_config.py +++ b/tests/test_api_config.py @@ -557,3 +557,102 @@ def test_put_config_invalid_mqtt_port_returns_422_and_does_not_write( conn.close() assert rows.get("MQTT_BROKER_PORT") != "not-a-number" + + +# --------------------------------------------------------------------------- +# M5-polish2 Area B: bool fields have input_type="checkbox" +# --------------------------------------------------------------------------- + +EXPECTED_CHECKBOX_FIELDS = { + "APP_DEBUG", + "SMTP_ENABLED", + "SMTP_USE_STARTTLS", + "AUTH_LOGIN_THROTTLE_ENABLED", + "MQTT_ENABLED", + "MQTT_TLS_ENABLED", + "HA_DISCOVERY_ENABLED", + "MODBUS_POLLING_ENABLED", +} + + +def test_bool_fields_have_checkbox_input_type(client: TestClient) -> None: + """All pure-bool config fields must return input_type='checkbox' from GET /api/config.""" + _login(client) + + response = client.get("/api/config") + assert response.status_code == 200 + body = response.json() + + all_fields = { + field["env_name"]: field + for section in body["sections"] + for field in section["fields"] + } + + for env_name in EXPECTED_CHECKBOX_FIELDS: + assert env_name in all_fields, f"{env_name} not found in config response" + actual_type = all_fields[env_name]["input_type"] + assert actual_type == "checkbox", ( + f"{env_name}: expected input_type='checkbox', got {actual_type!r}" + ) + + +def test_auth_cookie_secure_override_is_not_checkbox(client: TestClient) -> None: + """AUTH_COOKIE_SECURE_OVERRIDE is bool|None (three-state) and must NOT be checkbox.""" + _login(client) + + response = client.get("/api/config") + assert response.status_code == 200 + body = response.json() + + all_fields = { + field["env_name"]: field + for section in body["sections"] + for field in section["fields"] + } + + # This field is present in CONFIG_FIELDS; its type is bool|None so it stays text. + if "AUTH_COOKIE_SECURE_OVERRIDE" in all_fields: + actual_type = all_fields["AUTH_COOKIE_SECURE_OVERRIDE"]["input_type"] + assert actual_type != "checkbox", ( + f"AUTH_COOKIE_SECURE_OVERRIDE should not be checkbox (it is bool|None); got {actual_type!r}" + ) + + +def test_put_config_bool_field_roundtrip_true_false( + client: TestClient, test_database_urls +) -> None: + """Saving 'true'/'false' string for a bool checkbox field roundtrips correctly.""" + _login(client) + + # Set MQTT_ENABLED to 'true' via config PUT + payload_true = _full_config_payload({"MQTT_ENABLED": "true"}) + resp_true = client.put( + "/api/config", + json={"updates": payload_true}, + headers={"X-CSRF-Token": "token"}, + ) + assert resp_true.status_code == 200 + + conn = sqlite3.connect(test_database_urls["app_path"]) + try: + rows = dict(conn.execute("SELECT key, value FROM app_config").fetchall()) + finally: + conn.close() + assert rows.get("MQTT_ENABLED") == "true" + + # Now set to 'false' + payload_false = _full_config_payload({"MQTT_ENABLED": "false"}) + resp_false = client.put( + "/api/config", + json={"updates": payload_false}, + headers={"X-CSRF-Token": "token"}, + ) + assert resp_false.status_code == 200 + + conn = sqlite3.connect(test_database_urls["app_path"]) + try: + rows = dict(conn.execute("SELECT key, value FROM app_config").fetchall()) + finally: + conn.close() + assert rows.get("MQTT_ENABLED") == "false"