Skip to content

Commit 23ae363

Browse files
committed
Use selectize for author/reviewer filter for patch lists
This has a few benefits: 1. It makes it much easier to select a user, since you don't have to scroll through the very long list anymore. 2. It makes the pageload faster, because the full user list is not part of the generated html. 3. No heavy query to get all the users. Based on some previous quick attempts, this is expected to make the page load significantly faster. By doing this I also realized that the Author multiselect for patch create/edit was showing "username (firstname lastname)" instead of "firstname lastname (username)" like we do everywhere else. So this fixes that too.
1 parent 80eae00 commit 23ae363

File tree

9 files changed

+134
-112
lines changed

9 files changed

+134
-112
lines changed

pgcommitfest/commitfest/forms.py

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
from django import forms
22
from django.contrib.auth.models import User
3-
from django.db.models import Q
43
from django.forms import ValidationError
54
from django.forms.widgets import HiddenInput
65
from django.http import Http404
@@ -11,39 +10,57 @@
1110

1211

1312
class CommitFestFilterForm(forms.Form):
13+
selectize_fields = {
14+
"author": "/lookups/user",
15+
"reviewer": "/lookups/user",
16+
}
17+
1418
text = forms.CharField(max_length=50, required=False)
1519
status = forms.ChoiceField(required=False)
1620
targetversion = forms.ChoiceField(required=False)
17-
author = forms.ChoiceField(required=False)
18-
reviewer = forms.ChoiceField(required=False)
21+
author = forms.ChoiceField(required=False, label="Author (type to search)")
22+
reviewer = forms.ChoiceField(required=False, label="Reviewer (type to search)")
1923
sortkey = forms.IntegerField(required=False)
2024

21-
def __init__(self, cf, *args, **kwargs):
22-
super(CommitFestFilterForm, self).__init__(*args, **kwargs)
25+
def __init__(self, data, *args, **kwargs):
26+
super(CommitFestFilterForm, self).__init__(data, *args, **kwargs)
2327

2428
self.fields["sortkey"].widget = forms.HiddenInput()
2529

2630
c = [(-1, "* All")] + list(PatchOnCommitFest._STATUS_CHOICES)
2731
self.fields["status"] = forms.ChoiceField(choices=c, required=False)
2832

29-
if cf:
30-
q = Q(patch_author__commitfests=cf) | Q(patch_reviewer__commitfests=cf)
31-
else:
32-
q = Q()
33-
userchoices = [(-1, "* All"), (-2, "* None"), (-3, "* Yourself")] + [
34-
(u.id, "%s %s (%s)" % (u.first_name, u.last_name, u.username))
35-
for u in User.objects.filter(q)
36-
.distinct()
37-
.order_by("first_name", "last_name")
38-
]
33+
userchoices = [(-1, "* All"), (-2, "* None"), (-3, "* Yourself")]
34+
35+
print(data)
36+
selected_user_ids = set()
37+
if data and "author" in data:
38+
try:
39+
selected_user_ids.add(int(data["author"]))
40+
except ValueError:
41+
pass
42+
43+
if data and "reviewer" in data:
44+
try:
45+
selected_user_ids.add(int(data["reviewer"]))
46+
except ValueError:
47+
pass
48+
49+
if selected_user_ids:
50+
userchoices.extend(
51+
(u.id, f"{u.first_name} {u.last_name} ({u.username})")
52+
for u in User.objects.filter(pk__in=selected_user_ids)
53+
)
54+
print(userchoices)
55+
3956
self.fields["targetversion"] = forms.ChoiceField(
4057
choices=[("-1", "* All"), ("-2", "* None")]
4158
+ [(v.id, v.version) for v in TargetVersion.objects.all()],
4259
required=False,
4360
label="Target version",
4461
)
45-
self.fields["author"] = forms.ChoiceField(choices=userchoices, required=False)
46-
self.fields["reviewer"] = forms.ChoiceField(choices=userchoices, required=False)
62+
self.fields["author"].choices = userchoices
63+
self.fields["reviewer"].choices = userchoices
4764

4865
for f in (
4966
"status",
@@ -54,7 +71,7 @@ def __init__(self, cf, *args, **kwargs):
5471

5572

5673
class PatchForm(forms.ModelForm):
57-
selectize_multiple_fields = {
74+
selectize_fields = {
5875
"authors": "/lookups/user",
5976
"reviewers": "/lookups/user",
6077
}
@@ -80,7 +97,7 @@ def __init__(self, *args, **kwargs):
8097
)
8198

8299
# Selectize multiple fields -- don't pre-populate everything
83-
for field, url in list(self.selectize_multiple_fields.items()):
100+
for field, url in list(self.selectize_fields.items()):
84101
# If this is a postback of a selectize field, it may contain ids that are not currently
85102
# stored in the field. They must still be among the *allowed* values of course, which
86103
# are handled by the existing queryset on the field.
@@ -97,8 +114,8 @@ def __init__(self, *args, **kwargs):
97114
self.fields[field].queryset = self.fields[field].queryset.filter(
98115
pk__in=set(vals)
99116
)
100-
self.fields[field].label_from_instance = lambda u: "{} ({})".format(
101-
u.username, u.get_full_name()
117+
self.fields[field].label_from_instance = (
118+
lambda u: f"{u.get_full_name()} ({u.username})"
102119
)
103120

104121
# Only allow modifying reviewers and committers if the patch is closed.

pgcommitfest/commitfest/lookups.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ def userlookup(request):
2525
"values": [
2626
{
2727
"id": u.id,
28-
"value": "{} ({})".format(u.username, u.get_full_name()),
28+
"value": f"{u.get_full_name()} ({u.username})",
2929
}
3030
for u in users
3131
],

pgcommitfest/commitfest/templates/base_form.html

Lines changed: 4 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
<div class="alert alert-danger">{{e}}</div>
2323
{%endfor%}
2424
{%endif%}
25-
{%if not field.name in form.selectize_multiple_fields%}{{field|field_class:"form-control"}}{%else%}{{field}}{%endif%}
25+
{%if not field.name in form.selectize_fields%}{{field|field_class:"form-control"}}{%else%}{{field}}{%endif%}
2626
{%if field.help_text%}<br/>{{field.help_text|safe}}{%endif%}</div>
2727
</div>
2828
{%else%}
@@ -71,34 +71,12 @@ <h3>Search user</h3>
7171
{%endblock%}
7272

7373
{%block extrahead%}
74-
<link rel="stylesheet" href="/media/commitfest/css/selectize.css" />
75-
<link rel="stylesheet" href="/media/commitfest/css/selectize.default.css" />
74+
{%include "selectize_css.html" %}
7675
{%endblock%}
7776
{%block morescript%}
78-
<script src="/media/commitfest/js/selectize.min.js"></script>
77+
78+
{%include "selectize_js.html" %}
7979
<script>
80-
/* Set up selectize fields */
81-
{% for f, url in selectize_multiple_fields %}
82-
$('#id_{{f}}').selectize({
83-
plugins: ['remove_button'],
84-
valueField: 'id',
85-
labelField: 'value',
86-
searchField: 'value',
87-
load: function(query, callback) {
88-
if (!query.length) return callback();
89-
$.ajax({
90-
'url': '{{url}}',
91-
'type': 'GET',
92-
'dataType': 'json',
93-
'data': {
94-
'query': query,
95-
},
96-
'error': function() { callback();},
97-
'success': function(res) { callback(res.values);},
98-
});
99-
}
100-
});
101-
{%endfor%}
10280
{%if user.is_staff%}
10381
$('.selectize-control').after(
10482
$('<a href="#" class="btn btn-default btn-sm">Import user not listed</a>').click(function () {

pgcommitfest/commitfest/templates/commitfest.html

Lines changed: 5 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -14,36 +14,7 @@
1414
{%if cf.isopen or user.is_staff %}
1515
<a class="btn btn-default" href="new/">New patch</a>
1616
{%endif%}
17-
<div id="collapseFilters" class="collapse {%if has_filter%}in{%endif%}">
18-
<form id="filterform" method="GET" action="." style="margin-bottom: 0px">
19-
<table class="table table-condensed" style="margin-bottom: 0px">
20-
<thead>
21-
<tr>
22-
{%for f in form%}
23-
{%if not f.is_hidden%}
24-
<td>{{f.label}}</td>
25-
{%else%}
26-
<td></td>
27-
{%endif%}
28-
{%endfor%}
29-
{%comment%} Add one extra for buttons {%endcomment%}
30-
<td></td>
31-
</tr>
32-
</thead>
33-
<tbody>
34-
<tr>
35-
{%for f in form%}
36-
<td>{{f|field_class:"form-control"}}</td>
37-
{%endfor%}
38-
<td>
39-
<input type="submit" class="btn btn-default" value="Filter">
40-
<a class="btn btn-default" href=".">Clear</a>
41-
</td>
42-
</tr>
43-
</tbody>
44-
</table>
45-
</form>
46-
</div>
17+
{%include "filter_form.html" %}
4718

4819
<p>
4920
<br/>
@@ -160,7 +131,11 @@ <h3>{{p.is_open|yesno:"Active patches,Closed patches"}}</h3>
160131
</div>
161132
{%endblock%}
162133

134+
{%block extrahead%}
135+
{%include "selectize_css.html" %}
136+
{%endblock%}
163137
{%block morescript%}
138+
{%include "selectize_js.html" %}
164139
<script>
165140
{%if user.is_staff%}
166141
function send_selected() {
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
{%load commitfest %}
2+
<div id="collapseFilters" class="collapse {%if has_filter%}in{%endif%}">
3+
<form id="filterform" method="GET" action="." style="margin-bottom: 0px">
4+
<table class="table table-condensed" style="margin-bottom: 0px">
5+
<thead>
6+
<tr>
7+
{%for f in form%}
8+
{%if not f.is_hidden%}
9+
<td>{{f.label}}</td>
10+
{%else%}
11+
<td></td>
12+
{%endif%}
13+
{%endfor%}
14+
{%comment%} Add one extra for buttons {%endcomment%}
15+
<td></td>
16+
</tr>
17+
</thead>
18+
<tbody>
19+
<tr>
20+
{%for f in form%}
21+
<td>
22+
{%if not f.name in form.selectize_fields%}{{f|field_class:"form-control"}}{%else%}{{f}}{%endif%}
23+
</td>
24+
{%endfor%}
25+
<td>
26+
<input type="submit" class="btn btn-default" value="Filter">
27+
<a class="btn btn-default" href=".">Clear</a>
28+
</td>
29+
</tr>
30+
</tbody>
31+
</table>
32+
</form>
33+
</div>

pgcommitfest/commitfest/templates/home.html

Lines changed: 7 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -12,36 +12,7 @@
1212
</div>
1313
<button type="submit" class="btn btn-default">Search</button>
1414
</form>
15-
<div id="collapseFilters" class="collapse {%if has_filter%}in{%endif%}">
16-
<form id="filterform" method="GET" action="." style="margin-bottom: 0px">
17-
<table class="table table-condensed" style="margin-bottom: 0px">
18-
<thead>
19-
<tr>
20-
{%for f in form%}
21-
{%if not f.is_hidden%}
22-
<td>{{f.label}}</td>
23-
{%else%}
24-
<td></td>
25-
{%endif%}
26-
{%endfor%}
27-
{%comment%} Add one extra for buttons {%endcomment%}
28-
<td></td>
29-
</tr>
30-
</thead>
31-
<tbody>
32-
<tr>
33-
{%for f in form%}
34-
<td>{{f|field_class:"form-control"}}</td>
35-
{%endfor%}
36-
<td>
37-
<input type="submit" class="btn btn-default" value="Filter">
38-
<a class="btn btn-default" href=".">Clear</a>
39-
</td>
40-
</tr>
41-
</tbody>
42-
</table>
43-
</form>
44-
</div>
15+
{%include "filter_form.html" %}
4516
{%for p in patches %}
4617
{%ifchanged p.is_open%}
4718
{%if not forloop.first%}
@@ -133,3 +104,9 @@ <h3>{%if user.is_authenticated%}Open patches you are subscribed to{%elif p.is_op
133104
{%endif%}
134105
{%endfor%}
135106
{%endblock%}
107+
{%block extrahead%}
108+
{%include "selectize_css.html" %}
109+
{%endblock%}
110+
{%block morescript%}
111+
{%include "selectize_js.html" %}
112+
{%endblock%}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<link rel="stylesheet" href="/media/commitfest/css/selectize.css" />
2+
<link rel="stylesheet" href="/media/commitfest/css/selectize.default.css" />
3+
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<script src="/media/commitfest/js/selectize.min.js"></script>
2+
<script>
3+
{% for f, url in form.selectize_fields.items %}
4+
$('#id_{{f}}').selectize({
5+
plugins: ['remove_button'],
6+
valueField: 'id',
7+
labelField: 'value',
8+
searchField: 'value',
9+
load: function(query, callback) {
10+
if (!query.length) return callback();
11+
$.ajax({
12+
'url': '{{url}}',
13+
'type': 'GET',
14+
'dataType': 'json',
15+
'data': {
16+
'query': query,
17+
},
18+
'error': function() { callback();},
19+
'success': function(res) { callback(res.values);},
20+
});
21+
},
22+
onFocus: function() {
23+
if (this.$input.is('[multiple]')) {
24+
return;
25+
}
26+
console.log(this.getValue());
27+
this.lastValue = this.getValue();
28+
this.clear(false);
29+
},
30+
onBlur: function() {
31+
if (this.$input.is('[multiple]')) {
32+
return;
33+
}
34+
if(this.getValue() == '') {
35+
this.setValue(this.lastValue);
36+
}
37+
},
38+
});
39+
{%endfor%}
40+
</script>
41+

pgcommitfest/commitfest/views.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ def home(request):
5656

5757
# Generates a fairly expensive query, which we shouldn't do unless
5858
# the user is logged in. XXX: Figure out how to avoid doing that..
59-
form = CommitFestFilterForm(None, request.GET)
59+
form = CommitFestFilterForm(request.GET)
6060

6161
if request.user.is_authenticated:
6262
patch_list = patchlist(request, cf, personalized=True)
@@ -504,7 +504,7 @@ def commitfest(request, cfid):
504504

505505
# Generates a fairly expensive query, which we shouldn't do unless
506506
# the user is logged in. XXX: Figure out how to avoid doing that..
507-
form = CommitFestFilterForm(cf, request.GET)
507+
form = CommitFestFilterForm(request.GET)
508508

509509
return render(
510510
request,
@@ -707,7 +707,6 @@ def patchform(request, patchid):
707707
"form": form,
708708
"patch": patch,
709709
"title": "Edit patch",
710-
"selectize_multiple_fields": form.selectize_multiple_fields.items(),
711710
"breadcrumbs": [
712711
{"title": cf.title, "href": "/%s/" % cf.pk},
713712
{"title": "View patch", "href": "/%s/%s/" % (cf.pk, patch.pk)},
@@ -766,7 +765,6 @@ def newpatch(request, cfid):
766765
{"title": cf.title, "href": "/%s/" % cf.pk},
767766
],
768767
"savebutton": "Create patch",
769-
"selectize_multiple_fields": form.selectize_multiple_fields.items(),
770768
"threadbrowse": True,
771769
},
772770
)

0 commit comments

Comments
 (0)