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

pkgs/unified_analytics/lib/src/analytics.dart

@@ -286,7 +286,7 @@ class AnalyticsImpl implements Analytics {
   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 @@ class AnalyticsImpl implements Analytics {
     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

pkgs/unified_analytics/test/unified_analytics_test.dart

@@ -205,6 +205,8 @@ void main() {
   });
 
   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 @@ void main() {
         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(