store validation errors between request in session object and remove JS

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

app/main/routes.py

@@ -1,4 +1,4 @@
-from flask import current_app, g, redirect, render_template, request, url_for
+from flask import current_app, g, redirect, render_template, request, url_for, session
 from fsd_utils.authentication.config import SupportedApp
 from fsd_utils.authentication.decorators import login_requested, login_required
 from werkzeug.datastructures import FileStorage
@@ -49,36 +49,26 @@ 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
+        if not (pre_errors := check_file(excel_file)):
+            pre_errors, validation_errors, metadata = post_ingest(
+                excel_file, {"reporting_round": 4, "place_names": place_names, "do_load": is_load_enabled()}
             )
-            return render_template(
-                "upload.html",
-                pre_error=[error],
-                local_authorities=local_authorities,
-                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, "place_names": place_names, "do_load": is_load_enabled()}
-        )
 
-        if success:
-            if Config.SEND_CONFIRMATION_EMAILS:
-                send_confirmation_emails(excel_file, metadata, user_email=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))
             else:
                 current_app.logger.info(PRE_VALIDATION_LOG)
 
+            if "validation_errors" in session:
+                return render_template(
+                    "validation-errors.html",
+                    pre_error=pre_errors,
+                    validation_errors=session.get("validation_errors"),
+                )
+
             return render_template(
                 "upload.html",
                 pre_error=pre_errors,
@@ -87,17 +77,25 @@ 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))
             else:
                 current_app.logger.info(VALIDATION_LOG)
-
+            # Save validation errors in session
+            session['validation_errors'] = validation_errors
             return render_template(
                 "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)
+            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:
@@ -112,9 +110,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
 
 

app/static/src/js/custom.js

@@ -1,78 +1 @@
 // Your custom JS goes here...
-
-const noFileError = (
-    '<p class="govuk-error-message">\n' +
-    '    Some of your data is not what we expect, so we could not run the full checks.\n' +
-    '</p>\n' +
-    '<p class="govuk-error-message">\n' +
-    '    Fix these problems and re-upload:\n' +
-    '</p>\n' +
-    '<ul>\n' +
-    '    <li>Select your returns template.</li>' +
-    '</ul>\n')
-
-const noFileSummary = (
-    '<div role="alert">\n' +
-    '    <h2 class="govuk-error-summary__title">There is a problem</h2>\n' +
-    '    <div class="govuk-error-summary__body govuk-error-message">\n' +
-    '        <p class="govuk-error-message">\n' +
-    '            Some of your data is not what we expect, so we could not run the full checks.\n' +
-    '        </p>\n' +
-    '        <p class="govuk-error-message">\n' +
-    '            Fix these problems and re-upload:\n' +
-    '        </p>\n' +
-    '        <ul>\n' +
-    '            <li><a class="govuk-error-message" href="#ingest_spreadsheet">Select your returns template.</a></li>\n' +
-    '        </ul>\n' +
-    '    </div>\n' +
-    '</div>\n')
-
-
-document.addEventListener('DOMContentLoaded', function() {
-      "This event listener checks to see if a user submits an empty form and updates the DOM to display error messages. " +
-      "It is writen in such a way that if correct html classes cannot be found on the page the behaviour will default " +
-      "to standard error messageing e.g a new request will be made.";
-      // Find the form by its ID
-      var form = document.getElementById('submit_form');
-
-      // Add a submit event listener to the form
-      form.addEventListener('submit', function(event) {
-        // Get the value of the file input by its ID
-        var fileInput = document.getElementById('ingest_spreadsheet');
-
-        // Check if the file input is empty
-        if (fileInput.files.length === 0) {
-          fileInput.classList.add('govuk-file-upload--error');
-
-          var errorMessage = document.getElementById('error_message');
-          var errorSummary = document.getElementById('error_summary');
-          if (errorMessage && errorSummary) {
-            // If the error message and summary already exists, update its content
-            errorMessage.innerHTML = noFileError
-            errorSummary.innerHTML = noFileSummary
-          } else if (!errorMessage && !errorSummary){
-            // Create the error message and summary elements if they don't exist
-            errorMessage = document.createElement('p');
-            errorMessage.id = 'error_message';
-            errorMessage.className = 'govuk-error-message';
-            errorMessage.innerHTML = noFileError
-
-            errorSummary = document.createElement('div');
-            errorSummary.id = 'error_summary';
-            errorSummary.className = 'govuk-error-summary'
-            errorSummary.innerHTML = noFileSummary
-
-            // Insert into the DOM
-            var errorDiv = document.querySelector('.govuk-form-group');
-            errorDiv.classList.add('govuk-form-group--error');
-            errorDiv.prepend(errorMessage)
-
-            var container = document.getElementById('main_container');
-            container.prepend(errorSummary)
-          } else {
-            return
-          }
-          event.preventDefault();
-        }
-      });
-    });

app/templates/main/validation-errors.html

@@ -5,6 +5,8 @@
 {%- from "govuk_frontend_jinja/components/accordion/macro.html" import govukAccordion -%}
 {%- from "govuk_frontend_jinja/components/accordion/macro.html" import govukNotificationBanner -%}
 {%- from "govuk_frontend_jinja/components/table/macro.html" import govukTable -%}
+{%- from "errorSummary.html" import errorSummary -%}
+{%- from "errorSummary.html" import errorMessage -%}
 {%- from "errorTable.html" import errorTable -%}
 {%- from "errorSummary.html" import errorSummary -%}
 {%- from "help-links-dropdown.html" import helpLinksDropdown -%}
@@ -18,19 +20,31 @@
 
 {% block content %}
     <div class="upload-data-container govuk-!-margin-top-5" id="main_container">
+        {% if pre_error %}
+            {{ errorSummary(pre_error) }}
+        {% endif %}
         <form method="post" action="{{ url_for('main.upload') }}" enctype="multipart/form-data" id="submit_form">
             <h1 class="govuk-heading-l">There are errors in your return</h1>
             <p class="govuk-body">Fix these errors and re-upload your return.</p>
                 {{ errorTable(validation_errors) }}
             <h2 class="govuk-heading-l">Errors fixed?</h2>
             <label class="govuk-heading-m">Re-upload your data return</label>
 
-            {{ govukFileUpload({'id': 'ingest_spreadsheet',
-             "hint": {"text": ""},
-             "label": {"text": ""},
-             'attributes': {"accept": ".xlsx,.xls"},
-             'name': 'ingest_spreadsheet',
-             }) }}
+            {% if pre_error %}
+                {{ 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 %}
 
             {{ govukButton({
                 "element": "button",

tests/test_routes.py

@@ -1,6 +1,7 @@
 from datetime import datetime, timedelta
 
 from bs4 import BeautifulSoup
+from flask import session
 
 from config import Config
 
@@ -16,8 +17,8 @@ def test_upload_page(flask_test_client):
     assert response.status_code == 200
     assert b"Upload your data return" in response.data
     assert (
-        b"When you upload your return, we\xe2\x80\x99ll check it for missing data and formatting errors."
-        in response.data
+            b"When you upload your return, we\xe2\x80\x99ll check it for missing data and formatting errors."
+            in response.data
     )
 
 
@@ -96,12 +97,46 @@ def test_upload_xlsx_validation_errors(requests_mock, example_pre_ingest_data_fi
     response = flask_test_client.post("/upload", data={"ingest_spreadsheet": example_pre_ingest_data_file})
     page_html = BeautifulSoup(response.data)
     assert response.status_code == 200
+    assert len(session.get("validation_errors")) == 2
     assert "There are errors in your return" in str(page_html)
     assert "Project admin" in str(page_html)
     assert "You are missing project locations. Please enter a project location." in str(page_html)
     assert "Start date in an incorrect format. Please enter a dates in the format 'Dec-22'" in str(page_html)
 
 
+def test_upload_xlsx_validation_and_pre_validation_errors(requests_mock, example_pre_ingest_data_file, flask_test_client):
+    with flask_test_client.session_transaction() as sess:
+        sess["validation_errors"] = [
+            {
+                "sheet": "Project Admin",
+                "section": "section1",
+                "cell_index": "A1",
+                "description": "You are missing project locations. Please enter a project location.",
+                "error_type": "NonNullableConstraintFailure",
+            }
+        ]
+
+    requests_mock.post(
+        "http://data-store/ingest",
+        json={
+            "detail": "Workbook validation failed",
+            "status": 400,
+            "title": "Bad Request",
+            "pre_transformation_errors": ["The selected file must be an Excel file"],
+        },
+        status_code=400,
+    )
+    response = flask_test_client.post("/upload", data={"ingest_spreadsheet": example_pre_ingest_data_file},
+                                      environ_base={"SESSION": flask_test_client.session_transaction()})
+    page_html = BeautifulSoup(response.data)
+    assert response.status_code == 200
+    assert "The selected file must be an Excel file" in str(page_html)
+
+    assert "There are errors in your return" in str(page_html)
+    assert "Project admin" in str(page_html)
+    assert "You are missing project locations. Please enter a project location." in str(page_html)
+
+
 def test_upload_ingest_generic_bad_request(requests_mock, example_pre_ingest_data_file, flask_test_client):
     requests_mock.post(
         "http://data-store/ingest",