-
Notifications
You must be signed in to change notification settings - Fork 20
Fix planned maintenance filter dropdown #1960
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
base: main
Are you sure you want to change the base?
Changes from all commits
7ed1d23
feb2653
4ac3067
c605684
f134168
c195a69
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Fixed planned maintenance filter selection freezing the drop-down |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,8 @@ | ||
| from django import forms | ||
| from django.core.exceptions import PermissionDenied | ||
| from django.db.models import Q | ||
| from django.db.models import Case, IntegerField, When | ||
| from django.forms import modelform_factory | ||
| from django.http import HttpResponseRedirect, JsonResponse | ||
| from django.http import HttpResponse, HttpResponseRedirect | ||
| from django.shortcuts import render | ||
| from django.urls import reverse | ||
| from django.utils import timezone | ||
|
|
@@ -84,15 +84,22 @@ class FilterWidgetMixin: | |
|
|
||
| def get_filter_widget(self): | ||
| return SearchDropdownMultiSelect( | ||
| partial_get=reverse("htmx:search-filters"), | ||
| partial_get=reverse("htmx:plannedmaintenance-filter-widget"), | ||
| attrs={"placeholder": "Select filters..."}, | ||
| extra={"search_placeholder": "Search by filter name or user..."}, | ||
| extra={ | ||
| "search_placeholder": "Search by filter name or user...", | ||
| "preload": True, | ||
| }, | ||
| ) | ||
|
|
||
| def get_form(self, form_class=None): | ||
| form = super().get_form(form_class) | ||
| if "filters" in form.fields: | ||
| form.fields["filters"].queryset = Filter.objects.select_related("user") | ||
| form.fields["filters"].queryset = ( | ||
| Filter.objects.select_related("user") | ||
| .annotate(own=Case(When(user=self.request.user, then=0), default=1, output_field=IntegerField())) | ||
| .order_by("own", "name") | ||
| ) | ||
| form.fields["filters"].label_from_instance = lambda obj: f"{obj.name} ({obj.user.username})" | ||
| if "end_time" in form.fields: | ||
| form.fields["end_time"].required = False | ||
|
|
@@ -180,25 +187,23 @@ def form_valid(self, form): | |
| return super().form_valid(form) | ||
|
|
||
|
|
||
| class SearchFiltersView(UserIsStaffMixin, View): | ||
| class FilterWidgetPartialView(UserIsStaffMixin, View): | ||
| http_method_names = ["get"] | ||
|
|
||
| def get(self, request): | ||
| query = request.GET.get("q", "") | ||
|
|
||
| filters = Filter.objects.select_related("user") | ||
| if query: | ||
| filters = filters.filter( | ||
| Q(name__icontains=query) | ||
| | Q(user__username__icontains=query) | ||
| | Q(user__first_name__icontains=query) | ||
| | Q(user__last_name__icontains=query) | ||
| ) | ||
| filters = sorted(filters, key=lambda f: (f.user != request.user, f.name))[:20] | ||
| filter_ids = [fid for fid in request.GET.getlist("filters") if fid.isdigit()] | ||
| selected_filters = Filter.objects.filter(pk__in=filter_ids).select_related("user") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The previous code showed the user's own filters, and sorted them on users. This does not. Why?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was probably forgotten in some iteration, I re-added the functionality in the new version and added a test for the future. |
||
|
|
||
| options = [{"id": f.pk, "text": f"{f.name} ({f.user.username})"} for f in filters] | ||
| class FiltersForm(forms.Form): | ||
| filters = forms.ModelMultipleChoiceField( | ||
| queryset=selected_filters, | ||
| widget=FilterWidgetMixin().get_filter_widget(), | ||
| required=False, | ||
| ) | ||
|
|
||
| return JsonResponse({"results": options}) | ||
| form = FiltersForm(request.GET) | ||
| form.fields["filters"].label_from_instance = lambda obj: f"{obj.name} ({obj.user.username})" | ||
| return HttpResponse(str(form["filters"])) | ||
|
|
||
|
|
||
| class FilterPreviewView(UserIsStaffMixin, View): | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow-up: validate with a form. (Allows for standardized error handling/logging in case somebody do want to use this for SQL injection or something.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That seems reasonable, I made a follow-up issue so that can be done in isolation.