Skip to content

Commit ea6503e

Browse files
authored
Add organization view UI filters (#10847)
* Add organization view UI filters This filters are used for the new dashboard UI, they are not API filters. These were a challenge to create, as django-filter is really opinionated about the way it should work, and doesn't quite agree with use cases that need to use filtered querysets (such as limiting the field choices to objects the organization is related to). I went through many permutations of this code, trying to find a pattern that was not obtuse or awful. Unfortunately, I don't quite like the patterns here, but because all of the django-filter magic happens at the class level instead of at instantiation time, every direction required hacks to obtain something reasonable. Given the use we have for filters in our UI, I wouldn't mind making these into a more generalized filter solution. I think I'd like to replace some of the existing filters that currently hit the API with frontend code and replace those with proper filters too. These are mostly the project/version listing elements. * Add filter for organization dropdown This replaces an API driven UI element. It's not important that these UI filters are frontend, it was just easier than figuring out how to make django-filter work for this use case at that time. * Fix the team member filter yet again * Use a custom filter field to execute filter set queryset method * Add tests organization filter sets * Update code comments * A few more improvements - Make view code nicer, use django_filters.views.FilterMixin, sort of - Use FilterSet.is_valid() to give empty list on validation errors - Clean up tests with standard fixtures instead * Review updates for tests and arguments * Rename FilterMixin -> FilterContextMixin * Fix lint
1 parent 8784787 commit ea6503e

File tree

5 files changed

+931
-27
lines changed

5 files changed

+931
-27
lines changed

readthedocs/core/filters.py

+81
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
"""Extended classes for django-filter."""
2+
3+
from django_filters import ModelChoiceFilter, views
4+
5+
6+
class FilteredModelChoiceFilter(ModelChoiceFilter):
7+
8+
"""
9+
A model choice field for customizing choice querysets at initialization.
10+
11+
This extends the model choice field queryset lookup to include executing a
12+
method from the parent filter set. Normally, ModelChoiceFilter will use the
13+
underlying model manager ``all()`` method to populate choices. This of
14+
course is not correct as we need to worry about permissions to organizations
15+
and teams.
16+
17+
Using a method on the parent filterset, the queryset can be filtered using
18+
attributes on the FilterSet instance, which for now is view time parameters
19+
like ``organization``.
20+
21+
Additional parameters from this class:
22+
23+
:param queryset_method: Name of method on parent FilterSet to call to build
24+
queryset for choice population.
25+
:type queryset_method: str
26+
"""
27+
28+
def __init__(self, *args, **kwargs):
29+
self.queryset_method = kwargs.pop("queryset_method", None)
30+
super().__init__(*args, **kwargs)
31+
32+
def get_queryset(self, request):
33+
if self.queryset_method:
34+
fn = getattr(self.parent, self.queryset_method, None)
35+
if not callable(fn):
36+
raise ValueError(f"Method {self.queryset_method} is not callable")
37+
return fn()
38+
return super().get_queryset(request)
39+
40+
41+
class FilterContextMixin(views.FilterMixin):
42+
43+
"""
44+
Django-filter filterset mixin class for context data.
45+
46+
Django-filter gives two classes for constructing views:
47+
48+
- :py:class:`~django_filters.views.BaseFilterView`
49+
- :py:class:`~django_filters.views.FilterMixin`
50+
51+
These aren't quite yet usable, as some of our views still support our legacy
52+
dashboard. For now, this class will aim to be an intermediate step. It will
53+
expect these methods to be called from ``get_context_data()``, but will
54+
maintain some level of compatibility with the native mixin/view classes.
55+
"""
56+
57+
def get_filterset(self, **kwargs):
58+
"""
59+
Construct filterset for view.
60+
61+
This does not automatically execute like it would with BaseFilterView.
62+
Instead, this should be called directly from ``get_context_data()``.
63+
Unlike the parent methods, this method can be used to pass arguments
64+
directly to the ``FilterSet.__init__``.
65+
66+
:param kwargs: Arguments to pass to ``FilterSet.__init__``
67+
"""
68+
# This method overrides the method from FilterMixin with differing
69+
# arguments. We can switch this later if we ever resturcture the view
70+
# pylint: disable=arguments-differ
71+
if not getattr(self, "filterset", None):
72+
filterset_class = self.get_filterset_class()
73+
all_kwargs = self.get_filterset_kwargs(filterset_class)
74+
all_kwargs.update(kwargs)
75+
self.filterset = filterset_class(**all_kwargs)
76+
return self.filterset
77+
78+
def get_filtered_queryset(self):
79+
if self.filterset.is_valid() or not self.get_strict():
80+
return self.filterset.qs
81+
return self.filterset.queryset.none()

readthedocs/organizations/filters.py

+181-5
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,51 @@
1-
"""Filters used in organization dashboard."""
1+
"""Filters used in the organization dashboard views."""
22

33
import structlog
44
from django.db.models import F
5-
from django.forms.widgets import HiddenInput
65
from django.utils.translation import gettext_lazy as _
7-
from django_filters import CharFilter, FilterSet, OrderingFilter
6+
from django_filters import ChoiceFilter, FilterSet, OrderingFilter
7+
8+
from readthedocs.core.filters import FilteredModelChoiceFilter
9+
from readthedocs.organizations.constants import ACCESS_LEVELS
10+
from readthedocs.organizations.models import Organization, Team
811

912
log = structlog.get_logger(__name__)
1013

1114

15+
class OrganizationFilterSet(FilterSet):
16+
17+
"""
18+
Organization base filter set.
19+
20+
Adds some methods that are used for organization related queries and common
21+
base querysets for filter fields.
22+
23+
Note, the querysets here are also found in the organization base views and
24+
mixin classes. These are redefined here instead of passing in the querysets
25+
from the view.
26+
27+
:param organization: Organization instance for current view
28+
"""
29+
30+
def __init__(self, *args, organization=None, **kwargs):
31+
self.organization = organization
32+
super().__init__(*args, **kwargs)
33+
34+
def get_organization_queryset(self):
35+
return Organization.objects.for_user(user=self.request.user)
36+
37+
def get_team_queryset(self):
38+
return Team.objects.member(
39+
self.request.user,
40+
organization=self.organization,
41+
).prefetch_related("organization")
42+
43+
def is_valid(self):
44+
# This differs from the default logic as we want to consider unbound
45+
# data as a valid filterset state.
46+
return (self.is_bound is False) or self.form.is_valid()
47+
48+
1249
class OrganizationSortOrderingFilter(OrderingFilter):
1350

1451
"""Organization list sort ordering django_filters filter."""
@@ -53,13 +90,152 @@ def filter(self, qs, value):
5390
return qs.order_by(*order_bys)
5491

5592

56-
class OrganizationListFilterSet(FilterSet):
93+
class OrganizationListFilterSet(OrganizationFilterSet):
5794

5895
"""Filter and sorting for organization listing page."""
5996

60-
slug = CharFilter(field_name="slug", widget=HiddenInput)
97+
slug = FilteredModelChoiceFilter(
98+
label=_("Organization"),
99+
empty_label=_("All organizations"),
100+
to_field_name="slug",
101+
queryset_method="get_organization_queryset",
102+
method="get_organization",
103+
)
61104

62105
sort = OrganizationSortOrderingFilter(
63106
field_name="sort",
64107
label=_("Sort by"),
65108
)
109+
110+
def get_organization(self, queryset, field_name, organization):
111+
return queryset.filter(slug=organization.slug)
112+
113+
114+
class OrganizationProjectListFilterSet(OrganizationFilterSet):
115+
116+
"""
117+
Filter and sorting set for organization project listing page.
118+
119+
This filter set creates the following filters in the organization project
120+
listing UI:
121+
122+
Project
123+
A list of project names that the user has permissions to, using ``slug``
124+
as a lookup field. This is used when linking directly to a project in
125+
this filter list, and also for quick lookup in the list UI.
126+
127+
Team
128+
A list of team names that the user has access to, using ``slug`` as a
129+
lookup field. This filter is linked to directly by the team listing
130+
view, as a shortcut for listing projects managed by the team.
131+
"""
132+
133+
slug = FilteredModelChoiceFilter(
134+
label=_("Project"),
135+
empty_label=_("All projects"),
136+
to_field_name="slug",
137+
queryset_method="get_project_queryset",
138+
method="get_project",
139+
)
140+
141+
teams__slug = FilteredModelChoiceFilter(
142+
label=_("Team"),
143+
empty_label=_("All teams"),
144+
field_name="teams",
145+
to_field_name="slug",
146+
queryset_method="get_team_queryset",
147+
)
148+
149+
def get_project_queryset(self):
150+
return self.queryset
151+
152+
def get_project(self, queryset, field_name, project):
153+
return queryset.filter(slug=project.slug)
154+
155+
156+
class OrganizationTeamListFilterSet(OrganizationFilterSet):
157+
158+
"""
159+
Filter and sorting for organization team listing page.
160+
161+
This filter set creates the following filters in the organization team
162+
listing UI:
163+
164+
Team
165+
A list of team names that the user has access to, using ``slug`` as a
166+
lookup field. This filter is used mostly for direct linking to a
167+
specific team in the listing UI, but can be used for quick filtering
168+
with the dropdown too.
169+
"""
170+
171+
slug = FilteredModelChoiceFilter(
172+
label=_("Team"),
173+
empty_label=_("All teams"),
174+
field_name="teams",
175+
to_field_name="slug",
176+
queryset_method="get_team_queryset",
177+
method="get_team",
178+
)
179+
180+
def get_team_queryset(self):
181+
return self.queryset
182+
183+
def get_team(self, queryset, field_name, team):
184+
return queryset.filter(slug=team.slug)
185+
186+
187+
class OrganizationTeamMemberListFilterSet(OrganizationFilterSet):
188+
189+
"""
190+
Filter and sorting set for organization member listing page.
191+
192+
This filter set's underlying queryset from the member listing view is the
193+
manager method ``Organization.members``. The model described in this filter
194+
is effectively ``User``, but through a union of ``TeamMembers.user`` and
195+
``Organizations.owners``.
196+
197+
This filter set will result in the following filters in the UI:
198+
199+
Team
200+
A list of ``Team`` names, using ``Team.slug`` as the lookup field. This
201+
is linked to directly from the team listing page, to show the users that
202+
are members of a particular team.
203+
204+
Access
205+
This is an extension of ``Team.access`` in a way, but with a new option
206+
(``ACCESS_OWNER``) to describe ownership privileges through organization
207+
ownership.
208+
209+
Our modeling is not ideal here, so instead of aiming for model purity and
210+
a confusing UI/UX, this aims for hiding confusing modeling from the user
211+
with clear UI/UX. Otherwise, two competing filters are required for "user
212+
has privileges granted through a team" and "user has privileges granted
213+
through ownership".
214+
"""
215+
216+
ACCESS_OWNER = "owner"
217+
218+
teams__slug = FilteredModelChoiceFilter(
219+
label=_("Team"),
220+
empty_label=_("All teams"),
221+
field_name="teams",
222+
to_field_name="slug",
223+
queryset_method="get_team_queryset",
224+
)
225+
226+
access = ChoiceFilter(
227+
label=_("Access"),
228+
empty_label=_("All access levels"),
229+
choices=ACCESS_LEVELS + ((ACCESS_OWNER, _("Owner")),),
230+
method="get_access",
231+
)
232+
233+
def get_access(self, queryset, field_name, value):
234+
# Note: the queryset here is effectively against the ``User`` model, and
235+
# is from Organization.members, a union of TeamMember.user and
236+
# Organization.owners.
237+
if value == self.ACCESS_OWNER:
238+
return queryset.filter(owner_organizations=self.organization)
239+
if value is not None:
240+
return queryset.filter(teams__access=value)
241+
return queryset

0 commit comments

Comments
 (0)