M5: use DB-merged runtime settings for MQTT/discovery; add MQTT test button; move Expose panel into config accordion
This commit is contained in:
@@ -75,8 +75,11 @@ def put_config(
|
||||
detail="invalid config submission",
|
||||
) from exc
|
||||
|
||||
# Re-read settings after save (save_config_updates clears the settings cache)
|
||||
refreshed_settings = get_settings()
|
||||
# Re-read settings after save (save_config_updates clears the settings cache).
|
||||
# Use build_runtime_settings so the reconnect picks up DB-stored values, not
|
||||
# just the bootstrap env (otherwise a broker configured via the UI is ignored).
|
||||
from app.services.config_page import build_runtime_settings
|
||||
refreshed_settings = build_runtime_settings(db, get_settings())
|
||||
|
||||
# Reconnect MQTT client if any MQTT setting was updated.
|
||||
if mqtt_keys_submitted:
|
||||
|
||||
@@ -23,6 +23,7 @@ from sqlalchemy.orm import Session
|
||||
from app.api.routes.api.deps import require_csrf, require_session
|
||||
from app.config import get_settings
|
||||
from app.dependencies import get_db
|
||||
from app.services.config_page import build_runtime_settings
|
||||
from app.integrations.expose import CatalogEntry, build_catalog
|
||||
from app.integrations.mqtt import mqtt_manager
|
||||
from app.models.expose import ExposedEntityToggle
|
||||
@@ -48,9 +49,9 @@ router = APIRouter(prefix="/api", tags=["api-expose"])
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _get_mqtt_status() -> MqttStatusSchema:
|
||||
"""Build MQTT/Discovery status from live runtime state."""
|
||||
settings = get_settings()
|
||||
def _get_mqtt_status(db: Session) -> MqttStatusSchema:
|
||||
"""Build MQTT/Discovery status from live runtime state (DB-merged settings)."""
|
||||
settings = build_runtime_settings(db, get_settings())
|
||||
return MqttStatusSchema(
|
||||
mqtt_configured=mqtt_manager.is_configured(settings),
|
||||
mqtt_connected=mqtt_manager.is_connected,
|
||||
@@ -87,7 +88,7 @@ def _build_response_data(
|
||||
"""Return (catalog_entries, mqtt_status) for building GET / PUT responses."""
|
||||
catalog = build_catalog(session)
|
||||
catalog_schema = [_entry_to_schema(e) for e in catalog]
|
||||
mqtt_status = _get_mqtt_status()
|
||||
mqtt_status = _get_mqtt_status(session)
|
||||
return catalog_schema, mqtt_status
|
||||
|
||||
|
||||
@@ -179,7 +180,7 @@ def post_expose_republish(
|
||||
Returns a status indicating whether the publish was attempted (or skipped
|
||||
because MQTT / discovery is not enabled / connected).
|
||||
"""
|
||||
settings = get_settings()
|
||||
settings = build_runtime_settings(db, get_settings())
|
||||
if not (settings.mqtt_enabled and settings.ha_discovery_enabled and mqtt_manager.is_connected):
|
||||
return RepublishResponse(
|
||||
ok=False,
|
||||
|
||||
+19
-3
@@ -26,7 +26,7 @@ from app.api.routes.ticktick import router as ticktick_router
|
||||
from app.config import get_settings
|
||||
from app.integrations.mqtt import mqtt_manager
|
||||
from app.services.auth import AuthBootstrapError, initialize_auth_schema
|
||||
from app.services.config_page import seed_missing_config_from_bootstrap, sync_app_hostname_from_bootstrap
|
||||
from app.services.config_page import build_runtime_settings, seed_missing_config_from_bootstrap, sync_app_hostname_from_bootstrap
|
||||
from app.services.public_ip import check_public_ipv4_and_notify
|
||||
from app.services.modbus_poll import poll_all_enabled_devices, BASE_POLL_TICK_SECONDS
|
||||
from app.services.ha_discovery import publish_discovery, publish_states
|
||||
@@ -74,10 +74,19 @@ def _run_scheduled_ha_state_publish() -> None:
|
||||
``mqtt_manager.connect()`` would fire before the TCP handshake completes and
|
||||
be a no-op. Instead, this periodic job picks it up within 60 seconds of the
|
||||
broker becoming available — retained payloads make repeated publishes harmless.
|
||||
|
||||
Additionally, if MQTT is configured in DB but the manager is not yet connected
|
||||
(e.g. MQTT was enabled via UI after startup), this job attempts to connect so
|
||||
the user does not need to restart the server.
|
||||
"""
|
||||
session_local = get_session_local()
|
||||
session: Session = session_local()
|
||||
try:
|
||||
runtime_settings = build_runtime_settings(session, get_settings())
|
||||
# Reconnect if MQTT is configured (in DB) but not yet connected.
|
||||
if mqtt_manager.is_configured(runtime_settings) and not mqtt_manager.is_connected:
|
||||
logger.info("_run_scheduled_ha_state_publish: MQTT configured but not connected — attempting connect.")
|
||||
mqtt_manager.connect(runtime_settings)
|
||||
publish_discovery(session)
|
||||
publish_states(session)
|
||||
except Exception:
|
||||
@@ -140,10 +149,17 @@ async def lifespan(_: FastAPI):
|
||||
)
|
||||
scheduler.start()
|
||||
|
||||
# MQTT: connect if configured.
|
||||
# MQTT: connect using DB-merged runtime settings so broker configured via UI
|
||||
# is picked up on restart (not just from env/bootstrap settings).
|
||||
# Discovery will be published by the first run of _run_scheduled_ha_state_publish
|
||||
# (within 60 s of startup), after the async paho handshake completes.
|
||||
mqtt_manager.connect(get_settings())
|
||||
_startup_session_local = get_session_local()
|
||||
_startup_session: Session = _startup_session_local()
|
||||
try:
|
||||
_startup_runtime_settings = build_runtime_settings(_startup_session, get_settings())
|
||||
finally:
|
||||
_startup_session.close()
|
||||
mqtt_manager.connect(_startup_runtime_settings)
|
||||
|
||||
yield
|
||||
|
||||
|
||||
@@ -35,6 +35,7 @@ from sqlalchemy.orm import Session
|
||||
|
||||
from app.integrations.expose import ExposableEntity, build_catalog
|
||||
from app.integrations.mqtt import mqtt_manager
|
||||
from app.services.config_page import build_runtime_settings
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -44,12 +45,6 @@ logger = logging.getLogger(__name__)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _get_settings() -> Any:
|
||||
"""Return the current runtime settings (cached singleton)."""
|
||||
from app.config import get_settings
|
||||
return get_settings()
|
||||
|
||||
|
||||
def _should_publish(settings: Any) -> bool:
|
||||
"""Return True only if MQTT and HA Discovery are both enabled and connected."""
|
||||
return bool(
|
||||
@@ -180,7 +175,8 @@ def publish_discovery(session: Session) -> None:
|
||||
No-op if MQTT / discovery is not enabled or the client is not connected.
|
||||
All MQTT errors are caught internally — this function never raises.
|
||||
"""
|
||||
settings = _get_settings()
|
||||
from app.config import get_settings
|
||||
settings = build_runtime_settings(session, get_settings())
|
||||
if not _should_publish(settings):
|
||||
logger.debug("publish_discovery: skipped (MQTT/Discovery not enabled or not connected)")
|
||||
return
|
||||
@@ -232,7 +228,8 @@ def publish_states(session: Session) -> None:
|
||||
No-op if MQTT / discovery is not enabled or the client is not connected.
|
||||
All errors are caught internally — this function never raises.
|
||||
"""
|
||||
settings = _get_settings()
|
||||
from app.config import get_settings
|
||||
settings = build_runtime_settings(session, get_settings())
|
||||
if not _should_publish(settings):
|
||||
logger.debug("publish_states: skipped (MQTT/Discovery not enabled or not connected)")
|
||||
return
|
||||
@@ -320,7 +317,8 @@ def publish_device_state(session: Session, device: Any) -> None:
|
||||
No-op if MQTT / discovery is not enabled or the client is not connected.
|
||||
All errors are caught internally — never raises.
|
||||
"""
|
||||
settings = _get_settings()
|
||||
from app.config import get_settings
|
||||
settings = build_runtime_settings(session, get_settings())
|
||||
if not _should_publish(settings):
|
||||
return
|
||||
|
||||
@@ -386,7 +384,7 @@ def publish_device_state(session: Session, device: Any) -> None:
|
||||
)
|
||||
|
||||
|
||||
def publish_device_offline(device_uuid: str) -> None:
|
||||
def publish_device_offline(session: Session, device_uuid: str) -> None:
|
||||
"""Publish an "offline" availability payload for *device_uuid*.
|
||||
|
||||
Called by ``modbus_poll.poll_device`` when a poll fails, to immediately
|
||||
@@ -395,7 +393,8 @@ def publish_device_offline(device_uuid: str) -> None:
|
||||
No-op if MQTT / discovery is not enabled or the client is not connected.
|
||||
Never raises.
|
||||
"""
|
||||
settings = _get_settings()
|
||||
from app.config import get_settings
|
||||
settings = build_runtime_settings(session, get_settings())
|
||||
if not _should_publish(settings):
|
||||
return
|
||||
|
||||
|
||||
@@ -132,7 +132,7 @@ def poll_device(session: Session, device: ModbusDevice) -> ModbusReading | None:
|
||||
|
||||
# Best-effort: publish "offline" availability after failed poll.
|
||||
try:
|
||||
publish_device_offline(device.uuid)
|
||||
publish_device_offline(session, device.uuid)
|
||||
except Exception: # noqa: BLE001
|
||||
logger.exception(
|
||||
"poll_device: MQTT offline publish failed for device %r (id=%d)",
|
||||
|
||||
@@ -201,6 +201,7 @@ function RepublishButton() {
|
||||
return (
|
||||
<Stack gap="xs">
|
||||
<Button
|
||||
type="button"
|
||||
variant="outline"
|
||||
size="sm"
|
||||
onClick={handleRepublish}
|
||||
|
||||
@@ -399,6 +399,168 @@ describe('ConfigPage', () => {
|
||||
})
|
||||
})
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// M5-fix2: MqttTestButton — Issue 1 (frontend)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('ConfigPage — MQTT test button', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
// Use the MOCK_CONFIG_WITH_CHECKBOX fixture which includes an MQTT section
|
||||
mockGet.mockResolvedValue({ data: MOCK_CONFIG_WITH_CHECKBOX, response: { status: 200, ok: true } })
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
it('renders MQTT test button when MQTT section is present', async () => {
|
||||
renderConfig()
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId('mqtt-test-button')).toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
|
||||
it('does not render MQTT test button when no MQTT section present', async () => {
|
||||
// Use a config fixture without MQTT section
|
||||
mockGet.mockResolvedValueOnce({ data: MOCK_CONFIG, response: { status: 200, ok: true } })
|
||||
renderConfig()
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId('config-form')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
// No MQTT section in MOCK_CONFIG → button absent
|
||||
// Wait a bit for any async renders
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByTestId('mqtt-test-button')).not.toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
|
||||
it('shows success alert after MQTT test succeeds', async () => {
|
||||
mockPost.mockResolvedValueOnce({
|
||||
data: { result: 'success', message: 'Test message published.' },
|
||||
response: { status: 200, ok: true },
|
||||
})
|
||||
|
||||
renderConfig()
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId('mqtt-test-button')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
fireEvent.click(screen.getByTestId('mqtt-test-button'))
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId('mqtt-result-success')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
expect(screen.queryByTestId('mqtt-result-config-error')).not.toBeInTheDocument()
|
||||
expect(screen.queryByTestId('mqtt-result-failed')).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('shows config-error alert when MQTT test returns config-error', async () => {
|
||||
const { ApiError } = await import('../api/client')
|
||||
mockPost.mockRejectedValueOnce(
|
||||
new ApiError(400, { result: 'config-error', message: 'Broker host not configured.' }),
|
||||
)
|
||||
|
||||
renderConfig()
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId('mqtt-test-button')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
fireEvent.click(screen.getByTestId('mqtt-test-button'))
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId('mqtt-result-config-error')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
expect(screen.queryByTestId('mqtt-result-success')).not.toBeInTheDocument()
|
||||
expect(screen.queryByTestId('mqtt-result-failed')).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('shows failed alert when MQTT test returns failed (502)', async () => {
|
||||
const { ApiError } = await import('../api/client')
|
||||
mockPost.mockRejectedValueOnce(
|
||||
new ApiError(502, { result: 'failed', message: 'Connection refused.' }),
|
||||
)
|
||||
|
||||
renderConfig()
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId('mqtt-test-button')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
fireEvent.click(screen.getByTestId('mqtt-test-button'))
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId('mqtt-result-failed')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
expect(screen.queryByTestId('mqtt-result-success')).not.toBeInTheDocument()
|
||||
expect(screen.queryByTestId('mqtt-result-config-error')).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('MQTT test button has type="button" and does not submit the config form', async () => {
|
||||
renderConfig()
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId('mqtt-test-button')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
const mqttBtn = screen.getByTestId('mqtt-test-button')
|
||||
// The rendered button element should have type="button"
|
||||
expect(mqttBtn.getAttribute('type')).toBe('button')
|
||||
})
|
||||
})
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// M5-fix2: ExposeSettings in config accordion — Issue 3 (frontend)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('ConfigPage — ExposeSettings in main accordion', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
mockGet.mockResolvedValue({ data: MOCK_CONFIG, response: { status: 200, ok: true } })
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
it('renders Home Assistant Expose accordion item inside the config accordion', async () => {
|
||||
renderConfig()
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId('config-accordion')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
// The Expose accordion item should be inside the main config-accordion
|
||||
const configAccordion = screen.getByTestId('config-accordion')
|
||||
const exposeControl = screen.getByTestId('accordion-control-expose')
|
||||
expect(configAccordion.contains(exposeControl)).toBe(true)
|
||||
})
|
||||
|
||||
it('Expose accordion item appears before the Save button in DOM order', async () => {
|
||||
renderConfig()
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId('accordion-control-expose')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
const exposeControl = screen.getByTestId('accordion-control-expose')
|
||||
const saveButton = screen.getByTestId('config-save-button')
|
||||
|
||||
// compareDocumentPosition: if expose comes before save, position & Node.DOCUMENT_POSITION_FOLLOWING === true
|
||||
const position = exposeControl.compareDocumentPosition(saveButton)
|
||||
// DOCUMENT_POSITION_FOLLOWING = 4 means saveButton comes after exposeControl
|
||||
expect(position & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy()
|
||||
})
|
||||
})
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Area B: checkbox (Switch) rendering and value round-trip
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@@ -57,6 +57,13 @@ type SmtpResult =
|
||||
| { kind: 'failed'; message: string }
|
||||
| null
|
||||
|
||||
/** MQTT test result tri-state. */
|
||||
type MqttResult =
|
||||
| { kind: 'success'; message: string }
|
||||
| { kind: 'config-error'; message: string }
|
||||
| { kind: 'failed'; message: string }
|
||||
| null
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Hook: load config
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -263,6 +270,75 @@ function SmtpTestButton({ smtpResult, setSmtpResult }: SmtpTestButtonProps) {
|
||||
)
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// MqttTestButton — sends POST /api/config/mqtt/test and displays tri-state result
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
interface MqttTestButtonProps {
|
||||
mqttResult: MqttResult
|
||||
setMqttResult: (r: MqttResult) => void
|
||||
}
|
||||
|
||||
function MqttTestButton({ mqttResult, setMqttResult }: MqttTestButtonProps) {
|
||||
const [testing, setTesting] = useState(false)
|
||||
|
||||
async function handleTest() {
|
||||
setMqttResult(null)
|
||||
setTesting(true)
|
||||
try {
|
||||
const res = await apiClient.POST('/api/config/mqtt/test')
|
||||
if (res.data) {
|
||||
setMqttResult({ kind: 'success', message: res.data.message })
|
||||
}
|
||||
} catch (err) {
|
||||
if (err instanceof ApiError) {
|
||||
const body = err.body as { result?: string; message?: string } | null
|
||||
const result = body?.result
|
||||
const message = body?.message ?? 'Unknown error'
|
||||
if (result === 'config-error') {
|
||||
setMqttResult({ kind: 'config-error', message })
|
||||
} else {
|
||||
setMqttResult({ kind: 'failed', message })
|
||||
}
|
||||
} else {
|
||||
setMqttResult({ kind: 'failed', message: 'Unexpected error sending test message.' })
|
||||
}
|
||||
} finally {
|
||||
setTesting(false)
|
||||
}
|
||||
}
|
||||
|
||||
return (
|
||||
<Stack gap="xs">
|
||||
<Button
|
||||
type="button"
|
||||
variant="outline"
|
||||
onClick={handleTest}
|
||||
loading={testing}
|
||||
data-testid="mqtt-test-button"
|
||||
>
|
||||
Send Test Message
|
||||
</Button>
|
||||
|
||||
{mqttResult?.kind === 'success' && (
|
||||
<Alert color="green" data-testid="mqtt-result-success">
|
||||
MQTT test message sent successfully. {mqttResult.message}
|
||||
</Alert>
|
||||
)}
|
||||
{mqttResult?.kind === 'config-error' && (
|
||||
<Alert color="orange" data-testid="mqtt-result-config-error">
|
||||
MQTT configuration error — check your MQTT settings. {mqttResult.message}
|
||||
</Alert>
|
||||
)}
|
||||
{mqttResult?.kind === 'failed' && (
|
||||
<Alert color="red" data-testid="mqtt-result-failed">
|
||||
MQTT test failed. {mqttResult.message}
|
||||
</Alert>
|
||||
)}
|
||||
</Stack>
|
||||
)
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// ConfigPage — main component
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -295,6 +371,9 @@ export function ConfigPage() {
|
||||
// SMTP test tri-state
|
||||
const [smtpResult, setSmtpResult] = useState<SmtpResult>(null)
|
||||
|
||||
// MQTT test tri-state
|
||||
const [mqttResult, setMqttResult] = useState<MqttResult>(null)
|
||||
|
||||
function handleChange(envName: string, value: string) {
|
||||
setLocalValues((prev) => ({ ...prev, [envName]: value }))
|
||||
setSaveStatus(null)
|
||||
@@ -350,6 +429,9 @@ export function ConfigPage() {
|
||||
s.name.toLowerCase().includes('smtp') || s.name.toLowerCase().includes('email'),
|
||||
)
|
||||
|
||||
// Detect if there is an MQTT section (to show the MQTT test button).
|
||||
const hasMqttSection = data.sections.some((s) => s.name.toLowerCase() === 'mqtt')
|
||||
|
||||
// Default: open the first section so users immediately see content.
|
||||
const defaultAccordionValue = data.sections[0]?.name ?? null
|
||||
|
||||
@@ -387,6 +469,18 @@ export function ConfigPage() {
|
||||
</Accordion.Panel>
|
||||
</Accordion.Item>
|
||||
))}
|
||||
|
||||
{/* M5-fix2: Home Assistant Expose panel merged into the config accordion,
|
||||
above Save/Test buttons. All interactive elements inside ExposeSettings
|
||||
use type="button" so they do not trigger the config form submit. */}
|
||||
<Accordion.Item value="expose" data-testid="expose-accordion-item">
|
||||
<Accordion.Control data-testid="accordion-control-expose">
|
||||
<Text fw={500}>Home Assistant Expose</Text>
|
||||
</Accordion.Control>
|
||||
<Accordion.Panel>
|
||||
<ExposeSettings />
|
||||
</Accordion.Panel>
|
||||
</Accordion.Item>
|
||||
</Accordion>
|
||||
|
||||
<Divider />
|
||||
@@ -411,9 +505,14 @@ export function ConfigPage() {
|
||||
Save Configuration
|
||||
</Button>
|
||||
|
||||
<Group gap="sm" wrap="wrap">
|
||||
{hasSmtpSection && (
|
||||
<SmtpTestButton smtpResult={smtpResult} setSmtpResult={setSmtpResult} />
|
||||
)}
|
||||
{hasMqttSection && (
|
||||
<MqttTestButton mqttResult={mqttResult} setMqttResult={setMqttResult} />
|
||||
)}
|
||||
</Group>
|
||||
</Group>
|
||||
</Stack>
|
||||
</form>
|
||||
@@ -426,20 +525,6 @@ export function ConfigPage() {
|
||||
</Stack>
|
||||
)}
|
||||
|
||||
{/* M5-T12: Home Assistant Expose — entity toggle panel as an Accordion.Item
|
||||
below the config form. Kept outside <form> because it manages its own
|
||||
mutations (PUT /api/expose, POST /api/expose/republish). */}
|
||||
<Accordion variant="separated" radius="md" mt="xl" data-testid="expose-accordion">
|
||||
<Accordion.Item value="expose">
|
||||
<Accordion.Control data-testid="accordion-control-expose">
|
||||
<Text fw={500}>Home Assistant Expose</Text>
|
||||
</Accordion.Control>
|
||||
<Accordion.Panel>
|
||||
<ExposeSettings />
|
||||
</Accordion.Panel>
|
||||
</Accordion.Item>
|
||||
</Accordion>
|
||||
|
||||
{/* TOTP two-factor auth management */}
|
||||
<Stack mt="xl">
|
||||
<TotpSettings />
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import sqlite3
|
||||
from unittest.mock import patch
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
@@ -656,3 +656,90 @@ def test_put_config_bool_field_roundtrip_true_false(
|
||||
finally:
|
||||
conn.close()
|
||||
assert rows.get("MQTT_ENABLED") == "false"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# M5-fix2: DB-config → runtime settings (Issue 2 regression assertions)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_put_config_mqtt_reconnect_uses_db_merged_settings(
|
||||
client: TestClient, test_database_urls
|
||||
) -> None:
|
||||
"""PUT /api/config with MQTT keys must reconnect with DB-merged settings,
|
||||
not bootstrap-only settings. Asserts that mqtt_manager.reconnect receives
|
||||
settings where mqtt_enabled=True and mqtt_broker_host matches the submitted value.
|
||||
"""
|
||||
_login(client)
|
||||
|
||||
target_host = "mqtt.example.com"
|
||||
payload = _full_config_payload({
|
||||
"MQTT_ENABLED": "true",
|
||||
"MQTT_BROKER_HOST": target_host,
|
||||
})
|
||||
|
||||
mock_mgr = MagicMock()
|
||||
mock_mgr.is_configured.return_value = True
|
||||
|
||||
with patch("app.api.routes.api.config.mqtt_manager", mock_mgr):
|
||||
resp = client.put(
|
||||
"/api/config",
|
||||
json={"updates": payload},
|
||||
headers={"X-CSRF-Token": "token"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 200
|
||||
|
||||
# reconnect must have been called once
|
||||
assert mock_mgr.reconnect.call_count == 1, (
|
||||
f"Expected mqtt_manager.reconnect called once, got {mock_mgr.reconnect.call_count}"
|
||||
)
|
||||
reconnect_settings = mock_mgr.reconnect.call_args[0][0]
|
||||
assert reconnect_settings.mqtt_enabled is True, (
|
||||
f"reconnect settings.mqtt_enabled must be True, got {reconnect_settings.mqtt_enabled!r}"
|
||||
)
|
||||
assert reconnect_settings.mqtt_broker_host == target_host, (
|
||||
f"reconnect settings.mqtt_broker_host must be {target_host!r}, "
|
||||
f"got {reconnect_settings.mqtt_broker_host!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_post_mqtt_test_uses_db_broker_host(
|
||||
client: TestClient, test_database_urls
|
||||
) -> None:
|
||||
"""POST /api/config/mqtt/test must use the DB-stored broker host, not env/bootstrap."""
|
||||
_login(client)
|
||||
|
||||
db_host = "broker.db-configured.local"
|
||||
|
||||
# First write the MQTT config to DB via PUT /api/config
|
||||
payload_save = _full_config_payload({
|
||||
"MQTT_ENABLED": "true",
|
||||
"MQTT_BROKER_HOST": db_host,
|
||||
})
|
||||
with patch("app.api.routes.api.config.mqtt_manager"):
|
||||
save_resp = client.put(
|
||||
"/api/config",
|
||||
json={"updates": payload_save},
|
||||
headers={"X-CSRF-Token": "token"},
|
||||
)
|
||||
assert save_resp.status_code == 200
|
||||
|
||||
# Now POST /api/config/mqtt/test — capture the host that _run_mqtt_test uses.
|
||||
captured_host: list[str] = []
|
||||
|
||||
def _fake_run_mqtt_test(settings):
|
||||
captured_host.append(settings.mqtt_broker_host)
|
||||
# simulate success by just returning
|
||||
return
|
||||
|
||||
with patch("app.api.routes.api.config._run_mqtt_test", side_effect=_fake_run_mqtt_test):
|
||||
resp = client.post(
|
||||
"/api/config/mqtt/test",
|
||||
headers={"X-CSRF-Token": "token"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 200
|
||||
assert len(captured_host) == 1, "Expected _run_mqtt_test to be called once"
|
||||
assert captured_host[0] == db_host, (
|
||||
f"Expected mqtt test to use DB host {db_host!r}, got {captured_host[0]!r}"
|
||||
)
|
||||
|
||||
@@ -367,7 +367,7 @@ class TestPostRepublish:
|
||||
mock_settings.ha_discovery_enabled = True
|
||||
mock_settings.ha_discovery_prefix = "homeassistant"
|
||||
|
||||
with patch("app.api.routes.api.expose.get_settings", return_value=mock_settings):
|
||||
with patch("app.api.routes.api.expose.build_runtime_settings", return_value=mock_settings):
|
||||
with patch("app.api.routes.api.expose.mqtt_manager") as mock_mqtt:
|
||||
mock_mqtt.is_configured.return_value = True
|
||||
mock_mqtt.is_connected = True
|
||||
@@ -406,7 +406,7 @@ class TestPostRepublish:
|
||||
mock_settings.mqtt_enabled = True
|
||||
mock_settings.ha_discovery_enabled = True
|
||||
|
||||
with patch("app.api.routes.api.expose.get_settings", return_value=mock_settings):
|
||||
with patch("app.api.routes.api.expose.build_runtime_settings", return_value=mock_settings):
|
||||
with patch("app.api.routes.api.expose.mqtt_manager") as mock_mqtt:
|
||||
mock_mqtt.is_connected = False
|
||||
resp = client.post(
|
||||
@@ -475,3 +475,48 @@ class TestCatalogGrouping:
|
||||
components = {e["entity"]["component"] for e in catalog}
|
||||
assert "binary_sensor" in components
|
||||
assert "sensor" in components
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# M5-fix2: DB-config → mqtt_status runtime assertion (Issue 2)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestMqttStatusReadsFromDB:
|
||||
def test_get_expose_mqtt_configured_reflects_db_value(self, expose_client):
|
||||
"""GET /api/expose must return mqtt_configured=True when MQTT is enabled+configured
|
||||
in the DB (app_config), even if the bootstrap env says disabled.
|
||||
|
||||
This verifies that _get_mqtt_status uses build_runtime_settings (DB-merged)
|
||||
rather than bare get_settings() (bootstrap-only).
|
||||
"""
|
||||
client, engine = expose_client
|
||||
_login(client)
|
||||
|
||||
# Write MQTT_ENABLED=true and MQTT_BROKER_HOST into app_config directly.
|
||||
# The lifespan seeds all config keys on startup, so we must UPDATE existing rows.
|
||||
from datetime import UTC, datetime
|
||||
from app.models.config import AppConfigEntry
|
||||
|
||||
now = datetime.now(UTC)
|
||||
with Session(engine) as session:
|
||||
for key, value in [("MQTT_ENABLED", "true"), ("MQTT_BROKER_HOST", "broker.test.local")]:
|
||||
row = session.query(AppConfigEntry).filter(AppConfigEntry.key == key).first()
|
||||
if row is None:
|
||||
session.add(AppConfigEntry(key=key, value=value, updated_at=now))
|
||||
else:
|
||||
row.value = value
|
||||
row.updated_at = now
|
||||
session.commit()
|
||||
|
||||
# The bootstrap env does NOT have MQTT enabled (default False in Settings).
|
||||
# But GET /api/expose should still report mqtt_configured=True because it reads DB.
|
||||
with patch("app.integrations.expose._REGISTRY", []):
|
||||
resp = client.get("/api/expose")
|
||||
|
||||
assert resp.status_code == 200
|
||||
mqtt_status = resp.json()["mqtt_status"]
|
||||
assert mqtt_status["mqtt_configured"] is True, (
|
||||
f"Expected mqtt_configured=True from DB settings, "
|
||||
f"got {mqtt_status['mqtt_configured']!r}. Full status: {mqtt_status}"
|
||||
)
|
||||
|
||||
+61
-17
@@ -340,7 +340,7 @@ def test_publish_discovery_sends_retained_for_enabled_entity(disco_db) -> None:
|
||||
mock_mgr.publish.side_effect = _capture
|
||||
|
||||
with (
|
||||
patch("app.services.ha_discovery._get_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.build_runtime_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.mqtt_manager", mock_mgr),
|
||||
):
|
||||
from app.services.ha_discovery import publish_discovery
|
||||
@@ -383,7 +383,7 @@ def test_publish_discovery_sends_empty_payload_for_disabled_entity(disco_db) ->
|
||||
mock_mgr.publish.side_effect = _capture
|
||||
|
||||
with (
|
||||
patch("app.services.ha_discovery._get_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.build_runtime_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.mqtt_manager", mock_mgr),
|
||||
):
|
||||
from app.services.ha_discovery import publish_discovery
|
||||
@@ -425,7 +425,7 @@ def test_publish_discovery_enabled_vs_disabled_payload(disco_db) -> None:
|
||||
mock_mgr.publish.side_effect = _capture
|
||||
|
||||
with (
|
||||
patch("app.services.ha_discovery._get_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.build_runtime_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.mqtt_manager", mock_mgr),
|
||||
):
|
||||
from app.services.ha_discovery import publish_discovery
|
||||
@@ -501,7 +501,7 @@ def test_publish_states_calls_value_getter_for_enabled_sensor() -> None:
|
||||
mock_catalog = [CatalogEntry(entity=mock_entity, enabled=True)]
|
||||
|
||||
with (
|
||||
patch("app.services.ha_discovery._get_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.build_runtime_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.mqtt_manager", mock_mgr),
|
||||
patch("app.services.ha_discovery.build_catalog", return_value=mock_catalog),
|
||||
):
|
||||
@@ -549,7 +549,7 @@ def test_publish_states_skips_disabled_entities() -> None:
|
||||
mock_catalog = [CatalogEntry(entity=mock_entity, enabled=False)]
|
||||
|
||||
with (
|
||||
patch("app.services.ha_discovery._get_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.build_runtime_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.mqtt_manager", mock_mgr),
|
||||
patch("app.services.ha_discovery.build_catalog", return_value=mock_catalog),
|
||||
):
|
||||
@@ -614,7 +614,7 @@ def test_publish_device_state_pushes_availability_and_state() -> None:
|
||||
mock_device.last_poll_ok = True
|
||||
|
||||
with (
|
||||
patch("app.services.ha_discovery._get_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.build_runtime_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.mqtt_manager", mock_mgr),
|
||||
patch("app.services.ha_discovery.build_catalog", return_value=mock_catalog),
|
||||
):
|
||||
@@ -658,7 +658,7 @@ def test_publish_device_state_publishes_offline_on_failed_poll() -> None:
|
||||
mock_device.last_poll_ok = False
|
||||
|
||||
with (
|
||||
patch("app.services.ha_discovery._get_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.build_runtime_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.mqtt_manager", mock_mgr),
|
||||
patch("app.services.ha_discovery.build_catalog", return_value=[]),
|
||||
):
|
||||
@@ -695,12 +695,13 @@ def test_publish_device_offline_publishes_offline_topic() -> None:
|
||||
mock_mgr.publish.side_effect = _capture
|
||||
|
||||
with (
|
||||
patch("app.services.ha_discovery._get_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.build_runtime_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.mqtt_manager", mock_mgr),
|
||||
):
|
||||
from app.services.ha_discovery import publish_device_offline
|
||||
from unittest.mock import MagicMock as _MagicMock
|
||||
|
||||
publish_device_offline(uuid_val)
|
||||
publish_device_offline(_MagicMock(), uuid_val)
|
||||
|
||||
assert len(published) == 1
|
||||
topic, payload = published[0]
|
||||
@@ -713,13 +714,55 @@ def test_publish_device_offline_publishes_offline_topic() -> None:
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_should_publish_true_when_db_enabled_bootstrap_disabled(disco_db) -> None:
|
||||
"""_should_publish must use DB-merged settings: if DB has mqtt_enabled=True and
|
||||
ha_discovery_enabled=True (bootstrap has both False), publish should proceed.
|
||||
|
||||
This is the core regression test for Issue 2: bare get_settings() returns bootstrap
|
||||
(False); build_runtime_settings returns DB-merged (True).
|
||||
"""
|
||||
from app.services.config_page import build_runtime_settings
|
||||
from app.services.ha_discovery import _should_publish
|
||||
from app.models.config import AppConfigEntry
|
||||
from datetime import UTC, datetime
|
||||
|
||||
now = datetime.now(UTC)
|
||||
|
||||
# Write MQTT + discovery enabled into app_config (DB level)
|
||||
with Session(disco_db) as session:
|
||||
session.add(AppConfigEntry(key="MQTT_ENABLED", value="true", updated_at=now))
|
||||
session.add(AppConfigEntry(key="MQTT_BROKER_HOST", value="broker.test", updated_at=now))
|
||||
session.add(AppConfigEntry(key="HA_DISCOVERY_ENABLED", value="true", updated_at=now))
|
||||
session.commit()
|
||||
|
||||
# Bootstrap has mqtt_enabled=False and ha_discovery_enabled=False (env defaults)
|
||||
from app.config import Settings
|
||||
bootstrap = Settings(_env_file=None, mqtt_enabled=False, ha_discovery_enabled=False,
|
||||
app_database_url=str(disco_db.url))
|
||||
|
||||
with Session(disco_db) as session:
|
||||
runtime = build_runtime_settings(session, bootstrap)
|
||||
|
||||
# Simulate a connected broker for the _should_publish check
|
||||
mock_mgr_connected = MagicMock()
|
||||
mock_mgr_connected.is_connected = True
|
||||
|
||||
with patch("app.services.ha_discovery.mqtt_manager", mock_mgr_connected):
|
||||
result = _should_publish(runtime)
|
||||
|
||||
assert result is True, (
|
||||
"_should_publish must return True when DB has mqtt_enabled=True + ha_discovery_enabled=True, "
|
||||
"even if bootstrap Settings has both False"
|
||||
)
|
||||
|
||||
|
||||
def test_publish_discovery_noop_when_mqtt_disabled() -> None:
|
||||
"""publish_discovery must be a no-op when mqtt_enabled=False."""
|
||||
settings = _make_settings(mqtt_enabled=False, ha_discovery_enabled=True)
|
||||
mock_mgr = _make_mock_manager(is_connected=False)
|
||||
|
||||
with (
|
||||
patch("app.services.ha_discovery._get_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.build_runtime_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.mqtt_manager", mock_mgr),
|
||||
):
|
||||
from app.services.ha_discovery import publish_discovery
|
||||
@@ -739,7 +782,7 @@ def test_publish_states_noop_when_not_connected() -> None:
|
||||
mock_mgr = _make_mock_manager(is_connected=False)
|
||||
|
||||
with (
|
||||
patch("app.services.ha_discovery._get_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.build_runtime_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.mqtt_manager", mock_mgr),
|
||||
):
|
||||
from app.services.ha_discovery import publish_states
|
||||
@@ -758,7 +801,7 @@ def test_publish_discovery_noop_when_ha_discovery_disabled() -> None:
|
||||
mock_mgr = _make_mock_manager(is_connected=True)
|
||||
|
||||
with (
|
||||
patch("app.services.ha_discovery._get_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.build_runtime_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.mqtt_manager", mock_mgr),
|
||||
):
|
||||
from app.services.ha_discovery import publish_discovery
|
||||
@@ -781,7 +824,7 @@ def test_publish_device_state_noop_when_mqtt_disabled() -> None:
|
||||
mock_device.last_poll_ok = True
|
||||
|
||||
with (
|
||||
patch("app.services.ha_discovery._get_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.build_runtime_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.mqtt_manager", mock_mgr),
|
||||
):
|
||||
from app.services.ha_discovery import publish_device_state
|
||||
@@ -800,12 +843,13 @@ def test_publish_device_offline_noop_when_mqtt_disabled() -> None:
|
||||
mock_mgr = _make_mock_manager(is_connected=False)
|
||||
|
||||
with (
|
||||
patch("app.services.ha_discovery._get_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.build_runtime_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.mqtt_manager", mock_mgr),
|
||||
):
|
||||
from app.services.ha_discovery import publish_device_offline
|
||||
from unittest.mock import MagicMock as _MagicMock
|
||||
|
||||
publish_device_offline("some-uuid") # must not raise
|
||||
publish_device_offline(_MagicMock(), "some-uuid") # must not raise
|
||||
|
||||
mock_mgr.publish.assert_not_called()
|
||||
|
||||
@@ -984,7 +1028,7 @@ def test_publish_discovery_uses_retain_true_for_enabled_entity() -> None:
|
||||
catalog = [CatalogEntry(entity=entity, enabled=True)]
|
||||
|
||||
with (
|
||||
patch("app.services.ha_discovery._get_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.build_runtime_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.mqtt_manager", mock_mgr),
|
||||
patch("app.services.ha_discovery.build_catalog", return_value=catalog),
|
||||
):
|
||||
@@ -1085,7 +1129,7 @@ def test_real_provider_value_getter_returns_latest_reading_value(disco_db) -> No
|
||||
mock_device.last_poll_ok = True
|
||||
|
||||
with (
|
||||
patch("app.services.ha_discovery._get_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.build_runtime_settings", return_value=settings),
|
||||
patch("app.services.ha_discovery.mqtt_manager", mock_mgr),
|
||||
# NOTE: build_catalog is NOT mocked — we use the real modbus provider.
|
||||
):
|
||||
|
||||
Reference in New Issue
Block a user