From 44835659e7fae2cdbc785b20d9ff76bd8dc4e57d Mon Sep 17 00:00:00 2001
From: Neda Moeini <neda.moeini@geant.org>
Date: Thu, 27 Mar 2025 15:07:23 +0100
Subject: [PATCH 1/3] Refactor error messages in form validation
---
sage_validation/file_validator/forms.py | 35 ++++++++++++++++++-------
1 file changed, 26 insertions(+), 9 deletions(-)
diff --git a/sage_validation/file_validator/forms.py b/sage_validation/file_validator/forms.py
index 96ca943..ef6e9ec 100644
--- a/sage_validation/file_validator/forms.py
+++ b/sage_validation/file_validator/forms.py
@@ -130,17 +130,20 @@ class CSVUploadForm(forms.Form):
msg = f"Missing required columns: {', '.join(missing_columns)}"
raise forms.ValidationError(msg)
- @staticmethod
- def _validate_source_and_trader_type(data: list[dict]) -> list:
+ def _validate_source_and_trader_type(self, data: list[dict]) -> list:
"""Validate that 'Source' is always 80 and 'SYSTraderTranType' is always 4."""
errors = []
for index, row in enumerate(data, start=1):
+ claimant_name = self.get_account_name_from_code(row.get("AccountNumber"))
+ claim_number = row.get("SecondReference")
if row.get("Source") != "80":
- errors.append(f"Row {index}: 'Source' must be 80, but found {row.get('Source')}.")
+ errors.append(f"Row {index}, claimant: {claimant_name} with claim number: {claim_number}: "
+ f"'Source' must be 80, but found {row.get('Source')}.")
if row.get("SYSTraderTranType") != "4":
- errors.append(f"Row {index}: 'SYSTraderTranType' must be 4, but found {row.get('SYSTraderTranType')}.")
+ errors.append(f"Row {index}, claimant: {claimant_name} with claim number: {claim_number}: "
+ f"'SYSTraderTranType' must be 4, but found {row.get('SYSTraderTranType')}.")
return errors
@@ -189,6 +192,16 @@ class CSVUploadForm(forms.Form):
return errors
+ @staticmethod
+ def get_account_name_from_code(account_code: str| None) -> str | None:
+ """Get the account name from the PL Account Codes table."""
+ if account_code is None:
+ return None
+ try:
+ return MeoValidSuppliers.objects.using("meo").get(
+ supplier_account_number=account_code).supplier_account_name
+ except MeoValidSuppliers.DoesNotExist:
+ return None
def _validate_nc_cc_dep_combination_against_meo_sage_account(self, data: list[dict]) -> list[str]:
"""Validate that all nominal analysis fields exist in MEO.
@@ -218,6 +231,8 @@ class CSVUploadForm(forms.Form):
max_repeat = self._get_max_repeat(fieldnames, "NominalAnalysisNominalCostCentre")
for index, row in enumerate(data, start=1):
+ claimant_name = self.get_account_name_from_code(row.get("AccountNumber"))
+ claim_number = row.get("SecondReference")
for repeat in range(1, max_repeat + 1):
cc_field = f"NominalAnalysisNominalCostCentre/{repeat}"
dep_field = f"NominalAnalysisNominalDepartment/{repeat}"
@@ -251,13 +266,13 @@ class CSVUploadForm(forms.Form):
errors.append(
f"Row {index}: The combination of '{cc_field}' ({cc}), "
f"'{dep_field}' ({dep}), and '{nominal_account_field}' "
- f"({nc}) does not exist in MEO valid Sage accounts."
+ f"({nc}) for claimant '{claimant_name}' and claim number '{claim_number}' "
+ f"does not exist in MEO valid Sage accounts."
)
return errors
- @staticmethod
- def _cheque_fields_must_be_empty(data: list[dict]) -> list[str]:
+ def _cheque_fields_must_be_empty(self, data: list[dict]) -> list[str]:
"""Validate that cheque fields are empty.
The cheque fields are 'ChequeCurrencyName', 'ChequeToBankExchangeRate', and 'ChequeValueInChequeCurrency'.
@@ -267,10 +282,12 @@ class CSVUploadForm(forms.Form):
cheque_currency_name = row.get("ChequeCurrencyName")
cheque_to_bank_exchange_rate = row.get("ChequeToBankExchangeRate")
cheque_value_in_cheque_currency = row.get("ChequeValueInChequeCurrency")
-
+ claimant_name = self.get_account_name_from_code(row.get("AccountNumber"))
+ claim_number = row.get("SecondReference")
if any([cheque_currency_name, cheque_to_bank_exchange_rate, cheque_value_in_cheque_currency]):
errors.append(
- f"Row {index}: Unexpected values in the Cheque columns. All cheque columns must be empty."
+ f"Row {index}: Unexpected values in the Cheque columns for {claimant_name} with claim number: "
+ f"{claim_number}. All cheque columns must be empty."
)
return errors
--
GitLab
From 52c40ad37d1808ca2b0e1f4899a9faa8715fa511 Mon Sep 17 00:00:00 2001
From: Neda Moeini <neda.moeini@geant.org>
Date: Thu, 27 Mar 2025 15:07:40 +0100
Subject: [PATCH 2/3] Update failing unit tests.
---
test/test_file_validator/test_forms.py | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/test/test_file_validator/test_forms.py b/test/test_file_validator/test_forms.py
index 24681bc..d329630 100644
--- a/test/test_file_validator/test_forms.py
+++ b/test/test_file_validator/test_forms.py
@@ -61,8 +61,8 @@ def test_source_and_trader_type_validation(sample_input_file: SimpleUploadedFile
modified_file = create_modified_csv(sample_input_file, {"Source": "90", "SYSTraderTranType": "5"})
form = CSVUploadForm(files={"file": modified_file})
assert not form.is_valid()
- assert "Row 1: 'Source' must be 80" in form.errors["file"][0]
- assert "Row 1: 'SYSTraderTranType' must be 4" in form.errors["file"][1]
+ assert "'Source' must be 80" in form.errors["file"][0]
+ assert "'SYSTraderTranType' must be 4" in form.errors["file"][1]
def test_validate_nominal_analysis_account(sample_input_file: SimpleUploadedFile, mock_meo_database: MagicMock) -> None:
@@ -99,4 +99,4 @@ def test_cheque_fields_must_be_empty(sample_input_file: SimpleUploadedFile, mock
{"ChequeCurrencyName": "USD", "ChequeToBankExchangeRate": "1"})
form = CSVUploadForm(files={"file": modified_file})
assert not form.is_valid()
- assert "Row 1: Unexpected values in the Cheque columns. All cheque columns must be empty." in form.errors["file"][0]
+ assert "All cheque columns must be empty." in form.errors["file"][0]
--
GitLab
From 9f858de2b8a15bedad67ae691175853a6c985141 Mon Sep 17 00:00:00 2001
From: Neda Moeini <neda.moeini@geant.org>
Date: Thu, 27 Mar 2025 15:08:05 +0100
Subject: [PATCH 3/3] Enhance showing errors.
---
sage_validation/file_validator/templates/upload.html | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sage_validation/file_validator/templates/upload.html b/sage_validation/file_validator/templates/upload.html
index 1e5cb07..e124398 100644
--- a/sage_validation/file_validator/templates/upload.html
+++ b/sage_validation/file_validator/templates/upload.html
@@ -85,10 +85,10 @@
li.textContent = 'You are not authorized to perform this action.';
errorList.appendChild(li);
} else if (response.status === 400 && result.status === 'error') {
- for (const [field, messages] of Object.entries(result.errors)) {
+ for (const messages of Object.values(result.errors)) {
messages.forEach(message => {
const li = document.createElement('li');
- li.textContent = `${field}: ${message}`;
+ li.textContent = message;
errorList.appendChild(li);
});
}
--
GitLab