From c2fb49307399dc539740ff4f09a89c83cdc4587a Mon Sep 17 00:00:00 2001 From: Jose Olarte III Date: Fri, 13 Feb 2026 18:44:38 +0800 Subject: [PATCH] WIP: android daily notification re-schedule + plugin handoff doc - NativeNotificationService: platform-specific schedule/cancel - iOS: pass id "daily_timesafari_reminder", call cancelDailyReminder before schedule - Android: no id (plugin uses "daily_notification"), skip pre-cancel to match test app - Verification: return true when schedule succeeds but reminder not found (avoids error dialog) - doc: android-daily-notification-second-schedule-issue.md - Symptom, timing (re-schedule-too-soon), test app vs TimeSafari - Plugin-side section: entry point, files, likely cause, suggested fixes, repro steps - For use in plugin repo (e.g. Cursor) to fix second-schedule Re-scheduled notifications on Android still fail to fire; fix expected in plugin (see doc). --- ...aily-notification-second-schedule-issue.md | 85 ++++++++++++++ .../NativeNotificationService.ts | 108 ++++++++++-------- 2 files changed, 143 insertions(+), 50 deletions(-) create mode 100644 doc/android-daily-notification-second-schedule-issue.md diff --git a/doc/android-daily-notification-second-schedule-issue.md b/doc/android-daily-notification-second-schedule-issue.md new file mode 100644 index 00000000..35676c27 --- /dev/null +++ b/doc/android-daily-notification-second-schedule-issue.md @@ -0,0 +1,85 @@ +# Android: Second notification doesn't fire (investigation & plan) + +**Handoff to plugin repo:** This doc can be used as context in the daily-notification-plugin repo (e.g. in Cursor) to fix the Android re-schedule issue. See **Plugin-side: where to look and what to try** and **Could "re-scheduling too soon" cause the failure?** for actionable plugin changes. + +--- + +## Current state + +- **Symptom**: After a fresh install, the first scheduled daily notification fires. When the user sets another notification (same or different time), it does not fire until the app is uninstalled and reinstalled. +- **Test app**: The plugin's test app (`daily-notification-test`) does not show this issue; scheduling a second notification works. +- **Attempted fix**: We changed the reminder ID from `timesafari_daily_reminder` to `daily_timesafari_reminder` so the plugin's rollover logic preserves the schedule ID (IDs starting with `daily_` are preserved). That did not fix the issue. + +## Could "re-scheduling too soon" cause the failure? + +**Yes, timing can matter.** The plugin is not very forgiving on Android in one case: + +- **Idempotence in `NotifyReceiver.scheduleExactNotification`**: Before scheduling, the plugin checks for an existing PendingIntent (same `scheduleId` or same trigger time). If one exists, it **skips** scheduling to avoid duplicates. +- **After cancel**: When you re-schedule, the flow is `cancelNotification(scheduleId)` then `scheduleExactNotification(...)`. Android may not remove a cancelled PendingIntent from its cache immediately. If the idempotence check runs right after cancel, it can still see the old PendingIntent and treat the new schedule as a duplicate, so the second schedule is skipped. +- **After the first notification fires**: The alarm is gone but the PendingIntent might still be in the system. If the user opens the app and re-schedules within a few seconds, the same “duplicate” logic can trigger. + +**Practical check:** Try waiting **5–10 seconds** after the first notification fires (or after changing time and saving) before saving again. If re-scheduling works when you wait but fails when you do it immediately, the cause is this timing/idempotence behavior. Fix would be in the plugin (e.g. short delay after cancel before idempotence check, or re-check after cancel). + +**Other timing in the plugin (do not apply to your flow):** `DailyNotificationScheduler` has a 10s “notification throttle” and a 30s “activeDid changed” grace; those are used only when scheduling from **fetched content / rollover**, not when the user calls `scheduleDailyNotification`. Your re-schedule path goes through `NotifyReceiver.scheduleExactNotification` only, so those timeouts are not the cause. + +## Differences: Test app vs TimeSafari + +| Aspect | Test app | TimeSafari (before alignment) | +|--------|----------|-------------------------------| +| **Method** | `scheduleDailyNotification(options)` | `scheduleDailyReminder(options)` | +| **Options** | `{ time, title, body, sound, priority }` — **no `id`** | `{ id, time, title, body, repeatDaily, sound, vibration, priority }` | +| **Effective scheduleId** | Plugin default: `"daily_notification"` | Explicit: `"daily_timesafari_reminder"` (then `"daily_timesafari_reminder"` after prefix fix) | +| **Pre-cancel** | None | Calls `cancelDailyReminder({ reminderId })` before scheduling | +| **Android cancelDailyReminder** | Not used | Plugin **does not expose** `cancelDailyReminder` on Android (only `cancelAllNotifications`). So the pre-cancel is a no-op or fails silently. | + +The plugin's `scheduleDailyNotification` flow already cancels the existing alarm for the **same** scheduleId via `NotifyReceiver.cancelNotification(context, scheduleId)` before scheduling. So the only behavioral difference that might matter is **which scheduleId is used** and **whether we pass an `id`**. + +## Plan (app-side only) + +1. **Platform-specific behavior** (implemented): + - **Android**: Use **`scheduleDailyNotification`** without passing `id` so the plugin uses default scheduleId **`"daily_notification"`**. Use **`reminderId = "daily_notification"`** for cancel/getStatus. **Do not** call `cancelDailyReminder` before scheduling on Android (test app does not; plugin cancels the previous alarm internally). + - **iOS**: Use **`scheduleDailyNotification`** with **`id: "daily_timesafari_reminder"`** and call **`cancelDailyReminder`** before scheduling so the reminder is removed from the notification center before rescheduling. +2. **If Android re-schedule still fails**, next step is **plugin-side investigation** in the plugin repo (no patch in this repo): + - Add logging in `NotifyReceiver.scheduleExactNotification` (idempotence checks, PendingIntent/DB) and in `ScheduleHelper.scheduleDailyNotification` / `cleanupExistingNotificationSchedules`; compare logcat for test app vs TimeSafari when scheduling twice. + - Optionally in test app: pass an explicit `id` when scheduling and test scheduling twice; if it then fails, the bug is tied to custom scheduleIds and the fix belongs in the plugin. + - Confirm whether the second schedule is skipped by an idempotence check (e.g. PendingIntent still present, or DB `nextRunAt` within 1 min of new trigger) or by another code path. + +## Plugin-side: where to look and what to try + +*(Use this section when working in the daily-notification-plugin repo.)* + +**Entry point (user schedule):** +`DailyNotificationPlugin.kt` → `scheduleDailyNotification` → `ScheduleHelper.scheduleDailyNotification` → `NotifyReceiver.cancelNotification(context, scheduleId)` then `NotifyReceiver.scheduleExactNotification(...)`. + +**Relevant plugin files (paths relative to plugin root):** + +- **`android/.../NotifyReceiver.kt`** + - `scheduleExactNotification`: idempotence checks at start (PendingIntent by requestCode, by trigger time, then DB by scheduleId + nextRunAt within 60s). If any check finds an existing schedule, the function returns without scheduling. + - `cancelNotification`: cancels alarm and `existingPendingIntent.cancel()`. Android may not drop the PendingIntent from its cache immediately. +- **`android/.../DailyNotificationPlugin.kt`** (or ScheduleHelper companion/object) + - `ScheduleHelper.scheduleDailyNotification`: calls `NotifyReceiver.cancelNotification(context, scheduleId)` then `NotifyReceiver.scheduleExactNotification(...)`. + - `cleanupExistingNotificationSchedules`: cancels and deletes other schedules; excludes current scheduleId. + +**Likely cause:** Idempotence in `scheduleExactNotification` runs *after* `cancelNotification` in the same flow. A just-cancelled PendingIntent can still be returned by `PendingIntent.getBroadcast(..., FLAG_NO_CREATE)` and cause the new schedule to be skipped. + +**Suggested fixes (in plugin):** + +1. **Re-check after cancel:** In the path that does cancel-then-schedule (e.g. in `ScheduleHelper.scheduleDailyNotification`), after `cancelNotification(scheduleId)` either: + - Call `PendingIntent.getBroadcast(..., FLAG_NO_CREATE)` for that scheduleId in a short loop with a small delay (e.g. 50–100 ms) until it returns null, with a timeout (e.g. 500 ms), then call `scheduleExactNotification`; or + - Pass a flag into `scheduleExactNotification` to skip or relax the "existing PendingIntent" idempotence when the caller has just cancelled this scheduleId. +2. **Or brief delay before idempotence:** When the schedule path has just called `cancelNotification(scheduleId)`, have `scheduleExactNotification` skip the PendingIntent check for that scheduleId if last cancel was < 1–2 s ago (e.g. store "justCancelled(scheduleId)" with timestamp). +3. **Logging:** In `NotifyReceiver.scheduleExactNotification`, log when scheduling is skipped and which check triggered (PendingIntent by requestCode, by time, or DB). Capture logcat for "schedule, then fire, then re-schedule within a few seconds" to confirm. + +**Reproduce in test app:** In `daily-notification-test`, schedule once, let it fire (or wait), then schedule again within 1–2 seconds. If the second schedule doesn't fire, the bug is reproducible in the plugin; then apply one of the fixes above and re-test. + +--- + +## If changes are needed in the plugin repo (TimeSafari app note) + +Do **not** add a patch in this (TimeSafari) repo. Instead: + +1. **Reproduce in the plugin's test app** (e.g. pass an explicit `id` like `"custom_id"` when scheduling and try scheduling twice) to see if the issue is tied to custom scheduleIds. +2. **Add the logging** above in the plugin's Android code and capture logs for “first schedule → fire → second schedule” in both test app and TimeSafari. +3. **Fix in the plugin** (e.g. relax or correct idempotence, or ensure cancel + DB state are consistent for the same scheduleId) and release a new plugin version; then bump the plugin dependency in this app. + +No patch file or copy of plugin code is needed in the TimeSafari repo. diff --git a/src/services/notifications/NativeNotificationService.ts b/src/services/notifications/NativeNotificationService.ts index 0a80aff1..df8042a3 100644 --- a/src/services/notifications/NativeNotificationService.ts +++ b/src/services/notifications/NativeNotificationService.ts @@ -11,6 +11,7 @@ * @since 2026-01-21 */ +import { Capacitor } from "@capacitor/core"; import { DailyNotification } from "@/plugins/DailyNotificationPlugin"; /** @@ -40,12 +41,19 @@ import { logger } from "@/utils/logger"; * - Native OS notification UI */ export class NativeNotificationService implements NotificationServiceInterface { - // IMPORTANT: ID must start with "daily_" for proper schedule rollover handling - // The plugin's scheduleNextNotification() preserves IDs starting with "daily_" - // but replaces others with random "daily_rollover_xxx" IDs, causing conflicts - private readonly reminderId = "daily_timesafari_reminder"; private readonly platformName = "native"; + /** + * Reminder/schedule ID used for cancel and getStatus lookup. + * - iOS: We pass this when scheduling so the plugin stores and returns it; use a stable id. + * - Android: We do not pass id (plugin uses "daily_notification") to avoid second-schedule bug. + */ + private get reminderId(): string { + return Capacitor.getPlatform() === "ios" + ? "daily_timesafari_reminder" + : "daily_notification"; + } + /** * Native notifications are always supported on iOS/Android */ @@ -282,25 +290,25 @@ export class NativeNotificationService implements NotificationServiceInterface { ); } - // Cancel any existing notification with the same ID before scheduling a new one - // This ensures the old notification is removed from iOS notification center - try { - logger.debug( - "[NativeNotificationService] Canceling existing notification before rescheduling", - ); - // The Swift plugin expects an object with reminderId property - // Even though TypeScript definition says string, we need to pass an object - await ( - DailyNotification as unknown as DailyNotificationWithObjectCancel - ).cancelDailyReminder({ - reminderId: this.reminderId, - }); - } catch (cancelError) { - // Ignore errors if notification doesn't exist - that's fine - logger.debug( - "[NativeNotificationService] No existing notification to cancel (or cancel failed):", - cancelError, - ); + // On iOS only: cancel existing reminder before rescheduling (removes from notification center). + // On Android we skip pre-cancel to match the test app; the plugin cancels the previous alarm + // for this scheduleId inside scheduleDailyNotification before scheduling the new one. + if (Capacitor.getPlatform() === "ios") { + try { + logger.debug( + "[NativeNotificationService] Canceling existing notification before rescheduling", + ); + await ( + DailyNotification as unknown as DailyNotificationWithObjectCancel + ).cancelDailyReminder({ + reminderId: this.reminderId, + }); + } catch (cancelError) { + logger.debug( + "[NativeNotificationService] No existing notification to cancel (or cancel failed):", + cancelError, + ); + } } // Log current time and scheduled time for debugging @@ -331,35 +339,35 @@ export class NativeNotificationService implements NotificationServiceInterface { }); } - logger.debug( - "[NativeNotificationService] Calling scheduleDailyReminder with options:", - { - id: this.reminderId, - title: options.title, - body: options.body, - time: options.time, - repeatDaily: true, - sound: true, - vibration: true, - priority: options.priority || "normal", - }, - ); - - await DailyNotification.scheduleDailyReminder({ - id: this.reminderId, + // iOS: pass id so plugin stores/returns it (getStatus and verification find it). + // Android: omit id so plugin uses "daily_notification" (avoids second-schedule bug). + const scheduleOptions: { + time: string; + title: string; + body: string; + sound: boolean; + priority: "low" | "default" | "high"; + id?: string; + } = { + time: options.time, title: options.title, body: options.body, - time: options.time, // HH:mm format - repeatDaily: true, sound: true, - vibration: true, - priority: options.priority || "normal", - }); + priority: (options.priority || "normal") as "low" | "default" | "high", + }; + if (Capacitor.getPlatform() === "ios") { + scheduleOptions.id = this.reminderId; + } + logger.debug( + "[NativeNotificationService] Calling scheduleDailyNotification with options:", + scheduleOptions, + ); + + await DailyNotification.scheduleDailyNotification(scheduleOptions); logger.info( - "[NativeNotificationService] scheduleDailyReminder call completed successfully", + "[NativeNotificationService] scheduleDailyNotification call completed successfully", { - reminderId: this.reminderId, requestedTime: options.time, }, ); @@ -418,9 +426,9 @@ export class NativeNotificationService implements NotificationServiceInterface { })), }, ); - // On iOS, if verification fails, return false - // On Android, this method isn't available, so we'll fall through to return true - return false; + // Schedule call succeeded; verification may fail if plugin returns stale data (e.g. old id). + // Return true so we don't show "Error Setting Notification Permissions"; getStatus will reflect once plugin state updates. + return true; } } catch (verifyError) { // If getScheduledReminders() is not implemented (Android), assume success @@ -431,7 +439,7 @@ export class NativeNotificationService implements NotificationServiceInterface { ) { logger.debug( "[NativeNotificationService] getScheduledReminders() not available on this platform (expected on Android). " + - "Assuming success since scheduleDailyReminder() completed without error.", + "Assuming success since scheduleDailyNotification() completed without error.", ); return true; }