fix problem on android with continued notifications even when it was turned off
This commit is contained in:
25
LICENSE
25
LICENSE
@@ -1,21 +1,8 @@
|
||||
MIT License
|
||||
The author disclaims copyright to this source code. In place of a legal notice, here is a blessing:
|
||||
|
||||
Copyright (c) 2024 Matthew Raymer
|
||||
May you do good and not evil.
|
||||
May you find forgiveness for yourself and forgive others.
|
||||
May you share freely, never taking more than you give.
|
||||
|
||||
Permission is hereby granted, free of charge, to any person obtaining a copy
|
||||
of this software and associated documentation files (the "Software"), to deal
|
||||
in the Software without restriction, including without limitation the rights
|
||||
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
|
||||
copies of the Software, and to permit persons to whom the Software is
|
||||
furnished to do so, subject to the following conditions:
|
||||
|
||||
The above copyright notice and this permission notice shall be included in all
|
||||
copies or substantial portions of the Software.
|
||||
|
||||
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
|
||||
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
|
||||
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
|
||||
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
|
||||
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
|
||||
SOFTWARE.
|
||||
________________________________________________________________
|
||||
from https://www.sqlite.org/src/info/689401a6cfb4c234 and memorialized here https://spdx.org/licenses/blessing.html
|
||||
|
||||
@@ -12,7 +12,7 @@ This is to support apps that allow users to own their data. This approach is in
|
||||
|
||||
* Peer-to-peer network scenarios are not supported.
|
||||
|
||||
There are two types of notifications supported:
|
||||
There are two types of notifications supported, static & API:
|
||||
|
||||
* Periodic static reminder messages
|
||||
|
||||
|
||||
@@ -699,9 +699,13 @@ open class DailyNotificationPlugin : Plugin() {
|
||||
Log.i(TAG, "Cancelling all notifications")
|
||||
|
||||
// 1. Get all scheduled notifications from database
|
||||
// Includes "static_reminder" (isolated kind, see PLAN-simplify-static-and-refactor.md
|
||||
// §3.1) alongside "notify" so cancelAllNotifications() genuinely cancels everything.
|
||||
val schedules = getDatabase().scheduleDao().getAll()
|
||||
val notifySchedules = schedules.filter { it.kind == "notify" && it.enabled }
|
||||
|
||||
val notifySchedules = schedules.filter {
|
||||
(it.kind == "notify" || it.kind == "static_reminder") && it.enabled
|
||||
}
|
||||
|
||||
// 2. Cancel all AlarmManager alarms (delegate to ScheduleHelper)
|
||||
// Only cancel alarms we can prove we scheduled (from database)
|
||||
// This prevents accidental cancellation of other alarms and false confidence
|
||||
@@ -726,9 +730,10 @@ open class DailyNotificationPlugin : Plugin() {
|
||||
try {
|
||||
// Delegate to ScheduleHelper
|
||||
val disabledNotify = ScheduleHelper.disableAllSchedulesByKind(getDatabase(), "notify")
|
||||
val disabledStaticReminder = ScheduleHelper.disableAllSchedulesByKind(getDatabase(), "static_reminder")
|
||||
val disabledFetch = ScheduleHelper.disableAllSchedulesByKind(getDatabase(), "fetch")
|
||||
|
||||
Log.i(TAG, "Disabled $disabledNotify notification schedule(s) and $disabledFetch fetch schedule(s)")
|
||||
|
||||
Log.i(TAG, "Disabled $disabledNotify notification schedule(s), $disabledStaticReminder static reminder schedule(s), and $disabledFetch fetch schedule(s)")
|
||||
} catch (e: Exception) {
|
||||
Log.e(TAG, "Failed to clear database state", e)
|
||||
// Continue - alarms and jobs are already cancelled
|
||||
@@ -825,14 +830,24 @@ open class DailyNotificationPlugin : Plugin() {
|
||||
return
|
||||
}
|
||||
NotifyReceiver.cancelNotification(context, scheduleId = reminderId)
|
||||
// Pure single-row delete: now that static reminders are isolated to their own
|
||||
// "static_reminder" kind (see PLAN-simplify-static-and-refactor.md §3.1), there is
|
||||
// guaranteed to be at most one row to deal with, so cancel can fully remove it
|
||||
// ("cancel means gone") rather than leaving a disabled-but-present row that the
|
||||
// rearm chain could still find and resurrect. See plan §3.3 for the deleteById vs
|
||||
// setEnabled tradeoff discussion.
|
||||
try {
|
||||
kotlinx.coroutines.runBlocking {
|
||||
val db = getDatabase()
|
||||
db.scheduleDao().setEnabled(reminderId, false)
|
||||
db.scheduleDao().updateRunTimes(reminderId, null, null)
|
||||
db.scheduleDao().deleteById(reminderId)
|
||||
try {
|
||||
db.notificationContentDao().deleteNotification(reminderId)
|
||||
} catch (contentErr: Exception) {
|
||||
Log.w(TAG, "cancelDailyReminder: failed to delete notification content for $reminderId", contentErr)
|
||||
}
|
||||
}
|
||||
} catch (dbErr: Exception) {
|
||||
Log.w(TAG, "cancelDailyReminder: failed DB update for $reminderId", dbErr)
|
||||
Log.w(TAG, "cancelDailyReminder: failed DB delete for $reminderId", dbErr)
|
||||
}
|
||||
call.resolve()
|
||||
} catch (e: Exception) {
|
||||
@@ -1196,30 +1211,14 @@ open class DailyNotificationPlugin : Plugin() {
|
||||
// If user provides an ID, use it; otherwise use stable "daily_notification"
|
||||
val scheduleId = options.getString("id") ?: "daily_notification"
|
||||
Log.i(TAG, "scheduleDailyNotification: START - time=$time, scheduleId=$scheduleId")
|
||||
|
||||
// CRITICAL: Cancel and delete all existing notification schedules before creating new one
|
||||
// This ensures "one per day" semantics - only one daily notification schedule exists
|
||||
// Delegate cleanup to ScheduleHelper
|
||||
val cleanedCount = ScheduleHelper.cleanupExistingNotificationSchedules(
|
||||
context,
|
||||
getDatabase(),
|
||||
excludeScheduleId = scheduleId
|
||||
)
|
||||
|
||||
if (cleanedCount > 0) {
|
||||
Log.i(TAG, "scheduleDailyNotification: ✅ Cleaned up $cleanedCount existing notification schedule(s) before creating new one")
|
||||
} else {
|
||||
Log.i(TAG, "scheduleDailyNotification: No cleanup needed - existing schedule will be updated via upsert: $scheduleId")
|
||||
}
|
||||
|
||||
// Cancel only fetch-related WorkManager jobs so they cannot create a second (UUID) alarm
|
||||
// with fallback or placeholder text. Does not cancel display/dismiss; future fetched-content
|
||||
// flows should use distinct tags so they are not affected.
|
||||
val workCancelled = ScheduleHelper.cancelFetchRelatedWorkManagerJobs(context)
|
||||
if (workCancelled) {
|
||||
Log.i(TAG, "scheduleDailyNotification: Cancelled pending prefetch/fetch WorkManager jobs")
|
||||
}
|
||||
|
||||
|
||||
// No sweep/cleanup needed here: static reminders live in their own isolated
|
||||
// "static_reminder" kind (see PLAN-simplify-static-and-refactor.md §3.1) and
|
||||
// ScheduleHelper.scheduleDailyNotification() below does a direct upsert by the
|
||||
// stable scheduleId, so there is nothing else in that kind to collide with.
|
||||
// Static reminders also never enqueue prefetch/fetch WorkManager jobs, so there
|
||||
// is nothing for cancelFetchRelatedWorkManagerJobs() to clean up either.
|
||||
|
||||
val config = UserNotificationConfig(
|
||||
enabled = true,
|
||||
schedule = cronExpression,
|
||||
@@ -1423,56 +1422,6 @@ open class DailyNotificationPlugin : Plugin() {
|
||||
}
|
||||
}
|
||||
|
||||
@PluginMethod
|
||||
fun scheduleUserNotification(call: PluginCall) {
|
||||
try {
|
||||
if (context == null) {
|
||||
return call.reject("Context not available")
|
||||
}
|
||||
|
||||
// Check if exact alarms can be scheduled
|
||||
if (!canScheduleExactAlarms(context)) {
|
||||
// Delegate to PermissionManager to handle exact alarm permission request
|
||||
val activity = activity
|
||||
if (activity == null) {
|
||||
return call.reject("Activity not available")
|
||||
}
|
||||
permissionManager!!.requestExactAlarmPermission(call)
|
||||
return
|
||||
}
|
||||
|
||||
// Permission granted - proceed with scheduling
|
||||
val configJson = call.getObject("config")
|
||||
val config = parseUserNotificationConfig(configJson)
|
||||
|
||||
Log.i(TAG, "Scheduling user notification")
|
||||
|
||||
CoroutineScope(Dispatchers.IO).launch {
|
||||
try {
|
||||
// Delegate to ScheduleHelper
|
||||
val scheduleId = ScheduleHelper.scheduleUserNotification(
|
||||
context,
|
||||
getDatabase(),
|
||||
config,
|
||||
::calculateNextRunTime
|
||||
)
|
||||
|
||||
if (scheduleId != null) {
|
||||
call.resolve()
|
||||
} else {
|
||||
call.reject("User notification scheduling failed")
|
||||
}
|
||||
} catch (e: Exception) {
|
||||
Log.e(TAG, "Failed to schedule user notification", e)
|
||||
call.reject("User notification scheduling failed: ${e.message}")
|
||||
}
|
||||
}
|
||||
} catch (e: Exception) {
|
||||
Log.e(TAG, "Schedule user notification error", e)
|
||||
call.reject("User notification error: ${e.message}")
|
||||
}
|
||||
}
|
||||
|
||||
@PluginMethod
|
||||
fun scheduleDualNotification(call: PluginCall) {
|
||||
try {
|
||||
@@ -2776,8 +2725,8 @@ object ScheduleHelper {
|
||||
schedules.forEach { schedule ->
|
||||
val scheduleJson = scheduleToJson(schedule)
|
||||
|
||||
// Only check AlarmManager status for "notify" schedules with nextRunAt
|
||||
if (schedule.kind == "notify" && schedule.nextRunAt != null) {
|
||||
// Only check AlarmManager status for "notify"/"static_reminder" schedules with nextRunAt
|
||||
if ((schedule.kind == "notify" || schedule.kind == "static_reminder") && schedule.nextRunAt != null) {
|
||||
val isScheduled = NotifyReceiver.isAlarmScheduled(context, scheduleId = schedule.id, triggerAtMillis = schedule.nextRunAt!!)
|
||||
scheduleJson.put("isActuallyScheduled", isScheduled)
|
||||
} else {
|
||||
@@ -2842,9 +2791,14 @@ object ScheduleHelper {
|
||||
// notifications at fire time.
|
||||
|
||||
// Store schedule in database (include rollover interval for dev/testing; survives reboot)
|
||||
// kind = "static_reminder" isolates this from the shared "notify" kind used by the
|
||||
// dual/API-retrieval flow (generator #2) and legacy test-data injection, so the
|
||||
// cleanup sweep in cleanupExistingNotificationSchedules() (which only scans
|
||||
// kind="notify") can never touch or be confused by this row. See
|
||||
// doc/progress/PLAN-simplify-static-and-refactor.md §3.1.
|
||||
val schedule = Schedule(
|
||||
id = scheduleId,
|
||||
kind = "notify",
|
||||
kind = "static_reminder",
|
||||
cron = config.schedule,
|
||||
clockTime = clockTime,
|
||||
enabled = true,
|
||||
@@ -2904,15 +2858,18 @@ object ScheduleHelper {
|
||||
}
|
||||
|
||||
/**
|
||||
* Blocking: first enabled notify schedule with rolloverIntervalMinutes > 0 (canonical for rollover chain).
|
||||
* Blocking: first enabled static_reminder schedule with rolloverIntervalMinutes > 0
|
||||
* (canonical for rollover chain).
|
||||
* Used when the firing run has schedule_id = daily_rollover_* so we can still apply the interval.
|
||||
* This whole function only exists to support that orphan-fallback path; it becomes dead
|
||||
* code once §3.6 removes the daily_rollover_* fallback (see PLAN-simplify-static-and-refactor.md).
|
||||
*/
|
||||
@JvmStatic
|
||||
fun getCanonicalRolloverScheduleBlocking(context: Context): Schedule? {
|
||||
return kotlinx.coroutines.runBlocking {
|
||||
try {
|
||||
DailyNotificationDatabase.getDatabase(context).scheduleDao()
|
||||
.getByKindAndEnabled("notify", true)
|
||||
.getByKindAndEnabled("static_reminder", true)
|
||||
.firstOrNull { it.rolloverIntervalMinutes != null && it.rolloverIntervalMinutes > 0 }
|
||||
} catch (e: Exception) {
|
||||
Log.w("ScheduleHelper", "getCanonicalRolloverScheduleBlocking failed", e)
|
||||
@@ -3443,9 +3400,16 @@ object NotificationStatusHelper {
|
||||
*/
|
||||
suspend fun getNotificationStatus(database: DailyNotificationDatabase): JSObject {
|
||||
val schedules = database.scheduleDao().getAll()
|
||||
val notifySchedules = schedules.filter { it.kind == "notify" && it.enabled }
|
||||
|
||||
// Get last notification time from history
|
||||
// Includes "static_reminder" alongside "notify" so status reporting (isEnabled,
|
||||
// isScheduled, nextNotificationTime, etc.) still reflects the static reminder now
|
||||
// that it's isolated to its own kind. See PLAN-simplify-static-and-refactor.md §3.1.
|
||||
val notifySchedules = schedules.filter {
|
||||
(it.kind == "notify" || it.kind == "static_reminder") && it.enabled
|
||||
}
|
||||
|
||||
// Get last notification time from history. History.kind is a fixed event-type tag
|
||||
// written uniformly by NotifyReceiver.recordHistory() for any alarm fire (static
|
||||
// reminder or dual-notify) — independent of Schedule.kind, so no change needed here.
|
||||
val history = database.historyDao().getRecent(100) // Get last 100 entries
|
||||
val lastNotification = history
|
||||
.filter { it.kind == "notify" && it.outcome == "success" }
|
||||
|
||||
@@ -202,339 +202,14 @@ public class DailyNotificationReceiver extends BroadcastReceiver {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Handle notification intent
|
||||
*
|
||||
* @param context Application context
|
||||
* @param intent Intent containing notification data
|
||||
*/
|
||||
private void handleNotificationIntent(Context context, Intent intent) {
|
||||
try {
|
||||
String notificationId = intent.getStringExtra(EXTRA_NOTIFICATION_ID);
|
||||
|
||||
if (notificationId == null) {
|
||||
Log.w(TAG, "Notification ID not found in intent");
|
||||
return;
|
||||
}
|
||||
|
||||
Log.d(TAG, "Processing notification: " + notificationId);
|
||||
|
||||
// Get notification content from storage
|
||||
DailyNotificationStorage storage = new DailyNotificationStorage(context);
|
||||
NotificationContent content = storage.getNotificationContent(notificationId);
|
||||
|
||||
if (content == null) {
|
||||
Log.w(TAG, "Notification content not found: " + notificationId);
|
||||
return;
|
||||
}
|
||||
|
||||
// Check if notification is ready to display
|
||||
if (!content.isReadyToDisplay()) {
|
||||
Log.d(TAG, "Notification not ready to display yet: " + notificationId);
|
||||
return;
|
||||
}
|
||||
|
||||
// JIT Freshness Re-check (Soft TTL)
|
||||
content = performJITFreshnessCheck(context, content);
|
||||
|
||||
// Display the notification
|
||||
displayNotification(context, content);
|
||||
|
||||
// Schedule next notification if this is a recurring daily notification
|
||||
scheduleNextNotification(context, content);
|
||||
|
||||
Log.i(TAG, "Notification processed successfully: " + notificationId);
|
||||
|
||||
} catch (Exception e) {
|
||||
Log.e(TAG, "Error handling notification intent", e);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Perform JIT (Just-In-Time) freshness re-check for notification content
|
||||
*
|
||||
* This implements a soft TTL mechanism that attempts to refresh stale content
|
||||
* just before displaying the notification. If the refresh fails or content
|
||||
* is not stale, the original content is returned.
|
||||
*
|
||||
* @param context Application context
|
||||
* @param content Original notification content
|
||||
* @return Updated content if refresh succeeded, original content otherwise
|
||||
*/
|
||||
private NotificationContent performJITFreshnessCheck(Context context, NotificationContent content) {
|
||||
try {
|
||||
// Check if content is stale (older than 6 hours for JIT check)
|
||||
long currentTime = System.currentTimeMillis();
|
||||
long age = currentTime - content.getFetchedAt();
|
||||
long staleThreshold = 6 * 60 * 60 * 1000; // 6 hours in milliseconds
|
||||
|
||||
if (age < staleThreshold) {
|
||||
Log.d(TAG, "Content is fresh (age: " + (age / 1000 / 60) + " minutes), skipping JIT refresh");
|
||||
return content;
|
||||
}
|
||||
|
||||
Log.i(TAG, "Content is stale (age: " + (age / 1000 / 60) + " minutes), attempting JIT refresh");
|
||||
|
||||
// Attempt to fetch fresh content
|
||||
DailyNotificationFetcher fetcher = new DailyNotificationFetcher(context, new DailyNotificationStorage(context));
|
||||
|
||||
// Attempt immediate fetch for fresh content
|
||||
NotificationContent freshContent = fetcher.fetchContentImmediately();
|
||||
|
||||
if (freshContent != null && freshContent.getTitle() != null && !freshContent.getTitle().isEmpty()) {
|
||||
Log.i(TAG, "JIT refresh succeeded, using fresh content");
|
||||
|
||||
// Update the original content with fresh data while preserving the original ID and scheduled time
|
||||
String originalId = content.getId();
|
||||
long originalScheduledTime = content.getScheduledTime();
|
||||
|
||||
content.setTitle(freshContent.getTitle());
|
||||
content.setBody(freshContent.getBody());
|
||||
content.setSound(freshContent.isSound());
|
||||
content.setPriority(freshContent.getPriority());
|
||||
content.setUrl(freshContent.getUrl());
|
||||
content.setMediaUrl(freshContent.getMediaUrl());
|
||||
content.setScheduledTime(originalScheduledTime); // Preserve original scheduled time
|
||||
// Note: fetchedAt remains unchanged to preserve original fetch time
|
||||
|
||||
// Save updated content to storage
|
||||
DailyNotificationStorage storage = new DailyNotificationStorage(context);
|
||||
storage.saveNotificationContent(content);
|
||||
|
||||
return content;
|
||||
} else {
|
||||
Log.w(TAG, "JIT refresh failed or returned empty content, using original content");
|
||||
return content;
|
||||
}
|
||||
|
||||
} catch (Exception e) {
|
||||
Log.e(TAG, "Error during JIT freshness check", e);
|
||||
return content; // Return original content on error
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Display the notification to the user
|
||||
*
|
||||
* @param context Application context
|
||||
* @param content Notification content to display
|
||||
*/
|
||||
private void displayNotification(Context context, NotificationContent content) {
|
||||
try {
|
||||
Log.d(TAG, "Displaying notification: " + content.getId());
|
||||
|
||||
NotificationManager notificationManager =
|
||||
(NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE);
|
||||
|
||||
if (notificationManager == null) {
|
||||
Log.e(TAG, "NotificationManager not available");
|
||||
return;
|
||||
}
|
||||
|
||||
// Create notification builder
|
||||
NotificationCompat.Builder builder = new NotificationCompat.Builder(context, CHANNEL_ID)
|
||||
.setSmallIcon(android.R.drawable.ic_dialog_info)
|
||||
.setContentTitle(content.getTitle())
|
||||
.setContentText(content.getBody())
|
||||
.setPriority(getNotificationPriority(content.getPriority()))
|
||||
.setAutoCancel(true)
|
||||
.setCategory(NotificationCompat.CATEGORY_REMINDER);
|
||||
|
||||
// Add sound if enabled
|
||||
if (content.isSound()) {
|
||||
builder.setDefaults(NotificationCompat.DEFAULT_SOUND);
|
||||
}
|
||||
|
||||
// Add click action if URL is available
|
||||
if (content.getUrl() != null && !content.getUrl().isEmpty()) {
|
||||
Intent clickIntent = new Intent(Intent.ACTION_VIEW);
|
||||
clickIntent.setData(android.net.Uri.parse(content.getUrl()));
|
||||
|
||||
PendingIntent clickPendingIntent = PendingIntent.getActivity(
|
||||
context,
|
||||
content.getId().hashCode(),
|
||||
clickIntent,
|
||||
PendingIntent.FLAG_UPDATE_CURRENT | PendingIntent.FLAG_IMMUTABLE
|
||||
);
|
||||
|
||||
builder.setContentIntent(clickPendingIntent);
|
||||
}
|
||||
|
||||
// Add dismiss action
|
||||
Intent dismissIntent = new Intent(context, DailyNotificationReceiver.class);
|
||||
dismissIntent.setAction("org.timesafari.daily.DISMISS");
|
||||
dismissIntent.putExtra(EXTRA_NOTIFICATION_ID, content.getId());
|
||||
|
||||
PendingIntent dismissPendingIntent = PendingIntent.getBroadcast(
|
||||
context,
|
||||
content.getId().hashCode() + 1000, // Different request code
|
||||
dismissIntent,
|
||||
PendingIntent.FLAG_UPDATE_CURRENT | PendingIntent.FLAG_IMMUTABLE
|
||||
);
|
||||
|
||||
builder.addAction(
|
||||
android.R.drawable.ic_menu_close_clear_cancel,
|
||||
"Dismiss",
|
||||
dismissPendingIntent
|
||||
);
|
||||
|
||||
// Build and display notification
|
||||
int notificationId = content.getId().hashCode();
|
||||
notificationManager.notify(notificationId, builder.build());
|
||||
|
||||
Log.i(TAG, "Notification displayed successfully: " + content.getId());
|
||||
|
||||
} catch (Exception e) {
|
||||
Log.e(TAG, "Error displaying notification", e);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Schedule the next occurrence of this daily notification
|
||||
*
|
||||
* Uses centralized NotifyReceiver.scheduleExactNotification() with ROLLOVER_ON_FIRE source
|
||||
* to ensure idempotence and proper logging
|
||||
*
|
||||
* @param context Application context
|
||||
* @param content Current notification content
|
||||
*/
|
||||
private void scheduleNextNotification(Context context, NotificationContent content) {
|
||||
try {
|
||||
Log.d(TAG, "Scheduling next notification for: " + content.getId());
|
||||
|
||||
// Extract scheduleId from notificationId pattern or use fallback
|
||||
// Notification IDs are often "daily_${scheduleId}"
|
||||
String scheduleId = null;
|
||||
String cronExpression = null;
|
||||
long nextScheduledTime = content.getScheduledTime() + (24 * 60 * 60 * 1000);
|
||||
|
||||
// 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
|
||||
} else {
|
||||
scheduleId = "daily_rollover_" + System.currentTimeMillis();
|
||||
}
|
||||
|
||||
// Calculate cron from current scheduled time (extract hour:minute)
|
||||
try {
|
||||
java.util.Calendar cal = java.util.Calendar.getInstance();
|
||||
cal.setTimeInMillis(content.getScheduledTime());
|
||||
int hour = cal.get(java.util.Calendar.HOUR_OF_DAY);
|
||||
int minute = cal.get(java.util.Calendar.MINUTE);
|
||||
cronExpression = String.format("%d %d * * *", minute, hour);
|
||||
|
||||
// Recalculate next run time from cron (tomorrow at same time)
|
||||
nextScheduledTime = calculateNextRunTimeFromCron(cronExpression);
|
||||
} catch (Exception e) {
|
||||
Log.w(TAG, "Failed to calculate cron from scheduled time, using default", e);
|
||||
cronExpression = "0 9 * * *"; // Default to 9 AM
|
||||
}
|
||||
|
||||
// Create config for next notification
|
||||
org.timesafari.dailynotification.UserNotificationConfig config =
|
||||
new org.timesafari.dailynotification.UserNotificationConfig(
|
||||
true, // enabled
|
||||
cronExpression,
|
||||
content.getTitle() != null ? content.getTitle() : "Daily Notification",
|
||||
content.getBody(),
|
||||
content.isSound(),
|
||||
true, // vibration
|
||||
content.getPriority() != null ? content.getPriority() : "normal"
|
||||
);
|
||||
|
||||
// Use centralized scheduling function with ROLLOVER_ON_FIRE source
|
||||
org.timesafari.dailynotification.NotifyReceiver.scheduleExactNotification(
|
||||
context,
|
||||
nextScheduledTime,
|
||||
config,
|
||||
false, // isStaticReminder
|
||||
null, // reminderId
|
||||
scheduleId,
|
||||
org.timesafari.dailynotification.ScheduleSource.ROLLOVER_ON_FIRE,
|
||||
false // skipPendingIntentIdempotence – rollover path does not skip
|
||||
);
|
||||
|
||||
Log.i(TAG, "Next notification scheduled via centralized function: scheduleId=" + scheduleId);
|
||||
|
||||
} catch (Exception e) {
|
||||
Log.e(TAG, "Error scheduling next notification", e);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Helper to convert HH:mm time to cron expression
|
||||
*/
|
||||
private String convertTimeToCron(String clockTime) {
|
||||
try {
|
||||
String[] parts = clockTime.split(":");
|
||||
if (parts.length == 2) {
|
||||
int hour = Integer.parseInt(parts[0]);
|
||||
int minute = Integer.parseInt(parts[1]);
|
||||
return String.format("%d %d * * *", minute, hour);
|
||||
}
|
||||
} catch (Exception e) {
|
||||
Log.w(TAG, "Failed to parse clockTime: " + clockTime, e);
|
||||
}
|
||||
return "0 9 * * *"; // Default to 9 AM
|
||||
}
|
||||
|
||||
/**
|
||||
* Helper to calculate next run time from cron expression
|
||||
*/
|
||||
private long calculateNextRunTimeFromCron(String cron) {
|
||||
try {
|
||||
String[] parts = cron.trim().split("\\s+");
|
||||
if (parts.length >= 2) {
|
||||
int minute = Integer.parseInt(parts[0]);
|
||||
int hour = Integer.parseInt(parts[1]);
|
||||
|
||||
java.util.Calendar calendar = java.util.Calendar.getInstance();
|
||||
long now = calendar.getTimeInMillis();
|
||||
|
||||
calendar.set(java.util.Calendar.HOUR_OF_DAY, hour);
|
||||
calendar.set(java.util.Calendar.MINUTE, minute);
|
||||
calendar.set(java.util.Calendar.SECOND, 0);
|
||||
calendar.set(java.util.Calendar.MILLISECOND, 0);
|
||||
|
||||
long nextRun = calendar.getTimeInMillis();
|
||||
if (nextRun <= now) {
|
||||
calendar.add(java.util.Calendar.DAY_OF_YEAR, 1);
|
||||
nextRun = calendar.getTimeInMillis();
|
||||
}
|
||||
return nextRun;
|
||||
}
|
||||
} catch (Exception e) {
|
||||
Log.w(TAG, "Failed to calculate next run time from cron: " + cron, e);
|
||||
}
|
||||
// Fallback: 24 hours from now
|
||||
return System.currentTimeMillis() + (24 * 60 * 60 * 1000L);
|
||||
}
|
||||
|
||||
/**
|
||||
* Get notification priority constant
|
||||
*
|
||||
* @param priority Priority string from content
|
||||
* @return NotificationCompat priority constant
|
||||
*/
|
||||
private int getNotificationPriority(String priority) {
|
||||
if (priority == null) {
|
||||
return NotificationCompat.PRIORITY_DEFAULT;
|
||||
}
|
||||
|
||||
switch (priority.toLowerCase()) {
|
||||
case "high":
|
||||
return NotificationCompat.PRIORITY_HIGH;
|
||||
case "low":
|
||||
return NotificationCompat.PRIORITY_LOW;
|
||||
case "min":
|
||||
return NotificationCompat.PRIORITY_MIN;
|
||||
case "max":
|
||||
return NotificationCompat.PRIORITY_MAX;
|
||||
default:
|
||||
return NotificationCompat.PRIORITY_DEFAULT;
|
||||
}
|
||||
}
|
||||
// handleNotificationIntent/performJITFreshnessCheck/displayNotification/
|
||||
// scheduleNextNotification/convertTimeToCron/calculateNextRunTimeFromCron/
|
||||
// getNotificationPriority were removed here (PLAN-simplify-static-and-refactor.md §3.6):
|
||||
// onReceive() only ever calls enqueueNotificationWork()/enqueueDismissalWork() above, so
|
||||
// this entire chain — including scheduleNextNotification's "daily_rollover_<timestamp>"
|
||||
// orphan-id fallback (generator #4, the other source of stray uncancellable schedules) —
|
||||
// was confirmed dead (zero callers) and fully superseded by DailyNotificationWorker.java's
|
||||
// live equivalents.
|
||||
|
||||
/**
|
||||
* Handle notification dismissal
|
||||
|
||||
@@ -572,7 +572,19 @@ public class DailyNotificationWorker extends Worker {
|
||||
Integer rolloverMinutes = null;
|
||||
if (logicalScheduleIdForRollover != null && !logicalScheduleIdForRollover.isEmpty()) {
|
||||
org.timesafari.dailynotification.Schedule s = org.timesafari.dailynotification.ScheduleHelper.getScheduleBlocking(getApplicationContext(), logicalScheduleIdForRollover);
|
||||
if (s != null && s.getRolloverIntervalMinutes() != null && s.getRolloverIntervalMinutes() > 0) {
|
||||
// Cancellation guard: if the user cancelled (row deleted, see cancelDailyReminder's
|
||||
// deleteById) or disabled this schedule since the alarm fired, stop here rather than
|
||||
// unconditionally re-arming the next occurrence. This closes the race described in
|
||||
// PLAN-simplify-static-and-refactor.md §3.4 — without this check, scheduleNextNotification
|
||||
// always re-armed regardless of cancellation state, which is why "cancel" didn't
|
||||
// actually stop notifications that were already in-flight when cancelled.
|
||||
if (s == null || !s.getEnabled()) {
|
||||
Log.i(TAG, "DN|RESCHEDULE_SKIP_CANCELLED id=" + content.getId()
|
||||
+ " scheduleId=" + logicalScheduleIdForRollover
|
||||
+ " reason=" + (s == null ? "deleted" : "disabled"));
|
||||
return;
|
||||
}
|
||||
if (s.getRolloverIntervalMinutes() != null && s.getRolloverIntervalMinutes() > 0) {
|
||||
rolloverMinutes = s.getRolloverIntervalMinutes();
|
||||
Log.d(TAG, "DN|ROLLOVER_INTERVAL scheduleId=" + logicalScheduleIdForRollover + " minutes=" + rolloverMinutes);
|
||||
}
|
||||
@@ -612,9 +624,18 @@ public class DailyNotificationWorker extends Worker {
|
||||
String notificationId = content.getId();
|
||||
String scheduleId = scheduleIdForRollover;
|
||||
if (scheduleId == null || scheduleId.isEmpty()) {
|
||||
scheduleId = "daily_rollover_" + System.currentTimeMillis();
|
||||
// Previously fell back to a freshly-minted "daily_rollover_<timestamp>" id here,
|
||||
// which created an orphan schedule row every time it triggered — itself a source
|
||||
// of the "stray schedule that survives cancellation" bug (generator #4 in
|
||||
// PLAN-simplify-static-and-refactor.md §2). Per §3.6, this branch should be
|
||||
// unreachable for static reminders (NotifyReceiver.scheduleExactNotification
|
||||
// always sets the schedule_id intent extra), so fail loudly instead of silently
|
||||
// spawning an untrackable, uncancellable orphan alarm.
|
||||
Log.e(TAG, "DN|RESCHEDULE_ABORT id=" + content.getId()
|
||||
+ " reason=no_resolvable_schedule_id - not re-arming to avoid creating an orphan schedule");
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
// When using rollover interval, next time already set; otherwise compute from cron (tomorrow same time)
|
||||
if (rolloverMinutes == null || rolloverMinutes <= 0) {
|
||||
try {
|
||||
|
||||
@@ -87,7 +87,7 @@ data class History(
|
||||
NotificationDeliveryEntity::class,
|
||||
NotificationConfigEntity::class
|
||||
],
|
||||
version = 4, // 4: content_cache.cacheScope
|
||||
version = 5, // 5: isolate static reminder rows into kind="static_reminder" (was "notify")
|
||||
exportSchema = false
|
||||
)
|
||||
@TypeConverters(Converters::class)
|
||||
@@ -122,7 +122,7 @@ abstract class DailyNotificationDatabase : RoomDatabase() {
|
||||
DailyNotificationDatabase::class.java,
|
||||
DATABASE_NAME
|
||||
)
|
||||
.addMigrations(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4)
|
||||
.addMigrations(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4, MIGRATION_4_5)
|
||||
.addCallback(roomCallback)
|
||||
.build()
|
||||
INSTANCE = instance
|
||||
@@ -288,6 +288,31 @@ abstract class DailyNotificationDatabase : RoomDatabase() {
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Isolate the static reminder's row into its own "static_reminder" kind, away from the
|
||||
* shared "notify" kind (see PLAN-simplify-static-and-refactor.md §3.1, §3.7).
|
||||
*
|
||||
* The static reminder's id is caller-supplied (TimeSafari uses "daily_timesafari_reminder";
|
||||
* the plugin defaults to "daily_notification" if none is given), so we can't match it by a
|
||||
* fixed id. Instead, reclassify by elimination: any existing kind="notify" row that does
|
||||
* NOT match one of the three other generators' known id prefixes
|
||||
* (dual_notify_<ts>, notify_<ts>, daily_rollover_<ts>) must be the static reminder, since
|
||||
* those are the only generators that ever wrote kind="notify" rows.
|
||||
*/
|
||||
val MIGRATION_4_5 = object : Migration(4, 5) {
|
||||
override fun migrate(database: SupportSQLiteDatabase) {
|
||||
database.execSQL(
|
||||
"""
|
||||
UPDATE schedules SET kind = 'static_reminder'
|
||||
WHERE kind = 'notify'
|
||||
AND id NOT LIKE 'dual_notify_%'
|
||||
AND id NOT LIKE 'notify_%'
|
||||
AND id NOT LIKE 'daily_rollover_%'
|
||||
""".trimIndent()
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -435,7 +435,12 @@ class NotifyReceiver : BroadcastReceiver() {
|
||||
// nextRunAt with the rollover time and can leave the app's alarm in a bad state.
|
||||
if (scheduleToUpdate == null && !stableScheduleId.startsWith("daily_rollover_")) {
|
||||
val allSchedules = db.scheduleDao().getAll()
|
||||
scheduleToUpdate = allSchedules.firstOrNull { it.kind == "notify" && it.enabled }
|
||||
// Includes "static_reminder" so this fallback can find the real static
|
||||
// reminder row (now isolated from "notify") instead of falling through to
|
||||
// the "create new" branch below and spawning a duplicate row.
|
||||
scheduleToUpdate = allSchedules.firstOrNull {
|
||||
(it.kind == "notify" || it.kind == "static_reminder") && it.enabled
|
||||
}
|
||||
}
|
||||
|
||||
// Calculate cron expression from trigger time (HH:mm format)
|
||||
|
||||
@@ -117,26 +117,26 @@ class ReactivationManager(private val context: Context) {
|
||||
enabledSchedules.forEach { schedule ->
|
||||
try {
|
||||
when (schedule.kind) {
|
||||
"notify" -> {
|
||||
"notify", "static_reminder" -> {
|
||||
val nextRunTime = calculateNextRunTimeForSchedule(schedule, currentTime)
|
||||
|
||||
|
||||
if (nextRunTime < currentTime) {
|
||||
// Past alarm - mark as missed and schedule next occurrence
|
||||
markMissedNotificationForSchedule(schedule, nextRunTime, db)
|
||||
missedCount++
|
||||
|
||||
|
||||
// Schedule next occurrence (use rollover interval if set, else 24h)
|
||||
val nextOccurrence = calculateNextOccurrenceForSchedule(schedule, nextRunTime, currentTime)
|
||||
rescheduleAlarmForBoot(context, schedule, nextOccurrence, db)
|
||||
rescheduledCount++
|
||||
|
||||
|
||||
Log.i(TAG, "Marked missed notification: ${schedule.id}")
|
||||
Log.i(TAG, "Rescheduled alarm: ${schedule.id} for $nextOccurrence")
|
||||
} else {
|
||||
// Future alarm - reschedule immediately
|
||||
rescheduleAlarmForBoot(context, schedule, nextRunTime, db)
|
||||
rescheduledCount++
|
||||
|
||||
|
||||
Log.i(TAG, "Rescheduled alarm: ${schedule.id} for $nextRunTime")
|
||||
}
|
||||
}
|
||||
@@ -627,7 +627,7 @@ class ReactivationManager(private val context: Context) {
|
||||
// Step 3: Verify and reschedule future alarms
|
||||
val schedules = try {
|
||||
db.scheduleDao().getEnabled()
|
||||
.filter { it.kind == "notify" }
|
||||
.filter { it.kind == "notify" || it.kind == "static_reminder" }
|
||||
} catch (e: Exception) {
|
||||
Log.e(TAG, "Failed to query schedules", e)
|
||||
emptyList()
|
||||
@@ -731,7 +731,7 @@ class ReactivationManager(private val context: Context) {
|
||||
}
|
||||
|
||||
when (schedule.kind) {
|
||||
"notify" -> {
|
||||
"notify", "static_reminder" -> {
|
||||
val result = recoverNotifySchedule(schedule, currentTime, db)
|
||||
missedCount += result.missedCount
|
||||
rescheduledCount += result.rescheduledCount
|
||||
|
||||
461
doc/_archive/PLAN-simplify-static-and-refactor.md
Normal file
461
doc/_archive/PLAN-simplify-static-and-refactor.md
Normal file
@@ -0,0 +1,461 @@
|
||||
# Plan: Simplify Static Reminder Path & Remove Dead Notify Generators
|
||||
|
||||
**Purpose:** Fix the Android bug where cancelling a static daily reminder
|
||||
doesn't stop it from firing, by giving static reminders their own isolated
|
||||
schedule namespace — and remove now-unneeded legacy code along the way.
|
||||
|
||||
**Status:** Ready for execution
|
||||
**Date:** 2026-06-07
|
||||
**Platform:** Android only (the reported bug is Android-specific; iOS was not
|
||||
investigated — see "iOS" section below before assuming parity)
|
||||
|
||||
---
|
||||
|
||||
## 1. The bug this fixes
|
||||
|
||||
In the TimeSafari PWA (`crowd-funder-for-time-pwa`), turning off the daily
|
||||
reminder (`AccountViewView.vue` → `NativeNotificationService.cancelDailyNotification()`
|
||||
→ plugin `cancelDailyReminder`) reports success, but the notification **keeps
|
||||
firing every day**.
|
||||
|
||||
`cancelDailyReminder` (`DailyNotificationPlugin.kt:754-779`) only:
|
||||
1. Cancels the AlarmManager alarm for one specific `scheduleId` (the app's
|
||||
stable id, `"daily_timesafari_reminder"`), and
|
||||
2. Sets `enabled = false` for that one row in the `schedules` table.
|
||||
|
||||
By contrast, `scheduleDailyNotification` (`DailyNotificationPlugin.kt:1096-1195`)
|
||||
defensively sweeps the **entire** `schedules` table via
|
||||
`ScheduleHelper.cleanupExistingNotificationSchedules()` (kills every
|
||||
`kind == "notify"` row except the one being created) and cancels
|
||||
prefetch/fetch WorkManager jobs via `cancelFetchRelatedWorkManagerJobs()` —
|
||||
explicitly because, per its own comments, other code paths can spin up a
|
||||
"second (UUID) alarm" that fires duplicate/stray notifications.
|
||||
|
||||
So: if **any** stray `notify`-kind schedule exists with an ID other than
|
||||
`"daily_timesafari_reminder"`, cancellation completely ignores it, and it
|
||||
keeps re-arming itself and firing forever — because the rollover/rearm logic
|
||||
(`DailyNotificationWorker.scheduleNextNotification`,
|
||||
`DailyNotificationReceiver.scheduleNextNotification`) re-arms the *next*
|
||||
occurrence unconditionally on every fire, **never checking the `enabled` flag**.
|
||||
|
||||
---
|
||||
|
||||
## 2. Why the defensive sweeping exists (and why it can be deleted)
|
||||
|
||||
The `schedules` table (`DatabaseSchema.kt:41-55`) is shared by *four* different
|
||||
generators, distinguished only by a free-text `kind` column and by ID-prefix
|
||||
*conventions* that are not enforced anywhere:
|
||||
|
||||
| # | Generator | Schedule id pattern | Where | Used by TimeSafari? |
|
||||
|---|---|---|---|---|
|
||||
| 1 | **Static reminder** (the only thing TimeSafari uses) | caller-provided `id`, e.g. `"daily_timesafari_reminder"` | `scheduleDailyNotification`/`scheduleDailyReminder` → `ScheduleHelper.scheduleDailyNotification` (`DailyNotificationPlugin.kt:2743-2826`) | **Yes — the only path in use** |
|
||||
| 2 | **Dual/API-retrieval** (planned, not yet used) | `dual_fetch_<ts>` / `dual_notify_<ts>` | `scheduleDualNotification` → `ScheduleHelper.scheduleDualNotification` (`DailyNotificationPlugin.kt:2890-2936`) + `DualScheduleNotifyScheduler.scheduleChainedNotifyAlarm` (`DualScheduleNotifyScheduler.kt:19`) | No (planned for future) |
|
||||
| 3 | **Standalone user notification** (legacy, separate public API) | `notify_<ts>` | `scheduleUserNotification` plugin method (`DailyNotificationPlugin.kt:1364`) → `ScheduleHelper.scheduleUserNotification` (`DailyNotificationPlugin.kt:2949-2980`) | **No** — not called anywhere in `crowd-funder-for-time-pwa/src`; only referenced in the plugin's own docs/examples |
|
||||
| 4 | **Orphan rollover fallback** (accidental, not a real feature) | `daily_rollover_<ts>` (regenerated fresh — never stabilizes) | Inline fallback inside `DailyNotificationReceiver.scheduleNextNotification` (`DailyNotificationReceiver.java:416`) and `DailyNotificationWorker.scheduleNextNotification` (`DailyNotificationWorker.java:614-616`), triggered when the rearm code can't resolve a stable schedule id from intent extras/content id | **No** — pure bug-patch byproduct |
|
||||
|
||||
Generators #1 and #2 are real, intentional features (static reminders are live;
|
||||
dual/API-retrieval is planned). Generators #3 and #4 are **not** part of either
|
||||
of those — they are independent legacy/accidental code:
|
||||
|
||||
- **#3 (`scheduleUserNotification`)** is its own standalone public plugin
|
||||
method, parallel to (not called by) the dual flow. Confirmed unused by the
|
||||
app — eliminable as dead surface area in its own right.
|
||||
- **#4 (`daily_rollover_*` fallback)** is not tied to any feature; it's a
|
||||
brittle fallback branch inside the *shared* rearm logic that can fire for
|
||||
*any* notify schedule when ID-threading fails, and it spins off a fresh
|
||||
unstable orphan id every time it triggers — i.e. it is itself a source of
|
||||
the exact "stray schedule" problem that breaks cancellation.
|
||||
|
||||
**Conclusion:** the cleanup-sweep machinery in `scheduleDailyNotification`
|
||||
exists to paper over collisions between these four generators sharing one
|
||||
table/AlarmManager namespace. If the static reminder (the only feature
|
||||
TimeSafari actually uses today) is moved into its **own isolated `kind`** with
|
||||
a single, always-known, stable id, none of that sweeping is needed for it —
|
||||
scheduling and cancelling become trivial single-row operations, and the
|
||||
"cancel doesn't stop it" bug is structurally impossible (there is nothing else
|
||||
to collide with).
|
||||
|
||||
---
|
||||
|
||||
## 3. Refactor plan
|
||||
|
||||
### 3.1 Give static reminders their own `kind`
|
||||
|
||||
- Change the `kind` value used for static reminders from `"notify"` to a new,
|
||||
distinct value, e.g. `"static_reminder"`.
|
||||
- Touch points (search for `kind = "notify"` and `isStaticReminder`):
|
||||
- `ScheduleHelper.scheduleDailyNotification` (`DailyNotificationPlugin.kt:2784`,
|
||||
`Schedule(... kind = "notify" ...)`) → change to `"static_reminder"`.
|
||||
- Any `getByKind("notify")` / `getByKindAndEnabled("notify", ...)` calls that
|
||||
are meant to operate on static reminders specifically (audit all call
|
||||
sites at `DailyNotificationPlugin.kt:702, 727-728, 1661, 1722, 2716-2717,
|
||||
2852, 3079, 3283, 3288` — most of these are for the dual/fetch machinery
|
||||
and should stay `"notify"`/`"fetch"`; only the ones that exist purely to
|
||||
find/clean the static reminder's row need to move to `"static_reminder"`,
|
||||
and after this refactor most of them become unnecessary for the static
|
||||
path — see 3.2).
|
||||
- This alone makes `cleanupExistingNotificationSchedules(excludeScheduleId)`
|
||||
(which filters `getByKind("notify")`) blind to the static reminder row,
|
||||
which is what we want — it should never need to "clean up" the one
|
||||
legitimate static reminder.
|
||||
|
||||
### 3.2 Simplify scheduling for static reminders
|
||||
|
||||
Replace the sweep-and-recreate dance in `scheduleDailyNotification`
|
||||
(`DailyNotificationPlugin.kt:1130-1190`) with a direct upsert-in-place:
|
||||
|
||||
- Remove the call to `ScheduleHelper.cleanupExistingNotificationSchedules()`
|
||||
(`DailyNotificationPlugin.kt:1140-1150`) for the static-reminder path — it
|
||||
exists to prevent collisions with *other kinds*; with its own `kind`, the
|
||||
static reminder can never collide.
|
||||
- Remove the call to `ScheduleHelper.cancelFetchRelatedWorkManagerJobs()`
|
||||
(`DailyNotificationPlugin.kt:1155-1158`) for the static-reminder path —
|
||||
static reminders never enqueue prefetch/fetch work in the first place
|
||||
(already explicitly skipped, see comment at `DailyNotificationPlugin.kt:2775-2779`),
|
||||
so there is nothing to cancel.
|
||||
- Keep `NotifyReceiver.cancelNotification(context, scheduleId)` +
|
||||
`scheduleExactNotification(...)` (the actual alarm arm/rearm) — that part is
|
||||
correct and necessary (it ensures "one alarm per id" via stable requestCode).
|
||||
|
||||
### 3.3 Simplify cancellation for static reminders
|
||||
|
||||
Rewrite `cancelDailyReminder` (`DailyNotificationPlugin.kt:754-779`) to be a
|
||||
pure single-row operation, since with its own `kind` there is now guaranteed
|
||||
to be at most one row to deal with:
|
||||
|
||||
```kotlin
|
||||
@PluginMethod
|
||||
fun cancelDailyReminder(call: PluginCall) {
|
||||
try {
|
||||
val reminderId = call.getString("reminderId")
|
||||
?: call.getString("id")
|
||||
?: return call.reject("cancelDailyReminder: missing reminderId")
|
||||
|
||||
NotifyReceiver.cancelNotification(context, scheduleId = reminderId)
|
||||
|
||||
kotlinx.coroutines.runBlocking {
|
||||
val db = getDatabase()
|
||||
db.scheduleDao().deleteById(reminderId) // or setEnabled(false) — see note
|
||||
db.notificationContentDao().deleteById(reminderId) // if such a method exists; else equivalent cleanup
|
||||
}
|
||||
call.resolve()
|
||||
} catch (e: Exception) {
|
||||
Log.e(TAG, "cancelDailyReminder failed", e)
|
||||
call.reject("cancelDailyReminder failed: ${e.message}")
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Note: decide `deleteById` vs `setEnabled(id, false)`:
|
||||
- `deleteById` is cleaner (no stale rows ever accumulate) but means
|
||||
`BootReceiver`'s `getEnabled()` sweep simply won't find the row — which is
|
||||
correct (nothing to reschedule).
|
||||
- `setEnabled(id, false)` keeps history/diagnostics but requires every rearm
|
||||
path to check `enabled` before re-arming (see 3.4) — more moving parts.
|
||||
- **Recommendation: `deleteById`.** It's the simplest correct option and
|
||||
matches "cancel means gone," not "cancel means disabled-but-present."
|
||||
|
||||
### 3.4 Make the rearm chain check for cancellation
|
||||
|
||||
Even with `deleteById`, there is a narrow race: the alarm could already be
|
||||
in-flight (fired, worker running) at the moment the user cancels. Today,
|
||||
`scheduleNextNotification` in both `DailyNotificationWorker.java:547-667` and
|
||||
`DailyNotificationReceiver.java:401-...` (the latter looks unreachable — see
|
||||
3.6) **unconditionally** re-arms the next occurrence with no DB check.
|
||||
|
||||
Add a guard at the top of `DailyNotificationWorker.scheduleNextNotification`:
|
||||
look up the schedule row by id (`ScheduleHelper.getScheduleBlocking`); if it's
|
||||
missing (deleted) or `enabled == false`, log and return without re-arming.
|
||||
This closes the cancel race definitively and is a small, isolated change.
|
||||
|
||||
### 3.5 Remove generator #3 — `scheduleUserNotification`
|
||||
|
||||
**Confirmed unused in any executable path** — not called anywhere in
|
||||
`crowd-funder-for-time-pwa/src`, and the only "usages" in the plugin repo
|
||||
itself are inside doc/example files
|
||||
(`test-apps/daily-notification-test/docs/VUE3_NOTIFICATION_IMPLEMENTATION_GUIDE.md`,
|
||||
`examples/timesafari-integration-example.ts`) — not actual test-app source or
|
||||
test suites.
|
||||
|
||||
**Important nuance found on closer look:** those two doc/example usages both
|
||||
call it as a one-shot, event-triggered notification —
|
||||
|
||||
```typescript
|
||||
await DailyNotification.scheduleUserNotification({
|
||||
schedule: 'immediate',
|
||||
title: change.title,
|
||||
body: change.message,
|
||||
...
|
||||
})
|
||||
```
|
||||
|
||||
— i.e. "something happened in the app right now, show a notification." That's
|
||||
a *third* semantic, distinct from both #1 (recurring static daily reminder)
|
||||
and #2 (recurring fetch-then-notify cycle). **But it's also non-functional**:
|
||||
the real implementation has no special case for `schedule: 'immediate'`.
|
||||
`parseUserNotificationConfig` (`DailyNotificationPlugin.kt:2475-2484`) passes
|
||||
the `schedule` string straight through, and
|
||||
`ScheduleHelper.scheduleUserNotification` runs it through
|
||||
`calculateNextRunTime` → `ScheduleCronUtils.calculateNextRunTimeMillis(schedule)`
|
||||
(`DailyNotificationPlugin.kt:2487-2489`) — a **cron parser** that has no
|
||||
concept of `"immediate"`. So even the one documented usage pattern would fail
|
||||
or misbehave if anyone actually tried it. The docs describe a capability that
|
||||
was never built; `scheduleUserNotification` itself is just a thin "schedule
|
||||
one bare cron-based notification alarm" primitive that nothing exercises.
|
||||
|
||||
**Recommendation: remove it, and do not resurrect the same capability under
|
||||
the same API by patching in `'immediate'` support.** If "notify the user
|
||||
immediately when the app detects an event" is still wanted later, design it
|
||||
as its own clearly-named one-shot API (e.g. `notifyNow(options)` /
|
||||
`showImmediateNotification(options)`) with **no `schedule`/cron field at
|
||||
all** — just `title`/`body`/optional `triggerAtMillis` (default "now"),
|
||||
routed through a "display once, never rearm" path. That keeps the three
|
||||
concerns (recurring static reminder / recurring fetch+notify cycle / one-shot
|
||||
event notification) distinguished by API *shape*, not by a magic string a
|
||||
cron parser silently chokes on.
|
||||
|
||||
**[DONE 2026-06-19]** Removed from Android only:
|
||||
- `@PluginMethod fun scheduleUserNotification` (was `DailyNotificationPlugin.kt:1363-1411`)
|
||||
- `ScheduleHelper.scheduleUserNotification` (was `DailyNotificationPlugin.kt:2899-2935`)
|
||||
- No separate `methods.append(...)` registration existed — `@PluginMethod` annotation
|
||||
is the only registration mechanism in this codebase, so nothing else to clean up there.
|
||||
- Verified via `./gradlew :daily-notification-plugin:compileDebugKotlin` — compiles clean,
|
||||
no other call sites referenced either removed symbol.
|
||||
|
||||
**Scope correction — do NOT remove the TS/iOS layer.** `scheduleUserNotification` in
|
||||
`src/definitions.ts:543` / `src/web.ts:231` is a **cross-platform** method signature,
|
||||
and on iOS it is genuinely load-bearing: `DailyNotificationScheduleHelper.scheduleDualNotification`
|
||||
(`ios/Plugin/DailyNotificationScheduleHelper.swift:98-107`) takes a
|
||||
`scheduleUserNotification` closure that's bound to the real
|
||||
`@objc func scheduleUserNotification` (`ios/Plugin/DailyNotificationPlugin.swift:379`) —
|
||||
i.e. on iOS this *is* the dual flow's notification half, not a dead generator.
|
||||
Android's version was a standalone, never-wired-up duplicate of that concept
|
||||
(Android's actual dual flow uses `scheduleDualNotification`/`DualScheduleNotifyScheduler`
|
||||
instead and never called `ScheduleHelper.scheduleUserNotification`) — so removing it
|
||||
from Android only does not reduce iOS capability. Leave:
|
||||
- `src/definitions.ts`/`src/web.ts` — keep the TS signature
|
||||
- README section `#### scheduleUserNotification(config)` (lines ~295-306) — already
|
||||
documents the correct cron-based shape (`schedule: string; // Cron expression`),
|
||||
no `'immediate'` mentioned, nothing broken here
|
||||
- iOS Swift implementation — untouched, see
|
||||
[`PLAN-ios-static-reminder-fix.md`](./PLAN-ios-static-reminder-fix.md) §7 for why
|
||||
|
||||
Still worth doing (low-priority, independent of this fix, not yet done): strip or
|
||||
correct the broken `schedule: 'immediate'` example in
|
||||
`test-apps/daily-notification-test/docs/VUE3_NOTIFICATION_IMPLEMENTATION_GUIDE.md:226-240`
|
||||
and `examples/timesafari-integration-example.ts:101-109` — that pattern was never
|
||||
implemented on either platform's actual cron parser, regardless of the Android
|
||||
generator-#3 cleanup above.
|
||||
|
||||
This eliminates the Android-only `notify_<ts>` id convention entirely — one less
|
||||
thing that can collide in the shared `schedules` table — without touching the
|
||||
cross-platform API surface or iOS's dual-flow capability.
|
||||
|
||||
### 3.6 Remove generator #4 — `daily_rollover_*` fallback (and dead code around it)
|
||||
|
||||
- In `DailyNotificationWorker.scheduleNextNotification`
|
||||
(`DailyNotificationWorker.java:556-563, 613-616`): once static reminders
|
||||
always carry a known, stable `schedule_id` intent extra (true today —
|
||||
`NotifyReceiver.scheduleExactNotification` always sets it,
|
||||
`NotifyReceiver.kt:297`), the `scheduleIdForRollover == null` branch should
|
||||
be unreachable for the static-reminder path. After confirming (via logging
|
||||
or a temporary assertion in a debug build) that it never fires for static
|
||||
reminders, delete the `"daily_rollover_" + System.currentTimeMillis()`
|
||||
fallback branches and the `getCanonicalRolloverScheduleBlocking` resolution
|
||||
hack (`DailyNotificationWorker.java:564-571`,
|
||||
`ScheduleHelper.getCanonicalRolloverScheduleBlocking` at
|
||||
`DailyNotificationPlugin.kt:2844-2853`) that exists solely to paper over it.
|
||||
- `DailyNotificationReceiver.handleNotificationIntent` /
|
||||
`scheduleNextNotification` (`DailyNotificationReceiver.java:211-...,
|
||||
401-461`) appear to be **dead code** — `onReceive` only calls
|
||||
`enqueueNotificationWork`/`enqueueDismissalWork` (`DailyNotificationReceiver.java:62-84`),
|
||||
never `handleNotificationIntent`. Confirm with a repo-wide search for
|
||||
callers, then delete the whole block (it duplicates, slightly differently,
|
||||
what `DailyNotificationWorker` does — a second source of the same
|
||||
`daily_rollover_*` bug pattern).
|
||||
|
||||
### 3.7 Clean up DB after refactor
|
||||
|
||||
- Add a one-time migration (or lazy cleanup on plugin init) that deletes any
|
||||
pre-existing `notify_<ts>` and `daily_rollover_<ts>` rows from `schedules`
|
||||
(and their `notification_content` counterparts) — these are exactly the
|
||||
kind of orphan that causes the bug, and existing installs may already have
|
||||
them sitting in the DB from earlier buggy runs.
|
||||
- Re-point the existing `"daily_notification"` / app-supplied static reminder
|
||||
row to `kind = "static_reminder"` in that same migration (`UPDATE schedules
|
||||
SET kind = 'static_reminder' WHERE kind = 'notify' AND id = :appReminderId`,
|
||||
or simpler: `WHERE id NOT LIKE 'dual_%' AND id NOT LIKE 'notify_%' AND id NOT
|
||||
LIKE 'daily_rollover_%'` — i.e. whatever's left after excluding known
|
||||
dual/legacy/orphan patterns is the static reminder).
|
||||
|
||||
---
|
||||
|
||||
## 4. What NOT to touch (dual/API-retrieval flow — planned for future use)
|
||||
|
||||
Per the user: the dual/API-retrieval flow ("device wakes by timer or push →
|
||||
calls API → stores content → reschedules next check → OS later wakes app to
|
||||
show stored content") is **not used today but is planned**. Leave generator #2
|
||||
and its supporting machinery alone:
|
||||
- `scheduleDualNotification`, `ScheduleHelper.scheduleDualNotification`
|
||||
(`DailyNotificationPlugin.kt:2890-2936`)
|
||||
- `DualScheduleNotifyScheduler` (`DualScheduleNotifyScheduler.kt`)
|
||||
- `FetchWorker`, `DualScheduleFetchRecovery`, content-cache (`cacheScope =
|
||||
"dual"`), circuit breakers, etc.
|
||||
|
||||
These keep using `kind = "notify"`/`"fetch"` and the existing sweep machinery
|
||||
— which is fine, since that machinery's whole job is to keep *that* subsystem
|
||||
internally consistent. The point of 3.1 is just to make sure the static
|
||||
reminder's single row is no longer inside that sweep's blast radius (and vice
|
||||
versa).
|
||||
|
||||
---
|
||||
|
||||
## 5. iOS
|
||||
|
||||
**Investigated — see [`PLAN-ios-static-reminder-fix.md`](./PLAN-ios-static-reminder-fix.md)
|
||||
for the full analysis and fix plan. Summary:**
|
||||
|
||||
iOS does **not** have an analogous shared-table-with-multiple-generators
|
||||
design (no Room/SQLite `schedules` table, no AlarmManager, no rearm chain —
|
||||
it uses OS-native `UNUserNotificationCenter` with `UNCalendarNotificationTrigger`,
|
||||
which natively supports "repeat forever" and needs none of that machinery).
|
||||
|
||||
But it has its own, structurally different bug with the same user-visible
|
||||
symptom: the client's `scheduleDailyNotification`/`cancelDailyReminder` calls
|
||||
land in **two unrelated, non-communicating subsystems** inside
|
||||
`DailyNotificationPlugin.swift` —
|
||||
- `scheduleDailyNotification` (:1392) registers under a fresh
|
||||
`content.id = "daily_<timestamp>"` each call, with `repeats: false`
|
||||
(likely **one-shot, not actually recurring** — see the linked plan's §4)
|
||||
- `cancelDailyReminder` (:1174) only ever removes `"reminder_\(id)"`-shaped
|
||||
identifiers — which the schedule path above never registers, so cancel is a
|
||||
**silent no-op** that still reports success
|
||||
|
||||
Meanwhile a *third*, already-correct subsystem
|
||||
(`scheduleDailyReminder`/`cancelDailyReminder`/`updateDailyReminder`, :1088-1300)
|
||||
sits unused right next to it — a single `repeats: true` calendar trigger under
|
||||
a stable caller-supplied id, with matching cancel. The fix is to repoint
|
||||
`scheduleDailyNotification` onto that existing correct mechanism rather than
|
||||
to build Android-style sweep/rearm machinery iOS doesn't need.
|
||||
|
||||
This is a fully independent fix (different files, different language, no
|
||||
shared code) — see the linked plan's §5 for the side-by-side comparison.
|
||||
Execute in either order, or in parallel.
|
||||
|
||||
---
|
||||
|
||||
## 6. Verification plan
|
||||
|
||||
1. **Unit/integration**: existing plugin test suite (`npm test`) plus new
|
||||
tests asserting:
|
||||
- Scheduling a static reminder creates exactly one `schedules` row with
|
||||
`kind = "static_reminder"`.
|
||||
- Cancelling deletes that row and cancels its `PendingIntent`
|
||||
(`isAlarmScheduled` returns false afterward).
|
||||
- No `notify_<ts>`/`dual_*`/`daily_rollover_*` rows are created or touched
|
||||
by the static-reminder schedule/cancel/rearm cycle.
|
||||
- **Note:** `grep`ing `android/src/test/java/.../*.kt` today shows **zero**
|
||||
existing coverage of `cancelDailyReminder`, `scheduleNextNotification`, or
|
||||
`cleanupExistingNotificationSchedules` — this bug had a test-coverage
|
||||
blind spot. Add a regression test asserting "after `cancelDailyReminder`,
|
||||
`scheduleDao().getByKind('notify')` (or `'static_reminder'` post-3.1)
|
||||
returns zero rows" so this can't silently regress.
|
||||
|
||||
2. **Manual on-device (Android emulator) — reproduce-then-verify sequence**:
|
||||
|
||||
This environment already has `adb`, `emulator`, `gradle`, and two AVDs
|
||||
(`Pixel_3a_API_34_extension_level_7_arm64-v8a`, `Pixel_7`) plus a mature
|
||||
ADB-based toolkit in `test-apps/android-test-app/alarm-test-lib.sh` and
|
||||
`doc/alarms/PHASE1-EMULATOR-TESTING.md` using exactly the techniques below.
|
||||
`APP_ID = org.timesafari.dailynotification`.
|
||||
|
||||
**Setup:**
|
||||
```bash
|
||||
emulator -avd Pixel_7 -no-snapshot-load &
|
||||
adb wait-for-device
|
||||
# install a debug build of the test app or the TimeSafari PWA Android shell
|
||||
```
|
||||
|
||||
**Step A — schedule, then capture a baseline of native scheduler + DB state**
|
||||
(don't trust "the call resolved" — inspect the actual OS/DB state, since
|
||||
that's exactly where this bug hides):
|
||||
```bash
|
||||
# AlarmManager state:
|
||||
adb shell dumpsys alarm | grep -A 5 -i "org.timesafari.dailynotification"
|
||||
|
||||
# DB state — the crucial one; reveals orphans dumpsys alone won't make obvious:
|
||||
adb shell run-as org.timesafari.dailynotification sqlite3 \
|
||||
databases/daily_notification_plugin.db \
|
||||
"SELECT id, kind, enabled, nextRunAt FROM schedules;"
|
||||
```
|
||||
Record the count and exact `id`/`kind` values — this is the "before" baseline.
|
||||
Pre-fix, expect to possibly see stray `daily_rollover_*`/`dual_notify_*`/
|
||||
`notify_<ts>` rows even before cancelling, which alone partially explains
|
||||
the bug.
|
||||
|
||||
**Step B — cancel via the app UI, then immediately re-run the same two
|
||||
commands.** This is the actual test — not "did the app report success"
|
||||
(it always does), but: does `dumpsys alarm` still show an entry, and does
|
||||
`schedules` still contain *any* `kind='notify'`/`'static_reminder'` row
|
||||
(enabled or not)? Pre-fix expectation per §1: the targeted row gets
|
||||
`enabled=0` but isn't deleted, and/or a stray row survives untouched.
|
||||
|
||||
**Step C — prove it actually still fires** (closes the loop — a surviving
|
||||
row could theoretically be inert):
|
||||
```bash
|
||||
# Time-travel the emulator clock forward (technique already documented at
|
||||
# doc/alarms/PHASE1-EMULATOR-TESTING.md:412-418):
|
||||
adb shell date -s "$(date -v+1d '+%Y-%m-%d %H:%M:%S')"
|
||||
adb logcat -c
|
||||
adb logcat -s DailyNotification:* NotifyReceiver:* DailyNotificationWorker:* \
|
||||
| grep -i "notification displayed\|scheduleNextNotification\|rearm"
|
||||
|
||||
# Optional visual proof — capture and pull a screenshot:
|
||||
adb shell screencap -p /sdcard/after-cancel.png && adb pull /sdcard/after-cancel.png
|
||||
|
||||
# Reset the clock when done:
|
||||
adb shell date -s "$(date '+%Y-%m-%d %H:%M:%S')"
|
||||
```
|
||||
Seeing "notification displayed successfully" / rearm log lines *after*
|
||||
cancelling is the smoking gun.
|
||||
|
||||
**Step D — re-run the full A→B→C sequence after applying the fix.**
|
||||
Expected post-fix result: zero `dumpsys alarm` entries and zero `schedules`
|
||||
rows immediately after cancel, and no log lines / no notification after the
|
||||
time-jump. This A→B→C→(fix)→A→B→C before/after comparison *is* the proof
|
||||
the refactor worked — anything less ("the call resolved without error") is
|
||||
exactly the false confidence that let this bug ship originally.
|
||||
|
||||
Also still worth doing once the structural fix is confirmed:
|
||||
- Let it fire and rearm at least once more (or use
|
||||
`rolloverIntervalMinutes` for fast iteration); confirm the row's id stays
|
||||
stable across rearms
|
||||
- Reboot the device after cancelling; confirm `BootReceiver` does not
|
||||
resurrect it
|
||||
|
||||
3. **Regression**: confirm the dual/API-retrieval test paths (if any exist in
|
||||
`test-apps/`) still pass unchanged — this refactor must not touch generator
|
||||
#2's behavior.
|
||||
|
||||
4. **Fast deterministic checks to run alongside the above** (don't touch the
|
||||
native bug directly, but catch regressions in surrounding plumbing and are
|
||||
cheap to run on every change):
|
||||
- `npm test` — Jest suite for the TS/JS plugin layer (web stub, type defs)
|
||||
- `cd android && ./gradlew test` — Kotlin unit tests (currently has the
|
||||
coverage gap noted in §6.1 above)
|
||||
|
||||
---
|
||||
|
||||
## 7. Suggested execution order
|
||||
|
||||
1. 3.5 (remove `scheduleUserNotification`) — fully independent, smallest blast
|
||||
radius, immediate dead-code win.
|
||||
2. 3.1 + 3.2 + 3.3 (new `kind`, simplified schedule/cancel) — the core fix.
|
||||
3. 3.7 (DB migration/cleanup for existing installs).
|
||||
4. 3.4 (`enabled` guard in rearm — closes the race).
|
||||
5. 3.6 (remove `daily_rollover_*` fallback + dead `handleNotificationIntent`
|
||||
code) — do this last, after 3.1-3.4 make it provably unreachable for the
|
||||
static path; double check it isn't somehow load-bearing for the dual flow
|
||||
first (it shouldn't be — dual ids never start with `"daily_"` and dual
|
||||
rearming goes through `DualScheduleNotifyScheduler`, not this fallback —
|
||||
but confirm before deleting).
|
||||
Reference in New Issue
Block a user