From 3b158c27285266a387c6d9776f68c0fa3a7f8f4e Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Wed, 13 Sep 2023 13:16:20 -0700 Subject: [PATCH 1/3] update expense code validation rules --- coldfront/core/allocation/forms.py | 23 +++++---- coldfront/core/allocation/tests/test_forms.py | 48 +++++++++++++++---- 2 files changed, 51 insertions(+), 20 deletions(-) diff --git a/coldfront/core/allocation/forms.py b/coldfront/core/allocation/forms.py index d835acb2d..5bd6cb7bd 100644 --- a/coldfront/core/allocation/forms.py +++ b/coldfront/core/allocation/forms.py @@ -36,12 +36,12 @@ class ExpenseCodeField(forms.CharField): def clean(self, value): # Remove all dashes from the input string to count the number of digits value = super().clean(value) - digits_only = re.sub(r'[^0-9xX]', '', value) - insert_dashes = lambda d: '-'.join( - [d[:3], d[3:8], d[8:12], d[12:18], d[18:24], d[24:28], d[28:33]] - ) - formatted_value = insert_dashes(digits_only) - return formatted_value + # digits_only = re.sub(r'[^0-9xX]', '', value) + # insert_dashes = lambda d: '-'.join( + # [d[:3], d[3:8], d[8:12], d[12:18], d[18:24], d[24:28], d[28:33]] + # ) + # formatted_value = insert_dashes(digits_only) + return value class AllocationForm(forms.Form): @@ -122,20 +122,23 @@ def clean(self): seas_val = cleaned_data.get("seas_code") trues = sum(x for x in [(value not in ['', '------']), hsph_val, seas_val]) + digits_only = lambda v: re.sub(r'[^0-9xX]', '', v) if trues != 1: self.add_error("expense_code", "you must do exactly one of the following: manually enter an expense code, check the box to use SEAS' expense code, or check the box to use HSPH's expense code") elif value and value != '------': - digits_only = re.sub(r'[^0-9xX]', '', value) - if not re.fullmatch(r'^([0-9xX]+-?)*[0-9xX-]+$', value): + if re.search(r'[^0-9xX\-\.]', value): self.add_error("expense_code", "Input must consist only of digits (or x'es) and dashes.") - elif len(digits_only) != 33: + elif len(digits_only(value)) != 33: self.add_error("expense_code", "Input must contain exactly 33 digits.") + elif 'x' in digits_only(value)[:8]+digits_only(value)[12:]: + self.add_error("expense_code", "xes are only allowed in place of the product code (the third grouping of characters in the code)") else: + replace_productcode = lambda s: s[:8] + '8250' + s[12:] insert_dashes = lambda d: '-'.join( [d[:3], d[3:8], d[8:12], d[12:18], d[18:24], d[24:28], d[28:33]] ) - cleaned_data['expense_code'] = insert_dashes(digits_only) + cleaned_data['expense_code'] = insert_dashes(replace_productcode(digits_only(value))) elif hsph_val: cleaned_data['expense_code'] = HSPH_CODE elif seas_val: diff --git a/coldfront/core/allocation/tests/test_forms.py b/coldfront/core/allocation/tests/test_forms.py index 4b0300218..7cbc8881d 100644 --- a/coldfront/core/allocation/tests/test_forms.py +++ b/coldfront/core/allocation/tests/test_forms.py @@ -48,7 +48,7 @@ def setUp(self): 'tier': Resource.objects.filter(resource_type=tier_restype).first() } - def test_allocationform_offerlettercode_invalid(self): + def test_allocationform_expense_code_invalid1(self): """ensure correct error messages for incorrect expense_code value """ self.post_data['expense_code'] = '123456789' @@ -57,7 +57,25 @@ def test_allocationform_offerlettercode_invalid(self): form.errors['expense_code'], ['Input must contain exactly 33 digits.'] ) - def test_allocationform_offerlettercode_valid(self): + def test_allocationform_expense_code_invalid2(self): + """ensure correct error messages for incorrect expense_code value + """ + self.post_data['expense_code'] = '123-456AB-CDE789-22222-22222-22222-22222' + form = AllocationForm(data=self.post_data, request_user=self.pi_user, project_pk=self.project.pk) + self.assertEqual( + form.errors['expense_code'], ["Input must consist only of digits (or x'es) and dashes."] + ) + + def test_allocationform_expense_code_invalid3(self): + """ensure correct error messages for incorrect expense_code value + """ + self.post_data['expense_code'] = '1Xx-' * 11 + form = AllocationForm(data=self.post_data, request_user=self.pi_user, project_pk=self.project.pk) + self.assertEqual( + form.errors['expense_code'], ["xes are only allowed in place of the product code (the third grouping of characters in the code)"] + ) + + def test_allocationform_expense_code_valid(self): """Test POST to the AllocationCreateView - ensure 33-digit codes go through - ensure correctly entered codes get properly formatted @@ -65,31 +83,41 @@ def test_allocationform_offerlettercode_valid(self): # correct # of digits with no dashes cleaned_form = self.return_cleaned_form(AllocationForm) self.assertEqual( - cleaned_form['expense_code'], '123-12312-3123-123123-123123-1231-23123' + cleaned_form['expense_code'], '123-12312-8250-123123-123123-1231-23123' ) - def test_allocationform_offerlettercode_valid2(self): - # check that offer code was correctly formatted + def test_allocationform_expense_code_valid2(self): + # check that expense_code was correctly formatted # correct # of digits with many dashes self.post_data['expense_code'] = '123-' * 11 cleaned_form = self.return_cleaned_form(AllocationForm) self.assertEqual( - cleaned_form['expense_code'], '123-12312-3123-123123-123123-1231-23123' + cleaned_form['expense_code'], '123-12312-8250-123123-123123-1231-23123' ) - def test_allocationform_offerlettercode_valid3(self): + def test_allocationform_expense_code_valid3(self): """Test POST to the AllocationCreateView - ensure xes count as digits """ # correct # of digits with no dashes - self.post_data['expense_code'] = '1Xx-' * 11 + self.post_data['expense_code'] = '123-12312-xxxx-123123-123123-1231-23123' cleaned_form = self.return_cleaned_form(AllocationForm) self.assertEqual( - cleaned_form['expense_code'], '1Xx-1Xx1X-x1Xx-1Xx1Xx-1Xx1Xx-1Xx1-Xx1Xx' + cleaned_form['expense_code'], '123-12312-8250-123123-123123-1231-23123' ) - def test_allocationform_offerlettercode_multiplefield_invalid(self): + def test_allocationform_expense_code_valid4(self): + """Test POST to the AllocationCreateView + - ensure xes count as digits + """ + # correct # of digits with no dashes + self.post_data['expense_code'] = '123.12312.xxxx.123123.123123.1231.23123' + cleaned_form = self.return_cleaned_form(AllocationForm) + self.assertEqual( + cleaned_form['expense_code'], '123-12312-8250-123123-123123-1231-23123' + ) + def test_allocationform_expense_code_multiplefield_invalid(self): """Test POST to AllocationCreateView in circumstance where hsph and seas values are also checked""" self.post_data['expense_code'] = '123-' * 11 self.post_data['hsph_code'] = True From cc5f152f0cdd74e3f213188d9b41fa3180929a81 Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Wed, 13 Sep 2023 14:00:33 -0700 Subject: [PATCH 2/3] take expense codes straight from UserProductAccount table --- .../templates/allocation/allocation_detail.html | 6 +++++- coldfront/core/allocation/tests/test_forms.py | 1 + coldfront/core/allocation/views.py | 11 +++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/coldfront/core/allocation/templates/allocation/allocation_detail.html b/coldfront/core/allocation/templates/allocation/allocation_detail.html index f939e8f33..3aa3b7f94 100644 --- a/coldfront/core/allocation/templates/allocation/allocation_detail.html +++ b/coldfront/core/allocation/templates/allocation/allocation_detail.html @@ -117,7 +117,11 @@

Allocation Information

Expense Code: - {{ allocation.expense_code }} + + {% for code in expense_codes %} + {{ code.account.name }} ({{ code.account.code }}): {{ code.percent }}%
+ {% endfor %} + Total Users in Your Bill: diff --git a/coldfront/core/allocation/tests/test_forms.py b/coldfront/core/allocation/tests/test_forms.py index 7cbc8881d..06092ae1f 100644 --- a/coldfront/core/allocation/tests/test_forms.py +++ b/coldfront/core/allocation/tests/test_forms.py @@ -117,6 +117,7 @@ def test_allocationform_expense_code_valid4(self): self.assertEqual( cleaned_form['expense_code'], '123-12312-8250-123123-123123-1231-23123' ) + def test_allocationform_expense_code_multiplefield_invalid(self): """Test POST to AllocationCreateView in circumstance where hsph and seas values are also checked""" self.post_data['expense_code'] = '123-' * 11 diff --git a/coldfront/core/allocation/views.py b/coldfront/core/allocation/views.py index d06ffa9a8..ed8de7cc2 100644 --- a/coldfront/core/allocation/views.py +++ b/coldfront/core/allocation/views.py @@ -66,6 +66,9 @@ from coldfront.core.utils.common import get_domain_url, import_from_settings from coldfront.core.utils.mail import send_allocation_admin_email, send_allocation_customer_email + +if 'ifxbilling' in settings.INSTALLED_APPS: + from ifxbilling.models import Account, UserProductAccount if 'django_q' in settings.INSTALLED_APPS: from django_q.tasks import Task @@ -215,6 +218,14 @@ def get_context_data(self, **kwargs): user_sync_dt = None context['user_sync_dt'] = user_sync_dt + if 'ifxbilling' in settings.INSTALLED_APPS: + expense_codes = UserProductAccount.objects.filter( + user=allocation_obj.project.pi, + is_valid=1, + product__product_name=allocation_obj.get_parent_resource.name + ) + context['expense_codes'] = expense_codes + context['allocation_quota_bytes'] = quota_bytes context['allocation_usage_bytes'] = usage_bytes quota_tb = 0 if not quota_bytes else quota_bytes / 1099511627776 From 4f565c464bcb7728a37068c69766f064aefcd56b Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Thu, 14 Sep 2023 11:24:13 -0700 Subject: [PATCH 3/3] add dropdown menu of account expense codes for the group; add verification of account expense codes using FiineAPI --- coldfront/core/allocation/forms.py | 116 +++++++++++------- coldfront/core/allocation/tests/test_forms.py | 11 +- coldfront/core/allocation/tests/test_views.py | 8 +- 3 files changed, 81 insertions(+), 54 deletions(-) diff --git a/coldfront/core/allocation/forms.py b/coldfront/core/allocation/forms.py index 5bd6cb7bd..bb75fcf35 100644 --- a/coldfront/core/allocation/forms.py +++ b/coldfront/core/allocation/forms.py @@ -1,8 +1,10 @@ import re from django import forms +from django.conf import settings from django.db.models.functions import Lower from django.shortcuts import get_object_or_404 +from django.core.exceptions import ValidationError from coldfront.core.allocation.models import ( AllocationAccount, @@ -15,6 +17,10 @@ from coldfront.core.resource.models import Resource, ResourceType from coldfront.core.utils.common import import_from_settings +if 'ifxbilling' in settings.INSTALLED_APPS: + from fiine.client import API as FiineAPI + from ifxbilling.models import Account, UserProductAccount + ALLOCATION_ACCOUNT_ENABLED = import_from_settings( 'ALLOCATION_ACCOUNT_ENABLED', False) ALLOCATION_CHANGE_REQUEST_EXTENSION_DAYS = import_from_settings( @@ -22,21 +28,28 @@ HSPH_CODE = import_from_settings('HSPH_CODE', '000-000-000-000-000-000-000-000-000-000-000') SEAS_CODE = import_from_settings('SEAS_CODE', '111-111-111-111-111-111-111-111-111-111-111') + class ExpenseCodeField(forms.CharField): """custom field for expense_code""" - # def validate(self, value): - # if value: - # digits_only = re.sub(r'\D', '', value) - # if not re.fullmatch(r'^(\d+-?)*[\d-]+$', value): - # raise ValidationError("Input must consist only of digits and dashes.") - # if len(digits_only) != 33: - # raise ValidationError("Input must contain exactly 33 digits.") + def validate(self, value): + digits_only = lambda v: re.sub(r'[^0-9xX]', '', v) + if value and value != '------': + if re.search(r'[^0-9xX\-\.]', value): + raise ValidationError( + "Input must consist only of digits (or x'es) and dashes." + ) + if len(digits_only(value)) != 33: + raise ValidationError("Input must contain exactly 33 digits.") + if 'x' in digits_only(value)[:8]+digits_only(value)[12:]: + raise ValidationError( + "xes are only allowed in place of the product code (the third grouping of characters in the code)" + ) def clean(self, value): # Remove all dashes from the input string to count the number of digits value = super().clean(value) - # digits_only = re.sub(r'[^0-9xX]', '', value) + # digits_only = lambda v: re.sub(r'[^0-9xX]', '', v) # insert_dashes = lambda d: '-'.join( # [d[:3], d[3:8], d[8:12], d[12:18], d[18:24], d[24:28], d[28:33]] # ) @@ -45,24 +58,24 @@ def clean(self, value): class AllocationForm(forms.Form): + QS_CHOICES = [ + (HSPH_CODE, f'{HSPH_CODE} (PI is part of HSPH and storage should be billed to their code)'), + (SEAS_CODE, f'{SEAS_CODE} (PI is part of SEAS and storage should be billed to their code)') + ] DEFAULT_DESCRIPTION = """ We do not have information about your research. Please provide a detailed description of your work and update your field of science. Thank you! """ # resource = forms.ModelChoiceField(queryset=None, empty_label=None) quantity = forms.IntegerField(required=True, initial=1) - expense_code = ExpenseCodeField( - label="Lab's 33 digit expense code", required=False - ) - - hsph_code = forms.BooleanField( - label='The PI is part of HSPH and storage should be billed to their code', - required=False + existing_expense_codes = forms.ChoiceField( + label='Either select an existing expense code...', + choices=QS_CHOICES, + required=False, ) - seas_code = forms.BooleanField( - label='The PI is part of SEAS and storage should be billed to their code', - required=False + expense_code = ExpenseCodeField( + label="...or add a new 33 digit expense code manually here.", required=False ) tier = forms.ModelChoiceField( @@ -92,7 +105,6 @@ class AllocationForm(forms.Form): widget=forms.Textarea, help_text = '
Justification for requesting this allocation. Please provide details here about the usecase or datacenter choices (what data needs to be accessed, expectation of frequent transfer to or from Campus, need for Samba connectivity, etc.)' ) - #users = forms.MultipleChoiceField( # widget=forms.CheckboxSelectMultiple, required=False) @@ -103,9 +115,16 @@ def __init__(self, request_user, project_pk, *args, **kwargs): self.fields['tier'].queryset = get_user_resources(request_user).filter( resource_type__name='Storage Tier' ).order_by(Lower("name")) + existing_expense_codes = [(None, '------')] + [ + (a.code, f'{a.code} ({a.name})') for a in Account.objects.filter( + userproductaccount__is_valid=1, + userproductaccount__user=project_obj.pi + ).distinct() + ] + self.QS_CHOICES + self.fields['existing_expense_codes'].choices = existing_expense_codes user_query_set = project_obj.projectuser_set.select_related('user').filter( - status__name__in=['Active', ]).order_by("user__username") - user_query_set = user_query_set.exclude(user=project_obj.pi) + status__name__in=['Active', ] + ).order_by("user__username").exclude(user=project_obj.pi) # if user_query_set: # self.fields['users'].choices = ((user.user.username, "%s %s (%s)" % ( # user.user.first_name, user.user.last_name, user.user.username)) for user in user_query_set) @@ -113,36 +132,42 @@ def __init__(self, request_user, project_pk, *args, **kwargs): # else: # self.fields['users'].widget = forms.HiddenInput() - def clean(self): cleaned_data = super().clean() # Remove all dashes from the input string to count the number of digits - value = cleaned_data.get("expense_code") - hsph_val = cleaned_data.get("hsph_code") - seas_val = cleaned_data.get("seas_code") - trues = sum(x for x in [(value not in ['', '------']), hsph_val, seas_val]) - + expense_code = cleaned_data.get("expense_code") + existing_expense_codes = cleaned_data.get("existing_expense_codes") + trues = sum(x for x in [ + (expense_code not in ['', '------']), + (existing_expense_codes not in ['', '------']), + ]) digits_only = lambda v: re.sub(r'[^0-9xX]', '', v) if trues != 1: - self.add_error("expense_code", "you must do exactly one of the following: manually enter an expense code, check the box to use SEAS' expense code, or check the box to use HSPH's expense code") + self.add_error( + "existing_expense_codes", + "You must either select an existing expense code or manually enter a new one." + ) - elif value and value != '------': - if re.search(r'[^0-9xX\-\.]', value): - self.add_error("expense_code", "Input must consist only of digits (or x'es) and dashes.") - elif len(digits_only(value)) != 33: - self.add_error("expense_code", "Input must contain exactly 33 digits.") - elif 'x' in digits_only(value)[:8]+digits_only(value)[12:]: - self.add_error("expense_code", "xes are only allowed in place of the product code (the third grouping of characters in the code)") - else: - replace_productcode = lambda s: s[:8] + '8250' + s[12:] - insert_dashes = lambda d: '-'.join( - [d[:3], d[3:8], d[8:12], d[12:18], d[18:24], d[24:28], d[28:33]] - ) - cleaned_data['expense_code'] = insert_dashes(replace_productcode(digits_only(value))) - elif hsph_val: - cleaned_data['expense_code'] = HSPH_CODE - elif seas_val: - cleaned_data['expense_code'] = SEAS_CODE + elif expense_code and expense_code != '------': + replace_productcode = lambda s: s[:8] + '8250' + s[12:] + insert_dashes = lambda d: '-'.join( + [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 + elif existing_expense_codes and existing_expense_codes != '------': + cleaned_data['expense_code'] = existing_expense_codes return cleaned_data @@ -192,7 +217,6 @@ def __init__(self, *args, **kwargs): pk=allo_resource.pk ) - def clean(self): cleaned_data = super().clean() start_date = cleaned_data.get("start_date") diff --git a/coldfront/core/allocation/tests/test_forms.py b/coldfront/core/allocation/tests/test_forms.py index 06092ae1f..0fc92002b 100644 --- a/coldfront/core/allocation/tests/test_forms.py +++ b/coldfront/core/allocation/tests/test_forms.py @@ -3,7 +3,7 @@ from django.test import TestCase from coldfront.core.resource.models import Resource -from coldfront.core.allocation.forms import AllocationForm +from coldfront.core.allocation.forms import AllocationForm, HSPH_CODE from coldfront.core.test_helpers.factories import ( setup_models, ResourceTypeFactory, @@ -119,13 +119,16 @@ def test_allocationform_expense_code_valid4(self): ) def test_allocationform_expense_code_multiplefield_invalid(self): - """Test POST to AllocationCreateView in circumstance where hsph and seas values are also checked""" + """ + Test POST to AllocationCreateView in circumstance where code is entered + and an existing_expense_codes value has also been selected + """ self.post_data['expense_code'] = '123-' * 11 - self.post_data['hsph_code'] = True + self.post_data['existing_expense_codes'] = HSPH_CODE form = AllocationForm( data=self.post_data, request_user=self.pi_user, project_pk=self.project.pk ) - self.assertIn("you must do exactly one of the following", form.errors['expense_code'][0]) + self.assertIn("must either select an existing expense code or", form.errors['existing_expense_codes'][0]) class AllocationUpdateFormTest(AllocationFormBaseTest): diff --git a/coldfront/core/allocation/tests/test_views.py b/coldfront/core/allocation/tests/test_views.py index 7673c59b1..92b055552 100644 --- a/coldfront/core/allocation/tests/test_views.py +++ b/coldfront/core/allocation/tests/test_views.py @@ -439,14 +439,14 @@ def test_allocationcreateview_post_bools(self): def test_allocationcreateview_post_offerlettercode_multiplefield_invalid(self): """Ensure that form won't pass if multiple expense codes are given""" - self.post_data['hsph_code'] = '000-000-000-000-000-000-000-000-000-000-000' + self.post_data['existing_expense_codes'] = '000-000-000-000-000-000-000-000-000-000-000' response = self.client.post(self.url, data=self.post_data, follow=True) - self.assertContains(response, "you must do exactly one of the following") + self.assertContains(response, "must either select an existing expense code or") def test_allocationcreateview_post_hsph_offerlettercode(self): - """Ensure that form goes through if hsph is checked""" - self.post_data['hsph_code'] = '000-000-000-000-000-000-000-000-000-000-000' + """Ensure that form goes through if existing_expense_codes is checked""" + self.post_data['existing_expense_codes'] = '000-000-000-000-000-000-000-000-000-000-000' self.post_data.pop('expense_code') response = self.client.post(self.url, data=self.post_data, follow=True) self.assertContains(response, "Allocation requested.")