fix(android): enforce one-per-day semantics in scheduleDailyNotification
Fix duplicate alarm bug where updating schedule time created multiple schedules in database, violating "one notification per day" contract. Plugin Changes: - Use stable scheduleId "daily_notification" instead of timestamp-based IDs - Delete all existing notification schedules before creating new one - Cancel alarms in AlarmManager before database deletion - Add detailed logging for cleanup operations - Make scheduleDailyReminder delegate to scheduleDailyNotification Test Harness Changes: - Make TEST 2 fail when alarm count > 1 after schedule update - Make TEST 2 fail when alarm count > 1 after recovery - Add clear failure messages explaining "one per day" violation - Add final verdict section with detailed failure summary Results: - Before: 2-3 alarms, 2 schedules in DB, "Pending: 2" in UI - After: 1 alarm, 1 schedule in DB, "Pending: 1" in UI - TEST 2 now correctly passes with proper validation This ensures that updating schedule time maintains exactly one alarm per day, preventing duplicate notifications and database bloat.
This commit is contained in:
Matthew Raymer
@@ -732,117 +732,8 @@ open class DailyNotificationPlugin : Plugin() {
|
||||
@PluginMethod
|
||||
fun scheduleDailyReminder(call: PluginCall) {
|
||||
// Alias for scheduleDailyNotification for backward compatibility
|
||||
// scheduleDailyReminder accepts same parameters as scheduleDailyNotification
|
||||
try {
|
||||
if (context == null) {
|
||||
return call.reject("Context not available")
|
||||
}
|
||||
|
||||
// Check if exact alarms can be scheduled
|
||||
if (!canScheduleExactAlarms(context)) {
|
||||
// Permission not granted - request it
|
||||
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S && canRequestExactAlarmPermission(context)) {
|
||||
try {
|
||||
val intent = Intent(Settings.ACTION_REQUEST_SCHEDULE_EXACT_ALARM).apply {
|
||||
data = android.net.Uri.parse("package:${context.packageName}")
|
||||
addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
|
||||
}
|
||||
context.startActivity(intent)
|
||||
Log.w(TAG, "Exact alarm permission required. Opened Settings for user to grant permission.")
|
||||
call.reject(
|
||||
"Exact alarm permission required. Please grant 'Alarms & reminders' permission in Settings, then try again.",
|
||||
"EXACT_ALARM_PERMISSION_REQUIRED"
|
||||
)
|
||||
return
|
||||
} catch (e: Exception) {
|
||||
Log.e(TAG, "Failed to open exact alarm settings", e)
|
||||
call.reject("Failed to open exact alarm settings: ${e.message}")
|
||||
return
|
||||
}
|
||||
} else {
|
||||
try {
|
||||
val intent = Intent(Settings.ACTION_APPLICATION_DETAILS_SETTINGS).apply {
|
||||
data = android.net.Uri.parse("package:${context.packageName}")
|
||||
addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
|
||||
}
|
||||
context.startActivity(intent)
|
||||
Log.w(TAG, "Exact alarm permission denied. Directing user to app settings.")
|
||||
call.reject(
|
||||
"Exact alarm permission denied. Please enable 'Alarms & reminders' in app settings.",
|
||||
"PERMISSION_DENIED"
|
||||
)
|
||||
return
|
||||
} catch (e: Exception) {
|
||||
Log.e(TAG, "Failed to open app settings", e)
|
||||
call.reject("Failed to open app settings: ${e.message}")
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Permission granted - proceed with scheduling
|
||||
// Capacitor passes the object directly via call.data
|
||||
val options = call.data ?: return call.reject("Options are required")
|
||||
|
||||
// Extract required fields, with defaults
|
||||
val time = options.getString("time") ?: return call.reject("Time is required")
|
||||
val title = options.getString("title") ?: "Daily Reminder"
|
||||
val body = options.getString("body") ?: ""
|
||||
val sound = options.getBoolean("sound") ?: true
|
||||
val priority = options.getString("priority") ?: "default"
|
||||
|
||||
Log.i(TAG, "Scheduling daily reminder: time=$time, title=$title")
|
||||
|
||||
// Convert HH:mm time to cron expression (daily at specified time)
|
||||
val cronExpression = convertTimeToCron(time)
|
||||
|
||||
CoroutineScope(Dispatchers.IO).launch {
|
||||
try {
|
||||
val config = UserNotificationConfig(
|
||||
enabled = true,
|
||||
schedule = cronExpression,
|
||||
title = title,
|
||||
body = body,
|
||||
sound = sound,
|
||||
vibration = options.getBoolean("vibration") ?: true,
|
||||
priority = priority
|
||||
)
|
||||
|
||||
val nextRunTime = calculateNextRunTime(cronExpression)
|
||||
|
||||
// Generate scheduleId before scheduling (needed for stable requestCode)
|
||||
val scheduleId = options.getString("id") ?: "daily_reminder_${System.currentTimeMillis()}"
|
||||
|
||||
// Schedule AlarmManager notification
|
||||
NotifyReceiver.scheduleExactNotification(
|
||||
context,
|
||||
nextRunTime,
|
||||
config,
|
||||
scheduleId = scheduleId,
|
||||
source = ScheduleSource.INITIAL_SETUP
|
||||
)
|
||||
|
||||
// Store schedule in database
|
||||
val schedule = Schedule(
|
||||
id = scheduleId,
|
||||
kind = "notify",
|
||||
cron = cronExpression,
|
||||
clockTime = time,
|
||||
enabled = true,
|
||||
nextRunAt = nextRunTime
|
||||
)
|
||||
getDatabase().scheduleDao().upsert(schedule)
|
||||
|
||||
call.resolve()
|
||||
} catch (e: Exception) {
|
||||
Log.e(TAG, "Failed to schedule daily reminder", e)
|
||||
call.reject("Daily reminder scheduling failed: ${e.message}")
|
||||
}
|
||||
}
|
||||
} catch (e: Exception) {
|
||||
Log.e(TAG, "Schedule daily reminder error", e)
|
||||
call.reject("Daily reminder error: ${e.message}")
|
||||
}
|
||||
// This ensures both method names work the same way
|
||||
scheduleDailyNotification(call)
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -1380,6 +1271,54 @@ open class DailyNotificationPlugin : Plugin() {
|
||||
|
||||
CoroutineScope(Dispatchers.IO).launch {
|
||||
try {
|
||||
// Use stable scheduleId for daily notifications to ensure "one per day" semantics
|
||||
// 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
|
||||
// This cleanup runs regardless of whether user provided an ID or not
|
||||
val existingSchedules = getDatabase().scheduleDao().getByKind("notify")
|
||||
Log.i(TAG, "scheduleDailyNotification: Found ${existingSchedules.size} existing notification schedule(s) in database")
|
||||
|
||||
if (existingSchedules.isNotEmpty()) {
|
||||
Log.i(TAG, "scheduleDailyNotification: Existing schedule IDs: ${existingSchedules.map { it.id }.joinToString(", ")}")
|
||||
}
|
||||
|
||||
var cleanedCount = 0
|
||||
existingSchedules.forEach { existingSchedule ->
|
||||
try {
|
||||
// Skip if this is the same schedule we're about to create (will be upserted anyway)
|
||||
if (existingSchedule.id == scheduleId) {
|
||||
Log.i(TAG, "scheduleDailyNotification: Skipping cleanup of schedule with same ID (will be updated): ${existingSchedule.id}")
|
||||
return@forEach
|
||||
}
|
||||
|
||||
Log.i(TAG, "scheduleDailyNotification: Cleaning up existing schedule: id=${existingSchedule.id}, nextRunAt=${existingSchedule.nextRunAt}, enabled=${existingSchedule.enabled}")
|
||||
|
||||
// Cancel the alarm in AlarmManager
|
||||
NotifyReceiver.cancelNotification(context, existingSchedule.id)
|
||||
Log.i(TAG, "scheduleDailyNotification: Cancelled alarm for schedule: ${existingSchedule.id}")
|
||||
|
||||
// Delete from database
|
||||
getDatabase().scheduleDao().deleteById(existingSchedule.id)
|
||||
Log.i(TAG, "scheduleDailyNotification: Deleted schedule from database: ${existingSchedule.id}")
|
||||
cleanedCount++
|
||||
} catch (e: Exception) {
|
||||
Log.e(TAG, "scheduleDailyNotification: Failed to cancel/delete existing schedule: ${existingSchedule.id}", e)
|
||||
// Continue with other schedules - don't fail entire operation
|
||||
}
|
||||
}
|
||||
|
||||
if (cleanedCount > 0) {
|
||||
Log.i(TAG, "scheduleDailyNotification: ✅ Cleaned up $cleanedCount existing notification schedule(s) before creating new one (total found: ${existingSchedules.size})")
|
||||
} else if (existingSchedules.isNotEmpty()) {
|
||||
Log.i(TAG, "scheduleDailyNotification: No cleanup needed - existing schedule will be updated via upsert: $scheduleId")
|
||||
} else {
|
||||
Log.i(TAG, "scheduleDailyNotification: No existing schedules found - creating first notification schedule")
|
||||
}
|
||||
|
||||
val config = UserNotificationConfig(
|
||||
enabled = true,
|
||||
schedule = cronExpression,
|
||||
@@ -1394,7 +1333,6 @@ open class DailyNotificationPlugin : Plugin() {
|
||||
|
||||
// Schedule AlarmManager notification as static reminder
|
||||
// (doesn't require cached content)
|
||||
val scheduleId = "daily_${System.currentTimeMillis()}"
|
||||
NotifyReceiver.scheduleExactNotification(
|
||||
context,
|
||||
nextRunTime,
|
||||
@@ -1402,7 +1340,7 @@ open class DailyNotificationPlugin : Plugin() {
|
||||
isStaticReminder = true,
|
||||
reminderId = scheduleId,
|
||||
scheduleId = scheduleId,
|
||||
source = ScheduleSource.TEST_NOTIFICATION
|
||||
source = ScheduleSource.INITIAL_SETUP
|
||||
)
|
||||
|
||||
// Always schedule prefetch 2 minutes before notification
|
||||
|
||||
@@ -794,11 +794,13 @@ main() {
|
||||
|
||||
if [ "${UPDATED_ALARM_COUNT}" -eq "1" ] 2>/dev/null; then
|
||||
print_success "✅ Single alarm confirmed after schedule update (one per day maintained)"
|
||||
elif [ "${UPDATED_ALARM_COUNT}" -gt "1" ] 2>/dev/null; then
|
||||
print_error "❌ TEST 2 FAILED: Found ${UPDATED_ALARM_COUNT} plugin alarms after update (expected: 1)"
|
||||
print_error " Old alarm was NOT canceled - violates 'one per day' semantics"
|
||||
print_info " This indicates the plugin did not clean up existing schedules before creating new one"
|
||||
TEST2_FAILED=true
|
||||
else
|
||||
print_warn "⚠️ Found ${UPDATED_ALARM_COUNT} plugin alarms (expected: 1)"
|
||||
if [ "${UPDATED_ALARM_COUNT}" -gt "1" ] 2>/dev/null; then
|
||||
print_warn "⚠️ Multiple alarms detected - old alarm may not have been canceled"
|
||||
fi
|
||||
print_warn "⚠️ Found ${UPDATED_ALARM_COUNT} plugin alarms (expected: 1) - no alarm scheduled after update"
|
||||
fi
|
||||
|
||||
print_step "3" "Killing app and relaunching (triggers recovery)..."
|
||||
@@ -827,21 +829,33 @@ main() {
|
||||
ALARM_COUNT_AFTER_RECOVERY=$(get_plugin_alarm_count)
|
||||
print_info "Plugin alarms after recovery: ${ALARM_COUNT_AFTER_RECOVERY} (expected: 1)"
|
||||
|
||||
# CRITICAL: Test fails if alarm count > 1 after recovery (violates "one per day")
|
||||
if [ "${ALARM_COUNT_AFTER_RECOVERY}" -gt "1" ] 2>/dev/null; then
|
||||
print_error "❌ TEST 2 FAILED: Found ${ALARM_COUNT_AFTER_RECOVERY} plugin alarms after recovery (expected: 1)"
|
||||
print_error " Multiple schedules in database caused recovery to reschedule duplicates"
|
||||
print_error " This violates 'one per day' semantics - only one notification per day should exist"
|
||||
TEST2_FAILED=true
|
||||
fi
|
||||
|
||||
if [ "${RESCHEDULED_COUNT}" -gt "0" ] 2>/dev/null; then
|
||||
print_success "✅ TEST 2 PASSED: Missing alarm was detected and rescheduled (rescheduled=${RESCHEDULED_COUNT})!"
|
||||
if [ "${ALARM_COUNT_AFTER_RECOVERY}" -eq "1" ] 2>/dev/null; then
|
||||
print_success "✅ TEST 2 PASSED: Missing alarm was detected and rescheduled (rescheduled=${RESCHEDULED_COUNT})!"
|
||||
print_success "✅ Single alarm confirmed after recovery (one per day maintained)"
|
||||
else
|
||||
print_warn "⚠️ Alarm count is ${ALARM_COUNT_AFTER_RECOVERY} (expected: 1)"
|
||||
elif [ "${TEST2_FAILED:-false}" != "true" ]; then
|
||||
print_warn "⚠️ Recovery rescheduled ${RESCHEDULED_COUNT} alarm(s), but alarm count is ${ALARM_COUNT_AFTER_RECOVERY} (expected: 1)"
|
||||
fi
|
||||
elif [ "${VERIFIED_COUNT}" -gt "0" ] 2>/dev/null; then
|
||||
print_success "✅ TEST 2 PASSED: Alarm verified in AlarmManager (verified=${VERIFIED_COUNT})!"
|
||||
if [ "${ALARM_COUNT_AFTER_RECOVERY}" -eq "1" ] 2>/dev/null; then
|
||||
print_success "✅ TEST 2 PASSED: Alarm verified in AlarmManager (verified=${VERIFIED_COUNT})!"
|
||||
print_success "✅ Single alarm confirmed after recovery (one per day maintained)"
|
||||
else
|
||||
print_warn "⚠️ Alarm count is ${ALARM_COUNT_AFTER_RECOVERY} (expected: 1)"
|
||||
elif [ "${TEST2_FAILED:-false}" != "true" ]; then
|
||||
print_warn "⚠️ Recovery verified ${VERIFIED_COUNT} alarm(s), but alarm count is ${ALARM_COUNT_AFTER_RECOVERY} (expected: 1)"
|
||||
fi
|
||||
elif [ "${RESCHEDULED_COUNT}" -eq "0" ] 2>/dev/null && [ "${VERIFIED_COUNT}" -eq "0" ] 2>/dev/null; then
|
||||
if [ "${ALARM_COUNT_AFTER_RECOVERY}" -eq "1" ] 2>/dev/null; then
|
||||
print_success "✅ TEST 2 PASSED: No recovery needed - alarm already properly scheduled"
|
||||
print_success "✅ Single alarm confirmed (one per day maintained)"
|
||||
else
|
||||
print_warn "⚠️ TEST 2: No verification/rescheduling needed (both verified=0 and rescheduled=0)"
|
||||
print_info "This might mean:"
|
||||
echo " - Alarm was already properly scheduled and didn't need recovery"
|
||||
@@ -849,11 +863,24 @@ main() {
|
||||
if [ "${ALARM_COUNT_AFTER_RECOVERY}" -eq "1" ] 2>/dev/null; then
|
||||
print_success "✅ Single alarm still present - recovery may have verified it silently"
|
||||
fi
|
||||
fi
|
||||
else
|
||||
print_error "TEST 2 INCONCLUSIVE: Could not find recovery result"
|
||||
print_info "Recovery result: ${RECOVERY_RESULT}"
|
||||
fi
|
||||
|
||||
# Final verdict
|
||||
if [ "${TEST2_FAILED:-false}" = "true" ]; then
|
||||
print_error ""
|
||||
print_error "════════════════════════════════════════════════════════════"
|
||||
print_error "TEST 2 FINAL RESULT: ❌ FAILED"
|
||||
print_error "════════════════════════════════════════════════════════════"
|
||||
print_error "Reason: Multiple alarms detected - violates 'one per day' semantics"
|
||||
print_error "Expected: 1 alarm after update and after recovery"
|
||||
print_error "Actual: ${UPDATED_ALARM_COUNT} after update, ${ALARM_COUNT_AFTER_RECOVERY} after recovery"
|
||||
print_error "════════════════════════════════════════════════════════════"
|
||||
fi
|
||||
|
||||
print_step "5" "Verifying alarms are still scheduled in AlarmManager..."
|
||||
check_alarm_status
|
||||
|
||||
|
||||
Reference in New Issue
Block a user