Cleanup tech debt for the survey-related DevTools server APIs (#8282)

packages/devtools_app/lib/src/shared/server/_survey_api.dart

@@ -15,13 +15,13 @@ part of 'server.dart';
 Future<bool> setActiveSurvey(String value) async {
   if (isDevToolsServerAvailable) {
     final resp = await request(
-      '$apiSetActiveSurvey'
-      '?$activeSurveyName=$value',
+      '${SurveyApi.setActiveSurvey}'
+      '?$apiParameterValueKey=$value',
     );
     if ((resp?.statusOk ?? false) && json.decode(resp!.body)) {
       return true;
     } else {
-      logWarning(resp, apiSetActiveSurvey);
+      logWarning(resp, SurveyApi.setActiveSurvey);
     }
   }
   return false;
@@ -36,11 +36,11 @@ Future<bool> surveyActionTaken() async {
   bool surveyActionTaken = false;
 
   if (isDevToolsServerAvailable) {
-    final resp = await request(apiGetSurveyActionTaken);
+    final resp = await request(SurveyApi.getSurveyActionTaken);
     if (resp?.statusOk ?? false) {
       surveyActionTaken = json.decode(resp!.body);
     } else {
-      logWarning(resp, apiGetSurveyActionTaken);
+      logWarning(resp, SurveyApi.getSurveyActionTaken);
     }
   }
 
@@ -55,11 +55,11 @@ Future<bool> surveyActionTaken() async {
 Future<void> setSurveyActionTaken() async {
   if (isDevToolsServerAvailable) {
     final resp = await request(
-      '$apiSetSurveyActionTaken'
-      '?$surveyActionTakenPropertyName=true',
+      '${SurveyApi.setSurveyActionTaken}'
+      '?$apiParameterValueKey=true',
     );
     if (resp == null || !resp.statusOk || !(json.decode(resp.body) as bool)) {
-      logWarning(resp, apiSetSurveyActionTaken);
+      logWarning(resp, SurveyApi.setSurveyActionTaken);
     }
   }
 }
@@ -73,11 +73,11 @@ Future<int> surveyShownCount() async {
   int surveyShownCount = 0;
 
   if (isDevToolsServerAvailable) {
-    final resp = await request(apiGetSurveyShownCount);
+    final resp = await request(SurveyApi.getSurveyShownCount);
     if (resp?.statusOk ?? false) {
       surveyShownCount = json.decode(resp!.body);
     } else {
-      logWarning(resp, apiGetSurveyShownCount);
+      logWarning(resp, SurveyApi.getSurveyShownCount);
     }
   }
 
@@ -94,11 +94,11 @@ Future<int> incrementSurveyShownCount() async {
   int surveyShownCount = 0;
 
   if (isDevToolsServerAvailable) {
-    final resp = await request(apiIncrementSurveyShownCount);
+    final resp = await request(SurveyApi.incrementSurveyShownCount);
     if (resp?.statusOk ?? false) {
       surveyShownCount = json.decode(resp!.body);
     } else {
-      logWarning(resp, apiIncrementSurveyShownCount);
+      logWarning(resp, SurveyApi.incrementSurveyShownCount);
     }
   }
   return surveyShownCount;

packages/devtools_app/lib/src/shared/survey.dart

@@ -54,6 +54,10 @@ class SurveyService {
 
     _cachedSurvey ??= await fetchSurveyContent();
     if (_cachedSurvey?.id != null) {
+      // TODO(kenz): consider setting this value on the [SurveyService] and then
+      // we can send the active survey as a parameter in each survey-related
+      // DevTools server request. This would simplify the server API for
+      // DevTools surveys.
       await server.setActiveSurvey(_cachedSurvey!.id!);
     }
 

packages/devtools_shared/CHANGELOG.md

@@ -6,6 +6,13 @@
 * Deprecate `apiGetTestAppSizeFile` in favor of `AppSizeApi.getTestAppSizeFile`.
 * Deprecate `baseAppSizeFilePropertyName` in favor of `AppSizeApi.baseAppSizeFilePropertyName`.
 * Deprecate `testAppSizeFilePropertyName` in favor of `AppSizeApi.testAppSizeFilePropertyName`.
+* Deprecate `apiSetActiveSurvey` in favor of `SurveyApi.setActiveSurvey`.
+* Deprecate `activeSurveyName`.
+* Deprecate `apiGetSurveyActionTaken` in favor of `SurveyApi.getSurveyActionTaken`.
+* Deprecate `apiSetSurveyActionTaken` in favor of `SurveyApi.setSurveyActionTaken`.
+* Deprecate `surveyActionTakenPropertyName`.
+* Deprecate `apiGetSurveyShownCount` in favor of `SurveyApi.getSurveyShownCount`.
+* Deprecate `apiIncrementSurveyShownCount` in favor of `SurveyApi.incrementSurveyShownCount`.
 
 # 10.0.2
 * Update dependency `web_socket_channel: '>=2.4.0 <4.0.0'`.

packages/devtools_shared/lib/src/devtools_api.dart

@@ -28,29 +28,74 @@ const apiSetDevToolsEnabled = '${apiPrefix}setDevToolsEnabled';
 /// in queryParameter:
 const devToolsEnabledPropertyName = 'enabled';
 
-/// Survey properties APIs:
-/// setActiveSurvey sets the survey property to fetch and save JSON values e.g., Q1-2020
-const apiSetActiveSurvey = '${apiPrefix}setActiveSurvey';
+@Deprecated(
+  'Use SurveyApi.setActiveSurvey instead. '
+  'This field will be removed in devtools_shared >= 11.0.0.',
+)
+const apiSetActiveSurvey = SurveyApi.setActiveSurvey;
 
-/// Survey name passed in apiSetActiveSurvey, the activeSurveyName is the property name
-/// passed as a queryParameter and is the property in ~/.devtools too.
-const activeSurveyName = 'activeSurveyName';
+@Deprecated(
+  'Use apiParameterValueKey for the query parameter of the '
+  'SurveyApi.setActiveSurvey request instead. '
+  'This field will be removed in devtools_shared >= 11.0.0.',
+)
+const activeSurveyName = apiParameterValueKey;
 
-/// Returns the surveyActionTaken of the activeSurvey (apiSetActiveSurvey).
-const apiGetSurveyActionTaken = '${apiPrefix}getSurveyActionTaken';
+@Deprecated(
+  'Use SurveyApi.getSurveyActionTaken instead. '
+  'This field will be removed in devtools_shared >= 11.0.0.',
+)
+const apiGetSurveyActionTaken = SurveyApi.getSurveyActionTaken;
+
+@Deprecated(
+  'Use SurveyApi.setSurveyActionTaken instead. '
+  'This field will be removed in devtools_shared >= 11.0.0.',
+)
+const apiSetSurveyActionTaken = SurveyApi.setSurveyActionTaken;
 
-/// Sets the surveyActionTaken of the of the activeSurvey (apiSetActiveSurvey).
-const apiSetSurveyActionTaken = '${apiPrefix}setSurveyActionTaken';
+@Deprecated(
+  'Use apiParameterValueKey for the query parameter of the '
+  'SurveyApi.setSurveyActionTaken request instead. '
+  'This field will be removed in devtools_shared >= 11.0.0.',
+)
+const surveyActionTakenPropertyName = apiParameterValueKey;
 
-/// Property name to apiSetSurveyActionTaken the surveyActionTaken is the name
-/// passed in queryParameter:
-const surveyActionTakenPropertyName = 'surveyActionTaken';
+@Deprecated(
+  'Use SurveyApi.getSurveyShownCount instead. '
+  'This field will be removed in devtools_shared >= 11.0.0.',
+)
+const apiGetSurveyShownCount = SurveyApi.getSurveyShownCount;
 
-/// Returns the surveyShownCount of the of the activeSurvey (apiSetActiveSurvey).
-const apiGetSurveyShownCount = '${apiPrefix}getSurveyShownCount';
+@Deprecated(
+  'Use SurveyApi.incrementSurveyShownCount instead. '
+  'This field will be removed in devtools_shared >= 11.0.0.',
+)
+const apiIncrementSurveyShownCount = SurveyApi.incrementSurveyShownCount;
 
-/// Increments the surveyShownCount of the of the activeSurvey (apiSetActiveSurvey).
-const apiIncrementSurveyShownCount = '${apiPrefix}incrementSurveyShownCount';
+abstract class SurveyApi {
+  /// Sets the active survey value for the DevTools session.
+  ///
+  /// The active survey is used as a key to get and set values within the
+  /// DevTools store file.
+  static const setActiveSurvey = '${apiPrefix}setActiveSurvey';
+
+  /// Returns the 'surveyActionTaken' value for the active survey set by
+  /// [setActiveSurvey].
+  static const getSurveyActionTaken = '${apiPrefix}getSurveyActionTaken';
+
+  /// Sets the 'surveyActionTaken' value for the active survey set by
+  /// [setActiveSurvey].
+  static const setSurveyActionTaken = '${apiPrefix}setSurveyActionTaken';
+
+  /// Returns the 'surveyShownCount' value for the active survey set by
+  /// [setActiveSurvey].
+  static const getSurveyShownCount = '${apiPrefix}getSurveyShownCount';
+
+  /// Increments the 'surveyShownCount' value for the active survey set by
+  /// [setActiveSurvey].
+  static const incrementSurveyShownCount =
+      '${apiPrefix}incrementSurveyShownCount';
+}
 
 @Deprecated(
   'Use apiParameterValueKey for the query parameter of the '

packages/devtools_shared/lib/src/server/server_api.dart

@@ -113,68 +113,70 @@ class ServerApi {
         }
         return _encodeResponse(_devToolsStore.analyticsEnabled, api: api);
 
+      // TODO(kenz): move all the handlers into a separate handler class as a
+      // follow up PR to preserve the diff.
       // ----- DevTools survey store. -----
 
-      case apiSetActiveSurvey:
+      case SurveyApi.setActiveSurvey:
         // Assume failure.
         bool result = false;
 
         // Set the active survey used to store subsequent apiGetSurveyActionTaken,
         // apiSetSurveyActionTaken, apiGetSurveyShownCount, and
         // apiIncrementSurveyShownCount calls.
         if (queryParams.keys.length == 1 &&
-            queryParams.containsKey(activeSurveyName)) {
-          final surveyName = queryParams[activeSurveyName]!;
+            queryParams.containsKey(apiParameterValueKey)) {
+          final surveyName = queryParams[apiParameterValueKey]!;
 
           // Set the current activeSurvey.
           _devToolsStore.activeSurvey = surveyName;
           result = true;
         }
         return _encodeResponse(result, api: api);
-      case apiGetSurveyActionTaken:
+      case SurveyApi.getSurveyActionTaken:
         // Request setActiveSurvey has not been requested.
         if (_devToolsStore.activeSurvey == null) {
           return api.badRequest(
             '$errorNoActiveSurvey '
-            '- $apiGetSurveyActionTaken',
+            '- ${SurveyApi.getSurveyActionTaken}',
           );
         }
         // SurveyActionTaken has the survey been acted upon (taken or dismissed)
         return _encodeResponse(_devToolsStore.surveyActionTaken, api: api);
       // TODO(terry): remove the query param logic for this request.
       // setSurveyActionTaken should only be called with the value of true, so
       // we can remove the extra complexity.
-      case apiSetSurveyActionTaken:
+      case SurveyApi.setSurveyActionTaken:
         // Request setActiveSurvey has not been requested.
         if (_devToolsStore.activeSurvey == null) {
           return api.badRequest(
             '$errorNoActiveSurvey '
-            '- $apiSetSurveyActionTaken',
+            '- ${SurveyApi.setSurveyActionTaken}',
           );
         }
         // Set the SurveyActionTaken.
         // Has the survey been taken or dismissed..
-        if (queryParams.containsKey(surveyActionTakenPropertyName)) {
+        if (queryParams.containsKey(apiParameterValueKey)) {
           _devToolsStore.surveyActionTaken =
-              json.decode(queryParams[surveyActionTakenPropertyName]!);
+              json.decode(queryParams[apiParameterValueKey]!);
         }
         return _encodeResponse(_devToolsStore.surveyActionTaken, api: api);
-      case apiGetSurveyShownCount:
+      case SurveyApi.getSurveyShownCount:
         // Request setActiveSurvey has not been requested.
         if (_devToolsStore.activeSurvey == null) {
           return api.badRequest(
             '$errorNoActiveSurvey '
-            '- $apiGetSurveyShownCount',
+            '- ${SurveyApi.getSurveyShownCount}',
           );
         }
         // SurveyShownCount how many times have we asked to take survey.
         return _encodeResponse(_devToolsStore.surveyShownCount, api: api);
-      case apiIncrementSurveyShownCount:
+      case SurveyApi.incrementSurveyShownCount:
         // Request setActiveSurvey has not been requested.
         if (_devToolsStore.activeSurvey == null) {
           return api.badRequest(
             '$errorNoActiveSurvey '
-            '- $apiIncrementSurveyShownCount',
+            '- ${SurveyApi.incrementSurveyShownCount}',
           );
         }
         // Increment the SurveyShownCount, we've asked about the survey.