-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor/push notification #72
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
…rTokens map - Remove `token` and `provider` properties - Add `providerTokens` map to store tokens for multiple providers - Update JSON serialization and deserialization for the new `providerTokens` map - Adjust `props` and `copyWith` methods to accommodate the new change
- Remove FirebaseProviderConfig, OneSignalProviderConfig, and PushNotificationProviderConfig - Update imports and exports in config.dart - Simplify PushNotificationConfig by removing providerConfigs
- Removed hardcoded example values for push notification configurations - This change improves security by eliminating sensitive data in the codebase - It also enhances code clarity by removing specific configuration examples
- Remove FirebaseProviderConfig tests - Remove OneSignalProviderConfig tests - Remove PushNotificationProviderConfig tests - All related test files have been deleted
- Remove providerConfigs from the equality comparison in PushNotificationConfig test - Exclude providerConfigs when copying and comparing PushNotificationConfig instances
- Replace single token and provider with map of provider tokens - Update test cases to reflect new providerTokens structure - Adjust JSON representation to include multiple provider tokens - Modify copyWith test to use new providerTokens map
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 introduces a significant refactoring of the push notification system's configuration and device models. The primary goal is to delegate the management of push notification providers to the backend and centralizing the storage of provider-specific device tokens within the 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 significant and beneficial refactoring of the push notification configuration. By removing provider-specific credentials from the remote config, you've greatly improved security. The change to PushNotificationDevice to support multiple provider tokens per device is a smart move that increases flexibility. The code is clean, and the changes are consistently applied across all relevant models, fixtures, and tests. I have one minor suggestion to clean up a comment.
- Remove incorrect dartdoc tag 'push_notification_provider_config' - Improve documentation formatting for PushNotificationConfig class
Status
READY
Description
This pull request introduces a significant refactoring of the push notification system's configuration and device models. The primary goal is to delegate the management of push notification providers to the backend and centralizing the storage of provider-specific device tokens within the
PushNotificationDevicemodel itself. This change streamlines the data structures, making the system more cohesive and easier to maintain, albeit introducing breaking changes due to the altered data models.Type of Change