docs(openspec): add SDD artifact trail for recent alarm and EQ changes
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.
This commit is contained in:
@@ -0,0 +1,161 @@
|
||||
# Tasks: snooze-reschedule-fix
|
||||
|
||||
Strict TDD active. Every behavioral task = RED (failing test) -> GREEN (minimal fix) -> REFACTOR (cleanup, still green). Tasks are numbered hierarchically; "Parallel" tasks have no file overlap with concurrently-listed siblings and may be done in any order or by different people; "Sequential" tasks depend on a prior task's output and must follow it.
|
||||
|
||||
Affected files: `lib/estado/estado_alarmas.dart`, `lib/pantallas/pantalla_alarma_sonando.dart`, `lib/app.dart`, `test/helpers/fakes_alarmas.dart`, `test/estado/estado_alarmas_snooze_test.dart`, `test/pantallas/pantalla_alarma_sonando_dismiss_guard_test.dart` (or new `_failure_test.dart`).
|
||||
|
||||
---
|
||||
|
||||
## Phase 1 — Test infrastructure (failure-path enablement)
|
||||
|
||||
### 1.1 [x] Add `fallaProgramar` switch to `FakePuertoAlarmasAndroid` (Sequential — blocks all RED tasks below)
|
||||
- Satisfies: Design D7; Spec Requirement "Native Scheduling Failure Must Not Corrupt UI State".
|
||||
- File: `test/helpers/fakes_alarmas.dart`.
|
||||
- Add a `bool fallaProgramar = false` field to `FakePuertoAlarmasAndroid`.
|
||||
- In `programar()`, if `fallaProgramar` is true, `throw StateError('fake programar failure')` before appending to `programadas`. If false, behave exactly as today (append and return).
|
||||
- No test asserts on the fake itself (it's a test double) — verify by compiling and running the existing snooze test suite unchanged (must stay green, zero behavior change for `fallaProgramar == false`).
|
||||
- Cannot run RED for 1.2-1.5 without this; this task ships first and alone.
|
||||
|
||||
---
|
||||
|
||||
## Phase 2 — `posponerAlarma()` reliability (RED -> GREEN -> REFACTOR)
|
||||
|
||||
### 2.1 [x] [RED] Failure path does not throw, calls notifyListeners, records `_error` (Sequential — depends on 1.1)
|
||||
- Satisfies: Spec "Native Scheduling Failure Must Not Corrupt UI State" / Scenario "Native scheduling throws (failure path)".
|
||||
- File: `test/estado/estado_alarmas_snooze_test.dart`.
|
||||
- Add test: build `EstadoAlarmas` with `android.fallaProgramar = true` after `guardarAlarma`, call `await estado.posponerAlarma(alarma, minutos)`.
|
||||
- Assert: the `await` does NOT throw; `estado.error` is non-null after the call; a `notifyListeners()` fired (use an `addListener` counter, expect `>= 1`); `estado.alarmas.single.snoozeHasta` is still committed to the new value (proves `_aplicar(config)` ran before the native call, per design — in-memory state commits first).
|
||||
- Run: test MUST fail (current code has no try/catch — exception propagates uncaught, test fails on the `await` line, or `_error` stays null).
|
||||
|
||||
### 2.2 [x] [GREEN] Implement try/catch + permission pre-check + unconditional notifyListeners in `posponerAlarma()` (Sequential — depends on 2.1)
|
||||
- Satisfies: Spec "Permission Pre-Check Before Snooze Scheduling" + "Native Scheduling Failure Must Not Corrupt UI State"; Design D1, D2, D3, D5.
|
||||
- File: `lib/estado/estado_alarmas.dart`, method `posponerAlarma()` (current L194-212).
|
||||
- Add `_error = null;` at the start of the method (Design D5 — clear stale failure before a fresh attempt).
|
||||
- Wrap `_solicitarPermisosNecesariosParaAlarma()` (new call, Design's permission pre-check) + `await android.programar(actualizada)` in `try { ... } catch (e) { _error = '...'; }` — untyped `catch (e)`, NOT `on StateError` (Design D2).
|
||||
- Keep the catch block non-rethrowing (swallow into `_error`, mirror `guardarAlarma()` exactly).
|
||||
- Move `notifyListeners()` so it executes unconditionally after the try/catch (NOT inside try, NOT in finally — trailing call per Design D1).
|
||||
- Run: 2.1 test MUST pass. Run full `estado_alarmas_snooze_test.dart` suite — all prior tests (success path, sync state, native-event paths) MUST stay green with zero behavior change when `fallaProgramar == false`.
|
||||
|
||||
### 2.3 [x] [RED] Success path leaves `_error` null (regression guard for D5) (Sequential — depends on 2.2)
|
||||
- Satisfies: Design D5 (stale-error clearing); Spec Scenario "Native scheduling succeeds (happy path)".
|
||||
- File: `test/estado/estado_alarmas_snooze_test.dart`.
|
||||
- Add test: trigger a failed `posponerAlarma` first (`fallaProgramar = true`, confirm `estado.error != null`), then set `android.fallaProgramar = false` and call `posponerAlarma` again on the same alarm.
|
||||
- Assert: `estado.error` is `null` after the second (successful) call.
|
||||
- This MUST already pass after 2.2 (the `_error = null` reset at method start covers it) — write it as a deliberate regression guard, run it to confirm GREEN immediately; if it fails, the `_error = null` placement in 2.2 was wrong and must be fixed before continuing (loop back into 2.2, do not proceed to Phase 3).
|
||||
|
||||
### 2.4 [x] [REFACTOR] Cleanup pass on `posponerAlarma()` (Sequential — depends on 2.2, 2.3 green)
|
||||
- Satisfies: code quality, no spec/design behavior change.
|
||||
- File: `lib/estado/estado_alarmas.dart`.
|
||||
- Re-read the method top to bottom: confirm debugPrint placement still makes sense, confirm variable naming, confirm the catch comment matches `guardarAlarma()`'s phrasing style (e.g. "Alarma pospuesta, pero Android no pudo reprogramarla todavía: $e") for consistency with the existing error message in `guardarAlarma()`.
|
||||
- No new test required — full suite must remain green (this step touches no observable behavior).
|
||||
|
||||
---
|
||||
|
||||
## Phase 3 — `posponerProximaDesdePreaviso()` reliability (mirror of Phase 2)
|
||||
|
||||
### 3.1 [x] [RED] Failure path does not throw, calls notifyListeners, records `_error` (Parallel with Phase 2 once 1.1 lands — different test cases, same file; sequence within this phase is fixed)
|
||||
- Satisfies: Spec Scenario "Pre-notice snooze failure (variant path)".
|
||||
- File: `test/estado/estado_alarmas_snooze_test.dart`.
|
||||
- Add test mirroring 2.1 but calling `await estado.posponerProximaDesdePreaviso(alarma, minutos, ejecucion)` with `android.fallaProgramar = true`.
|
||||
- Assert: no throw; `estado.error` non-null; `notifyListeners()` fired; `snoozeHasta`/`snoozeOrigen` still committed (state mutation happens before native call, same as `posponerAlarma`).
|
||||
- Run: test MUST fail against current code (no try/catch in this method either).
|
||||
|
||||
### 3.2 [x] [GREEN] Implement identical try/catch + permission pre-check + unconditional notifyListeners in `posponerProximaDesdePreaviso()` (Sequential — depends on 3.1)
|
||||
- Satisfies: Spec "Permission Pre-Check Before Snooze Scheduling" + "Native Scheduling Failure Must Not Corrupt UI State"; Design D1-D5 (same shape as 2.2).
|
||||
- File: `lib/estado/estado_alarmas.dart`, method `posponerProximaDesdePreaviso()` (current L214-236).
|
||||
- Same changes as 2.2: `_error = null;` at start; guard `_solicitarPermisosNecesariosParaAlarma()` + `android.programar(actualizada)` in try/catch(e); unconditional `notifyListeners()` after.
|
||||
- Keep `_snoozeSeguro`/`snoozeHasta` computation and `ocultarNotificacionAlarma` call untouched (outside the guard, per Design).
|
||||
- Run: 3.1 test MUST pass. Full suite stays green.
|
||||
|
||||
### 3.3 [x] [RED] Success path leaves `_error` null for preaviso method (Sequential — depends on 3.2)
|
||||
- Satisfies: Design D5 regression guard, preaviso variant.
|
||||
- File: `test/estado/estado_alarmas_snooze_test.dart`.
|
||||
- Mirror 2.3: fail once, then succeed, assert `estado.error == null` after the successful call.
|
||||
- MUST pass immediately after 3.2; if not, fix `_error = null` placement before proceeding.
|
||||
|
||||
### 3.4 [x] [REFACTOR] Cleanup pass on `posponerProximaDesdePreaviso()` (Sequential — depends on 3.2, 3.3 green)
|
||||
- Satisfies: code quality, no behavior change.
|
||||
- File: `lib/estado/estado_alarmas.dart`.
|
||||
- Align error message phrasing with 2.4's, confirm both snooze methods now read as structural twins of `guardarAlarma()`.
|
||||
- Full suite must remain green.
|
||||
|
||||
---
|
||||
|
||||
## Phase 4 — UI failure feedback: ringing-screen `_posponer()`
|
||||
|
||||
### 4.1 [x] [RED] Failure surfaces a SnackBar and screen still dismisses (Sequential — depends on 2.2 GREEN, since `posponerAlarma` no longer throws)
|
||||
- Satisfies: Spec "User-Facing Failure Feedback" / Scenario "Snooze fails and ringing screen dismisses".
|
||||
- File: `test/pantallas/pantalla_alarma_sonando_dismiss_guard_test.dart` (add to existing group, or new file `pantalla_alarma_sonando_failure_test.dart` if the existing group's `_buildEnv()` helper needs a failure-mode variant — prefer extending the existing file's helpers with an optional `fallaProgramar` param to avoid duplicating `_buildEnv`/`_Env`/`_montarConHistorial`).
|
||||
- Add test: build env with `android.fallaProgramar = true` (set after `guardarAlarma`, since the initial save must succeed for setup), mount with history (`_montarConHistorial`), tap the snooze button.
|
||||
- Assert: `find.byType(PantallaAlarmaSonando)` is gone (screen still dismissed — existing dismiss-by-design behavior preserved); a `SnackBar` widget is found with text content (use the existing `l10n` lookup; failure message comes from `estado.error`, which is the `androidExactAlarmScheduleError`-sourced string per Design's l10n decision).
|
||||
- Run: test MUST fail against current code (today `_posponer()` only `debugPrint`s on catch, no SnackBar is shown — also today's code DOES throw so this scenario currently relies on the try/catch in `_posponer()` itself, not on a SnackBar at all).
|
||||
|
||||
### 4.2 [x] [GREEN] Wire SnackBar feedback in `_posponer()` (Sequential — depends on 4.1)
|
||||
- Satisfies: Spec "User-Facing Failure Feedback"; Design D4 ("capture root/app-level ScaffoldMessenger BEFORE dismiss"), UI Wiring section.
|
||||
- File: `lib/pantallas/pantalla_alarma_sonando.dart`, method `_posponer()` (current L173-188).
|
||||
- Before the `await alarmas.posponerAlarma(...)` call, capture `final messenger = ScaffoldMessenger.of(context);` (BEFORE await, per Design D4, so it survives screen dismissal).
|
||||
- After the await completes (inside or after the existing try block — `posponerAlarma` no longer throws after 2.2, so the existing `catch (e)` becomes a dead/safety-net branch per Design "method no longer throws on native failure" — keep it as a defensive net per Design's explicit instruction, do not delete it), read `alarmas.error` (the `EstadoAlarmas` instance captured earlier as `alarmas`). If non-null, call `messenger.showSnackBar(SnackBar(content: Text(alarmas.error!)))`.
|
||||
- Update the stale comment at L177-180 (currently claims `posponerAlarma` can throw and that's why dismiss is in `finally`) to reflect the new reality: dismiss-in-finally is now a structural safety net, not a workaround for an expected throw; failure is now reported via `_error`/SnackBar, not via an exception.
|
||||
- Keep `if (mounted) _dismissScreen();` in `finally` unchanged (dismiss-by-design preserved, per spec scenario).
|
||||
- Run: 4.1 test MUST pass. Full `pantalla_alarma_sonando_dismiss_guard_test.dart` suite stays green (success-path snooze tests must show NO SnackBar — verify no new SnackBar finder conflicts with existing assertions).
|
||||
|
||||
### 4.3 [x] [RED] Success path shows no failure SnackBar (Sequential — depends on 4.2)
|
||||
- Satisfies: Spec Scenario "Snooze succeeds" (no failure message shown).
|
||||
- File: same test file as 4.1.
|
||||
- Add/confirm test: default env (`fallaProgramar` stays `false`), tap snooze, assert no `SnackBar` with the failure text is present (or `find.byType(SnackBar)` is absent, depending on whether other snackbars exist on this screen — check current widget tree first via the existing dismiss-guard tests, which assert no SnackBar today).
|
||||
- Should already be GREEN after 4.2 (only shows SnackBar when `alarmas.error != null`) — run to confirm; if it fails, fix the conditional in 4.2 before proceeding.
|
||||
|
||||
### 4.4 [x] [REFACTOR] Cleanup `_posponer()` (Sequential — depends on 4.2, 4.3 green)
|
||||
- Satisfies: code quality, no behavior change.
|
||||
- File: `lib/pantallas/pantalla_alarma_sonando.dart`.
|
||||
- Re-read full method, confirm `messenger` variable naming/placement is unambiguous, confirm comment clarity, confirm no duplicate `ScaffoldMessenger.of(context)` lookups remain.
|
||||
- Full suite stays green.
|
||||
|
||||
---
|
||||
|
||||
## Phase 5 — UI failure feedback: pre-notice `POSTPONE_NEXT` action in `app.dart`
|
||||
|
||||
### 5.1 [BLOCKED — see note] [RED] Failure branches to a failure SnackBar instead of the success message (Sequential — depends on 3.2 GREEN)
|
||||
- Satisfies: Spec Scenario "Pre-notice snooze failure (variant path)" combined with "User-Facing Failure Feedback".
|
||||
- File: new or existing widget test covering `app.dart`'s `POSTPONE_NEXT` handling (locate existing test coverage for this action first — search `test/` for `POSTPONE_NEXT` or `alarmPostponedCurrentExecution` before creating a new file, to extend rather than duplicate harness setup).
|
||||
- Add test: simulate the `POSTPONE_NEXT` native event with `android.fallaProgramar = true`, assert the resulting SnackBar shows `estado.error` text, NOT `alarmPostponedCurrentExecution`.
|
||||
- Run: test MUST fail against current code (today's code unconditionally shows the success SnackBar after the await, regardless of failure, since `posponerProximaDesdePreaviso` doesn't yet set `_error` until 3.2 — and even after 3.2, `app.dart` doesn't yet branch on it).
|
||||
- **APPLY NOTE (documented exception, not silently skipped)**: confirmed no existing test mounts `PluriWaveApp`/`_PaginaPrincipal`. Investigated building one: `PluriWaveApp.build()` hardcodes `EstadoAlarmas(prefs: prefs)` and `EstadoRadio(prefs:..., dispositivoAudio: ServicioDispositivoAudioReal())` with zero DI seam. Worse, `EstadoRadio`'s default `ServicioAudio()` asserts `registrarHandler()` (audio_service init from `main.dart`) was called — mounting `PluriWaveApp` in a widget test without full `AudioService.init()` + platform-channel mocking throws/asserts immediately. This is a pre-existing architectural gap in `app.dart`, not something this change's spec/design authorized fixing (no DI-seam task in scope). Building the required test infra (10+ MethodChannel mocks for `ServicioAlarmasAndroid`, plus `audio_service`/`just_audio` mocking) is disproportionate to this change's ~40-60 line estimate and risks brittle, unmaintainable scaffolding. Flagging for sdd-verify judgment.
|
||||
|
||||
### 5.2 [x] [GREEN] Branch on `estado.error` in `app.dart`'s `POSTPONE_NEXT` handler (Sequential — depends on 5.1)
|
||||
- Satisfies: Spec "User-Facing Failure Feedback"; Design "UI Wiring" section (app.dart POSTPONE_NEXT L291-311).
|
||||
- File: `lib/app.dart`, current L291-311.
|
||||
- After `await estado.posponerProximaDesdePreaviso(alarma, evento.snoozeMinutes, ejecucion);`, branch: if `estado.error != null`, show a failure SnackBar with that text; else keep the existing `alarmPostponedCurrentExecution` success SnackBar unchanged.
|
||||
- Keep `setState(() => _indice = 3);` and the early `if (!mounted) return;` guard unchanged — only the SnackBar content/branch changes.
|
||||
- Implemented directly (no preceding RED widget test per 5.1's documented exception) since the change is a mechanical mirror of the already fully-tested Phase 4 SnackBar branch pattern. `flutter analyze` clean, full `flutter test` suite (238 tests) green, no regression.
|
||||
|
||||
### 5.3 [x] [REFACTOR] Cleanup `app.dart` POSTPONE_NEXT handler (Sequential — depends on 5.2 green)
|
||||
- Satisfies: code quality, no behavior change.
|
||||
- File: `lib/app.dart`.
|
||||
- Confirm branch readability, confirm no duplicated `ScaffoldMessenger.of(context)` calls, confirm comment if needed to explain the error/success branch.
|
||||
- Full suite stays green.
|
||||
|
||||
---
|
||||
|
||||
## Phase 6 — Full regression sweep (Sequential — depends on all of Phase 2-5 green)
|
||||
|
||||
### 6.1 [x] Run full `flutter test` suite
|
||||
- Satisfies: Proposal Success Criteria — "Dart regression tests cover success+failure for both paths, pass under flutter test".
|
||||
- Run the entire test suite (not just touched files) to catch any cross-file regression (e.g. other tests relying on `posponerAlarma`/`posponerProximaDesdePreaviso` throwing, or on the old `_posponer()` catch/debugPrint behavior).
|
||||
- Fix any incidental breakage found; do not skip or weaken unrelated tests to make this pass.
|
||||
- Result: 238 tests passed, 0 failures. `flutter analyze`: No issues found.
|
||||
|
||||
### 6.2 [x] Manual/device verification note (optional, non-blocking)
|
||||
- Satisfies: Proposal Risks R1/R2 — root cause is permission-revocation, confirmable via device logcat.
|
||||
- Not a coded task — record in the PR description that device-level confirmation (adb logcat tag PluriWave during a real snooze) is recommended but not required to merge, since the fix targets the confirmed structural defect regardless of root cause confirmation.
|
||||
|
||||
---
|
||||
|
||||
## Review Workload Forecast
|
||||
|
||||
- Files touched: `lib/estado/estado_alarmas.dart` (~30-40 changed lines across two methods), `lib/pantallas/pantalla_alarma_sonando.dart` (~15-20 changed lines), `lib/app.dart` (~10-15 changed lines), `test/helpers/fakes_alarmas.dart` (~5 lines), `test/estado/estado_alarmas_snooze_test.dart` (~80-100 new lines, 4 new tests), `test/pantallas/pantalla_alarma_sonando_dismiss_guard_test.dart` or new file (~40-60 new lines, 2 new tests), possible new/extended `app.dart` widget test (~40-60 new lines).
|
||||
- Estimated total changed/added lines: roughly 220-300 lines (well under the 400-line single-PR budget).
|
||||
- Chained PRs recommended: No — single PR is appropriate; change is Dart-only, narrowly scoped, fully covered by Strict TDD, no Kotlin/native edits, no schema or persisted-state changes (per Proposal Rollback Plan).
|
||||
- 400-line budget risk: Low.
|
||||
- Decision needed before apply: No — proceed as a single PR under `delivery_strategy: ask-on-risk` without triggering the chained-PR conversation, since none of the risk thresholds are met.
|
||||
- Ownership/dependency note: Phase 1 (1.1) is a hard sequential blocker for every RED task in Phases 2-5 — it must land first and alone. Within Phase 2 and Phase 3, tasks are strictly sequential (RED before GREEN before REFACTOR). Phase 2 and Phase 3 touch the same file (`estado_alarmas.dart`) but different methods — a single implementer should do both phases serially to avoid merge conflicts inside one file; do not parallelize across two people without coordinating hunks. Phase 4 depends on Phase 2's GREEN state (posponerAlarma no longer throwing) and Phase 5 depends on Phase 3's GREEN state — both UI phases can proceed in parallel with each other (different files: `pantalla_alarma_sonando.dart` vs `app.dart`) once their respective state-layer phase is green.
|
||||
Reference in New Issue
Block a user