feat: added SDK config #1

Shahroz16 · 2022-11-15T10:14:11Z

closes: https://github.com/customerio/issues/issues/8530

ami-aman · 2022-11-15T12:00:32Z

ios/Classes/SwiftCustomerIoPlugin.swift

+    }
+
+    private func initialize(params : Dictionary<String, Any>){
+        guard let siteId = params[Keys.Environment.siteId] as? String,


Do we expect organisation id in these parameters?

we will have it in next PRs

ami-aman · 2022-11-15T12:03:10Z

ios/Classes/SwiftCustomerIoPlugin.swift

+    private func setUserAgentClient(params : Dictionary<String, Any>){
+        let version = params[Keys.PackageConfig.version] as? String ?? "n/a"
+        let sdkSource = SdkWrapperConfig.Source.flutter
+        CustomerIO.config {


Set SDK configuration during initialisation only.

sure will do

ami-aman · 2022-11-15T12:05:27Z

ios/Classes/SwiftCustomerIoPlugin.swift

+    }
+
+    private func setupConfig(params : Dictionary<String, Any>){
+        CustomerIO.config {


Set SDK configuration during initialisation and all attributes can be set in one go !

ami-aman · 2022-11-15T12:29:38Z

lib/customer_io_config.dart

+class CustomerIOConfig {
+  final String siteId;
+  final String apiKey;
+  Region region;


Region is also optional with a default value. I could be mistaken as this is dart and setting a default value could be done in a different way.

yup its optional, late in this file constructor is available which shows required attributes

CustomerIOConfig( {required this.siteId, required this.apiKey, this.region = Region.us, this.organizationId, this.logLevel = CioLogLevel.debug, this.autoTrackDeviceAttributes = true, this.trackingApiUrl = "", this.autoTrackPushEvents = true, this.backgroundQueueMinNumberOfTasks = 10, this.backgroundQueueSecondsDelay = 30.0, this.version = ""});

ami-aman · 2022-11-15T12:31:41Z

ios/Classes/Keys.swift

+//  customer_io
+//
+//  Created by ShahrozAli on 11/11/22.
+//


Can be ignored right now, but we would need to remove the headers across all files before final release.

The Swift format and swift lint tools that we use on iOS SDK do this for us.

We can configure git hooks like the Android SDK in this project that run the linter/formatter against this code?

Yup will move on to lint/hooks/ci/cd after the basic implementation is done. Since i am the only one working, i dont get any conflicts as such either.

levibostian · 2022-11-15T14:51:55Z

android/src/main/kotlin/io/customer/customer_io/CustomerIoPlugin.kt

-      result.success("Android ${android.os.Build.VERSION.RELEASE}")
-    } else {
-      result.notImplemented()
+class CustomerIoPlugin : FlutterPlugin, MethodCallHandler {


Suggested change

class CustomerIoPlugin : FlutterPlugin, MethodCallHandler {

class CustomerIOPlugin : FlutterPlugin, MethodCallHandler {

Usually when referring to CIO, we use CustomerIO instead of CustomerIo.

Since our team is not knowledgable with flutter, classes like this could benefit from more comment explaining what a Flutter plugin is?

levibostian · 2022-11-15T14:53:11Z

android/src/main/kotlin/io/customer/customer_io/CustomerIoPlugin.kt

+    /// The MethodChannel that will the communication between Flutter and native Android
+    ///
+    /// This local reference serves to register the plugin with the Flutter Engine and unregister it
+    /// when the Flutter Engine is detached from the Activity


Great comment! I appreciate that since our team doesn't know Flutter.

levibostian · 2022-11-15T14:54:00Z

android/src/main/kotlin/io/customer/customer_io/CustomerIoPlugin.kt

+    ///
+    /// This local reference serves to register the plugin with the Flutter Engine and unregister it
+    /// when the Flutter Engine is detached from the Activity
+    private lateinit var channel: MethodChannel


Suggested change

private lateinit var channel: MethodChannel

private lateinit var flutterCommunicationChannel: MethodChannel

I like the idea of having a more descriptive variable name so it's easy to understand that MethodChannel is specific to flutter and it's purpose.

levibostian · 2022-11-15T14:55:04Z

android/src/main/kotlin/io/customer/customer_io/CustomerIoPlugin.kt

+
+    override fun onAttachedToEngine(@NonNull flutterPluginBinding: FlutterPlugin.FlutterPluginBinding) {
+        context = flutterPluginBinding.applicationContext
+        channel = MethodChannel(flutterPluginBinding.binaryMessenger, "customer_io")


What is the purpose of "customer_io"? Is this string meant to be unique for the host Flutter app?

If so, should we use package name? io.customer.sdk.flutter?

Its the channel name, we can have multiple channels to communicate with the native platform and flutter, also the convention in dart is to use "_" rather than "."

levibostian · 2022-11-15T14:55:49Z

android/src/main/kotlin/io/customer/customer_io/CustomerIoPlugin.kt

+            "getPlatformVersion" -> {
+                result.success("Android-${android.os.Build.VERSION.RELEASE}")
+            }


Is this function just for your own testing or is this something we need to implement for a flutter plugin?

just for internal testing, will remove it afterwards.

levibostian · 2022-11-15T15:24:40Z

ios/customer_io.podspec

@@ -15,7 +15,8 @@ A plugin for Customer.io
  s.source           = { :path => '.' }
  s.source_files = 'Classes/**/*'
  s.dependency 'Flutter'
-  s.platform = :ios, '9.0'
+  s.dependency 'CustomerIO/Tracking'


Is this file used for the example Flutter iOS app? Or is this the file used for us to push the flutter SDK to cocoapods so our flutter customers can use it in their app?

If it's the cocoapods file that we use for publishing, I would like to make some changes to the file to match the naming style of our iOS SDK so the SDK is easier to find.

This file is used by iOS internally only, it fetches CustomerIO native SDK for us that we make native bindings with, but i'll dig down more once i start looking into publishing the flutter plugins.

levibostian · 2022-11-15T15:25:06Z

lib/customer_io.dart

 import 'customer_io_platform_interface.dart';

 class CustomerIo {
-  Future<String?> getPlatformVersion() {
-    return CustomerIoPlatform.instance.getPlatformVersion();
+  const CustomerIo._();


🤔 What is this for? Curious....

This is how you make singleton classes in dart

levibostian · 2022-11-15T15:25:48Z

lib/customer_io.dart

+    return _customerIO.getPlatformVersion();
+  }
+
+  static Future<void> initialize({


Ah, this must be why the other Dart file said await CustomerIO.initialize().

Does this function have to be a Future since the native SDK initialize() function isn't async?

This doesn't have to be Future but it's a common practice in plugin development that I have noticed, they return back with success/failures.

I believe it's for the sake of state management as if they want to perform all initializations first and then move on to the other screens, native platforms provide it themselves, but maybe Cross platform doesn't

levibostian · 2022-11-15T15:26:43Z

lib/customer_io_config.dart

+  final String siteId;
+  final String apiKey;


Can we move these 2 properties out of the config object and into parameters of Dart's CustomerIO.initialize() function? To match the native SDKs?

We can but then we will have to make another map that combines these values along with the other config, cause we can only pass 1 map from Flutter/React to native bindings.

Thats why we keep it in one class in both wrappers (RN/Flutter)

levibostian · 2022-11-15T15:28:22Z

lib/customer_io_config.dart

+import 'customer_io_enums.dart';
+
+/// Configure plugin using CustomerIOConfig
+class CustomerIOConfig {


Ah, bummer. Looks like we might need to keep a Dart equivalent to the native SDK CustomerIOConfig maintained?

Sounds like a lot of work that every time that we add a value to the Android or iOS SDK config object, we need to update it across RN SDK and Flutter SDK. But, I am not sure if there is a way around this?

Yup, that's the only way, we have to keep these wrapper updated with native SDK

android/src/main/kotlin/io/customer/customer_io/CustomerIoPlugin.kt

mrehan27 · 2022-11-21T13:47:43Z

android/src/main/kotlin/io/customer/customer_io/CustomerIoPlugin.kt

+        val logLevel = config.getProperty<String>(Keys.Config.LOG_LEVEL).toCIOLogLevel()
+        setLogLevel(level = logLevel)
+        config.getProperty<String>(Keys.Config.TRACKING_API_URL)?.takeIfNotBlank()?.let { value ->
+            setTrackingApiURL(value)
+        }
+        config.getProperty<Boolean>(Keys.Config.AUTO_TRACK_DEVICE_ATTRIBUTES)?.let { value ->
+            autoTrackDeviceAttributes(shouldTrackDeviceAttributes = value)
+        }
+        config.getProperty<Double>(Keys.Config.BACKGROUND_QUEUE_MIN_NUMBER_OF_TASKS)?.let { value ->
+            setBackgroundQueueMinNumberOfTasks(backgroundQueueMinNumberOfTasks = value.toInt())
+        }
+        config.getProperty<Double>(Keys.Config.BACKGROUND_QUEUE_SECONDS_DELAY)?.let { value ->
+            setBackgroundQueueSecondsDelay(backgroundQueueSecondsDelay = value)
+        }


I think we can support initialization through maps in Native SDKs to help in this for both React Native and Flutter. But we need to be careful and not rename any key used by these wrappers or change its type to avoid breaking this.

matt-frizzell

Looks good enough to push through on

mrehan27

LGTM 👍🏻

## 1.0.0-alpha.1 (2022-12-22) ### Features * added SDK config ([#1](#1)) ([e8ed7dd](e8ed7dd)) * tracking and in-app added ([#2](#2)) ([c23f2d9](c23f2d9)) ### Bug Fixes * typo fixed ([#9](#9)) ([a5107df](a5107df)), closes [#7](#7) [#8](#8)

ami-ci · 2022-12-22T18:15:30Z

🎉 This PR is included in version 1.0.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

## 1.0.0-alpha.1 (2022-12-22) ### Features * added SDK config ([#1](#1)) ([e8ed7dd](e8ed7dd)) * tracking and in-app added ([#2](#2)) ([c23f2d9](c23f2d9)) ### Bug Fixes * typo fixed ([#9](#9)) ([a5107df](a5107df)), closes [#7](#7) [#8](#8)

ami-ci · 2022-12-22T18:26:34Z

🎉 This PR is included in version 1.0.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

## 1.0.0-beta.1 (2023-03-07) ### Features * added missing methods ([#17](#17)) ([73f29e6](73f29e6)) * added SDK config ([#1](#1)) ([e8ed7dd](e8ed7dd)) * tracking and in-app added ([#2](#2)) ([c23f2d9](c23f2d9)) ### Bug Fixes * formatting issues ([d67fa7e](d67fa7e)) * in-app remove gist org id ([#19](#19)) ([ce4cc9e](ce4cc9e)) * missing methods and extra dependency ([2c5deca](2c5deca)) * obj-c bindings issue ([0dbe4ef](0dbe4ef)) * plugin version in user-agent ([a10e482](a10e482)) * typo fixed ([#9](#9)) ([a5107df](a5107df)), closes [#7](#7) [#8](#8) * updated android dependency to auto update ([#24](#24)) ([040cef2](040cef2)) * updated icon and typo ([57c6eef](57c6eef))

ami-ci · 2023-03-07T11:14:42Z

🎉 This PR is included in version 1.0.0-beta.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

## 1.0.0 (2023-04-03) ### Features * added missing methods ([#17](#17)) ([73f29e6](73f29e6)) * added SDK config ([#1](#1)) ([e8ed7dd](e8ed7dd)) * tracking and in-app added ([#2](#2)) ([c23f2d9](c23f2d9)) ### Bug Fixes * add test coverage and refactored scripts ([#34](#34)) ([f7f2387](f7f2387)) * formatting issues ([d67fa7e](d67fa7e)) * in-app remove gist org id ([#19](#19)) ([ce4cc9e](ce4cc9e)) * missing methods and extra dependency ([2c5deca](2c5deca)) * obj-c bindings issue ([0dbe4ef](0dbe4ef)) * plugin version in user-agent ([a10e482](a10e482)) * release script typo ([2a8b7ae](2a8b7ae)) * typo fixed ([#9](#9)) ([a5107df](a5107df)), closes [#7](#7) [#8](#8) * updated android dependency to auto update ([#24](#24)) ([040cef2](040cef2)) * updated icon and typo ([57c6eef](57c6eef))

Shahroz16 added 3 commits November 10, 2022 02:37

added support for config along with android native bindings

a53ff8b

added ios config bindings

c0b7e3f

removed logging config

15841c4

Shahroz16 self-assigned this Nov 15, 2022

Shahroz16 requested review from ami-aman and mrehan27 November 15, 2022 10:22

ami-aman reviewed Nov 15, 2022

View reviewed changes

levibostian reviewed Nov 15, 2022

View reviewed changes

PR suggestions

8ff88ca

Shahroz16 requested a review from ami-aman November 16, 2022 11:07

mrehan27 reviewed Nov 21, 2022

View reviewed changes

android/src/main/kotlin/io/customer/customer_io/CustomerIoPlugin.kt Show resolved Hide resolved

mrehan27 reviewed Nov 21, 2022

View reviewed changes

matt-frizzell self-requested a review November 23, 2022 12:09

matt-frizzell approved these changes Nov 23, 2022

View reviewed changes

removed unused config attributes

2589675

mrehan27 approved these changes Nov 24, 2022

View reviewed changes

Shahroz16 merged commit e8ed7dd into main Nov 24, 2022

ami-ci added the released on @alpha label Dec 22, 2022

ami-ci added the released on @beta label Mar 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: added SDK config #1

feat: added SDK config #1

Shahroz16 commented Nov 15, 2022

ami-aman Nov 15, 2022

Shahroz16 Nov 15, 2022

ami-aman Nov 15, 2022

Shahroz16 Nov 15, 2022

ami-aman Nov 15, 2022

ami-aman Nov 15, 2022

Shahroz16 Nov 15, 2022

ami-aman Nov 15, 2022

levibostian Nov 15, 2022

Shahroz16 Nov 15, 2022

levibostian Nov 15, 2022

levibostian Nov 15, 2022

levibostian Nov 15, 2022

levibostian Nov 15, 2022

levibostian Nov 15, 2022

Shahroz16 Nov 16, 2022

levibostian Nov 15, 2022

Shahroz16 Nov 15, 2022

levibostian Nov 15, 2022

Shahroz16 Nov 15, 2022

levibostian Nov 15, 2022

Shahroz16 Nov 15, 2022

levibostian Nov 15, 2022

Shahroz16 Nov 16, 2022

levibostian Nov 15, 2022

Shahroz16 Nov 15, 2022

levibostian Nov 15, 2022

Shahroz16 Nov 15, 2022

mrehan27 Nov 21, 2022

matt-frizzell left a comment

mrehan27 left a comment

ami-ci commented Dec 22, 2022

ami-ci commented Dec 22, 2022

ami-ci commented Mar 7, 2023

	class CustomerIoPlugin : FlutterPlugin, MethodCallHandler {
	class CustomerIOPlugin : FlutterPlugin, MethodCallHandler {

	private lateinit var channel: MethodChannel
	private lateinit var flutterCommunicationChannel: MethodChannel

feat: added SDK config #1

feat: added SDK config #1

Conversation

Shahroz16 commented Nov 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matt-frizzell left a comment

Choose a reason for hiding this comment

mrehan27 left a comment

Choose a reason for hiding this comment

ami-ci commented Dec 22, 2022

ami-ci commented Dec 22, 2022

ami-ci commented Mar 7, 2023