Do not submit survey values that are not defined

CHANGELOG.md

@@ -3,6 +3,7 @@
 [Full changelog](https://github.com/mozilla-rally/core-addon/compare/v1.3.1...master)
 
 * [#624](https://github.com/mozilla-rally/rally-core-addon/pull/624): Update the `income` field to use the key `exactIncome`; update `metrics.yaml` to change `income` to `exact_income` of type `quantity`.
+* [#636](https://github.com/mozilla-rally/rally-core-addon/pull/636): Remove undefined values from demographic survey submission.
 
 # v1.3.1 (2021-05-20)
 

src/routes/Onboarding.svelte

@@ -48,7 +48,6 @@
     dispatch("onboarding-complete");
   }
 
-  let results;
   let formattedResults;
 </script>
 
@@ -65,7 +64,7 @@
         {:else if view === 'terms'}
           <TermsOfService />
         {:else if view === 'demographics'}
-          <Demographics bind:results bind:formattedResults />
+          <Demographics bind:formattedResults />
         {/if}
       </Main>
       <OnboardingCTAContainer

src/routes/demographics/Content.svelte

@@ -10,9 +10,11 @@
   import { formatInput, formatAnswersForResponse } from "./formatters";
 
   export let results = createResultObject(schema);
-  // create the outputted formatted results.
-  export let formattedResults = formatAnswersForResponse(schema, results, inputFormatters);
-  $: formattedResults = formatAnswersForResponse(schema, results, inputFormatters);
+  export let workingResults = createResultObject(schema, results);
+  $: workingResults = createResultObject(schema, workingResults);
+  // create the outputted formatted workingResults.
+  export let formattedResults = formatAnswersForResponse(schema, workingResults, inputFormatters);
+  $: formattedResults = formatAnswersForResponse(schema, workingResults, inputFormatters);
 </script>
 
 <style>
@@ -141,17 +143,17 @@
   <hr />
 
   <form class="mzp-c-form">
-    {#each Object.keys(results) as question}
+    {#each Object.keys(workingResults) as question}
       <fieldset class="mzp-c-field-set" class:mzp-c-field-set-text={schema[question].type === 'text'}>
         <legend class="mzp-c-field-label"
           for={schema[question].key}
           class:remove-bottom-margin={schema[question].sublabel}
         >
           {schema[question].label}
-          {#if questionIsAnswered(results[question], schema[question].type)}
+          {#if questionIsAnswered(workingResults[question], schema[question].type)}
             <ClearAnswerButton on:click={(e) => {
               e.preventDefault();
-              results[question] = clearAnswer(schema[question].type);
+              workingResults[question] = clearAnswer(schema[question].type);
             }} />
           {/if}
         </legend>
@@ -171,27 +173,27 @@
               class:mzp-is-error={
                 inputFormatters.showErrors(question) &&
                 inputFormatters.hasValidator(question) &&
-                inputFormatters[question].isInvalid(results[question])
+                inputFormatters[question].isInvalid(workingResults[question])
             }>
               <input
                 type="text"
                 use:formatInput={inputFormatters[question]}
                 class:right={inputFormatters[question].alignRight}
-                on:blur={(event) => { results[question] = event.target.value; }}
-                on:focus={(event) => { results[question] = event.target.value; }}
-                on:input={(event) => { results[question] = event.target.value; }}
-                value={results[question]}
+                on:blur={(event) => { workingResults[question] = event.target.value; }}
+                on:focus={(event) => { workingResults[question] = event.target.value; }}
+                on:input={(event) => { workingResults[question] = event.target.value; }}
+                value={workingResults[question]}
                 />
                 <span style="min-height: 24px; display: block;">
               {#if 
                   inputFormatters.showErrors(question) && // show errors if blurred
                   inputFormatters.hasValidator(question) && // show errors if there's a validator for this question
-                  inputFormatters[question].isInvalid(results[question])
+                  inputFormatters[question].isInvalid(workingResults[question])
                 }
                 <span
                   class="mzp-c-fieldnote"
                   transition:fly={{ duration: 300, y: 5 }}>
-                  {inputFormatters[question].isInvalid(results[question])}
+                  {inputFormatters[question].isInvalid(workingResults[question])}
                 </span>
               {/if}
               </span>
@@ -204,14 +206,14 @@
                     class="mzp-c-choice-control"
                     type="radio"
                     id="answer-{answer.key}"
-                    bind:group={results[question]}
+                    bind:group={workingResults[question]}
                     value={answer.key} />
                 {:else if schema[question].type === 'multi'}
                   <input
                     class="mzp-c-choice-control"
                     type="checkbox"
                     id="answer-{answer.key}"
-                    bind:group={results[question]}
+                    bind:group={workingResults[question]}
                     value={answer.key} />
                 {/if}
                 <label class="mzp-c-choice-label" for="answer-{answer.key}">
@@ -225,5 +227,5 @@
     {/each}
   </form>
   <!-- Add a slot to aid in  -->
-  <slot name='call-to-action' formattedResults={formatAnswersForResponse(schema, results, inputFormatters)} validated={inputFormatters.validateAllQuestions(schema, results)}></slot>
+  <slot name='call-to-action' formattedResults={formatAnswersForResponse(schema, workingResults, inputFormatters)} validated={inputFormatters.validateAllQuestions(schema, workingResults)}></slot>
 </div>

src/routes/demographics/formatters.js

@@ -1,9 +1,9 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
- 
- 
- 
+
+import { questionIsAnswered } from "./survey-tools.js";
+
  /** This Svelte action will format an input element on input, focus, and blur events. 
   * It takes in a formatter below of type FieldFormatter.
  */
@@ -93,6 +93,9 @@ export class FieldFormatter {
 
   export function currencyFormatter() {
     const formatCurrencyInput = (value) => {
+      if (value === undefined) {
+        return '';
+      }
       const r = /^\d/;
       return value.split('').filter(a => r.test(a)).join('');
     }
@@ -138,6 +141,9 @@ export class FieldFormatter {
 
   export function zipcodeFormatter() {
     const formatZipcodeInput = (value) => {
+      if (value === undefined) {
+        return '';
+      }
       const r = /^\d/;
       return value.split('').filter(a => r.test(a)).join('').slice(0,5);
     }
@@ -148,6 +154,7 @@ export class FieldFormatter {
     zipcode.formatForEditing(formatZipcodeInput);
     zipcode.formatForResponse(formatZipcodeInput);
     zipcode.setValidator((text) => {
+        if (text === undefined) return false;
         const number = Number(text);
         if (text.length > 0 && text.length < 5) return 'zipcode must be a five-digit number';
         if (Number.isNaN(number)) return 'zipcode must be a five-digit number';
@@ -214,19 +221,20 @@ export function createInputFormatters(schema) {
   }
 }
 
-export function _formatFor(schema, answers, formatters, formatType) {
+export function _formatFor(schema, answers, formatters, formatType, removeUnanswered = false) {
   const transformedAnswers = {};
   Object.keys(schema).forEach(key => {
     const answer = answers[key];
-    // currently, only text fields can have response transformations.
+
+    if (!removeUnanswered || questionIsAnswered(answer, schema[key].type))
     if (schema[key].type === 'text') {
       if (formatters.has(key, formatType)) {
         transformedAnswers[key] = formatters[key][formatType](answer);
       } else {
         transformedAnswers[key] = answer;
       }
     } else if (schema[key].type === 'multi') {
-      transformedAnswers[key] = [...answer];
+      transformedAnswers[key] = answer ? [...answer] : [];
     } else if (schema[key].type === 'single') {
       transformedAnswers[key] = answer;
     }
@@ -235,7 +243,7 @@ export function _formatFor(schema, answers, formatters, formatType) {
 }
 
 export function formatAnswersForResponse(schema, answers, formatters) {
-  return _formatFor(schema, answers, formatters, 'response');
+  return _formatFor(schema, answers, formatters, 'response', true);
 }
 
 export function formatAnswersForDisplay(schema, answers, formatters) {

src/routes/demographics/survey-tools.js

@@ -2,7 +2,16 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-export function createResultObject(schema) {
+ /**
+  * Constructs an answer object based on the schema & any pre-existing answers.
+  * @param {object} schema 
+  * @param {object} currentAnswers the current survey answers, if any. If there is an entry in currentAnswers
+  * that does not exist in the schema, will ignore the value and only impute values outlined in the schema.
+  * This ensures that the working answer object will always be up-to-date with the latest schema.
+  * @returns {object} the schema with default answers to questions, and any additional 
+  * pre-existing answers from currentAnswers.
+  */
+export function createResultObject(schema, currentAnswers) {
     return Object.values(schema).reduce((acc, config) => {
         let defaultValue = undefined;
         switch (config.type) {
@@ -22,18 +31,30 @@ export function createResultObject(schema) {
             break;
         }
         // create response entry here.
-        acc[config.key] = defaultValue;
+        let answer;
+        if (currentAnswers && currentAnswers[config.key] !== undefined) {
+          answer = currentAnswers[config.key];
+        } else {
+          answer = defaultValue;
+        }
+        acc[config.key] = answer;
         return acc;
       }, {});
 }
 
 export function questionIsAnswered(answer, questionType) {
+  if (answer === undefined) return false;
   if (questionType === 'text') return answer.length > 0;
   if (questionType === 'single') return answer !== undefined;
   if (questionType === 'multi') return answer.length > 0;
   throw Error('unknown question type');
 }
 
+/**
+ * Clears the display version of the answer.
+ * @param {string} questionType the question type (one of text, single, or multi)
+ * @returns {string || undefined || []} 
+ */
 export function clearAnswer(questionType) {
   switch (questionType) {
     case "text": {

tests/options-page/unit/formatters.test.js

@@ -87,14 +87,17 @@
             // We get RangeError: maximumFractionDigits value is out of range.
             assert.equal(formatter.display("90210"), "90210");
             assert.equal(formatter.display("90210abcd"), "90210");
+            assert.equal(formatter.display(undefined), "");
         })
         it("sets the response value to be a numeric string", function() {
             assert.equal(formatter.response("90210"), "90210");
             assert.equal(formatter.response("90210abcd!!!!"), "90210");
+            assert.equal(formatter.response(undefined), "");
         })
         it("properly validates a zipcode string", function() {
             assert.equal(formatter.isInvalid("90210"), false);
             assert.equal(formatter.isInvalid("90"), 'zipcode must be a five-digit number');
+            assert.equal(formatter.isInvalid(undefined), false);
         })
     })
     describe("_formatFor", function() {
@@ -157,5 +160,13 @@
                 otherMulti: ["1", "3"]
             })
         });
+
+        it("formats a typical payload for response, removing unanswered questions", function() {
+            const answers = { income: undefined, zipcode: "90210", other: undefined, otherMulti: []};
+            const formattedAnswers = _formatFor(schema, answers, formatters, "response", true);
+            assert.deepEqual(formattedAnswers, {
+                zipcode: "90210",
+            })
+        });
     })
  })

tests/options-page/unit/survey-tools.test.js

@@ -0,0 +1,65 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+ import { createResultObject } from '../../../src/routes/demographics/survey-tools.js';
+ import { strict as assert } from 'assert';
+
+ const schema = {
+    income: {
+        key: "income",
+        type: "text",
+        formatter: 'currency'
+    },
+    zipcode: {
+        key: "zipcode",
+        type: "text",
+        formatter: "zipcode"
+    },
+    other: {
+        key: "other",
+        type: "single",
+        values: [
+            {"key": 'a', label: "A"},
+            {"key": 'b', label: "B"},
+        ]
+    },
+    otherMulti: {
+        key: "otherMulti",
+        type: "multi",
+        values: [
+            {key: "1", label: "One"},
+            {key: "2", label: "Two"},
+            {key: "3", label: "Three"},
+        ]
+    }
+};
+
+ describe('survey-tools', function() {
+     describe('creatResultObject', function() {
+        it("creates an empty resultObject based on a schema", function() {
+            assert.deepEqual(createResultObject(schema), {
+                income: '',
+                zipcode: '',
+                other: undefined,
+                otherMulti: []
+            })
+        });
+        it("creates a new resultObject based on a schema and additional answers", function() {
+            assert.deepEqual(createResultObject(schema, {income: '10000', otherMulti: ["1", "3"]}), {
+                income: '10000',
+                zipcode: '',
+                other: undefined,
+                otherMulti: ["1", "3"]
+            })
+        })
+        it("ignores any extraneous pre-existing answers passed in and only constructs based on the schema keys", function() {
+            assert.deepEqual(createResultObject(schema, {income: '10000', otherMulti: ["1", "3"], whateverValue: "test value that won't appear"}), {
+                income: '10000',
+                zipcode: '',
+                other: undefined,
+                otherMulti: ["1", "3"]
+            })
+        })
+     })
+ })