From a9da85560151d1594c83cec395b4f8b0711f4d69 Mon Sep 17 00:00:00 2001 From: freetlab Date: Mon, 3 Aug 2026 21:59:19 +0200 Subject: [PATCH] fix(alarmas): stop Detener from consuming an occurrence that never rang Reported: an alarm set for Monday 16:20 never rang, and the "next alarm" banner showed a different alarm (the next morning's) instead. No vacation range involved, both alarms active. finalizarEjecucion anchored the completed occurrence to proximaEjecucion with no check that it was the one actually ringing. On the native-fire path the fire-time sync advances proximaEjecucion to the NEXT occurrence before the user can reach the ring screen, so tapping Detener recorded a FUTURE occurrence in ultimaEjecucionGestionada. ServicioProgramacionAlarmas._esValida then rejects that occurrence for real: a Monday-only alarm stopped today simply never rings next Monday, and every sibling outranks it in the banner because its own proximaEjecucion is a week out. Reproduced at its purest in the second test: with nothing ringing at 09:01 on Monday, Detener pushed a 16:20 alarm to the FOLLOWING Monday. posponerAlarma already had exactly this guard -- 9c7cf4e, "anchor snooze to the ringing occurrence, never a future one", written after the same failure showed up as a snooze armed a day out. It was applied to the snooze path and never to the stop path, which sat ten lines below it in the same file with the identical hazard. Both paths now share one _ocurrenciaSonando helper so they cannot drift apart again, and the reason lives in its doc comment rather than in a comment on one of the two callers. Also drops snoozeHasta from the stop path's candidate chain: a pending snooze target is in the future by definition, and snoozeOrigen already covers a ring that follows a snooze. Tests: 1120 -> 1122. --- lib/estado/estado_alarmas.dart | 60 +++++--- .../estado_alarmas_detener_ancla_test.dart | 130 ++++++++++++++++++ 2 files changed, 169 insertions(+), 21 deletions(-) create mode 100644 test/estado/estado_alarmas_detener_ancla_test.dart diff --git a/lib/estado/estado_alarmas.dart b/lib/estado/estado_alarmas.dart index 77937d0..c44f327 100644 --- a/lib/estado/estado_alarmas.dart +++ b/lib/estado/estado_alarmas.dart @@ -316,29 +316,47 @@ class EstadoAlarmas extends ChangeNotifier { notifyListeners(); } - Future posponerAlarma(AlarmaMusical alarma, int minutos) async { - _error = null; - // The snooze anchors to the occurrence that is RINGING — never a future - // one. When the native fire works, the fire-time sync advances - // proximaEjecucion to the NEXT day before the user can even tap snooze, - // so anchoring to proximaEjecucion re-armed "posponer 3" a full day out - // (observed on-device: snooze armed for tomorrow 23:02). The ringing - // occurrence is the newest candidate not meaningfully in the future: - // snoozeOrigen (a re-snooze keeps the original anchor), then - // proximaEjecucion (watchdog path: still today's just-due occurrence), - // then ultimaEjecucionGestionada (native-fire path: the sync recorded - // the ringing occurrence there), then now. + /// The occurrence that is ACTUALLY ringing right now — the anchor both + /// ring-screen actions (Posponer and Detener) must close. + /// + /// It is NEVER a future occurrence. When the native fire works, the + /// fire-time sync advances `proximaEjecucion` to the next one before the + /// user can even reach the ring screen, so taking `proximaEjecucion` + /// unguarded closes an occurrence that has not happened yet. For snooze + /// that showed up as "posponer 3" arming a full day out (observed + /// on-device: tomorrow 23:02). For stop it was worse and silent: the + /// future occurrence was recorded in `ultimaEjecucionGestionada`, which + /// `ServicioProgramacionAlarmas._esValida` then rejects for real — so a + /// Monday-only alarm stopped today simply never rang next Monday, and + /// every sibling alarm outranked it in the "next alarm" banner. + /// + /// The candidates, newest first, each gated on "not meaningfully in the + /// future": [AlarmaMusical.snoozeOrigen] (a re-snooze keeps the original + /// anchor), then [AlarmaMusical.proximaEjecucion] (watchdog path: still + /// today's just-due occurrence), then + /// [AlarmaMusical.ultimaEjecucionGestionada] (native-fire path: the sync + /// recorded the ringing occurrence there), then now. + /// + /// ONE helper for BOTH callers on purpose. This guard was written for + /// `posponerAlarma` alone (`9c7cf4e`) while `finalizarEjecucion` sat ten + /// lines below with the identical hazard and no guard, and it stayed that + /// way until a user lost a whole week of alarms. Do not re-inline it. + DateTime _ocurrenciaSonando(AlarmaMusical? alarma) { final ahora = servicio.ahora(); final limite = ahora.add( ServicioProgramacionAlarmas.toleranciaDisparoInminente, ); DateTime? sonando(DateTime? candidata) => candidata != null && !candidata.isAfter(limite) ? candidata : null; - final ejecucion = - sonando(alarma.snoozeOrigen) ?? - sonando(alarma.proximaEjecucion) ?? - sonando(alarma.ultimaEjecucionGestionada) ?? + return sonando(alarma?.snoozeOrigen) ?? + sonando(alarma?.proximaEjecucion) ?? + sonando(alarma?.ultimaEjecucionGestionada) ?? ahora; + } + + Future posponerAlarma(AlarmaMusical alarma, int minutos) async { + _error = null; + final ejecucion = _ocurrenciaSonando(alarma); debugPrint( '[PluriWave][alarmas] posponer id=${alarma.id} minutos=$minutos ejecucion=${ejecucion.toIso8601String()}', ); @@ -401,11 +419,11 @@ class EstadoAlarmas extends ChangeNotifier { debugPrint('[PluriWave][alarmas] finalizar ejecucion id=$alarmaId'); _error = null; final alarma = _buscarAlarma(alarmaId); - final ejecucion = - alarma?.snoozeOrigen ?? - alarma?.proximaEjecucion ?? - alarma?.snoozeHasta ?? - DateTime.now(); + // Same anchor as posponerAlarma, through the same helper: closing a + // future occurrence here marks it handled, and _esValida then skips it + // for real -- the alarm silently never rings that day. See + // [_ocurrenciaSonando]. + final ejecucion = _ocurrenciaSonando(alarma); await android.ocultarNotificacionAlarma(alarmaId); // Stop/Snooze Result Verification (SS-2a/SS-2b): the Stop path calls the // id-agnostic fail-safe stop directly (it always targets whatever is diff --git a/test/estado/estado_alarmas_detener_ancla_test.dart b/test/estado/estado_alarmas_detener_ancla_test.dart new file mode 100644 index 0000000..618cf34 --- /dev/null +++ b/test/estado/estado_alarmas_detener_ancla_test.dart @@ -0,0 +1,130 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:pluriwave/estado/estado_alarmas.dart'; +import 'package:pluriwave/modelos/alarma_musical.dart'; +import 'package:pluriwave/servicios/servicio_alarmas.dart'; +import 'package:pluriwave/servicios/servicio_alarmas_android.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +import '../helpers/fakes_alarmas.dart'; + +/// Reported on-device: an alarm set for Monday 16:20 never rang, and the +/// "next alarm" banner showed a DIFFERENT alarm (the next morning's) instead. +/// +/// Root cause: `finalizarEjecucion` ("Detener") anchored the completed +/// occurrence to `proximaEjecucion` with no check that it was the one +/// actually ringing. On the native-fire path the fire-time sync advances +/// `proximaEjecucion` to the NEXT occurrence BEFORE the user can reach the +/// ring screen — so stopping today's ring recorded NEXT week's occurrence as +/// already handled. `ServicioProgramacionAlarmas._esValida` then rejected +/// that occurrence for real, and the alarm silently jumped past it: it never +/// rang, and every sibling alarm outranked it in the banner. +/// +/// This is the exact hazard `posponerAlarma` was fixed for in `9c7cf4e` +/// ("anchor snooze to the ringing occurrence, never a future one"). The guard +/// landed on the snooze path and never on the stop path, which sits directly +/// below it in the same file. +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + setUp(() { + SharedPreferences.setMockInitialValues({}); + }); + + AlarmaMusical semanalLunes(String id) => AlarmaMusical( + id: id, + nombre: 'Tarde del lunes', + hora: 16, + minuto: 20, + tipoProgramacion: TipoProgramacionAlarma.diasSemana, + diasSemana: const [DateTime.monday], + ); + + test('Detener cierra la ocurrencia que sonaba, no quema la siguiente ' + '(el nativo ya avanzó proximaEjecucion antes de que el usuario ' + 'llegue a la pantalla)', () async { + // Monday 2026-08-03. + var ahora = DateTime(2026, 8, 3, 16, 0); + final android = FakePuertoAlarmasAndroid(); + final estado = EstadoAlarmas( + servicio: ServicioAlarmas(reloj: () => ahora), + android: android, + iniciarAutomaticamente: false, + ); + addTearDown(estado.dispose); + addTearDown(android.dispose); + + await estado.guardarAlarma(semanalLunes('a1')); + expect( + estado.alarmas.single.proximaEjecucion, + DateTime(2026, 8, 3, 16, 20), + ); + + // 16:20 — it rings. The native scheduler records the occurrence handled + // and rearms; the cold-start sync brings that over, which advances + // proximaEjecucion to NEXT Monday while the alarm is still ringing. + ahora = DateTime(2026, 8, 3, 16, 20, 5); + android.ejecucionesNativas.add( + EjecucionAlarmaNativa( + alarmaId: 'a1', + gestionadaEn: DateTime(2026, 8, 3, 16, 20), + ), + ); + await estado.inicializar(); + + expect( + estado.alarmas.single.proximaEjecucion, + DateTime(2026, 8, 10, 16, 20), + reason: 'precondición: el nativo ya avanzó a la semana siguiente', + ); + + // NOW the user taps "Detener" on the ring screen. + await estado.finalizarEjecucion('a1'); + + expect( + estado.alarmas.single.proximaEjecucion, + DateTime(2026, 8, 10, 16, 20), + reason: + 'Detener debe cerrar la ocurrencia que sonaba (hoy), no consumir ' + 'la del lunes que viene empujándola a 2026-08-17', + ); + expect( + estado.alarmas.single.ultimaEjecucionGestionada, + isNot(DateTime(2026, 8, 10, 16, 20)), + reason: + 'marcar como gestionada una ocurrencia futura es justo lo que hace ' + 'que _esValida la rechace y esa alarma no suene ese día', + ); + }); + + test( + 'Detener sin nada sonando tampoco consume la próxima ocurrencia', + () async { + // Defensive: the ring screen is the only production caller, but a stale + // route or a duplicated stop event must not silently eat a day. + var ahora = DateTime(2026, 8, 3, 9, 0); + final android = FakePuertoAlarmasAndroid(); + final estado = EstadoAlarmas( + servicio: ServicioAlarmas(reloj: () => ahora), + android: android, + iniciarAutomaticamente: false, + ); + addTearDown(estado.dispose); + addTearDown(android.dispose); + + await estado.guardarAlarma(semanalLunes('a2')); + expect( + estado.alarmas.single.proximaEjecucion, + DateTime(2026, 8, 3, 16, 20), + ); + + ahora = DateTime(2026, 8, 3, 9, 1); + await estado.finalizarEjecucion('a2'); + + expect( + estado.alarmas.single.proximaEjecucion, + DateTime(2026, 8, 3, 16, 20), + reason: 'a las 09:01 la ocurrencia de las 16:20 no está sonando', + ); + }, + ); +}