Enhance DevTools survey logic (#6754)
diff --git a/packages/devtools_app/lib/src/shared/analytics/_analytics_stub.dart b/packages/devtools_app/lib/src/shared/analytics/_analytics_stub.dart index 3f4a200..8906b46 100644 --- a/packages/devtools_app/lib/src/shared/analytics/_analytics_stub.dart +++ b/packages/devtools_app/lib/src/shared/analytics/_analytics_stub.dart
@@ -15,6 +15,12 @@ final _log = Logger('_analytics_stub'); +/// The IDE that DevTools was launched from. +/// +/// This is just a stub value so that we can access the [ideLaunched] field on +/// both web and desktop, and manipulate this value for tests running on the VM. +String ideLaunched = ''; + Future<void> setAnalyticsEnabled(bool value) async {} FutureOr<bool> isAnalyticsEnabled() => false;
diff --git a/packages/devtools_app/lib/src/shared/analytics/_analytics_web.dart b/packages/devtools_app/lib/src/shared/analytics/_analytics_web.dart index ab69c09..63d1c92 100644 --- a/packages/devtools_app/lib/src/shared/analytics/_analytics_web.dart +++ b/packages/devtools_app/lib/src/shared/analytics/_analytics_web.dart
@@ -18,7 +18,6 @@ import '../../../devtools.dart' as devtools show version; import '../config_specific/server/server.dart' as server; import '../globals.dart'; -import '../primitives/url_utils.dart'; import '../utils.dart'; import 'analytics_common.dart'; import 'constants.dart' as gac; @@ -269,7 +268,7 @@ devtools_platform: devtoolsPlatformType, devtools_chrome: devtoolsChrome, devtools_version: devtoolsVersion, - ide_launched: ideLaunched, + ide_launched: _ideLaunched, flutter_client_id: flutterClientId, is_external_build: isExternalBuild.toString(), is_embedded: ideTheme.embed.toString(), @@ -730,12 +729,11 @@ _devtoolsChrome = newDevtoolsChrome; } +/// The IDE that DevTools was launched from. +/// +/// Defaults to [ideLaunchedCLI] if DevTools was not launched from the IDE. String get ideLaunched => _ideLaunched; -set ideLaunched(String newIdeLaunched) { - _ideLaunched = newIdeLaunched; -} - String get ideLaunchedFeature => _ideLaunchedFeature; set ideLaunchedFeature(String newIdeLaunchedFeature) { @@ -827,11 +825,11 @@ // Look at the query parameters '&ide=' and record in GA. void computeDevToolsQueryParams() { - ideLaunched = ideLaunchedCLI; // Default is Command Line launch. + _ideLaunched = ideLaunchedCLI; // Default is Command Line launch. final ideValue = ideFromUrl(); if (ideValue != null) { - ideLaunched = ideValue; + _ideLaunched = ideValue; } final ideFeature = lookupFromQueryParams('ideFeature'); @@ -882,15 +880,11 @@ Map<String, dynamic> generateSurveyQueryParameters() { const ideKey = 'IDE'; - const fromKey = 'From'; + const versionKey = 'Version'; const internalKey = 'Internal'; - - final internalValue = (!isExternalBuild).toString(); - final fromPage = extractCurrentPageFromUrl(window.location.toString()); - return { - ideKey: ideLaunched, - fromKey: fromPage, - internalKey: internalValue, + ideKey: _ideLaunched, + versionKey: devtoolsVersion, + internalKey: (!isExternalBuild).toString(), }; }
diff --git a/packages/devtools_app/lib/src/shared/survey.dart b/packages/devtools_app/lib/src/shared/survey.dart index 42fb1b2..a29888c 100644 --- a/packages/devtools_app/lib/src/shared/survey.dart +++ b/packages/devtools_app/lib/src/shared/survey.dart
@@ -4,10 +4,13 @@ import 'dart:convert'; +import 'package:clock/clock.dart'; +import 'package:devtools_shared/devtools_shared.dart'; import 'package:flutter/widgets.dart'; import 'package:http/http.dart'; import 'package:logging/logging.dart'; +import '../../../devtools.dart' as devtools show version; import '../shared/notifications.dart'; import 'analytics/analytics.dart' as ga; import 'config_specific/launch_url/launch_url.dart'; @@ -40,7 +43,7 @@ // If the server is unavailable we don't need to do anything survey related. if (!server.isDevToolsServerAvailable) return null; - _cachedSurvey ??= await _fetchSurveyContent(); + _cachedSurvey ??= await fetchSurveyContent(); if (_cachedSurvey?.id != null) { await server.setActiveSurvey(_cachedSurvey!.id!); } @@ -107,15 +110,11 @@ final surveyActionTaken = await server.surveyActionTaken(); if (surveyActionTaken) return false; - final currentTimeMs = DateTime.now().millisecondsSinceEpoch; - final activeSurveyRange = Range( - _cachedSurvey!.startDate!.millisecondsSinceEpoch, - _cachedSurvey!.endDate!.millisecondsSinceEpoch, - ); - return activeSurveyRange.contains(currentTimeMs); + return _cachedSurvey!.shouldShow; } - Future<DevToolsSurvey?> _fetchSurveyContent() async { + @visibleForTesting + Future<DevToolsSurvey?> fetchSurveyContent() async { try { final response = await get(_metadataUrl); if (response.statusCode == 200) { @@ -152,19 +151,43 @@ this.endDate, this.title, this.url, + this.minDevToolsVersion, + this.devEnvironments, ); factory DevToolsSurvey.parse(Map<String, dynamic> json) { - final id = json['uniqueId']; - final startDate = - json['startDate'] != null ? DateTime.parse(json['startDate']) : null; + final id = json[_uniqueIdKey]; + final startDate = json[_startDateKey] != null + ? DateTime.parse(json[_startDateKey]) + : null; final endDate = - json['startDate'] != null ? DateTime.parse(json['endDate']) : null; - final title = json['title']; - final surveyUrl = json['url']; - return DevToolsSurvey._(id, startDate, endDate, title, surveyUrl); + json[_endDateKey] != null ? DateTime.parse(json[_endDateKey]) : null; + final title = json[_titleKey]; + final surveyUrl = json[_urlKey]; + final minDevToolsVersion = json[_minDevToolsVersionKey] != null + ? SemanticVersion.parse(json[_minDevToolsVersionKey]) + : null; + final devEnvironments = + (json[_devEnvironmentsKey] as List?)?.cast<String>().toList(); + return DevToolsSurvey._( + id, + startDate, + endDate, + title, + surveyUrl, + minDevToolsVersion, + devEnvironments, + ); } + static const _uniqueIdKey = 'uniqueId'; + static const _startDateKey = 'startDate'; + static const _endDateKey = 'endDate'; + static const _titleKey = 'title'; + static const _urlKey = 'url'; + static const _minDevToolsVersionKey = 'minDevToolsVersion'; + static const _devEnvironmentsKey = 'devEnvironments'; + final String? id; final DateTime? startDate; @@ -173,5 +196,47 @@ final String? title; + /// The url for the survey that the user will open in a browser when they + /// respond to the survey prompt. final String? url; + + /// The minimum DevTools version that this survey should is for. + /// + /// If the current version of DevTools is older than [minDevToolsVersion], the + /// survey prompt in DevTools will not be shown. + /// + /// If [minDevToolsVersion] is null, the survey will be shown for any version + /// of DevTools as long as all the other requirements are satisfied. + final SemanticVersion? minDevToolsVersion; + + /// A list of development environments to show the survey for (e.g. 'VSCode', + /// 'Android-Studio', 'IntelliJ-IDEA', 'CLI', etc.). + /// + /// If [devEnvironments] is null, the survey can be shown to any platform. + /// + /// The possible values for this list correspond to the possible values of + /// [_ideLaunched] from [shared/analytics/_analytics_web.dart]. + final List<String>? devEnvironments; +} + +extension ShowSurveyExtension on DevToolsSurvey { + bool get meetsDateRequirement => (startDate == null || endDate == null) + ? false + : Range( + startDate!.millisecondsSinceEpoch, + endDate!.millisecondsSinceEpoch, + ).contains(clock.now().millisecondsSinceEpoch); + + bool get meetsMinVersionRequirement => + minDevToolsVersion == null || + SemanticVersion.parse(devtools.version) + .isSupported(minSupportedVersion: minDevToolsVersion!); + + bool get meetsEnvironmentRequirement => + devEnvironments == null || devEnvironments!.contains(ga.ideLaunched); + + bool get shouldShow => + meetsDateRequirement && + meetsMinVersionRequirement && + meetsEnvironmentRequirement; }
diff --git a/packages/devtools_app/pubspec.yaml b/packages/devtools_app/pubspec.yaml index 4d052f0..feee3cb 100644 --- a/packages/devtools_app/pubspec.yaml +++ b/packages/devtools_app/pubspec.yaml
@@ -21,6 +21,7 @@ # TODO: https://github.com/flutter/devtools/issues/4728 - remove constraint when archive is fixed archive: <3.3.3 async: ^2.0.0 + clock: ^1.1.1 codicon: ^1.0.0 collection: ^1.15.0 dap: ^1.1.0
diff --git a/packages/devtools_app/test/shared/survey_test.dart b/packages/devtools_app/test/shared/survey_test.dart index 6b16d66..80215f2 100644 --- a/packages/devtools_app/test/shared/survey_test.dart +++ b/packages/devtools_app/test/shared/survey_test.dart
@@ -2,10 +2,19 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:clock/clock.dart'; +import 'package:devtools_app/devtools_app.dart'; import 'package:devtools_app/src/shared/survey.dart'; import 'package:flutter_test/flutter_test.dart'; void main() { + group('SurveyService', () { + test('can fetch survey metadata', () async { + final survey = await SurveyService().fetchSurveyContent(); + expect(survey, isNotNull); + }); + }); + group('DevToolsSurvey', () { test('parse constructor succeeds', () { final survey = DevToolsSurvey.parse({ @@ -19,6 +28,8 @@ 'url': 'https://google.qualtrics.com/jfe/form/SV_9XDmbo8lhv0VaUl', 'startDate': '2020-10-30T09:00:00-07:00', 'endDate': '2020-11-30T09:00:00-07:00', + 'minDevToolsVersion': '2.29.0', + 'devEnvironments': ['VSCode'], }); expect(survey.id, equals('2020Q4')); expect(survey.startDate, equals(DateTime.utc(2020, 10, 30, 16))); @@ -31,6 +42,8 @@ survey.url, 'https://google.qualtrics.com/jfe/form/SV_9XDmbo8lhv0VaUl', ); + expect(survey.minDevToolsVersion.toString(), '2.29.0'); + expect(survey.devEnvironments, ['VSCode']); final emptySurvey = DevToolsSurvey.parse({}); expect(emptySurvey.id, isNull); @@ -38,6 +51,89 @@ expect(emptySurvey.endDate, isNull); expect(emptySurvey.title, isNull); expect(emptySurvey.url, isNull); + expect(emptySurvey.minDevToolsVersion, isNull); + expect(emptySurvey.devEnvironments, isNull); + }); + + group('should show', () { + test('empty survey', () { + final emptySurvey = DevToolsSurvey.parse({}); + + withClock(Clock.fixed(DateTime(2023, 11, 7)), () { + expect(emptySurvey.meetsDateRequirement, isFalse); + }); + withClock(Clock.fixed(DateTime(2023, 11, 15)), () { + expect(emptySurvey.meetsDateRequirement, isFalse); + }); + expect(emptySurvey.meetsMinVersionRequirement, isTrue); + expect(emptySurvey.meetsEnvironmentRequirement, isTrue); + expect(emptySurvey.shouldShow, isFalse); + }); + + test('real survey', () { + final survey = DevToolsSurvey.parse({ + '_comments': [ + 'uniqueId must be updated with each new survey so DevTools knows to re-prompt users.', + 'title should not exceed 45 characters.', + 'startDate and endDate should follow ISO 8601 standard with a timezone offset.', + ], + 'uniqueId': '2020Q4', + 'title': 'Help improve DevTools! Take our Q4 survey.', + 'url': 'https://google.qualtrics.com/jfe/form/SV_9XDmbo8lhv0VaUl', + 'startDate': '2023-10-30T09:00:00-07:00', + 'endDate': '2023-11-14T09:00:00-07:00', + 'minDevToolsVersion': '2.29.0', + 'devEnvironments': ['VSCode'], + }); + + ideLaunched = 'VSCode'; + withClock(Clock.fixed(DateTime(2023, 11, 7)), () { + expect(survey.shouldShow, isTrue); + }); + }); + + test('meetsDateRequirement', () { + final survey = DevToolsSurvey.parse({ + 'startDate': '2023-10-30T09:00:00-07:00', + 'endDate': '2023-11-14T09:00:00-07:00', + }); + + withClock(Clock.fixed(DateTime(2023, 11, 7)), () { + expect(survey.meetsDateRequirement, isTrue); + }); + withClock(Clock.fixed(DateTime(2023, 11, 15)), () { + expect(survey.meetsDateRequirement, isFalse); + }); + }); + + test('meetsMinVersionRequirement', () { + var survey = DevToolsSurvey.parse({'minDevToolsVersion': '2.25.0'}); + expect(survey.meetsMinVersionRequirement, isTrue); + + survey = DevToolsSurvey.parse({'minDevToolsVersion': '5.25.0'}); + expect(survey.meetsMinVersionRequirement, isFalse); + }); + + test('meetsEnvironmentRequirement', () { + final vsCodeOnlySurvey = DevToolsSurvey.parse({ + 'devEnvironments': ['VSCode'], + }); + final intelliJSurvey = DevToolsSurvey.parse({ + 'devEnvironments': ['Android-Studio', 'IntelliJ-IDEA'], + }); + + ideLaunched = 'Android-Studio'; + expect(vsCodeOnlySurvey.meetsEnvironmentRequirement, isFalse); + expect(intelliJSurvey.meetsEnvironmentRequirement, isTrue); + + ideLaunched = 'IntelliJ-IDEA'; + expect(vsCodeOnlySurvey.meetsEnvironmentRequirement, isFalse); + expect(intelliJSurvey.meetsEnvironmentRequirement, isTrue); + + ideLaunched = 'VSCode'; + expect(vsCodeOnlySurvey.meetsEnvironmentRequirement, isTrue); + expect(intelliJSurvey.meetsEnvironmentRequirement, isFalse); + }); }); }); }
diff --git a/packages/devtools_shared/lib/src/utils/semantic_version.dart b/packages/devtools_shared/lib/src/utils/semantic_version.dart index 80cd0bd..0682347 100644 --- a/packages/devtools_shared/lib/src/utils/semantic_version.dart +++ b/packages/devtools_shared/lib/src/utils/semantic_version.dart
@@ -117,8 +117,8 @@ bool get isPreRelease => preReleaseMajor != null || preReleaseMinor != null; - bool isSupported({required SemanticVersion supportedVersion}) => - compareTo(supportedVersion) >= 0; + bool isSupported({required SemanticVersion minSupportedVersion}) => + compareTo(minSupportedVersion) >= 0; @override // ignore: avoid-dynamic, necessary here.
diff --git a/packages/devtools_shared/test/semantic_version_test.dart b/packages/devtools_shared/test/semantic_version_test.dart index 38eefb8..b4f202e 100644 --- a/packages/devtools_shared/test/semantic_version_test.dart +++ b/packages/devtools_shared/test/semantic_version_test.dart
@@ -81,27 +81,27 @@ test('isVersionSupported', () { final supportedVersion = SemanticVersion(major: 1, minor: 1, patch: 1); expect( - SemanticVersion().isSupported(supportedVersion: SemanticVersion()), + SemanticVersion().isSupported(minSupportedVersion: SemanticVersion()), isTrue, ); expect( SemanticVersion(major: 1, minor: 1, patch: 2) - .isSupported(supportedVersion: supportedVersion), + .isSupported(minSupportedVersion: supportedVersion), isTrue, ); expect( SemanticVersion(major: 1, minor: 2, patch: 1) - .isSupported(supportedVersion: supportedVersion), + .isSupported(minSupportedVersion: supportedVersion), isTrue, ); expect( SemanticVersion(major: 2, minor: 1, patch: 1) - .isSupported(supportedVersion: supportedVersion), + .isSupported(minSupportedVersion: supportedVersion), isTrue, ); expect( SemanticVersion(major: 2, minor: 1, patch: 1).isSupported( - supportedVersion: SemanticVersion(major: 2, minor: 2, patch: 1), + minSupportedVersion: SemanticVersion(major: 2, minor: 2, patch: 1), ), isFalse, );