Added notifications #33
Conversation
Reviewer's Guide by SourceryThis PR implements Firebase Cloud Messaging (FCM) and local notifications in the Flutter application. The implementation includes Firebase initialization, notification handling for both foreground and background messages, local storage setup using Hive, and proper Android configuration for notifications. Sequence diagram for notification handlingsequenceDiagram
participant User
participant App
participant Firebase
participant LocalStorage
participant NotificationConfig
User->>App: Open App
App->>Firebase: Initialize Firebase
Firebase-->>App: Initialization Complete
App->>Firebase: Register Notification
Firebase-->>App: Notification Token
App->>Firebase: Check for Initial Message
Firebase-->>App: Initial Message (if any)
App->>Firebase: Listen for Notifications
Firebase-->>App: Notification Received
App->>NotificationConfig: Show Notification
NotificationConfig->>LocalStorage: Add Notification to Storage
Class diagram for Firebase and Notification configurationclassDiagram
class FirebaseConfig {
+initialize()
+listenNotification()
+checkForInitialMessage()
+registerNotification()
+_firebaseMessagingBackgroundHandler()
}
class NotificationConfig {
+initialize()
+showNotification(RemoteMessage message)
}
class LocalStorage {
+addNotification(notification)
+getNotifications()
}
class Storage {
+initialize()
}
FirebaseConfig --> NotificationConfig : uses
FirebaseConfig --> LocalStorage : uses
FirebaseConfig --> Storage : uses
NotificationConfig --> LocalStorage : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Abhimanyu-dev - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Avoid logging sensitive FCM tokens in production code (link)
- Hardcoded Firebase API key found. (link)
- Hardcoded Firebase API key found in google-services.json. (link)
Overall Comments:
- Please remove sensitive information (API keys, Firebase config) from version control and use environment variables or a secure configuration management system instead.
- The PR description sections are empty. Please fill out the description, motivation, and testing sections to help reviewers understand the changes.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🔴 Security: 3 blocking issues
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| import 'firebase_options.dart'; | ||
|
|
||
| class FirebaseConfig { | ||
| static Future<void> initialize() async { |
There was a problem hiding this comment.
suggestion: Consider adding error handling for Firebase initialization
The initialization could fail for various reasons (network issues, invalid config). Consider wrapping this in a try-catch block and handling potential errors appropriately.
static Future<void> initialize() async {
try {
await Firebase.initializeApp();
} catch (e) {
throw FirebaseException(
plugin: 'firebase_core',
message: 'Failed to initialize Firebase: $e',
);
}
| print(status); | ||
| } | ||
| var messaging = FirebaseMessaging.instance; | ||
| print("FCM TOKEN: ${await messaging.getToken()}"); |
There was a problem hiding this comment.
🚨 issue (security): Avoid logging sensitive FCM tokens in production code
The FCM token should be treated as sensitive information. Consider removing this log or limiting it to debug builds with more secure logging practices.
| ); | ||
| } | ||
|
|
||
| static void listenNotification() { |
There was a problem hiding this comment.
issue: Empty notification listener could miss important events
The empty listener for onMessageOpenedApp means you're not handling notifications when the app is opened from a notification tap.
| await flutterLocalNotificationsPlugin.initialize(initializationsSettings); | ||
| } | ||
|
|
||
| static Future showNotification(RemoteMessage message) async { |
There was a problem hiding this comment.
suggestion: Consider refactoring duplicate notification configuration code
The AndroidNotificationDetails configuration is largely duplicated. Consider extracting common configuration into a separate method.
static Future<void> showNotification(RemoteMessage message, {AndroidNotificationDetails? androidDetails}) async {
| } | ||
|
|
||
| static const FirebaseOptions android = FirebaseOptions( | ||
| apiKey: 'AIzaSyATmGPGCnhEzncmC9lYhixCTn0jvXrhykA', |
There was a problem hiding this comment.
🚨 issue (security): Hardcoded Firebase API key found.
This Firebase API key is hardcoded in the source code. Consider using environment variables or a secure vault to manage API keys.
| "oauth_client": [], | ||
| "api_key": [ | ||
| { | ||
| "current_key": "AIzaSyATmGPGCnhEzncmC9lYhixCTn0jvXrhykA" |
There was a problem hiding this comment.
🚨 issue (security): Hardcoded Firebase API key found in google-services.json.
The Firebase API key is present in the google-services.json file. Ensure this file is not exposed publicly and consider using environment variables for sensitive information.
Description
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist:
Summary by Sourcery
Implement a Firebase-based notification system, including initialization and handling of notifications, and update the Android package configuration.
New Features:
Enhancements:
Build:
Deployment: