Skip to content

Commit 6b12651

Browse files
andrewkolosandrewkolos
authored
Fix analytics enabled/disabled event not being sent when the user enables/disables analytics (#160060)
Fixes flutter/flutter#160058. In addition, only send an event when the enabled-state of analytics actually gets flipped. <details> <summary> Pre-launch checklist </summary> - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. </details> <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
1 parent 6966a2e commit 6b12651

2 files changed

Lines changed: 83 additions & 40 deletions

File tree

‎packages/flutter_tools/lib/runner.dart‎

Lines changed: 16 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -73,47 +73,28 @@ Future<int> run(
7373
exitCode: 1);
7474
}
7575

76-
// Disable analytics if user passes in the `--disable-analytics` option
77-
// "flutter --disable-analytics"
78-
//
79-
// Same functionality as "flutter config --no-analytics" for disabling
80-
// except with the `value` hard coded as false
8176
if (args.contains('--disable-analytics')) {
82-
// The tool sends the analytics event *before* toggling the flag
83-
// intentionally to be sure that opt-out events are sent correctly.
84-
AnalyticsConfigEvent(enabled: false).send();
85-
86-
// Normally, the tool waits for the analytics to all send before the
87-
// tool exits, but only when analytics are enabled. When reporting that
88-
// analytics have been disable, the wait must be done here instead.
89-
await globals.flutterUsage.ensureAnalyticsSent();
90-
91-
globals.flutterUsage.enabled = false;
92-
globals.printStatus('Analytics reporting disabled.');
93-
94-
// TODO(eliasyishak): Set the telemetry for the unified_analytics
95-
// package as well, the above will be removed once we have
96-
// fully transitioned to using the new package, https://github.com/flutter/flutter/issues/128251
77+
if (globals.analytics.telemetryEnabled) {
78+
globals.analytics.send(
79+
Event.analyticsCollectionEnabled(status: false),
80+
);
81+
// Before disablig analytics, we need to close the client to make
82+
// sure the above collection event is sent.
83+
await globals.analytics.close();
84+
}
9785
await globals.analytics.setTelemetry(false);
86+
globals.printStatus('Analytics reporting disabled.');
9887
}
9988

100-
// Enable analytics if user passes in the `--enable-analytics` option
101-
// `flutter --enable-analytics`
102-
//
103-
// Same functionality as `flutter config --analytics` for enabling
104-
// except with the `value` hard coded as true
10589
if (args.contains('--enable-analytics')) {
106-
// The tool sends the analytics event *before* toggling the flag
107-
// intentionally to be sure that opt-out events are sent correctly.
108-
AnalyticsConfigEvent(enabled: true).send();
109-
110-
globals.flutterUsage.enabled = true;
111-
globals.printStatus('Analytics reporting enabled.');
112-
113-
// TODO(eliasyishak): Set the telemetry for the unified_analytics
114-
// package as well, the above will be removed once we have
115-
// fully transitioned to using the new package, https://github.com/flutter/flutter/issues/128251
90+
final bool alreadyEnabled = globals.analytics.telemetryEnabled;
11691
await globals.analytics.setTelemetry(true);
92+
if (!alreadyEnabled) {
93+
globals.analytics.send(
94+
Event.analyticsCollectionEnabled(status: true),
95+
);
96+
}
97+
globals.printStatus('Analytics reporting enabled.');
11798
}
11899

119100
// Send an event to GA3 for any users that are opted into GA3

‎packages/flutter_tools/test/general.shard/runner/runner_test.dart‎

Lines changed: 67 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,7 @@ void main() {
541541
);
542542

543543
testUsingContext(
544-
'runner enabling analytics with flag',
544+
'--enable-analytics and --disable-analytics enables/disables telemetry',
545545
() async {
546546
io.setExitFunctionForTests((int exitCode) {});
547547

@@ -550,8 +550,6 @@ void main() {
550550
await runner.run(
551551
<String>['--disable-analytics'],
552552
() => <FlutterCommand>[],
553-
// This flutterVersion disables crash reporting.
554-
flutterVersion: '[user-branch]/',
555553
shutdownHooks: ShutdownHooks(),
556554
);
557555

@@ -560,8 +558,6 @@ void main() {
560558
await runner.run(
561559
<String>['--enable-analytics'],
562560
() => <FlutterCommand>[],
563-
// This flutterVersion disables crash reporting.
564-
flutterVersion: '[user-branch]/',
565561
shutdownHooks: ShutdownHooks(),
566562
);
567563

@@ -574,6 +570,72 @@ void main() {
574570
},
575571
);
576572

573+
testUsingContext(
574+
'--enable-analytics and --disable-analytics send an event when telemetry is enabled/disabled',
575+
() async {
576+
io.setExitFunctionForTests((int exitCode) {});
577+
await globals.analytics.setTelemetry(true);
578+
579+
await runner.run(
580+
<String>['--disable-analytics'],
581+
() => <FlutterCommand>[],
582+
shutdownHooks: ShutdownHooks(),
583+
);
584+
585+
expect(
586+
(globals.analytics as FakeAnalytics).sentEvents,
587+
<Event>[Event.analyticsCollectionEnabled(status: false)],
588+
);
589+
590+
(globals.analytics as FakeAnalytics).sentEvents.clear();
591+
expect(globals.analytics.telemetryEnabled, false);
592+
await runner.run(
593+
<String>['--enable-analytics'],
594+
() => <FlutterCommand>[],
595+
shutdownHooks: ShutdownHooks(),
596+
);
597+
598+
expect((globals.analytics as FakeAnalytics).sentEvents, <Event>[
599+
Event.analyticsCollectionEnabled(status: true),
600+
]);
601+
},
602+
overrides: <Type, Generator>{
603+
Analytics: () => fakeAnalytics,
604+
FileSystem: () => MemoryFileSystem.test(),
605+
ProcessManager: () => FakeProcessManager.any(),
606+
},
607+
);
608+
609+
testUsingContext(
610+
'--enable-analytics and --disable-analytics do not send an event when telemetry is already enabled/disabled',
611+
() async {
612+
io.setExitFunctionForTests((int exitCode) {});
613+
614+
await globals.analytics.setTelemetry(false);
615+
await runner.run(
616+
<String>['--disable-analytics'],
617+
() => <FlutterCommand>[],
618+
shutdownHooks: ShutdownHooks(),
619+
);
620+
621+
expect((globals.analytics as FakeAnalytics).sentEvents, isEmpty);
622+
623+
await globals.analytics.setTelemetry(true);
624+
await runner.run(
625+
<String>['--enable-analytics'],
626+
() => <FlutterCommand>[],
627+
shutdownHooks: ShutdownHooks(),
628+
);
629+
630+
expect((globals.analytics as FakeAnalytics).sentEvents, isEmpty);
631+
},
632+
overrides: <Type, Generator>{
633+
Analytics: () => fakeAnalytics,
634+
FileSystem: () => MemoryFileSystem.test(),
635+
ProcessManager: () => FakeProcessManager.any(),
636+
},
637+
);
638+
577639
testUsingContext(
578640
'throw error when both flags passed',
579641
() async {

0 commit comments

Comments
 (0)