Skip to content

Commit

Permalink
Resolve 500 error with dissemination search/advanced (#4497)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
rnovak338 authored Dec 11, 2024
1 parent e515a38 commit d0733d5
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 68 deletions.
18 changes: 18 additions & 0 deletions backend/dissemination/search.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import json
import logging
import time
from .searchlib.search_constants import ORDER_BY, DIRECTION, DAS_LIMIT
Expand Down Expand Up @@ -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
15 changes: 15 additions & 0 deletions backend/dissemination/templates/search-alert-error.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{% load humanize %}
{% load sprite_helper %}
<div class="usa-alert usa-alert--error">
<div class="usa-alert__body">
<h2 class="usa-alert__heading">Error</h2>
<p>The information you entered is invalid. Please double-check your information for errors.</p>
{% if errors %}
<ul>
{% for error in errors %}
<li>{{ error }}</li>
{% endfor %}
</ul>
{% endif %}
</div>
</div>
4 changes: 3 additions & 1 deletion backend/dissemination/templates/search.html
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@ <h3>Filters</h3>
<div class="grid-col audit-search-results">
<h1 class="font-sans-2xl">Search single audit reports</h1>
{% include "search-alert-info.html" %}
{% if results|length > 0 %}
{% if form.errors %}
{% include "search-alert-error.html" %}
{% elif results|length > 0 %}
<div class="margin-y-2 display-flex flex-justify-center">
<img hidden id="loader"
src="{% static 'img/loader.svg' %}"
Expand Down
26 changes: 14 additions & 12 deletions backend/dissemination/templates/search_filters/acceptance-date.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,29 @@
aria-controls="fac-release-date">FAC acceptance date</button>
<div id="fac-release-date">
<div class="usa-form-group">
<label class="usa-label" id="start-date-label" for="start-date">Start date</label>
<div class="usa-hint" id="start-date-hint">mm/dd/yyyy</div>
<label class="usa-label" id="start_date-label" for="start_date">Start date</label>
<div class="usa-hint" id="start_date-hint">mm/dd/yyyy</div>
<div class="usa-date-picker"
data-default-value="{{ form_user_input.start_date }}">
<input class="usa-input"
id="start-date"
name="start_date"
aria-labelledby="start-date-label"
aria-describedby="start-date-hint" />
<span class="usa-error-message margin-top-1">{{ form.start_date.errors|striptags }}</span>
<input class="usa-input"
id="start_date"
name="start_date"
aria-labelledby="start_date-label"
aria-describedby="start_date-hint" />
</div>
</div>
<div class="usa-form-group">
<label class="usa-label" id="end-date-label" for="end-date">End date</label>
<div class="usa-hint" id="end-date-hint">mm/dd/yyyy</div>
<label class="usa-label" id="end_date-label" for="end_date">End date</label>
<div class="usa-hint" id="end_date-hint">mm/dd/yyyy</div>
<div class="usa-date-picker"
data-default-value="{{ form_user_input.end_date }}">
<span class="usa-error-message margin-top-1">{{ form.end_date.errors|striptags }}</span>
<input class="usa-input"
id="end-date"
id="end_date"
name="end_date"
aria-labelledby="end-date-label"
aria-describedby="end-date-hint" />
aria-labelledby="end_date-label"
aria-describedby="end_date-hint" />
</div>
</div>
</div>
18 changes: 12 additions & 6 deletions backend/dissemination/templates/search_filters/audit-year.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,18 @@
aria-controls="audit-year">Audit Year</button>
<div id="audit-year">
{% for value, text in form.audit_year.field.choices %}
<div class="usa-checkbox bg-base-lighter">
<input class="usa-checkbox__input"
id="audit-year-{{ text }}"
name="audit_year" type="checkbox"
value={{ value }}
{% if value in form_user_input.audit_year or text in form_user_input.audit_year %}checked{% endif %} />
<div class="usa-checkbox bg-base-lighter">
<input class="usa-checkbox__input"
id="audit-year-{{ text }}"
name="audit_year"
type="checkbox"
value={{ value }}
{% for item in form_user_input.audit_year %}
{% if item == value or item == text %}
checked
{% endif %}
{% endfor %}
/>
<label class="usa-checkbox__label" for="audit-year-{{ text }}">{{ text }}</label>
</div>
{% endfor %}
Expand Down
112 changes: 63 additions & 49 deletions backend/dissemination/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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"],
Expand Down Expand Up @@ -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
Expand All @@ -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"],
Expand Down

0 comments on commit d0733d5

Please sign in to comment.