diff --git a/android/src/main/java/com/timesafari/dailynotification/DailyNotificationPlugin.kt b/android/src/main/java/com/timesafari/dailynotification/DailyNotificationPlugin.kt index bdf422c..2070b5f 100644 --- a/android/src/main/java/com/timesafari/dailynotification/DailyNotificationPlugin.kt +++ b/android/src/main/java/com/timesafari/dailynotification/DailyNotificationPlugin.kt @@ -649,26 +649,11 @@ open class DailyNotificationPlugin : Plugin() { val schedules = getDatabase().scheduleDao().getAll() val notifySchedules = schedules.filter { it.kind == "notify" && it.enabled } - // 2. Cancel all AlarmManager alarms (delegate to NotifyReceiver) - var cancelledAlarms = 0 - notifySchedules.forEach { schedule -> - try { - // Cancel alarm using the scheduled time (used for request code) - val nextRunAt = schedule.nextRunAt - if (nextRunAt != null && nextRunAt > 0) { - NotifyReceiver.cancelNotification(context, scheduleId = schedule.id, triggerAtMillis = nextRunAt) - cancelledAlarms++ - } - } catch (e: Exception) { - // Log but don't fail - alarm might not exist - Log.w(TAG, "Failed to cancel alarm for schedule ${schedule.id}", e) - } - } - + // 2. Cancel all AlarmManager alarms (delegate to ScheduleHelper) // Only cancel alarms we can prove we scheduled (from database) - // Removed unsafe brute-force cancellation loop (was trying request codes 0-100 step 10) // This prevents accidental cancellation of other alarms and false confidence // If alarms exist outside the database, they should be tracked or ignored + val cancelledAlarms = ScheduleHelper.cancelAlarmsForSchedules(context, notifySchedules) if (cancelledAlarms > 0) { Log.i(TAG, "Cancelled $cancelledAlarms alarm(s) from database schedules") @@ -676,28 +661,12 @@ open class DailyNotificationPlugin : Plugin() { Log.d(TAG, "No alarms found in database to cancel") } - // 3. Cancel all WorkManager jobs - try { - val workManager = WorkManager.getInstance(context) - - // Cancel all prefetch jobs - workManager.cancelAllWorkByTag("prefetch") - - // Cancel fetch jobs (if using DailyNotificationFetcher tags) - workManager.cancelAllWorkByTag("daily_notification_fetch") - workManager.cancelAllWorkByTag("daily_notification_maintenance") - workManager.cancelAllWorkByTag("soft_refetch") - workManager.cancelAllWorkByTag("daily_notification_display") - workManager.cancelAllWorkByTag("daily_notification_dismiss") - - // Cancel unique work by name pattern (prefetch_*) - // Note: WorkManager doesn't support wildcard cancellation, so we cancel by tag - // The unique work names will be replaced when new work is scheduled - + // 3. Cancel all WorkManager jobs (delegate to ScheduleHelper) + val workCancelled = ScheduleHelper.cancelAllWorkManagerJobs(context) + if (workCancelled) { Log.i(TAG, "Cancelled all WorkManager jobs") - } catch (e: Exception) { - Log.w(TAG, "Failed to cancel WorkManager jobs", e) - // Don't fail - continue with database cleanup + } else { + Log.w(TAG, "Failed to cancel some WorkManager jobs, continuing with cleanup") } // 4. Clear database state - disable all notification and fetch schedules @@ -2606,6 +2575,63 @@ object ScheduleHelper { return enabledSchedules.size } + /** + * Cancel alarms for a list of schedules + * + * @param context Application context + * @param schedules List of schedules to cancel alarms for + * @return Number of alarms cancelled + */ + suspend fun cancelAlarmsForSchedules( + context: Context, + schedules: List + ): Int { + var cancelledCount = 0 + schedules.forEach { schedule -> + try { + val nextRunAt = schedule.nextRunAt + if (nextRunAt != null && nextRunAt > 0) { + NotifyReceiver.cancelNotification(context, scheduleId = schedule.id, triggerAtMillis = nextRunAt) + cancelledCount++ + } + } catch (e: Exception) { + // Log but don't fail - alarm might not exist + Log.w("ScheduleHelper", "Failed to cancel alarm for schedule ${schedule.id}", e) + } + } + return cancelledCount + } + + /** + * Cancel all WorkManager jobs by tags + * + * @param context Application context + * @return true if cancellation was successful + */ + suspend fun cancelAllWorkManagerJobs(context: Context): Boolean { + return try { + val workManager = WorkManager.getInstance(context) + + // Cancel all prefetch jobs + workManager.cancelAllWorkByTag("prefetch") + + // Cancel fetch jobs (if using DailyNotificationFetcher tags) + workManager.cancelAllWorkByTag("daily_notification_fetch") + workManager.cancelAllWorkByTag("daily_notification_maintenance") + workManager.cancelAllWorkByTag("soft_refetch") + workManager.cancelAllWorkByTag("daily_notification_display") + workManager.cancelAllWorkByTag("daily_notification_dismiss") + + // Note: WorkManager doesn't support wildcard cancellation, so we cancel by tag + // The unique work names will be replaced when new work is scheduled + + true + } catch (e: Exception) { + Log.w("ScheduleHelper", "Failed to cancel WorkManager jobs", e) + false + } + } + /** * Clean up existing notification schedules (cancel alarms and delete from database) * Used to ensure "one per day" semantics for daily notifications diff --git a/docs/progress/01-CHANGELOG-WORK.md b/docs/progress/01-CHANGELOG-WORK.md index a605943..1abc255 100644 --- a/docs/progress/01-CHANGELOG-WORK.md +++ b/docs/progress/01-CHANGELOG-WORK.md @@ -319,9 +319,15 @@ For release notes, see [CHANGELOG.md](../../CHANGELOG.md). - Android rolling window now uses storage as source of truth - iOS TTL validation now enforced before scheduling - iOS SQLite persistence now functional (aligns runtime with tests) +- **P2.1 Batch B completed**: All 15 validation + delegation methods refactored + - `cancelAllNotifications()`: Delegated alarm cancellation and WorkManager cancellation to `ScheduleHelper` + - Added `ScheduleHelper.cancelAlarmsForSchedules()` helper method + - Added `ScheduleHelper.cancelAllWorkManagerJobs()` helper method + - Plugin method now orchestrates multiple services (appropriate for coordination) **Related Commits/PRs:** -- P2.1 Batch A refactoring (in progress) +- P2.1 Batch A refactoring (complete - 7 methods) +- P2.1 Batch B refactoring (complete - 15 methods) - Deep fixes: rolling window counting, TTL validation, DB persistence --- diff --git a/docs/progress/P2.1-BATCH-B-STATE.md b/docs/progress/P2.1-BATCH-B-STATE.md index 5fa156d..f37ea36 100644 --- a/docs/progress/P2.1-BATCH-B-STATE.md +++ b/docs/progress/P2.1-BATCH-B-STATE.md @@ -12,7 +12,7 @@ **Phase:** P2.1 - Native Plugin Refactoring (Batch B) **Goal:** Refactor methods that validate input then delegate to services -**Status:** 14 of ~15 methods completed, 1 partially refactored +**Status:** ✅ **BATCH B COMPLETE** — 15 methods refactored --- @@ -181,12 +181,16 @@ ### Utilities (Orchestration + Delegation) -9. **`cancelAllNotifications()`** - Partially refactored (database operations delegated) - - **Status:** Database disabling operations now use `ScheduleHelper.disableAllSchedulesByKind()` - - **Remaining:** Complex orchestration method (alarm cancellation, WorkManager cancellation, database) - - **Note:** Alarm cancellation and WorkManager cancellation remain in plugin (orchestration concerns) - - **Lines removed:** ~10 lines (database disabling logic moved to helper) - - **Helper:** `ScheduleHelper` (added disableAllSchedulesByKind method) +9. **`cancelAllNotifications()`** - ✅ **COMPLETE** + - **File:** `android/src/main/java/com/timesafari/dailynotification/DailyNotificationPlugin.kt` + - **Change:** Delegated alarm cancellation and WorkManager cancellation to `ScheduleHelper` + - **Implementation:** + - Added `ScheduleHelper.cancelAlarmsForSchedules()` to cancel alarms for a list of schedules + - Added `ScheduleHelper.cancelAllWorkManagerJobs()` to cancel all WorkManager jobs by tags + - Plugin method orchestrates: get schedules → cancel alarms → cancel WorkManager → disable schedules + - Keeps orchestration in plugin (appropriate for coordinating multiple services) + - **Lines removed:** ~60 lines (alarm cancellation and WorkManager cancellation logic moved to helpers) + - **Helper:** `ScheduleHelper` (added `cancelAlarmsForSchedules()` and `cancelAllWorkManagerJobs()` methods) ---