From 3934675f1595ed2fee2297d7d1cc96beaaa50c9e Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Tue, 28 Jan 2025 08:49:54 -0800 Subject: [PATCH] Send `enabled_features` as an event parameter rather than a user property --- pkgs/unified_analytics/lib/src/analytics.dart | 7 +++- pkgs/unified_analytics/lib/src/event.dart | 1 - .../lib/src/user_property.dart | 3 -- pkgs/unified_analytics/lib/src/utils.dart | 6 ++- .../test/unified_analytics_test.dart | 40 ++++++++++++++----- 5 files changed, 41 insertions(+), 16 deletions(-) diff --git a/pkgs/unified_analytics/lib/src/analytics.dart b/pkgs/unified_analytics/lib/src/analytics.dart index cc8b4bc11..8b9fc11db 100644 --- a/pkgs/unified_analytics/lib/src/analytics.dart +++ b/pkgs/unified_analytics/lib/src/analytics.dart @@ -339,6 +339,7 @@ class AnalyticsImpl implements Analytics { final File _clientIdFile; final UserProperty _userProperty; final LogHandler _logHandler; + final String? _enabledFeatures; /// Tells the client if they need to show a message to the /// user; this will return true if it is the first time the @@ -406,8 +407,8 @@ class AnalyticsImpl implements Analytics { truncateStringToLength(io.Platform.operatingSystemVersion, 36), locale: io.Platform.localeName, clientIde: clientIde, - enabledFeatures: enabledFeatures, ), + _enabledFeatures = enabledFeatures, _configHandler = ConfigHandler( homeDirectory: homeDirectory, configFile: homeDirectory @@ -613,6 +614,7 @@ class AnalyticsImpl implements Analytics { eventName: event.eventName, eventData: event.eventData, userProperty: _userProperty, + enabledFeatures: _enabledFeatures, ); if (_enableAsserts) checkBody(body); @@ -654,6 +656,7 @@ class AnalyticsImpl implements Analytics { eventName: collectionEvent.eventName, eventData: collectionEvent.eventData, userProperty: _userProperty, + enabledFeatures: _enabledFeatures, ); _logHandler.save(data: body); @@ -664,6 +667,7 @@ class AnalyticsImpl implements Analytics { clientId: clientId, eventName: collectionEvent.eventName, eventData: collectionEvent.eventData, + enabledFeatures: _enabledFeatures, userProperty: _userProperty, ); @@ -774,6 +778,7 @@ class FakeAnalytics extends AnalyticsImpl { eventName: event.eventName, eventData: event.eventData, userProperty: _userProperty, + enabledFeatures: _enabledFeatures, ); if (_enableAsserts) checkBody(body); diff --git a/pkgs/unified_analytics/lib/src/event.dart b/pkgs/unified_analytics/lib/src/event.dart index ed3066596..b0d0a9f8d 100644 --- a/pkgs/unified_analytics/lib/src/event.dart +++ b/pkgs/unified_analytics/lib/src/event.dart @@ -569,7 +569,6 @@ final class Event { if (reloadVMTimeInMs != null) 'reloadVMTimeInMs': reloadVMTimeInMs, }; - // TODO: eliasyishak, add better dartdocs to explain each param /// Event that is emitted periodically to report the number of times each lint /// has been enabled. /// diff --git a/pkgs/unified_analytics/lib/src/user_property.dart b/pkgs/unified_analytics/lib/src/user_property.dart index f0e177d09..a607b07e9 100644 --- a/pkgs/unified_analytics/lib/src/user_property.dart +++ b/pkgs/unified_analytics/lib/src/user_property.dart @@ -21,7 +21,6 @@ class UserProperty { final String hostOsVersion; final String locale; final String? clientIde; - final String? enabledFeatures; final File sessionFile; @@ -43,7 +42,6 @@ class UserProperty { required this.hostOsVersion, required this.locale, required this.clientIde, - required this.enabledFeatures, required this.sessionFile, }); @@ -157,6 +155,5 @@ class UserProperty { 'host_os_version': hostOsVersion, 'locale': locale, 'client_ide': clientIde, - 'enabled_features': enabledFeatures, }; } diff --git a/pkgs/unified_analytics/lib/src/utils.dart b/pkgs/unified_analytics/lib/src/utils.dart index 41848a204..182facb6d 100644 --- a/pkgs/unified_analytics/lib/src/utils.dart +++ b/pkgs/unified_analytics/lib/src/utils.dart @@ -76,13 +76,17 @@ Map generateRequestBody({ required DashEvent eventName, required Map eventData, required UserProperty userProperty, + required String? enabledFeatures, }) => { 'client_id': clientId, 'events': >[ { 'name': eventName.label, - 'params': eventData, + 'params': { + ...eventData, + if (enabledFeatures != null) 'enabled_features': enabledFeatures, + }, } ], 'user_properties': userProperty.preparePayload() diff --git a/pkgs/unified_analytics/test/unified_analytics_test.dart b/pkgs/unified_analytics/test/unified_analytics_test.dart index 2e78df426..cd5b94210 100644 --- a/pkgs/unified_analytics/test/unified_analytics_test.dart +++ b/pkgs/unified_analytics/test/unified_analytics_test.dart @@ -498,7 +498,7 @@ void main() { # All other lines are configuration lines. They have # the form "name=value". If multiple lines contain # the same configuration name with different values, -# the parser will default to a conservative value. +# the parser will default to a conservative value. # DISABLING TELEMETRY REPORTING # @@ -548,7 +548,7 @@ reporting=1 # All other lines are configuration lines. They have # the form "name=value". If multiple lines contain # the same configuration name with different values, -# the parser will default to a conservative value. +# the parser will default to a conservative value. # DISABLING TELEMETRY REPORTING # @@ -632,7 +632,7 @@ ${initialTool.label}=$dateStamp,$toolsMessageVersion ); }); - test('Check that UserProperty class has all the necessary keys', () { + test('The UserProperty class has all the necessary keys', () { const userPropertyKeys = [ 'session_id', 'flutter_channel', @@ -645,7 +645,6 @@ ${initialTool.label}=$dateStamp,$toolsMessageVersion 'host_os_version', 'locale', 'client_ide', - 'enabled_features', ]; expect(analytics.userPropertyMap.keys.length, userPropertyKeys.length, reason: 'There should only be ${userPropertyKeys.length} keys'); @@ -826,6 +825,7 @@ ${initialTool.label}=$dateStamp,$toolsMessageVersion eventName: DashEvent.hotReloadTime, eventData: eventData, userProperty: analytics.userProperty, + enabledFeatures: 'enable-native-assets', ); // Checks for the top level keys @@ -833,8 +833,6 @@ ${initialTool.label}=$dateStamp,$toolsMessageVersion reason: '"client_id" is required at the top level'); expect(body.containsKey('events'), true, reason: '"events" is required at the top level'); - expect(body.containsKey('user_properties'), true, - reason: '"user_properties" is required at the top level'); // Regex for the client id final clientIdPattern = RegExp( @@ -846,14 +844,36 @@ ${initialTool.label}=$dateStamp,$toolsMessageVersion expect(clientIdPattern.hasMatch(body['client_id'] as String), true, reason: 'The client id is not properly formatted, ie ' '46cc0ba6-f604-4fd9-aa2f-8a20beb24cd4'); - expect( - (body['events'][0] as Map).containsKey('name'), true, + expect(body['events'][0] as Map, contains('name'), reason: 'Each event in the events array needs a name'); - expect( - (body['events'][0] as Map).containsKey('params'), true, + expect(body['events'][0] as Map, contains('params'), reason: 'Each event in the events array needs a params key'); }); + test( + 'The list of enabled features is included as an event parameter in every sent event', + () { + final eventData = { + 'time': 5, + 'command': 'run', + }; + + final Map body = generateRequestBody( + clientId: Uuid().generateV4(), + eventName: DashEvent.hotReloadTime, + eventData: eventData, + userProperty: analytics.userProperty, + enabledFeatures: 'enable-native-assets', + ); + + expect((body['events'][0] as Map)['params'], + contains('enabled_features')); + expect( + (body['events'][0] as Map)['params'] + ['enabled_features'], + 'enable-native-assets'); + }); + test('Check that log file is correctly persisting events sent', () { final int numberOfEvents = max((kLogFileLength * 0.1).floor(), 5);