diff --git a/coldfront/core/allocation/forms.py b/coldfront/core/allocation/forms.py index dd0d1875f..90b1ef48b 100644 --- a/coldfront/core/allocation/forms.py +++ b/coldfront/core/allocation/forms.py @@ -150,18 +150,7 @@ def clean(self): [d[:3], d[3:8], d[8:12], d[12:18], d[18:24], d[24:28], d[28:33]] ) cleaned_expensecode = insert_dashes(replace_productcode(digits_only(expense_code))) - if 'ifxbilling' in settings.INSTALLED_APPS: - try: - matched_fiineaccts = FiineAPI.listAccounts(code=cleaned_expensecode) - if not matched_fiineaccts: - self.add_error( - "expense_code", - "expense code not found in system - please check the code or get in touch with a system administrator." - ) - except Exception: - #Not authorized to use accounts_list - pass - cleaned_data['expense_code'] = cleaned_expensecode + cleaned_data['expense_code'] = cleaned_expensecode elif existing_expense_codes and existing_expense_codes != '------': cleaned_data['expense_code'] = existing_expense_codes return cleaned_data diff --git a/coldfront/core/allocation/models.py b/coldfront/core/allocation/models.py index a6b37d81d..0a7a431b3 100644 --- a/coldfront/core/allocation/models.py +++ b/coldfront/core/allocation/models.py @@ -532,20 +532,29 @@ def clean(self): expected_value_type = self.allocation_attribute_type.attribute_type.name.strip() error = None - if expected_value_type == 'Float' and not isinstance(literal_eval(self.value), (float,int)): - error = 'Value must be a float.' - elif expected_value_type == 'Int' and not isinstance(literal_eval(self.value), int): - error = 'Value must be an integer.' - elif expected_value_type == 'Yes/No' and self.value not in ['Yes', 'No']: + if expected_value_type in ['Float', 'Int']: + try: + literal_val = literal_eval(self.value) + except SyntaxError as exc: + error = 'Value must be entirely numeric. Please remove any non-numeric characters.' + raise ValidationError( + f'Invalid Value "{self.value}" for "{self.allocation_attribute_type.name}". {error}' + ) from exc + if expected_value_type == 'Float' and not isinstance(literal_val, (float,int)): + error = 'Value must be a float.' + elif expected_value_type == 'Int' and not isinstance(literal_val, int): + error = 'Value must be an integer.' + elif expected_value_type == "Yes/No" and self.value not in ["Yes", "No"]: error = 'Allowed inputs are "Yes" or "No".' - elif expected_value_type == 'Date': + elif expected_value_type == "Date": try: datetime.datetime.strptime(self.value.strip(), '%Y-%m-%d') except ValueError: error = 'Date must be in format YYYY-MM-DD' if error: raise ValidationError( - 'Invalid Value "%s" for "%s". %s' % (self.value, self.allocation_attribute_type.name, error)) + f'Invalid Value "{self.value}" for "{self.allocation_attribute_type.name}". {error}' + ) def __str__(self): return str(self.allocation_attribute_type.name) diff --git a/coldfront/core/allocation/tests/test_models.py b/coldfront/core/allocation/tests/test_models.py index 940e2cb2b..ff7cf2ded 100644 --- a/coldfront/core/allocation/tests/test_models.py +++ b/coldfront/core/allocation/tests/test_models.py @@ -1,6 +1,7 @@ """Unit tests for the allocation models""" from django.test import TestCase +from django.core.exceptions import ValidationError from coldfront.core.test_helpers.factories import setup_models, AllocationFactory @@ -8,13 +9,14 @@ "coldfront/core/test_helpers/test_data/test_fixtures/ifx.json", ] + class AllocationModelTests(TestCase): """tests for Allocation model""" fixtures = UTIL_FIXTURES @classmethod def setUpTestData(cls): - """Set up project to test model properties and methods""" + """Set up allocation to test model properties and methods""" setup_models(cls) def test_allocation_str(self): @@ -23,9 +25,9 @@ def test_allocation_str(self): self.proj_allocation.get_parent_resource.name, self.proj_allocation.project.pi ) - self.assertEqual(str(self.proj_allocation), allocation_str) + def test_allocation_usage_property(self): """Test that allocation usage property displays correctly""" self.assertEqual(self.proj_allocation.usage, 10) @@ -34,3 +36,27 @@ def test_allocation_usage_property_na(self): """Create allocation with no usage. Usage property should return None""" allocation = AllocationFactory() self.assertIsNone(allocation.usage) + +class AllocationAttributeModelTests(TestCase): + """Tests for allocationattribute models""" + fixtures = UTIL_FIXTURES + + @classmethod + def setUpTestData(cls): + """Set up allocationattribute to test model properties and methods""" + setup_models(cls) + cls.allocationattribute = cls.proj_allocation.allocationattribute_set.get( + allocation_attribute_type__name='Storage Quota (TB)' + ) + + def test_allocationattribute_clean_no_error(self): + """cleaning a numeric value for an int or float AllocationAttributeType produces no error""" + self.allocationattribute.value = "1000" + self.allocationattribute.clean() + + def test_allocationattribute_clean_nonnumeric_error(self): + """cleaning a non-numeric value for int or float AllocationAttributeTypes returns an informative error message""" + + self.allocationattribute.value = "1000TB" + with self.assertRaisesMessage(ValidationError, 'Value must be entirely numeric. Please remove any non-numeric characters.'): + self.allocationattribute.clean() diff --git a/coldfront/core/allocation/views.py b/coldfront/core/allocation/views.py index ce59ba295..7ca822eb8 100644 --- a/coldfront/core/allocation/views.py +++ b/coldfront/core/allocation/views.py @@ -68,6 +68,7 @@ if 'ifxbilling' in settings.INSTALLED_APPS: + from fiine.client import API as FiineAPI from ifxbilling.models import Account, UserProductAccount if 'django_q' in settings.INSTALLED_APPS: from django_q.tasks import Task @@ -711,7 +712,19 @@ def form_valid(self, form): 'quantity':quantity, 'nese': nese, 'used_percentage': used_percentage, + 'expense_code': expense_code, + 'unmatched_code': False, } + + if 'ifxbilling' in settings.INSTALLED_APPS: + try: + matched_fiineaccts = FiineAPI.listAccounts(code=expense_code) + if not matched_fiineaccts: + other_vars['unmatched_code'] = True + except Exception: + #Not authorized to use accounts_list + pass + send_allocation_admin_email( allocation_obj, 'New Allocation Request', diff --git a/coldfront/templates/email/new_allocation_request.txt b/coldfront/templates/email/new_allocation_request.txt index 8c09761d3..4697a00fe 100644 --- a/coldfront/templates/email/new_allocation_request.txt +++ b/coldfront/templates/email/new_allocation_request.txt @@ -8,6 +8,12 @@ Justification: {{resource}} was last recorded as {{used_percentage}}% allocated. {% endif %} +{% if unmatched_code %} +The expense code entered does not match any in the FIINE database. +Please check it for accuracy and ensure that it is added to FIINE: +{{expense_code}} +{% endif %} + {% if nese %} Here is a draft ticket to send to NESE: