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: add client info for react native user agent on ios #345

Closed
wants to merge 3 commits into from

Conversation

mrehan27
Copy link
Contributor

@mrehan27 mrehan27 commented Oct 9, 2024

part of MBL-540
blocked by customerio/customerio-ios#825

Changes

  • Added CIOClientInfo.json so native iOS SDK can generate a user agent based on it
  • Excluded clientVersion from CIOClientInfo.json as CFBundleShortVersionString in resources plist is synced with React Native package version, removing the need for version updates in multiple places
  • Updated customerio-reactnative-richpush.podspec to include same files and resources as the default customerio-reactnative.podspec
  • Updated sample apps Podfile to align more closely with the live implementation for NSE
  • Updated lock and project files

@mrehan27 mrehan27 self-assigned this Oct 9, 2024
Copy link

github-actions bot commented Oct 9, 2024

Sample app builds 📱

Below you will find the list of the latest versions of the sample apps. It's recommended to always download the latest builds of the sample apps to accurately test the pull request.


  • FCM: 345.3.0 (28807920)
  • APN: 345.3.0 (28807920)
  • FCM: 345.3.0 (28807978)

@@ -19,7 +19,7 @@ target 'SampleApp' do
flags = get_default_flags()

# TODO: Remove this once iOS Native SDK is released and revert to original code
install_non_production_ios_sdk_git_branch(branch_name: 'feature/react-native-cdp', is_app_extension: false, push_service: "apn")
install_non_production_ios_sdk_git_branch(branch_name: 'rehan/rn-ios-user-agent', is_app_extension: false, push_service: "apn")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: Revert this to feature branch once iOS PR is merged

# TODO: Remove this once iOS Native SDK is released and revert to original code
install_non_production_ios_sdk_git_branch(branch_name: 'feature/react-native-cdp', is_app_extension: true, push_service: "apn")
# pod 'customerio-reactnative-richpush/apn', :path => '../node_modules/customerio-reactnative'
pod 'customerio-reactnative-richpush/apn', :path => '../node_modules/customerio-reactnative'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since pods are already installed, I think we can revert this to original implementation. This should give us more confidence in the changes.

@@ -47,7 +47,7 @@ target 'FCMSampleApp' do
)

# TODO: Remove this once iOS Native SDK is released and revert to original code
install_non_production_ios_sdk_git_branch(branch_name: 'feature/react-native-cdp', is_app_extension: false, push_service: "fcm")
install_non_production_ios_sdk_git_branch(branch_name: 'rehan/rn-ios-user-agent', is_app_extension: false, push_service: "fcm")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: Revert this to feature branch once iOS PR is merged

@@ -15,17 +15,23 @@ Pod::Spec.new do |s|
s.platforms = { :ios => "13.0" }
s.source = { :git => "https://github.com/customerio/customerio-ios.git", :tag => "#{s.version}" }

s.source_files = "ios/**/*.{h,m,mm,swift}"
s.source_files = "ios/wrappers/**/*.{h,m,mm,swift}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated it to match with default podspec


# s.dependency "X", "X"
# Careful when declaring dependencies here. All dependencies will be included in the App Extension target in Xcode, not the host iOS app.

# Subspecs allow customers to choose between multiple options of what type of version of this rich push package they would like to install.
s.subspec 'apn' do |ss|
ss.dependency "CustomerIO/MessagingPushAPN", package["cioNativeiOSSdkVersion"]
end
ss.resource_bundles = {
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 tried keeping it in parent podspec to avoid duplication, but it didn't work for some reason. So added it to each subspec.


# s.dependency "X", "X"
# Careful when declaring dependencies here. All dependencies will be included in the App Extension target in Xcode, not the host iOS app.

# Subspecs allow customers to choose between multiple options of what type of version of this rich push package they would like to install.
s.subspec 'apn' do |ss|
ss.dependency "CustomerIO/MessagingPushAPN", package["cioNativeiOSSdkVersion"]
end
ss.resource_bundles = {
'CustomerIO_NSEResources' => ['ios/resources/*']
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having resources with the same name was causing failures in CI, so I added resource with different name for NSE to avoid any risks.

Copy link
Contributor

@Shahroz16 Shahroz16 left a comment

Choose a reason for hiding this comment

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

We discussed this on team call and we would not be opting for this choice for now.

Base automatically changed from feature/cdp-main to main October 16, 2024 20:46
@mrehan27
Copy link
Contributor Author

Closing this as we’ve decided to proceed with other solutions for now. We’ll continue discussing this internally and will open PRs once we finalize further changes.

@mrehan27 mrehan27 closed this Oct 17, 2024
@mrehan27 mrehan27 deleted the rehan/ios-user-agent branch October 17, 2024 06:37
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.

2 participants