diff --git a/webapp/apps/taxbrain/views.py b/webapp/apps/taxbrain/views.py index 99e41952..fb033b3a 100644 --- a/webapp/apps/taxbrain/views.py +++ b/webapp/apps/taxbrain/views.py @@ -6,6 +6,7 @@ import os import tempfile import re +from collections import defaultdict #Mock some module for imports because we can't fit them on Heroku slugs from mock import Mock @@ -151,6 +152,21 @@ def normalize(x): return ans +def append_errors_warnings(errors_warnings, append_func): + """ + Appends warning/error messages to some object, append_obj, according to + the provided function, append_func + """ + for action in ['warnings', 'errors']: + for param in errors_warnings[action]: + for year in sorted( + errors_warnings[action][param].keys(), + key=lambda x: int(x) + ): + msg = errors_warnings[action][param][year] + append_func(param, msg) + + def parse_errors_warnings(errors_warnings, map_back_to_tb): """ Parse error messages so that they can be mapped to Taxbrain param ID. This @@ -160,7 +176,7 @@ def parse_errors_warnings(errors_warnings, map_back_to_tb): returns: dictionary 'parsed' with keys: 'errors' and 'warnings' parsed['errors/warnings'] = {year: {tb_param_name: 'error message'}} """ - parsed = {'errors': {}, 'warnings': {}} + parsed = {'errors': defaultdict(dict), 'warnings': defaultdict(dict)} for action in errors_warnings: msgs = errors_warnings[action] if len(msgs) == 0: @@ -171,11 +187,9 @@ def parse_errors_warnings(errors_warnings, map_back_to_tb): msg_spl = msg.split() msg_action = msg_spl[0] year = msg_spl[1] - curr_id = msg_spl[2] + curr_id = msg_spl[2][1:] msg_parse = msg_spl[2:] - if year not in parsed[action]: - parsed[action][year] = {} - parsed[action][year][curr_id[1:]] = ' '.join([msg_action] + msg_parse + + parsed[action][curr_id][year] = ' '.join([msg_action] + msg_parse + ['for', year]) return parsed @@ -467,14 +481,10 @@ def submit_reform(request, user=None, json_reform_id=None): # only handle GUI errors for now if ((taxcalc_errors or taxcalc_warnings) and personal_inputs is not None): - for action in errors_warnings: - for year in sorted(errors_warnings[action].keys(), - key=lambda x: float(x)): - for param in errors_warnings[action][year]: - personal_inputs.add_error( - param, - errors_warnings[action][year][param] - ) + append_errors_warnings( + errors_warnings, + lambda param, msg: personal_inputs.add_error(param, msg) + ) has_parse_errors = any(['Unrecognize value' in e[0] for e in personal_inputs.errors.values()]) if no_inputs: @@ -592,22 +602,10 @@ def file_input(request): if (has_errors and (errors_warnings['warnings'] or errors_warnings['errors'])): errors.append(OUT_OF_RANGE_ERROR_MSG) - # group messages by parameter name - group_by_param = {} - for action in errors_warnings: - for year in sorted(errors_warnings[action].keys(), - key=lambda x: float(x)): - for param in errors_warnings[action][year]: - if param in group_by_param: - group_by_param[param].append( - errors_warnings[action][year][param] - ) - else: - group_by_param[param] = \ - [errors_warnings[action][year][param]] - # append to errors - for param in group_by_param: - errors += group_by_param[param] + append_errors_warnings( + errors_warnings, + lambda _, msg: errors.append(msg) + ) else: return redirect(unique_url) else: diff --git a/webapp/apps/test_assets/test_param_formatters.py b/webapp/apps/test_assets/test_param_formatters.py index 3a5088b4..8587fa8a 100644 --- a/webapp/apps/test_assets/test_param_formatters.py +++ b/webapp/apps/test_assets/test_param_formatters.py @@ -2,8 +2,11 @@ import pytest import taxcalc import numpy as np +from collections import defaultdict + +from ..taxbrain.views import (read_json_reform, parse_errors_warnings, + append_errors_warnings) -from ..taxbrain.views import read_json_reform, parse_errors_warnings from ..taxbrain.helpers import (get_default_policy_param_name, to_json_reform) from test_reform import (test_coverage_fields, test_coverage_reform, @@ -93,6 +96,53 @@ def test_parse_errors_warnings(): np.testing.assert_equal(act, exp_errors_warnings) +def test_append_ew_file_input(): + """ + Tests append_errors_warnings when add warning/error messages from the + file input interface + """ + errors_warnings = {"warnings": {"param1": {"2017": "msg2", "2016": "msg1"}}, + "errors": {"param2": {"2018": "msg3", "2019": "msg4"}} + } + errors = [] + exp = ["msg1", "msg2", "msg3", "msg4"] + + append_errors_warnings( + errors_warnings, + lambda _, msg: errors.append(msg) + ) + + assert errors == exp + +def test_append_ew_personal_inputs(): + """ + Tests append_errors_warnings when adding warning/error messages from the + GUI input interface + """ + errors_warnings = {"warnings": {"param1": {"2017": "msg2", "2016": "msg1"}}, + "errors": {"param2": {"2018": "msg3", "2019": "msg4"}} + } + # fake PersonalExemptionForm object to simulate add_error method + class FakeForm: + def __init__(self): + self.errors = defaultdict(list) + + def add_error(self, param_name, msg): + self.errors[param_name].append(msg) + + exp = {"param1": ["msg1", "msg2"], + "param2": ["msg3", "msg4"]} + + personal_inputs = FakeForm() + append_errors_warnings( + errors_warnings, + lambda param, msg: personal_inputs.add_error(param, msg) + ) + + assert (exp["param1"] == personal_inputs.errors["param1"] and + exp["param2"] == personal_inputs.errors["param2"]) + + ############################################################################### # Test read_json_reform # 3 Cases: diff --git a/webapp/apps/test_assets/test_reform.py b/webapp/apps/test_assets/test_reform.py index 31d89200..8c1a1017 100644 --- a/webapp/apps/test_assets/test_reform.py +++ b/webapp/apps/test_assets/test_reform.py @@ -372,30 +372,34 @@ exp_errors_warnings = { 'errors': { - '2017': { - 'FICA_ss_trt': 'ERROR: _FICA_ss_trt value -1.0 < min value 0 for 2017', - 'II_brk4_0': 'ERROR: _II_brk4_0 value 500.0 < min value 91900.0 for _II_brk3_0 for 2017'}, - '2018': { - 'FICA_ss_trt': 'ERROR: _FICA_ss_trt value -1.0 < min value 0 for 2018', - 'II_brk4_0': 'ERROR: _II_brk4_0 value 511.25 < min value 93967.75 for _II_brk3_0 for 2018'}, - '2019': {'II_brk4_0': 'ERROR: _II_brk4_0 value 522.7 < min value 96072.63 for _II_brk3_0 for 2019'}, - '2020': {'II_brk4_0': 'ERROR: _II_brk4_0 value 534.77 < min value 98291.91 for _II_brk3_0 for 2020'}, - '2021': {'II_brk4_0': 'ERROR: _II_brk4_0 value 547.5 < min value 100631.26 for _II_brk3_0 for 2021'}, - '2022': {'II_brk4_0': 'ERROR: _II_brk4_0 value 560.8 < min value 103076.6 for _II_brk3_0 for 2022'}, - '2023': {'II_brk4_0': 'ERROR: _II_brk4_0 value 574.09 < min value 105519.52 for _II_brk3_0 for 2023'}, - '2024': {'II_brk4_0': 'ERROR: _II_brk4_0 value 587.87 < min value 108051.99 for _II_brk3_0 for 2024'}, - '2025': {'II_brk4_0': 'ERROR: _II_brk4_0 value 601.86 < min value 110623.63 for _II_brk3_0 for 2025'}, - '2026': {'II_brk4_0': 'ERROR: _II_brk4_0 value 616.18 < min value 113256.47 for _II_brk3_0 for 2026'}, - '2027': {'II_brk4_0': 'ERROR: _II_brk4_0 value 630.97 < min value 115974.63 for _II_brk3_0 for 2027'} + 'FICA_ss_trt': { + '2017': 'ERROR: _FICA_ss_trt value -1.0 < min value 0 for 2017', + '2018': 'ERROR: _FICA_ss_trt value -1.0 < min value 0 for 2018' + }, + 'II_brk4_0': { + '2017': 'ERROR: _II_brk4_0 value 500.0 < min value 91900.0 for _II_brk3_0 for 2017', + '2018': 'ERROR: _II_brk4_0 value 511.25 < min value 93967.75 for _II_brk3_0 for 2018', + '2019': 'ERROR: _II_brk4_0 value 522.7 < min value 96072.63 for _II_brk3_0 for 2019', + '2020': 'ERROR: _II_brk4_0 value 534.77 < min value 98291.91 for _II_brk3_0 for 2020', + '2021': 'ERROR: _II_brk4_0 value 547.5 < min value 100631.26 for _II_brk3_0 for 2021', + '2022': 'ERROR: _II_brk4_0 value 560.8 < min value 103076.6 for _II_brk3_0 for 2022', + '2023': 'ERROR: _II_brk4_0 value 574.09 < min value 105519.52 for _II_brk3_0 for 2023', + '2024': 'ERROR: _II_brk4_0 value 587.87 < min value 108051.99 for _II_brk3_0 for 2024', + '2025': 'ERROR: _II_brk4_0 value 601.86 < min value 110623.63 for _II_brk3_0 for 2025', + '2026': 'ERROR: _II_brk4_0 value 616.18 < min value 113256.47 for _II_brk3_0 for 2026', + '2027': 'ERROR: _II_brk4_0 value 630.97 < min value 115974.63 for _II_brk3_0 for 2027' + } }, 'warnings': { - '2020': {'STD_3': 'WARNING: _STD_3 value 150.0 < min value 9900.32 for 2020'}, - '2021': {'STD_3': 'WARNING: _STD_3 value 153.57 < min value 10138.33 for 2021'}, - '2022': {'STD_3': 'WARNING: _STD_3 value 157.3 < min value 10387.12 for 2022'}, - '2023': {'STD_3': 'WARNING: _STD_3 value 161.03 < min value 10635.66 for 2023'}, - '2024': {'STD_3': 'WARNING: _STD_3 value 164.89 < min value 10893.32 for 2024'}, - '2025': {'STD_3': 'WARNING: _STD_3 value 168.81 < min value 11154.96 for 2025'}, - '2026': {'STD_3': 'WARNING: _STD_3 value 172.83 < min value 11422.83 for 2026'}, - '2027': {'STD_3': 'WARNING: _STD_3 value 176.98 < min value 11699.38 for 2027'} + 'STD_3': { + '2020': 'WARNING: _STD_3 value 150.0 < min value 9900.32 for 2020', + '2021': 'WARNING: _STD_3 value 153.57 < min value 10138.33 for 2021', + '2022': 'WARNING: _STD_3 value 157.3 < min value 10387.12 for 2022', + '2023': 'WARNING: _STD_3 value 161.03 < min value 10635.66 for 2023', + '2024': 'WARNING: _STD_3 value 164.89 < min value 10893.32 for 2024', + '2025': 'WARNING: _STD_3 value 168.81 < min value 11154.96 for 2025', + '2026': 'WARNING: _STD_3 value 172.83 < min value 11422.83 for 2026', + '2027': 'WARNING: _STD_3 value 176.98 < min value 11699.38 for 2027' + } } }