Merge pull request #109 from communitiesuk/feature/SMD-151-show-error…

…-state-on-errors-screen Feature/[smd 151] show error state on errors screen
communitiesuk · Dec 15, 2023 · e2f56fd · e2f56fd
2 parents 4de2b8a + 8b2c3da
commit e2f56fd
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 32 deletions.
diff --git a/app/main/data_requests.py b/app/main/data_requests.py
@@ -6,7 +6,7 @@
 from config import Config
 
 
-def post_ingest(file: FileStorage, data: dict = None) -> tuple[bool, dict | None, dict | None, dict | None]:
+def post_ingest(file: FileStorage, data: dict = None) -> tuple[dict | None, dict | None, dict | None]:
     """Send an HTTP POST request to ingest into the data store
      server and return the response.
 
@@ -30,14 +30,12 @@ def post_ingest(file: FileStorage, data: dict = None) -> tuple[bool, dict | None
     file.seek(0)  # reset the stream position
     response_json = response.json()
 
-    success = False
     pre_transformation_errors = None
     validation_errors = None
     metadata = None
 
     match response.status_code:
         case 200:
-            success = True
             loaded = response_json.get("loaded", False)
             if not loaded:
                 # TODO: replace this 500 with specific content explaining that loading has been disabled
@@ -57,4 +55,4 @@ def post_ingest(file: FileStorage, data: dict = None) -> tuple[bool, dict | None
             current_app.logger.error(f"Bad response: {request_url} returned {response.status_code}")
             abort(500)
 
-    return success, pre_transformation_errors, validation_errors, metadata
+    return pre_transformation_errors, validation_errors, metadata
diff --git a/app/main/routes.py b/app/main/routes.py
@@ -49,31 +49,17 @@ def upload():
 
     if request.method == "POST":
         excel_file = request.files.get("ingest_spreadsheet")
-        error = check_file(excel_file)
-        if error:
-            current_app.logger.info(
-                PRE_VALIDATION_ERROR_LOG.format(error=error) if Config.ENABLE_VALIDATION_LOGGING else PRE_VALIDATION_LOG
-            )
-            return render_template(
-                "upload.html",
-                pre_error=[error],
-                days_to_deadline=calculate_days_to_deadline(),
-                reporting_period=Config.REPORTING_PERIOD,
-                fund=Config.FUND_NAME,
-            )
 
-        success, pre_errors, validation_errors, metadata = post_ingest(
-            excel_file,
-            {"reporting_round": 4, "auth": json.dumps(g.auth.get_auth_dict()), "do_load": is_load_enabled()},
-        )
+        if pre_errors := check_file(excel_file):
+            validation_errors, metadata = None, None
+        else:
+            pre_errors, validation_errors, metadata = post_ingest(
+                excel_file,
+                {"reporting_round": 4, "auth": json.dumps(g.auth.get_auth_dict()), "do_load": is_load_enabled()},
+            )
 
-        if success:
-            if Config.SEND_CONFIRMATION_EMAILS:
-                send_confirmation_emails(excel_file, metadata, user_email=g.user.email)
-            metadata["User"] = g.user.email
-            current_app.logger.info(f"Upload successful: {metadata}")
-            return render_template("success.html", file_name=excel_file.filename)
-        elif pre_errors:
+        if pre_errors:
+            # Pre-validation failure
             if Config.ENABLE_VALIDATION_LOGGING:
                 for pre_err in pre_errors:
                     current_app.logger.info(PRE_VALIDATION_ERROR_LOG.format(error=pre_err))
@@ -87,7 +73,8 @@ def upload():
                 reporting_period=Config.REPORTING_PERIOD,
                 fund=Config.FUND_NAME,
             )
-        else:
+        elif validation_errors:
+            # Validation failure
             if Config.ENABLE_VALIDATION_LOGGING:
                 for validation_err in validation_errors:
                     current_app.logger.info(VALIDATION_ERROR_LOG.format(error=validation_err))
@@ -98,9 +85,17 @@ def upload():
                 "validation-errors.html",
                 validation_errors=validation_errors,
             )
+        else:
+            # Success
+            if Config.SEND_CONFIRMATION_EMAILS:
+                send_confirmation_emails(excel_file, metadata, user_email=g.user.email)
+            metadata["User"] = g.user.email
+            current_app.logger.info(f"Upload successful: {metadata}")
+
+            return render_template("success.html", file_name=excel_file.filename)
 
 
-def check_file(excel_file: FileStorage) -> str | None:
+def check_file(excel_file: FileStorage) -> list[str] | None:
     """Basic checks on an uploaded file.
 
     Checks:
@@ -112,9 +107,9 @@ def check_file(excel_file: FileStorage) -> str | None:
     """
     error = None
     if not excel_file:
-        error = "Select your returns template"
+        error = ["Select your returns template."]
     elif excel_file.content_type != MIMETYPE.XLSX:
-        error = "The selected file must be an XLSX"
+        error = ["The selected file must be an XLSX."]
     return error
 
 

diff --git a/app/templates/main/upload.html b/app/templates/main/upload.html
@@ -58,12 +58,14 @@
                     {{ govukFileUpload({'id': 'ingest_spreadsheet',
                      "hint": {"text": ""},
                      "label": {"text": ""},
+                     'attributes': {"accept": ".xlsx,.xls"},
                      "errorMessage": {"html": errorMessage(pre_error)},
                      'name': 'ingest_spreadsheet'}) }}
                 {% else %}
                     {{ govukFileUpload({'id': 'ingest_spreadsheet',
                      "hint": {"text": ""},
                      "label": {"text": ""},
+                     'attributes': {"accept": ".xlsx,.xls"},
                      'name': 'ingest_spreadsheet'}) }}
                 {% endif %}
                 <p class="govuk-body">When you upload your return, we’ll check it for missing data and formatting errors.</p>

diff --git a/app/templates/main/validation-errors.html b/app/templates/main/validation-errors.html
@@ -28,7 +28,9 @@ <h2 class="govuk-heading-l">Errors fixed?</h2>
             {{ govukFileUpload({'id': 'ingest_spreadsheet',
              "hint": {"text": ""},
              "label": {"text": ""},
-             'name': 'ingest_spreadsheet'}) }}
+             'attributes': {"accept": ".xlsx,.xls"},
+             'name': 'ingest_spreadsheet',
+             }) }}
 
             {{ govukButton({
                 "element": "button",