FreeTLab
bccc5c48b8
Persist the exploration, proposal, spec, design, tasks, and verify/archive reports produced during the multi-device EQ, alarm-countdown, and notification-visual-polish SDD cycles.
139 lines
14 KiB
Markdown
139 lines
14 KiB
Markdown
# Technical Design: snooze-reschedule-fix
|
|
|
|
## 1. Executive Summary
|
|
|
|
Make both snooze reschedule methods in `EstadoAlarmas` mirror the proven `guardarAlarma()` error-handling shape: permission pre-check, try/catch around the native `programar()` call that records the failure into `_error`, and a `notifyListeners()` that is ALWAYS reached so the widget tree rebuilds even when native scheduling fails. Surface the failure to the user from the two UI call sites via the existing `ScaffoldMessenger` SnackBar pattern. The change is 100% Dart-side; no Kotlin/native edits are required for the fix.
|
|
|
|
## 2. Architecture Approach
|
|
|
|
### 2.1 Pattern: mirror the canonical write path
|
|
|
|
The reference is `guardarAlarma()` (estado_alarmas.dart L98-115). Its shape:
|
|
|
|
```
|
|
_aplicar(config); // mutate in-memory state FIRST
|
|
try {
|
|
await _solicitarPermisosNecesariosParaAlarma(); // permission pre-check
|
|
await android.programar(guardada); // native schedule (may throw StateError)
|
|
} catch (e) {
|
|
_error = '...: $e'; // record failure, do NOT rethrow
|
|
}
|
|
notifyListeners(); // ALWAYS reached (catch swallows the throw)
|
|
```
|
|
|
|
Critical structural insight: `guardarAlarma()` does NOT use a `finally` block. `notifyListeners()` is an unconditional statement that follows a try/catch whose `catch` swallows the exception. Because the catch never rethrows, control always falls through to `notifyListeners()`. This is the exact pattern both snooze methods must adopt. Using `try/catch` + trailing `notifyListeners()` (NOT `try/finally`) keeps the two methods textually consistent with `guardarAlarma()` and avoids the subtle difference of a `finally` that would also run on an unexpected rethrow.
|
|
|
|
### 2.2 Layering / boundaries (unchanged)
|
|
|
|
The existing hexagonal boundary is preserved:
|
|
|
|
- `PantallaAlarmaSonando._posponer()` and `app.dart` POSTPONE_NEXT handler — UI/adapter layer (drives feedback).
|
|
- `EstadoAlarmas` — application/state layer (ChangeNotifier, single source of truth).
|
|
- `PuertoAlarmasAndroid` (port) / `ServicioAlarmasAndroid` (adapter) — native boundary.
|
|
|
|
This fix touches only the state layer (error handling) and the UI layer (feedback). No port signatures change. No persistence-format change.
|
|
|
|
## 3. Component & Data-Flow Design
|
|
|
|
### 3.1 `EstadoAlarmas.posponerAlarma()` (target rewrite)
|
|
|
|
Current (L194-212) awaits `android.programar(actualizada)` with no try/catch; a thrown `StateError` skips `notifyListeners()` (L211). Redesigned data flow:
|
|
|
|
1. Compute `ejecucion`, call `android.ocultarNotificacionAlarma(alarma.id)` (unchanged — hides ring notification; keep outside the guarded block, same as it is today, since it is not the failure point of interest and dismiss must happen regardless).
|
|
2. `final config = await servicio.posponerEjecucion(...)` then `_aplicar(config)` — in-memory snooze state committed FIRST (mirrors guardarAlarma ordering).
|
|
3. `final actualizada = _buscarAlarma(alarma.id);`
|
|
4. Guarded block:
|
|
- `await _solicitarPermisosNecesariosParaAlarma();` (NEW — re-requests revoked exact-alarm/notification permission at snooze time, no-op when already granted).
|
|
- `if (actualizada != null) await android.programar(actualizada);`
|
|
- `catch (e) { _error = '<message>: $e'; }`
|
|
5. `notifyListeners();` — unconditional, always reached.
|
|
|
|
### 3.2 `EstadoAlarmas.posponerProximaDesdePreaviso()` (identical treatment)
|
|
|
|
Same restructuring (L214-236): keep `_snoozeSeguro`/`snoozeHasta` compute and `ocultarNotificacionAlarma` as-is; wrap `_solicitarPermisosNecesariosParaAlarma()` + `android.programar(actualizada)` in try/catch into `_error`; unconditional `notifyListeners()` after.
|
|
|
|
### 3.3 Decision — catch type: generic `Exception`/`Object`, not `StateError`
|
|
|
|
`programar()` throws `StateError` (servicio_alarmas_android.dart L263-264) today, but:
|
|
- `_solicitarPermisosNecesariosParaAlarma()` already swallows its own errors internally, yet a future change or a `diagnostico()` MethodChannel failure could surface other exception types.
|
|
- `guardarAlarma()` uses an untyped `catch (e)` (catches any `Object`). Mirroring it means using `catch (e)` (untyped) here too.
|
|
|
|
Decision: use untyped `catch (e)` for parity with `guardarAlarma()` and resilience against any thrown type. Do NOT narrow to `on StateError` — narrowing would let a `PlatformException` or other failure escape past `notifyListeners()`, reintroducing the exact stale-UI bug we are fixing.
|
|
|
|
### 3.4 Decision — failure signal propagation to the UI
|
|
|
|
Two options were considered:
|
|
|
|
- **Option A — method returns `bool` / result object.** `posponerAlarma()` returns `false` on failure; the screen reads it and shows a SnackBar.
|
|
- **Option B — method swallows into `_error`; UI reads `estado.error` (or method still throws and UI catches).**
|
|
|
|
Decision: **hybrid that keeps the method non-throwing (Option B-style) and exposes a return value for the immediate caller.** Concretely:
|
|
|
|
- The methods remain `Future<void>` but record failure into the existing `_error` field (already exposed via the `error` getter, L61). This keeps them faithful to `guardarAlarma()` (which also surfaces failures via `_error`, never throws to its caller).
|
|
- For `_posponer()` (ringing screen) the SnackBar cannot be shown on the about-to-be-dismissed `PantallaAlarmaSonando` scaffold. The dismiss is by design (L177-187). Therefore the failure feedback must survive screen dismissal. Resolution: capture the `ScaffoldMessenger` from the ROOT navigator/messenger BEFORE dismiss (or read `estado.error` after the awaited call and post via the app-level messenger), since `app.dart` mounts the persistent `Scaffold`/`ScaffoldMessenger` that outlives the ringing screen.
|
|
|
|
Rationale for not throwing: throwing would force every caller (the ringing screen AND the app.dart POSTPONE_NEXT handler) to wrap in try/catch and would re-create divergent error handling — the opposite of the consistency this change is buying. Recording into `_error` and letting callers read it (or returning a `bool` for ergonomics) centralizes the contract.
|
|
|
|
#### 3.4.1 Concrete UI wiring
|
|
|
|
- `_posponer()` (pantalla_alarma_sonando.dart L173-188): capture `final messenger = ScaffoldMessenger.of(context);` BEFORE the `await` (before any dismiss). After `await alarmas.posponerAlarma(...)`, if `alarmas.error != null`, call `messenger.showSnackBar(SnackBar(content: Text(<localized failure>)))`. Keep the existing `try/catch`+`finally` dismiss intact (defensive: the method should no longer throw, but the dismiss-in-finally stays as a safety net). Because the method no longer throws on a native failure, the existing `catch (e) { debugPrint }` becomes a pure safety net for unexpected throws.
|
|
- `app.dart` POSTPONE_NEXT handler (L291-311): currently shows the success SnackBar (`alarmPostponedCurrentExecution`) unconditionally. After `await estado.posponerProximaDesdePreaviso(...)`, branch on `estado.error`: show the failure SnackBar when `error != null`, otherwise the existing success SnackBar. `context`/`mounted` is already guarded there.
|
|
|
|
### 3.5 Decision — clearing `_error`
|
|
|
|
`guardarAlarma()` sets `_error` on failure but never clears it on success (pre-existing behavior). To avoid a stale SnackBar firing on a later successful snooze, set `_error = null` at the START of each snooze method (before the guarded block), so a successful run leaves `error == null` and the UI shows the success path. This is a small, local improvement consistent with `inicializar()` which sets `_error = null` up front (L77).
|
|
|
|
## 4. Native (Kotlin) scope confirmation
|
|
|
|
The fix is **100% Dart-side**. The Kotlin early-`return false` in `AlarmScheduler.scheduleSpec()` (which skips `scheduleSnoozeCountdown` when the main fire fails) is the CORRECT behavior: if the OS refused to arm the real alarm, posting a countdown notification for an alarm that will never ring would be a lie. Once the Dart permission pre-check re-arms the exact-alarm permission and `scheduleMainAlarm` succeeds, the countdown is posted by the existing Kotlin path with no change. The optional `AlarmScheduler.kt` "distinguish main-fire-failed" item from the proposal is explicitly OUT for this change — the Dart fix fully addresses all three symptoms (no re-fire, no countdown, stale UI) without it.
|
|
|
|
## 5. Testability Design (Strict TDD)
|
|
|
|
Current `FakePuertoAlarmasAndroid.programar()` (test/helpers/fakes_alarmas.dart L30-32) never throws, so failure cannot yet be simulated. Design additions:
|
|
|
|
### 5.1 Fake enhancement
|
|
Add a controllable failure switch to `FakePuertoAlarmasAndroid`:
|
|
- `bool fallaProgramar = false;` (or a `Object? errorProgramar`) — when set, `programar()` throws `StateError('...')` to emulate the real adapter's L263-264 throw.
|
|
- Optionally record permission-request calls (counters already partially exist via `solicitudesExencionBateria`) to assert the pre-check ran.
|
|
|
|
### 5.2 State-layer tests (test/estado/estado_alarmas_snooze_test.dart)
|
|
- `posponerAlarma` failure path: set `android.fallaProgramar = true`, register a listener, call `posponerAlarma`, assert (a) it does NOT throw, (b) `notifyListeners` fired (notification count >= 1), (c) `estado.error != null`, (d) in-memory `snoozeHasta` still committed (`_aplicar` ran before the throw).
|
|
- `posponerAlarma` success path: existing tests already cover; add explicit `expect(estado.error, isNull)` (validates §3.5 clearing).
|
|
- `posponerAlarma` re-requests permission: assert the diagnostico/permission flow was invoked (mirror how guardarAlarma is implicitly covered).
|
|
- Mirror all three for `posponerProximaDesdePreaviso` (currently has thinner coverage).
|
|
|
|
### 5.3 Widget-layer tests (test/pantallas/pantalla_alarma_sonando_*)
|
|
- Snooze failure shows a SnackBar: pump `PantallaAlarmaSonando` with an `EstadoAlarmas` whose fake throws, tap a snooze option, `await tester.pump()`, assert a `SnackBar` with the failure text is present on the app-level messenger AND the screen dismissed. Reuse the existing dismiss-guard test scaffolding (pantalla_alarma_sonando_dismiss_guard_test.dart) which already drives `_posponer`.
|
|
- Snooze success shows no failure SnackBar (regression guard).
|
|
|
|
### 5.4 l10n
|
|
A new failure-feedback string is needed for the SnackBar. `androidExactAlarmScheduleError` already exists (all 15 locales, L525/L557) and is the message thrown by `programar()`. The SnackBar can reuse `estado.error` text directly (which already contains that localized message) OR introduce a dedicated `alarmSnoozeFailed` key. Decision: reuse the message already captured in `_error` to avoid touching 15 ARB files; the tasks phase may add a wrapper key only if a snooze-specific phrasing is desired. Keep the l10n surface minimal for this fix.
|
|
|
|
## 6. ADR-style Decisions
|
|
|
|
| ID | Decision | Rationale | Rejected alternative |
|
|
|----|----------|-----------|----------------------|
|
|
| D1 | Mirror `guardarAlarma()` shape (try/catch + trailing unconditional `notifyListeners()`), NOT try/finally | Textual parity with the proven reference; avoids `finally`-on-rethrow subtlety | `try/finally` with rethrow — would still propagate the throw to callers, breaking the no-throw contract |
|
|
| D2 | Untyped `catch (e)` | Parity with guardarAlarma; resilient to non-StateError failures (PlatformException, diagnostico errors) | `on StateError` — too narrow, lets other failures escape past notifyListeners (reintroduces the bug) |
|
|
| D3 | Methods stay `Future<void>`, record into existing `_error`; UI reads `estado.error` after await | Centralizes contract, avoids divergent per-caller try/catch, faithful to guardarAlarma | Method throws and each caller catches — re-creates the divergence we are removing |
|
|
| D4 | Capture root `ScaffoldMessenger` before dismiss / use app-level messenger | Ringing screen dismisses by design; SnackBar must outlive it | Show SnackBar on the ringing screen — impossible, scaffold is being torn down |
|
|
| D5 | Clear `_error = null` at start of each snooze method | Prevents stale failure SnackBar on a later successful snooze | Leave _error sticky — false-positive failure feedback |
|
|
| D6 | 100% Dart, no Kotlin edit | Kotlin early-return-false is correct; Dart permission pre-check + UI feedback fully resolve all 3 symptoms | Edit AlarmScheduler.kt to split countdown — out of scope, untested native infra, no benefit once main fire succeeds |
|
|
| D7 | Add `fallaProgramar` switch to FakePuertoAlarmasAndroid | Enables Strict-TDD failure-path coverage that is impossible today | Mock library / new fake — unnecessary, existing shared fake is the right home |
|
|
|
|
## 7. Affected Files (design-level map)
|
|
|
|
- `lib/estado/estado_alarmas.dart` — restructure `posponerAlarma()` (L194-212) and `posponerProximaDesdePreaviso()` (L214-236): clear `_error`, permission pre-check, guarded `programar()`, unconditional `notifyListeners()`.
|
|
- `lib/pantallas/pantalla_alarma_sonando.dart` — `_posponer()` (L173-188): capture messenger before dismiss, post failure SnackBar from `estado.error`.
|
|
- `lib/app.dart` — POSTPONE_NEXT handler (L291-311): branch success vs failure SnackBar on `estado.error`.
|
|
- `test/helpers/fakes_alarmas.dart` — add `fallaProgramar` failure switch to `programar()`.
|
|
- `test/estado/estado_alarmas_snooze_test.dart` — failure + success + permission-precheck tests for both methods.
|
|
- `test/pantallas/pantalla_alarma_sonando_*` — widget test for failure SnackBar + dismiss.
|
|
|
|
## 8. Architectural Risks / Open Items
|
|
|
|
- R1 (Med): Root cause is permission-revocation, only fully confirmable via device logcat. Fix is valid regardless — the missing try/catch + missing notifyListeners is a confirmed structural defect. The pre-check is the recovery mechanism.
|
|
- R2 (Low): Re-requesting permission mid-ring could momentarily surface a system dialog over the ringing screen. Mirrors guardarAlarma exactly and is a no-op when permission is granted; acceptable.
|
|
- R3 (Low): SnackBar timing across screen dismiss — must capture the messenger/read error after the awaited call but before/independent of the dismiss. Covered by widget test 5.3.
|
|
- R4 (assumption): app.dart mounts a persistent ScaffoldMessenger that outlives the ringing screen — confirmed by existing SnackBar usage at app.dart L280/L303 inside the same handler.
|