Merge branch 'enhancement/improve-error-messages' into 'develop'

Enhancement/improve error messages

See merge request !6

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

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);
                             });
                         }

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]