-
Notifications
You must be signed in to change notification settings - Fork 16
feat: add Firebase basic configuration for push notifications #391
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
WalkthroughAdds Firebase and Firebase Cloud Messaging configuration and initialization support to the app: platform-specific Firebase options, Android Google Services plugin and JSON, new dependencies, project mappings, and documentation including a phased FCM implementation and Linux-specific notes. Changes
Sequence Diagram(s)(Skipped — changes are primarily configuration, generated options, and documentation; no new multi-component runtime control flow was introduced in code.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks✅ Passed checks (3 passed)
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. Comment |
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.
Actionable comments posted: 2
🧹 Nitpick comments (4)
android/app/google-services.json (1)
1-29: Firebase configuration is correctly structured.This
google-services.jsonfile follows the standard Firebase Android configuration format. The API key flagged by Gitleaks is a Firebase client identifier designed for public distribution—Firebase security is enforced through server-side Security Rules, not API key secrecy. This is safe to commit as documented in Firebase best practices.Optional: Add newline at end of file.
For consistency with POSIX standards, consider adding a trailing newline at the end of the file.
docs/FIREBASE_LINUX_NOTE.md (1)
11-27: Specify language identifier for code fence.The Dart code example is well-structured and demonstrates the correct platform-check pattern. For consistency with markdown conventions, add
dartlanguage identifier to the opening code fence.-```dart +``` import 'dart:io' show Platform;docs/FCM_IMPLEMENTATION.md (2)
55-84: Specify language identifier for architecture diagram code fence.The ASCII architecture diagram is clear and helpful. For consistency with markdown linting standards, add a language identifier (such as
textorascii) to the opening fence.-``` +```text ┌─────────────────┐
88-91: Remove redundant phrase for clarity.Line 90 contains a redundant phrase: "facilitate easier" should be simplified to "facilitate".
-The implementation is divided into multiple phases (Pull Requests) to facilitate easier code review and incremental testing. +The implementation is divided into multiple phases (Pull Requests) to facilitate code review and incremental testing.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
.firebaserc.gitignoreandroid/app/build.gradleandroid/app/google-services.jsonandroid/settings.gradledocs/FCM_IMPLEMENTATION.mddocs/FIREBASE_LINUX_NOTE.mdfirebase.jsonlib/firebase_options.dartpubspec.yaml
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{dart,flutter}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{dart,flutter}: Runflutter analyzeafter any code change - Mandatory before commits to ensure zero linting issues
Runflutter testafter any code change - Mandatory before commits to ensure all unit tests pass
Files:
lib/firebase_options.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.dart: Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
All code comments must be in English - use clear, concise English for variable names, function names, and comments
Always checkmountedbefore using BuildContext after async operations to prevent errors on disposed widgets
Useconstconstructors where possible for better performance and immutability
Remove unused imports and dependencies to maintain code cleanliness and reduce build size
**/*.dart: Application code should be organized underlib/, grouped by domain withlib/features/<feature>/structure, shared utilities inlib/shared/, dependency wiring inlib/core/, and services inlib/services/
Persistence, APIs, and background jobs should live inlib/data/andlib/background/; generated localization output must be inlib/generated/and must stay untouched
Applyflutter format .to enforce canonical Dart formatting (two-space indentation, trailing commas) before committing
Resolve every analyzer warning in Dart code
Name Riverpod providers using the<Feature>Provideror<Feature>Notifierconvention
Localize all user-facing strings via ARB files and access them withS.of(context)rather than hard-coded literals
Files:
lib/firebase_options.dart
🧠 Learnings (13)
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to android/local.properties : Local file: git-ignored, generated by CI or locally, includes `flutter.minSdkVersion=23` to prevent build.gradle auto-modifications, never commit this file or secrets
Applied to files:
android/settings.gradleandroid/app/build.gradle.gitignorefirebase.jsonandroid/app/google-services.json
📚 Learning: 2025-09-17T20:45:32.468Z
Learnt from: grunch
Repo: MostroP2P/mobile PR: 306
File: docs/architecture/REQUEST_ID_ANALYSIS.md:176-183
Timestamp: 2025-09-17T20:45:32.468Z
Learning: For PR #306 in MostroP2P/mobile repository: This is a documentation-only PR intended to explain how the current requestId system works, not to fix any bugs or issues in the code. The documentation should accurately reflect existing behavior.
Applied to files:
docs/FCM_IMPLEMENTATION.md
📚 Learning: 2025-09-17T20:45:07.179Z
Learnt from: grunch
Repo: MostroP2P/mobile PR: 306
File: docs/architecture/REQUEST_ID_ANALYSIS.md:114-118
Timestamp: 2025-09-17T20:45:07.179Z
Learning: For the MostroP2P/mobile project, grunch prefers to separate documentation PRs from code improvement PRs. Documentation PRs should focus on explaining how the current system works, while code fixes and improvements should be handled in separate PRs.
Applied to files:
docs/FCM_IMPLEMENTATION.md
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: All changes must pass `flutter analyze` with zero issues before committing to ensure code quality standards are maintained
Applied to files:
pubspec.yaml
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to **/*.dart : Remove unused imports and dependencies to maintain code cleanliness and reduce build size
Applied to files:
pubspec.yaml
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to **/*.{g.dart,mocks.dart} : Never manually edit generated files (`.g.dart`, `.mocks.dart`) or add individual ignore comments - regenerate with `dart run build_runner build -d` instead
Applied to files:
.gitignore
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to **/*.{js,ts,tsx,jsx} : This Flutter mobile project uses Dart and Flutter - JavaScript/TypeScript guidelines do not apply
Applied to files:
.gitignorefirebase.json
📚 Learning: 2025-11-27T12:10:26.407Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T12:10:26.407Z
Learning: Applies to **/*.dart : Persistence, APIs, and background jobs should live in `lib/data/` and `lib/background/`; generated localization output must be in `lib/generated/` and must stay untouched
Applied to files:
.gitignore
📚 Learning: 2025-11-27T12:10:26.407Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T12:10:26.407Z
Learning: Applies to **/*.dart : Apply `flutter format .` to enforce canonical Dart formatting (two-space indentation, trailing commas) before committing
Applied to files:
.gitignore
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to **/*.{dart,flutter} : Run `flutter test` after any code change - Mandatory before commits to ensure all unit tests pass
Applied to files:
.gitignore
📚 Learning: 2025-11-27T12:10:26.407Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T12:10:26.407Z
Learning: Applies to **/*.dart : Application code should be organized under `lib/`, grouped by domain with `lib/features/<feature>/` structure, shared utilities in `lib/shared/`, dependency wiring in `lib/core/`, and services in `lib/services/`
Applied to files:
.gitignore
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to **/*.{dart,flutter} : Run `flutter analyze` after any code change - Mandatory before commits to ensure zero linting issues
Applied to files:
.gitignore
📚 Learning: 2025-08-15T01:37:12.243Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 270
File: lib/shared/widgets/order_filter.dart:133-135
Timestamp: 2025-08-15T01:37:12.243Z
Learning: The MostroP2P/mobile project requires Flutter >=3.27.0 as specified in pubspec.yaml, which supports the Color.withValues() method, so usage of withValues() throughout the codebase is valid and should not be flagged as a compatibility issue.
Applied to files:
firebase.json
🪛 Gitleaks (8.30.0)
lib/firebase_options.dart
[high] 47-47: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
[high] 56-56: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
[high] 64-64: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
[high] 73-73: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
[high] 82-82: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
android/app/google-services.json
[high] 18-18: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
🪛 LanguageTool
docs/FCM_IMPLEMENTATION.md
[style] ~90-~90: This phrase is redundant. Consider writing “facilitate”.
Context: ...into multiple phases (Pull Requests) to facilitate easier code review and incremental testing. -...
(FACILITATE_EASIER)
🪛 markdownlint-cli2 (0.18.1)
docs/FCM_IMPLEMENTATION.md
55-55: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (7)
.firebaserc (1)
1-5: LGTM!The Firebase project configuration is correctly formatted and aligns with the project setup across other Firebase configuration files.
android/app/build.gradle (1)
3-5: LGTM!The Google Services plugin is correctly applied in the plugins block, completing the Firebase setup for Android. The placement and configuration align with FlutterFire CLI conventions.
firebase.json (1)
1-1: LGTM!This FlutterFire configuration file is correctly generated and maps all supported platforms (Android, iOS, macOS, Web, Windows) to their respective Firebase configurations. As a generated file, it should not be manually edited—regenerate using FlutterFire CLI if changes are needed.
lib/firebase_options.dart (3)
1-9: LGTM! Generated file with appropriate metadata.The file header correctly identifies this as FlutterFire CLI-generated code with proper lint suppression and a helpful note about Linux being unsupported. As a generated file, this should not be manually edited.
20-44: LGTM! Platform detection logic is sound.The
currentPlatformgetter correctly handles web detection first (viakIsWeb) before switching ondefaultTargetPlatform. The Linux case appropriately throwsUnsupportedErrorwith a helpful message, and the default case provides a safety net for unexpected platforms.
46-89: Firebase API keys are client identifiers, not secrets.The five API keys flagged by Gitleaks are Firebase client identifiers designed for public distribution and safe to commit. Firebase security is enforced through server-side Security Rules, App Check, and other mechanisms—not through API key secrecy. This is standard Firebase practice and aligns with the PR's intention to commit "public credentials."
.gitignore (1)
48-48: Removing google-services.json and GoogleService-Info.plist from .gitignore aligns with Firebase best practices.These files contain Firebase client-side API keys, which are identifiers designed for public distribution and safe to commit to repositories. Per current Firebase security guidance, the critical protections are server-side: ensure Cloud Firestore, Realtime Database, and Cloud Storage are secured with Firebase Security Rules, implement Firebase App Check to restrict which apps can call your backend, and monitor API key usage quotas. True secrets (server API keys, service account JSON) must be kept out of version control and stored in environment variables or secret managers instead.
grunch
left a comment
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.
great first PR of FCM, please give a look on my comments and also check code rabbit requests
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/FCM_IMPLEMENTATION.md (2)
170-170: Add language specification to fenced code block.The ASCII art diagram should specify a language for the fenced code block per Markdown best practices.
🔎 Proposed fix
-``` +```text ┌─────────────────┐
205-205: Remove redundant "easier" from phrase.The word "facilitate" already implies making easier; "easier" is redundant.
🔎 Proposed fix
-The implementation is divided into multiple phases (Pull Requests) to facilitate easier code review and incremental testing. +The implementation is divided into multiple phases (Pull Requests) to facilitate code review and incremental testing.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
android/settings.gradledocs/FCM_IMPLEMENTATION.mdpubspec.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- android/settings.gradle
- pubspec.yaml
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-17T20:45:32.468Z
Learnt from: grunch
Repo: MostroP2P/mobile PR: 306
File: docs/architecture/REQUEST_ID_ANALYSIS.md:176-183
Timestamp: 2025-09-17T20:45:32.468Z
Learning: For PR #306 in MostroP2P/mobile repository: This is a documentation-only PR intended to explain how the current requestId system works, not to fix any bugs or issues in the code. The documentation should accurately reflect existing behavior.
Applied to files:
docs/FCM_IMPLEMENTATION.md
📚 Learning: 2025-09-17T20:45:07.179Z
Learnt from: grunch
Repo: MostroP2P/mobile PR: 306
File: docs/architecture/REQUEST_ID_ANALYSIS.md:114-118
Timestamp: 2025-09-17T20:45:07.179Z
Learning: For the MostroP2P/mobile project, grunch prefers to separate documentation PRs from code improvement PRs. Documentation PRs should focus on explaining how the current system works, while code fixes and improvements should be handled in separate PRs.
Applied to files:
docs/FCM_IMPLEMENTATION.md
🪛 LanguageTool
docs/FCM_IMPLEMENTATION.md
[style] ~205-~205: This phrase is redundant. Consider writing “facilitate”.
Context: ...into multiple phases (Pull Requests) to facilitate easier code review and incremental testing. -...
(FACILITATE_EASIER)
🪛 markdownlint-cli2 (0.18.1)
docs/FCM_IMPLEMENTATION.md
170-170: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
docs/FCM_IMPLEMENTATION.md (2)
1-127: Excellent integration of all past feedback on MIP-05 overview and phase status.The detailed breakdown of implemented/partially-implemented/not-implemented aspects and clear explanations of why (lines 7-127) directly addresses the previous comment about explaining which MIP-05 aspects are being implemented. Phase status indicators are properly present throughout. Well-structured documentation.
1-486: Comprehensive documentation successfully addressing all past feedback.This documentation file effectively addresses all three previous review comments: MIP-05 overview (lines 7-127), phase status indicators (lines 106-109), and mostro-push-server repository references. The file provides thorough explanation of the privacy-preserving approach, implementation plan, architecture, and testing strategy. Only minor linting issues remain (code block language and redundant phrasing).
grunch
left a comment
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.
LGTM
This PR sets up the basic Firebase infrastructure for push notifications in the MostroP2P mobile app.
Changes
mostro-mobilegoogle-services.jsonin Git (public credentials - safe to commit)firebase_coreandfirebase_messagingdependenciesImplementation Notes
Next Steps
See docs/FCM_IMPLEMENTATION.md for the complete 4-phase implementation plan.
Testing
flutter analyzepasses without errorsSummary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.