diff --git a/cfgov/agreements/jinja2/agreements/search.html b/cfgov/agreements/jinja2/agreements/search.html index 4ef35f9fec0..43660abacbe 100755 --- a/cfgov/agreements/jinja2/agreements/search.html +++ b/cfgov/agreements/jinja2/agreements/search.html @@ -49,7 +49,7 @@

Agreements by '{{ issuer.name }}'

{% if page.has_next() or page.has_previous() %}
- {% import 'v1/includes/molecules/pagination.html' as pagination with context %} + {% import 'v1/includes/molecules/pagination.html' as pagination %} {{ pagination.render( page.paginator.num_pages, page.number, diff --git a/cfgov/ask_cfpb/jinja2/ask-cfpb/answer-search-results.html b/cfgov/ask_cfpb/jinja2/ask-cfpb/answer-search-results.html index d513ada07e7..2ca759bdaa7 100644 --- a/cfgov/ask_cfpb/jinja2/ask-cfpb/answer-search-results.html +++ b/cfgov/ask_cfpb/jinja2/ask-cfpb/answer-search-results.html @@ -1,4 +1,4 @@ -{% import 'v1/includes/molecules/pagination.html' as pagination with context %} +{% import 'v1/includes/molecules/pagination.html' as pagination %} {% import 'ask-cfpb/_ask-search.html' as ask_search with context %} {% extends 'v1/layouts/layout-2-1.html' %} @@ -52,7 +52,8 @@

{{ _('Showing') }} {{ results.start_index() }}-{{ res paginator.num_pages, current_page | int, '', - index + index, + pagination_params ) }}

diff --git a/cfgov/ask_cfpb/jinja2/ask-cfpb/see-all.html b/cfgov/ask_cfpb/jinja2/ask-cfpb/see-all.html index 6b3a40f7266..e530bff9a67 100644 --- a/cfgov/ask_cfpb/jinja2/ask-cfpb/see-all.html +++ b/cfgov/ask_cfpb/jinja2/ask-cfpb/see-all.html @@ -1,6 +1,6 @@ {% extends 'v1/layouts/layout-1-3.html' %} -{% import 'v1/includes/molecules/pagination.html' as pagination with context %} +{% import 'v1/includes/molecules/pagination.html' as pagination %} {% block css -%} {{ super() }} @@ -77,7 +77,7 @@

{% endfor %}
- {{ pagination.render( paginator.num_pages, current_page, '', 0 ) }} + {{ pagination.render( paginator.num_pages, current_page, '', 0, pagination_params ) }}
{% if not page.portal_category %} diff --git a/cfgov/ask_cfpb/models/pages.py b/cfgov/ask_cfpb/models/pages.py index c54fe5a0117..233be2b2093 100644 --- a/cfgov/ask_cfpb/models/pages.py +++ b/cfgov/ask_cfpb/models/pages.py @@ -289,6 +289,11 @@ def get_results(self, request): "pages": paginator.page(page_number), "paginator": paginator, "current_page": page_number, + "pagination_params": ( + {"search_term": search.search_term} + if search.search_term + else {} + ), } ) return TemplateResponse(request, "ask-cfpb/see-all.html", context) @@ -377,6 +382,12 @@ def get_context(self, request, **kwargs): ) context["about_us"] = get_standard_text(self.language, "about_us") context["disclaimer"] = get_standard_text(self.language, "disclaimer") + + # AnswerResultsPage.query is set by ask_cfpb.views.ask_search for + # cases where the user is making a query-based search. + query = getattr(self, "query", None) + context["pagination_params"] = {"q": query} if query else {} + return context diff --git a/cfgov/prepaid_agreements/jinja2/prepaid_agreements/search_results.html b/cfgov/prepaid_agreements/jinja2/prepaid_agreements/search_results.html index b443920de3a..b005d126321 100644 --- a/cfgov/prepaid_agreements/jinja2/prepaid_agreements/search_results.html +++ b/cfgov/prepaid_agreements/jinja2/prepaid_agreements/search_results.html @@ -2,7 +2,7 @@ {% import 'v1/includes/atoms/radio-button.html' as radio %} {% import 'v1/includes/atoms/tag-filter.html' as tag %} {% import 'prepaid_agreements//search_result_item.html' as search_item %} -{% import 'v1/includes/molecules/pagination.html' as pagination with context %} +{% import 'v1/includes/molecules/pagination.html' as pagination %} {% from 'v1/includes/organisms/expandable.html' import expandable with context %} {% if current_count and current_count > 0 %} @@ -168,7 +168,7 @@

No results match your search.

{% if paginator.num_pages > 1 %}
- {{ pagination.render( paginator.num_pages, current_page, '', '' ) }} + {{ pagination.render( paginator.num_pages, current_page, '', '', pagination_params ) }}
{% endif %}
diff --git a/cfgov/prepaid_agreements/views.py b/cfgov/prepaid_agreements/views.py index c18c4d23bd9..fabb63b2822 100644 --- a/cfgov/prepaid_agreements/views.py +++ b/cfgov/prepaid_agreements/views.py @@ -137,6 +137,12 @@ def index(request): paginator = Paginator(products.all(), 25) page = paginator.get_page(page_number) + pagination_params = dict(filters) + if search_term: + pagination_params["q"] = search_term + if search_field: + pagination_params["search_field"] = search_field + return TemplateResponse( request, "prepaid_agreements/index.html", @@ -153,6 +159,7 @@ def index(request): "search_field": search_field, "disclaimer_text": get_disclaimer_text(), "support_text": get_support_text(), + "pagination_params": pagination_params, }, ) diff --git a/cfgov/regulations3k/jinja2/regulations3k/search-regulations-results.html b/cfgov/regulations3k/jinja2/regulations3k/search-regulations-results.html index 4a9dd4f3ede..91ee00cb15a 100644 --- a/cfgov/regulations3k/jinja2/regulations3k/search-regulations-results.html +++ b/cfgov/regulations3k/jinja2/regulations3k/search-regulations-results.html @@ -1,6 +1,6 @@ {% import 'v1/includes/atoms/tag-filter.html' as tag %} {% import 'regulations3k/regulations3k-search-result-item.html' as search_item %} -{% import 'v1/includes/molecules/pagination.html' as pagination with context %} +{% import 'v1/includes/molecules/pagination.html' as pagination %}
{% if current_count and current_count > 0 %} @@ -51,6 +51,6 @@

No results match your filters.

{% endfor %}
- {{ pagination.render( paginator.num_pages, current_page, '', '' ) }} + {{ pagination.render( paginator.num_pages, current_page, '', '', pagination_params ) }}
{% endif %} diff --git a/cfgov/regulations3k/jinja2/regulations3k/search-regulations.html b/cfgov/regulations3k/jinja2/regulations3k/search-regulations.html index 5e6be2866d6..bd36e12371a 100644 --- a/cfgov/regulations3k/jinja2/regulations3k/search-regulations.html +++ b/cfgov/regulations3k/jinja2/regulations3k/search-regulations.html @@ -3,7 +3,6 @@ {% import 'v1/includes/atoms/radio-button.html' as radio with context %} {% import 'regulations3k/regulations3k-search-bar.html' as search_bar %} {% import 'regulations3k/regulations3k-search-result-item.html' as search_item %} -{% import 'v1/includes/molecules/pagination.html' as pagination with context %} {% from 'v1/includes/organisms/expandable.html' import expandable with context %} {# HEAD items #} diff --git a/cfgov/regulations3k/models/pages.py b/cfgov/regulations3k/models/pages.py index f481d1700e2..82df4960ed3 100644 --- a/cfgov/regulations3k/models/pages.py +++ b/cfgov/regulations3k/models/pages.py @@ -144,6 +144,12 @@ def regulation_results_page(self, request): "show_filters": any( reg["selected"] is True for reg in payload["all_regs"] ), + "pagination_params": { + "q": search_query, + "regs": regs, + "order": order, + "results": num_results, + }, } ) return TemplateResponse(request, self.get_template(request), context) diff --git a/cfgov/searchgov/jinja2/searchgov/index.html b/cfgov/searchgov/jinja2/searchgov/index.html index 2632cfef227..19b20f93391 100644 --- a/cfgov/searchgov/jinja2/searchgov/index.html +++ b/cfgov/searchgov/jinja2/searchgov/index.html @@ -2,7 +2,7 @@ {% from './recommended.html' import render as render_recommended %} {% from './result.html' import render as render_result %} {% import 'v1/includes/molecules/notification.html' as notification %} -{% import 'v1/includes/molecules/pagination.html' as pagination with context %} +{% import 'v1/includes/molecules/pagination.html' as pagination %} {% set search_header = _('Search the CFPB') %} @@ -81,7 +81,8 @@

Results

{{ pagination.render( total_pages, - current_page + current_page, + extra_params=pagination_params ) }}
{% endblock content_main %} diff --git a/cfgov/searchgov/views.py b/cfgov/searchgov/views.py index a8615596d0f..ee65b03992f 100644 --- a/cfgov/searchgov/views.py +++ b/cfgov/searchgov/views.py @@ -120,6 +120,7 @@ def get(self, request, **kwargs): "current_page": current_page, "recommended": recommended, "results": results, + "pagination_params": {"q": query} if query else {}, } if form.cleaned_data.get("format") == "json": diff --git a/cfgov/teachers_digital_platform/jinja2/teachers_digital_platform/activity_search_facets_and_results.html b/cfgov/teachers_digital_platform/jinja2/teachers_digital_platform/activity_search_facets_and_results.html index 693f4ecea44..fedc0536b59 100644 --- a/cfgov/teachers_digital_platform/jinja2/teachers_digital_platform/activity_search_facets_and_results.html +++ b/cfgov/teachers_digital_platform/jinja2/teachers_digital_platform/activity_search_facets_and_results.html @@ -1,5 +1,5 @@ {% import 'v1/includes/atoms/tag-filter.html' as tag %} -{% import 'v1/includes/molecules/pagination.html' as pagination with context %} +{% import 'v1/includes/molecules/pagination.html' as pagination %} {% from 'teachers_digital_platform/tdp_expandable.html' import expandable with context %} {% macro build_filter_section(name) %} @@ -162,7 +162,7 @@

No results match your search.

- {{ pagination.render( paginator.num_pages, current_page, 'tdp-search-facets-and-results', '' ) }} + {{ pagination.render( paginator.num_pages, current_page, 'tdp-search-facets-and-results', '', pagination_params ) }}
Activities align with the My Money Five principles introduced by the statutorily created federal Financial Literacy and Education Commission.
diff --git a/cfgov/teachers_digital_platform/models/activity_index_page.py b/cfgov/teachers_digital_platform/models/activity_index_page.py index 71df48ffab8..ee3dbaf8156 100644 --- a/cfgov/teachers_digital_platform/models/activity_index_page.py +++ b/cfgov/teachers_digital_platform/models/activity_index_page.py @@ -115,6 +115,10 @@ def dsl_search(self, request, *args, **kwargs): return {"facets": []} data = self.search_form.cleaned_data + pagination_params = { + key: data.get(key) + for key in ("q", "grade_level", "activity_duration", "topic") + } all_facets = copy.copy(self.activity_setups.facet_setup) selected_facets = {} card_setup = self.activity_setups.ordered_cards @@ -145,6 +149,7 @@ def dsl_search(self, request, *args, **kwargs): "current_page": current_page, "paginator": paginator, "show_filters": bool(selected_facets), + "pagination_params": pagination_params, } return context_update @@ -216,6 +221,7 @@ def dsl_search(self, request, *args, **kwargs): "current_page": current_page, "paginator": paginator, "show_filters": bool(selected_facets), + "pagination_params": pagination_params, } return context_update diff --git a/cfgov/v1/forms.py b/cfgov/v1/forms.py index f6dab919808..3729d8e3d22 100644 --- a/cfgov/v1/forms.py +++ b/cfgov/v1/forms.py @@ -13,7 +13,7 @@ from v1.models import enforcement_action_page from v1.util import ERROR_MESSAGES, ref -from v1.util.categories import clean_categories +from v1.util.categories import expand_shorthand_categories from v1.util.datetimes import end_of_time_period @@ -129,8 +129,6 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - clean_categories(selected_categories=self.data.get("categories")) - # When the form is created, it needs to make a request to the search # backend to be able to populate some of its fields: topics, languages, # and the minimum page date. @@ -139,6 +137,10 @@ def __init__(self, *args, **kwargs): self.set_topics() self.set_languages() + def clean_categories(self): + """Expand category shorthand into appropriate subcategories.""" + return expand_shorthand_categories(self.cleaned_data.get("categories")) + def has_unfiltered_results(self): return self.aggregations.hits.total.value > 0 diff --git a/cfgov/v1/jinja2/v1/includes/macros/util/url_parameters.html b/cfgov/v1/jinja2/v1/includes/macros/util/url_parameters.html deleted file mode 100644 index e04921a4f27..00000000000 --- a/cfgov/v1/jinja2/v1/includes/macros/util/url_parameters.html +++ /dev/null @@ -1,24 +0,0 @@ -{# ========================================================================== - - url_parameters() - - ========================================================================== - - Description: - - Builds pagination markup when given: - - parameters: A dictionary of url parameters. - - ========================================================================== #} - -{% macro url_parameters(parameters) %} - {%- set ignored_params = ('page', 'partial') -%} - {%- for key in parameters.keys() -%} - {% if parameters.getlist(key) and key not in ignored_params -%} - {%- for value in parameters.getlist(key, []) -%} - &{{ key }}={{ value }} - {%- endfor -%} - {%- endif %} - {%- endfor -%} -{% endmacro %} diff --git a/cfgov/v1/jinja2/v1/includes/molecules/pagination.html b/cfgov/v1/jinja2/v1/includes/molecules/pagination.html index e7aa292c251..6d5a18b0c6e 100644 --- a/cfgov/v1/jinja2/v1/includes/molecules/pagination.html +++ b/cfgov/v1/jinja2/v1/includes/molecules/pagination.html @@ -20,14 +20,35 @@ index: A unique number given to render the form and its fields with. Default is 0. + extra_params: A dict of additional query parameters to carry forward onto + the prev/next/page links, e.g. the caller's active search or + filter values. Each value may be a single string or a list + of strings (for repeated params). + ========================================================================== #} -{% macro render(total_pages, current_page, fragment_id='', index=0) %} +{%- macro render( + total_pages, current_page, fragment_id='', index=0, extra_params=none +) %} {% if total_pages > 1 and current_page <= total_pages %} -{% from 'v1/includes/macros/util/url_parameters.html' import url_parameters %} {% set fragment_id = '#' + fragment_id if fragment_id else '' %} +{%- set pairs = [] %} +{%- for key, values in (extra_params or {}).items() if key not in ('page', 'partial') %} + {%- for value in ( + [values] if values is string or values is not iterable else values + ) %} + {%- if value is not none and value != '' %} + {%- do pairs.append((key, value)) %} + {%- endif %} + {%- endfor %} +{%- endfor -%} + +{%- set extra_qs %} + {%- for key, value in pairs %}&{{ key }}={{ value }}{% endfor %} +{%- endset -%} +
{% endif %} diff --git a/cfgov/v1/models/filterable_page.py b/cfgov/v1/models/filterable_page.py index 457b123578e..b38357cbb18 100644 --- a/cfgov/v1/models/filterable_page.py +++ b/cfgov/v1/models/filterable_page.py @@ -175,10 +175,18 @@ def get_valid_form_results(self, request, form): filtered_qs_page, many=True, context={"request": request} ) + # cleaned_data["categories"] is expanded from shorthand groups + # like "blog" into a set of subcategory slugs, which is what the + # search backend filters on. Pagination query strings should include + # the original shorthand, not the expanded set. + pagination_params = dict(form.cleaned_data) + pagination_params["categories"] = form.data.get("categories", []) + return { "paginator": paginator, "page_number": filtered_qs_page.number, "results": serializer.data, + "pagination_params": pagination_params, } def set_do_not_index(self, field, value): diff --git a/cfgov/v1/tests/atomic_elements/test_molecules.py b/cfgov/v1/tests/atomic_elements/test_molecules.py index 61de605d24d..805ee4eae63 100644 --- a/cfgov/v1/tests/atomic_elements/test_molecules.py +++ b/cfgov/v1/tests/atomic_elements/test_molecules.py @@ -1,9 +1,12 @@ +import datetime from io import StringIO +from django.template import engines from django.test import SimpleTestCase, TestCase from wagtail.blocks import StreamValue from wagtail.blocks.struct_block import StructBlockValidationError +from wagtail.test.utils.wagtail_tests import WagtailTestUtils from scripts import _atomic_helpers as atomic from search.elasticsearch_helpers import ElasticsearchTestsMixin @@ -295,3 +298,136 @@ def test_render_both_child_and_parent_page_provide_feed(self): html = self.render(context={"page": child_page}) self.assertHTMLContainsLinkToPageFeed(html, child_page) + + +class PaginationTests(WagtailTestUtils, SimpleTestCase): + def setUp(self): + self.jinja_engine = engines["wagtail-env"] + self.template_source = """ +{% import "v1/includes/molecules/pagination.html" as pagination %} +{{ pagination.render( + total_pages, current_page, fragment_id, index, extra_params +) }} +""".strip() + + def render(self, **kwargs): + template = self.jinja_engine.from_string(self.template_source) + return template.render(kwargs) + + def test_no_output_when_only_one_page(self): + html = self.render(total_pages=1, current_page=1) + self.assertEqual(html.strip(), "") + + def test_no_output_when_current_page_beyond_total(self): + html = self.render(total_pages=3, current_page=4) + self.assertEqual(html.strip(), "") + + def test_middle_page_has_both_prev_and_next_links(self): + html = self.render(total_pages=3, current_page=2) + self.assertTagInHTML( + '', html + ) + self.assertTagInHTML( + '', html + ) + + def test_first_page_disables_prev_link(self): + html = self.render(total_pages=3, current_page=1) + self.assertTagInHTML( + '', html + ) + self.assertTagInHTML( + '', html + ) + + def test_last_page_disables_next_link(self): + html = self.render(total_pages=3, current_page=3) + self.assertTagInHTML( + '', html + ) + self.assertTagInHTML( + '', html + ) + + def test_fragment_id_appended_to_links(self): + html = self.render( + total_pages=3, current_page=2, fragment_id="my-results" + ) + self.assertIn('href="?page=1#my-results">', html) + self.assertIn('href="?page=3#my-results">', html) + + def test_index_used_in_current_page_field_id(self): + html = self.render(total_pages=3, current_page=2, index=7) + self.assertIn('id="m-pagination__current-page-7"', html) + + def test_string_extra_param_included_in_links_and_hidden_field(self): + html = self.render( + total_pages=3, current_page=2, extra_params={"q": "hello world"} + ) + self.assertIn('href="?page=1&q=hello world">', html) + self.assertIn('href="?page=3&q=hello world">', html) + self.assertTagInHTML( + '', html + ) + + def test_list_extra_param_repeated_for_each_value(self): + html = self.render( + total_pages=3, + current_page=2, + extra_params={"categories": ["a", "b"]}, + ) + self.assertIn('href="?page=1&categories=a&categories=b">', html) + self.assertTagInHTML( + '', html + ) + self.assertTagInHTML( + '', html + ) + + def test_empty_list_extra_param_produces_nothing(self): + html = self.render( + total_pages=3, current_page=2, extra_params={"categories": []} + ) + self.assertNotIn("categories", html) + + def test_non_string_scalar_extra_param_is_stringified(self): + html = self.render( + total_pages=3, current_page=2, extra_params={"results": 50} + ) + self.assertIn("results=50", html) + + def test_date_extra_param_is_stringified(self): + html = self.render( + total_pages=3, + current_page=2, + extra_params={"from_date": datetime.date(2024, 1, 15)}, + ) + self.assertIn("from_date=2024-01-15", html) + + def test_none_and_empty_string_extra_params_are_omitted(self): + html = self.render( + total_pages=3, + current_page=2, + extra_params={"from_date": None, "title": ""}, + ) + self.assertNotIn("from_date", html) + self.assertNotIn("title", html) + + def test_page_and_partial_keys_are_always_excluded(self): + html = self.render( + total_pages=3, + current_page=2, + extra_params={"page": 99, "partial": True, "q": "hi"}, + ) + self.assertNotIn("page=99", html) + self.assertNotIn("partial", html) + self.assertIn("q=hi", html) + + def test_extra_param_values_are_html_escaped(self): + html = self.render( + total_pages=3, + current_page=2, + extra_params={"q": '">'}, + ) + self.assertNotIn("