From c0b0dc71cbe267f78b12e2f941c774f1e814c966 Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Fri, 30 May 2025 10:00:33 -0700 Subject: [PATCH 1/2] Add Tags for patches A Tag is an arbitrary label for a patch in the Commitfest UI. Other than helping users identify patches of interest, it has no other semantic meaning to the CF application. Tags are created using the administrator interface. They consist of a unique name and a background color. The color should be sent from compliant browsers in #rrggbb format, which is stored without backend validation; to avoid CSS injection, any non-conforming values are replaced with black during templating. --- pgcommitfest/commitfest/admin.py | 19 +++++++++ .../migrations/0011_tag_patch_tags.py | 39 ++++++++++++++++++ pgcommitfest/commitfest/models.py | 26 ++++++++++++ .../commitfest/templates/commitfest.html | 6 +++ pgcommitfest/commitfest/templates/patch.html | 8 ++++ .../commitfest/templatetags/commitfest.py | 40 +++++++++++++++++++ pgcommitfest/commitfest/views.py | 4 ++ 7 files changed, 142 insertions(+) create mode 100644 pgcommitfest/commitfest/migrations/0011_tag_patch_tags.py diff --git a/pgcommitfest/commitfest/admin.py b/pgcommitfest/commitfest/admin.py index 8c8d62e5..2023cbe2 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,24 @@ 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): + 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/migrations/0011_tag_patch_tags.py b/pgcommitfest/commitfest/migrations/0011_tag_patch_tags.py new file mode 100644 index 00000000..9ab361ad --- /dev/null +++ b/pgcommitfest/commitfest/migrations/0011_tag_patch_tags.py @@ -0,0 +1,39 @@ +# 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/commitfest.html b/pgcommitfest/commitfest/templates/commitfest.html index 972e0bf8..ce056b1f 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,11 @@

{{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..1c286950 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/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..a3aecc36 100644 --- a/pgcommitfest/commitfest/views.py +++ b/pgcommitfest/commitfest/views.py @@ -42,6 +42,7 @@ Patch, PatchHistory, PatchOnCommitFest, + Tag, ) @@ -485,6 +486,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 +533,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 +565,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, From 02aa97fded5db1390fd8fac9518cd7769a0e0c43 Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Fri, 30 May 2025 22:08:39 -0700 Subject: [PATCH 2/2] Help admins choose tag colors with contrast Add front-end soft validation to the TagAdmin form. This uses WCAG 2.2 guidelines to warn the administrator if a color choice is going to be low-contrast when compared to our text/background color of white. (The warning may be ignored.) --- media/commitfest/js/change_tag.js | 44 +++++++++++++++++++ pgcommitfest/commitfest/admin.py | 2 + .../commitfest/templates/change_tag_form.html | 7 +++ 3 files changed, 53 insertions(+) create mode 100644 media/commitfest/js/change_tag.js create mode 100644 pgcommitfest/commitfest/templates/change_tag_form.html 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 2023cbe2..1cfa4bc7 100644 --- a/pgcommitfest/commitfest/admin.py +++ b/pgcommitfest/commitfest/admin.py @@ -51,6 +51,8 @@ class ColorInput(widgets.Input): 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}, } diff --git a/pgcommitfest/commitfest/templates/change_tag_form.html b/pgcommitfest/commitfest/templates/change_tag_form.html new file mode 100644 index 00000000..4ec39605 --- /dev/null +++ b/pgcommitfest/commitfest/templates/change_tag_form.html @@ -0,0 +1,7 @@ +{% extends 'admin/change_form.html' %} +{% load static %} + +{% block admin_change_form_document_ready %} +{{ block.super }} + +{% endblock %}