diff --git a/app/api/routes/api/modbus.py b/app/api/routes/api/modbus.py index fd580b9..6b56146 100644 --- a/app/api/routes/api/modbus.py +++ b/app/api/routes/api/modbus.py @@ -4,14 +4,14 @@ All endpoints are under /api/modbus, require an authenticated session, and write endpoints (POST/PATCH/DELETE + test-read) additionally require a non-empty X-CSRF-Token header. -Deletion safety (red-line): - DELETE /api/modbus/devices/{uuid} explicitly queries the application - layer for existing modbus_reading rows before deleting. If any exist - the endpoint returns 409 and suggests disabling the device instead. - We do NOT rely on the SQLite FK RESTRICT constraint because SQLite does - not enforce foreign-key constraints at runtime unless - PRAGMA foreign_keys=ON is set per connection (and it is not in this - project — see M5-T02 review note OBS-1). +Deletion safety: + DELETE /api/modbus/devices/{uuid} queries the application layer for + existing modbus_reading rows before deleting. Without ``cascade`` it + returns 409 and suggests disabling the device instead — a friendly guard + rather than letting the DB raise. With ``cascade=true`` it removes the + readings (and expose toggles) first, then the device. SQLite FK RESTRICT + *is* enforced at runtime here (the app sets PRAGMA foreign_keys=ON; see + app/db.py), so the cascade delete order matters. """ from __future__ import annotations @@ -21,7 +21,7 @@ from datetime import UTC, datetime from typing import Any from fastapi import APIRouter, Depends, HTTPException, Query, status -from sqlalchemy import func, select +from sqlalchemy import delete as sa_delete, func, select from sqlalchemy.orm import Session from app.api.routes.api.deps import require_csrf, require_session @@ -34,9 +34,11 @@ from app.integrations.modbus.profiles import ( list_profiles, load_profile, ) +from app.models.expose import ExposedEntityToggle from app.models.modbus import ModbusDevice, ModbusReading from app.schemas.modbus import ( MetricInfo, + ModbusDeleteResponse, ModbusDeviceCreate, ModbusDeviceListResponse, ModbusDeviceResponse, @@ -220,29 +222,41 @@ def patch_device( ) def delete_device( uuid: str, + cascade: bool = Query(default=False), db: Session = Depends(get_db), _auth: AuthenticatedSession = Depends(require_session), _csrf: None = Depends(require_csrf), -) -> None: +) -> None | ModbusDeleteResponse: """Delete a Modbus device. + **Default behaviour (cascade=false)**: Returns 409 Conflict if the device has any associated readings; use ``enabled=false`` to disable it instead. - **Application-layer safety**: the check is performed via an explicit - SELECT COUNT query — not by relying on SQLite's FK RESTRICT constraint, - which is not enforced at runtime in this project (no PRAGMA foreign_keys=ON). + **Cascade delete (cascade=true)**: + Permanently deletes the device together with all its readings and any + ``ExposedEntityToggle`` rows whose key matches ``modbus..*``. + Also makes a best-effort attempt to clear the device's HA Discovery + config topics from MQTT (empty retained payload) before the DB rows + are removed. MQTT failures are swallowed — the DB deletion proceeds + regardless. + Returns HTTP 200 with a ``ModbusDeleteResponse`` JSON body on success. + + **Application-layer safety**: the 409 guard uses an explicit SELECT COUNT + query to return a friendly message. FK RESTRICT is enforced at runtime + (the app sets ``PRAGMA foreign_keys=ON``), so the cascade path deletes + readings before the device. """ + from app.services.ha_discovery import clear_device_discovery + device = _get_device_or_404(db, uuid) # Application-layer guard: refuse deletion if any readings exist. - # We do NOT rely on SQLite FK RESTRICT — it is not enforced at runtime - # without PRAGMA foreign_keys=ON, which is not set in this project. reading_count: int = db.execute( select(func.count(ModbusReading.id)).where(ModbusReading.device_id == device.id) ).scalar_one() - if reading_count > 0: + if reading_count > 0 and not cascade: raise HTTPException( status_code=status.HTTP_409_CONFLICT, detail=( @@ -252,6 +266,60 @@ def delete_device( ), ) + if cascade: + # --- Cascade deletion path --- + logger.info( + "Cascade-deleting Modbus device %r (uuid=%s): %d reading(s)", + device.friendly_name, + uuid, + reading_count, + ) + + # 1. Best-effort HA Discovery cleanup (before DB rows are gone). + try: + clear_device_discovery(db, uuid) + except Exception: # noqa: BLE001 + logger.exception( + "delete_device(cascade): HA discovery cleanup raised for uuid=%s; continuing", + uuid, + ) + + # 2. Delete all readings for this device (must happen before device row). + deleted_readings_result = db.execute( + sa_delete(ModbusReading).where(ModbusReading.device_id == device.id) + ) + readings_deleted: int = deleted_readings_result.rowcount + + # 3. Delete all ExposedEntityToggle rows for this device. + # Keys follow the pattern "modbus..". + toggle_prefix = f"modbus.{uuid}.%" + deleted_toggles_result = db.execute( + sa_delete(ExposedEntityToggle).where(ExposedEntityToggle.key.like(toggle_prefix)) + ) + toggles_deleted: int = deleted_toggles_result.rowcount + + # 4. Delete the device itself. + db.delete(device) + db.commit() + + logger.info( + "Cascade-delete complete for device uuid=%s: %d reading(s), %d toggle(s) removed", + uuid, + readings_deleted, + toggles_deleted, + ) + + from fastapi.responses import JSONResponse + return JSONResponse( + status_code=status.HTTP_200_OK, + content={ + "deleted": True, + "readings_deleted": readings_deleted, + "toggles_deleted": toggles_deleted, + }, + ) + + # --- Non-cascade path (no readings exist at this point) --- logger.info("Deleting Modbus device %r (uuid=%s)", device.friendly_name, uuid) db.delete(device) db.commit() diff --git a/app/schemas/modbus.py b/app/schemas/modbus.py index 9918ce3..8d1ef61 100644 --- a/app/schemas/modbus.py +++ b/app/schemas/modbus.py @@ -152,3 +152,20 @@ class ModbusTestReadResponse(BaseModel): ok: bool payload: dict[str, Any] | None = None error: str | None = None + + +# --------------------------------------------------------------------------- +# Cascade-delete schema +# --------------------------------------------------------------------------- + + +class ModbusDeleteResponse(BaseModel): + """Response for DELETE /api/modbus/devices/{uuid}?cascade=true. + + Returned only when cascade deletion succeeds (HTTP 200). Non-cascade + successful deletes continue to return HTTP 204 (no body). + """ + + deleted: bool + readings_deleted: int + toggles_deleted: int diff --git a/app/services/ha_discovery.py b/app/services/ha_discovery.py index 3b8a3f4..852f56a 100644 --- a/app/services/ha_discovery.py +++ b/app/services/ha_discovery.py @@ -407,6 +407,74 @@ def publish_device_state(session: Session, device: Any) -> None: ) +def clear_device_discovery(session: Session, device_uuid: str) -> None: + """Best-effort: send empty retained payloads to all HA Discovery config topics + for the given device, effectively removing the device's entities from HA. + + This must be called **before** the device rows are deleted from the DB, so + that ``build_catalog`` can still enumerate the device's entities. + + Behaviour + --------- + - No-op if MQTT / HA Discovery is not enabled or the MQTT client is not + connected. + - All exceptions are caught internally; this function never raises. + The caller proceeds with the DB deletion regardless of MQTT outcome. + + Parameters + ---------- + session: + Active SQLAlchemy session (device must still exist in DB at call time). + device_uuid: + UUID string of the device being deleted. + """ + from app.config import get_settings + settings = build_runtime_settings(session, get_settings()) + if not _should_publish(settings): + logger.debug( + "clear_device_discovery: skipped (MQTT/Discovery not enabled or not connected)" + ) + return + + discovery_prefix = settings.ha_discovery_prefix + state_prefix = settings.ha_state_topic_prefix + + try: + catalog = build_catalog(session) + except Exception: + logger.exception( + "clear_device_discovery: failed to build catalog for device uuid=%s; skipping HA cleanup", + device_uuid, + ) + return + + cleared = 0 + for entry in catalog: + entity = entry.entity + # Only clear entities belonging to this device. + if entity.device.identifiers[1] != device_uuid: + continue + try: + topic, _config = build_discovery_payload(entity, discovery_prefix, state_prefix) + mqtt_manager.publish(topic, b"", retain=True) + cleared += 1 + logger.debug( + "clear_device_discovery: cleared config topic for entity %r → %s", + entity.key, + topic, + ) + except Exception: + logger.exception( + "clear_device_discovery: error clearing entity %r; continuing", entity.key + ) + + logger.info( + "clear_device_discovery: cleared %d HA discovery topic(s) for device uuid=%s", + cleared, + device_uuid, + ) + + def publish_device_offline(session: Session, device_uuid: str) -> None: """Publish an "offline" availability payload for *device_uuid*. diff --git a/frontend/src/api/schema.d.ts b/frontend/src/api/schema.d.ts index 214ca8c..e5c38bc 100644 --- a/frontend/src/api/schema.d.ts +++ b/frontend/src/api/schema.d.ts @@ -686,12 +686,23 @@ export interface paths { * Delete Device * @description Delete a Modbus device. * + * **Default behaviour (cascade=false)**: * Returns 409 Conflict if the device has any associated readings; use * ``enabled=false`` to disable it instead. * - * **Application-layer safety**: the check is performed via an explicit - * SELECT COUNT query — not by relying on SQLite's FK RESTRICT constraint, - * which is not enforced at runtime in this project (no PRAGMA foreign_keys=ON). + * **Cascade delete (cascade=true)**: + * Permanently deletes the device together with all its readings and any + * ``ExposedEntityToggle`` rows whose key matches ``modbus..*``. + * Also makes a best-effort attempt to clear the device's HA Discovery + * config topics from MQTT (empty retained payload) before the DB rows + * are removed. MQTT failures are swallowed — the DB deletion proceeds + * regardless. + * Returns HTTP 200 with a ``ModbusDeleteResponse`` JSON body on success. + * + * **Application-layer safety**: the 409 guard uses an explicit SELECT COUNT + * query to return a friendly message. FK RESTRICT is enforced at runtime + * (the app sets ``PRAGMA foreign_keys=ON``), so the cascade path deletes + * readings before the device. */ delete: operations["delete_device_api_modbus_devices__uuid__delete"]; options?: never; @@ -3190,7 +3201,9 @@ export interface operations { }; delete_device_api_modbus_devices__uuid__delete: { parameters: { - query?: never; + query?: { + cascade?: boolean; + }; header?: { "X-CSRF-Token"?: string | null; }; diff --git a/frontend/src/energy/hooks.test.tsx b/frontend/src/energy/hooks.test.tsx index 73d86bc..f375e69 100644 --- a/frontend/src/energy/hooks.test.tsx +++ b/frontend/src/energy/hooks.test.tsx @@ -173,7 +173,7 @@ describe('useUpdateDevice', () => { describe('useDeleteDevice', () => { beforeEach(() => vi.clearAllMocks()) - it('calls DELETE /api/modbus/devices/{uuid}', async () => { + it('calls DELETE /api/modbus/devices/{uuid} without query param when cascade omitted', async () => { mockDelete.mockResolvedValue({ data: null }) mockGet.mockResolvedValue({ data: { items: [], total: 0 } }) @@ -182,13 +182,32 @@ describe('useDeleteDevice', () => { const { result } = renderHook(() => useDeleteDevice(), { wrapper: Wrapper }) await act(async () => { - await result.current.mutateAsync('test-uuid-1') + await result.current.mutateAsync({ uuid: 'test-uuid-1' }) }) expect(mockDelete).toHaveBeenCalledWith('/api/modbus/devices/{uuid}', { params: { path: { uuid: 'test-uuid-1' } }, }) }) + + it('calls DELETE /api/modbus/devices/{uuid} with cascade=true in query when cascade=true', async () => { + mockDelete.mockResolvedValue({ + data: { deleted: true, readings_deleted: 5, toggles_deleted: 2 }, + }) + mockGet.mockResolvedValue({ data: { items: [], total: 0 } }) + + const { Wrapper } = makeWrapper() + const { useDeleteDevice } = await import('./hooks') + const { result } = renderHook(() => useDeleteDevice(), { wrapper: Wrapper }) + + await act(async () => { + await result.current.mutateAsync({ uuid: 'test-uuid-1', cascade: true }) + }) + + expect(mockDelete).toHaveBeenCalledWith('/api/modbus/devices/{uuid}', { + params: { path: { uuid: 'test-uuid-1' }, query: { cascade: true } }, + }) + }) }) describe('useTestReadDevice', () => { diff --git a/frontend/src/energy/hooks.ts b/frontend/src/energy/hooks.ts index e13ae1e..90502d5 100644 --- a/frontend/src/energy/hooks.ts +++ b/frontend/src/energy/hooks.ts @@ -115,12 +115,21 @@ export function useUpdateDevice() { // Mutation: delete device // --------------------------------------------------------------------------- +export interface DeleteDeviceParams { + uuid: string + /** When true, perform a cascade delete (removes readings + expose toggles). */ + cascade?: boolean +} + export function useDeleteDevice() { const qc = useQueryClient() return useMutation({ - mutationFn: (uuid: string) => + mutationFn: ({ uuid, cascade }: DeleteDeviceParams) => apiClient.DELETE('/api/modbus/devices/{uuid}', { - params: { path: { uuid } }, + params: { + path: { uuid }, + ...(cascade ? { query: { cascade: true } } : {}), + }, }), onSuccess: () => qc.invalidateQueries({ queryKey: ['modbus-devices'] }), }) diff --git a/frontend/src/pages/EnergyPage.test.tsx b/frontend/src/pages/EnergyPage.test.tsx index c47f257..0e0c518 100644 --- a/frontend/src/pages/EnergyPage.test.tsx +++ b/frontend/src/pages/EnergyPage.test.tsx @@ -288,6 +288,7 @@ describe('EnergyPage — delete device', () => { await waitFor(() => expect(screen.getByTestId(`device-delete-${DEVICE.uuid}`)).toBeInTheDocument()) fireEvent.click(screen.getByTestId(`device-delete-${DEVICE.uuid}`)) + // On first open (no 409 yet), the regular confirm button is visible. await waitFor(() => expect(screen.getByTestId('device-delete-confirm')).toBeInTheDocument()) fireEvent.click(screen.getByTestId('device-delete-confirm')) @@ -295,6 +296,38 @@ describe('EnergyPage — delete device', () => { await waitFor(() => { expect(screen.getByTestId('device-delete-409-hint')).toBeInTheDocument() }) + // After 409, the force-delete button appears instead of the normal confirm. + expect(screen.getByTestId('device-delete-force')).toBeInTheDocument() + expect(screen.queryByTestId('device-delete-confirm')).not.toBeInTheDocument() + }) + + it('force-delete button triggers cascade delete after 409', async () => { + const { ApiError } = await import('../api/client') + // First call → 409, second call (cascade) → success + mockDelete + .mockRejectedValueOnce(new ApiError(409, { detail: 'device has readings' })) + .mockResolvedValueOnce({ data: { deleted: true, readings_deleted: 3, toggles_deleted: 1 } }) + + renderEnergy() + + await waitFor(() => expect(screen.getByTestId(`device-delete-${DEVICE.uuid}`)).toBeInTheDocument()) + fireEvent.click(screen.getByTestId(`device-delete-${DEVICE.uuid}`)) + await waitFor(() => expect(screen.getByTestId('device-delete-confirm')).toBeInTheDocument()) + + // First attempt → 409 + fireEvent.click(screen.getByTestId('device-delete-confirm')) + + // Force-delete button appears + await waitFor(() => expect(screen.getByTestId('device-delete-force')).toBeInTheDocument()) + + // Click force-delete → should call DELETE with cascade=true + fireEvent.click(screen.getByTestId('device-delete-force')) + + await waitFor(() => { + expect(mockDelete).toHaveBeenCalledWith('/api/modbus/devices/{uuid}', { + params: { path: { uuid: DEVICE.uuid }, query: { cascade: true } }, + }) + }) }) }) diff --git a/frontend/src/pages/EnergyPage.tsx b/frontend/src/pages/EnergyPage.tsx index f9518c8..fbaafb3 100644 --- a/frontend/src/pages/EnergyPage.tsx +++ b/frontend/src/pages/EnergyPage.tsx @@ -57,13 +57,22 @@ import { formatLocalDateTime } from '../utils/datetime' interface ConfirmDeleteProps { device: ModbusDevice onConfirm: () => void + /** Called when the user explicitly opts into cascade (force) deletion. */ + onForceDelete: () => void onCancel: () => void loading: boolean /** Set when the delete failed with 409. */ has409Error: boolean } -function ConfirmDeleteModal({ device, onConfirm, onCancel, loading, has409Error }: ConfirmDeleteProps) { +function ConfirmDeleteModal({ + device, + onConfirm, + onForceDelete, + onCancel, + loading, + has409Error, +}: ConfirmDeleteProps) { return ( {has409Error && ( - - This device has existing readings and cannot be deleted. Consider{' '} - disabling it instead (click Edit → uncheck Enabled). - + <> + + This device has existing readings and cannot be deleted. Consider{' '} + disabling it instead (click Edit → uncheck Enabled). + + + Alternatively, you can permanently delete the device together with{' '} + all its historical readings and expose settings. This action{' '} + cannot be undone. + + )} - + {!has409Error && ( + + )} + {has409Error && ( + + )} @@ -532,7 +560,7 @@ function DevicesTab() { if (!deleteDevice) return setDelete409(false) try { - await deleteMutation.mutateAsync(deleteDevice.uuid) + await deleteMutation.mutateAsync({ uuid: deleteDevice.uuid }) closeDelete() } catch (err) { if (err instanceof ApiError && err.status === 409) { @@ -542,6 +570,17 @@ function DevicesTab() { } } + async function handleForceDelete() { + if (!deleteDevice) return + try { + await deleteMutation.mutateAsync({ uuid: deleteDevice.uuid, cascade: true }) + closeDelete() + } catch { + // If cascade delete also fails for some reason, keep the modal open. + // (Should be very rare — only if the device was concurrently deleted.) + } + } + if (devicesQuery.isLoading) { return (
@@ -600,6 +639,7 @@ function DevicesTab() { .*``.\nAlso makes a best-effort attempt to clear the device's HA Discovery\nconfig topics from MQTT (empty retained payload) before the DB rows\nare removed. MQTT failures are swallowed — the DB deletion proceeds\nregardless.\nReturns HTTP 200 with a ``ModbusDeleteResponse`` JSON body on success.\n\n**Application-layer safety**: the 409 guard uses an explicit SELECT COUNT\nquery to return a friendly message. FK RESTRICT is enforced at runtime\n(the app sets ``PRAGMA foreign_keys=ON``), so the cascade path deletes\nreadings before the device.", "operationId": "delete_device_api_modbus_devices__uuid__delete", "parameters": [ { @@ -1737,6 +1737,16 @@ "title": "Uuid" } }, + { + "name": "cascade", + "in": "query", + "required": false, + "schema": { + "type": "boolean", + "default": false, + "title": "Cascade" + } + }, { "name": "X-CSRF-Token", "in": "header", diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml index d38b4c6..15241ae 100644 --- a/openapi/openapi.yaml +++ b/openapi/openapi.yaml @@ -1332,16 +1332,37 @@ paths: description: 'Delete a Modbus device. + **Default behaviour (cascade=false)**: + Returns 409 Conflict if the device has any associated readings; use ``enabled=false`` to disable it instead. - **Application-layer safety**: the check is performed via an explicit + **Cascade delete (cascade=true)**: - SELECT COUNT query — not by relying on SQLite''s FK RESTRICT constraint, + Permanently deletes the device together with all its readings and any - which is not enforced at runtime in this project (no PRAGMA foreign_keys=ON).' + ``ExposedEntityToggle`` rows whose key matches ``modbus..*``. + + Also makes a best-effort attempt to clear the device''s HA Discovery + + config topics from MQTT (empty retained payload) before the DB rows + + are removed. MQTT failures are swallowed — the DB deletion proceeds + + regardless. + + Returns HTTP 200 with a ``ModbusDeleteResponse`` JSON body on success. + + + **Application-layer safety**: the 409 guard uses an explicit SELECT COUNT + + query to return a friendly message. FK RESTRICT is enforced at runtime + + (the app sets ``PRAGMA foreign_keys=ON``), so the cascade path deletes + + readings before the device.' operationId: delete_device_api_modbus_devices__uuid__delete parameters: - name: uuid @@ -1350,6 +1371,13 @@ paths: schema: type: string title: Uuid + - name: cascade + in: query + required: false + schema: + type: boolean + default: false + title: Cascade - name: X-CSRF-Token in: header required: false diff --git a/tests/test_api_modbus.py b/tests/test_api_modbus.py index d881750..dffb3f8 100644 --- a/tests/test_api_modbus.py +++ b/tests/test_api_modbus.py @@ -432,6 +432,158 @@ def test_delete_device_not_found_returns_404(modbus_client): assert resp.status_code == 404 +# --------------------------------------------------------------------------- +# DELETE /api/modbus/devices/{uuid}?cascade=true +# --------------------------------------------------------------------------- + + +def test_cascade_false_with_readings_still_returns_409(modbus_client): + """cascade=false (default) + readings → 409, device and readings remain.""" + client, engine = modbus_client + _login(client) + device = _make_device(engine) + _make_reading(engine, device.id, datetime.now(UTC), {"voltage": 230.0}) + + resp = client.delete( + f"/api/modbus/devices/{device.uuid}", + params={"cascade": "false"}, + headers={"X-CSRF-Token": _CSRF}, + ) + assert resp.status_code == 409 + + # Device and reading still in DB + with Session(engine) as session: + assert session.query(ModbusDevice).count() == 1 + assert session.query(ModbusReading).count() == 1 + + +def test_cascade_true_with_readings_deletes_device_readings_and_toggles(modbus_client): + """cascade=true + readings → device, readings, and toggle rows all deleted; 200 response.""" + from datetime import UTC + from app.models.expose import ExposedEntityToggle + + client, engine = modbus_client + _login(client) + device = _make_device(engine) + + # Add two readings + now = datetime.now(UTC) + _make_reading(engine, device.id, now, {"voltage": 230.0}) + _make_reading(engine, device.id, now - timedelta(minutes=1), {"voltage": 229.0}) + + # Add an ExposedEntityToggle row for this device + with Session(engine) as session: + toggle = ExposedEntityToggle( + key=f"modbus.{device.uuid}.voltage", + enabled=True, + updated_at=now, + ) + session.add(toggle) + # Add one toggle for a different device to verify isolation + other_toggle = ExposedEntityToggle( + key="modbus.other-uuid.voltage", + enabled=True, + updated_at=now, + ) + session.add(other_toggle) + session.commit() + + resp = client.delete( + f"/api/modbus/devices/{device.uuid}", + params={"cascade": "true"}, + headers={"X-CSRF-Token": _CSRF}, + ) + assert resp.status_code == 200 + body = resp.json() + assert body["deleted"] is True + assert body["readings_deleted"] == 2 + assert body["toggles_deleted"] == 1 + + # All rows for this device must be gone + with Session(engine) as session: + assert session.query(ModbusDevice).count() == 0 + assert session.query(ModbusReading).count() == 0 + # Only the other device's toggle remains + remaining_toggles = session.query(ExposedEntityToggle).all() + assert len(remaining_toggles) == 1 + assert remaining_toggles[0].key == "modbus.other-uuid.voltage" + + +def test_cascade_true_no_readings_deletes_device(modbus_client): + """cascade=true + no readings → device deleted, response shows 0 counts.""" + client, engine = modbus_client + _login(client) + device = _make_device(engine) + + resp = client.delete( + f"/api/modbus/devices/{device.uuid}", + params={"cascade": "true"}, + headers={"X-CSRF-Token": _CSRF}, + ) + assert resp.status_code == 200 + body = resp.json() + assert body["deleted"] is True + assert body["readings_deleted"] == 0 + assert body["toggles_deleted"] == 0 + + with Session(engine) as session: + assert session.query(ModbusDevice).count() == 0 + + +def test_cascade_unauthenticated_returns_401(modbus_client): + """Unauthenticated cascade delete request → 401.""" + client, engine = modbus_client + device = _make_device(engine) + resp = client.delete( + f"/api/modbus/devices/{device.uuid}", + params={"cascade": "true"}, + headers={"X-CSRF-Token": _CSRF}, + ) + assert resp.status_code == 401 + + +def test_cascade_missing_csrf_returns_403(modbus_client): + """cascade=true without CSRF → 403.""" + client, engine = modbus_client + _login(client) + device = _make_device(engine) + resp = client.delete( + f"/api/modbus/devices/{device.uuid}", + params={"cascade": "true"}, + ) + assert resp.status_code == 403 + + +def test_cascade_mqtt_unavailable_still_deletes(modbus_client): + """When MQTT is not connected, cascade delete must still succeed (HA cleanup is best-effort).""" + from unittest.mock import patch + + client, engine = modbus_client + _login(client) + device = _make_device(engine) + _make_reading(engine, device.id, datetime.now(UTC), {"voltage": 230.0}) + + # Simulate MQTT not connected so clear_device_discovery is a no-op + with patch("app.services.ha_discovery.mqtt_manager") as mock_mqtt: + mock_mqtt.is_connected = False + + resp = client.delete( + f"/api/modbus/devices/{device.uuid}", + params={"cascade": "true"}, + headers={"X-CSRF-Token": _CSRF}, + ) + + assert resp.status_code == 200 + body = resp.json() + assert body["deleted"] is True + assert body["readings_deleted"] == 1 + + # DB rows gone + with Session(engine) as session: + assert session.query(ModbusDevice).count() == 0 + assert session.query(ModbusReading).count() == 0 + + # --------------------------------------------------------------------------- # GET /api/modbus/devices/{uuid}/latest # ---------------------------------------------------------------------------