Skip to content
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

Change default analytics to opt out #2483

Merged
merged 8 commits into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 0 additions & 1 deletion docs/documentation/tips-and-tricks.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ an example:
$ cat .patrol.env
[email protected]
PASSWORD=ny4ncat
DISABLE_ANALYTICS=true
```

### Granting sensitive permission through the Settings app
Expand Down
1 change: 0 additions & 1 deletion docs/tips-and-tricks.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ an example:
$ cat .patrol.env
[email protected]
PASSWORD=ny4ncat
DISABLE_ANALYTICS=true
```

### Granting sensitive permission through the Settings app
Expand Down
5 changes: 5 additions & 0 deletions packages/patrol_cli/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## Unreleased

- Add `PATROL_ANALYTICS_ENABLED` environment variable to disable analytics.
- Enable analytics by default.
pdenert marked this conversation as resolved.
Show resolved Hide resolved

## 3.4.1

- Add android product flavor to dart-define. (#2425)
Expand Down
5 changes: 5 additions & 0 deletions packages/patrol_cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ directories to PATH:
- on Unix-like systems, add `$HOME/.pub-cache/bin`
- on Windows, add `%USERPROFILE%\AppData\Local\Pub\Cache\bin`

### Disabling analytics

To disable analytics, set the `PATROL_ANALYTICS_ENABLED` environment variable to
`false`.

### Shell completion

Patrol CLI supports shell completion for bash, zsh and fish, thanks to the
Expand Down
16 changes: 13 additions & 3 deletions packages/patrol_cli/lib/src/analytics/analytics.dart
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,13 @@ class Analytics {
required Platform platform,
http.Client? httpClient,
required bool isCI,
required bool? envAnalyticsEnabled,
}) : _fs = fs,
_platform = platform,
_httpClient = httpClient ?? http.Client(),
_postUrl = _getAnalyticsUrl(measurementId, apiSecret),
_isCI = isCI;
_isCI = isCI,
_envAnalyticsEnabled = envAnalyticsEnabled;

final FileSystem _fs;
final Platform _platform;
Expand All @@ -53,6 +55,7 @@ class Analytics {
final String _postUrl;

final bool _isCI;
final bool? _envAnalyticsEnabled;

/// Sends an event to Google Analytics that command [name] run.
///
Expand All @@ -67,8 +70,15 @@ class Analytics {
return false;
}

final enabled = _config?.enabled ?? false;
if (!enabled) {
/// If the environment variable `PATROL_ANALYTICS_ENABLED` is set,
/// use it to determine if the command should be sent.
/// If not set, use the value from the config file.
final enabled = _config?.enabled ?? true;
if (_envAnalyticsEnabled != null) {
if (!_envAnalyticsEnabled) {
return false;
}
} else if (!enabled) {
return false;
}
pdenert marked this conversation as resolved.
Show resolved Hide resolved

Expand Down
2 changes: 1 addition & 1 deletion packages/patrol_cli/lib/src/runner/patrol_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ abstract class PatrolCommand extends Command<int> {
}

FlutterCommand get flutterCommand {
final arg = globalResults!['flutter-command'] as String?;
final arg = globalResults?['flutter-command'] as String?;

var cmd = arg;
if (cmd == null || cmd.isEmpty) {
Expand Down
45 changes: 27 additions & 18 deletions packages/patrol_cli/lib/src/runner/patrol_command_runner.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import 'dart:io' show ProcessSignal, stdin;
import 'dart:io' as p show Platform;

import 'package:adb/adb.dart';
import 'package:args/args.dart';
Expand Down Expand Up @@ -41,6 +42,8 @@ Future<int> patrolCommandRunner(List<String> args) async {
const platform = LocalPlatform();
final processManager = LoggingLocalProcessManager(logger: logger);
final isCI = ci.isCI;
final analyticsEnv = p.Platform.environment['PATROL_ANALYTICS_ENABLED'];
final analyticsEnabled = bool.tryParse(analyticsEnv ?? '');

final runner = PatrolCommandRunner(
pubUpdater: pubUpdater,
Expand All @@ -53,6 +56,7 @@ Future<int> patrolCommandRunner(List<String> args) async {
fs: fs,
platform: platform,
isCI: isCI,
envAnalyticsEnabled: analyticsEnabled,
),
processManager: processManager,
isCI: isCI,
Expand All @@ -79,6 +83,19 @@ Future<int> patrolCommandRunner(List<String> args) async {

const _gaTrackingId = 'G-W8XN8GS5BC';
const _gaApiSecret = 'CUIwI1nCQWGJQAK8E0AIfg';
const _helloPatrol = '''
+---------------------------------------------------+
| Patrol - Ready for action! |
+---------------------------------------------------+
| We would like to collect anonymous usage data |
| to improve Patrol CLI. No sensitive or private |
| information will ever leave your machine. |
| |
| By default, analytics is enabled. If you want to |
| disable it, please set the environment variable: |
| `PATROL_ANALYTICS_ENABLED=false` |
+---------------------------------------------------+
''';

class PatrolCommandRunner extends CompletionCommandRunner<int> {
PatrolCommandRunner({
Expand Down Expand Up @@ -380,24 +397,16 @@ Ask questions, get support at https://github.com/leancodepl/patrol/discussions''
}

void _handleAnalytics() {
_logger.info(
'''
\n
+---------------------------------------------------+
| Patrol - Ready for action! |
+---------------------------------------------------+
| We would like to collect anonymous usage data |
| to improve Patrol CLI. No sensitive or private |
| information will ever leave your machine. |
+---------------------------------------------------+
\n''',
);
final analyticsEnabled = _logger.confirm(
'Enable analytics?',
defaultValue: true,
);
_analytics.enabled = analyticsEnabled;
if (analyticsEnabled) {
_logger.info(_helloPatrol);

/// If the environment variable `PATROL_ANALYTICS_ENABLED` is set,
/// use it to determine if the command should be sent.
/// If not, analytics will be enabled by default.
final patrolAnalyticsEnabled =
p.Platform.environment['PATROL_ANALYTICS_ENABLED'];
pdenert marked this conversation as resolved.
Show resolved Hide resolved
_analytics.enabled =
bool.tryParse(patrolAnalyticsEnabled ?? 'true') ?? true;
if (_analytics.enabled) {
_logger.info('Analytics enabled. Thank you!');
} else {
_logger.info('Analytics disabled.');
Expand Down
111 changes: 110 additions & 1 deletion packages/patrol_cli/test/analytics/analytics_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import '../src/fakes.dart';
import '../src/mocks.dart';

void main() {
group('Analytics', () {
group('Analytics with no env variable', () {
late Analytics analytics;
late FileSystem fs;

Expand All @@ -36,6 +36,7 @@ void main() {
platform: fakePlatform('/Users/john'),
httpClient: httpClient,
isCI: false,
envAnalyticsEnabled: null,
);
});

Expand Down Expand Up @@ -63,6 +64,114 @@ void main() {
expect(sent, false);
});
});

group('Analytics with env variable enabled', () {
late Analytics analytics;
late FileSystem fs;

setUp(() {
setUpFakes();

fs = MemoryFileSystem.test();

final httpClient = MockHttpClient();
when(
() => httpClient.post(
any(),
body: any(named: 'body'),
headers: any(named: 'headers'),
),
).thenAnswer((_) async => http.Response('', 200));

analytics = Analytics(
measurementId: 'measurementId',
apiSecret: 'apiSecret',
fs: fs,
platform: fakePlatform('/Users/john'),
httpClient: httpClient,
isCI: false,
envAnalyticsEnabled: true,
);
});

test('sends data when enabled', () async {
// given
_createFakeFileSystem(fs, analyticsEnabled: true);

// when
final sent =
await analytics.sendCommand(FlutterVersion.test(), 'test command');

// then
expect(sent, true);
});

test('does not send data when disabled', () async {
// given
_createFakeFileSystem(fs, analyticsEnabled: false);

// when
final sent =
await analytics.sendCommand(FlutterVersion.test(), 'test command');

// then
expect(sent, true);
});
});

group('Analytics with env variable disabled', () {
late Analytics analytics;
late FileSystem fs;

setUp(() {
setUpFakes();

fs = MemoryFileSystem.test();

final httpClient = MockHttpClient();
when(
() => httpClient.post(
any(),
body: any(named: 'body'),
headers: any(named: 'headers'),
),
).thenAnswer((_) async => http.Response('', 200));

analytics = Analytics(
measurementId: 'measurementId',
apiSecret: 'apiSecret',
fs: fs,
platform: fakePlatform('/Users/john'),
httpClient: httpClient,
isCI: false,
envAnalyticsEnabled: false,
);
});

test('sends data when enabled', () async {
// given
_createFakeFileSystem(fs, analyticsEnabled: true);

// when
final sent =
await analytics.sendCommand(FlutterVersion.test(), 'test command');

// then
expect(sent, false);
});

test('does not send data when disabled', () async {
// given
_createFakeFileSystem(fs, analyticsEnabled: false);

// when
final sent =
await analytics.sendCommand(FlutterVersion.test(), 'test command');

// then
expect(sent, false);
});
});
}

void _createFakeFileSystem(FileSystem fs, {required bool analyticsEnabled}) {
Expand Down
Loading