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

Conversation

mrehan27
Copy link
Contributor

@mrehan27 mrehan27 commented Nov 8, 2024

closes: MBL-634

Changes

  • Updated SDK screen method to use revised parameter names
  • Adjusted native SDK, tests, and sample app accordingly

Code Changes

Before

CustomerIO.screen(name: "Favourites");
CustomerIO.screen(name: "Settings", attributes: {
  "source": "Dashboard",
  "is_logged_in": true,
});

After

CustomerIO.instance.screen(title: "Favourites");
CustomerIO.instance.screen(title: "Settings", properties: {
  "source": "Dashboard",
  "is_logged_in": true,
});

@mrehan27 mrehan27 requested a review from Shahroz16 November 8, 2024 11:48
@mrehan27 mrehan27 self-assigned this Nov 8, 2024
Copy link

github-actions bot commented Nov 8, 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.


  • amiapp_flutter: 167.5.0 (28851304)

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

@@ -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

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!

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.

@mrehan27 mrehan27 merged commit 76ea710 into feature/data-pipelines-support Nov 11, 2024
6 checks passed
@mrehan27 mrehan27 deleted the rehan/mbl-634-track-screens branch November 11, 2024 09:20
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