Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tp2000 959 measure search results page revamp #1021

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

LaurenMullally
Copy link
Contributor

TP-??? Your PR title here

Why

What

@LaurenMullally LaurenMullally force-pushed the TP2000-959-measure-search-results-page-revamp branch from c86907e to 37287b7 Compare September 6, 2023 14:02
@LaurenMullally LaurenMullally force-pushed the TP2000-959-measure-search-results-page-revamp branch from 37287b7 to b234531 Compare September 6, 2023 14:16
{% else %}
<a class="govuk-link govuk-link--no-visited-state sort-icon--up" href="{{ base_url }}?sort_by={{ sort_by }}&order=desc{{ anchor }}">
{% else %}
<a class="govuk-link govuk-link--no-visited-state sort-icon--up" href="{{ base_url }}?sort_by={{ sort_by }}&ordered=desc{{ anchor }}">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the name of the order to ordered as it clashed with the order number in the measures filter

@@ -1,4 +1,127 @@
{% extends "layouts/layout.jinja" %}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this template as the design wants some changes to the wording of the pagination, so thought it would make sense just to have it all in one template as it's a one off different design

@@ -125,21 +127,46 @@ class MeasureSearch(FilterView):
filterset_class = MeasureFilter

def form_valid(self, form):
print(form)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to take this out oops

@@ -153,14 +180,42 @@ def paginator(self):
return MeasurePaginator(self.filterset.qs, per_page=20)

def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
context = {}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The get_context_data call ruined everything I had set in the paginator. It goes up the calling tree and somehow undoes all of the max counts, limits breached etc. I've taken the approach to manually do it in this get context.. and this seems to work and apply on the page.

@@ -736,7 +736,7 @@
LIMITED_PAGINATOR_MAX_COUNT = 200
# Default max number of objects that will be accurately counted by MeasurePaginator.
MEASURES_PAGINATOR_MAX_COUNT = int(
os.environ.get("MEASURES_PAGINATOR_MAX_COUNT", "1000"),
os.environ.get("MEASURES_PAGINATOR_MAX_COUNT", "30000"),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whacking the limit up from the previously unapplied 1000.. could be a mistake but 1000 measures doesn't seem sensible for what TM's will want to do at a time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant