Fix bug where extra variables were reset on schedule edit

Fix survey prompt presentation inconsistencies

Remove unnecessary conditional

This conditional always returned true.  See the following warning: This condition will always return 'true' since JavaScript compares objects by reference, not value.

Fix schedule edit tests

awx/ui/src/components/LaunchPrompt/steps/useSurveyStep.js

@@ -67,27 +67,18 @@ function getInitialValues(launchConfig, surveyConfig, resource) {
   const values = {};
   if (surveyConfig?.spec) {
     surveyConfig.spec.forEach((question) => {
-      if (question.type === 'multiselect') {
+      if (resource?.extra_data && resource?.extra_data[question.variable]) {
+        values[`survey_${question.variable}`] =
+          resource.extra_data[question.variable];
+      } else if (question.type === 'multiselect') {
         values[`survey_${question.variable}`] = question.default
           ? question.default.split('\n')
           : [];
       } else {
         values[`survey_${question.variable}`] = question.default ?? '';
       }
-      if (resource?.extra_data) {
-        Object.entries(resource.extra_data).forEach(([key, value]) => {
-          if (key === question.variable) {
-            if (question.type === 'multiselect') {
-              values[`survey_${question.variable}`] = value;
-            } else {
-              values[`survey_${question.variable}`] = value;
-            }
-          }
-        });
-      }
     });
   }
-
   return values;
 }
 

awx/ui/src/components/Schedule/ScheduleEdit/ScheduleEdit.js

@@ -13,6 +13,18 @@ import ScheduleForm from '../shared/ScheduleForm';
 import buildRuleSet from '../shared/buildRuleSet';
 import { CardBody } from '../../Card';
 
+function generateExtraData(extra_vars, surveyValues, surveyConfiguration) {
+  const extraVars = parseVariableField(
+    yaml.dump(mergeExtraVars(extra_vars, surveyValues))
+  );
+  surveyConfiguration.spec.forEach((q) => {
+    if (!surveyValues[q.variable]) {
+      delete extraVars[q.variable];
+    }
+  });
+  return extraVars;
+}
+
 function ScheduleEdit({
   hasDaysToKeepField,
   schedule,
@@ -33,10 +45,12 @@ function ScheduleEdit({
     surveyConfiguration,
     originalInstanceGroups,
     originalLabels,
-    scheduleCredentials = []
+    scheduleCredentials = [],
+    isPromptTouched = false
   ) => {
     const {
       execution_environment,
+      extra_vars = null,
       instance_groups,
       inventory,
       credentials = [],
@@ -48,45 +62,54 @@ function ScheduleEdit({
       labels,
       ...submitValues
     } = values;
-    let extraVars;
+
     const surveyValues = getSurveyValues(values);
 
     if (
-      !Object.values(surveyValues).length &&
+      isPromptTouched &&
-      surveyConfiguration?.spec?.length
+      surveyConfiguration?.spec &&
+      launchConfiguration?.ask_variables_on_launch
     ) {
-      surveyConfiguration.spec.forEach((q) => {
+      submitValues.extra_data = generateExtraData(
-        surveyValues[q.variable] = q.default;
+        extra_vars,
-      });
+        surveyValues,
+        surveyConfiguration
+      );
+    } else if (
+      isPromptTouched &&
+      surveyConfiguration?.spec &&
+      !launchConfiguration?.ask_variables_on_launch
+    ) {
+      submitValues.extra_data = generateExtraData(
+        schedule.extra_data,
+        surveyValues,
+        surveyConfiguration
+      );
+    } else if (
+      isPromptTouched &&
+      launchConfiguration?.ask_variables_on_launch
+    ) {
+      submitValues.extra_data = parseVariableField(extra_vars);
     }
 
-    const initialExtraVars =
-      launchConfiguration?.ask_variables_on_launch &&
-      (values.extra_vars || '---');
-    if (surveyConfiguration?.spec) {
-      extraVars = yaml.dump(mergeExtraVars(initialExtraVars, surveyValues));
-    } else {
-      extraVars = yaml.dump(mergeExtraVars(initialExtraVars, {}));
-    }
-    submitValues.extra_data = extraVars && parseVariableField(extraVars);
-
     if (
-      Object.keys(submitValues.extra_data).length === 0 &&
+      isPromptTouched &&
-      Object.keys(schedule.extra_data).length > 0
+      launchConfiguration?.ask_inventory_on_launch &&
+      inventory
     ) {
-      submitValues.extra_data = schedule.extra_data;
-    }
-    delete values.extra_vars;
-    if (inventory) {
       submitValues.inventory = inventory.id;
     }
 
-    if (execution_environment) {
+    if (
+      isPromptTouched &&
+      launchConfiguration?.ask_execution_environment_on_launch &&
+      execution_environment
+    ) {
       submitValues.execution_environment = execution_environment.id;
     }
 
     try {
-      if (launchConfiguration?.ask_labels_on_launch) {
+      if (isPromptTouched && launchConfiguration?.ask_labels_on_launch) {
         const { labelIds, error } = createNewLabels(
           values.labels,
           resource.organization
@@ -120,9 +143,16 @@ function ScheduleEdit({
         }
       }
 
+      const cleanedRequestData = Object.keys(requestData)
+        .filter((key) => !key.startsWith('survey_'))
+        .reduce((acc, key) => {
+          acc[key] = requestData[key];
+          return acc;
+        }, {});
+
       const {
         data: { id: scheduleId },
-      } = await SchedulesAPI.update(schedule.id, requestData);
+      } = await SchedulesAPI.update(schedule.id, cleanedRequestData);
 
       const { added: addedCredentials, removed: removedCredentials } =
         getAddedAndRemoved(

awx/ui/src/components/Schedule/ScheduleEdit/ScheduleEdit.test.js

@@ -6,6 +6,7 @@ import {
   InventoriesAPI,
   CredentialsAPI,
   CredentialTypesAPI,
+  JobTemplatesAPI,
 } from 'api';
 import { mountWithContexts } from '../../../../testUtils/enzymeHelpers';
 import ScheduleEdit from './ScheduleEdit';
@@ -125,6 +126,7 @@ describe('<ScheduleEdit />', () => {
         id: 27,
       },
     });
+
     await act(async () => {
       wrapper = mountWithContexts(
         <ScheduleEdit
@@ -206,7 +208,6 @@ describe('<ScheduleEdit />', () => {
     expect(SchedulesAPI.update).toHaveBeenCalledWith(27, {
       description: 'test description',
       name: 'Run once schedule',
-      extra_data: {},
       rrule:
         'DTSTART;TZID=America/New_York:20200325T100000 RRULE:INTERVAL=1;COUNT=1;FREQ=MINUTELY',
     });
@@ -233,7 +234,6 @@ describe('<ScheduleEdit />', () => {
     expect(SchedulesAPI.update).toHaveBeenCalledWith(27, {
       description: 'test description',
       name: 'Run every 10 minutes 10 times',
-      extra_data: {},
       rrule:
         'DTSTART;TZID=America/New_York:20200325T103000 RRULE:INTERVAL=10;FREQ=MINUTELY;COUNT=10',
     });
@@ -262,7 +262,6 @@ describe('<ScheduleEdit />', () => {
     expect(SchedulesAPI.update).toHaveBeenCalledWith(27, {
       description: 'test description',
       name: 'Run every hour until date',
-      extra_data: {},
       rrule:
         'DTSTART;TZID=America/New_York:20200325T104500 RRULE:INTERVAL=1;FREQ=HOURLY;UNTIL=20200326T144500Z',
     });
@@ -288,7 +287,6 @@ describe('<ScheduleEdit />', () => {
     expect(SchedulesAPI.update).toHaveBeenCalledWith(27, {
       description: 'test description',
       name: 'Run daily',
-      extra_data: {},
       rrule:
         'DTSTART;TZID=America/New_York:20200325T104500 RRULE:INTERVAL=1;FREQ=DAILY',
     });
@@ -316,7 +314,6 @@ describe('<ScheduleEdit />', () => {
     expect(SchedulesAPI.update).toHaveBeenCalledWith(27, {
       description: 'test description',
       name: 'Run weekly on mon/wed/fri',
-      extra_data: {},
       rrule: `DTSTART;TZID=America/New_York:20200325T104500 RRULE:INTERVAL=1;FREQ=WEEKLY;BYDAY=${RRule.MO},${RRule.WE},${RRule.FR}`,
     });
   });
@@ -344,7 +341,6 @@ describe('<ScheduleEdit />', () => {
     expect(SchedulesAPI.update).toHaveBeenCalledWith(27, {
       description: 'test description',
       name: 'Run on the first day of the month',
-      extra_data: {},
       rrule:
         'DTSTART;TZID=America/New_York:20200401T104500 RRULE:INTERVAL=1;FREQ=MONTHLY;BYMONTHDAY=1',
     });
@@ -376,7 +372,6 @@ describe('<ScheduleEdit />', () => {
     expect(SchedulesAPI.update).toHaveBeenCalledWith(27, {
       description: 'test description',
       name: 'Run monthly on the last Tuesday',
-      extra_data: {},
       rrule:
         'DTSTART;TZID=America/New_York:20200331T110000 RRULE:INTERVAL=1;FREQ=MONTHLY;BYSETPOS=-1;BYDAY=TU',
     });
@@ -406,7 +401,6 @@ describe('<ScheduleEdit />', () => {
     expect(SchedulesAPI.update).toHaveBeenCalledWith(27, {
       description: 'test description',
       name: 'Yearly on the first day of March',
-      extra_data: {},
       rrule:
         'DTSTART;TZID=America/New_York:20200301T000000 RRULE:INTERVAL=1;FREQ=YEARLY;BYMONTH=3;BYMONTHDAY=1',
     });
@@ -437,7 +431,6 @@ describe('<ScheduleEdit />', () => {
     expect(SchedulesAPI.update).toHaveBeenCalledWith(27, {
       description: 'test description',
       name: 'Yearly on the second Friday in April',
-      extra_data: {},
       rrule:
         'DTSTART;TZID=America/New_York:20200410T111500 RRULE:INTERVAL=1;FREQ=YEARLY;BYSETPOS=2;BYDAY=FR;BYMONTH=4',
     });
@@ -468,7 +461,6 @@ describe('<ScheduleEdit />', () => {
     expect(SchedulesAPI.update).toHaveBeenCalledWith(27, {
       description: 'test description',
       name: 'Yearly on the first weekday in October',
-      extra_data: {},
       rrule:
         'DTSTART;TZID=America/New_York:20200410T111500 RRULE:INTERVAL=1;FREQ=YEARLY;BYSETPOS=1;BYDAY=MO,TU,WE,TH,FR;BYMONTH=10',
     });
@@ -562,7 +554,6 @@ describe('<ScheduleEdit />', () => {
     wrapper.update();
 
     expect(SchedulesAPI.update).toBeCalledWith(27, {
-      extra_data: {},
       name: 'mock schedule',
       rrule:
         'DTSTART;TZID=America/New_York:20210128T141500 RRULE:INTERVAL=1;COUNT=1;FREQ=MINUTELY',
@@ -633,15 +624,13 @@ describe('<ScheduleEdit />', () => {
       endDateTime: undefined,
       startDateTime: undefined,
       description: '',
-      extra_data: {},
       name: 'foo',
-      inventory: 702,
       rrule:
         'DTSTART;TZID=America/New_York:20200402T144500 RRULE:INTERVAL=1;COUNT=1;FREQ=MINUTELY',
     });
   });
 
-  test('should submit survey with default values properly, without opening prompt wizard', async () => {
+  test('should submit update values properly when prompt is not opened', async () => {
     let scheduleSurveyWrapper;
     await act(async () => {
       scheduleSurveyWrapper = mountWithContexts(
@@ -746,9 +735,195 @@ describe('<ScheduleEdit />', () => {
     expect(SchedulesAPI.update).toHaveBeenCalledWith(27, {
       description: 'test description',
       name: 'Run once schedule',
-      extra_data: { mc: 'first', text: 'text variable' },
       rrule:
         'DTSTART;TZID=America/New_York:20200325T100000 RRULE:INTERVAL=1;COUNT=1;FREQ=MINUTELY',
     });
   });
+  test('should submit update values properly when survey values change', async () => {
+    JobTemplatesAPI.readSurvey.mockResolvedValue({
+      data: {
+        spec: [
+          {
+            question_name: 'text',
+            question_description: '',
+            required: true,
+            type: 'text',
+            variable: 'text',
+            min: 0,
+            max: 1024,
+            default: 'text variable',
+            choices: '',
+            new_question: true,
+          },
+        ],
+      },
+    });
+
+    JobTemplatesAPI.readLaunch.mockResolvedValue({
+      data: {
+        can_start_without_user_input: false,
+        passwords_needed_to_start: [],
+        ask_scm_branch_on_launch: false,
+        ask_variables_on_launch: false,
+        ask_tags_on_launch: false,
+        ask_diff_mode_on_launch: false,
+        ask_skip_tags_on_launch: false,
+        ask_job_type_on_launch: false,
+        ask_limit_on_launch: false,
+        ask_verbosity_on_launch: false,
+        ask_inventory_on_launch: true,
+        ask_credential_on_launch: true,
+        survey_enabled: true,
+        variables_needed_to_start: [],
+        credential_needed_to_start: true,
+        inventory_needed_to_start: true,
+        job_template_data: {
+          name: 'Demo Job Template',
+          id: 7,
+          description: '',
+        },
+        defaults: {
+          extra_vars: '---',
+          diff_mode: false,
+          limit: '',
+          job_tags: '',
+          skip_tags: '',
+          job_type: 'run',
+          verbosity: 0,
+          inventory: {
+            name: null,
+            id: null,
+          },
+          scm_branch: '',
+          credentials: [],
+        },
+      },
+    });
+
+    let scheduleSurveyWrapper;
+    await act(async () => {
+      scheduleSurveyWrapper = mountWithContexts(
+        <ScheduleEdit
+          schedule={mockSchedule}
+          resource={{
+            id: 700,
+            type: 'job_template',
+            iventory: 1,
+            summary_fields: {
+              credentials: [
+                { name: 'job template credential', id: 75, kind: 'ssh' },
+              ],
+            },
+            name: 'Foo Job Template',
+            description: '',
+          }}
+          resourceDefaultCredentials={[]}
+          launchConfig={{
+            can_start_without_user_input: false,
+            passwords_needed_to_start: [],
+            ask_scm_branch_on_launch: false,
+            ask_variables_on_launch: false,
+            ask_tags_on_launch: false,
+            ask_diff_mode_on_launch: false,
+            ask_skip_tags_on_launch: false,
+            ask_job_type_on_launch: false,
+            ask_limit_on_launch: false,
+            ask_verbosity_on_launch: false,
+            ask_inventory_on_launch: true,
+            ask_credential_on_launch: true,
+            survey_enabled: true,
+            variables_needed_to_start: [],
+            credential_needed_to_start: true,
+            inventory_needed_to_start: true,
+            job_template_data: {
+              name: 'Demo Job Template',
+              id: 7,
+              description: '',
+            },
+            defaults: {
+              extra_vars: '---',
+              diff_mode: false,
+              limit: '',
+              job_tags: '',
+              skip_tags: '',
+              job_type: 'run',
+              verbosity: 0,
+              inventory: {
+                name: null,
+                id: null,
+              },
+              scm_branch: '',
+              credentials: [],
+            },
+          }}
+          surveyConfig={{
+            spec: [
+              {
+                question_name: 'text',
+                question_description: '',
+                required: true,
+                type: 'text',
+                variable: 'text',
+                min: 0,
+                max: 1024,
+                default: 'text variable',
+                choices: '',
+                new_question: true,
+              },
+            ],
+          }}
+        />
+      );
+    });
+    scheduleSurveyWrapper.update();
+
+    await act(async () =>
+      scheduleSurveyWrapper
+        .find('Button[aria-label="Prompt"]')
+        .prop('onClick')()
+    );
+    scheduleSurveyWrapper.update();
+    expect(scheduleSurveyWrapper.find('WizardNavItem').length).toBe(4);
+    await act(async () =>
+      scheduleSurveyWrapper.find('WizardFooterInternal').prop('onNext')()
+    );
+    scheduleSurveyWrapper.update();
+    await act(async () =>
+      scheduleSurveyWrapper.find('WizardFooterInternal').prop('onNext')()
+    );
+    scheduleSurveyWrapper.update();
+    await act(async () =>
+      scheduleSurveyWrapper
+        .find('input#survey-question-text')
+        .simulate('change', {
+          target: { value: 'foo', name: 'survey_text' },
+        })
+    );
+    scheduleSurveyWrapper.update();
+    await act(async () =>
+      scheduleSurveyWrapper.find('WizardFooterInternal').prop('onNext')()
+    );
+    scheduleSurveyWrapper.update();
+    await act(async () =>
+      scheduleSurveyWrapper.find('WizardFooterInternal').prop('onNext')()
+    );
+    scheduleSurveyWrapper.update();
+
+    expect(scheduleSurveyWrapper.find('Wizard').length).toBe(0);
+
+    await act(async () =>
+      scheduleSurveyWrapper.find('Button[aria-label="Save"]').prop('onClick')()
+    );
+
+    expect(SchedulesAPI.update).toHaveBeenCalledWith(27, {
+      description: '',
+      name: 'mock schedule',
+      inventory: 702,
+      extra_data: {
+        text: 'foo',
+      },
+      rrule:
+        'DTSTART;TZID=America/New_York:20200402T144500 RRULE:INTERVAL=1;COUNT=1;FREQ=MINUTELY',
+    });
+  });
 });

awx/ui/src/components/Schedule/shared/ScheduleForm.js

@@ -40,6 +40,7 @@ function ScheduleForm({
   resourceDefaultCredentials,
 }) {
   const [isWizardOpen, setIsWizardOpen] = useState(false);
+  const [isPromptTouched, setIsPromptTouched] = useState(false);
   const [isSaveDisabled, setIsSaveDisabled] = useState(false);
   const originalLabels = useRef([]);
   const originalInstanceGroups = useRef([]);
@@ -492,7 +493,8 @@ function ScheduleForm({
               surveyConfig,
               originalInstanceGroups.current,
               originalLabels.current,
-              credentials
+              credentials,
+              isPromptTouched
             );
           }}
           validate={validate}
@@ -518,6 +520,7 @@ function ScheduleForm({
                     onSave={() => {
                       setIsWizardOpen(false);
                       setIsSaveDisabled(false);
+                      setIsPromptTouched(true);
                     }}
                     resourceDefaultCredentials={resourceDefaultCredentials}
                     labels={originalLabels.current}

awx/ui/src/util/prompt/getSurveyValues.js

@@ -1,7 +1,7 @@
 export default function getSurveyValues(values) {
   const surveyValues = {};
   Object.keys(values).forEach((key) => {
-    if (key.startsWith('survey_') && values[key] !== []) {
+    if (key.startsWith('survey_')) {
       if (Array.isArray(values[key]) && values[key].length === 0) {
         return;
       }

awx/ui/src/util/prompt/mergeExtraVars.js

@@ -1,7 +1,12 @@
 import yaml from 'js-yaml';
 
 export default function mergeExtraVars(extraVars = '', survey = {}) {
-  const vars = yaml.load(extraVars) || {};
+  let vars = {};
+  if (typeof extraVars === 'string') {
+    vars = yaml.load(extraVars);
+  } else if (typeof extraVars === 'object') {
+    vars = extraVars;
+  }
   return {
     ...vars,
     ...survey,