forked from trent_larson/crowd-funder-for-time-pwa
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).
This commit is contained in:
Jose Olarte III
85
doc/android-daily-notification-second-schedule-issue.md
Normal file
85
doc/android-daily-notification-second-schedule-issue.md
Normal file
@@ -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.
|
||||
Reference in New Issue
Block a user