WIP feat(fcm): Add Support for AndroidConfigV2#3205
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the Android V2 configuration (androidV2) for Firebase Cloud Messaging (FCM) payloads, supporting remote notifications and background sync. It includes new TypeScript interfaces, validation logic, and comprehensive unit tests. The feedback suggests refactoring the validateAndroidConfigV2 function to use the existing renameProperties utility for renaming remoteNotification and backgroundSync properties, which would improve consistency and simplify the code.
|
/gemini review |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the new Android Config V2 (androidV2) to the Firebase Admin Messaging API, deprecating the legacy android configuration. It adds several new interfaces and types to support remote notifications and background sync configurations, along with their corresponding validation logic and unit tests. Feedback on the changes highlights that the validateLightSettings helper function contains hardcoded error messages referencing the legacy android.notification path, which will produce misleading error messages when validation fails for androidV2 payloads.
| (notification as any).visibility = visibility; | ||
| } | ||
|
|
||
| validateLightSettings(notification.lightSettings); |
There was a problem hiding this comment.
The validateLightSettings function contains hardcoded error messages that reference 'android.notification.lightSettings'. When called from validateAndroidNotificationV2 (which validates the new androidV2 payload), any validation errors inside validateLightSettings will throw misleading error messages pointing to the legacy android.notification path instead of androidV2.remoteNotification.notification.\n\nConsider refactoring validateLightSettings to accept an optional path prefix parameter (e.g., defaulting to 'android.notification') so that it can dynamically report the correct path for both V1 and V2 configurations.
androidV2property toBaseMessageandMulticastMessageinterfacesAndroidConfigV2,AndroidRemoteNotificationConfig, andAndroidBackgroundSyncConfigtypesandroidV2properties, including nestedremoteNotificationandbackgroundSyncexclusivity constraintsandroidand newandroidV2payload configurationsandroidV2property serialization, types, and error constraints