diff --git a/pkgs/unified_analytics/CHANGELOG.md b/pkgs/unified_analytics/CHANGELOG.md index 56389af1b..25ed04f8c 100644 --- a/pkgs/unified_analytics/CHANGELOG.md +++ b/pkgs/unified_analytics/CHANGELOG.md @@ -1,5 +1,6 @@ ## 5.8.1 +- Refactor logic for `okToSend` and `shouldShowMessage` - Check devtools config file for legacy opt out status ## 5.8.0 diff --git a/pkgs/unified_analytics/lib/src/analytics.dart b/pkgs/unified_analytics/lib/src/analytics.dart index 5d6df92bf..90fefb5ef 100644 --- a/pkgs/unified_analytics/lib/src/analytics.dart +++ b/pkgs/unified_analytics/lib/src/analytics.dart @@ -333,19 +333,6 @@ class AnalyticsImpl implements Analytics { /// message has been updated by the package. late bool _showMessage; - /// This will be switch to true once it has been confirmed by the - /// client using this package that they have shown the - /// consent message to the developer. - /// - /// If the tool using this package as already shown the consent message - /// and it has been added to the config file, it will be set as true. - /// - /// It will also be set to true once the tool using this package has - /// invoked [clientShowedMessage]. - /// - /// If this is false, all events will be blocked from being sent. - bool _clientShowedMessage = false; - /// When set to `true`, various assert statements will be enabled /// to ensure usage of this class is within GA4 limitations. final bool _enableAsserts; @@ -353,6 +340,9 @@ class AnalyticsImpl implements Analytics { /// Telemetry suppression flag that is set via [Analytics.suppressTelemetry]. bool _telemetrySuppressed = false; + /// Indicates if this is the first run for a given tool. + bool _firstRun = false; + /// The list of futures that will contain all of the send events /// from the [GAClient]. final _futures = >[]; @@ -388,7 +378,13 @@ class AnalyticsImpl implements Analytics { toolsMessageVersion: toolsMessageVersion, ); initializer.run(); - _showMessage = initializer.firstRun; + if (initializer.firstRun) { + _showMessage = true; + _firstRun = true; + } else { + _showMessage = false; + _firstRun = false; + } // Create the config handler that will parse the config file _configHandler = ConfigHandler( @@ -397,13 +393,6 @@ class AnalyticsImpl implements Analytics { initializer: initializer, ); - // If the tool has already been added to the config file - // we can assume that the client has successfully shown - // the consent message - if (_configHandler.parsedTools.containsKey(tool.label)) { - _clientShowedMessage = true; - } - // Check if the tool has already been onboarded, and if it // has, check if the latest message version is greater to // prompt the client to show a message @@ -414,6 +403,11 @@ class AnalyticsImpl implements Analytics { _configHandler.parsedTools[tool.label]?.versionNumber ?? -1; if (currentVersion < toolsMessageVersion) { _showMessage = true; + + // If the message version has been updated, it will be considered + // as if it was a first run and any events attempting to get sent + // will be blocked + _firstRun = true; } _clientIdFile = fs.file( @@ -464,22 +458,19 @@ class AnalyticsImpl implements Analytics { /// Checking the [telemetryEnabled] boolean reflects what the /// config file reflects. /// - /// Checking the [_showMessage] boolean indicates if this the first - /// time the tool is using analytics or if there has been an update - /// the messaging found in constants.dart - in both cases, analytics - /// will not be sent until the second time the tool is used. - /// - /// Additionally, if the client has not invoked - /// [Analytics.clientShowedMessage], then no events shall be sent. + /// Checking the [_showMessage] boolean indicates if the consent + /// message has been shown for the user, this boolean is set to `true` + /// when the tool using this package invokes the [clientShowedMessage] + /// method. /// /// If the user has suppressed telemetry [_telemetrySuppressed] will /// return `true` to prevent events from being sent for current invocation. + /// + /// Checking if it is the first time a tool is running with this package + /// as indicated by [_firstRun]. @override bool get okToSend => - telemetryEnabled && - !_showMessage && - _clientShowedMessage && - !_telemetrySuppressed; + telemetryEnabled && !_showMessage && !_telemetrySuppressed && !_firstRun; @override Map get parsedTools => _configHandler.parsedTools; @@ -496,22 +487,24 @@ class AnalyticsImpl implements Analytics { @override void clientShowedMessage() { + // Check the tool needs to be added to the config file if (!_configHandler.parsedTools.containsKey(tool.label)) { _configHandler.addTool( tool: tool.label, versionNumber: toolsMessageVersion, ); - _showMessage = true; } + + // When the tool already exists but the consent message version + // has been updated if (_configHandler.parsedTools[tool.label]!.versionNumber < toolsMessageVersion) { _configHandler.incrementToolVersion( tool: tool.label, newVersionNumber: toolsMessageVersion, ); - _showMessage = true; } - _clientShowedMessage = true; + _showMessage = false; } @override diff --git a/pkgs/unified_analytics/test/unified_analytics_test.dart b/pkgs/unified_analytics/test/unified_analytics_test.dart index 659af5139..22e088596 100644 --- a/pkgs/unified_analytics/test/unified_analytics_test.dart +++ b/pkgs/unified_analytics/test/unified_analytics_test.dart @@ -74,7 +74,9 @@ void main() { fs: fs, platform: platform, ); + expect(initializationAnalytics.shouldShowMessage, true); initializationAnalytics.clientShowedMessage(); + expect(initializationAnalytics.shouldShowMessage, false); // The main analytics instance, other instances can be spawned within tests // to test how to instances running together work @@ -143,8 +145,6 @@ void main() { expect(dartToolDirectory.listSync().length, equals(5), reason: 'There should only be 5 files in the $kDartToolDirectoryName directory'); - expect(initializationAnalytics.shouldShowMessage, true, - reason: 'For the first run, the message should be shown'); expect(configFile.readAsLinesSync().length, kConfigString.split('\n').length + 1, reason: 'The number of lines should equal lines in constant value + 1 ' @@ -430,7 +430,12 @@ void main() { fs: fs, platform: platform, ); + expect(secondAnalytics.shouldShowMessage, true); + expect(secondAnalytics.okToSend, false); secondAnalytics.clientShowedMessage(); + expect(secondAnalytics.shouldShowMessage, false); + expect(secondAnalytics.okToSend, false, + reason: 'New version for the message will be treated as a first run'); expect(secondAnalytics.parsedTools[initialTool.label]?.versionNumber, toolsMessageVersion + 1, diff --git a/pkgs/unified_analytics/test/workflow_test.dart b/pkgs/unified_analytics/test/workflow_test.dart index 844e3a21c..45ca62c33 100644 --- a/pkgs/unified_analytics/test/workflow_test.dart +++ b/pkgs/unified_analytics/test/workflow_test.dart @@ -59,6 +59,99 @@ void main() { .childFile(kDismissedSurveyFileName); }); + test('Confirm workflow for first run', () { + final firstAnalytics = Analytics.test( + tool: initialTool, + homeDirectory: home, + measurementId: measurementId, + apiSecret: apiSecret, + flutterChannel: flutterChannel, + toolsMessageVersion: toolsMessageVersion, + toolsMessage: toolsMessage, + flutterVersion: flutterVersion, + dartVersion: dartVersion, + fs: fs, + platform: platform, + ); + + expect(firstAnalytics.shouldShowMessage, true); + expect(firstAnalytics.okToSend, false); + + firstAnalytics.clientShowedMessage(); + expect(firstAnalytics.shouldShowMessage, false); + expect(firstAnalytics.okToSend, false, + reason: 'On the first run, we should not be ok ' + 'to send any events, even if the user accepts'); + }); + + test('Confirm workflow for updated tools message version + new tool', () { + // Helper function to check the state of the instance + void checkAnalyticsInstance(Analytics instance) { + expect(instance.shouldShowMessage, true); + expect(instance.okToSend, false); + + instance.clientShowedMessage(); + expect(instance.shouldShowMessage, false); + expect(instance.okToSend, false, + reason: 'On the first run, we should not be ok ' + 'to send any events, even if the user accepts'); + } + + final firstAnalytics = Analytics.test( + tool: initialTool, + homeDirectory: home, + measurementId: measurementId, + apiSecret: apiSecret, + flutterChannel: flutterChannel, + toolsMessageVersion: toolsMessageVersion, + toolsMessage: toolsMessage, + flutterVersion: flutterVersion, + dartVersion: dartVersion, + fs: fs, + platform: platform, + ); + + checkAnalyticsInstance(firstAnalytics); + + // Instance where we increment the version of the message + final secondAnalytics = Analytics.test( + tool: initialTool, + homeDirectory: home, + measurementId: measurementId, + apiSecret: apiSecret, + flutterChannel: flutterChannel, + toolsMessageVersion: toolsMessageVersion + 1, // Incrementing version + toolsMessage: toolsMessage, + flutterVersion: flutterVersion, + dartVersion: dartVersion, + fs: fs, + platform: platform, + ); + + // Running the same checks for the second instance, it should + // behave the same as if it was a first run + checkAnalyticsInstance(secondAnalytics); + + // Instance for a different tool with the incremented version + final thirdAnalytics = Analytics.test( + tool: secondTool, // Different tool + homeDirectory: home, + measurementId: measurementId, + apiSecret: apiSecret, + flutterChannel: flutterChannel, + toolsMessageVersion: toolsMessageVersion + 1, // Incrementing version + toolsMessage: toolsMessage, + flutterVersion: flutterVersion, + dartVersion: dartVersion, + fs: fs, + platform: platform, + ); + + // The instance with a new tool getting onboarded should be + // treated the same as the 2 previous instances + checkAnalyticsInstance(thirdAnalytics); + }); + test('Confirm workflow for checking tools into the config file', () { final firstAnalytics = Analytics.test( tool: initialTool,