-
Notifications
You must be signed in to change notification settings - Fork 62
Honor legacy opt out status #80
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
eliasyishak
merged 13 commits into
dart-lang:main
from
eliasyishak:74-honor-legacy-opt-out-status
Apr 17, 2023
Merged
Changes from 5 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
b9eb98a
Function added to check dart and flutter legacy analytics files
eliasyishak 80152c8
Sort members in utils.dart
eliasyishak fe81441
Logic updated to change config string if legacy optout
eliasyishak 22d1753
Update tests for both dart and flutter legacy analytics
eliasyishak 7a5c7d5
Doc clean up
eliasyishak 68fa790
On error, assume user has opted out of legacy analytics
eliasyishak 1caefe7
Merge pull request #1 from eliasyishak/main
eliasyishak b3b5a86
Update CHANGELOG.md
eliasyishak 4f3a3cb
Alter consent message based on tool using package
eliasyishak 16870f6
Clean up try blocks
eliasyishak 2c618f8
Update try blocks for add'l exception + ignore lint for http client
eliasyishak 18eeb66
Change home directory location for windows
eliasyishak 3dbc207
Update documentation on http client
eliasyishak File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,178 @@ | ||
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file | ||
// for details. All rights reserved. Use of this source code is governed by a | ||
// BSD-style license that can be found in the LICENSE file. | ||
|
||
import 'dart:io' as io; | ||
|
||
import 'package:file/file.dart'; | ||
import 'package:file/memory.dart'; | ||
import 'package:test/test.dart'; | ||
|
||
import 'package:unified_analytics/unified_analytics.dart'; | ||
|
||
void main() { | ||
late FileSystem fs; | ||
late Directory home; | ||
late Analytics analytics; | ||
|
||
const String homeDirName = 'home'; | ||
const DashTool initialTool = DashTool.flutterTool; | ||
const String measurementId = 'measurementId'; | ||
const String apiSecret = 'apiSecret'; | ||
const int toolsMessageVersion = 1; | ||
const String toolsMessage = 'toolsMessage'; | ||
const String flutterChannel = 'flutterChannel'; | ||
const String flutterVersion = 'flutterVersion'; | ||
const String dartVersion = 'dartVersion'; | ||
const DevicePlatform platform = DevicePlatform.macos; | ||
|
||
setUp(() { | ||
// Setup the filesystem with the home directory | ||
final FileSystemStyle fsStyle = | ||
io.Platform.isWindows ? FileSystemStyle.windows : FileSystemStyle.posix; | ||
fs = MemoryFileSystem.test(style: fsStyle); | ||
home = fs.directory(homeDirName); | ||
}); | ||
|
||
test('Honor legacy dart analytics opt out', () { | ||
// Create the file for the dart legacy opt out | ||
final File dartLegacyConfigFile = | ||
home.childDirectory('.dart').childFile('dartdev.json'); | ||
dartLegacyConfigFile.createSync(recursive: true); | ||
dartLegacyConfigFile.writeAsStringSync(''' | ||
{ | ||
"firstRun": false, | ||
"enabled": false, | ||
"disclosureShown": true, | ||
"clientId": "52710e60-7c70-4335-b3a4-9d922630f12a" | ||
} | ||
'''); | ||
|
||
// The main analytics instance, other instances can be spawned within tests | ||
// to test how to instances running together work | ||
// | ||
// This instance should have the same parameters as the one above for | ||
// [initializationAnalytics] | ||
analytics = 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(analytics.telemetryEnabled, false); | ||
}); | ||
|
||
test('Telemetry enabled if legacy dart analytics is enabled', () { | ||
// Create the file for the dart legacy opt out | ||
final File dartLegacyConfigFile = | ||
home.childDirectory('.dart').childFile('dartdev.json'); | ||
dartLegacyConfigFile.createSync(recursive: true); | ||
dartLegacyConfigFile.writeAsStringSync(''' | ||
{ | ||
"firstRun": false, | ||
"enabled": true, | ||
"disclosureShown": true, | ||
"clientId": "52710e60-7c70-4335-b3a4-9d922630f12a" | ||
} | ||
'''); | ||
|
||
// The main analytics instance, other instances can be spawned within tests | ||
// to test how to instances running together work | ||
// | ||
// This instance should have the same parameters as the one above for | ||
// [initializationAnalytics] | ||
analytics = 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(analytics.telemetryEnabled, true); | ||
}); | ||
|
||
test('Honor legacy flutter analytics opt out', () { | ||
// Create the file for the dart legacy opt out | ||
final File flutterLegacyConfigFile = | ||
home.childDirectory('.dart').childFile('dartdev.json'); | ||
flutterLegacyConfigFile.createSync(recursive: true); | ||
flutterLegacyConfigFile.writeAsStringSync(''' | ||
{ | ||
"firstRun": false, | ||
"clientId": "4c3a3d1e-e545-47e7-b4f8-10129f6ab169", | ||
"enabled": false | ||
} | ||
'''); | ||
|
||
// The main analytics instance, other instances can be spawned within tests | ||
// to test how to instances running together work | ||
// | ||
// This instance should have the same parameters as the one above for | ||
// [initializationAnalytics] | ||
analytics = 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(analytics.telemetryEnabled, false); | ||
}); | ||
|
||
test('Telemetry enabled if legacy flutter analytics is enabled', () { | ||
// Create the file for the dart legacy opt out | ||
final File flutterLegacyConfigFile = | ||
home.childDirectory('.dart').childFile('dartdev.json'); | ||
flutterLegacyConfigFile.createSync(recursive: true); | ||
flutterLegacyConfigFile.writeAsStringSync(''' | ||
{ | ||
"firstRun": false, | ||
"clientId": "4c3a3d1e-e545-47e7-b4f8-10129f6ab169", | ||
"enabled": true | ||
} | ||
'''); | ||
|
||
// The main analytics instance, other instances can be spawned within tests | ||
// to test how to instances running together work | ||
// | ||
// This instance should have the same parameters as the one above for | ||
// [initializationAnalytics] | ||
analytics = 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(analytics.telemetryEnabled, true); | ||
}); | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless I'm reading this package's code wrong, it looks like this will be
platform.environment['UserProfile']
on windows (https://github.com/dart-lang/tools/blob/main/pkgs/unified_analytics/lib/src/utils.dart#L78), when the tool will be writing toplatform.environment['AppData']
on Windows https://github.com/dart-lang/usage/blob/master/lib/src/usage_impl_io.dart#L89There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh that's a very good catch. In the PDD, it says all files will be written to the user's home directory, am I correct to assume the home directory on windows is stored in
UserProfile
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my understanding is that "home directory" is an imprecise term, and especially on Windows there is not one "correct" answer for where a user's home directory is across different Windows versions and also application usages. In other words:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The home directory as the value of the UserProfile environment variable seems correct to me and fulfills the PDD. The most common definitions of "home directory" under Windows align with this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably should adjust the PDD to allow writing the files in a more Windows-friendly location like AppData.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we would be okay if we changed the env variable for home on windows to point to
APPDATA
then? That way we can continue what was done in the past and address @christopherfujino 'sitem 1
above?