From d0733d59148575039b1eb8a763a4cd7e35106b94 Mon Sep 17 00:00:00 2001 From: Bobby Novak <176936850+rnovak338@users.noreply.github.com> Date: Wed, 11 Dec 2024 10:22:06 -0500 Subject: [PATCH] Resolve 500 error with dissemination search/advanced (#4497) * Include error-handling Handled search AND advanced search. - Add form error labels for start and end dates in search. - Re-render search template instead of raising validation errors. * Create error alert template * Update POST for Search/Advanced - Removed "duplicate" logic for rendering form when it is not valid. - Instead, re-uses the logic for success and only applies a search on the tables when `form.is_valid()`. * Render user selection of audit years on form error * Linting * Provide error context Now displaying list of erroneous fields in error message when the form is invalid. * Update search-alert-error.html Fixes padding issue if there are no errors. --- backend/dissemination/search.py | 18 +++ .../templates/search-alert-error.html | 15 +++ backend/dissemination/templates/search.html | 4 +- .../search_filters/acceptance-date.html | 26 ++-- .../templates/search_filters/audit-year.html | 18 ++- backend/dissemination/views.py | 112 ++++++++++-------- 6 files changed, 125 insertions(+), 68 deletions(-) create mode 100644 backend/dissemination/templates/search-alert-error.html diff --git a/backend/dissemination/search.py b/backend/dissemination/search.py index 4c3ed69a0a..827dcb1a3d 100644 --- a/backend/dissemination/search.py +++ b/backend/dissemination/search.py @@ -1,3 +1,4 @@ +import json import logging import time from .searchlib.search_constants import ORDER_BY, DIRECTION, DAS_LIMIT @@ -147,3 +148,20 @@ def _make_distinct(results, params): results = results.distinct("report_id", order_by) return results + + +def gather_errors(form): + """ + Gather errors based on the form fields inputted by the user. + """ + formatted_errors = [] + if form.errors: + if not form.is_valid(): + errors = json.loads(form.errors.as_json()) + for error in reversed(errors): + if "start_date" in error: + formatted_errors.append("The start date you entered is invalid.") + if "end_date" in error: + formatted_errors.append("The end date you entered is invalid.") + + return formatted_errors diff --git a/backend/dissemination/templates/search-alert-error.html b/backend/dissemination/templates/search-alert-error.html new file mode 100644 index 0000000000..8139d63011 --- /dev/null +++ b/backend/dissemination/templates/search-alert-error.html @@ -0,0 +1,15 @@ +{% load humanize %} +{% load sprite_helper %} +
+
+

Error

+

The information you entered is invalid. Please double-check your information for errors.

+ {% if errors %} + + {% endif %} +
+
diff --git a/backend/dissemination/templates/search.html b/backend/dissemination/templates/search.html index 597c6cf42e..3fc9ba66c8 100644 --- a/backend/dissemination/templates/search.html +++ b/backend/dissemination/templates/search.html @@ -90,7 +90,9 @@

Filters

Search single audit reports

{% include "search-alert-info.html" %} - {% if results|length > 0 %} + {% if form.errors %} + {% include "search-alert-error.html" %} + {% elif results|length > 0 %}
FAC acceptance date
- -
mm/dd/yyyy
+ +
mm/dd/yyyy
- + {{ form.start_date.errors|striptags }} +
- -
mm/dd/yyyy
+ +
mm/dd/yyyy
+ {{ form.end_date.errors|striptags }} + aria-labelledby="end_date-label" + aria-describedby="end_date-hint" />
diff --git a/backend/dissemination/templates/search_filters/audit-year.html b/backend/dissemination/templates/search_filters/audit-year.html index 5eeca2f482..0156f5d0dd 100644 --- a/backend/dissemination/templates/search_filters/audit-year.html +++ b/backend/dissemination/templates/search_filters/audit-year.html @@ -5,12 +5,18 @@ aria-controls="audit-year">Audit Year
{% for value, text in form.audit_year.field.choices %} -
- +
+
{% endfor %} diff --git a/backend/dissemination/views.py b/backend/dissemination/views.py index 32183e15d8..5cdcf838ab 100644 --- a/backend/dissemination/views.py +++ b/backend/dissemination/views.py @@ -18,7 +18,7 @@ from dissemination.file_downloads import get_download_url, get_filename from dissemination.forms import SearchForm from dissemination.forms import AdvancedSearchForm -from dissemination.search import search +from dissemination.search import search, gather_errors from dissemination.mixins import ReportAccessRequiredMixin from dissemination.models import ( General, @@ -165,16 +165,20 @@ def post(self, request, *args, **kwargs): time_starting_post = time.time() form = AdvancedSearchForm(request.POST) + paginator_results = None + results_count = None + page = 1 results = [] + errors = [] context = {} - if form.is_valid(): - form_data = form.cleaned_data - form_user_input = { - k: v[0] if len(v) == 1 else v for k, v in form.data.lists() - } - else: - raise ValidationError(f"Form error in Search POST. {form.errors}") + # Obtain cleaned form data. + form.is_valid() + form_data = form.cleaned_data + form_user_input = {k: v[0] if len(v) == 1 else v for k, v in form.data.lists()} + + # build error list. + errors = gather_errors(form) # Tells the backend we're running advanced search. form_data["advanced_search_flag"] = True @@ -183,35 +187,38 @@ def post(self, request, *args, **kwargs): include_private = include_private_results(request) - results = run_search(form_data) + # Generate results on valid user input. + if form.is_valid(): + results = run_search(form_data) - results_count = results.count() + results_count = results.count() - # Reset page to one if the page number surpasses how many pages there actually are - page = form_data["page"] - ceiling = math.ceil(results_count / form_data["limit"]) - if page > ceiling: - page = 1 + # Reset page to one if the page number surpasses how many pages there actually are + page = form_data["page"] + ceiling = math.ceil(results_count / form_data["limit"]) + if page > ceiling: + page = 1 - logger.info(f"TOTAL: results_count: [{results_count}]") + logger.info(f"TOTAL: results_count: [{results_count}]") - # The paginator object handles splicing the results to a one-page iterable and calculates which page numbers to show. - paginator = Paginator(object_list=results, per_page=form_data["limit"]) - paginator_results = paginator.get_page(page) - paginator_results.adjusted_elided_pages = paginator.get_elided_page_range( - page, on_each_side=1 - ) + # The paginator object handles splicing the results to a one-page iterable and calculates which page numbers to show. + paginator = Paginator(object_list=results, per_page=form_data["limit"]) + paginator_results = paginator.get_page(page) + paginator_results.adjusted_elided_pages = paginator.get_elided_page_range( + page, on_each_side=1 + ) - # Reformat these so the date-picker elements in HTML prepopulate - if form_data["start_date"]: + # Reformat dates for pre-populating the USWDS date-picker. + if form_data.get("start_date"): form_user_input["start_date"] = form_data["start_date"].strftime("%Y-%m-%d") - if form_data["end_date"]: + if form_data.get("end_date"): form_user_input["end_date"] = form_data["end_date"].strftime("%Y-%m-%d") context = context | { "advanced_search_flag": True, "form_user_input": form_user_input, "form": form, + "errors": errors, "include_private": include_private, "limit": form_data["limit"], "order_by": form_data["order_by"], @@ -262,16 +269,20 @@ def post(self, request, *args, **kwargs): time_starting_post = time.time() form = SearchForm(request.POST) + paginator_results = None + results_count = None + page = 1 results = [] + errors = [] context = {} - if form.is_valid(): - form_data = form.cleaned_data - form_user_input = { - k: v[0] if len(v) == 1 else v for k, v in form.data.lists() - } - else: - raise ValidationError(f"Form error in Search POST. {form.errors}") + # Obtain cleaned form data. + form.is_valid() + form_data = form.cleaned_data + form_user_input = {k: v[0] if len(v) == 1 else v for k, v in form.data.lists()} + + # build error list. + errors = gather_errors(form) # Tells the backend we're running basic search. form_data["advanced_search_flag"] = False @@ -280,35 +291,38 @@ def post(self, request, *args, **kwargs): include_private = include_private_results(request) - results = run_search(form_data) + # Generate results on valid user input. + if form.is_valid(): + results = run_search(form_data) - results_count = results.count() + results_count = results.count() - # Reset page to one if the page number surpasses how many pages there actually are - page = form_data["page"] - ceiling = math.ceil(results_count / form_data["limit"]) - if page > ceiling: - page = 1 + # Reset page to one if the page number surpasses how many pages there actually are + page = form_data["page"] + ceiling = math.ceil(results_count / form_data["limit"]) + if page > ceiling: + page = 1 - logger.info(f"TOTAL: results_count: [{results_count}]") + logger.info(f"TOTAL: results_count: [{results_count}]") - # The paginator object handles splicing the results to a one-page iterable and calculates which page numbers to show. - paginator = Paginator(object_list=results, per_page=form_data["limit"]) - paginator_results = paginator.get_page(page) - paginator_results.adjusted_elided_pages = paginator.get_elided_page_range( - page, on_each_side=1 - ) + # The paginator object handles splicing the results to a one-page iterable and calculates which page numbers to show. + paginator = Paginator(object_list=results, per_page=form_data["limit"]) + paginator_results = paginator.get_page(page) + paginator_results.adjusted_elided_pages = paginator.get_elided_page_range( + page, on_each_side=1 + ) - # Reformat these so the date-picker elements in HTML prepopulate - if form_data["start_date"]: + # Reformat dates for pre-populating the USWDS date-picker. + if form_data.get("start_date"): form_user_input["start_date"] = form_data["start_date"].strftime("%Y-%m-%d") - if form_data["end_date"]: + if form_data.get("end_date"): form_user_input["end_date"] = form_data["end_date"].strftime("%Y-%m-%d") context = context | { "advanced_search_flag": False, "form_user_input": form_user_input, "form": form, + "errors": errors, "include_private": include_private, "limit": form_data["limit"], "order_by": form_data["order_by"],