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/htdocs/css/style.css b/htdocs/css/style.css index 268a8c37..ede3314c 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; 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..9b8e1eb5 100644 --- a/patchwork/models.py +++ b/patchwork/models.py @@ -503,6 +503,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 +582,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 +593,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 +831,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.""" diff --git a/patchwork/templates/patchwork/partials/patch-list.html b/patchwork/templates/patchwork/partials/patch-list.html index 981ceee5..fcc4793e 100644 --- a/patchwork/templates/patchwork/partials/patch-list.html +++ b/patchwork/templates/patchwork/partials/patch-list.html @@ -80,6 +80,10 @@ S/W/F +