diff --git a/media/commitfest/js/change_tag.js b/media/commitfest/js/change_tag.js new file mode 100644 index 00000000..ae4c224a --- /dev/null +++ b/media/commitfest/js/change_tag.js @@ -0,0 +1,44 @@ +// An input validator for the color picker. Points out low-contrast tag color +// choices. +const input = document.getElementById("id_color"); +input.addEventListener("input", (event) => { + // Don't do anything if the color code doesn't pass default validity. + input.setCustomValidity(""); + if (!input.validity.valid) { + return; + } + + // Break the #rrggbb color code into RGB components. + color = parseInt(input.value.substr(1), 16); + red = ((color & 0xFF0000) >> 16) / 255.; + green = ((color & 0x00FF00) >> 8) / 255.; + blue = (color & 0x0000FF) / 255.; + + // Compare the contrast ratio against white. All the magic math comes from + // Web Content Accessibility Guidelines (WCAG) 2.2, Technique G18: + // + // https://www.w3.org/WAI/WCAG22/Techniques/general/G18.html + // + function l(val) { + if (val <= 0.04045) { + return val / 12.92; + } + return ((val + 0.055) / 1.055) ** 2.4; + } + + lum = 0.2126 * l(red) + 0.7152 * l(green) + 0.0722 * l(blue); + contrast = (1 + 0.05) / (lum + 0.05); + + // Complain if we're below WCAG 2.2 recommendations. + if (contrast < 4.5) { + input.setCustomValidity( + "Consider choosing a darker color. " + + "(Tag text is small and white.)\n\n" + + "Contrast ratio: " + (Math.trunc(contrast * 10) / 10) + " (< 4.5)" + ); + + // The admin form uses novalidate, so manually display the browser's + // validity popup. (The user can still ignore it if desired.) + input.reportValidity(); + } +}); diff --git a/pgcommitfest/commitfest/admin.py b/pgcommitfest/commitfest/admin.py index 8c8d62e5..1cfa4bc7 100644 --- a/pgcommitfest/commitfest/admin.py +++ b/pgcommitfest/commitfest/admin.py @@ -1,8 +1,10 @@ from django.contrib import admin +from django.forms import widgets from .models import ( CfbotBranch, CfbotTask, + ColorField, CommitFest, Committer, MailThread, @@ -10,6 +12,7 @@ Patch, PatchHistory, PatchOnCommitFest, + Tag, TargetVersion, Topic, ) @@ -38,8 +41,26 @@ class MailThreadAttachmentAdmin(admin.ModelAdmin): ) +class ColorInput(widgets.Input): + """ + A color picker widget. + TODO: this will be natively available in Django 5.2. + """ + + input_type = "color" + + +class TagAdmin(admin.ModelAdmin): + # Customize the Tag form with a color picker and soft validation. + change_form_template = "change_tag_form.html" + formfield_overrides = { + ColorField: {"widget": ColorInput}, + } + + admin.site.register(Committer, CommitterAdmin) admin.site.register(CommitFest) +admin.site.register(Tag, TagAdmin) admin.site.register(Topic) admin.site.register(Patch, PatchAdmin) admin.site.register(PatchHistory) diff --git a/pgcommitfest/commitfest/forms.py b/pgcommitfest/commitfest/forms.py index c3b9a18d..312e1d9a 100644 --- a/pgcommitfest/commitfest/forms.py +++ b/pgcommitfest/commitfest/forms.py @@ -5,7 +5,7 @@ from django.http import Http404 from .ajax import _archivesAPI -from .models import MailThread, Patch, PatchOnCommitFest, TargetVersion +from .models import MailThread, Patch, PatchOnCommitFest, Tag, TargetVersion from .widgets import ThreadPickWidget @@ -13,11 +13,13 @@ class CommitFestFilterForm(forms.Form): selectize_fields = { "author": "/lookups/user", "reviewer": "/lookups/user", + "tag": None, } text = forms.CharField(max_length=50, required=False) status = forms.ChoiceField(required=False) targetversion = forms.ChoiceField(required=False) + tag = forms.ChoiceField(required=False, label="Tag (type to search)") author = forms.ChoiceField(required=False, label="Author (type to search)") reviewer = forms.ChoiceField(required=False, label="Reviewer (type to search)") sortkey = forms.IntegerField(required=False) @@ -59,6 +61,9 @@ def __init__(self, data, *args, **kwargs): ) self.fields["author"].choices = userchoices self.fields["reviewer"].choices = userchoices + self.fields["tag"].choices = [(-1, "* All"), (-2, "* None")] + list( + Tag.objects.all().values_list("id", "name") + ) for f in ( "status", @@ -72,6 +77,7 @@ class PatchForm(forms.ModelForm): selectize_fields = { "authors": "/lookups/user", "reviewers": "/lookups/user", + "tags": None, } class Meta: @@ -94,8 +100,14 @@ def __init__(self, *args, **kwargs): x.user.username, ) + self.fields["authors"].widget.attrs["class"] = "add-user-picker" + self.fields["reviewers"].widget.attrs["class"] = "add-user-picker" + # Selectize multiple fields -- don't pre-populate everything for field, url in list(self.selectize_fields.items()): + if url is None: + continue + # If this is a postback of a selectize field, it may contain ids that are not currently # stored in the field. They must still be among the *allowed* values of course, which # are handled by the existing queryset on the field. diff --git a/pgcommitfest/commitfest/migrations/0011_tag_patch_tags.py b/pgcommitfest/commitfest/migrations/0011_tag_patch_tags.py new file mode 100644 index 00000000..6bbbaba1 --- /dev/null +++ b/pgcommitfest/commitfest/migrations/0011_tag_patch_tags.py @@ -0,0 +1,40 @@ +# Generated by Django 4.2.19 on 2025-05-30 18:09 + +from django.db import migrations, models + +import pgcommitfest.commitfest.models + + +class Migration(migrations.Migration): + dependencies = [ + ("commitfest", "0010_add_failing_since_column"), + ] + + operations = [ + migrations.CreateModel( + name="Tag", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("name", models.CharField(max_length=50, unique=True)), + ("color", pgcommitfest.commitfest.models.ColorField(max_length=7)), + ], + options={ + "ordering": ("name",), + }, + ), + migrations.AddField( + model_name="patch", + name="tags", + field=models.ManyToManyField( + blank=True, related_name="patches", to="commitfest.tag" + ), + ), + ] diff --git a/pgcommitfest/commitfest/models.py b/pgcommitfest/commitfest/models.py index fcd9edb9..f795f16a 100644 --- a/pgcommitfest/commitfest/models.py +++ b/pgcommitfest/commitfest/models.py @@ -102,11 +102,37 @@ def __str__(self): return self.version +class ColorField(models.CharField): + """ + A small wrapper around a CharField that can hold a #RRGGBB color code. The + primary reason to have this wrapper class is so that the TagAdmin class can + explicitly key off of it to inject a color picker in the admin interface. + """ + + def __init__(self, *args, **kwargs): + kwargs["max_length"] = 7 # for `#RRGGBB` format + super().__init__(*args, **kwargs) + + +class Tag(models.Model): + """Represents a tag/label on a patch.""" + + name = models.CharField(max_length=50, unique=True) + color = ColorField() + + class Meta: + ordering = ("name",) + + def __str__(self): + return self.name + + class Patch(models.Model, DiffableModel): name = models.CharField( max_length=500, blank=False, null=False, verbose_name="Description" ) topic = models.ForeignKey(Topic, blank=False, null=False, on_delete=models.CASCADE) + tags = models.ManyToManyField(Tag, related_name="patches", blank=True) # One patch can be in multiple commitfests, if it has history commitfests = models.ManyToManyField(CommitFest, through="PatchOnCommitFest") diff --git a/pgcommitfest/commitfest/templates/base_form.html b/pgcommitfest/commitfest/templates/base_form.html index 40518ca9..788627a9 100644 --- a/pgcommitfest/commitfest/templates/base_form.html +++ b/pgcommitfest/commitfest/templates/base_form.html @@ -75,7 +75,7 @@

Search user

{%include "selectize_js.html" %} +{% endblock %} diff --git a/pgcommitfest/commitfest/templates/commitfest.html b/pgcommitfest/commitfest/templates/commitfest.html index 972e0bf8..234df1a6 100644 --- a/pgcommitfest/commitfest/templates/commitfest.html +++ b/pgcommitfest/commitfest/templates/commitfest.html @@ -34,6 +34,7 @@

{{p.is_open|yesno:"Active patches,Closed patches"}}

Patch{%if sortkey == 5%}
{%elif sortkey == -5%}
{%endif%} ID{%if sortkey == 4%}
{%elif sortkey == -4%}
{%endif%} Status + Tags Ver CI status{%if sortkey == 7%}
{%elif sortkey == -7%}
{%endif%} Stats{%if sortkey == 6%}
{%elif sortkey == -6%}
{%endif%} @@ -59,6 +60,14 @@

{{p.is_open|yesno:"Active patches,Closed patches"}}

{{p.name}} {{p.id}} {{p.status|patchstatusstring}} + + {%for t in p.tag_ids%} + + + {{all_tags|tagname:t}} + + {%endfor%} + {%if p.targetversion%}{{p.targetversion}}{%endif%} {%with p.cfbot_results as cfb%} diff --git a/pgcommitfest/commitfest/templates/patch.html b/pgcommitfest/commitfest/templates/patch.html index 2aafd141..f29f6089 100644 --- a/pgcommitfest/commitfest/templates/patch.html +++ b/pgcommitfest/commitfest/templates/patch.html @@ -67,6 +67,14 @@ Topic {{patch.topic}} + + Tags + + {%for tag in patch.tags.all%} + {{tag.name}} + {%endfor%} + + Created {{patch.created}} diff --git a/pgcommitfest/commitfest/templates/selectize_js.html b/pgcommitfest/commitfest/templates/selectize_js.html index 7cd5fb7d..0991ac65 100644 --- a/pgcommitfest/commitfest/templates/selectize_js.html +++ b/pgcommitfest/commitfest/templates/selectize_js.html @@ -5,19 +5,21 @@ valueField: 'id', labelField: 'value', searchField: 'value', - load: function(query, callback) { - if (!query.length) return callback(); - $.ajax({ - 'url': '{{url}}', - 'type': 'GET', - 'dataType': 'json', - 'data': { - 'query': query, - }, - 'error': function() { callback();}, - 'success': function(res) { callback(res.values);}, - }); - }, + {%if url%} + load: function(query, callback) { + if (!query.length) return callback(); + $.ajax({ + 'url': '{{url}}', + 'type': 'GET', + 'dataType': 'json', + 'data': { + 'query': query, + }, + 'error': function() { callback();}, + 'success': function(res) { callback(res.values);}, + }); + }, + {%endif%} onFocus: function() { if (this.$input.is('[multiple]')) { return; diff --git a/pgcommitfest/commitfest/templatetags/commitfest.py b/pgcommitfest/commitfest/templatetags/commitfest.py index 25fd21f2..403d1ceb 100644 --- a/pgcommitfest/commitfest/templatetags/commitfest.py +++ b/pgcommitfest/commitfest/templatetags/commitfest.py @@ -6,6 +6,7 @@ from django.utils.translation import ngettext_lazy import datetime +import string from uuid import uuid4 from pgcommitfest.commitfest.models import CommitFest, PatchOnCommitFest @@ -41,6 +42,45 @@ def patchstatuslabel(value): return [v for k, v in PatchOnCommitFest._STATUS_LABELS if k == i][0] +@register.filter(name="tagname") +def tagname(value, arg): + """ + Looks up a tag by ID and returns its name. The filter value is the map of + tags, and the argument is the ID. (Unlike tagcolor, there is no + argument-less variant; just use tag.name directly.) + + Example: + tag_map|tagname:tag_id + """ + return value[arg].name + + +@register.filter(name="tagcolor") +def tagcolor(value, key=None): + """ + Returns the color code of a tag. The filter value is either a single tag, in + which case no argument should be given, or a map of tags with the tag ID as + the argument, as with the tagname filter. + + Since color codes are injected into CSS, any nonconforming inputs are + replaced with black here. (Prefer `tag|tagcolor` over `tag.color` in + templates, for this reason.) + """ + if key is not None: + code = value[key].color + else: + code = value.color + + if ( + len(code) == 7 + and code.startswith("#") + and all(c in string.hexdigits for c in code[1:]) + ): + return code + + return "#000000" + + @register.filter(is_safe=True) def label_class(value, arg): return value.label_tag(attrs={"class": arg}) diff --git a/pgcommitfest/commitfest/views.py b/pgcommitfest/commitfest/views.py index 7b6dd63d..46d6da94 100644 --- a/pgcommitfest/commitfest/views.py +++ b/pgcommitfest/commitfest/views.py @@ -42,6 +42,7 @@ Patch, PatchHistory, PatchOnCommitFest, + Tag, ) @@ -262,6 +263,21 @@ def patchlist(request, cf, personalized=False): # int() failed, ignore pass + if request.GET.get("tag", "-1") != "-1": + if request.GET["tag"] == "-2": + whereclauses.append( + "NOT EXISTS (SELECT 1 FROM commitfest_patch_tags tags WHERE tags.patch_id=p.id)" + ) + else: + try: + whereparams["tag"] = int(request.GET["tag"]) + whereclauses.append( + "EXISTS (SELECT 1 FROM commitfest_patch_tags tags WHERE tags.patch_id=p.id AND tags.tag_id=%(tag)s)" + ) + except ValueError: + # int() failed -- so just ignore this filter + pass + if request.GET.get("author", "-1") != "-1": if request.GET["author"] == "-2": whereclauses.append( @@ -485,6 +501,7 @@ def patchlist(request, cf, personalized=False): (SELECT string_agg(first_name || ' ' || last_name || ' (' || username || ')', ', ') FROM auth_user INNER JOIN commitfest_patch_authors cpa ON cpa.user_id=auth_user.id WHERE cpa.patch_id=p.id) AS author_names, (SELECT string_agg(first_name || ' ' || last_name || ' (' || username || ')', ', ') FROM auth_user INNER JOIN commitfest_patch_reviewers cpr ON cpr.user_id=auth_user.id WHERE cpr.patch_id=p.id) AS reviewer_names, (SELECT count(1) FROM commitfest_patchoncommitfest pcf WHERE pcf.patch_id=p.id) AS num_cfs, +(SELECT array_agg(tag_id) FROM commitfest_patch_tags t WHERE t.patch_id=p.id) AS tag_ids, branch.needs_rebase_since, branch.failing_since, @@ -531,6 +548,7 @@ def patchlist(request, cf, personalized=False): ) +@transaction.atomic # tie the patchlist() query to Tag.objects.all() def commitfest(request, cfid): # Find ourselves cf = get_object_or_404(CommitFest, pk=cfid) @@ -562,6 +580,7 @@ def commitfest(request, cfid): "form": form, "patches": patch_list.patches, "statussummary": statussummary, + "all_tags": {t.id: t for t in Tag.objects.all()}, "has_filter": patch_list.has_filter, "title": cf.title, "grouping": patch_list.sortkey == 0,