Restore active reminder notifications after device reboot#850
Merged
Conversation
Updated the base note to display the most relevant reminder. Added visual differences for elapsed reminders on the base note: strikethrough text, 50% opacity.
Tools namespace to show reminder chip in Layout Editor.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded Changes
Sequence DiagramsequenceDiagram
participant System as System (BOOT)
participant Receiver as ReminderReceiver
participant DB as Database
participant NM as NotificationManager
participant User as User
System->>Receiver: BOOT_COMPLETED
Receiver->>DB: query notes & reminders
DB-->>Receiver: reminders list
Receiver->>NM: post notifications (include delete PendingIntent)
Receiver->>DB: set isNotificationVisible = true
User->>NM: dismiss notification
NM->>Receiver: ACTION_NOTIFICATION_DISMISSED
Receiver->>DB: set isNotificationVisible = false
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt (1)
194-208:restoreRemindersNotificationsvisibility and minor issues.
This function is
publicbut only called internally fromonReceive. Consider making itprivate.
val now = Date(System.currentTimeMillis())at line 198 is created insideforEach, allocating a newDateper note. Move it before the loop:Proposed fix
fun restoreRemindersNotifications(context: Context, baseNoteDao: BaseNoteDao) { CoroutineScope(Dispatchers.IO).launch { val allNotes = baseNoteDao.getAllNotes() + val now = Date() allNotes.forEach { note -> - val now = Date(System.currentTimeMillis()) val mostRecentReminder = note.reminders .filter { it.dateTime <= now } .maxByOrNull { it.dateTime } ?: return@forEach
notify(...)at line 204 internally launches yet anotherCoroutineScope, creating fire-and-forget nested coroutines. It also redundantly re-writesisNotificationVisible = truefor reminders that are alreadytrue. Consider extracting asuspendversion of the notify logic to avoid nesting and skip the redundant DB update during restore.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt` around lines 194 - 208, Make restoreRemindersNotifications private (it's only used from onReceive), move the now = Date(...) allocation before the allNotes.forEach loop to avoid per-note allocation, and replace the current fire-and-forget notify(...) call with a suspend variant (e.g., notifySuspend(context, note.id, mostRecentReminder.id)) so you can call it directly from the existing CoroutineScope without creating nested CoroutineScope launches; implement the suspend notify to skip writing isNotificationVisible = true to the DB when the reminder already has isNotificationVisible true and use BaseNoteDao to perform any necessary updates only when the flag actually changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt`:
- Around line 70-72: In ReminderReceiver's onReceive, replace unstructured
CoroutineScope(Dispatchers.IO).launch calls with a goAsync()-backed
PendingResult: call val pendingResult = goAsync() then launch your coroutine
(e.g., the blocks that call setIsNotificationVisible(..., baseNoteDao, noteId,
reminderId) and the other DB/notification work) on Dispatchers.IO and ensure you
invoke pendingResult.finish() in a finally block so the system won't kill the
process before the work completes; update each launch site (the coroutines that
perform DB writes and notification posts) to use this pattern.
- Around line 172-192: The current getDeletePendingIntent uses
"$noteId-$reminderId".hashCode() which can collide; replace that with a
deterministic integer requestCode computed from the two Long IDs (e.g., combine
them into an int via a stable formula such as (noteId.toInt() * 31 +
reminderId.toInt()) or another deterministic combine you prefer) and pass that
requestCode to PendingIntent.getBroadcast so each noteId/reminderId pair gets a
unique PendingIntent while retaining the existing Intent extras and flags in
getDeletePendingIntent.
- Around line 53-56: The boot-completed branch currently only calls
restoreRemindersNotifications when canScheduleExactAlarms is true, which can
drop visible notifications if exact-alarm permission was revoked; update the
Intent.ACTION_BOOT_COMPLETED handling so rescheduleAlarms(context) remains gated
by canScheduleExactAlarms but restoreRemindersNotifications(context,
baseNoteDao) is always called on boot regardless of canScheduleExactAlarms;
locate the branch using canScheduleExactAlarms, rescheduleAlarms(context) and
restoreRemindersNotifications(context, baseNoteDao) in
ReminderReceiver.onReceive (or the receiver handling the intent) and move the
restoreRemindersNotifications call out of the canScheduleExactAlarms conditional
into the boot-completed path unconditionally.
- Around line 210-212: The call to getDatabase(context) inside
ReminderReceiver.onReceive runs on the main thread and can block during the Room
database creation; wrap the database access and subsequent DAO work (e.g., calls
to NotallyDatabase.getDatabase(...) and getBaseNoteDao()/methods) in a
background context by either launching CoroutineScope(Dispatchers.IO).launch {
... } or using goAsync() to obtain a PendingResult and then performing the
DB/DAO calls off the main thread before calling finish(); move the logic that
references getDatabase(context) and DAO operations into that coroutine or the
goAsync callback so onReceive returns promptly.
---
Nitpick comments:
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt`:
- Around line 194-208: Make restoreRemindersNotifications private (it's only
used from onReceive), move the now = Date(...) allocation before the
allNotes.forEach loop to avoid per-note allocation, and replace the current
fire-and-forget notify(...) call with a suspend variant (e.g.,
notifySuspend(context, note.id, mostRecentReminder.id)) so you can call it
directly from the existing CoroutineScope without creating nested CoroutineScope
launches; implement the suspend notify to skip writing isNotificationVisible =
true to the DB when the reminder already has isNotificationVisible true and use
BaseNoteDao to perform any necessary updates only when the flag actually
changes.
created goAsyncScope to use goAsync converted multiple functions to suspended
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt (1)
184-197: Minor efficiency nits inrestoreRemindersNotifications.
getAllNotes()loads every note (including those without reminders) —getAllReminders()is the scoped query already used byrescheduleAlarms/cancelAlarmsand avoids pulling irrelevant note data.Date(System.currentTimeMillis())is allocated inside theforEachloop; it should be hoisted above the loop (consistent with howrescheduleAlarmsdeclaresval now = Date()).♻️ Sketch of refactored approach
private suspend fun restoreRemindersNotifications(context: Context) { val baseNoteDao = getDatabase(context).getBaseNoteDao() - val allNotes = baseNoteDao.getAllNotes() - allNotes.forEach { note -> - val now = Date(System.currentTimeMillis()) - val mostRecentReminder = - note.reminders - .filter { it.dateTime <= now } - .maxByOrNull { it.dateTime } ?: return@forEach - if (mostRecentReminder.isNotificationVisible) { - notify(context, note.id, mostRecentReminder.id) - } - } + val now = Date() + val noteReminders = baseNoteDao.getAllReminders() + noteReminders.forEach { (noteId, reminders) -> + val mostRecentReminder = + reminders + .filter { it.dateTime <= now && it.isNotificationVisible } + .maxByOrNull { it.dateTime } ?: return@forEach + notify(context, noteId, mostRecentReminder.id) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt` around lines 184 - 197, The restoreRemindersNotifications function currently calls getAllNotes() and allocates Date(System.currentTimeMillis()) inside the loop; change it to query only reminders via the existing getAllReminders() scoped query (same approach as rescheduleAlarms/cancelAlarms) and hoist the now timestamp outside the loop (use val now = Date()) so you don't load all notes or recreate Date on every iteration; keep the existing logic that finds mostRecentReminder and calls notify(context, note.id, mostRecentReminder.id) but adapt to the reminder-centric results (use the reminder's parent note id where needed).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt`:
- Around line 93-94: Comments on the notification builder lines in
ReminderReceiver.kt are inaccurate — they say the values come "from intent" but
the data is pulled from the DB note object; update the inline comments on
setContentTitle(note.title) and setContentText(note.body.truncate(200)) to
correctly state that the title and text are sourced from the DB note (or remove
the redundant comments entirely), ensuring references to "intent" are removed
and the comment accurately references the note object.
- Around line 184-197: restoreRemindersNotifications currently calls notify
which calls scheduleReminder and causes redundant scheduling for repeating
reminders (rescheduleAlarms already scheduled them); fix by separating
notification posting from scheduling: either add a new helper
postNotification(context, noteId, reminderId) that only builds/posts the
notification and call that from restoreRemindersNotifications, or add a flag to
notify (e.g., skipScheduling: Boolean) that, when true, skips calling
scheduleReminder; update all callers accordingly and keep scheduleReminder only
invoked where alarms must be (e.g., initial creation or explicit rescheduling)
while rescheduleAlarms remains responsible for boot-time scheduling.
---
Duplicate comments:
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt`:
- Around line 163-182: getDeletePendingIntent currently uses noteId.toInt() as
the PendingIntent requestCode which causes collisions for multiple reminders on
the same note; change the requestCode to a deterministic combination of noteId
and reminderId (e.g., compute a combinedInt from noteId and reminderId such as
("$noteId:$reminderId").hashCode() or another stable int hash) and pass that
combinedInt into PendingIntent.getBroadcast so each (noteId, reminderId) pair
yields a unique PendingIntent while keeping the same Intent action/component and
flags (ACTION_NOTIFICATION_DISMISSED, EXTRA_NOTE_ID, EXTRA_REMINDER_ID,
FLAG_UPDATE_CURRENT | FLAG_IMMUTABLE).
---
Nitpick comments:
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt`:
- Around line 184-197: The restoreRemindersNotifications function currently
calls getAllNotes() and allocates Date(System.currentTimeMillis()) inside the
loop; change it to query only reminders via the existing getAllReminders()
scoped query (same approach as rescheduleAlarms/cancelAlarms) and hoist the now
timestamp outside the loop (use val now = Date()) so you don't load all notes or
recreate Date on every iteration; keep the existing logic that finds
mostRecentReminder and calls notify(context, note.id, mostRecentReminder.id) but
adapt to the reminder-centric results (use the reminder's parent note id where
needed).
…d, so no need to restoreReminderNotifications to schedule again
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt (1)
190-203:getAllNotes()loads every note when only notes with visible reminders are needed;Date(System.currentTimeMillis())is redundant.Two minor issues in
restoreRemindersNotifications:
Line 192:
getAllNotes()fetches all notes from the DB, even those with no reminders or no visible notifications. At boot (when the system is under load), this could be a costly scan. A targeted DAO query (e.g.,getNotesWithVisibleReminders()) or at minimumgetAllReminders()would be more efficient.Line 194:
Date(System.currentTimeMillis())is redundant — the no-argDate()constructor already captures the current time.♻️ Proposed fix for the redundant `Date` construction
- val now = Date(System.currentTimeMillis()) + val now = Date()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt` around lines 190 - 203, The method restoreRemindersNotifications currently calls getAllNotes() and constructs Date(System.currentTimeMillis()), which is inefficient and redundant; update it to query only notes/reminders with visible reminders (e.g., add or use a DAO method like getNotesWithVisibleReminders() or getAllReminders() and filter by isNotificationVisible) instead of getAllNotes(), and replace Date(System.currentTimeMillis()) with the no-arg Date() constructor; adjust the loop to iterate over the targeted result and call notify(context, note.id, mostRecentReminder.id, schedule = false) as before when mostRecentReminder.isNotificationVisible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt`:
- Around line 169-188: getDeletePendingIntent currently uses noteId.toInt() as
the PendingIntent request code which causes collisions for multiple reminders of
the same note and allows FLAG_UPDATE_CURRENT to overwrite extras; update
getDeletePendingIntent so the request code is derived deterministically from
both noteId and reminderId (e.g., combine them into a string or compute a hash
from "$noteId-$reminderId" or an equivalent stable hash) so each reminder
produces a unique request code while keeping ACTION_NOTIFICATION_DISMISSED, the
same Intent extras (EXTRA_NOTE_ID, EXTRA_REMINDER_ID), and the same flags
(PendingIntent.FLAG_UPDATE_CURRENT or FLAG_IMMUTABLE).
---
Duplicate comments:
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt`:
- Around line 98-99: The inline comments next to .setContentTitle(note.title)
and .setContentText(note.body.truncate(200)) are stale (they say "from intent"
while the values come from the DB note); update or remove those comments in
ReminderReceiver (the notification builder block) so they accurately reflect the
source (e.g., "from DB note" or no comment), ensuring the lines referencing
note.title and note.body.truncate(200) have correct/consistent comments.
---
Nitpick comments:
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt`:
- Around line 190-203: The method restoreRemindersNotifications currently calls
getAllNotes() and constructs Date(System.currentTimeMillis()), which is
inefficient and redundant; update it to query only notes/reminders with visible
reminders (e.g., add or use a DAO method like getNotesWithVisibleReminders() or
getAllReminders() and filter by isNotificationVisible) instead of getAllNotes(),
and replace Date(System.currentTimeMillis()) with the no-arg Date() constructor;
adjust the loop to iterate over the targeted result and call notify(context,
note.id, mostRecentReminder.id, schedule = false) as before when
mostRecentReminder.isNotificationVisible.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt (1)
194-194:Dateinstantiated on every loop iteration — hoist it aboveforEach.
Date(System.currentTimeMillis())captures a timestamp that is effectively constant for the duration of the loop; creating a new object per note is wasteful.♻️ Proposed fix
+ val now = Date(System.currentTimeMillis()) allNotes.forEach { note -> - val now = Date(System.currentTimeMillis())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt` at line 194, The loop creates a new Date on every iteration; move the timestamp capture out of the loop by declaring val now = Date(System.currentTimeMillis()) once before iterating over the notes (the forEach block in ReminderReceiver.kt) and use that single now value inside the loop instead of recreating Date per note.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt`:
- Around line 193-201: The restoreRemindersNotifications logic currently uses
maxByOrNull to pick a single past reminder per note (mostRecentReminder) and
skips notifying if that one is not visible, which drops older visible reminders;
change the loop to iterate over all past reminders for each note that are still
visible instead of taking a single max: inside the allNotes.forEach block
compute now = Date(System.currentTimeMillis()) and replace the
mostRecentReminder logic with note.reminders.filter { it.dateTime <= now &&
it.isNotificationVisible } and call notify(context, note.id, reminder.id,
schedule = false) for each matching reminder so every un-dismissed past reminder
is restored.
- Around line 111-114: The notification visibility flag is set after posting the
notification, causing a race where a user dismissal triggers
ACTION_NOTIFICATION_DISMISSED to clear isNotificationVisible before
setIsNotificationVisible(true, ...) runs; move the call to
setIsNotificationVisible(true, context, note.id, reminderId) so it executes
before manager.notify(note.id.toString(), reminderId.toInt(), notification),
ensuring the visibility flag is true prior to posting (also keep the existing
scheduleReminder and setIsNotificationVisible call semantics intact).
---
Duplicate comments:
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt`:
- Around line 98-99: Update the stale inline comments that say "from intent" in
ReminderReceiver: change or remove the comments on the calls to
setContentTitle(note.title) and setContentText(note.body.truncate(200)) to
correctly indicate the values come from the DB-backed note object (e.g., "Set
title from DB note" / "Set content text from DB note, truncated to 200 chars"),
so the comments match the actual data source used by ReminderReceiver when
building the notification.
---
Nitpick comments:
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt`:
- Line 194: The loop creates a new Date on every iteration; move the timestamp
capture out of the loop by declaring val now = Date(System.currentTimeMillis())
once before iterating over the notes (the forEach block in ReminderReceiver.kt)
and use that single now value inside the loop instead of recreating Date per
note.
Comment on lines
+111
to
+114
| manager.notify(note.id.toString(), reminderId.toInt(), notification) | ||
| if (schedule) | ||
| context.scheduleReminder(note.id, reminder, forceRepetition = true) | ||
| setIsNotificationVisible(true, context, note.id, reminderId) |
Contributor
There was a problem hiding this comment.
setIsNotificationVisible(true) after manager.notify() introduces a dismiss-then-reboot race.
The notification becomes dismissible the instant manager.notify() returns (line 111). If the user swipes it away before line 114 executes, the ACTION_NOTIFICATION_DISMISSED handler fires and sets isNotificationVisible = false. Line 114 then overwrites it back to true, so after a reboot the notification is re-posted even though the user explicitly dismissed it.
Move setIsNotificationVisible(true, ...) to before manager.notify():
🐛 Proposed fix
note.reminders
.find { it.id == reminderId }
?.let { reminder: Reminder ->
+ setIsNotificationVisible(true, context, note.id, reminderId)
manager.notify(note.id.toString(), reminderId.toInt(), notification)
if (schedule)
context.scheduleReminder(note.id, reminder, forceRepetition = true)
- setIsNotificationVisible(true, context, note.id, reminderId)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt`
around lines 111 - 114, The notification visibility flag is set after posting
the notification, causing a race where a user dismissal triggers
ACTION_NOTIFICATION_DISMISSED to clear isNotificationVisible before
setIsNotificationVisible(true, ...) runs; move the call to
setIsNotificationVisible(true, context, note.id, reminderId) so it executes
before manager.notify(note.id.toString(), reminderId.toInt(), notification),
ensuring the visibility flag is true prior to posting (also keep the existing
scheduleReminder and setIsNotificationVisible call semantics intact).
Comments removed Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt (2)
209-218:goAsyncScopehas noCoroutineExceptionHandler— uncaught exceptions can crash the process.If
codeBlock()throws,finallycorrectly callspendingResult.finish(), but the exception then propagates to the thread'sUncaughtExceptionHandler, which may terminate the process. Adding aCoroutineExceptionHandlerkeeps the exception contained without silently swallowing it.♻️ Proposed fix
private fun goAsyncScope(codeBlock: suspend CoroutineScope.() -> Unit) { val pendingResult = goAsync() - CoroutineScope(Dispatchers.IO).launch { + val exceptionHandler = CoroutineExceptionHandler { _, throwable -> + Log.e(TAG, "Unhandled exception in goAsyncScope", throwable) + } + CoroutineScope(Dispatchers.IO + exceptionHandler).launch { try { codeBlock() } finally { pendingResult.finish() } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt` around lines 209 - 218, The goAsyncScope function launches a coroutine without a CoroutineExceptionHandler so uncaught exceptions will hit the thread’s UncaughtExceptionHandler; add a CoroutineExceptionHandler and attach it to the coroutine scope or the launch call to catch exceptions from codeBlock(), log them (or report them via your logger), and still ensure pendingResult.finish() runs in the finally block; update references in this function (goAsyncScope, pendingResult, codeBlock, CoroutineScope(Dispatchers.IO).launch) to use the handler when creating the scope/launch so exceptions are contained and handled.
190-194:nowre-allocated on every loop iteration — hoist it outsideforEach.
Date(System.currentTimeMillis())at line 194 is re-created for each note, producing a slightly skewed timestamp per iteration and an unnecessary allocation per note.♻️ Proposed fix
private suspend fun restoreRemindersNotifications(context: Context) { val baseNoteDao = getDatabase(context).getBaseNoteDao() val allNotes = baseNoteDao.getAllNotes() + val now = Date(System.currentTimeMillis()) allNotes.forEach { note -> - val now = Date(System.currentTimeMillis()) val mostRecentReminder =🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt` around lines 190 - 194, In restoreRemindersNotifications, hoist the timestamp allocation so you compute now once before iterating allNotes instead of inside the allNotes.forEach block; get the current time (e.g., Date(System.currentTimeMillis()) or Instant.now()) into a local val (now) above the loop and then reference that single now inside the loop when processing each note from baseNoteDao.getAllNotes().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt`:
- Around line 111-114: Persist the notification-visible state before posting the
notification to avoid the dismiss-before-persist race: call
setIsNotificationVisible(true, context, note.id, reminderId) (the same call
currently at line 114) before invoking manager.notify(note.id.toString(),
reminderId.toInt(), notification) so ACTION_NOTIFICATION_DISMISSED cannot clear
the flag between persistence and posting; ensure any scheduling via
context.scheduleReminder(...) still runs after persistence if needed.
---
Nitpick comments:
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/note/reminders/ReminderReceiver.kt`:
- Around line 209-218: The goAsyncScope function launches a coroutine without a
CoroutineExceptionHandler so uncaught exceptions will hit the thread’s
UncaughtExceptionHandler; add a CoroutineExceptionHandler and attach it to the
coroutine scope or the launch call to catch exceptions from codeBlock(), log
them (or report them via your logger), and still ensure pendingResult.finish()
runs in the finally block; update references in this function (goAsyncScope,
pendingResult, codeBlock, CoroutineScope(Dispatchers.IO).launch) to use the
handler when creating the scope/launch so exceptions are contained and handled.
- Around line 190-194: In restoreRemindersNotifications, hoist the timestamp
allocation so you compute now once before iterating allNotes instead of inside
the allNotes.forEach block; get the current time (e.g.,
Date(System.currentTimeMillis()) or Instant.now()) into a local val (now) above
the loop and then reference that single now inside the loop when processing each
note from baseNoteDao.getAllNotes().
Owner
|
Thanks @ulibte for your contribution, definitely a good addition for the notifications 🙂 |
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I think this is good to not lose the reminder notification if you need to restart the phone.
2026-02-18.15-09-47.mp4
Summary by CodeRabbit
New Features
Bug Fixes
Tests