Skip to content

Avoid duplicate before-event reminders after offset changes - #111

Merged
FleetAdmiralJakob merged 1 commit into
notificationsfrom
codex/github-mention-codex]-implement-local-notifications-and-in
Jun 9, 2026
Merged

Avoid duplicate before-event reminders after offset changes#111
FleetAdmiralJakob merged 1 commit into
notificationsfrom
codex/github-mention-codex]-implement-local-notifications-and-in

Conversation

@FleetAdmiralJakob

@FleetAdmiralJakob FleetAdmiralJakob commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Motivation

  • Prevent creating duplicate in-app "before-event" notifications when a user changes the reminderOffsetMinutes preference after a before-event notification has already been created.
  • Keep on-device scheduled notification metadata aligned with Convex inbox de-duplication so preference edits still allow rescheduling without producing extra inbox items.

Description

  • Use a stable per-entry before-event event key before:<entryId> in the Convex mutation syncDueNotifications and in local planner output so de-duplication ignores the reminder offset value.
  • Update the local notification planner to emit key: "before:<entryId>" and adjust the local notification scheduler expectations to include the same stable key in scheduled notification data.dayovaNotificationKey.
  • Add a regression test changing reminder offset does not duplicate an already-created before-event notification to convex/notifications.test.ts that asserts updating reminderOffsetMinutes does not create a second inbox notification.

Testing

  • Ran unit tests with pnpm test src/lib/notification-planner.test.ts src/lib/local-notification-scheduler.test.ts convex/notifications.test.ts and all tests passed (3 files, 9 tests).
  • Ran full checks with pnpm check (lint + tsc --noEmit) and the checks completed successfully.

Codex Task

Summary by CodeRabbit

  • Bug Fixes

    • Fixed an issue where changing your reminder offset would create duplicate before-event notifications instead of updating the existing one.
  • Tests

    • Added test coverage for notification deduplication when reminder preferences change.
    • Updated test expectations to reflect new notification key format.

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d9ad91dc-82c0-45d5-b47f-075dc4321c93

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • ✅ Review completed - (🔄 Check again to review again)
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/github-mention-codex]-implement-local-notifications-and-in

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details
{}

@FleetAdmiralJakob
FleetAdmiralJakob merged commit c0d7b3f into notifications Jun 9, 2026
1 check was pending
@FleetAdmiralJakob
FleetAdmiralJakob deleted the codex/github-mention-codex]-implement-local-notifications-and-in branch June 9, 2026 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant