From cb3cb5a78eb2a76cd0b006e5fc8fafa715a91629 Mon Sep 17 00:00:00 2001 From: Jose Olarte III Date: Mon, 16 Feb 2026 16:57:01 +0800 Subject: [PATCH] fix(android): reset alarm and static reminder rollover; add cancelDailyReminder Fixes two integration bugs with the consuming app (Time Safari) and adds Android parity for cancel-by-id. Problem: - Re-setting a daily notification (edit/save same time) could cancel the alarm then skip re-scheduling because DB idempotence still ran and treated the update as a duplicate. - After the first fire, rollover scheduled the next run with isStaticReminder=false, so title/body reverted to fallback. - App calls cancelDailyReminder({ reminderId }) but Android had no implementation (only cancelAllNotifications and scheduleDailyReminder). Changes: - NotifyReceiver.kt: Run DB idempotence only when !skipPendingIntentIdempotence. When true (e.g. app reset flow), skip the check and log; prevents "no alarm" after cancel-then-schedule. - DailyNotificationWorker.java: In scheduleNextNotification(), read is_static_reminder from WorkManager input; keep stable scheduleId for static reminders; pass preserveStaticReminder and reminderId into scheduleExactNotification(); add DN|ROLLOVER log. - DailyNotificationPlugin.kt: Add cancelDailyReminder(call) that parses reminderId (or id, reminder_id, scheduleId), calls NotifyReceiver.cancelNotification(context, scheduleId), and does best-effort DB cleanup (setEnabled false, updateRunTimes null). Files modified: - android/.../NotifyReceiver.kt - android/.../DailyNotificationWorker.java - android/.../DailyNotificationPlugin.kt --- .../DailyNotificationPlugin.kt | 28 +++++ .../DailyNotificationWorker.java | 19 +-- .../dailynotification/NotifyReceiver.kt | 39 ++++--- docs/ACTION_PLAN_INTEGRATION_FIXES.md | 108 ++++++++++++++++++ 4 files changed, 170 insertions(+), 24 deletions(-) create mode 100644 docs/ACTION_PLAN_INTEGRATION_FIXES.md diff --git a/android/src/main/java/com/timesafari/dailynotification/DailyNotificationPlugin.kt b/android/src/main/java/com/timesafari/dailynotification/DailyNotificationPlugin.kt index 00e9e8d..604c7c0 100644 --- a/android/src/main/java/com/timesafari/dailynotification/DailyNotificationPlugin.kt +++ b/android/src/main/java/com/timesafari/dailynotification/DailyNotificationPlugin.kt @@ -706,6 +706,34 @@ open class DailyNotificationPlugin : Plugin() { scheduleDailyNotification(call) } + @PluginMethod + fun cancelDailyReminder(call: PluginCall) { + try { + val reminderId = call.getString("reminderId") + ?: call.getString("id") + ?: call.getString("reminder_id") + ?: call.getString("scheduleId") + if (reminderId.isNullOrBlank()) { + call.reject("cancelDailyReminder: missing reminderId") + return + } + NotifyReceiver.cancelNotification(context, scheduleId = reminderId) + try { + kotlinx.coroutines.runBlocking { + val db = getDatabase() + db.scheduleDao().setEnabled(reminderId, false) + db.scheduleDao().updateRunTimes(reminderId, null, null) + } + } catch (dbErr: Exception) { + Log.w(TAG, "cancelDailyReminder: failed DB update for $reminderId", dbErr) + } + call.resolve() + } catch (e: Exception) { + Log.e(TAG, "cancelDailyReminder failed", e) + call.reject("cancelDailyReminder failed: ${e.message}") + } + } + /** * Check if exact alarms can be scheduled * Helper method for internal use diff --git a/android/src/main/java/com/timesafari/dailynotification/DailyNotificationWorker.java b/android/src/main/java/com/timesafari/dailynotification/DailyNotificationWorker.java index a5ed1d8..ff78256 100644 --- a/android/src/main/java/com/timesafari/dailynotification/DailyNotificationWorker.java +++ b/android/src/main/java/com/timesafari/dailynotification/DailyNotificationWorker.java @@ -540,15 +540,19 @@ public class DailyNotificationWorker extends Worker { return; } + // Preserve static reminder semantics across rollover so title/body don't revert to fallback + Data inputData = getInputData(); + boolean preserveStaticReminder = inputData.getBoolean("is_static_reminder", false); + // Extract scheduleId from notificationId pattern or use fallback - // Notification IDs are often "daily_${scheduleId}" + // For static reminders, keep stable scheduleId across days String scheduleId = null; String cronExpression = null; - - // Try to extract scheduleId from notificationId (e.g., "daily_1764578136269") String notificationId = content.getId(); - if (notificationId != null && notificationId.startsWith("daily_")) { - scheduleId = notificationId; // Use notificationId as scheduleId + if (preserveStaticReminder && notificationId != null && !notificationId.isEmpty()) { + scheduleId = notificationId; + } else if (notificationId != null && notificationId.startsWith("daily_")) { + scheduleId = notificationId; } else { scheduleId = "daily_rollover_" + System.currentTimeMillis(); } @@ -581,12 +585,13 @@ public class DailyNotificationWorker extends Worker { ); // Use centralized scheduling function with ROLLOVER_ON_FIRE source + Log.d(TAG, "DN|ROLLOVER next=" + nextScheduledTime + " scheduleId=" + scheduleId + " static=" + preserveStaticReminder); com.timesafari.dailynotification.NotifyReceiver.scheduleExactNotification( getApplicationContext(), nextScheduledTime, config, - false, // isStaticReminder - null, // reminderId + preserveStaticReminder, // isStaticReminder – preserve so next run keeps title/body + preserveStaticReminder ? scheduleId : null, // reminderId scheduleId, com.timesafari.dailynotification.ScheduleSource.ROLLOVER_ON_FIRE, false // skipPendingIntentIdempotence – rollover path does not skip diff --git a/android/src/main/java/com/timesafari/dailynotification/NotifyReceiver.kt b/android/src/main/java/com/timesafari/dailynotification/NotifyReceiver.kt index 953f0b6..665d905 100644 --- a/android/src/main/java/com/timesafari/dailynotification/NotifyReceiver.kt +++ b/android/src/main/java/com/timesafari/dailynotification/NotifyReceiver.kt @@ -204,26 +204,31 @@ class NotifyReceiver : BroadcastReceiver() { } // DB-LEVEL IDEMPOTENCE CHECK: Verify no existing schedule for this scheduleId and nextRun - // This prevents logical duplicates before even hitting AlarmManager - try { - runBlocking { - val db = DailyNotificationDatabase.getDatabase(context) - val existingSchedule = db.scheduleDao().getById(stableScheduleId) - - if (existingSchedule != null && existingSchedule.nextRunAt != null) { - val timeDiff = Math.abs(existingSchedule.nextRunAt - triggerAtMillis) - // If we already have a schedule for this ID with the same nextRun (within 1 minute), skip - if (timeDiff < 60000) { - val triggerTimeStr = java.text.SimpleDateFormat("yyyy-MM-dd HH:mm:ss", java.util.Locale.US) - .format(java.util.Date(triggerAtMillis)) - Log.w(SCHEDULE_TAG, "Skipping duplicate schedule for id=$stableScheduleId at $triggerTimeStr from source=$source") - Log.w(SCHEDULE_TAG, "Existing schedule found in DB: nextRunAt=${existingSchedule.nextRunAt}, diff=${timeDiff}ms") - return@runBlocking + // When skipPendingIntentIdempotence is true (e.g. "re-set" flow), skip this check so we don't + // cancel the alarm and then skip re-scheduling, resulting in no alarm. + if (!skipPendingIntentIdempotence) { + try { + runBlocking { + val db = DailyNotificationDatabase.getDatabase(context) + val existingSchedule = db.scheduleDao().getById(stableScheduleId) + + if (existingSchedule != null && existingSchedule.nextRunAt != null) { + val timeDiff = Math.abs(existingSchedule.nextRunAt - triggerAtMillis) + // If we already have a schedule for this ID with the same nextRun (within 1 minute), skip + if (timeDiff < 60000) { + val triggerTimeStr = java.text.SimpleDateFormat("yyyy-MM-dd HH:mm:ss", java.util.Locale.US) + .format(java.util.Date(triggerAtMillis)) + Log.w(SCHEDULE_TAG, "Skipping duplicate schedule for id=$stableScheduleId at $triggerTimeStr from source=$source") + Log.w(SCHEDULE_TAG, "Existing schedule found in DB: nextRunAt=${existingSchedule.nextRunAt}, diff=${timeDiff}ms") + return@runBlocking + } } } + } catch (e: Exception) { + Log.w(SCHEDULE_TAG, "DB idempotence check failed, continuing with schedule: $stableScheduleId", e) } - } catch (e: Exception) { - Log.w(SCHEDULE_TAG, "DB idempotence check failed, continuing with schedule: $stableScheduleId", e) + } else { + Log.d(SCHEDULE_TAG, "Skipping DB idempotence (skipPendingIntentIdempotence=true) for scheduleId=$stableScheduleId") } val triggerTimeStr = java.text.SimpleDateFormat("yyyy-MM-dd HH:mm:ss", java.util.Locale.US) diff --git a/docs/ACTION_PLAN_INTEGRATION_FIXES.md b/docs/ACTION_PLAN_INTEGRATION_FIXES.md new file mode 100644 index 0000000..3e10524 --- /dev/null +++ b/docs/ACTION_PLAN_INTEGRATION_FIXES.md @@ -0,0 +1,108 @@ +# Action Plan: Plugin + Consuming App Integration Fixes + +**Source:** Comparison output from Cursor session (daily-notification-plugin ↔ Time Safari / crowd-funder-for-time-pwa). +**Bugs addressed:** (A) Re-setting a notification doesn't fire; (B) Notification text always defaults to fallback values. + +--- + +## Objective + +Implement plugin-side and app-side changes so that: +1. **Reset works:** Editing/re-saving a daily reminder (even with the same time) reliably re-schedules and the alarm fires. +2. **Text persists:** Custom title/body persist across the first fire and rollover (next day); no silent fallback to generic text. +3. **Cancel works on Android:** App can call `cancelDailyReminder({ reminderId })` and the plugin performs per-id cancellation (parity with iOS). + +--- + +## Plugin-Side Implementation (this repo) + +### 1. Bug A: Skip DB idempotence when caller requests reset + +**File:** `android/src/main/java/com/timesafari/dailynotification/NotifyReceiver.kt` + +**Problem:** `scheduleExactNotification()` already skips *PendingIntent* idempotence when `skipPendingIntentIdempotence=true`, but the **DB-level idempotence check** (lines ~206–226) still runs. On "re-set same time," the DB still has the same `nextRunAt`, so the check returns early and **no alarm is scheduled**. + +**Change:** Wrap the entire DB idempotence block so it runs only when `!skipPendingIntentIdempotence`. When `skipPendingIntentIdempotence=true`, log and skip the DB check. + +- **Locate:** The block starting with `// DB-LEVEL IDEMPOTENCE CHECK` that loads `existingSchedule` and compares `existingSchedule.nextRunAt` with `triggerAtMillis` (60s tolerance), and `return@runBlocking` on duplicate. +- **Wrap:** Put that block inside `if (!skipPendingIntentIdempotence) { ... }` and add an `else` that logs: + `"Skipping DB idempotence (skipPendingIntentIdempotence=true) for scheduleId=$stableScheduleId"`. + +**Verification:** After editing a reminder without changing time, logs should show both "Skipping PendingIntent idempotence..." and "Skipping DB idempotence (skipPendingIntentIdempotence=true)...", and the alarm should fire. + +--- + +### 2. Bug B: Preserve static reminder on rollover + +**File:** `android/src/main/java/com/timesafari/dailynotification/DailyNotificationWorker.java` + +**Problem:** In `scheduleNextNotification()`, the call to `NotifyReceiver.scheduleExactNotification()` uses **hardcoded** `false` for `isStaticReminder` and `null` for `reminderId`. So the *next* occurrence is treated as non-static and content is loaded from storage/default → fallback text. + +**Change:** +1. At the start of `scheduleNextNotification()`, read from WorkManager input: + `boolean preserveStaticReminder = getInputData().getBoolean("is_static_reminder", false);` +2. When choosing `scheduleId`: if `preserveStaticReminder && notificationId != null && !notificationId.isEmpty()`, set `scheduleId = notificationId`. Otherwise keep existing logic (`daily_*` → use as scheduleId, else `daily_rollover_` + timestamp). +3. Replace the existing `scheduleExactNotification(...)` call with: + - `isStaticReminder` = `preserveStaticReminder` + - `reminderId` = `preserveStaticReminder ? scheduleId : null` + - `scheduleId` = the chosen `scheduleId` (stable for static reminders). +4. (Optional but useful) Add log before scheduling: + `Log.d("DN|ROLLOVER", "next=" + nextScheduledTime + " scheduleId=" + scheduleId + " static=" + preserveStaticReminder);` + +**Verification:** Set a custom title/body, let it fire once, then confirm the next scheduled run still uses the same text; logs should show `DN|ROLLOVER ... scheduleId=daily_timesafari_reminder static=true`. + +--- + +### 3. Integration: Add Android `cancelDailyReminder` + +**File:** `android/src/main/java/com/timesafari/dailynotification/DailyNotificationPlugin.kt` + +**Problem:** The app calls `DailyNotification.cancelDailyReminder({ reminderId })`. iOS implements this; Android only has `cancelAllNotifications()` and `scheduleDailyReminder()` alias. On Android the call fails (method missing / not implemented), so "turn off" and "reset" flows cannot rely on explicit cancel. + +**Change:** Add a new `@PluginMethod fun cancelDailyReminder(call: PluginCall)` (e.g. immediately after `scheduleDailyReminder()`). + +- **Parse ID:** `reminderId = call.getString("reminderId") ?: call.getString("id") ?: call.getString("reminder_id") ?: call.getString("scheduleId")`. Reject if null/blank. +- **Cancel alarm:** `NotifyReceiver.cancelNotification(context, scheduleId = reminderId)`. +- **DB cleanup (best-effort):** In a try/catch, `runBlocking`: + - `db = getDatabase()` (or `DailyNotificationDatabase.getDatabase(context)` as used elsewhere in plugin). + - `db.scheduleDao().setEnabled(reminderId, false)` and `db.scheduleDao().updateRunTimes(reminderId, null, null)`. + - ScheduleDao already has `setEnabled` and `updateRunTimes` (see `DatabaseSchema.kt`). +- On success: `call.resolve()`. On exception: log and `call.reject("cancelDailyReminder failed: ...")`. + +**Verification:** From the app, call `cancelDailyReminder({ reminderId: "daily_notification" })` (or your app’s id); it should resolve and the alarm for that id should be gone. + +--- + +## Verification Checklist (plugin) + +After implementing the three items above: + +1. **Reset test:** Schedule reminder 2–3 minutes from now → Edit and re-save **without changing time** → Confirm it still fires. Logs: "Skipping DB idempotence (skipPendingIntentIdempotence=true)...". +2. **Rollover test:** Set custom title/body → Let it fire once → Confirm next scheduled notification keeps the same title/body. Logs: `DN|ROLLOVER ... static=true scheduleId=daily_timesafari_reminder`. +3. **Cancel test:** Call `cancelDailyReminder({ reminderId })` from app or test harness; no error and alarm cleared. + +--- + +## Consuming App Work + +App-side changes are described in a separate document intended for the **crowd-funder-for-time-pwa** (Time Safari) repo: **CONSUMING_APP_CURSOR_BRIEF.md**. That document is written so you can paste it into Cursor in the app repo to implement: + +- Gate cancel in `editReminderNotification()` so Android skips pre-cancel (schedule path already cancels internally). +- Replace `TimeSafariNativeFetcher` placeholder with real content fetch and token persistence if using native fetcher for daily content. + +--- + +## References + +- NotifyReceiver: DB idempotence at ~206–226; skipPendingIntentIdempotence at ~159–204. +- DailyNotificationWorker: `scheduleNextNotification()` ~512–594; pass `preserveStaticReminder` and stable `scheduleId` into `scheduleExactNotification`. +- DailyNotificationPlugin: add `cancelDailyReminder` after `scheduleDailyReminder`; use `NotifyReceiver.cancelNotification` and ScheduleDao `setEnabled` / `updateRunTimes`. +- DatabaseSchema.kt: ScheduleDao `getById`, `upsert`, `setEnabled`, `updateRunTimes`. + +--- + +## Assumptions & Limits + +- App uses a stable reminder id (e.g. `daily_timesafari_reminder`); plugin preserves that id for static reminders on rollover. +- DAO method names are as in DatabaseSchema.kt; if the plugin’s Schedule entity uses different field names, adjust the `updateRunTimes` call accordingly (signature is `id, lastRunAt, nextRunAt`). +- Native fetcher and token persistence are app responsibilities; the plugin only needs to preserve static reminder semantics and provide cancel-by-id.