diff --git a/docs/api/schemas/latest/patchwork.yaml b/docs/api/schemas/latest/patchwork.yaml index b2bb220f..7847095c 100644 --- a/docs/api/schemas/latest/patchwork.yaml +++ b/docs/api/schemas/latest/patchwork.yaml @@ -2312,6 +2312,11 @@ components: type: array items: $ref: '#/components/schemas/PatchEmbedded' + attention_set: + title: AttentionSet + type: array + items: + $ref: '#/components/schemas/PatchAttentionSet' PatchDetail: type: object title: Patches @@ -2390,6 +2395,11 @@ components: type: array items: type: integer + attention_set: + title: Attention Set + type: array + items: + type: integer Person: type: object title: Person @@ -2831,6 +2841,19 @@ components: type: string format: uri readOnly: true + PatchAttentionSet: + type: object + title: PatchAttentionSet + description: | + A user interested or with pending actions over a patch + properties: + user: + $ref: '#/components/schemas/UserEmbedded' + last_updated: + title: last_updated + type: string + format: iso8601 + readOnly: true PatchEmbedded: type: object title: Patch diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2 index f37d3213..31ef75ce 100644 --- a/docs/api/schemas/patchwork.j2 +++ b/docs/api/schemas/patchwork.j2 @@ -2397,6 +2397,13 @@ components: type: array items: $ref: '#/components/schemas/PatchEmbedded' +{% endif %} +{% if version >= (1, 4) %} + attention_set: + title: AttentionSet + type: array + items: + $ref: '#/components/schemas/PatchAttentionSet' {% endif %} PatchDetail: type: object @@ -2477,6 +2484,13 @@ components: type: array items: type: integer +{% endif %} +{% if version >= (1, 4) %} + attention_set: + title: Attention Set + type: array + items: + type: integer {% endif %} Person: type: object @@ -2941,6 +2955,21 @@ components: type: string format: uri readOnly: true +{% if version >= (1, 4) %} + PatchAttentionSet: + type: object + title: PatchAttentionSet + description: | + A user interested or with pending actions over a patch + properties: + user: + $ref: '#/components/schemas/UserEmbedded' + last_updated: + title: last_updated + type: string + format: iso8601 + readOnly: true +{% endif %} PatchEmbedded: type: object title: Patch diff --git a/docs/api/schemas/v1.4/patchwork.yaml b/docs/api/schemas/v1.4/patchwork.yaml index 036fe15f..41d59daf 100644 --- a/docs/api/schemas/v1.4/patchwork.yaml +++ b/docs/api/schemas/v1.4/patchwork.yaml @@ -2312,6 +2312,11 @@ components: type: array items: $ref: '#/components/schemas/PatchEmbedded' + attention_set: + title: AttentionSet + type: array + items: + $ref: '#/components/schemas/PatchAttentionSet' PatchDetail: type: object title: Patches @@ -2390,6 +2395,11 @@ components: type: array items: type: integer + attention_set: + title: Attention Set + type: array + items: + type: integer Person: type: object title: Person @@ -2831,6 +2841,19 @@ components: type: string format: uri readOnly: true + PatchAttentionSet: + type: object + title: PatchAttentionSet + description: | + A user interested or with pending actions over a patch + properties: + user: + $ref: '#/components/schemas/UserEmbedded' + last_updated: + title: last_updated + type: string + format: iso8601 + readOnly: true PatchEmbedded: type: object title: Patch diff --git a/docs/usage/overview.rst b/docs/usage/overview.rst index 539de9ca..0c04cf4b 100644 --- a/docs/usage/overview.rst +++ b/docs/usage/overview.rst @@ -154,6 +154,7 @@ or a cover letter or the web URL to a patch or a series: Depends-on: <20240726221429.221611-1-user@example.com> Depends-on: https://pw.example.com/project/myproject/list?series=1234 + Depends-on: https://pw.example.com/project/myproject/series/1234 .. note:: diff --git a/htdocs/css/style.css b/htdocs/css/style.css index 268a8c37..49366230 100644 --- a/htdocs/css/style.css +++ b/htdocs/css/style.css @@ -288,6 +288,18 @@ table.patch-meta tr th, table.patch-meta tr td { text-decoration: underline; } +.patchinterest { + display: inline-block; + border-radius: 7px; + min-width: 0.9em; + padding: 0 2px; + text-align: center; +} + +.patchinterest.exists { + background-color: #82ca9d; +} + .patchlistchecks { display: inline-block; border-radius: 7px; @@ -344,6 +356,18 @@ table.patch-meta tr th, table.patch-meta tr td { font-family: "DejaVu Sans Mono", fixed; } +.submission-attention-set { + display: flex; + flex-wrap: wrap; + align-items: center; + gap: 8px; +} + +button[class*=interest-action] { + padding: 0.2em 0.5em; + border-radius: 4px; +} + div[class^="comment-status-bar-"] { display: flex; flex-wrap: wrap; @@ -407,6 +431,12 @@ span.p_mod { color: #a020f0; } font-weight: bold; } +/* series */ +a.series-list-header { + color: inherit; /* Inherit color from parent element */ + text-decoration: none; /* Optional: removes underline */ +} + /* bundles */ table.bundlelist { margin-top: 2em; diff --git a/patchwork/admin.py b/patchwork/admin.py index d1c389a1..409df2f8 100644 --- a/patchwork/admin.py +++ b/patchwork/admin.py @@ -23,6 +23,7 @@ from patchwork.models import State from patchwork.models import Tag from patchwork.models import UserProfile +from patchwork.models import PatchAttentionSet class UserProfileInline(admin.StackedInline): @@ -86,6 +87,17 @@ class CoverAdmin(admin.ModelAdmin): admin.site.register(Cover, CoverAdmin) +class PatchAttentionSetInline(admin.StackedInline): + model = PatchAttentionSet + fields = ('user',) + extra = 0 + verbose_name = 'user' + verbose_name_plural = 'attention set users' + + def has_change_permission(self, request, obj=None): + return False + + class PatchAdmin(admin.ModelAdmin): list_display = ( 'name', @@ -99,6 +111,7 @@ class PatchAdmin(admin.ModelAdmin): list_filter = ('project', 'submitter', 'state', 'archived') list_select_related = ('submitter', 'project', 'state') search_fields = ('name', 'submitter__name', 'submitter__email') + inlines = (PatchAttentionSetInline,) date_hierarchy = 'date' def is_pull_request(self, patch): diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py index 443c3822..dae4269b 100644 --- a/patchwork/api/patch.py +++ b/patchwork/api/patch.py @@ -7,6 +7,7 @@ import email.parser +from django.contrib.auth.models import User from django.core.exceptions import ValidationError from django.utils.text import slugify from django.utils.translation import gettext_lazy as _ @@ -15,6 +16,7 @@ from rest_framework.generics import ListAPIView from rest_framework.generics import RetrieveUpdateAPIView from rest_framework.relations import RelatedField +from rest_framework.response import Response from rest_framework.reverse import reverse from rest_framework.serializers import SerializerMethodField from rest_framework import status @@ -28,6 +30,7 @@ from patchwork.api.embedded import UserSerializer from patchwork.api.filters import PatchFilterSet from patchwork.models import Patch +from patchwork.models import PatchAttentionSet from patchwork.models import PatchRelation from patchwork.models import State from patchwork.parser import clean_subject @@ -76,12 +79,27 @@ class PatchConflict(APIException): ) +class PatchAttentionSetSerializer(BaseHyperlinkedModelSerializer): + user = UserSerializer() + + class Meta: + model = PatchAttentionSet + fields = [ + 'user', + 'last_updated', + ] + + class PatchListSerializer(BaseHyperlinkedModelSerializer): web_url = SerializerMethodField() project = ProjectSerializer(read_only=True) state = StateField() submitter = PersonSerializer(read_only=True) delegate = UserSerializer(allow_null=True) + attention_set = PatchAttentionSetSerializer( + source='patchattentionset_set', + many=True, + ) mbox = SerializerMethodField() series = SeriesSerializer(read_only=True) comments = SerializerMethodField() @@ -170,6 +188,7 @@ class Meta: 'hash', 'submitter', 'delegate', + 'attention_set', 'mbox', 'series', 'comments', @@ -201,6 +220,7 @@ class Meta: 'list_archive_url', 'related', ), + '1.4': ('attention_set',), } extra_kwargs = { 'url': {'view_name': 'api-patch-detail'}, @@ -228,16 +248,7 @@ def get_headers(self, patch): def get_prefixes(self, instance): return clean_subject(instance.name)[1] - def update(self, instance, validated_data): - # d-r-f cannot handle writable nested models, so we handle that - # specifically ourselves and let d-r-f handle the rest - if 'related' not in validated_data: - return super(PatchDetailSerializer, self).update( - instance, validated_data - ) - - related = validated_data.pop('related') - + def update_related(self, instance, related): # Validation rules # ---------------- # @@ -278,9 +289,7 @@ def update(self, instance, validated_data): if instance.related and instance.related.patches.count() == 2: instance.related.delete() instance.related = None - return super(PatchDetailSerializer, self).update( - instance, validated_data - ) + return # break before make relations = {patch.related for patch in patches if patch.related} @@ -304,6 +313,14 @@ def update(self, instance, validated_data): instance.related = relation instance.save() + def update(self, instance, validated_data): + # d-r-f cannot handle writable nested models, so we handle that + # specifically ourselves and let d-r-f handle the rest + + if 'related' in validated_data: + related = validated_data.pop('related') + self.update_related(instance, related) + return super(PatchDetailSerializer, self).update( instance, validated_data ) @@ -367,6 +384,7 @@ def get_queryset(self): 'project', 'series__project', 'related__patches__project', + 'patchattentionset_set', ) .select_related('state', 'submitter', 'series') .defer('content', 'diff', 'headers') @@ -381,11 +399,16 @@ class PatchDetail(RetrieveUpdateAPIView): patch: Update a patch. + Users can set their intention to review or comment about a patch using the + `attention_set` property. Users can set their intentions by adding their + IDs or its negative value to the list. Maintainers can remove people from + the list but only a user can add itself. + + put: Update a patch. """ - permission_classes = (PatchworkPermission,) serializer_class = PatchDetailSerializer def get_queryset(self): @@ -396,3 +419,62 @@ def get_queryset(self): 'project', 'state', 'submitter', 'delegate', 'series' ) ) + + def partial_update(self, request, *args, **kwargs): + obj = self.get_object() + req_user_id = request.user.id + is_maintainer = request.user.is_authenticated and ( + obj.project in request.user.profile.maintainer_projects.all() + ) + + if 'attention_set' in request.data and request.method in ('PATCH',): + attention_set = request.data.get('attention_set', None) + del request.data['attention_set'] + removal_list = [ + -user_id for user_id in set(attention_set) if user_id < 0 + ] + addition_list = [ + user_id for user_id in set(attention_set) if user_id > 0 + ] + + if not addition_list and not removal_list: + removal_list = [req_user_id] + + if len(addition_list) > 1 or ( + addition_list and req_user_id not in addition_list + ): + raise PermissionDenied( + detail="Only the user can declare it's own intention of " + 'reviewing a patch' + ) + + if not is_maintainer: + if removal_list and req_user_id not in removal_list: + raise PermissionDenied( + detail="Only the user can remove it's own " + 'intention of reviewing a patch' + ) + + try: + if addition_list: + PatchAttentionSet.objects.upsert(obj, addition_list) + if removal_list: + PatchAttentionSet.objects.soft_delete( + obj, removal_list, reason=f'removed by {request.user}' + ) + except User.DoesNotExist: + return Response( + {'message': 'Unable to find referenced user'}, + status=404, + ) + + if not is_maintainer: + serializer = self.get_serializer(obj) + return Response( + serializer.data, + status=200, + ) + + return super(PatchDetail, self).partial_update( + request, *args, **kwargs + ) diff --git a/patchwork/forms.py b/patchwork/forms.py index cf77bdcc..f877e625 100644 --- a/patchwork/forms.py +++ b/patchwork/forms.py @@ -252,8 +252,27 @@ def save(self, instance, commit=True): if field.is_no_change(data[f.name]): continue + if f.name == 'review_status': + if data[f.name]: + self.instance.attention_set.add(self.user) + else: + self.instance.attention_set.remove(self.user) + continue + setattr(instance, f.name, data[f.name]) if commit: instance.save() return instance + + def review_status_only(self): + review_status_only = True + field_names = set(self.fields.keys()) + field_names.discard({'review_status', 'action'}) + + for field_name in field_names: + data = self.data.get(field_name, '*') + if data != '*': + review_status_only = False + + return review_status_only diff --git a/patchwork/migrations/0049_add_attention_set_to_patches.py b/patchwork/migrations/0049_add_attention_set_to_patches.py new file mode 100644 index 00000000..864f9b9f --- /dev/null +++ b/patchwork/migrations/0049_add_attention_set_to_patches.py @@ -0,0 +1,61 @@ +# Generated by Django 5.1.7 on 2025-03-22 05:06 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ('patchwork', '0048_series_dependencies'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='PatchAttentionSet', + fields=[ + ( + 'id', + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name='ID', + ), + ), + ('last_updated', models.DateTimeField(auto_now=True)), + ('removed', models.BooleanField(default=False)), + ( + 'removed_reason', + models.CharField(blank=True, max_length=50), + ), + ( + 'patch', + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to='patchwork.patch', + ), + ), + ( + 'user', + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to=settings.AUTH_USER_MODEL, + ), + ), + ], + options={ + 'unique_together': {('patch', 'user')}, + }, + ), + migrations.AddField( + model_name='patch', + name='attention_set', + field=models.ManyToManyField( + related_name='attention_set', + through='patchwork.PatchAttentionSet', + to=settings.AUTH_USER_MODEL, + ), + ), + ] diff --git a/patchwork/models.py b/patchwork/models.py index ae2f4a6d..287478a8 100644 --- a/patchwork/models.py +++ b/patchwork/models.py @@ -14,6 +14,7 @@ from django.contrib.auth.models import User from django.core.exceptions import ValidationError from django.core.validators import validate_unicode_slug +from django.db.models import Count from django.db import models from django.urls import reverse from django.utils.functional import cached_property @@ -503,6 +504,9 @@ class Patch(SubmissionMixin): null=True, on_delete=models.CASCADE, ) + attention_set = models.ManyToManyField( + User, through='PatchAttentionSet', related_name='attention_set' + ) state = models.ForeignKey(State, null=True, on_delete=models.CASCADE) archived = models.BooleanField(default=False) hash = HashField(null=True, blank=True, db_index=True) @@ -579,7 +583,7 @@ def save(self, *args, **kwargs): self.refresh_tag_counts() - def is_editable(self, user): + def is_editable(self, user, declare_interest_only=False): if not user.is_authenticated: return False @@ -590,7 +594,8 @@ def is_editable(self, user): if self.project.is_editable(user): self._edited_by = user return True - return False + + return declare_interest_only @staticmethod def filter_unique_checks(checks): @@ -827,6 +832,104 @@ class Meta: ] +class PatchAttentionSetManager(models.Manager): + def get_queryset(self): + return super().get_queryset().filter(removed=False) + + def upsert(self, patch, users): + """Add or updates deleted attention set entries + + :param patch: patch object to be updated + :type patch: Patch + :param users: list of users to be added to the attention set list + :type users: list[int] + """ + qs = super().get_queryset().filter(patch=patch) + + existing = { + obj.user.id: obj for obj in qs.filter(user__in=users).all() + } + update_list = [] + for obj in existing.values(): + if obj.removed: + obj.removed = False + obj.removed_reason = '' + update_list.append(obj) + insert_list = [user for user in users if user not in existing.keys()] + + qs.bulk_create( + [PatchAttentionSet(patch=patch, user_id=id) for id in insert_list] + ) + qs.bulk_update(update_list, ['removed', 'removed_reason']) + + def soft_delete(self, patch, users, reason=''): + """Mark attention set entries as deleted + + :param patch: patch object to be updated + :type patch: Patch + :param users: list of users to be added to the attention set list + :type users: list[int] + :param reason: reason for removal + :type reason: string + """ + qs = super().get_queryset().filter(patch=patch) + + existing = { + obj.user.id: obj for obj in qs.filter(user__in=users).all() + } + update_list = [] + for obj in existing.values(): + if not obj.removed: + obj.removed = True + obj.removed_reason = reason + update_list.append(obj) + + self.bulk_update(update_list, ['removed', 'removed_reason']) + + +class PatchAttentionSet(models.Model): + patch = models.ForeignKey(Patch, on_delete=models.CASCADE) + user = models.ForeignKey(User, on_delete=models.CASCADE) + last_updated = models.DateTimeField(auto_now=True) + removed = models.BooleanField(default=False) + removed_reason = models.CharField(max_length=50, blank=True) + + objects = PatchAttentionSetManager() + raw_objects = models.Manager() + + def delete(self): + """Soft deletes an user from the patch attention set""" + self.removed = True + self.removed_reason = 'reviewed or commented on the patch' + self.save() + + def __str__(self): + return f'<{self.user} - {self.user.email}>' + + class Meta: + unique_together = [('patch', 'user')] + + +def _remove_user_from_patch_attention_set(sender, instance, created, **kwargs): + if created: + submitter = instance.submitter + patch = instance.patch + if submitter.user: + try: + # Don't use the RelatedManager since it will execute a hard + # delete + PatchAttentionSet.objects.get( + patch=patch, user=submitter.user + ).delete() + except PatchAttentionSet.DoesNotExist: + pass + + +models.signals.post_save.connect( + _remove_user_from_patch_attention_set, sender=PatchComment +) + + class Series(FilenameMixin, models.Model): """A collection of patches.""" @@ -913,6 +1016,30 @@ def add_dependencies(self, dependencies): ) self.save() + @property + def interest_count(self): + count = self.patches.aggregate(Count('attention_set', distinct=True)) + return count['attention_set__count'] + + @property + def check_count(self): + """Generate a list of unique checks for all patchs in the series. + + Compile a list of checks associated with this series patches for each + type of check. Only "unique" checks are considered, identified by their + 'context' field. This means, given n checks with the same 'context', the + newest check is the only one counted regardless of its value. The end + result will be a association of types to number of unique checks for + said type. + """ + counts = {key: 0 for key, _ in Check.STATE_CHOICES} + + for p in self.patches.all(): + for check in p.checks: + counts[check.state] += 1 + + return counts + def add_cover_letter(self, cover): """Add a cover letter to the series. @@ -968,10 +1095,10 @@ def add_patch(self, patch, number): return patch def get_absolute_url(self): - # TODO(stephenfin): We really need a proper series view return reverse( - 'patch-list', kwargs={'project_id': self.project.linkname} - ) + ('?series=%d' % self.id) + 'series-detail', + kwargs={'project_id': self.project.linkname, 'series_id': self.id}, + ) def get_mbox_url(self): return reverse('series-mbox', kwargs={'series_id': self.id}) diff --git a/patchwork/parser.py b/patchwork/parser.py index c33ada8d..8c516250 100644 --- a/patchwork/parser.py +++ b/patchwork/parser.py @@ -1068,7 +1068,10 @@ def find_series_from_url(url): logging.warning('Failed to resolve series or patch URL: %s', url) return None - # TODO: Use the series detail view here. + if result.view_name == 'series-detail': + return Series.objects.get(pk=result.kwargs['series_id']) + + # Handles series as a patch-list view if result.view_name == 'patch-list' and parse_result.query: # Parse the query string. # This can be replaced with something much friendlier once the diff --git a/patchwork/templates/patchwork/partials/download-buttons.html b/patchwork/templates/patchwork/partials/download-buttons.html index 34c5f8fc..292ecb7b 100644 --- a/patchwork/templates/patchwork/partials/download-buttons.html +++ b/patchwork/templates/patchwork/partials/download-buttons.html @@ -1,4 +1,5 @@
+{% if submission %} + + series + +{% endif %}
diff --git a/patchwork/templates/patchwork/partials/patch-list.html b/patchwork/templates/patchwork/partials/patch-list.html index 981ceee5..482ce3db 100644 --- a/patchwork/templates/patchwork/partials/patch-list.html +++ b/patchwork/templates/patchwork/partials/patch-list.html @@ -80,6 +80,10 @@ S/W/F + + Review Interest + + {% if not order.editable %} {% if order.name == "date" %} @@ -175,13 +179,14 @@ {% if patch.series %} - + {{ patch.series|truncatechars:100 }} {% endif %} {{ patch|patch_tags }} {{ patch|patch_checks }} + {{ patch|patch_interest }} {{ patch.date|date:"Y-m-d" }} {{ patch.submitter|personify:project }} {{ patch.delegate.username }} diff --git a/patchwork/templates/patchwork/series-detail.html b/patchwork/templates/patchwork/series-detail.html new file mode 100644 index 00000000..3da9edbb --- /dev/null +++ b/patchwork/templates/patchwork/series-detail.html @@ -0,0 +1,54 @@ +{% extends "base.html" %} + +{% load humanize %} +{% load syntax %} +{% load person %} +{% load patch %} +{% load static %} +{% load utils %} + +{% block title %}{{series.name}}{% endblock %} + +{% block body %} + +
+{% include "patchwork/partials/download-buttons.html" %} +

{{ series.name }}

+
+ + + + + + + + + + + + + + + + + + +
Series ID + {{ series.id }} + +
Date{{ series.date }}
Submitter{{ series.submitter }}
Total{{ series.patches }}
+
+

Patches

+
+{% include "patchwork/partials/patch-list.html" %} + +
+

Cover Letter

+ {% if series.cover_letter.content %} +
{{ series.cover_letter.content }}
+ {% else %} + No cover letter available + {% endif %} +
+ +{% endblock %} diff --git a/patchwork/templates/patchwork/series-list.html b/patchwork/templates/patchwork/series-list.html new file mode 100644 index 00000000..02887ebb --- /dev/null +++ b/patchwork/templates/patchwork/series-list.html @@ -0,0 +1,113 @@ +{% extends "base.html" %} + +{% load person %} +{% load static %} + +{% block title %}{{project.name}}{% endblock %} +{% block series_active %}active{% endblock %} + +{% block body %} + +{% load person %} +{% load listurl %} +{% load patch %} +{% load series %} +{% load project %} +{% load static %} + +{% include "patchwork/partials/pagination.html" %} + + + + + + + +{% if user.is_authenticated and user.profile.show_ids %} + +{% endif %} + + + + + + + + + + + + + + + + + + + + +{% for series in page %} + +{% if user.is_authenticated and user.profile.show_ids %} + +{% endif %} + + + + + + + + + + +{% empty %} + + + +{% endfor %} + +
+ ID + + Series + + Version + + {% project_tags %} + + S/W/F + + Review Interest + + Patches + + {% if 'date.asc' == order %} + + + {% elif 'date.desc' == order %} + + + {% endif %} + Date + {% if 'date.asc' == order or 'date.desc' == order%} + + {% endif %} + + Submitter +
+ + + + {{ series.name|default:"[no subject]"|truncatechars:100 }} + + + {{ series.version|default:"-"}} + {{ series|series_tags }}{{ series|series_checks }}{{ series|series_interest }}{{ series.received_total}}{{ series.date|date:"Y-m-d" }}{{ series.submitter|personify:project }}
No series to display
+ +{% if page.paginator.count %} +{% include "patchwork/partials/pagination.html" %} +{% endif %} +{% endblock %} diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html index cd74491c..9fe89dd1 100644 --- a/patchwork/templates/patchwork/submission.html +++ b/patchwork/templates/patchwork/submission.html @@ -20,6 +20,20 @@

{{ submission.name }}

+
+ + next + +
+ +
+ + previous + +
+ @@ -62,7 +76,7 @@

{{ submission.name }}

Message ID
Series - + {{ submission.series.name }} | @@ -187,6 +201,34 @@

Message

+
+

Users pending actions

+ {% if user.is_authenticated and user not in attention_set %} +
+ {% csrf_token %} + + +
+ {% endif %} +
+{% if attention_set %} +
    + {% for set_user in attention_set %} +
  • +
    + {% csrf_token %} + + + {{ set_user.username }} ({{ set_user.email }}) + {% if set_user == user or is_maintainer %} + + {% endif %} +
    +
  • + {% endfor %} +
+{% endif %} + {% for item in comments %} {% if forloop.first %}

Comments

diff --git a/patchwork/templatetags/patch.py b/patchwork/templatetags/patch.py index d3f023f7..26282d83 100644 --- a/patchwork/templatetags/patch.py +++ b/patchwork/templatetags/patch.py @@ -75,3 +75,18 @@ def patch_commit_display(patch): return mark_safe( '%s' % (escape(fmt.format(commit)), escape(commit)) ) + + +@register.filter(name='patch_interest') +def patch_interest(patch): + reviews = patch.attention_set.count() + review_title = ( + f'has {reviews} interested reviewers' + if reviews > 0 + else 'no interested reviewers' + ) + review_class = 'exists' if reviews > 0 else '' + return mark_safe( + '%s' + % (review_class, review_title, reviews if reviews > 0 else '-') + ) diff --git a/patchwork/templatetags/series.py b/patchwork/templatetags/series.py new file mode 100644 index 00000000..a235b420 --- /dev/null +++ b/patchwork/templatetags/series.py @@ -0,0 +1,76 @@ +# Patchwork - automated patch tracking system +# Copyright (C) 2008 Jeremy Kerr +# Copyright (C) 2015 Intel Corporation +# +# SPDX-License-Identifier: GPL-2.0-or-later + +from django import template +from django.utils.safestring import mark_safe + +from patchwork.models import Check + + +register = template.Library() + + +@register.filter(name='series_tags') +def series_tags(series): + counts = [] + titles = [] + + for tag in [t for t in series.project.tags if t.show_column]: + count = 0 + for patch in series.patches.with_tag_counts(series.project).all(): + count += getattr(patch, tag.attr_name) + + titles.append('%d %s' % (count, tag.name)) + if count == 0: + counts.append('-') + else: + counts.append(str(count)) + + return mark_safe( + '%s' % (' / '.join(titles), ' '.join(counts)) + ) + + +@register.filter(name='series_checks') +def series_checks(series): + required = [Check.STATE_SUCCESS, Check.STATE_WARNING, Check.STATE_FAIL] + titles = ['Success', 'Warning', 'Fail'] + counts = series.check_count + + check_elements = [] + for state in required[::-1]: + if counts[state]: + color = dict(Check.STATE_CHOICES).get(state) + count = str(counts[state]) + else: + color = '' + count = '-' + + check_elements.append( + f'{count}' + ) + + check_elements.reverse() + + return mark_safe( + '%s' + % (' / '.join(titles), ''.join(check_elements)) + ) + + +@register.filter(name='series_interest') +def series_interest(series): + reviews = series.interest_count + review_title = ( + f'has {reviews} interested reviewers' + if reviews > 0 + else 'no interested reviewers' + ) + review_class = 'exists' if reviews > 0 else '' + return mark_safe( + '%s' + % (review_class, review_title, reviews if reviews > 0 else '-') + ) diff --git a/patchwork/tests/api/test_patch.py b/patchwork/tests/api/test_patch.py index 2661d75c..252a345e 100644 --- a/patchwork/tests/api/test_patch.py +++ b/patchwork/tests/api/test_patch.py @@ -11,9 +11,9 @@ from django.urls import reverse from rest_framework import status -from patchwork.models import Patch +from patchwork.models import Patch, PatchAttentionSet from patchwork.tests.api import utils -from patchwork.tests.utils import create_maintainer +from patchwork.tests.utils import create_attention_set, create_maintainer from patchwork.tests.utils import create_patch from patchwork.tests.utils import create_patches from patchwork.tests.utils import create_person @@ -238,7 +238,7 @@ def test_list_bug_335(self): series = create_series() create_patches(5, series=series) - with self.assertNumQueries(5): + with self.assertNumQueries(6): self.client.get(self.api_url()) @utils.store_samples('patch-detail') @@ -456,3 +456,235 @@ def test_delete(self): self.client.authenticate(user=user) resp = self.client.delete(self.api_url(patch.id)) self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) + + def test_declare_review_intention(self): + project = create_project() + state = create_state() + patch = create_patch(project=project, state=state) + user = create_user() + self.client.authenticate(user=user) + + # No intention of reviewing + self.assertEqual( + len( + PatchAttentionSet.objects.filter(patch=patch, user=user).all() + ), + 0, + ) + + # declare intention + resp = self.client.patch( + self.api_url(patch.id), + {'attention_set': [user.id]}, + ) + + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertEqual( + len( + PatchAttentionSet.objects.filter(patch=patch, user=user).all() + ), + 1, + ) + + # redeclare intention should have no effect + resp = self.client.patch( + self.api_url(patch.id), + {'attention_set': [user.id]}, + ) + + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertEqual( + len( + PatchAttentionSet.objects.filter(patch=patch, user=user).all() + ), + 1, + ) + + def test_remove_review_intention(self): + project = create_project() + state = create_state() + patch = create_patch(project=project, state=state) + user = create_user() + create_attention_set(patch=patch, user=user) + self.client.authenticate(user=user) + + # Existing intention of reviewing + self.assertEqual( + len( + PatchAttentionSet.objects.filter(patch=patch, user=user).all() + ), + 1, + ) + + # remove intention + resp = self.client.patch( + self.api_url(patch.id), + {'attention_set': [-user.id]}, + ) + + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertEqual( + len( + PatchAttentionSet.objects.filter(patch=patch, user=user).all() + ), + 0, + ) + # uses soft delete + self.assertEqual( + len( + PatchAttentionSet.raw_objects.filter( + patch=patch, user=user + ).all() + ), + 1, + ) + + def test_add_review_intention_updates_old_entry(self): + project = create_project() + state = create_state() + patch = create_patch(project=project, state=state) + user = create_user() + interest = create_attention_set(patch=patch, user=user, removed=True) + self.client.authenticate(user=user) + + # Existing deleted intention of reviewing + self.assertTrue(interest.removed) + + # updates intention + resp = self.client.patch( + self.api_url(patch.id), + {'attention_set': [user.id]}, + ) + + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertEqual( + len( + PatchAttentionSet.objects.filter(patch=patch, user=user).all() + ), + 1, + ) + # uses upsert + self.assertEqual( + len( + PatchAttentionSet.raw_objects.filter( + patch=patch, user=user + ).all() + ), + 1, + ) + + def test_remove_review_intention_with_empty_array(self): + project = create_project() + state = create_state() + patch = create_patch(project=project, state=state) + user = create_user() + create_attention_set(patch=patch, user=user) + self.client.authenticate(user=user) + + # Existing intention of reviewing + self.assertEqual( + len( + PatchAttentionSet.objects.filter(patch=patch, user=user).all() + ), + 1, + ) + + # remove intention + resp = self.client.patch( + self.api_url(patch.id), + {'attention_set': []}, + ) + + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertEqual( + len( + PatchAttentionSet.objects.filter(patch=patch, user=user).all() + ), + 0, + ) + + def test_remove_review_intention_of_others(self): + project = create_project() + state = create_state() + patch = create_patch(project=project, state=state) + user = create_user() + user2 = create_user() + create_attention_set(patch=patch, user=user2) + + self.client.authenticate(user=user) + + # remove intention + resp = self.client.patch( + self.api_url(patch.id), + {'attention_set': [-user2.id]}, + ) + + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual( + len( + PatchAttentionSet.objects.filter(patch=patch, user=user2).all() + ), + 1, + ) + + def test_remove_review_intention_of_others_as_maintainer(self): + project = create_project() + state = create_state() + patch = create_patch(project=project, state=state) + maintainer = create_maintainer(project) + user2 = create_user() + create_attention_set(patch=patch, user=user2) + + self.client.authenticate(user=maintainer) + + # remove intention + resp = self.client.patch( + self.api_url(patch.id), + {'attention_set': [-user2.id]}, + ) + + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertEqual( + len( + PatchAttentionSet.objects.filter(patch=patch, user=user2).all() + ), + 0, + ) + + def test_declare_review_intention_of_others(self): + project = create_project() + state = create_state() + patch = create_patch(project=project, state=state) + user = create_user() + maintainer = create_maintainer(project) + user2 = create_user() + self.client.authenticate(user=user) + + # declare intention + resp = self.client.patch( + self.api_url(patch.id), + {'attention_set': [user2.id]}, + ) + + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual( + len( + PatchAttentionSet.objects.filter(patch=patch, user=user).all() + ), + 0, + ) + + # maintaners also can't assign someone + self.client.authenticate(user=maintainer) + resp = self.client.patch( + self.api_url(patch.id), + {'attention_set': [user2.id]}, + ) + + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual( + len( + PatchAttentionSet.objects.filter(patch=patch, user=user).all() + ), + 0, + ) diff --git a/patchwork/tests/test_signals.py b/patchwork/tests/test_signals.py index c2c6cc6d..f7c45620 100644 --- a/patchwork/tests/test_signals.py +++ b/patchwork/tests/test_signals.py @@ -5,7 +5,7 @@ from django.test import TestCase -from patchwork.models import Event +from patchwork.models import Event, PatchAttentionSet from patchwork.tests import utils BASE_FIELDS = [ @@ -311,3 +311,19 @@ def test_patch_comment_created(self): ) self.assertEqual(events[0].project, comment.patch.project) self.assertEventFields(events[0]) + + def test_comment_removes_user_from_attention_set(self): + patch = utils.create_patch() + user = utils.create_user() + submitter = utils.create_person(user=user) + interest = utils.create_attention_set(patch=patch, user=user) + + # we have an active interest + self.assertFalse(interest.removed) + utils.create_patch_comment(patch=patch, submitter=submitter) + + attention_set = PatchAttentionSet.raw_objects.filter( + patch=patch, user=user + ).all() + self.assertEqual(len(attention_set), 1) + self.assertTrue(attention_set[0].removed) diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py index 4f404891..9d75ac13 100644 --- a/patchwork/tests/utils.py +++ b/patchwork/tests/utils.py @@ -11,7 +11,7 @@ from django.contrib.auth.models import User from django.utils import timezone as tz_utils -from patchwork.models import Bundle +from patchwork.models import Bundle, PatchAttentionSet from patchwork.models import Check from patchwork.models import Cover from patchwork.models import CoverComment @@ -206,6 +206,16 @@ def create_patch(**kwargs): return patch +def create_attention_set(**kwargs): + values = { + 'patch': create_patch() if 'patch' not in kwargs else None, + 'user': create_person() if 'user' not in kwargs else None, + } + values.update(kwargs) + + return PatchAttentionSet.objects.create(**values) + + def create_cover(**kwargs): """Create 'Cover' object.""" num = Cover.objects.count() diff --git a/patchwork/tests/views/test_patch.py b/patchwork/tests/views/test_patch.py index 3de558f0..eec50526 100644 --- a/patchwork/tests/views/test_patch.py +++ b/patchwork/tests/views/test_patch.py @@ -17,7 +17,7 @@ from patchwork.models import Check from patchwork.models import Patch from patchwork.models import State -from patchwork.tests.utils import create_check +from patchwork.tests.utils import create_attention_set, create_check from patchwork.tests.utils import create_maintainer from patchwork.tests.utils import create_patch from patchwork.tests.utils import create_patch_comment @@ -205,6 +205,10 @@ def test_utf8_handling(self): class PatchViewTest(TestCase): + def setUp(self): + self.project = create_project() + self.maintainer = create_maintainer(self.project) + def test_redirect(self): patch = create_patch() @@ -380,6 +384,122 @@ def test_patch_with_checks(self): ), ) + def test_patch_with_attention_set(self): + user = create_user() + patch = create_patch(project=self.project) + create_attention_set(patch=patch, user=user) + create_attention_set(patch=patch, user=self.maintainer) + + self.client.login( + username=self.maintainer.username, + password=self.maintainer.username, + ) + requested_url = reverse( + 'patch-detail', + kwargs={ + 'project_id': patch.project.linkname, + 'msgid': patch.encoded_msgid, + }, + ) + response = self.client.get(requested_url) + + # the response should contain attention set list + self.assertContains(response, '

Users pending actions

') + + # and it should show the existing users in the list + self.assertEqual( + response.content.decode().count( + f'{self.maintainer.username} ({self.maintainer.email})' + ), + 1, + ) + self.assertEqual( + response.content.decode().count(f'{user.username} ({user.email})'), + 1, + ) + + # should display remove button for all + self.assertEqual( + response.content.decode().count('glyphicon-trash'), + 2, + ) + + def test_patch_with_anonymous_user_with_attention_list(self): + # show not show a declare interest button nor remove buttons + user = create_user() + patch = create_patch(project=self.project) + create_attention_set(patch=patch, user=user) + create_attention_set(patch=patch, user=self.maintainer) + + requested_url = reverse( + 'patch-detail', + kwargs={ + 'project_id': patch.project.linkname, + 'msgid': patch.encoded_msgid, + }, + ) + response = self.client.get(requested_url) + + self.assertEqual( + response.content.decode().count('Declare interest'), + 0, + ) + self.assertEqual( + response.content.decode().count('glyphicon-trash'), + 0, + ) + + def test_patch_with_user_not_in_attention_list(self): + # a declare interest button should be displayed + patch = create_patch(project=self.project) + + self.client.login( + username=self.maintainer.username, + password=self.maintainer.username, + ) + requested_url = reverse( + 'patch-detail', + kwargs={ + 'project_id': patch.project.linkname, + 'msgid': patch.encoded_msgid, + }, + ) + response = self.client.get(requested_url) + + self.assertEqual( + response.content.decode().count('Declare interest'), + 1, + ) + + def test_patch_with_user_in_attention_list(self): + # a remove button should be displayed if he is authenticated + # should not show option for other users + user = create_user() + patch = create_patch(project=self.project) + create_attention_set(patch=patch, user=user) + create_attention_set(patch=patch, user=self.maintainer) + + self.client.login( + username=user.username, + password=user.username, + ) + requested_url = reverse( + 'patch-detail', + kwargs={ + 'project_id': patch.project.linkname, + 'msgid': patch.encoded_msgid, + }, + ) + response = self.client.get(requested_url) + self.assertEqual( + response.content.decode().count(f'{user.username} ({user.email})'), + 1, + ) + self.assertEqual( + response.content.decode().count('glyphicon-trash'), + 1, + ) + class PatchUpdateTest(TestCase): properties_form_id = 'patch-form-properties' diff --git a/patchwork/tests/views/test_series.py b/patchwork/tests/views/test_series.py new file mode 100644 index 00000000..c37a98df --- /dev/null +++ b/patchwork/tests/views/test_series.py @@ -0,0 +1,51 @@ +# Patchwork - automated patch tracking system +# Copyright (C) 2024 Meta Platforms, Inc. and affiliates. +# +# SPDX-License-Identifier: GPL-2.0-or-later + +from datetime import datetime as dt + +from django.test import TestCase +from django.urls import reverse + +from patchwork.models import Person +from patchwork.tests.utils import create_patch +from patchwork.tests.utils import create_cover +from patchwork.tests.utils import create_person +from patchwork.tests.utils import create_project +from patchwork.tests.utils import create_series +from patchwork.tests.utils import create_user + + +class SeriesList(TestCase): + def setUp(self): + self.project = create_project() + self.user = create_user() + self.person_1 = Person.objects.get(user=self.user) + self.person_2 = create_person() + self.series_1 = create_series(project=self.project) + self.series_2 = create_series(project=self.project) + create_cover(project=self.project, series=self.series_1) + + for i in range(5): + create_patch( + submitter=self.person_1, + project=self.project, + series=self.series_1, + date=dt(2014, 3, 16, 13, 4, 50, 155643), + ) + create_patch( + submitter=self.person_2, + project=self.project, + series=self.series_2, + date=dt(2014, 3, 16, 13, 4, 50, 155643), + ) + + def test_series_list(self): + requested_url = reverse( + 'series-list', + kwargs={'project_id': self.project.linkname}, + ) + response = self.client.get(requested_url) + + self.assertEqual(response.status_code, 200) diff --git a/patchwork/urls.py b/patchwork/urls.py index 11cd8e7c..9c036bb6 100644 --- a/patchwork/urls.py +++ b/patchwork/urls.py @@ -33,9 +33,19 @@ path('', project_views.project_list, name='project-list'), path( 'project//list/', + patch_views.patch_list_redirect, + name='patch-list-redirect', + ), + path( + 'project//patches/', patch_views.patch_list, name='patch-list', ), + path( + 'project//series/', + series_views.series_list, + name='series-list', + ), path( 'project//bundles/', bundle_views.bundle_list, @@ -110,6 +120,11 @@ name='comment-redirect', ), # series views + path( + 'project//series//', + series_views.series_detail, + name='series-detail', + ), path( 'series//mbox/', series_views.series_mbox, diff --git a/patchwork/views/__init__.py b/patchwork/views/__init__.py index db484c79..76f2ce20 100644 --- a/patchwork/views/__init__.py +++ b/patchwork/views/__init__.py @@ -344,7 +344,7 @@ def process_multiplepatch_form(request, form, action, patches, context): changed_patches = 0 for patch in patches: - if not patch.is_editable(request.user): + if not patch.is_editable(request.user, form.review_status_only()): errors.append( "You don't have permissions to edit patch '%s'" % patch.name ) diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py index efe94f17..9b7339cc 100644 --- a/patchwork/views/patch.py +++ b/patchwork/views/patch.py @@ -4,18 +4,21 @@ # SPDX-License-Identifier: GPL-2.0-or-later from django.contrib import messages +from django.contrib.auth.models import User from django.http import Http404 from django.http import HttpResponse from django.http import HttpResponseForbidden from django.http import HttpResponseRedirect from django.shortcuts import get_object_or_404 from django.shortcuts import render +from django.shortcuts import redirect from django.urls import reverse from patchwork.forms import CreateBundleForm from patchwork.forms import PatchForm from patchwork.models import Cover from patchwork.models import Patch +from patchwork.models import PatchAttentionSet from patchwork.models import Project from patchwork.views import generic_list from patchwork.views import set_bundle @@ -38,6 +41,11 @@ def patch_list(request, project_id): return render(request, 'patchwork/list.html', context) +def patch_list_redirect(request, project_id): + new_url = reverse('patch-list', kwargs={'project_id': project_id}) + return redirect(f'{new_url}?{request.GET.urlencode()}') + + def patch_detail(request, project_id, msgid): project = get_object_or_404(Project, linkname=project_id) db_msgid = Patch.decode_msgid(msgid) @@ -61,6 +69,10 @@ def patch_detail(request, project_id, msgid): editable = patch.is_editable(request.user) context = {'project': patch.project} + is_maintainer = ( + request.user.is_authenticated + and project in request.user.profile.maintainer_projects.all() + ) form = None create_bundle_form = None @@ -80,6 +92,50 @@ def patch_detail(request, project_id, msgid): errors = set_bundle( request, project, action, request.POST, [patch] ) + elif action in ['add-interest', 'remove-interest']: + if request.user.is_authenticated: + if action == 'add-interest': + PatchAttentionSet.objects.get_or_create( + patch=patch, user=request.user + ) + message = ( + 'You have declared interest in reviewing this patch' + ) + else: + user_id = request.POST.get('attention_set') + + if is_maintainer or user_id == str(request.user.id): + rm_user = User.objects.get(pk=user_id) + PatchAttentionSet.objects.filter( + patch=patch, user=rm_user + ).delete() + + rm_user_name = ( + f"'{rm_user.username}'" + if rm_user != request.user + else 'yourself' + ) + message = ( + f"You removed {rm_user_name} from patch's " + 'attention list' + ) + + patch.save() + messages.success( + request, + message, + ) + else: + messages.error( + request, + "You can't remove another user interest in this " + 'patch', + ) + else: + messages.error( + request, + 'You must be logged in to change the user attention list.', + ) elif not editable: return HttpResponseForbidden() @@ -93,6 +149,13 @@ def patch_detail(request, project_id, msgid): if request.user.is_authenticated: context['bundles'] = request.user.bundles.all() + attention_set = [ + data.user for data in PatchAttentionSet.objects.filter(patch=patch) + ] + + context['attention_set'] = attention_set + context['is_maintainer'] = is_maintainer + comments = patch.comments.all() comments = comments.select_related('submitter') comments = comments.only( @@ -127,6 +190,20 @@ def patch_detail(request, project_id, msgid): if errors: context['errors'] = errors + try: + context['previous_submission'] = Patch.objects.get( + series=patch.series, number=patch.number - 1 + ) + except Patch.DoesNotExist: + context['previous_submission'] = None + + try: + context['next_submission'] = Patch.objects.get( + series=patch.series, number=patch.number + 1 + ) + except Patch.DoesNotExist: + context['next_submission'] = None + return render(request, 'patchwork/submission.html', context) diff --git a/patchwork/views/series.py b/patchwork/views/series.py index a8892ae6..a36b8041 100644 --- a/patchwork/views/series.py +++ b/patchwork/views/series.py @@ -2,12 +2,18 @@ # Copyright (C) 2017 Stephen Finucane # # SPDX-License-Identifier: GPL-2.0-or-later +import collections from django.http import HttpResponse from django.shortcuts import get_object_or_404 +from django.shortcuts import render from patchwork.models import Series +from patchwork.models import Patch +from patchwork.models import Project +from patchwork.views import generic_list from patchwork.views.utils import series_to_mbox +from patchwork.paginator import Paginator def series_mbox(request, series_id): @@ -20,3 +26,64 @@ def series_mbox(request, series_id): ) return response + + +def series_detail(request, project_id, series_id): + series = get_object_or_404(Series, id=series_id) + + patches = Patch.objects.filter(series=series) + + context = generic_list( + request, + series.project, + 'series-detail', + view_args={ + 'project_id': project_id, + 'series_id': series_id, + }, + patches=patches, + ) + + context.update({'series': series}) + + return render(request, 'patchwork/series-detail.html', context) + + +def series_list(request, project_id): + project = get_object_or_404(Project, linkname=project_id) + sort = request.GET.get('order', 'date.desc') + sort_field, sort_dir = sort.split('.') + sort_order = f"{'-' if sort_dir == 'desc' else ''}{sort_field}" + context = {} + series_list = ( + Series.objects.filter(project=project) + .only( + 'submitter', + 'project', + 'version', + 'name', + 'date', + 'id', + 'cover_letter', + ) + .select_related('project', 'submitter', 'cover_letter') + .order_by(sort_order) + ) + + paginator = Paginator(request, series_list) + context.update( + { + 'project': project, + 'projects': Project.objects.all(), + 'series_list': series_list, + 'page': paginator.current_page, + 'order': sort, + 'list_view': { + 'view': 'series-list', + 'view_params': {'project_id': project.linkname}, + 'params': collections.OrderedDict(), + }, + } + ) + + return render(request, 'patchwork/series-list.html', context) diff --git a/releasenotes/notes/add-series-dependencies-6696458586e795c7.yaml b/releasenotes/notes/add-series-dependencies-6696458586e795c7.yaml index 3cb80fef..c8fb6a7f 100644 --- a/releasenotes/notes/add-series-dependencies-6696458586e795c7.yaml +++ b/releasenotes/notes/add-series-dependencies-6696458586e795c7.yaml @@ -13,6 +13,7 @@ features: ``Depends-on: <20240726221429.221611-1-user@example.com>`` Alternatively, the web URL of the patch or series may be given: ``Depends-on: http://patchwork.example.com/project/test/list?series=1111`` + ``Depends-on: http://patchwork.example.com/project/test/series/1111`` api: - | The API version has been updated to v1.4. diff --git a/releasenotes/notes/add_series_list_view-bf219022216fea6a.yaml b/releasenotes/notes/add_series_list_view-bf219022216fea6a.yaml new file mode 100644 index 00000000..c8b32e6a --- /dev/null +++ b/releasenotes/notes/add_series_list_view-bf219022216fea6a.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + A series view is now available, allowing users to list available series and + view details of individual series. diff --git a/templates/base.html b/templates/base.html index 9519ecc5..49663d7f 100644 --- a/templates/base.html +++ b/templates/base.html @@ -48,6 +48,12 @@ {% block navbarmenu %} {% if project %}