Fix bugs for determining step completeness (don't check conditional requirement for question if its target question is hidden)

edivorce/apps/core/utils/step_completeness.py

@@ -2,6 +2,7 @@ from django.urls import reverse
 
 from edivorce.apps.core.models import Question
 from edivorce.apps.core.utils.question_step_mapping import page_step_mapping, pre_qual_step_question_mapping
+from edivorce.apps.core.utils.conditional_logic import get_cleaned_response_value
 
 
 def evaluate_numeric_condition(target, reveal_response):
@@ -31,18 +32,18 @@ def evaluate_numeric_condition(target, reveal_response):
     return None
 
 
-def get_step_completeness(responses_by_step):
+def get_step_completeness(questions_by_step):
     """
     Accepts a dictionary of {step: {question_id: {question__name, question_id, value, error}}} <-- from get_step_responses
     Returns {step: status}, {step: [missing_question_key]}
     """
     status_dict = {}
     missing_response_dict = {}
-    for step, responses_list in responses_by_step.items():
-        if len(responses_list) == 0:
+    for step, question_list in questions_by_step.items():
+        if not_started(question_list):
             status_dict[step] = "Not started"
         else:
-            complete, missing_responses = is_complete(responses_list)
+            complete, missing_responses = is_complete(question_list)
             if complete:
                 status_dict[step] = "Complete"
             else:
@@ -51,9 +52,16 @@ def get_step_completeness(responses_by_step):
     return status_dict, missing_response_dict
 
 
-def is_complete(response_list):
+def not_started(question_list):
+    for question_dict in question_list:
+        if get_cleaned_response_value(question_dict['value']):
+            return False
+    return True
+
+
+def is_complete(question_list):
     missing_responses = []
-    for question_dict in response_list:
+    for question_dict in question_list:
         if question_dict['error']:
             missing_responses.append(question_dict)
     return len(missing_responses) == 0, missing_responses
@@ -86,5 +94,6 @@ def get_error_dict(step, missing_questions):
     responses_dict = {}
     question_step = page_step_mapping[step]
     for question_dict in missing_questions.get(question_step, []):
-        responses_dict[question_dict['question_id'] + '_error'] = True
+        field_error_key = question_dict['question_id'] + '_error'
+        responses_dict[field_error_key] = True
     return responses_dict

edivorce/apps/core/utils/user_response.py

@@ -1,10 +1,16 @@
 from edivorce.apps.core.models import UserResponse, Question
 from edivorce.apps.core.utils import conditional_logic
+from edivorce.apps.core.utils.conditional_logic import get_cleaned_response_value
 from edivorce.apps.core.utils.question_step_mapping import page_step_mapping, question_step_mapping
 from edivorce.apps.core.utils.step_completeness import evaluate_numeric_condition
 from collections import OrderedDict
 
 
+REQUIRED = 0
+HIDDEN = 1
+OPTIONAL = 2
+
+
 def get_data_for_user(bceid_user):
     """
     Return a dictionary of {question_key: user_response_value}
@@ -54,13 +60,6 @@ def _get_questions_dict_set_for_step(step):
     return questions_dict
 
 
-def _cleaned_response_value(response):
-    ignore_values = [None, '', '[]', '[["",""]]', '[["also known as",""]]']
-    if response not in ignore_values:
-        return response
-    return None
-
-
 def _condition_met(target_response, reveal_response):
     # check whether using a numeric condition
     numeric_condition_met = evaluate_numeric_condition(target_response, reveal_response)
@@ -83,35 +82,24 @@ def _get_question_details(question, questions_dict, responses_by_key):
       error: True if the question has an error (e.g. required but not answered)
       show: False if the response shouldn't be displayed (e.g. don't show 'Also known as' name, but 'Does your spouse go by any other names' is NO)
     """
-    question_dict = questions_dict[question]
     required = False
     show = True
-    if question_dict["question__required"] == 'Required':
+    question_required = _is_question_required(question, questions_dict, responses_by_key)
+    if question_required == REQUIRED:
         required = True
-    elif question_dict["question__required"] == 'Conditional':
-        target = question_dict["question__conditional_target"]
-        if target.startswith('determine_'):
-            # Look for the right function to evaluate conditional logic
-            derived_condition = getattr(conditional_logic, target)
-            if not derived_condition:
-                raise NotImplemented(target)
-            result = derived_condition(responses_by_key)
-            if result and _condition_met(result, question_dict["question__reveal_response"]):
-                required = True
-            else:
-                show = False
-        elif question in questions_dict:
-            target_response = responses_by_key.get(target)
-            if target_response and _condition_met(target_response, question_dict["question__reveal_response"]):
-                required = True
-            else:
-                show = False
+        show = True
+    elif question_required == HIDDEN:
+        required = False
+        show = False
+    elif question_required == OPTIONAL:
+        required = False
+        show = True
 
     if show:
         value = None
         response = responses_by_key.get(question)
         if response:
-            value = _cleaned_response_value(response)
+            value = get_cleaned_response_value(response)
         error = required and not value
     else:
         value = None
@@ -125,6 +113,43 @@ def _get_question_details(question, questions_dict, responses_by_key):
     return details
 
 
+def _is_question_required(question, questions_dict, responses_by_key):
+    """
+    returns REQUIRED, HIDDEN, or OPTIONAL
+    raises KeyError if the question is conditional and improperly configured (for development testing purposes)
+    """
+    question_dict = questions_dict[question]
+    if question_dict['question__required'] == 'Required':
+        return REQUIRED
+    elif question_dict['question__required'] == 'Conditional':
+        target = question_dict.get('question__conditional_target')
+        reveal_response = question_dict.get('question__reveal_response')
+        if not target or not reveal_response:
+            raise KeyError(f"Improperly configured question '{question}'. Needs target and reveal response")
+
+        if target.startswith('determine_'):
+            # Look for the right function to evaluate conditional logic
+            derived_condition = getattr(conditional_logic, target)
+            if not derived_condition:
+                raise NotImplemented(target)
+            result = derived_condition(responses_by_key)
+            if result and _condition_met(result, reveal_response):
+                return REQUIRED
+            else:
+                return HIDDEN
+        elif target in questions_dict:
+            target_question_requirement = _is_question_required(target, questions_dict, responses_by_key)
+            if target_question_requirement == REQUIRED:
+                target_response = responses_by_key.get(target)
+                if target_response and _condition_met(target_response, reveal_response):
+                    return REQUIRED
+            return HIDDEN
+        else:
+            raise KeyError(f"Invalid conditional target '{target}' for question '{question}'")
+    else:
+        return OPTIONAL
+
+
 def get_responses_from_session(request):
     return OrderedDict(sorted(request.session.items()))