From 7a1a592bc709c1ac9e7a6efd101bc48b32c8ce42 Mon Sep 17 00:00:00 2001 From: Mauro MSL Date: Wed, 27 Nov 2024 20:47:52 +0000 Subject: [PATCH 1/3] refactor: move CBV form generation to its own method --- src/core/views.py | 48 ++++++++++++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/src/core/views.py b/src/core/views.py index 2b3812f82..c720ad5e5 100755 --- a/src/core/views.py +++ b/src/core/views.py @@ -29,6 +29,7 @@ from django.views.decorators.csrf import ensure_csrf_cookie from django.contrib.contenttypes.models import ContentType from django.utils.translation import gettext_lazy as _ +from django.utils.html import mark_safe from django.utils import translation from django.db.models import Q, OuterRef, Subquery, Count, Avg from django.views import generic @@ -2451,6 +2452,7 @@ class GenericFacetedListView(generic.ListView): This is a generic base class for creating filterable list views with Janeway models. """ + form_class = forms.CBVFacetForm model = NotImplementedField template_name = NotImplementedField @@ -2472,25 +2474,28 @@ def get_paginate_by(self, queryset): paginate_by = self.paginate_by return paginate_by + def get_facet_form(self, queryset): + GET_data = dict(self.request.GET.copy().lists()) + + for keyword, values in GET_data.items(): + if keyword in self.facets: + if self.facets[keyword]['type'] in self.single_value_fields: + GET_data[keyword] = values[0] + + form = self.form_class( + GET_data, + queryset=queryset, + facets=self.facets, + ) + + return form + def get_context_data(self, **kwargs): params_querydict = self.request.GET.copy() context = super().get_context_data(**kwargs) queryset = self.get_queryset() context['paginate_by'] = params_querydict.get('paginate_by', self.paginate_by) - facets = self.get_facets() - - initial = dict(params_querydict.lists()) - - for keyword, value in initial.items(): - if keyword in facets: - if facets[keyword]['type'] in self.single_value_fields: - initial[keyword] = value[0] - - context['facet_form'] = forms.CBVFacetForm( - queryset=queryset, - facets=facets, - initial=initial, - ) + context['facet_form'] = self.get_facet_form(queryset) context['actions'] = self.get_actions() @@ -2507,6 +2512,7 @@ def get_context_data(self, **kwargs): return context def get_queryset(self, params_querydict=None): + self.facets = self.get_facets() if not params_querydict: params_querydict = self.request.GET.copy() @@ -2517,11 +2523,20 @@ def get_queryset(self, params_querydict=None): # Clear order_by, since it is handled separately params_querydict.pop('order_by', '') + facets = self.facets self.queryset = super().get_queryset() - q_stack = [] - facets = self.get_facets() + for facet in facets.values(): self.queryset = self.queryset.annotate(**facet.get('annotations', {})) + self.queryset = self.queryset.filter(self.get_journal_filter_query()) + + # Validate params against form ahead of filtering + if params_querydict: + form = self.get_facet_form(self.queryset) + if not form.is_valid(): + return self.queryset + + q_stack = [] for keyword, value_list in params_querydict.lists(): # The following line prevents the user from passing any parameters # other than those specified in the facets. @@ -2545,7 +2560,6 @@ def get_queryset(self, params_querydict=None): for predicate in predicates: query |= Q(predicate) q_stack.append(query) - q_stack.append(self.get_journal_filter_query()) self.queryset = self.queryset.filter(*q_stack).distinct() return self.order_queryset(self.queryset) From d99c2dbae3dd3aec1cae961c4663a5255871250a Mon Sep 17 00:00:00 2001 From: Mauro MSL Date: Wed, 27 Nov 2024 20:49:28 +0000 Subject: [PATCH 2/3] feat: Adds validation to CBV form #4497 --- src/core/views.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/core/views.py b/src/core/views.py index c720ad5e5..6cab3a3c9 100755 --- a/src/core/views.py +++ b/src/core/views.py @@ -2494,6 +2494,15 @@ def get_context_data(self, **kwargs): params_querydict = self.request.GET.copy() context = super().get_context_data(**kwargs) queryset = self.get_queryset() + form = self.get_facet_form(queryset) + if params_querydict and not form.is_valid(): + messages.add_message( + self.request, + messages.ERROR, + mark_safe(_("The form did not validate: ") + + (f"{form.errors}") + ), + ) context['paginate_by'] = params_querydict.get('paginate_by', self.paginate_by) context['facet_form'] = self.get_facet_form(queryset) From 366675f3d0ebfe38af99a8a6afb97a87f974d7dd Mon Sep 17 00:00:00 2001 From: Mauro MSL Date: Wed, 27 Nov 2024 20:50:26 +0000 Subject: [PATCH 3/3] fix: Correct type for CBV's multi choice field #4497 --- src/core/forms/forms.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/forms/forms.py b/src/core/forms/forms.py index 3ce9c45db..fc575c7c1 100755 --- a/src/core/forms/forms.py +++ b/src/core/forms/forms.py @@ -598,7 +598,6 @@ def __init__(self, *args, **kwargs): self.id = 'facet_form' self.queryset = kwargs.pop('queryset') self.facets = kwargs.pop('facets') - self.fields = {} super().__init__(*args, **kwargs) @@ -619,7 +618,7 @@ def __init__(self, *args, **kwargs): choices.append((each.pk, label_with_count)) choices = sorted(choices, key=lambda x: x[1]) - self.fields[facet_key] = forms.ChoiceField( + self.fields[facet_key] = forms.MultipleChoiceField( widget=forms.widgets.CheckboxSelectMultiple, choices=choices, required=False,