From 0e096b1a46e0646c0f5e9ed109f0247c45737c93 Mon Sep 17 00:00:00 2001 From: Jose Olarte III Date: Mon, 16 Feb 2026 21:25:07 +0800 Subject: [PATCH] Fix: Android daily notification: single schedule on edit, no double-cancel Resolves long-standing issue where the second scheduled time (after editing the reminder) did not fire on Android. - PushNotificationPermission: add open(..., options?: { skipSchedule }). When skipSchedule is true (edit flow), dialog only invokes callback with time/message; parent is sole scheduler so the plugin is not called twice. - AccountViewView: pass { skipSchedule: true } when opening the dialog for edit; keep cancel (iOS only) + single scheduleDailyNotification in callback. - NativeNotificationService: serialize scheduleDailyNotification so only one schedule runs at a time (scheduleLock + doScheduleDailyNotification). - AccountViewView: guard edit-reminder callback with editReminderScheduleInProgress so one schedule per user action. - Gate pre-cancel on Android in edit flow (CONSUMING_APP brief): skip cancelDailyNotification before schedule on Android; plugin cancels internally. - Use single stable reminder id and always pass id on both platforms (plugin 1.1.2+). - Add doc/plugin-android-edit-reschedule-alarm-not-firing.md for plugin repo (cancel-before-reschedule may cancel the PendingIntent used for setAlarmClock). --- ...ndroid-edit-reschedule-alarm-not-firing.md | 27 ++++ docs/DAILY_NOTIFICATION_BUG_DIAGNOSIS.md | 120 ++++++++++++++++++ package-lock.json | 8 +- src/components/PushNotificationPermission.vue | 10 ++ .../NativeNotificationService.ts | 21 ++- src/views/AccountViewView.vue | 21 ++- 6 files changed, 197 insertions(+), 10 deletions(-) create mode 100644 doc/plugin-android-edit-reschedule-alarm-not-firing.md create mode 100644 docs/DAILY_NOTIFICATION_BUG_DIAGNOSIS.md diff --git a/doc/plugin-android-edit-reschedule-alarm-not-firing.md b/doc/plugin-android-edit-reschedule-alarm-not-firing.md new file mode 100644 index 00000000..70be40f3 --- /dev/null +++ b/doc/plugin-android-edit-reschedule-alarm-not-firing.md @@ -0,0 +1,27 @@ +# Plugin: Android — Alarm set after edit doesn’t fire (cancel-before-reschedule) + +**Context:** Consuming app (TimeSafari) — user sets reminder at 6:57pm (fires), then edits to 7:00pm. Only one `scheduleDailyNotification` call is made (skipSchedule fix). Logs show "Scheduling OS alarm" and "Updated schedule in database" for 19:00, but the notification never fires at 7:00pm. + +**Likely cause (plugin):** In `NotifyReceiver.kt`, before calling `setAlarmClock(pendingIntent)` the code: + +1. Creates `pendingIntent` with `PendingIntent.getBroadcast(..., requestCode, intent, FLAG_UPDATE_CURRENT | FLAG_IMMUTABLE)`. +2. Gets `existingPendingIntent` with `PendingIntent.getBroadcast(..., requestCode, intent, FLAG_NO_CREATE | FLAG_IMMUTABLE)` (same `requestCode`, same `intent`). +3. If not null: `alarmManager.cancel(existingPendingIntent)` and **`existingPendingIntent.cancel()`**. +4. Then calls `alarmManager.setAlarmClock(alarmClockInfo, pendingIntent)`. + +On Android, PendingIntent equality for caching is based on requestCode and Intent (action, component, etc.), not necessarily all extras. So `existingPendingIntent` is often the **same** (cached) PendingIntent as `pendingIntent`. Then we call **`existingPendingIntent.cancel()`**, which cancels that PendingIntent for future use. We then use the same (now cancelled) PendingIntent in **`setAlarmClock(..., pendingIntent)`**. On some devices/versions, setting an alarm with a cancelled PendingIntent can result in the alarm not firing. + +**Suggested fix (plugin repo):** + +- Remove the **`existingPendingIntent.cancel()`** call. Use only **`alarmManager.cancel(existingPendingIntent)`** to clear any existing alarm for this requestCode. That way the PendingIntent we pass to `setAlarmClock` is not cancelled; only the previous alarm is removed. +- Optionally: only run the “cancel existing” block when we know there was a previous schedule (e.g. from DB) for this scheduleId that hasn’t fired yet, so we don’t cancel when the previous alarm already fired (e.g. user edited after first fire). + +**Verification:** + +- In the consuming app: set reminder 2–3 min from now, let it fire, then edit to 2–3 min from then and save. Capture logcat through the second scheduled time. +- If the receiver never logs at the second time, the OS didn’t deliver the alarm; fixing the cancel-before-reschedule logic as above should be tried first in the plugin. + +**References:** + +- CONSUMING_APP_ANDROID_NOTES.md (double schedule, alarm scheduled but not firing). +- NotifyReceiver.kt around “Cancelling existing alarm before rescheduling” and the following `setAlarmClock` use of `pendingIntent`. diff --git a/docs/DAILY_NOTIFICATION_BUG_DIAGNOSIS.md b/docs/DAILY_NOTIFICATION_BUG_DIAGNOSIS.md new file mode 100644 index 00000000..ea153ed9 --- /dev/null +++ b/docs/DAILY_NOTIFICATION_BUG_DIAGNOSIS.md @@ -0,0 +1,120 @@ +# Daily Notification Bugs — Diagnosis (Plugin + App) + +**Context:** Fixes were applied in both the plugin and the app, but "reset doesn't fire" and "notification text defaults to fallback" still occur. This doc summarizes what was checked and what to do next. + +--- + +## What Was Verified + +### App integration (correct) + +- **NativeNotificationService.ts** + - Pre-cancel is gated: only iOS calls `cancelDailyReminder()` before scheduling (lines 289–305). Android skips it. + - Schedules with `id: this.reminderId` (`"daily_timesafari_reminder"`), plus `time`, `title`, `body`. + - Calls `DailyNotification.scheduleDailyNotification(scheduleOptions)` (not `scheduleDailyReminder`). + +- **AccountViewView.vue** + - `editReminderNotification()` only calls `cancelDailyNotification()` when **not** Android (lines 1303–1305). On Android it only calls `scheduleDailyNotification()`. + +So the app is not double-cancelling on Android and is passing the expected options. + +### Plugin in app’s node_modules (fixed code present) + +- **node_modules/@timesafari/daily-notification-plugin** is at **version 1.1.4** and contains: + - **NotifyReceiver.kt:** DB idempotence is skipped when `skipPendingIntentIdempotence=true` (wrapped in `if (!skipPendingIntentIdempotence)`). + - **DailyNotificationWorker.java:** `preserveStaticReminder` read from input, stable `scheduleId` for static reminders, and `scheduleExactNotification(..., preserveStaticReminder, ...)`. + - **DailyNotificationPlugin.kt:** `cancelDailyReminder(call)` implemented. + +So the **source** the app uses (from its dependency) already has the fixes. + +### Plugin schedule path (correct) + +- App calls `scheduleDailyNotification` → plugin’s `scheduleDailyNotification(call)` → `ScheduleHelper.scheduleDailyNotification(...)`. +- That helper calls `NotifyReceiver.cancelNotification(context, scheduleId)` then `scheduleExactNotification(..., skipPendingIntentIdempotence = true)`. +- So the “re-set” path does set `skipPendingIntentIdempotence = true` and the DB idempotence skip should apply. + +--- + +## Likely Causes Why Bugs Still Appear + +### 1. Stale Android build / old APK + +The Android app compiles the plugin from: + +`android/capacitor.settings.gradle` → +`project(':timesafari-daily-notification-plugin').projectDir = new File('../node_modules/@timesafari/daily-notification-plugin/android')` + +If the app was not fully rebuilt after the plugin in node_modules was updated, the running APK may still contain old plugin code. + +**Do this:** + +- In the **app** repo (`crowd-funder-for-time-pwa`): + - `./gradlew clean` (or Android Studio → Build → Clean Project) + - Build and reinstall the app (e.g. Run on device/emulator). +- Confirm you’re not installing an older APK from somewhere else. + +### 2. Dependency not actually updated after plugin changes + +The app depends on: + +```json +"@timesafari/daily-notification-plugin": "git+https://gitea.anomalistdesign.com/trent_larson/daily-notification-plugin.git#master" +``` + +If the fixes were only made in a **different** clone (e.g. `daily-notification-plugin_test`) and never pushed to that gitea `master`, then: + +- `npm install` / `npm update` in the app would not pull the fixes. +- The app’s `node_modules` would only have the fixes if they were copied/linked from the fixed repo. + +**Do this:** + +- If the fixes live in another clone: either **push** the fixed plugin to gitea `master` and run `npm update @timesafari/daily-notification-plugin` (then `npx cap sync android`, then clean build), **or** point the app at the fixed plugin locally, e.g. in **app** `package.json`: + - `"@timesafari/daily-notification-plugin": "file:../daily-notification-plugin"` + (adjust path to your fixed plugin repo), then `npm install`, `npx cap sync android`, clean build and reinstall. + +### 3. Fallback text from native fetcher (Bug 2 only) + +**TimeSafariNativeFetcher.java** in the app is still a placeholder: it always returns: + +- Title: `"TimeSafari Update"` +- Body: `"Check your starred projects for updates!"` + +That only affects flows that **fetch** content (e.g. prefetch or any path that uses the fetcher for display). The **static** daily reminder path does not use the fetcher for display: title/body come from the schedule Intent and WorkManager input. So if you only use the “daily reminder” (one time + custom title/body), the fetcher placeholder should not be the cause. If you have any flow that relies on **fetched** content for the text, you’ll see that placeholder until the fetcher is implemented and wired (and optionally token persistence). + +--- + +## Verification Steps (after clean build + reinstall) + +1. **Reset / “re-set” (Bug 1)** + - Set reminder for 2–3 minutes from now. + - Edit and save **without changing the time**. + - Wait for the time; the notification should fire. + - In logcat, filter by the plugin’s tags and look for: + - `Skipping DB idempotence (skipPendingIntentIdempotence=true) for scheduleId=...` + - `Scheduling next daily alarm: id=daily_timesafari_reminder ...` + If you see these, the fixed path is running. + +2. **Static text on rollover (Bug 2)** + - Set a custom title/body, let the notification fire once. + - In logcat look for: + - `DN|ROLLOVER next=... scheduleId=daily_timesafari_reminder static=true` + If you see `static=true` and the same `scheduleId`, the next occurrence should keep your custom text. + +3. **Plugin version at build time** + - In the app’s `node_modules/@timesafari/daily-notification-plugin/package.json`, confirm `"version": "1.1.4"` (or the version that includes the fixes). + - After that, a clean build ensures that version is what’s in the APK. + +--- + +## Summary + +| Check | Status | +|-------|--------| +| App gates cancel on Android | OK | +| App calls scheduleDailyNotification with id/title/body | OK | +| Plugin in app node_modules has DB idempotence skip | OK (1.1.4) | +| Plugin in app node_modules has static rollover fix | OK | +| Plugin in app node_modules has cancelDailyReminder | OK | +| Schedule path passes skipPendingIntentIdempotence = true | OK | + +Most likely the app is still running an **old Android build**. Do a **clean build and reinstall**, and ensure the plugin dependency in the app really points at the fixed code (gitea master or local path). Then re-test and check logcat for the lines above. If the bugs persist after that, the next step is to capture a full logcat from “edit reminder (same time)” through the next fire and from “first fire” through “next day” to see which path runs. diff --git a/package-lock.json b/package-lock.json index 9632beb5..54aa7524 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "timesafari", - "version": "1.1.6-beta", + "version": "1.2.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "timesafari", - "version": "1.1.6-beta", + "version": "1.2.0", "dependencies": { "@capacitor-community/electron": "^5.0.1", "@capacitor-community/sqlite": "6.0.2", @@ -9452,8 +9452,8 @@ } }, "node_modules/@timesafari/daily-notification-plugin": { - "version": "1.1.3", - "resolved": "git+https://gitea.anomalistdesign.com/trent_larson/daily-notification-plugin.git#a62f54b8a877d78f757373897b36804c203b8351", + "version": "1.1.6", + "resolved": "git+https://gitea.anomalistdesign.com/trent_larson/daily-notification-plugin.git#9feaf60c8472d54365712473353bdd4eadb7dc90", "license": "MIT", "workspaces": [ "packages/*" diff --git a/src/components/PushNotificationPermission.vue b/src/components/PushNotificationPermission.vue index 663f8f84..785d4080 100644 --- a/src/components/PushNotificationPermission.vue +++ b/src/components/PushNotificationPermission.vue @@ -156,6 +156,8 @@ export default class PushNotificationPermission extends Vue { messageInput = ""; minuteInput = "00"; pushType = ""; + /** When true, dialog only returns time/message to parent; parent does cancel+schedule (avoids double schedule on edit). */ + skipScheduleForOpen = false; serviceWorkerReady = false; vapidKey = ""; @@ -169,10 +171,12 @@ export default class PushNotificationPermission extends Vue { async open( pushType: string, callback?: (success: boolean, time: string, message?: string) => void, + options?: { skipSchedule?: boolean }, ) { this.callback = callback || this.callback; this.isVisible = true; this.pushType = pushType; + this.skipScheduleForOpen = options?.skipSchedule ?? false; // Native platforms: Skip web push initialization if (this.isNativePlatform) { @@ -689,6 +693,12 @@ export default class PushNotificationPermission extends Vue { "[PushNotificationPermission] Starting native notification setup", ); + // Edit flow: parent will cancel + schedule; avoid double schedule (second call cancels alarm first set). + if (this.skipScheduleForOpen) { + this.callback(true, this.notificationTimeText, this.messageInput); + return; + } + // Import and check plugin availability before using service const { DailyNotification } = await import( "@/plugins/DailyNotificationPlugin" diff --git a/src/services/notifications/NativeNotificationService.ts b/src/services/notifications/NativeNotificationService.ts index 06f0363b..bd9e8b64 100644 --- a/src/services/notifications/NativeNotificationService.ts +++ b/src/services/notifications/NativeNotificationService.ts @@ -49,6 +49,12 @@ export class NativeNotificationService implements NotificationServiceInterface { */ private readonly reminderId = "daily_timesafari_reminder"; + /** + * Ensures only one scheduleDailyNotification runs at a time (no rapid successive plugin calls). + * Each new call waits for the previous to complete before running. + */ + private scheduleLock: Promise = Promise.resolve(true); + /** * Native notifications are always supported on iOS/Android */ @@ -235,10 +241,23 @@ export class NativeNotificationService implements NotificationServiceInterface { } /** - * Schedule a daily notification using native alarms + * Schedule a daily notification using native alarms. + * Serialized so only one schedule runs at a time (avoids rapid successive plugin calls on Android). */ async scheduleDailyNotification( options: DailyNotificationOptions, + ): Promise { + const run = (): Promise => + this.doScheduleDailyNotification(options); + this.scheduleLock = this.scheduleLock.then(() => run()); + return this.scheduleLock; + } + + /** + * Internal implementation of schedule; called under scheduleLock. + */ + private async doScheduleDailyNotification( + options: DailyNotificationOptions, ): Promise { try { logger.info( diff --git a/src/views/AccountViewView.vue b/src/views/AccountViewView.vue index 3a479018..88358bef 100644 --- a/src/views/AccountViewView.vue +++ b/src/views/AccountViewView.vue @@ -824,6 +824,7 @@ interface PushNotificationPermissionRef { open: ( title: string, callback: (success: boolean, timeText: string, message?: string) => void, + options?: { skipSchedule?: boolean }, ) => void; hourInput?: string; minuteInput?: string; @@ -896,6 +897,8 @@ export default class AccountViewView extends Vue { notifyingReminder: boolean = false; notifyingReminderMessage: string = ""; notifyingReminderTime: string = ""; + /** Guard: only one edit-reminder schedule per user action (avoids double schedule on Android). */ + editReminderScheduleInProgress: boolean = false; subscription: PushSubscription | null = null; // UI state properties @@ -1293,16 +1296,21 @@ export default class AccountViewView extends Vue { const dialog = this.$refs .pushNotificationPermission as PushNotificationPermission; - // Open the dialog - it will use the same callback pattern as showReminderNotificationChoice + // skipSchedule: true so only this callback schedules (dialog does not). Avoids double schedule on Android. dialog.open( DIRECT_PUSH_TITLE, async (success: boolean, timeText: string, message?: string) => { - if (success) { - // Cancel the old notification before scheduling the new one + if (!success) return; + if (this.editReminderScheduleInProgress) return; + this.editReminderScheduleInProgress = true; + try { const service = NotificationService.getInstance(); - await service.cancelDailyNotification(); + // On iOS: cancel then schedule. On Android: plugin cancels internally when scheduling with same id; skip pre-cancel to avoid double-cancel edge cases. + if (Capacitor.getPlatform() !== "android") { + await service.cancelDailyNotification(); + } - // Schedule the updated notification + // Schedule the updated notification (one call per user action) const time24h = this.parseTimeTo24Hour(timeText); const title = "Daily Reminder"; const body = @@ -1325,8 +1333,11 @@ export default class AccountViewView extends Vue { this.notifyingReminderMessage = message || ""; this.notifyingReminderTime = timeText; } + } finally { + this.editReminderScheduleInProgress = false; } }, + { skipSchedule: true }, ); // Pre-populate the form with current values after dialog opens