From 8b2c3da18a1717768f3b5fa744adaf440ba751d2 Mon Sep 17 00:00:00 2001 From: Connor O'Shaughnessy Date: Wed, 15 Nov 2023 11:12:59 +0000 Subject: [PATCH] [SMD-151] Show error state on error screen WIP add javascript event listener to detect if form is submitted empty ... ... ... ... store validation errors between request in session object and remove JS ... ... ... ... .. ... ... ... ... --- app/main/data_requests.py | 6 +-- app/main/routes.py | 49 ++++++++++------------- app/templates/main/upload.html | 2 + app/templates/main/validation-errors.html | 4 +- 4 files changed, 29 insertions(+), 32 deletions(-) diff --git a/app/main/data_requests.py b/app/main/data_requests.py index 84b3890..3ccf453 100644 --- 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 index c8a08eb..6c2bebd 100644 --- 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 index b1c311e..3b924ce 100644 --- 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 %}

When you upload your return, we’ll check it for missing data and formatting errors.

diff --git a/app/templates/main/validation-errors.html b/app/templates/main/validation-errors.html index 7804d38..ee7ee1e 100644 --- a/app/templates/main/validation-errors.html +++ b/app/templates/main/validation-errors.html @@ -28,7 +28,9 @@

Errors fixed?

{{ govukFileUpload({'id': 'ingest_spreadsheet', "hint": {"text": ""}, "label": {"text": ""}, - 'name': 'ingest_spreadsheet'}) }} + 'attributes': {"accept": ".xlsx,.xls"}, + 'name': 'ingest_spreadsheet', + }) }} {{ govukButton({ "element": "button",