From b9c45ed54a73c439829da62ce0d3d31ca2c43731 Mon Sep 17 00:00:00 2001
From: AlanCoding <arominge@redhat.com>
Date: Thu, 6 Apr 2017 17:32:08 -0400
Subject: [PATCH] Fix bug where API assumed survey choices were list

Applies to both single-select and multi-select type questions.
UI sends choices in the form of text with line breaks
separating the options.
Customer complained about empty string erronously sent by
the UI being passed to playbook, which is a component of this.
---
 awx/main/models/mixins.py                     | 15 +++++---
 awx/main/tests/factories/tower.py             | 10 +++---
 .../tests/unit/models/test_survey_models.py   | 34 +++++++++++++++++++
 3 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/awx/main/models/mixins.py b/awx/main/models/mixins.py
index e818b5b648..b490197adc 100644
--- a/awx/main/models/mixins.py
+++ b/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):
diff --git a/awx/main/tests/factories/tower.py b/awx/main/tests/factories/tower.py
index 975adde43b..d1b7ae142d 100644
--- a/awx/main/tests/factories/tower.py
+++ b/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
diff --git a/awx/main/tests/unit/models/test_survey_models.py b/awx/main/tests/unit/models/test_survey_models.py
index eefc5d97ab..502ee5c5d9 100644
--- a/awx/main/tests/unit/models/test_survey_models.py
+++ b/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"