-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/define shared notification models #71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Create a new enum to identify the mobile operating system of a user's device - Add two values: ios and android - Include template comments for documentation purposes
- Introduce new enum 'PushNotificationProvider' to define available push notification service providers - Include two providers: Firebase Cloud Messaging and OneSignal - Enhance backend functionality by allowing selection of appropriate client for sending notifications - Enable device registration with configured push notification provider
- Defines the types of notifications a user can receive for a subscription - Includes options for breaking news, daily digest, and weekly roundup - Allows users to opt into multiple delivery types for flexible alert configurations
- Create Device model to represent user's device registered for push notifications - Include properties for device token, push notification provider, platform, and user association - Implement JSON serialization and deserialization - Add Equatable mixin for value comparison
- Define the generic structure of a push notification message - Include properties for title, body, optional image URL, and custom data map - Implement JSON serialization and deserialization - Add copyWith method for easy instance modification
- Represents a user's saved notification filter - Stores criteria for topics, sources, and countries - Defines notification preferences for the filter - Includes factory constructor for JSON deserialization - Provides method for JSON serialization - Implements Equatable for value comparison - Includes copyWith method for instance duplication
- Add isBreaking property to Headline class - Update props list to include isBreaking - Modify copyWith method to support isBreaking - Add documentation for the new isBreaking flag
- Define PushNotificationConfig model for remote configuration of push notifications - Include properties for enabling/disabling notifications and setting primary provider - Add methods for JSON serialization and copying with updated values
- Remove unused import statement for 'DevicePlatform' enum
- Add PushNotificationConfig to RemoteConfig model - Include core package instead of individual imports - Update RemoteConfig constructor and copyWith method - Modify props list to include new pushNotificationConfig field
- Add information about new data models for push notification system - Update changelog with upcoming features and breaking changes
- Add models for push notifications and subscriptions - Include PushNotificationConfig in Application Configuration - Update documentation to reflect new features
- Introduce an abstract class for push notification provider configurations - Implement polymorphic deserialization using a factory constructor - Support Firebase and OneSignal provider configurations - Include providerName as a discriminator field for JSON serialization
- Create FirebaseProviderConfig class for Firebase Cloud Messaging credentials - Extend PushNotificationProviderConfig - Implement JSON serialization and deserialization - Include projectId, clientEmail, and privateKey fields - Add copyWith method for easy instance duplication
- Create a new model for OneSignal push notification provider configuration - Implement JSON serialization and deserialization - Include appId and restApiKey fields for OneSignal credentials - Extend PushNotificationProviderConfig with specific OneSignal implementation
- Create new model for role-specific push notification delivery configuration - Include subscription limit as a configurable parameter - Implement Equatable for value comparison and JSON serialization
- Define a new model for push notification delivery configuration - Include visibility and limits based on user roles - Support JSON serialization and deserialization - Add Equatable mixin for value comparison
- Add providerConfigs and deliveryConfigs to PushNotificationConfig - Update imports to include new model classes - Expand class documentation to reflect new capabilities - Modify props to include new fields for equality comparison - Update copyWith method to support new configuration options
- Adjusted the order of properties in the RemoteConfig class - Moved 'appStatus' below 'userPreferenceConfig' for better logical flow - Updated the 'props' list to match the new property order
- Import necessary enums and models for push notifications - Add pushNotificationSubscriptions field to User class - Implement JSON serialization and deserialization for pushNotificationSubscriptions - Update User copyWith method to include pushNotificationSubscriptions - Add helper functions for handling push notification subscriptions map
…Preferences - Import necessary packages for notifications and push subscriptions - Add notificationSubscriptions field to UserContentPreferences class - Update constructor, toJson, and copyWith methods to include new field - Add documentation for the new notificationSubscriptions field
- Update import paths for consistency across multiple files - Rename `SubscriptionDeliveryType` to `PushNotificationSubscriptionDeliveryType` - Rename `NotificationDeliveryRoleConfig` to `PushNotificationDeliveryRoleConfig` - Adjust part file references for better organization - Update `RemoteConfig` to import from `config` instead of `push_notification_config`
- Replace hardcoded config instances with fixtures - Remove manual JSON map creation, use toJson() instead - Import necessary packages for enums and fixtures
- Remove hardcoded push notification config in test - Use remoteConfigsFixturesData from fixtures package instead - Simplify test by leveraging predefined application configuration
…try IDs - Replace hardcoded IDs with IDs from fixtures - Import necessary fixture files for countries, sources, and topics - Update test to ensure data integrity and consistency
- Add test for value equality using fixture data - Implement props correctness test - Separate JSON serialization and deserialization tests - Enhance copyWith test to include more properties - Add test for inequality with different properties
- Increment props length expectation from 12 to 13 - Add isBreaking field to props list test
- Create a new test file for DevicePlatform enum - Add tests to verify the correct values of DevicePlatform enum - Ensure the enum has exactly two values: ios and android
- Change notificationSubscriptions type from Map to List - Remove unused import of enums package - Update toJson and copyWith methods to reflect the structural change
- Add test to check if notificationSubscriptions is a List<PushNotificationSubscription> - Implement test for copyWith method to update notificationSubscriptions - Include notificationSubscriptions in props list test
…ptions
Replace empty map with empty list in userContentPreferencesFixturesData for
notificationSubscriptions field. This change ensures consistency with the
UserContentPreferences class definition, which uses a list for notification
subscriptions.
- Updated three instances in the fixtures data
- Changed from const {} to const []
- Expand the description of the upcoming feature to include models, fixtures, and tests for the push notification system. - This change provides a more comprehensive overview of the implemented functionality.
- Refactor Feature Showcase section with added details on various features - Reorganize content into categorized sections for better clarity - Enhance readability with improved formatting and consistent structure - Expand descriptions to provide more detailed information on each feature - Reorder some sections to present content in a more logical flow
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 lays the groundwork for a comprehensive and flexible push notification system within the application. It introduces all necessary data models, enums, and configuration structures to support user-defined notification subscriptions, device registration, and dynamic management of notification providers and delivery types. This feature will significantly enhance user engagement through personalized and timely alerts, all controllable via remote configuration. 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a comprehensive set of shared models, enums, and configurations for a new push notification system. The changes are well-structured, with clear separation of concerns for different parts of the notification system, such as providers, delivery types, and user subscriptions. The inclusion of extensive fixtures and thorough unit tests for the new models is excellent and ensures the new data structures are robust. I've identified a few minor issues, including an unused parameter in a copyWith method and a misleading error message, which should be straightforward to address.
- Remove `pushNotificationSubscriptions` property from User class - This property is no longer used in the application and is redundant
- Capitalize first letter of the feat description
- Modify the error message for missing 'provider' field to be more specific - Change 'FeedItem JSON' to 'PushNotificationProviderConfig JSON' for clarity
- Removed unused import statement for PushNotificationSubscription - This change simplifies the code and removes an unnecessary dependency
Status
READY
Description
This pull request lays the groundwork for a comprehensive and flexible push notification system within the application. It introduces all necessary data models, enums, and configuration structures to support user-defined notification subscriptions, device registration, and dynamic management of notification providers and delivery types. This feature will significantly enhance user engagement through personalized and timely alerts, all controllable via remote configuration.
Type of Change