Merge pull request #5973 from AlanCoding/survey_choices_string

When survey choices are strings, don't just check "X in choices"

awx/main/models/mixins.py

@@ -1,5 +1,6 @@
 # Python
 import json
+from copy import copy
 
 # Django
 from django.db import models
@@ -196,16 +197,22 @@ class SurveyJobTemplateMixin(models.Model):
                 if type(data[survey_element['variable']]) != list:
                     errors.append("'%s' value is expected to be a list." % survey_element['variable'])
                 else:
+                    choice_list = copy(survey_element['choices'])
+                    if isinstance(choice_list, basestring):
+                        choice_list = choice_list.split('\n')
                     for val in data[survey_element['variable']]:
-                        if val not in survey_element['choices']:
+                        if val not in choice_list:
                             errors.append("Value %s for '%s' expected to be one of %s." % (val, survey_element['variable'],
-                                                                                           survey_element['choices']))
+                                                                                           choice_list))
         elif survey_element['type'] == 'multiplechoice':
+            choice_list = copy(survey_element['choices'])
+            if isinstance(choice_list, basestring):
+                choice_list = choice_list.split('\n')
             if survey_element['variable'] in data:
-                if data[survey_element['variable']] not in survey_element['choices']:
+                if data[survey_element['variable']] not in choice_list:
                     errors.append("Value %s for '%s' expected to be one of %s." % (data[survey_element['variable']],
                                                                                    survey_element['variable'],
-                                                                                   survey_element['choices']))
+                                                                                   choice_list))
         return errors
 
     def survey_variable_validation(self, data):

awx/main/tests/factories/tower.py

@@ -135,13 +135,15 @@ def create_survey_spec(variables=None, default_type='integer', required=True):
     argument specifying variable name(s)
     '''
     if isinstance(variables, list):
-        name = "%s survey" % variables[0]
-        description = "A survey that starts with %s." % variables[0]
         vars_list = variables
     else:
-        name = "%s survey" % variables
-        description = "A survey about %s." % variables
         vars_list = [variables]
+    if isinstance(variables[0], basestring):
+        slogan = variables[0]
+    else:
+        slogan = variables[0].get('question_name', 'something')
+    name = "%s survey" % slogan
+    description = "A survey that asks about %s." % slogan
 
     spec = []
     index = 0

awx/main/tests/unit/models/test_survey_models.py

@@ -91,6 +91,40 @@ def test_update_kwargs_survey_invalid_default(survey_spec_factory):
     assert json.loads(defaulted_extra_vars['extra_vars'])['var2'] == 2
 
 
+@pytest.mark.parametrize("question_type,default,expect_use,expect_value", [
+    ("multiplechoice", "",       False, 'N/A'),  # historical bug
+    ("multiplechoice", "zeb",    False, 'N/A'),  # zeb not in choices
+    ("multiplechoice", "coffee", True,  'coffee'),
+    ("multiselect",    None,     False, 'N/A'),  # NOTE: Behavior is arguable, value of [] may be prefered
+    ("multiselect",    "",       False, 'N/A'),
+    ("multiselect",    ["zeb"],  False, 'N/A'),
+    ("multiselect",    ["milk"], True,  ["milk"]),
+    ("multiselect",    ["orange\nmilk"], False,  'N/A'),  # historical bug
+])
+def test_optional_survey_question_defaults(
+        survey_spec_factory, question_type, default, expect_use, expect_value):
+    spec = survey_spec_factory([
+        {
+            "required": False,
+            "default": default,
+            "choices": "orange\nmilk\nchocolate\ncoffee",
+            "variable": "c",
+            "type": question_type
+        },
+    ])
+    jt = JobTemplate(name="test-jt", survey_spec=spec, survey_enabled=True)
+    defaulted_extra_vars = jt._update_unified_job_kwargs()
+    element = spec['spec'][0]
+    if expect_use:
+        assert jt._survey_element_validation(element, {element['variable']: element['default']}) == []
+    else:
+        assert jt._survey_element_validation(element, {element['variable']: element['default']})
+    if expect_use:
+        assert json.loads(defaulted_extra_vars['extra_vars'])['c'] == expect_value
+    else:
+        assert 'c' not in defaulted_extra_vars['extra_vars']
+
+
 class TestWorkflowSurveys:
     def test_update_kwargs_survey_defaults(self, survey_spec_factory):
         "Assure that the survey default over-rides a JT variable"