Fix empty string for `CLIENT_ID` from being sent when user decides to opt in (#144)
* unified_analytics and graphs: cleanup lints, bump pkg deps (#108)
* Fix to conditional logic for setting telemetry
* Test Client id not empty + different from original id
* Test using a new instance to check client id
* Reread the new client id after clearing it
diff --git a/pkgs/unified_analytics/lib/src/analytics.dart b/pkgs/unified_analytics/lib/src/analytics.dart
index 6d72864..a1b18c6 100644
--- a/pkgs/unified_analytics/lib/src/analytics.dart
+++ b/pkgs/unified_analytics/lib/src/analytics.dart
@@ -286,7 +286,7 @@
late final ConfigHandler _configHandler;
final GAClient _gaClient;
final SurveyHandler _surveyHandler;
- late final String _clientId;
+ late String _clientId;
late final File _clientIdFile;
late final UserProperty userProperty;
late final LogHandler _logHandler;
@@ -556,32 +556,47 @@
final collectionEvent =
Event.analyticsCollectionEnabled(status: reportingBool);
- // Construct the body of the request to signal
- // telemetry status toggling
- //
- // We use don't use the sendEvent method because it may
- // be blocked by the [telemetryEnabled] getter
- final body = generateRequestBody(
- clientId: _clientId,
- eventName: collectionEvent.eventName,
- eventData: collectionEvent.eventData,
- userProperty: userProperty,
- );
+ // The body of the request that will be sent to GA4
+ final Map<String, Object?> body;
- _logHandler.save(data: body);
-
- // Conditional logic for clearing contents of persisted
- // files (except for config file) on opt out
- if (!reportingBool) {
- _sessionHandler.sessionFile.writeAsStringSync('');
- _logHandler.logFile.writeAsStringSync('');
- _clientIdFile.writeAsStringSync('');
- } else {
+ if (reportingBool) {
// Recreate the session and client id file; no need to
// recreate the log file since it will only receives events
// to persist from events sent
Initializer.createClientIdFile(clientFile: _clientIdFile);
Initializer.createSessionFile(sessionFile: _sessionHandler.sessionFile);
+
+ // Reread the client ID string so an empty string is not being
+ // sent to GA4 since the persisted files are cleared when a user
+ // decides to opt out of telemetry collection
+ _clientId = _clientIdFile.readAsStringSync();
+
+ // We must construct the body at this point after we have read in the
+ // new client id string that was generated
+ body = generateRequestBody(
+ clientId: _clientId,
+ eventName: collectionEvent.eventName,
+ eventData: collectionEvent.eventData,
+ userProperty: userProperty,
+ );
+
+ _logHandler.save(data: body);
+ } else {
+ // Construct the body of the request to signal
+ // telemetry status toggling
+ body = generateRequestBody(
+ clientId: _clientId,
+ eventName: collectionEvent.eventName,
+ eventData: collectionEvent.eventData,
+ userProperty: userProperty,
+ );
+
+ // For opted out users, data in the persisted files is cleared
+ _sessionHandler.sessionFile.writeAsStringSync('');
+ _logHandler.logFile.writeAsStringSync('');
+ _clientIdFile.writeAsStringSync('');
+
+ _clientId = _clientIdFile.readAsStringSync();
}
// Pass to the google analytics client to send
diff --git a/pkgs/unified_analytics/test/unified_analytics_test.dart b/pkgs/unified_analytics/test/unified_analytics_test.dart
index 05973be..2eb3cda 100644
--- a/pkgs/unified_analytics/test/unified_analytics_test.dart
+++ b/pkgs/unified_analytics/test/unified_analytics_test.dart
@@ -205,6 +205,8 @@
});
test('Toggling telemetry boolean through Analytics class api', () async {
+ final originalClientId = clientIdFile.readAsStringSync();
+
expect(analytics.telemetryEnabled, true,
reason: 'Telemetry should be enabled by default '
'when initialized for the first time');
@@ -239,6 +241,50 @@
reason: 'Check on event name');
expect((lastLogItem['events'] as List).last['params']['status'], true,
reason: 'Status should be false');
+ expect((lastLogItem['client_id'] as String).isNotEmpty, true);
+ expect(originalClientId != lastLogItem['client_id'], true,
+ reason: 'When opting in again, the client id should be regenerated');
+ });
+
+ test('Confirm client id is not empty string after opting in', () async {
+ await analytics.setTelemetry(false);
+ expect(logFile.readAsLinesSync().length, 0,
+ reason: 'Log file should have been cleared after opting out');
+ expect(clientIdFile.readAsStringSync().length, 0,
+ reason: 'CLIENT ID file gets cleared on opt out');
+
+ // Start up a second instance to simulate starting another
+ // command being run
+ final secondAnalytics = Analytics.test(
+ tool: initialTool,
+ homeDirectory: home,
+ measurementId: measurementId,
+ apiSecret: apiSecret,
+ flutterChannel: flutterChannel,
+ toolsMessageVersion: toolsMessageVersion,
+ toolsMessage: toolsMessage,
+ flutterVersion: flutterVersion,
+ dartVersion: dartVersion,
+ fs: fs,
+ platform: platform,
+ );
+
+ // Setting telemetry back on will emit a new event
+ // where the client id string should not be empty
+ await secondAnalytics.setTelemetry(true);
+ expect(analytics.telemetryEnabled, true,
+ reason: 'Analytics telemetry should be enabled');
+ expect(logFile.readAsLinesSync().length, 1,
+ reason: 'There should only one event since it was cleared on opt out');
+ expect(clientIdFile.readAsStringSync().length, greaterThan(0),
+ reason: 'CLIENT ID file gets regenerated on opt in');
+
+ // Extract the last log item to check for the keys
+ final lastLogItem =
+ jsonDecode(logFile.readAsLinesSync().last) as Map<String, Object?>;
+ expect((lastLogItem['client_id'] as String).isNotEmpty, true,
+ reason: 'The client id should have been regenerated and '
+ 'emitted in the opt in event');
});
test(
