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 screen tracking parameters #167

Merged
merged 5 commits into from
Nov 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import io.flutter.embedding.engine.plugins.activity.ActivityPluginBinding
import io.flutter.plugin.common.MethodCall
import io.flutter.plugin.common.MethodChannel
import io.flutter.plugin.common.MethodChannel.MethodCallHandler
import io.flutter.plugin.common.MethodChannel.Result
import java.lang.ref.WeakReference

/**
Expand Down Expand Up @@ -171,7 +172,7 @@ class CustomerIoPlugin : FlutterPlugin, MethodCallHandler, ActivityAware {

private fun track(params: Map<String, Any>) {
val name = requireNotNull(params.getAsTypeOrNull<String>(Keys.Tracking.NAME)) {
"Event name is required to track event"
"Event name is missing in params: $params"
}
val properties = params.getAsTypeOrNull<Map<String, Any>>(Keys.Tracking.PROPERTIES)

Expand Down Expand Up @@ -225,18 +226,16 @@ class CustomerIoPlugin : FlutterPlugin, MethodCallHandler, ActivityAware {
}

private fun screen(params: Map<String, Any>) {
// TODO: Fix screen implementation
/*
val name = params.getString(Keys.Tracking.EVENT_NAME)
val attributes =
params.getProperty<Map<String, Any>>(Keys.Tracking.ATTRIBUTES) ?: emptyMap()
val title = requireNotNull(params.getAsTypeOrNull<String>(Keys.Tracking.TITLE)) {
"Screen title is missing in params: $params"
}
val properties = params.getAsTypeOrNull<Map<String, Any>>(Keys.Tracking.PROPERTIES)

if (attributes.isEmpty()) {
CustomerIO.instance().screen(name)
if (properties.isNullOrEmpty()) {
CustomerIO.instance().screen(title)
} else {
CustomerIO.instance().screen(name, attributes)
CustomerIO.instance().screen(title, properties)
}
*/
}

private fun initialize(args: Map<String, Any>): kotlin.Result<Unit> = runCatching {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,6 @@ internal object Keys {

const val NAME = "name"
const val PROPERTIES = "properties"
const val TITLE = "title"
}
}
2 changes: 1 addition & 1 deletion apps/amiapp_flutter/lib/src/app.dart
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ class _AmiAppState extends State<AmiApp> {
if (_customerIOSDK.sdkConfig?.screenTrackingEnabled == true) {
final Screen? screen = _router.currentLocation().toAppScreen();
if (screen != null) {
CustomerIO.instance.screen(name: screen.name);
CustomerIO.instance.screen(title: screen.name);
}
}
}
Expand Down
13 changes: 7 additions & 6 deletions apps/amiapp_flutter/lib/src/data/config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ class CustomerIOSDKConfig {
final String cdpApiKey;
final String? migrationSiteId;
final Region? region;
final bool? debugModeEnabled;
final bool? screenTrackingEnabled;
final bool debugModeEnabled;
final bool screenTrackingEnabled;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because these are only app settings, I updated them to non-nullable

final bool? autoTrackDeviceAttributes;
final String? apiHost;
final String? cdnHost;
Expand All @@ -22,8 +22,8 @@ class CustomerIOSDKConfig {
required this.cdpApiKey,
this.migrationSiteId,
this.region,
this.debugModeEnabled,
this.screenTrackingEnabled,
this.debugModeEnabled = true,
this.screenTrackingEnabled = true,
this.autoTrackDeviceAttributes,
this.apiHost,
this.cdnHost,
Expand Down Expand Up @@ -53,9 +53,10 @@ class CustomerIOSDKConfig {
cdpApiKey: cdpApiKey,
migrationSiteId: prefs.getString(_PreferencesKey.migrationSiteId),
region: region,
debugModeEnabled: prefs.getBool(_PreferencesKey.debugModeEnabled),
debugModeEnabled:
prefs.getBool(_PreferencesKey.debugModeEnabled) != false,
screenTrackingEnabled:
prefs.getBool(_PreferencesKey.screenTrackingEnabled),
prefs.getBool(_PreferencesKey.screenTrackingEnabled) != false,
autoTrackDeviceAttributes:
prefs.getBool(_PreferencesKey.autoTrackDeviceAttributes),
apiHost: prefs.getString(_PreferencesKey.apiHost),
Expand Down
10 changes: 5 additions & 5 deletions apps/amiapp_flutter/lib/src/screens/settings.dart
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ class _SettingsScreenState extends State<SettingsScreen> {
_cdnHostValueController = TextEditingController(text: cioConfig?.cdnHost);
_flushAtValueController =
TextEditingController(text: cioConfig?.flushAt?.toString());
_flushIntervalValueController = TextEditingController(
text: cioConfig?.flushInterval?.toTrimmedString());
_flushIntervalValueController =
TextEditingController(text: cioConfig?.flushInterval?.toString());
_featureTrackScreens = cioConfig?.screenTrackingEnabled ?? true;
_featureTrackDeviceAttributes =
cioConfig?.autoTrackDeviceAttributes ?? true;
Expand Down Expand Up @@ -115,11 +115,11 @@ class _SettingsScreenState extends State<SettingsScreen> {
_cdnHostValueController.text = defaultConfig.cdnHost ?? '';
_flushAtValueController.text = defaultConfig.flushAt?.toString() ?? '';
_flushIntervalValueController.text =
defaultConfig.flushInterval?.toTrimmedString() ?? '';
_featureTrackScreens = defaultConfig.screenTrackingEnabled ?? true;
defaultConfig.flushInterval?.toString() ?? '';
_featureTrackScreens = defaultConfig.screenTrackingEnabled;
_featureTrackDeviceAttributes =
defaultConfig.autoTrackDeviceAttributes ?? true;
_featureDebugMode = defaultConfig.debugModeEnabled ?? true;
_featureDebugMode = defaultConfig.debugModeEnabled;
_saveSettings(context);
});
}
Expand Down
9 changes: 0 additions & 9 deletions apps/amiapp_flutter/lib/src/utils/extensions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -108,15 +108,6 @@ extension AmiAppStringExtensions on String {
}
}

extension AmiAppIntExtensions on int {
String? toTrimmedString() {
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 we now use Int for flush properties, we no longer need this extension

if (this % 1.0 != 0.0) {
return toString();
}
return toStringAsFixed(0);
}
}

extension LocationExtensions on GoRouter {
// Get location of current route
// This is a workaround to get the current location as location property
Expand Down
1 change: 1 addition & 0 deletions ios/Classes/Keys.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ struct Keys {

static let name = "name"
static let properties = "properties"
static let title = "title"
}

struct Environment{
Expand Down
25 changes: 11 additions & 14 deletions ios/Classes/SwiftCustomerIoPlugin.swift
Original file line number Diff line number Diff line change
Expand Up @@ -106,20 +106,17 @@ public class SwiftCustomerIoPlugin: NSObject, FlutterPlugin {
}

func screen(params : Dictionary<String, AnyHashable>) {
// TODO: Fix screen implementation
/*
guard let name = params[Keys.Tracking.eventName] as? String
else {
return
}

guard let attributes = params[Keys.Tracking.attributes] as? Dictionary<String, AnyHashable> else{
CustomerIO.shared.screen(name: name)
return
}

CustomerIO.shared.screen(name: name, data: attributes)
*/
guard let title = params[Keys.Tracking.title] as? String else {
logger.error("Missing screen title in: \(params) for key: \(Keys.Tracking.title)")
return
}

guard let properties = params[Keys.Tracking.properties] as? Dictionary<String, AnyHashable> else {
CustomerIO.shared.screen(title: title)
return
}

CustomerIO.shared.screen(title: title, properties: properties)
}


Expand Down
18 changes: 11 additions & 7 deletions lib/customer_io.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import 'customer_io_config.dart';
import 'customer_io_enums.dart';
import 'customer_io_inapp.dart';
import 'customer_io_platform_interface.dart';
import 'extensions/map_extensions.dart';
import 'messaging_in_app/platform_interface.dart';
import 'messaging_push/platform_interface.dart';

Expand Down Expand Up @@ -88,9 +89,9 @@ class CustomerIO {
/// @param userId unique identifier for a profile
/// @param traits (Optional) params to set profile attributes
void identify(
{required String userId,
Map<String, dynamic> traits = const {}}) {
return _platform.identify(userId: userId, traits: traits);
{required String userId, Map<String, dynamic> traits = const {}}) {
return _platform.identify(
userId: userId, traits: traits.excludeNullValues());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Excluded null from all currently implemented functions, as we don't currently support null attributes in native SDKs. Open to any suggestions if there are other ideas on how to handle this!

}

/// Call this function to stop identifying a person.
Expand All @@ -108,7 +109,8 @@ class CustomerIO {
/// @param properties (Optional) params to be sent with event
void track(
{required String name, Map<String, dynamic> properties = const {}}) {
return _platform.track(name: name, properties: properties);
return _platform.track(
name: name, properties: properties.excludeNullValues());
}

/// Track a push metric
Expand All @@ -131,8 +133,9 @@ class CustomerIO {
/// @param name name of the screen user visited
/// @param attributes (Optional) params to be sent with event
void screen(
{required String name, Map<String, dynamic> attributes = const {}}) {
return _platform.screen(name: name, attributes: attributes);
{required String title, Map<String, dynamic> properties = const {}}) {
return _platform.screen(
title: title, properties: properties.excludeNullValues());
}

/// Use this function to send custom device attributes
Expand All @@ -148,7 +151,8 @@ class CustomerIO {
///
/// @param attributes additional attributes for a user profile
void setProfileAttributes({required Map<String, dynamic> attributes}) {
return _platform.setProfileAttributes(traits: attributes);
return _platform.setProfileAttributes(
traits: attributes.excludeNullValues());
}

/// Subscribes to an in-app event listener.
Expand Down
1 change: 1 addition & 0 deletions lib/customer_io_const.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,5 @@ class TrackingConsts {

static const String name = "name";
static const String properties = "properties";
static const String title = "title";
}
8 changes: 4 additions & 4 deletions lib/customer_io_method_channel.dart
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,12 @@ class CustomerIOMethodChannel extends CustomerIOPlatform {
/// Track screen events to record the screens a user visits
@override
void screen(
{required String name,
Map<String, dynamic> attributes = const {}}) async {
{required String title,
Map<String, dynamic> properties = const {}}) async {
try {
final payload = {
TrackingConsts.eventName: name,
TrackingConsts.attributes: attributes
TrackingConsts.title: title,
TrackingConsts.properties: properties
};
methodChannel.invokeMethod(MethodConsts.screen, payload);
} on PlatformException catch (exception) {
Expand Down
2 changes: 1 addition & 1 deletion lib/customer_io_platform_interface.dart
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ abstract class CustomerIOPlatform extends PlatformInterface {
}

void screen(
{required String name, Map<String, dynamic> attributes = const {}}) {
{required String title, Map<String, dynamic> properties = const {}}) {
throw UnimplementedError('screen() has not been implemented.');
}

Expand Down
7 changes: 7 additions & 0 deletions lib/extensions/map_extensions.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/// Extensions for [Map] class that provide additional functionality and convenience methods.
extension CustomerIOMapExtension on Map<String, dynamic> {
/// Returns a new map with entries that have non-null values, excluding null values.
Map<String, dynamic> excludeNullValues() {
return Map.fromEntries(entries.where((entry) => entry.value != null));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return Map.fromEntries(entries.where((entry) => entry.value != null));
return map.removeWhere((key, value) => value == null);

not this exact place but dart provides this utility method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It definitely sounds good, but I deliberately avoided modifying the same map because removing items directly could unintentionally alter customer provided map, which might cause issues in their app. So I created a new map instance to avoid any potential side effects.

}
}
6 changes: 3 additions & 3 deletions test/customer_io_method_channel_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,12 @@ void main() {

test('screen() should call platform method with correct arguments', () async {
final Map<String, dynamic> args = {
'eventName': 'screen_event',
'attributes': {'screenName': '你好'}
'title': 'screen_event',
'properties': {'screenName': '你好'}
};

final customerIO = CustomerIOMethodChannel();
customerIO.screen(name: args['eventName'], attributes: args['attributes']);
customerIO.screen(title: args['title'], properties: args['properties']);

expectMethodInvocationArguments('screen', args);
});
Expand Down
13 changes: 13 additions & 0 deletions test/customer_io_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,19 @@ void main() {
);
});

test('screen() correct arguments are passed', () {
const title = 'checkout';
final givenProperties = {'source': 'push'};
CustomerIO.instance.screen(title: title, properties: givenProperties);
expect(
verify(mockPlatform.screen(
title: captureAnyNamed("title"),
properties: captureAnyNamed("properties"),
)).captured,
[title, givenProperties],
);
});

test('trackMetric() calls platform', () {
const deliveryID = '123';
const deviceToken = 'abc';
Expand Down
8 changes: 4 additions & 4 deletions test/customer_io_test.mocks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -124,16 +124,16 @@ class MockTestCustomerIoPlatform extends _i1.Mock
);
@override
void screen({
required String? name,
Map<String, dynamic>? attributes = const {},
required String? title,
Map<String, dynamic>? properties = const {},
}) =>
super.noSuchMethod(
Invocation.method(
#screen,
[],
{
#name: name,
#attributes: attributes,
#title: title,
#properties: properties,
},
),
returnValueForMissingStub: null,
Expand Down
Loading