Skip to content

Add DashEvent for toggling telemetry collection + send event whenever toggled #23

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

Merged
merged 16 commits into from
Mar 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions pkgs/unified_analytics/lib/src/analytics.dart
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,9 @@ abstract class Analytics {

/// Pass a boolean to either enable or disable telemetry and make
/// the necessary changes in the persisted configuration file
///
/// Setting the telemetry status will also send an event to GA
/// indicating the latest status of the telemetry from [reportingBool]
Future<void> setTelemetry(bool reportingBool);
}

Expand Down Expand Up @@ -260,8 +263,26 @@ class AnalyticsImpl implements Analytics {
}

@override
Future<void> setTelemetry(bool reportingBool) =>
_configHandler.setTelemetry(reportingBool);
Future<void> setTelemetry(bool reportingBool) {
_configHandler.setTelemetry(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 Map<String, Object?> body = generateRequestBody(
clientId: _clientId,
eventName: DashEvent.analyticsCollectionEnabled,
eventData: {'status': reportingBool},
userProperty: userProperty,
);

_logHandler.save(data: body);

// Pass to the google analytics client to send
return _gaClient.sendData(body);
}
}

/// This class extends [AnalyticsImpl] and subs out any methods that
Expand Down
12 changes: 10 additions & 2 deletions pkgs/unified_analytics/lib/src/enums.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@
/// The [label] for each enum value is what will be logged, the [description]
/// is here for documentation purposes
enum DashEvent {
// Events that can be sent by all tools; these
// events should not be tool specific; toolOwner
// not necessary for these events
analyticsCollectionEnabled(
label: 'analytics_collection_enabled',
description: 'The opt-in status for analytics collection',
),

// Events for flutter_tools
hotReloadTime(
label: 'hot_reload_time',
Expand Down Expand Up @@ -49,11 +57,11 @@ enum DashEvent {

final String label;
final String description;
final DashTool toolOwner;
final DashTool? toolOwner;
const DashEvent({
required this.label,
required this.description,
required this.toolOwner,
this.toolOwner,
});
}

Expand Down
24 changes: 23 additions & 1 deletion pkgs/unified_analytics/test/unified_analytics_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -142,16 +142,38 @@ void main() {
expect(analytics.telemetryEnabled, true,
reason: 'Telemetry should be enabled by default '
'when initialized for the first time');

// Use the API to disable analytics
expect(logFile.readAsLinesSync().length, 0);
await analytics.setTelemetry(false);
expect(analytics.telemetryEnabled, false,
reason: 'Analytics telemetry should be disabled');
expect(logFile.readAsLinesSync().length, 1,
reason: 'One event should have been logged for disabling analytics');

// Extract the last log item to check for the keys
Map<String, Object?> lastLogItem =
jsonDecode(logFile.readAsLinesSync().last);
expect((lastLogItem['events'] as List).last['name'],
'analytics_collection_enabled',
reason: 'Check on event name');
expect((lastLogItem['events'] as List).last['params']['status'], false,
reason: 'Status should be false');

// Toggle it back to being enabled
await analytics.setTelemetry(true);
expect(analytics.telemetryEnabled, true,
reason: 'Analytics telemetry should be enabled');
expect(logFile.readAsLinesSync().length, 2,
reason: 'Second event should have been logged toggling '
'analytics back on');

// Extract the last log item to check for the keys
lastLogItem = jsonDecode(logFile.readAsLinesSync().last);
expect((lastLogItem['events'] as List).last['name'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider extracting (lastLogItem['events'] as List).last as a helper or extension method, given that it occurs 4 times in this method alone.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll expand this out if an additional test needs this functionality

'analytics_collection_enabled',
reason: 'Check on event name');
expect((lastLogItem['events'] as List).last['params']['status'], true,
reason: 'Status should be false');
});

test(
Expand Down