Skip to content
This repository has been archived by the owner on Jul 25, 2024. It is now read-only.

Commit

Permalink
store validation errors between request in session object and remove JS
Browse files Browse the repository at this point in the history 
...

...

...

...

..

...
  • Loading branch information
Connor O'Shaughnessy committed Dec 7, 2023
1 parent 8099734 commit d8ee621
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 114 deletions.
6 changes: 2 additions & 4 deletions app/main/data_requests.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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
61 changes: 34 additions & 27 deletions app/main/routes.py
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import json

from flask import current_app, g, redirect, render_template, request, url_for
from flask import current_app, g, redirect, render_template, request, session, url_for
from fsd_utils.authentication.config import SupportedApp
from fsd_utils.authentication.decorators import login_requested, login_required
from werkzeug.datastructures import FileStorage
Expand Down Expand Up @@ -40,6 +40,7 @@ def login():
@auth_required
def upload():
if request.method == "GET":
session.pop("validation_errors", None)
return render_template(
"upload.html",
days_to_deadline=calculate_days_to_deadline(),
Expand All @@ -49,55 +50,61 @@ 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 file_errors := check_file(excel_file):
pre_errors, validation_errors, metadata = file_errors, 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))
else:
current_app.logger.info(PRE_VALIDATION_LOG)

validation_errors = session.pop("validation_errors", None)
if file_errors and validation_errors:
return render_template(
"validation-errors.html",
pre_error=pre_errors,
validation_errors=validation_errors,
)

return render_template(
"upload.html",
pre_error=pre_errors,
days_to_deadline=calculate_days_to_deadline(),
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)
metadata["User"] = g.user.email
current_app.logger.info(f"Upload successful: {metadata}")

session.pop("validation_errors", None)
return render_template("success.html", file_name=excel_file.filename)


def check_file(excel_file: FileStorage) -> str | None:
Expand All @@ -112,9 +119,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


Expand Down
77 changes: 0 additions & 77 deletions app/static/src/js/custom.js
View file
Original file line number Diff line number Diff line change
@@ -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();
}
});
});
26 changes: 20 additions & 6 deletions app/templates/main/validation-errors.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 -%}
Expand All @@ -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",
Expand Down
69 changes: 69 additions & 0 deletions tests/test_routes.py
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from datetime import datetime, timedelta

from bs4 import BeautifulSoup
from flask import session

from config import Config

Expand Down Expand Up @@ -96,12 +97,80 @@ 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_errors_stored_in_session(requests_mock, example_ingest_wrong_format, flask_test_client):
"""Given a user has validation errors from a submission when they submit again getting pre-validation errors then
both sets of errors should be displayed on the screen"""
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_ingest_wrong_format},
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 XLSX." 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_xlsx_validation_and_reset_session(
requests_mock, mocker, example_pre_ingest_data_file, flask_test_client
):
"""Given a user has validation errors from a submission when they submit again and are successful then
no validation errors should be stored in the session"""
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",
}
]

mocker.patch("app.main.routes.send_confirmation_emails")
requests_mock.post(
"http://data-store/ingest",
json={"detail": "Spreadsheet successfully uploaded", "status": 200, "title": "success", "loaded": True},
status_code=200,
)
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 "Return submitted" in str(page_html)
assert not session.get("validation_errors")


def test_upload_ingest_generic_bad_request(requests_mock, example_pre_ingest_data_file, flask_test_client):
requests_mock.post(
"http://data-store/ingest",
Expand Down

0 comments on commit d8ee621

Please sign in to comment.