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: update native module to initialize & configure SDK using Datapipeline (iOS) #299

Merged

Conversation

ami-aman
Copy link
Collaborator

Linear ticket : https://linear.app/customerio/issue/MBL-446/[ios]-update-native-module-to-initialize-and-configure-sdk-using

Problem

As a developer, I want to be able to consume customerio-reactnative package in my application without any error. I want to update the native module to initialize and configure the SDK using Datapipeline so that I can ensure seamless integration and configuration of the SDK with the latest data pipeline functionalities using iOS SDK.

Solution

  • Updated podspec to use CioDataPipeline and remove CioTracking module
    customerio-reactnative.podspec now uses CioDataPipeline and no longer uses CioTracking
  • Updated Native modules to use DataPipeline
    Native module code has been updated to use CioDataPipeline and removes CioTracking where ever used
  • Configuration update
    With data ppiline module, updated configurations have been used
  • Removed deprecated/outdated code
    Removed the old tracking module references and any associated code that is no longer necessary with the use of the DataPipeline module.
  • Updated package.json
    To help the customer use specific & strict iOS SDK version, we use cioNativeiOSSdkVersion attribute in package's package.json file. This has been updated to use latest iOS SDK version i.e. 3.3.3 providing CDP support.
  • TODOs have been added
    Todo(s) have been added to respective files wherever necessary, to be removed later when respective features are implemented
  • Sample app updates
    Sample app has been updated (FCM for now, as APN is failing to build for all devs. Need to fix this in another PR)

Testing and Validation

Completed development testing with no additional test steps required, as this update is focused solely on the initialization of the SDK. Added identify code to further test the feature, confirming that it works as expected.
Steps:

  • Launch React native FCM sample app
  • Identify a user either by creating a new one or by randomly generating one

Test user created - https://fly.customer.io/workspaces/122175/journeys/people/bfba070080048104

@ami-aman ami-aman requested a review from a team August 14, 2024 13:26
@ami-aman ami-aman requested review from Shahroz16 and a team August 16, 2024 07:15
Copy link
Contributor

@mrehan27 mrehan27 left a comment

Choose a reason for hiding this comment

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

Looks good overall

import React

func flush() {
#if DEBUG
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the debug flag is added here? Do we intend to not perform any action on production for this method?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, this is intended to be called only in debug/development build to help testing.

ios/wrappers/CioRctWrapper.swift Show resolved Hide resolved
@objc
func clearIdentify() {
CustomerIO.shared.clearIdentify()
flush()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we flushing after every call?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is meant to facilitate testing on sample apps. Not a requirement though, but rather a method to help test the package more efficiently and quickly.

}
}

struct CioConfig {
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this be exposed to public? If yes, I would suggest to use full name instead i.e. CustomerIOConfig

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, this will be exposed to public. This is same as CustomerIOConfig as we have today, just a rename and has been called out in the tech proposal as well.

I’m okay with renaming it back to CustomerIOConfig if there’s a strong reason to do so!
For me, CioConfig is short and precisely describes the context such as within a module or codebase whenever we use “CIO” it is commonly understood to refer to Customer.io.

@ami-aman ami-aman merged commit ca7b870 into feature/cdp-milestone-one Aug 16, 2024
1 of 7 checks passed
@ami-aman ami-aman deleted the feature/cdp/native-initialisation-config branch August 16, 2024 10:57
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.

3 participants