Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Issue #507] Allow non delegate users to declare interest in a patch #588

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
90 changes: 77 additions & 13 deletions patchwork/api/patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import email.parser

from django.core.exceptions import ValidationError
from django.utils import timezone
from django.utils.text import slugify
from django.utils.translation import gettext_lazy as _
from rest_framework.exceptions import APIException
Expand All @@ -17,6 +18,7 @@
from rest_framework.relations import RelatedField
from rest_framework.reverse import reverse
from rest_framework.serializers import SerializerMethodField
from rest_framework import permissions
from rest_framework import status

from patchwork.api.base import BaseHyperlinkedModelSerializer
Expand All @@ -28,8 +30,10 @@
from patchwork.api.embedded import UserSerializer
from patchwork.api.filters import PatchFilterSet
from patchwork.models import Patch
from patchwork.models import PatchReviewIntention
from patchwork.models import PatchRelation
from patchwork.models import State
from patchwork.models import User
from patchwork.parser import clean_subject


Expand Down Expand Up @@ -76,12 +80,41 @@ class PatchConflict(APIException):
)


class PatchReviewIntentionSerializer(BaseHyperlinkedModelSerializer):
user = UserSerializer()
patch = PatchSerializer()
is_stale = SerializerMethodField()

def get_is_stale(self, review_intention):
expiration_time = (
review_intention.patch.state.review_intention_expiration_time
)
valid_until = (
review_intention.last_time_marked_for_review + expiration_time
)

return timezone.now() > valid_until

class Meta:
model = PatchReviewIntention
fields = [
'id',
'user',
'patch',
'last_time_marked_for_review',
'is_stale',
]


class PatchListSerializer(BaseHyperlinkedModelSerializer):
web_url = SerializerMethodField()
project = ProjectSerializer(read_only=True)
state = StateField()
submitter = PersonSerializer(read_only=True)
delegate = UserSerializer(allow_null=True)
planning_to_review = PatchReviewIntentionSerializer(
source='patchreviewintention_set', many=True
)
mbox = SerializerMethodField()
series = SeriesSerializer(read_only=True)
comments = SerializerMethodField()
Expand Down Expand Up @@ -170,6 +203,8 @@ class Meta:
'hash',
'submitter',
'delegate',
'planning_to_review',
'has_planned_review',
'mbox',
'series',
'comments',
Expand All @@ -188,6 +223,7 @@ class Meta:
'name',
'hash',
'submitter',
'has_planned_review',
'mbox',
'series',
'comments',
Expand Down Expand Up @@ -228,16 +264,11 @@ 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_planning_to_review(self, instance, patchreviewintention_set):
intereted_user = patchreviewintention_set.pop()['user']
instance.planning_to_review.add(intereted_user.id)

def update_related(self, instance, related):
# Validation rules
# ----------------
#
Expand Down Expand Up @@ -278,9 +309,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}
Expand All @@ -304,6 +333,20 @@ 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)

if 'patchreviewintention_set' in validated_data:
patchreviewintention_set = validated_data.pop(
'patchreviewintention_set'
)
self.update_planning_to_review(instance, patchreviewintention_set)

return super(PatchDetailSerializer, self).update(
instance, validated_data
)
Expand Down Expand Up @@ -367,12 +410,33 @@ def get_queryset(self):
'project',
'series__project',
'related__patches__project',
'patchreviewintention_set__user',
)
.select_related('state', 'submitter', 'series')
.defer('content', 'diff', 'headers')
)


class PatchDetailPermission(PatchworkPermission):
non_delegate_editable_fields = set(['planning_to_review'])

def has_object_permission(self, request, view, obj):
if request.method in permissions.SAFE_METHODS:
return True

data = request.data

if set(data.keys()).issubset(self.non_delegate_editable_fields):
user_id = data['planning_to_review'][0]['user']
reviewing_user = User.objects.get(id=user_id)
if request.user == reviewing_user:
return True
detail = "Only the user can declare it's own intention to reviewing a patch"
raise PermissionDenied(detail=detail)
else:
return super().has_object_permission(self, request, view, obj)


class PatchDetail(RetrieveUpdateAPIView):
"""
get:
Expand All @@ -385,7 +449,7 @@ class PatchDetail(RetrieveUpdateAPIView):
Update a patch.
"""

permission_classes = (PatchworkPermission,)
permission_classes = (PatchDetailPermission,)
serializer_class = PatchDetailSerializer

def get_queryset(self):
Expand Down
34 changes: 33 additions & 1 deletion patchwork/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,14 +190,27 @@ class MultiplePatchForm(forms.Form):
empty_value='*',
)

def __init__(self, project, *args, **kwargs):
def __init__(self, project, user=None, *args, **kwargs):
super(MultiplePatchForm, self).__init__(*args, **kwargs)
self.fields['delegate'] = OptionalModelChoiceField(
queryset=_get_delegate_qs(project=project), required=False
)
self.fields['state'] = OptionalModelChoiceField(
queryset=State.objects.all()
)
self.user = user
if self.user:
self.fields['review_status'] = OptionalBooleanField(
choices=[
('*', 'no change'),
('True', 'Planning to review'),
('False', 'Not planning to review'),
],
coerce=lambda x: x == 'True',
empty_value='*',
required=False,
initial='*',
)

def save(self, instance, commit=True):
opts = instance.__class__._meta
Expand All @@ -219,8 +232,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.planning_to_review.add(self.user)
else:
self.instance.planning_to_review.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
25 changes: 25 additions & 0 deletions patchwork/management/commands/update_reviewers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Patchwork - automated patch tracking system
# Copyright (C) 2015 Jeremy Kerr <[email protected]>
#
# SPDX-License-Identifier: GPL-2.0-or-later

from django.core.management.base import BaseCommand
from patchwork.models import Patch
from patchwork.api.patch import PatchReviewIntentionSerializer


class Command(BaseCommand):
help = 'Updates the patch has_planned_review field'

def handle(self, *args, **kwargs):
for patch in Patch.objects.all():
has_planned_review = False
for (
patch_interest
) in patch.planning_to_review.through.objects.filter(patch=patch):
serializer = PatchReviewIntentionSerializer(patch_interest)
if not serializer.data['is_stale']:
has_planned_review = True
break
patch.has_planned_review = has_planned_review
patch.save()
71 changes: 71 additions & 0 deletions patchwork/migrations/0047_patch_has_planned_review_and_more.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
# Generated by Django 5.0.4 on 2024-04-16 04:31

import datetime
import django.db.models.deletion
import django.utils.timezone
from django.conf import settings
from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
('patchwork', '0046_patch_comment_events'),
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
]

operations = [
migrations.AddField(
model_name='patch',
name='has_planned_review',
field=models.BooleanField(default=False),
),
migrations.AddField(
model_name='state',
name='review_intention_expiration_time',
field=models.DurationField(default=datetime.timedelta(days=30)),
),
migrations.CreateModel(
name='PatchReviewIntention',
fields=[
(
'id',
models.AutoField(
auto_created=True,
primary_key=True,
serialize=False,
verbose_name='ID',
),
),
(
'last_time_marked_for_review',
models.DateTimeField(default=django.utils.timezone.now),
),
(
'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='planning_to_review',
field=models.ManyToManyField(
related_name='planning_to_review',
through='patchwork.PatchReviewIntention',
to=settings.AUTH_USER_MODEL,
),
),
]
Loading