-
Notifications
You must be signed in to change notification settings - Fork 459
add firebase integration #565
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git βοΈ
|
WalkthroughTwo new JSON configuration files have been introduced under Changes
Estimated code review effortπ― 2 (Simple) | β±οΈ ~7 minutes Poem
Note β‘οΈ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. β¨ Finishing Touchesπ§ͺ Generate unit tests
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. πͺ§ TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
β¨ No files to analyze in this PR. Need help? Join our Discord for support! |
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: 1
π§Ή Nitpick comments (3)
backend/apps/firebase/app.json (1)
8-19: Consider addingaccess_type=offline(or equivalent) to guarantee refresh-token issuanceIf this OAuth client will run in backend/CLI contexts youβll usually need a long-lived refresh token. Google only returns one when the authorization request explicitly asks for offline access (or includes
prompt=consent). Surfacing these parameters in the security-scheme block (or at least documenting the requirement) avoids surprise 401s once the short-lived access token expires.backend/apps/firebase/functions.json (2)
355-360:databaseIdis required but hidden from UI β add it tovisibleor drop fromrequired
FIREBASE__FIRESTORE_GET_DOCUMENTSdeclaresdatabaseIdas required yet omits it from thevisiblelist.
That means front-end generators will not render the input, forcing users to fall back on raw-JSON editing. Either:
- add
"databaseId"tovisible, or- keep it hidden but remove it from
requiredand rely on the default"(default)".
100-104: ExposepageTokento enable pagination when listing projects
pageTokenis optional but excluding it fromvisibleprevents callers from traversing result pages. Consider:- "visible": ["pageSize"], + "visible": ["pageSize", "pageToken"],This keeps strict whitelisting intact while supporting full pagination workflows.
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (2)
backend/apps/firebase/app.json(1 hunks)backend/apps/firebase/functions.json(1 hunks)
π§° Additional context used
π§ Learnings (2)
π Common learnings
Learnt from: jiwei-aipolabs
PR: aipotheosis-labs/aci#538
File: backend/apps/microsoft_calendar/functions.json:276-308
Timestamp: 2025-07-22T19:31:44.823Z
Learning: In backend/apps/microsoft_calendar/functions.json and similar API function definitions, the `additionalProperties: false` restriction on query objects is intentionally designed for strict validation, even if it blocks some legitimate OData/HTTP headers. The user prefers explicit whitelisting of parameters over flexible schema validation.
Learnt from: jiwei-aipolabs
PR: aipotheosis-labs/aci#540
File: backend/aci/server/app_connectors/microsoft_onedrive.py:85-87
Timestamp: 2025-07-22T11:42:10.132Z
Learning: User jiwei-aipolabs prefers concise code review suggestions and finds detailed error handling with multiple specific status codes too verbose.
backend/apps/firebase/functions.json (1)
Learnt from: jiwei-aipolabs
PR: #538
File: backend/apps/microsoft_calendar/functions.json:276-308
Timestamp: 2025-07-22T19:31:44.823Z
Learning: In backend/apps/microsoft_calendar/functions.json and similar API function definitions, the additionalProperties: false restriction on query objects is intentionally designed for strict validation, even if it blocks some legitimate OData/HTTP headers. The user prefers explicit whitelisting of parameters over flexible schema validation.
β° 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). (4)
- GitHub Check: cubic Β· AI code reviewer
- GitHub Check: Format, Lint, and Test
- GitHub Check: Compose Tests
- GitHub Check: Format & Lint
π Additional comments (3)
backend/apps/firebase/app.json (1)
21-28: LGTM β minimal, well-scoped app descriptorNaming, categorisation and placeholder secrets are all consistent with the rest of the connector catalogue.
backend/apps/firebase/functions.json (2)
440-470: Very restrictivestructuredQuerymay block common Firestore queriesOnly
fromandlimitare allowed; fields likewhere,orderBy,offset, etc. are disallowed byadditionalProperties: false.
If the intent is to start small, fine β but be aware that most real-world queries will require at leastwhere/orderBy.
1-567: Overall: schema is consistent with previous connectors and honours the strict-validation preferencePaths, HTTP methods, and
additionalProperties: falseusage align with your earlier Microsoft Calendar & OneDrive specs. Nice work keeping the catalogue uniform.
| "message": { | ||
| "type": "object", | ||
| "description": "The message to send.", | ||
| "properties": { | ||
| "token": { | ||
| "type": "string", | ||
| "description": "Registration token to send a message to." | ||
| }, | ||
| "topic": { | ||
| "type": "string", | ||
| "description": "Topic name to send a message to." | ||
| }, | ||
| "notification": { | ||
| "type": "object", | ||
| "description": "Notification payload.", | ||
| "properties": { | ||
| "title": { | ||
| "type": "string", | ||
| "description": "The notification's title." | ||
| }, | ||
| "body": { | ||
| "type": "string", | ||
| "description": "The notification's body text." | ||
| } | ||
| }, | ||
| "required": [], | ||
| "visible": ["title", "body"], | ||
| "additionalProperties": false | ||
| }, | ||
| "data": { | ||
| "type": "object", | ||
| "description": "Arbitrary key/value payload.", | ||
| "properties": {}, | ||
| "required": [], | ||
| "visible": [], | ||
| "additionalProperties": { | ||
| "type": "string" | ||
| } | ||
| } | ||
| }, | ||
| "required": [], | ||
| "visible": ["token", "topic", "notification", "data"], | ||
| "additionalProperties": false | ||
| } | ||
| }, |
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.
token OR topic must be required β current schema permits sending an empty message and will 400
The FCM messages:send endpoint rejects a request unless at least one target (token, topic, or condition) is supplied.
Your schema marks every field optional, so nothing prevents a caller from posting { "message": {} }, leading to a runtime failure that static validation could have caught.
@@
"additionalProperties": false
}
},
- "required": [],
+ "required": [],
+ "oneOf": [
+ { "required": ["token"] },
+ { "required": ["topic"] }
+ ],
"visible": ["token", "topic", "notification", "data"],
"additionalProperties": falseYou may also want to surface the condition field for parity with the REST API.
π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "message": { | |
| "type": "object", | |
| "description": "The message to send.", | |
| "properties": { | |
| "token": { | |
| "type": "string", | |
| "description": "Registration token to send a message to." | |
| }, | |
| "topic": { | |
| "type": "string", | |
| "description": "Topic name to send a message to." | |
| }, | |
| "notification": { | |
| "type": "object", | |
| "description": "Notification payload.", | |
| "properties": { | |
| "title": { | |
| "type": "string", | |
| "description": "The notification's title." | |
| }, | |
| "body": { | |
| "type": "string", | |
| "description": "The notification's body text." | |
| } | |
| }, | |
| "required": [], | |
| "visible": ["title", "body"], | |
| "additionalProperties": false | |
| }, | |
| "data": { | |
| "type": "object", | |
| "description": "Arbitrary key/value payload.", | |
| "properties": {}, | |
| "required": [], | |
| "visible": [], | |
| "additionalProperties": { | |
| "type": "string" | |
| } | |
| } | |
| }, | |
| "required": [], | |
| "visible": ["token", "topic", "notification", "data"], | |
| "additionalProperties": false | |
| } | |
| }, | |
| "message": { | |
| "type": "object", | |
| "description": "The message to send.", | |
| "properties": { | |
| "token": { | |
| "type": "string", | |
| "description": "Registration token to send a message to." | |
| }, | |
| "topic": { | |
| "type": "string", | |
| "description": "Topic name to send a message to." | |
| }, | |
| "notification": { | |
| "type": "object", | |
| "description": "Notification payload.", | |
| "properties": { | |
| "title": { | |
| "type": "string", | |
| "description": "The notification's title." | |
| }, | |
| "body": { | |
| "type": "string", | |
| "description": "The notification's body text." | |
| } | |
| }, | |
| "required": [], | |
| "visible": ["title", "body"], | |
| "additionalProperties": false | |
| }, | |
| "data": { | |
| "type": "object", | |
| "description": "Arbitrary key/value payload.", | |
| "properties": {}, | |
| "required": [], | |
| "visible": [], | |
| "additionalProperties": { | |
| "type": "string" | |
| } | |
| } | |
| }, | |
| "required": [], | |
| "oneOf": [ | |
| { "required": ["token"] }, | |
| { "required": ["topic"] } | |
| ], | |
| "visible": ["token", "topic", "notification", "data"], | |
| "additionalProperties": false | |
| } |
π€ Prompt for AI Agents
In backend/apps/firebase/functions.json between lines 514 and 558, the message
schema currently allows all target fields (token, topic) to be optional, which
permits sending an empty message and causes a 400 error at runtime. Update the
schema to require at least one of token, topic, or condition fields by making
them conditionally required or adding a validation rule that enforces this.
Also, add the condition field to the message properties to match the FCM REST
API and ensure proper validation before sending messages.
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.
cubic analysis
1 issue found across 2 files β’ Review in cubic
React with π or π to teach cubic. You can also tag @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| "active": true, | ||
| "protocol": "rest", | ||
| "protocol_data": { | ||
| "method": "GET", |
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.
documents:listCollectionIds must be called with POST, not GET, otherwise the request will fail at runtime.
Prompt for AI agents
Address the following comment on backend/apps/firebase/functions.json at line 44:
<comment>documents:listCollectionIds must be called with POST, not GET, otherwise the request will fail at runtime.</comment>
<file context>
@@ -0,0 +1,569 @@
+[
+ {
+ "name": "FIREBASE__CREATE_PROJECT",
+ "description": "Adds Firebase to an existing Google Cloud project. Note: This requires Firebase project creation permissions.",
+ "tags": ["project", "create"],
+ "visibility": "public",
+ "active": true,
+ "protocol": "rest",
+ "protocol_data": {
</file context>
π·οΈ Ticket
https://www.notion.so/Firebase-Integration-23b8378d6a4780c194b3e3878587380d?source=copy_link
π Description
[Describe your changes in detail (optional if the issue you linked already contains a
detail description of the change)]
π₯ Demo (if applicable)
πΈ Screenshots (if applicable)
β Checklist
Summary by cubic
Added Firebase integration with support for project management, web app creation, database setup, Firestore operations, and messaging.
Summary by CodeRabbit
New Features
Chores