From a96227119d004ff6d70f336db86498cc3674daa0 Mon Sep 17 00:00:00 2001 From: Joseph Muller Date: Sun, 21 Apr 2024 20:02:14 +0100 Subject: [PATCH 1/9] Improvements to filtered list view --- src/core/forms/forms.py | 5 ++++ src/core/views.py | 27 ++++++++++--------- .../journal/article_list_filters.html | 1 + .../journal/article_list_filters.html | 1 + .../journal/article_list_filters.html | 1 + 5 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/core/forms/forms.py b/src/core/forms/forms.py index 380b6db884..d4a73138aa 100755 --- a/src/core/forms/forms.py +++ b/src/core/forms/forms.py @@ -649,6 +649,11 @@ def __init__(self, *args, **kwargs): ), ) + elif facet['type'] == 'integer': + self.fields[facet_key] = forms.IntegerField( + required=False, + ) + elif facet['type'] == 'boolean': pass diff --git a/src/core/views.py b/src/core/views.py index c504a3b6fc..6c8562f7ea 100755 --- a/src/core/views.py +++ b/src/core/views.py @@ -2442,7 +2442,7 @@ def get_context_data(self, **kwargs): initial = dict(params_querydict.lists()) for keyword, value in initial.items(): if keyword in facets: - if facets[keyword]['type'] in ['date_time', 'date']: + if facets[keyword]['type'] in ['date_time', 'date', 'integer']: initial[keyword] = value[0] context['facet_form'] = forms.CBVFacetForm( @@ -2488,7 +2488,7 @@ def get_queryset(self, params_querydict=None): if keyword in facets and value_list: if value_list[0]: predicates = [(keyword, value) for value in value_list] - elif facets[keyword]['type'] not in ['date_time', 'date']: + elif facets[keyword]['type'] not in ['date_time', 'date', 'integer']: if value_list[0] == '': predicates = [(keyword, '')] else: @@ -2499,13 +2499,14 @@ def get_queryset(self, params_querydict=None): for predicate in predicates: query |= Q(predicate) q_stack.append(query) - return self.order_queryset( - self.filter_queryset_if_journal( - self.queryset.filter(*q_stack) - ) - ).exclude( - stage=submission_models.STAGE_UNSUBMITTED, + self.queryset = self.filter_queryset_if_journal( + self.queryset.filter(*q_stack) ) + if hasattr(self.model, 'stage'): + self.queryset = self.queryset.exclude( + stage=submission_models.STAGE_UNSUBMITTED + ) + return self.order_queryset(self.queryset) def order_queryset(self, queryset): order_by = self.get_order_by() @@ -2534,14 +2535,16 @@ def get_facets(self): def get_facet_queryset(self): # The default behavior is for the facets to stay the same # when a filter is chosen. - # To make them change dynamically, return None + # To make them change dynamically, return None # instead of a separate facet. # return None queryset = self.filter_queryset_if_journal( super().get_queryset() - ).exclude( - stage=submission_models.STAGE_UNSUBMITTED ) + if hasattr(self.model, 'stage'): + queryset = queryset.exclude( + stage=submission_models.STAGE_UNSUBMITTED + ) facets = self.get_facets() for facet in facets.values(): queryset = queryset.annotate(**facet.get('annotations', {})) @@ -2567,7 +2570,7 @@ def post(self, request, *args, **kwargs): if request.journal: querysets.extend(self.split_up_queryset_if_needed(queryset)) - else: + elif hasattr(self.model, 'journal'): for journal in journal_models.Journal.objects.all(): journal_queryset = queryset.filter(journal=journal) if journal_queryset: diff --git a/src/themes/OLH/templates/elements/journal/article_list_filters.html b/src/themes/OLH/templates/elements/journal/article_list_filters.html index 622955ad46..02f9930832 100644 --- a/src/themes/OLH/templates/elements/journal/article_list_filters.html +++ b/src/themes/OLH/templates/elements/journal/article_list_filters.html @@ -7,6 +7,7 @@

{% trans 'Filter' %}

{{ facet_form|foundation }} + {% trans 'Clear all' %}
{% endif %} diff --git a/src/themes/clean/templates/elements/journal/article_list_filters.html b/src/themes/clean/templates/elements/journal/article_list_filters.html index cfd012f438..e5ecb86ce1 100644 --- a/src/themes/clean/templates/elements/journal/article_list_filters.html +++ b/src/themes/clean/templates/elements/journal/article_list_filters.html @@ -8,6 +8,7 @@

{% trans 'Filter' %}

{% bootstrap_form facet_form %} + {% trans 'Clear all' %}
diff --git a/src/themes/material/templates/elements/journal/article_list_filters.html b/src/themes/material/templates/elements/journal/article_list_filters.html index 37d1c5bbba..c41ed662b5 100644 --- a/src/themes/material/templates/elements/journal/article_list_filters.html +++ b/src/themes/material/templates/elements/journal/article_list_filters.html @@ -12,6 +12,7 @@

Filter

{{ facet_form|materializecss }}
+ {% trans 'Clear all' %}
From e8c28057c5d70d2fb79073d958ceb7bc32fef796 Mon Sep 17 00:00:00 2001 From: Joseph Muller Date: Sun, 21 Apr 2024 20:36:27 +0100 Subject: [PATCH 2/9] Add search using admin search --- src/core/forms/forms.py | 8 ++++++++ src/core/views.py | 13 ++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/core/forms/forms.py b/src/core/forms/forms.py index d4a73138aa..f5f7c3f7cc 100755 --- a/src/core/forms/forms.py +++ b/src/core/forms/forms.py @@ -654,6 +654,14 @@ def __init__(self, *args, **kwargs): required=False, ) + elif facet['type'] == 'search': + self.fields[facet_key] = forms.CharField( + required=False, + widget=forms.TextInput( + attrs={'type': 'search'} + ), + ) + elif facet['type'] == 'boolean': pass diff --git a/src/core/views.py b/src/core/views.py index 6c8562f7ea..4f5069e248 100755 --- a/src/core/views.py +++ b/src/core/views.py @@ -2442,7 +2442,7 @@ def get_context_data(self, **kwargs): initial = dict(params_querydict.lists()) for keyword, value in initial.items(): if keyword in facets: - if facets[keyword]['type'] in ['date_time', 'date', 'integer']: + if facets[keyword]['type'] in ['date_time', 'date', 'integer', 'search']: initial[keyword] = value[0] context['facet_form'] = forms.CBVFacetForm( @@ -2486,9 +2486,16 @@ def get_queryset(self, params_querydict=None): # The following line prevents the user from passing any parameters # other than those specified in the facets. if keyword in facets and value_list: - if value_list[0]: + if keyword == 'q': + self.queryset, _duplicates = facets[keyword]['admin'].get_search_results( + self.request, + self.queryset, + value_list[0], + ) + predicates = [] + elif value_list[0]: predicates = [(keyword, value) for value in value_list] - elif facets[keyword]['type'] not in ['date_time', 'date', 'integer']: + elif facets[keyword]['type'] not in ['date_time', 'date', 'integer', 'search']: if value_list[0] == '': predicates = [(keyword, '')] else: From 72a95f075491d667b3ba042e799c62d95deac585 Mon Sep 17 00:00:00 2001 From: Joseph Muller Date: Mon, 22 Apr 2024 03:20:43 +0100 Subject: [PATCH 3/9] Implement boolean fields on filtered list view --- src/core/forms/forms.py | 4 +++- src/core/views.py | 7 ++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/core/forms/forms.py b/src/core/forms/forms.py index f5f7c3f7cc..9f27acc85c 100755 --- a/src/core/forms/forms.py +++ b/src/core/forms/forms.py @@ -663,7 +663,9 @@ def __init__(self, *args, **kwargs): ) elif facet['type'] == 'boolean': - pass + self.fields[facet_key] = forms.BooleanField( + required=False, + ) self.fields[facet_key].label = facet['field_label'] diff --git a/src/core/views.py b/src/core/views.py index 4f5069e248..61aad51f22 100755 --- a/src/core/views.py +++ b/src/core/views.py @@ -2442,7 +2442,7 @@ def get_context_data(self, **kwargs): initial = dict(params_querydict.lists()) for keyword, value in initial.items(): if keyword in facets: - if facets[keyword]['type'] in ['date_time', 'date', 'integer', 'search']: + if facets[keyword]['type'] in ['date_time', 'date', 'integer', 'search', 'boolean']: initial[keyword] = value[0] context['facet_form'] = forms.CBVFacetForm( @@ -2493,6 +2493,11 @@ def get_queryset(self, params_querydict=None): value_list[0], ) predicates = [] + elif facets[keyword]['type'] == 'boolean': + if value_list[0]: + predicates = [(keyword, True)] + else: + predicates = [(keyword, False)] elif value_list[0]: predicates = [(keyword, value) for value in value_list] elif facets[keyword]['type'] not in ['date_time', 'date', 'integer', 'search']: From eda957b006f5f029a7c7a11d8a3e437eb7c648ee Mon Sep 17 00:00:00 2001 From: Joseph Muller Date: Mon, 22 Apr 2024 16:14:35 +0100 Subject: [PATCH 4/9] Make add user field work if no journal --- src/core/views.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/views.py b/src/core/views.py index 61aad51f22..16d9a78b6a 100755 --- a/src/core/views.py +++ b/src/core/views.py @@ -1198,9 +1198,10 @@ def add_user(request): if registration_form.is_valid(): new_user = registration_form.save() # Every new user is given the author role - new_user.add_account_role('author', request.journal) + if request.journal: + new_user.add_account_role('author', request.journal) - if role: + if role and request.journal: new_user.add_account_role(role, request.journal) form = forms.EditAccountForm( From 761b0075b96caac1d3db1cd27bf1fb8c345aa288 Mon Sep 17 00:00:00 2001 From: Joseph Muller Date: Wed, 8 May 2024 15:58:02 +0100 Subject: [PATCH 5/9] Make model utility for searching via admin search --- src/core/model_utils.py | 19 +++++++++++++++++++ src/core/tests/test_models.py | 28 +++++++++++++++++++++++++++- src/core/views.py | 11 +++++++---- 3 files changed, 53 insertions(+), 5 deletions(-) diff --git a/src/core/model_utils.py b/src/core/model_utils.py index 9d1af2f9a0..3096599e49 100644 --- a/src/core/model_utils.py +++ b/src/core/model_utils.py @@ -12,6 +12,7 @@ from django import forms from django.apps import apps +from django.contrib import admin from django.contrib.postgres.lookups import SearchLookup as PGSearchLookup from django.contrib.postgres.search import ( SearchVector as DjangoSearchVector, @@ -572,6 +573,24 @@ def set_source_expressions(self, _): template = '%(expressions)s' +def search_model_admin(request, model, q=None, queryset=None): + """ + A simple search using the admin search functionality, + for use in class-based views where our methods for + article search do not suit. + :param request: A Django request object + :param model: Any model that has search_fields specified in its admin + :param q: the search term + :param queryset: a pre-existing queryset to filter by the search term + """ + if not q: + q = request.POST['q'] if request.POST else request.GET['q'] + if not queryset: + queryset = model.objects.all() + registered_admin = admin.site._registry[model] + return registered_admin.get_search_results(request, queryset, q) + + class JanewayBleachField(BleachField): """ An override of BleachField to avoid casting SafeString from db Bleachfield automatically casts the default return type (string) into diff --git a/src/core/tests/test_models.py b/src/core/tests/test_models.py index 13ee46801b..e191a24c66 100644 --- a/src/core/tests/test_models.py +++ b/src/core/tests/test_models.py @@ -2,13 +2,18 @@ from django.core.files.uploadedfile import SimpleUploadedFile from django.db import IntegrityError +from django.http import HttpRequest, QueryDict from django.forms import Form from django.test import TestCase from django.utils import timezone from freezegun import freeze_time from core import forms, models -from core.model_utils import merge_models, SVGImageFieldForm +from core.model_utils import ( + merge_models, + SVGImageFieldForm, + search_model_admin, +) from journal import models as journal_models from utils.testing import helpers from submission import models as submission_models @@ -371,3 +376,24 @@ def test_last_modified_model_recursive_doubly_linked(self): # Test self.assertEqual(self.article.best_last_modified_date(), file_date) + + +class TestModelUtils(TestCase): + + @classmethod + def setUpTestData(cls): + cls.account = helpers.create_user( + 'Ab6CrWPPxQ7FoLj5dgdH@example.org', + ) + + def test_search_model_admin(self): + request = HttpRequest() + request.GET = QueryDict('q=Ab6CrWPPxQ7FoLj5dgdH') + results, _duplicates = search_model_admin( + request, + models.Account, + ) + self.assertIn( + self.account, + results, + ) diff --git a/src/core/views.py b/src/core/views.py index 16d9a78b6a..2b6f235e00 100755 --- a/src/core/views.py +++ b/src/core/views.py @@ -32,6 +32,7 @@ from django.views import generic from core import models, forms, logic, workflow, models as core_models +from core.model_utils import search_model_admin from security.decorators import ( editor_user_required, article_author_required, has_journal, any_editor_user_required, role_can_access, @@ -2412,6 +2413,7 @@ class FilteredArticlesListView(generic.ListView): It does not have access controls applied because some public views use it. For staff views, be sure to filter to published articles in get_queryset. Do not use this view directly. + This view can also be subclassed and modified for use with other models. """ model = submission_models.Article @@ -2487,11 +2489,12 @@ def get_queryset(self, params_querydict=None): # The following line prevents the user from passing any parameters # other than those specified in the facets. if keyword in facets and value_list: - if keyword == 'q': - self.queryset, _duplicates = facets[keyword]['admin'].get_search_results( + if facets[keyword]['type'] == 'search': + self.queryset, _duplicates = search_model_admin( self.request, - self.queryset, - value_list[0], + self.model, + q=value_list[0], + queryset=self.queryset, ) predicates = [] elif facets[keyword]['type'] == 'boolean': From d950673e206548ebcf9ad4b60ae82b049ee37c8d Mon Sep 17 00:00:00 2001 From: Joseph Muller Date: Wed, 29 May 2024 17:37:02 +0100 Subject: [PATCH 6/9] Make FilteredListView properly generic --- src/core/model_utils.py | 4 ++++ src/core/views.py | 35 +++++++++++++---------------------- src/identifiers/views.py | 5 ++--- src/journal/views.py | 26 +++++++++++++++++++++++++- 4 files changed, 44 insertions(+), 26 deletions(-) diff --git a/src/core/model_utils.py b/src/core/model_utils.py index 3096599e49..3e42e0674d 100644 --- a/src/core/model_utils.py +++ b/src/core/model_utils.py @@ -675,3 +675,7 @@ class DateTimePickerModelField(models.DateTimeField): def formfield(self, **kwargs): kwargs['form_class'] = DateTimePickerFormField return super().formfield(**kwargs) + +@property +def NotImplementedField(self): + raise NotImplementedError diff --git a/src/core/views.py b/src/core/views.py index 2b6f235e00..90260ae9a2 100755 --- a/src/core/views.py +++ b/src/core/views.py @@ -32,7 +32,7 @@ from django.views import generic from core import models, forms, logic, workflow, models as core_models -from core.model_utils import search_model_admin +from core.model_utils import NotImplementedField, search_model_admin from security.decorators import ( editor_user_required, article_author_required, has_journal, any_editor_user_required, role_can_access, @@ -2407,20 +2407,20 @@ def manage_access_requests(request): ) -class FilteredArticlesListView(generic.ListView): +class GenericFacetedListView(generic.ListView): """ - This is a base class for article list views. - It does not have access controls applied because some public views use it. - For staff views, be sure to filter to published articles in get_queryset. - Do not use this view directly. - This view can also be subclassed and modified for use with other models. + This is a generic base class for creating filterable list views + with Janeway models. """ + model = NotImplementedField + template_name = NotImplementedField - model = submission_models.Article - template_name = 'core/manager/article_list.html' paginate_by = '25' facets = {} + # These fields will receive a single initial value, not a list + single_value_fields = ['date_time', 'date', 'integer', 'search', 'boolean'] + # None or integer action_queryset_chunk_size = None @@ -2440,12 +2440,11 @@ def get_context_data(self, **kwargs): context['paginate_by'] = params_querydict.get('paginate_by', self.paginate_by) facets = self.get_facets() - # Most initial values are in list form - # The exception is date_time facets initial = dict(params_querydict.lists()) + for keyword, value in initial.items(): if keyword in facets: - if facets[keyword]['type'] in ['date_time', 'date', 'integer', 'search', 'boolean']: + if facets[keyword]['type'] in self.single_value_fields: initial[keyword] = value[0] context['facet_form'] = forms.CBVFacetForm( @@ -2489,7 +2488,7 @@ def get_queryset(self, params_querydict=None): # The following line prevents the user from passing any parameters # other than those specified in the facets. if keyword in facets and value_list: - if facets[keyword]['type'] == 'search': + if facets[keyword]['type'] == 'search' and value_list[0]: self.queryset, _duplicates = search_model_admin( self.request, self.model, @@ -2518,10 +2517,6 @@ def get_queryset(self, params_querydict=None): self.queryset = self.filter_queryset_if_journal( self.queryset.filter(*q_stack) ) - if hasattr(self.model, 'stage'): - self.queryset = self.queryset.exclude( - stage=submission_models.STAGE_UNSUBMITTED - ) return self.order_queryset(self.queryset) def order_queryset(self, queryset): @@ -2557,10 +2552,6 @@ def get_facet_queryset(self): queryset = self.filter_queryset_if_journal( super().get_queryset() ) - if hasattr(self.model, 'stage'): - queryset = queryset.exclude( - stage=submission_models.STAGE_UNSUBMITTED - ) facets = self.get_facets() for facet in facets.values(): queryset = queryset.annotate(**facet.get('annotations', {})) @@ -2617,7 +2608,7 @@ def split_up_queryset_if_needed(self, queryset): return [queryset] def filter_queryset_if_journal(self, queryset): - if self.request.journal: + if self.request.journal and hasattr(self.model, 'journal'): return queryset.filter(journal=self.request.journal) else: return queryset diff --git a/src/identifiers/views.py b/src/identifiers/views.py index adf04cfeff..871c531e42 100755 --- a/src/identifiers/views.py +++ b/src/identifiers/views.py @@ -11,8 +11,7 @@ from identifiers import models, forms from submission import models as submission_models -from core import views as core_views -from journal import models as journal_models +from journal import models as journal_models, views as journal_views from security.decorators import production_user_or_editor_required, editor_user_required from identifiers import logic @@ -321,7 +320,7 @@ def delete_identifier(request, article_id, identifier_id): @method_decorator(editor_user_required, name='dispatch') -class IdentifierManager(core_views.FilteredArticlesListView): +class IdentifierManager(journal_views.FacetedArticlesListView): template_name = 'core/manager/identifier_manager.html' # None or integer diff --git a/src/journal/views.py b/src/journal/views.py index 43d3fb94a7..5108fb51d7 100755 --- a/src/journal/views.py +++ b/src/journal/views.py @@ -2650,9 +2650,33 @@ def manage_languages(request): ) +class FacetedArticlesListView(core_views.GenericFacetedListView): + """ + This is a base class for article list views. + It does not have access controls applied because some public views use it. + For staff views, be sure to filter to published articles in get_queryset. + Do not use this view directly. + This view can also be subclassed and modified for use with other models. + """ + model = submission_models.Article + template_name = 'core/manager/article_list.html' + + def get_queryset(self, params_querydict=None): + self.queryset = super().get_queryset(params_querydict=params_querydict) + return self.queryset.exclude( + stage=submission_models.STAGE_UNSUBMITTED + ) + + def get_facet_queryset(self, **kwargs): + queryset = super().get_facet_queryset(**kwargs) + return queryset.exclude( + stage=submission_models.STAGE_UNSUBMITTED + ) + + @method_decorator(has_journal, name='dispatch') @method_decorator(decorators.frontend_enabled, name='dispatch') -class PublishedArticlesListView(core_views.FilteredArticlesListView): +class PublishedArticlesListView(FacetedArticlesListView): """ A list of published articles that can be searched, From 6249fabbcc60d37691838bde2f3655d443028fd0 Mon Sep 17 00:00:00 2001 From: Joseph Muller Date: Wed, 29 May 2024 17:38:03 +0100 Subject: [PATCH 7/9] Remove deprecated null check The if/else check deleted in this commit was used in an earlier version of the view to let the user filter by whether data was missing on a given facet. In other words, we wanted to be able to send [something]__isnull=True in the query. This was superceded with another appraoch, where some option representing missing data was added to choices in a select field, and translated into the query by means of an annotation. --- src/core/views.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/core/views.py b/src/core/views.py index 90260ae9a2..e588c1f00a 100755 --- a/src/core/views.py +++ b/src/core/views.py @@ -2503,11 +2503,6 @@ def get_queryset(self, params_querydict=None): predicates = [(keyword, False)] elif value_list[0]: predicates = [(keyword, value) for value in value_list] - elif facets[keyword]['type'] not in ['date_time', 'date', 'integer', 'search']: - if value_list[0] == '': - predicates = [(keyword, '')] - else: - predicates = [(keyword+'__isnull', True)] else: predicates = [] query = Q() From 057ae67014c20f877d3136e768768559040460db Mon Sep 17 00:00:00 2001 From: Joseph Muller Date: Fri, 31 May 2024 12:32:06 +0100 Subject: [PATCH 8/9] Use set for efficiency #63 --- src/core/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/views.py b/src/core/views.py index e588c1f00a..a5112881d9 100755 --- a/src/core/views.py +++ b/src/core/views.py @@ -2419,7 +2419,7 @@ class GenericFacetedListView(generic.ListView): facets = {} # These fields will receive a single initial value, not a list - single_value_fields = ['date_time', 'date', 'integer', 'search', 'boolean'] + single_value_fields = {'date_time', 'date', 'integer', 'search', 'boolean'} # None or integer action_queryset_chunk_size = None From 111651a33ba0dc931b9fbe0d14698451ad5be925 Mon Sep 17 00:00:00 2001 From: Joseph Muller Date: Mon, 3 Jun 2024 16:56:32 +0100 Subject: [PATCH 9/9] Backwards compatibility --- src/core/views.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/core/views.py b/src/core/views.py index a5112881d9..fcf47b2434 100755 --- a/src/core/views.py +++ b/src/core/views.py @@ -2614,3 +2614,18 @@ def filter_facets_if_journal(self, facets): return facets else: return facets + + +class FilteredArticlesListView(GenericFacetedListView): + """ + Deprecated. Former base class for article list views. + """ + + model = submission_models.Article + template_name = 'core/manager/article_list.html' + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + raise DeprecationWarning( + 'This view is deprecated. Use GenericFacetedListView instead.' + )