Skip to content
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

chore: FCM base implementation #217

Open
wants to merge 3 commits into
base: feature/fcm-support
Choose a base branch
from

Conversation

mahmoud-elmorabea
Copy link
Contributor

@mahmoud-elmorabea mahmoud-elmorabea commented Mar 3, 2025

This PR adds the basic support for customers to be able to use FCM as push provider.

@mahmoud-elmorabea mahmoud-elmorabea self-assigned this Mar 3, 2025
@mahmoud-elmorabea mahmoud-elmorabea force-pushed the MBL-885-fcm-support-base-impl branch from d644763 to eeffc32 Compare March 3, 2025 03:17
@mahmoud-elmorabea mahmoud-elmorabea force-pushed the MBL-885-fcm-support-base-impl branch from f4db990 to 53b596a Compare March 3, 2025 04:56
sd "\"cdpApiKey\".*" "\"cdpApiKey\": \"${{ secrets.CUSTOMERIO_EXPO_WORKSPACE_CDP_API_KEY }}\"," "$APP_JSON_FILE"
sd "\"siteId\".*" "\"siteId\": \"${{ secrets.CUSTOMERIO_EXPO_WORKSPACE_SITE_ID }}\"," "$APP_JSON_FILE"
sd "\"cdpApiKey\".*" "\"cdpApiKey\": \"${{ secrets.TMP_CUSTOMERIO_EXPO_FCM_WORKSPACE_CDP_API_KEY }}\"," "$APP_JSON_FILE"
sd "\"siteId\".*" "\"siteId\": \"${{ secrets.TMP_CUSTOMERIO_EXPO_FCM_WORKSPACE_SITE_ID }}\"," "$APP_JSON_FILE"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is temporary to be able to test to connect test-app on Firebase to a workspace that supports FCM on iOS.

@@ -0,0 +1,7 @@
import Foundation
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have chosen to have separate files for FCM and APN.

  • This does have some duplication but it makes it easier to understand what code we generate for FCM and APN
  • Prevents us from having placeholders and regex logic in our own created files

const snippetToInjectInPodfile = `
${blockStart}
target 'NotificationService' do
${useFrameworks === 'static' ? 'use_frameworks! :linkage => :static' : ''}
pod 'customerio-reactnative-richpush/apn', :path => '${getRelativePathToRNSDK(
pod 'customerio-reactnative-richpush/${useFcm ? "fcm" : "apn"}', :path => '${getRelativePathToRNSDK(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I plan to create a global util to centralize the logic to check if push provider is FCM or APN

stringContents = injectCodeByLineNumber(
stringContents,
0,
'@protocol FIRMessagingDelegate;'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This solves an issue we had caused by FIRMessagingDelegate not being found during compilation.

We are adding this "Forward Declaration" here to indicate to the compiler that this type will be found later during linking/runtime.

Thanks @Ahmed-CIO for the suggestion!

"expo-build-properties",
{
"ios": {
"useFrameworks": "static"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommended setup from Firebase.

@mahmoud-elmorabea mahmoud-elmorabea marked this pull request as ready for review March 3, 2025 05:10
@mahmoud-elmorabea mahmoud-elmorabea requested a review from a team as a code owner March 3, 2025 05:10
@mahmoud-elmorabea mahmoud-elmorabea changed the base branch from beta to feature/fcm-support March 3, 2025 05:11
@Shahroz16 Shahroz16 requested a review from Copilot March 3, 2025 21:13

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This PR implements basic support for using FCM as a push provider in the iOS project. Key changes include adding a new plugin to copy the FCM Google services file, updating Xcode project modifications to include FCM-specific configurations, and updating related Podfile injection utilities.

Reviewed Changes

File Description
plugin/src/ios/withGoogleServicesJsonFile.ts New plugin to copy and add GoogleService-Info.plist when FCM is used
plugin/src/ios/withXcodeProject.ts Updated to pass push provider argument to Podfile injection
plugin/src/ios/withNotificationsXcodeProject.ts Modified to select FCM or APN resource files based on provider
plugin/src/helpers/utils/injectCIOPodfileCode.ts Updated to inject proper customerio-reactnative Podfile snippet based on provider
plugin/src/ios/withAppDelegateModifications.ts Added Firebase delegate forward declaration when using FCM
plugin/src/ios/withCIOIos.ts Included the new Google services file plugin in the overall configuration
plugin/src/types/cio-types.ts Extended iOS plugin options with new provider and googleServicesFile fields
.github/workflows/reusable_build_sample_apps.yml Updated secret references for the FCM workspace

Copilot reviewed 24 out of 24 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

plugin/src/ios/withGoogleServicesJsonFile.ts:32

  • The error log in the catch block incorrectly references 'google-services.json' instead of the actual file 'GoogleService-Info.plist' being copied. Update the log to reference the correct file name.
console.log(`There was an error copying your google-services.json file. You can copy it manually into ${iosPath}/google-services.json`);

plugin/src/ios/withGoogleServicesJsonFile.ts:37

  • The log in the else block incorrectly instructs to copy the file into 'google-services.json' instead of 'GoogleService-Info.plist'. Please update the log message for consistency.
console.log(`The Google Services file provided in ${googleServicesFile} doesn't seem to exist. You can copy it manually into ${iosPath}/google-services.json`);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant