Feat/in app notification center #214
Conversation
- Add Arabic and English translations for the in-app notification center - Include strings for page title, buttons, loading states, failure states, empty states, and tab labels - Update existing notification-related translations
- Implement in-app notification client and repository - Integrate in-app notification support with push notification service - Update bootstrap process to include in-app notification initialization
- Add inAppNotificationRepository to AppInitializationPage constructor - Pass inAppNotificationRepository to app life cycle status pages - Update formatting and indentation in the build method
- Add handlers for AppInAppNotificationMarkedAsRead and AppAllInAppNotificationsMarkedAsRead events - Update AppBloc to listen for in-app notification received events - Implement logic to update unread notification status - Add new AppInAppNotificationMarkedAsRead and AppAllInAppNotificationsMarkedAsRead events
- Add InAppNotification repository as a required parameter in App constructor - Store inAppNotificationRepository as an instance variable - Provide inAppNotificationRepository using RepositoryProvider - Pass inAppNotificationRepository to AppBloc initialization
- Add new route for in-app notification center in router.dart - Define new route constants for notifications in routes.dart - Import in-app notification center page in router.dart
- Add a new ListTile for notifications in the account page - Implement a BlocSelector to check for unread notifications - Create a helper function to build ListTiles with optional indicators - Update imports to include the shared widgets library
- Add InAppNotificationCenterBloc to manage notification center state - Implement events for subscription, mark as read, and tab changes - Define state with status, notifications, and error handling - Add logic to fetch, filter, and update notifications - Integrate with AppBloc for global unread status management
…st item widget - Implement InAppNotificationCenterPage to display a chronological list of in-app notifications - Add InAppNotificationListItem widget to show individual notifications in a list - Include functionality to mark notifications as read and navigate to related content - Add tabs for different notification categories: All, Breaking News, and Digests - Implement loading, failure, and empty states for the notification list
…tificationService - Introduce new abstract method for handling in-app notifications - Update documentation for existing methods
- Add InAppNotification repository dependency - Implement in-app notification persistence logic - Add onInAppNotificationReceived stream - Update registerDevice method to store current user
- Add InAppNotification repository dependency - Implement in-app notification persistence logic - Add onInAppNotificationReceived stream - Update registerDevice method to store current user ID - Modify handleMessage to create and emit in-app notifications
- Import 'dart:async' for stream support - Add 'onInAppNotificationReceived' stream to NoOpPushNotificationService - This change ensures the no-op service implements the full PushNotificationService interface
- Update the color of the notification indicator from error to primary - This change affects the appearance of the indicator in the UI
- Move in-app notification handling logic from push notification services to AppBloc - Remove InAppNotification stream and related logic from PushNotificationService implementations - Update push notification services to only handle message routing - Simplify registration and device handling in push notification services
- Remove inAppNotificationRepository parameter from FirebasePushNotificationService and OneSignalPushNotificationService - This change simplifies the push notification service initialization process
- Remove notifications route from main routes list - Add notifications route inside account modal routes - This change ensures proper navigation and back stack behavior - Fixes bug where going back from notifications page would close the app
- Change route path and name from 'notifications' to 'notifications-center' - Remove duplicate notifications route defined in two places - Update import order to be consistent with other imports
- Replace incorrect route name `Routes.notificationsName` with correct `Routes.notificationsCenterName` - This change ensures proper navigation to the Notification Center page when the account notifications tile is tapped
- Remove 'All' tab and related logic - Consolidate notifications into breaking news and digests only - Update UI to reflect new tab structure - Improve code readability and maintainability
- Add in-app notification initialization to the DemoDataInitializerService - Update bootstrap.dart to include in-app notification repository and fixtures - Implement logic to clone and assign notifications from fixture data for new users
- Replace deliveryType-based filtering with contentType-based filtering - Simplify notification categorization into breakingNews and digests - Ensure all notification types are visible to the user
- Enhance NoOpPushNotificationService to simulate push notifications in demo environment - Update DemoDataInitializerService to handle empty inAppNotificationsFixturesData - Modify bootstrap.dart to pass additional parameters to NoOpPushNotificationService - Improve logging and add stream handling for simulated push notifications
…mode - Change the delay from 3 to 10 seconds for in-app notifications in demo mode - This modification helps frontend developers by providing more time to - observe and test the notification flow
- Remove foreground notification handling feature - Add integrated notification center feature with subtle in-app indicator and unread count - Improve description for better clarity
Summary of ChangesHello @fulleni, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a significant new feature: an in-app notification center. This central hub allows users to browse their notification history, categorized into 'Breaking News' and 'Digests'. It provides functionality to mark notifications as read, both individually and in bulk, and integrates seamlessly with the application's global state to provide real-time unread indicators. The underlying push notification services have been enhanced to support this new in-app experience, including a simulation for demo environments to ensure a consistent user journey. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a well-structured in-app notification center, a significant feature enhancement. The implementation spans across the BLoC, UI, routing, and services layers, and is generally of high quality. I've identified a critical issue in the new notification BLoC that could lead to a crash, and a medium-severity issue related to ID generation for notifications. I've provided detailed feedback and code suggestions for these points to improve the robustness of the new feature. The rest of the changes, including dependency injection updates, routing, and UI components, are well-implemented.
- Use firstWhereOrNull instead of firstWhere to safely handle null case - Add logging for attempted markAsRead on non-existent notification - Prevent exception by returning early if notification is not found
- Remove unnecessary import of dart:math - Add uuid package to dependencies - Update InAppNotification ID generation to use UUID instead of random number - Add Uuid as a dependency in AppBloc constructor
- Move BlocProvider from InAppNotificationCenterPage to router - Ensure InAppNotificationCenterBloc is available in BuildContext when InAppNotificationCenterPage's initState runs - Refactor InAppNotificationCenterPage widget build
- Prioritize 'notificationType' from the backend over 'contentType' - Explicitly check for 'dailyDigest' and 'weeklyRoundup' notification types - Maintain existing behavior for breaking news and headline content
- Remove Uuid dependency from AppBloc - Remove notification persistence logic from push notification handler - Update comment to reflect new backend responsibility for notification persistence
…from a deep-link - Implement new event InAppNotificationCenterMarkOneAsRead - Add new handler _onMarkOneAsRead in InAppNotificationCenterBloc - Extract shared logic to _markOneAsRead helper method - Update imports to follow package order
- Add handling for 'notificationId' in notification payload - Use pushNamed instead of goNamed for navigation to ensure correct context - Pass 'notificationId' as extra data when navigating to article details
- Add notificationId parameter to HeadlineDetailsPage - Mark in-app notification as read when navigating from a push notification - Import InAppNotificationCenterBloc for handling notification state
- Add support for push notification deep-links in account and global article details routes - Improve type safety for route 'extra' parameters - Refactor BlocProvider creations for better readability - Extract common provider setup for entity and headline details pages
- Align the NoOp service's behavior with the backend architecture - Persist notification in the in-memory repository before emitting the push payload - This change ensures a more accurate simulation of the real push notification process
- Update core dependency in pubspec.yaml and pubspec.lock - Change ref from f409be2 to b01f9b4 in both files
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a full-featured in-app notification center, a significant enhancement to user engagement. The implementation is solid, with a new BLoC to manage notification state, a well-structured UI, and updates to the routing and dependency injection to support the feature. The handling of foreground notifications and deep-linking is well thought out. My review includes a couple of suggestions to improve the robustness of payload data handling by using safer type casting and avoiding magic strings, which will make the code more resilient and maintainable.
github-project-automation
bot
moved this from Backlog
to Done
in Flutter News App Project
Status
READY
Description
Type of Change