Bug 1547034: Add userFacingName and userFacingDescription to schema r=mythmon
authorEthan Glasser-Camp <ethan@betacantrips.com>
Thu, 16 May 2019 15:04:25 +0000
changeset 474248 31cf332c9b21698336c5e6c18939a3a8b58a7902
parent 474247 0de43d5275d15b83e80fa021bac4175f85a5d3a4
child 474249 440aa7a30693d06e6aef4bc256edc226f407ef49
push id113144
push usershindli@mozilla.com
push dateFri, 17 May 2019 16:44:55 +0000
treeherdermozilla-inbound@f4c4b796f845 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmythmon
bugs1547034
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1547034: Add userFacingName and userFacingDescription to schema r=mythmon Display these when available instead of generating one. We play some games here to let SinglePreferenceExperiment continue to validate according to the PreferenceExperiment schema. This is kind of ugly. Another approach might be to move the about-studies code that generates a description. I was hesitant to do this because it would mean losing the formatting. Depends on D29873 Differential Revision: https://phabricator.services.mozilla.com/D30969
toolkit/components/normandy/actions/SinglePreferenceExperimentAction.jsm
toolkit/components/normandy/actions/schemas/index.js
toolkit/components/normandy/content/about-studies/about-studies.js
toolkit/components/normandy/lib/PreferenceExperiments.jsm
toolkit/components/normandy/test/browser/browser_actions_PreferenceExperimentAction.js
toolkit/components/normandy/test/browser/browser_actions_SinglePreferenceExperimentAction.js
--- a/toolkit/components/normandy/actions/SinglePreferenceExperimentAction.jsm
+++ b/toolkit/components/normandy/actions/SinglePreferenceExperimentAction.jsm
@@ -24,16 +24,22 @@ class SinglePreferenceExperimentAction e
       preferenceBranchType,
       preferenceName,
       preferenceType,
       branches,
       ...remainingArguments
     } = recipe.arguments;
 
     const newArguments = {
+      // The multiple-preference-experiment schema requires a string
+      // name/description, which are necessary in the wire format, but
+      // experiment objects can have null for these fields. Add some
+      // filler fields here and remove them after validation.
+      userFacingName: "temp-name",
+      userFacingDescription: "temp-description",
       ...remainingArguments,
       branches: branches.map(branch => {
         const { value, ...branchProps } = branch;
         return {
           ...branchProps,
           preferences: {
             [preferenceName]: {
               preferenceBranchType,
@@ -47,16 +53,19 @@ class SinglePreferenceExperimentAction e
 
     const multiprefSchema = ActionSchemas["multiple-preference-experiment"];
 
     let [valid, validatedArguments] = JsonSchemaValidator.validateAndParseParameters(newArguments, multiprefSchema);
     if (!valid) {
       throw new Error(`Transformed arguments do not match schema. Original arguments: ${JSON.stringify(recipe.arguments)}, new arguments: ${JSON.stringify(newArguments)}, schema: ${JSON.stringify(multiprefSchema)}`);
     }
 
+    validatedArguments.userFacingName = null;
+    validatedArguments.userFacingDescription = null;
+
     recipe.arguments = validatedArguments;
 
     const newRecipe = {
       ...recipe,
       arguments: validatedArguments,
     };
 
     return super._run(newRecipe);
--- a/toolkit/components/normandy/actions/schemas/index.js
+++ b/toolkit/components/normandy/actions/schemas/index.js
@@ -168,24 +168,36 @@ const ActionSchemas = {
   },
 
   "multiple-preference-experiment": {
     $schema: "http://json-schema.org/draft-04/schema#",
     title: "Run a feature experiment activated by a set of preferences.",
     type: "object",
     required: [
       "slug",
+      "userFacingName",
+      "userFacingDescription",
       "branches",
     ],
     properties: {
       slug: {
         description: "Unique identifier for this experiment",
         type: "string",
         pattern: "^[A-Za-z0-9\\-_]+$",
       },
+      userFacingName: {
+        description: "User-facing name of the experiment",
+        type: "string",
+        minLength: 1,
+      },
+      userFacingDescription: {
+        description: "User-facing description of the experiment",
+        type: "string",
+        minLength: 1,
+      },
       experimentDocumentUrl: {
         description: "URL of a document describing the experiment",
         type: "string",
         format: "uri",
         default: "",
       },
       isHighPopulation: {
         description: "Marks the preference experiment as a high population experiment, that should be excluded from certain types of telemetry",
--- a/toolkit/components/normandy/content/about-studies/about-studies.js
+++ b/toolkit/components/normandy/content/about-studies/about-studies.js
@@ -246,38 +246,46 @@ class PreferenceStudyListItem extends Re
       experimentName: this.props.study.name,
       reason: "individual-opt-out",
     });
   }
 
   render() {
     const { study, translations } = this.props;
 
-    // Assume there is exactly one preference (old-style preference experiment).
-    const [preferenceName, { preferenceValue }] = Object.entries(study.preferences)[0];
-    // Sanitize the values by setting them as the text content of an element,
-    // and then getting the HTML representation of that text. This will have the
-    // browser safely sanitize them. Use outerHTML to also include the <code>
-    // element in the string.
-    const sanitizer = document.createElement("code");
-    sanitizer.textContent = preferenceName;
-    const sanitizedPreferenceName = sanitizer.outerHTML;
-    sanitizer.textContent = preferenceValue;
-    const sanitizedPreferenceValue = sanitizer.outerHTML;
-    const description = translations.preferenceStudyDescription
-      .replace(/%(?:1\$)?S/, sanitizedPreferenceName)
-      .replace(/%(?:2\$)?S/, sanitizedPreferenceValue);
+    let userFacingName = study.userFacingName;
+    if (!userFacingName) {
+      userFacingName = study.name.replace(/-?pref-?(flip|study)-?/, "").replace(/-?study-?/, "").slice(0, 1);
+    }
+
+    let description = study.userFacingDescription;
+    if (!description) {
+      // Assume there is exactly one preference (old-style preference experiment).
+      const [preferenceName, { preferenceValue }] = Object.entries(study.preferences)[0];
+      // Sanitize the values by setting them as the text content of an element,
+      // and then getting the HTML representation of that text. This will have the
+      // browser safely sanitize them. Use outerHTML to also include the <code>
+      // element in the string.
+      const sanitizer = document.createElement("code");
+      sanitizer.textContent = preferenceName;
+      const sanitizedPreferenceName = sanitizer.outerHTML;
+      sanitizer.textContent = preferenceValue;
+      const sanitizedPreferenceValue = sanitizer.outerHTML;
+      description = translations.preferenceStudyDescription
+            .replace(/%(?:1\$)?S/, sanitizedPreferenceName)
+            .replace(/%(?:2\$)?S/, sanitizedPreferenceValue);
+    }
 
     return (
       r("li", {
         className: classnames("study pref-study", { disabled: study.expired }),
         "data-study-name": study.name,
       },
         r("div", { className: "study-icon" },
-          study.name.replace(/-?pref-?(flip|study)-?/, "").replace(/-?study-?/, "").slice(0, 1)
+          userFacingName,
         ),
         r("div", { className: "study-details" },
           r("div", { className: "study-header" },
             r("span", { className: "study-name" }, study.name),
             r("span", {}, "\u2022"), // &bullet;
             r("span", { className: "study-status" }, study.expired ? translations.completeStatus : translations.activeStatus),
           ),
           r("div", { className: "study-description", dangerouslySetInnerHTML: { __html: description }}),
--- a/toolkit/components/normandy/lib/PreferenceExperiments.jsm
+++ b/toolkit/components/normandy/lib/PreferenceExperiments.jsm
@@ -17,16 +17,24 @@
  * an active experiment.
  */
 
 /**
  * Experiments store info about an active or expired preference experiment.
  * @typedef {Object} Experiment
  * @property {string} name
  *   Unique name of the experiment
+ * @property {string|null} userFacingName
+ *   A user-friendly name for the experiment. Null on old-style
+ *   single-preference experiments, which do not have a
+ *   userFacingName.
+ * @property {string|null} userFacingDescription
+ *   A user-friendly description of the experiment. Null on old-style
+ *   single-preference experiments, which do not have a
+ *   userFacingDescription.
  * @property {string} branch
  *   Experiment branch that the user was matched to
  * @property {boolean} expired
  *   If false, the experiment is active.
  * @property {string} lastSeen
  *   ISO-formatted date string of when the experiment was last seen from the
  *   recipe server.
  * @property {Object} preferences
@@ -378,16 +386,18 @@ var PreferenceExperiments = {
    *   - if an experiment for the given preference is active
    *   - If the given preferenceType does not match the existing stored preference
    */
   async start({
     name,
     branch,
     preferences,
     experimentType = "exp",
+    userFacingName = null,
+    userFacingDescription = null,
   }) {
     log.debug(`PreferenceExperiments.start(${name}, ${branch})`);
 
     const store = await ensureStorage();
     if (name in store.data.experiments) {
       TelemetryEvents.sendEvent("enrollFailed", "preference_study", name, {reason: "name-conflict"});
       throw new Error(`A preference experiment named "${name}" already exists.`);
     }
@@ -452,16 +462,18 @@ var PreferenceExperiments = {
     /** @type {Experiment} */
     const experiment = {
       name,
       branch,
       expired: false,
       lastSeen: new Date().toJSON(),
       preferences,
       experimentType,
+      userFacingName,
+      userFacingDescription,
     };
 
     store.data.experiments[name] = experiment;
     store.saveSoon();
 
     TelemetryEnvironment.setExperimentActive(name, branch, {type: EXPERIMENT_TYPE_PREFIX + experimentType});
     TelemetryEvents.sendEvent("enroll", "preference_study", name, {experimentType, branch});
     await this.saveStartupPrefs();
--- a/toolkit/components/normandy/test/browser/browser_actions_PreferenceExperimentAction.js
+++ b/toolkit/components/normandy/test/browser/browser_actions_PreferenceExperimentAction.js
@@ -33,16 +33,18 @@ function branchFactory(opts = {}) {
   };
 }
 
 function argumentsFactory(args) {
   const defaultBranches = (args && args.branches) || [{}];
   const branches = defaultBranches.map(branchFactory);
   return {
     slug: "test",
+    userFacingName: "Super Cool Test Experiment",
+    userFacingDescription: "Test experiment from browser_actions_PreferenceExperimentAction.",
     isHighPopulation: false,
     ...args,
     branches,
   };
 }
 
 function preferenceExperimentFactory(args) {
   return recipeFactory({
--- a/toolkit/components/normandy/test/browser/browser_actions_SinglePreferenceExperimentAction.js
+++ b/toolkit/components/normandy/test/browser/browser_actions_SinglePreferenceExperimentAction.js
@@ -63,16 +63,18 @@ decorate_task(
     await action.runRecipe(recipe);
     await action.finalize();
 
     Assert.deepEqual(runStub.args, [[{
       id: recipe.id,
       name: "preference-experiment",
       arguments: {
         slug: "test",
+        userFacingName: null,
+        userFacingDescription: null,
         isHighPopulation: false,
         branches: [{
           slug: "branch1",
           ratio: 1,
           preferences: {
             "fake.preference": {
               preferenceValue: "branch1",
               preferenceType: "string",