Skip to content

Notification System Refactor - #1923

Draft
AugustoVSoares wants to merge 1 commit into
developfrom
refactor/notifications
Draft

AugustoVSoares wants to merge 1 commit into
developfrom
refactor/notifications

Conversation

@AugustoVSoares

@AugustoVSoares AugustoVSoares commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Closes #1772

Refactored the notification system so that adding a new notification type is a
declaration rather than an implementation.

Before: Notification was an abstract class holding uniqueID + timeout,
plus three methods each subclass had to write: shouldDisplay(), buildNotificationContent() and displayNotification(). Every type hand-rolled its own AndroidNotificationDetails/DarwinNotificationDetails pair and its own throttle logic, so the platform presentation config was duplicated per type and the Workmanager isolate init was easy to get wrong.

After: Notification is an abstract interface of getters. Channel/image/group config, id resolution, the settings toggle check and the throttle are all implemented once on the base class and in NotificationManager. A new type declares its identity, its channel, its id strategy and its trigger, and nothing else.

Debugging

There's currently a debugging test notification class that can be triggered and toggled to show up every 30 seconds in the user profile. It is gated behind kDebugMode, but it is committed in this PR and is marked with // WARN: Remove this before merging to develop. It should be stripped in a follow-up commit so the framework change stays reviewable on its own.

Note: it is deliberately not in notificationMap, that map is the production registry consumed by the Android Workmanager task and the iOS background task, and TestNotification._shouldFire returns true unconditionally with defaultEnabled: true and no user-facing switch. Registering it there would ship a test notification to every user. The tradeoff is that it does not exercise the real background-isolate path, only the in-app poll.

Planned features TBA in this PR:

  • Notification grouping
  • exact-alarm permission, boot rescheduling, first ScheduledTrigger type

UI changes

DEBUG: test notifications
image
DEBUG: notification appearance
image

Other important mentions

iOS support still needs to be tested.
ScheduledTrigger has no consumers yet.

  • Tuition is the only registered type and it uses PollTrigger. The scheduling path is implemented but unexercised; it will also need SCHEDULE_EXACT_ALARM on Android 12+ and rescheduling on boot before it's used in production.

Review checklist

  • Terms and conditions reflect the changes

View Changes

  • Description has screenshots of the UI changes.
  • Tested both in light and dark mode.
  • New text is both in portuguese (PT) and english (EN).
  • Works in different text zoom levels.
  • Works in different screen sizes.

Performance

  • No helper functions to return widgets are added. New widgets are created instead.
  • Used ListView.builder for Long Lists.
  • Controllers (TextEditingController, ...) are being disposed of in dispose() method.

… and added a debu test notification (must be remove when branch is merged)
@coderabbitai

coderabbitai Bot commented Oct 9, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rework notification system so it is more reliable and ready for new types of notifications

1 participant