Skip to content

Commit

Permalink
Add organization view UI filters (#10847)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
agjohnson authored Nov 1, 2023
1 parent 8784787 commit ea6503e
Show file tree
Hide file tree
Showing 5 changed files with 931 additions and 27 deletions.
81 changes: 81 additions & 0 deletions readthedocs/core/filters.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
"""Extended classes for django-filter."""

from django_filters import ModelChoiceFilter, views


class FilteredModelChoiceFilter(ModelChoiceFilter):

"""
A model choice field for customizing choice querysets at initialization.
This extends the model choice field queryset lookup to include executing a
method from the parent filter set. Normally, ModelChoiceFilter will use the
underlying model manager ``all()`` method to populate choices. This of
course is not correct as we need to worry about permissions to organizations
and teams.
Using a method on the parent filterset, the queryset can be filtered using
attributes on the FilterSet instance, which for now is view time parameters
like ``organization``.
Additional parameters from this class:
:param queryset_method: Name of method on parent FilterSet to call to build
queryset for choice population.
:type queryset_method: str
"""

def __init__(self, *args, **kwargs):
self.queryset_method = kwargs.pop("queryset_method", None)
super().__init__(*args, **kwargs)

def get_queryset(self, request):
if self.queryset_method:
fn = getattr(self.parent, self.queryset_method, None)
if not callable(fn):
raise ValueError(f"Method {self.queryset_method} is not callable")
return fn()
return super().get_queryset(request)


class FilterContextMixin(views.FilterMixin):

"""
Django-filter filterset mixin class for context data.
Django-filter gives two classes for constructing views:
- :py:class:`~django_filters.views.BaseFilterView`
- :py:class:`~django_filters.views.FilterMixin`
These aren't quite yet usable, as some of our views still support our legacy
dashboard. For now, this class will aim to be an intermediate step. It will
expect these methods to be called from ``get_context_data()``, but will
maintain some level of compatibility with the native mixin/view classes.
"""

def get_filterset(self, **kwargs):
"""
Construct filterset for view.
This does not automatically execute like it would with BaseFilterView.
Instead, this should be called directly from ``get_context_data()``.
Unlike the parent methods, this method can be used to pass arguments
directly to the ``FilterSet.__init__``.
:param kwargs: Arguments to pass to ``FilterSet.__init__``
"""
# This method overrides the method from FilterMixin with differing
# arguments. We can switch this later if we ever resturcture the view
# pylint: disable=arguments-differ
if not getattr(self, "filterset", None):
filterset_class = self.get_filterset_class()
all_kwargs = self.get_filterset_kwargs(filterset_class)
all_kwargs.update(kwargs)
self.filterset = filterset_class(**all_kwargs)
return self.filterset

def get_filtered_queryset(self):
if self.filterset.is_valid() or not self.get_strict():
return self.filterset.qs
return self.filterset.queryset.none()
186 changes: 181 additions & 5 deletions readthedocs/organizations/filters.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,51 @@
"""Filters used in organization dashboard."""
"""Filters used in the organization dashboard views."""

import structlog
from django.db.models import F
from django.forms.widgets import HiddenInput
from django.utils.translation import gettext_lazy as _
from django_filters import CharFilter, FilterSet, OrderingFilter
from django_filters import ChoiceFilter, FilterSet, OrderingFilter

from readthedocs.core.filters import FilteredModelChoiceFilter
from readthedocs.organizations.constants import ACCESS_LEVELS
from readthedocs.organizations.models import Organization, Team

log = structlog.get_logger(__name__)


class OrganizationFilterSet(FilterSet):

"""
Organization base filter set.
Adds some methods that are used for organization related queries and common
base querysets for filter fields.
Note, the querysets here are also found in the organization base views and
mixin classes. These are redefined here instead of passing in the querysets
from the view.
:param organization: Organization instance for current view
"""

def __init__(self, *args, organization=None, **kwargs):
self.organization = organization
super().__init__(*args, **kwargs)

def get_organization_queryset(self):
return Organization.objects.for_user(user=self.request.user)

def get_team_queryset(self):
return Team.objects.member(
self.request.user,
organization=self.organization,
).prefetch_related("organization")

def is_valid(self):
# This differs from the default logic as we want to consider unbound
# data as a valid filterset state.
return (self.is_bound is False) or self.form.is_valid()


class OrganizationSortOrderingFilter(OrderingFilter):

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


class OrganizationListFilterSet(FilterSet):
class OrganizationListFilterSet(OrganizationFilterSet):

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

slug = CharFilter(field_name="slug", widget=HiddenInput)
slug = FilteredModelChoiceFilter(
label=_("Organization"),
empty_label=_("All organizations"),
to_field_name="slug",
queryset_method="get_organization_queryset",
method="get_organization",
)

sort = OrganizationSortOrderingFilter(
field_name="sort",
label=_("Sort by"),
)

def get_organization(self, queryset, field_name, organization):
return queryset.filter(slug=organization.slug)


class OrganizationProjectListFilterSet(OrganizationFilterSet):

"""
Filter and sorting set for organization project listing page.
This filter set creates the following filters in the organization project
listing UI:
Project
A list of project names that the user has permissions to, using ``slug``
as a lookup field. This is used when linking directly to a project in
this filter list, and also for quick lookup in the list UI.
Team
A list of team names that the user has access to, using ``slug`` as a
lookup field. This filter is linked to directly by the team listing
view, as a shortcut for listing projects managed by the team.
"""

slug = FilteredModelChoiceFilter(
label=_("Project"),
empty_label=_("All projects"),
to_field_name="slug",
queryset_method="get_project_queryset",
method="get_project",
)

teams__slug = FilteredModelChoiceFilter(
label=_("Team"),
empty_label=_("All teams"),
field_name="teams",
to_field_name="slug",
queryset_method="get_team_queryset",
)

def get_project_queryset(self):
return self.queryset

def get_project(self, queryset, field_name, project):
return queryset.filter(slug=project.slug)


class OrganizationTeamListFilterSet(OrganizationFilterSet):

"""
Filter and sorting for organization team listing page.
This filter set creates the following filters in the organization team
listing UI:
Team
A list of team names that the user has access to, using ``slug`` as a
lookup field. This filter is used mostly for direct linking to a
specific team in the listing UI, but can be used for quick filtering
with the dropdown too.
"""

slug = FilteredModelChoiceFilter(
label=_("Team"),
empty_label=_("All teams"),
field_name="teams",
to_field_name="slug",
queryset_method="get_team_queryset",
method="get_team",
)

def get_team_queryset(self):
return self.queryset

def get_team(self, queryset, field_name, team):
return queryset.filter(slug=team.slug)


class OrganizationTeamMemberListFilterSet(OrganizationFilterSet):

"""
Filter and sorting set for organization member listing page.
This filter set's underlying queryset from the member listing view is the
manager method ``Organization.members``. The model described in this filter
is effectively ``User``, but through a union of ``TeamMembers.user`` and
``Organizations.owners``.
This filter set will result in the following filters in the UI:
Team
A list of ``Team`` names, using ``Team.slug`` as the lookup field. This
is linked to directly from the team listing page, to show the users that
are members of a particular team.
Access
This is an extension of ``Team.access`` in a way, but with a new option
(``ACCESS_OWNER``) to describe ownership privileges through organization
ownership.
Our modeling is not ideal here, so instead of aiming for model purity and
a confusing UI/UX, this aims for hiding confusing modeling from the user
with clear UI/UX. Otherwise, two competing filters are required for "user
has privileges granted through a team" and "user has privileges granted
through ownership".
"""

ACCESS_OWNER = "owner"

teams__slug = FilteredModelChoiceFilter(
label=_("Team"),
empty_label=_("All teams"),
field_name="teams",
to_field_name="slug",
queryset_method="get_team_queryset",
)

access = ChoiceFilter(
label=_("Access"),
empty_label=_("All access levels"),
choices=ACCESS_LEVELS + ((ACCESS_OWNER, _("Owner")),),
method="get_access",
)

def get_access(self, queryset, field_name, value):
# Note: the queryset here is effectively against the ``User`` model, and
# is from Organization.members, a union of TeamMember.user and
# Organization.owners.
if value == self.ACCESS_OWNER:
return queryset.filter(owner_organizations=self.organization)
if value is not None:
return queryset.filter(teams__access=value)
return queryset
Loading

0 comments on commit ea6503e

Please sign in to comment.