[PATCH 4/4] REST: Add patch relations
Daniel Axtens
dja at axtens.net
Tue Feb 11 17:06:47 AEDT 2020
Daniel Axtens <dja at axtens.net> writes:
> From: Mete Polat <metepolat2000 at gmail.com>
>
> View relations and add/update/delete them as a maintainer. Maintainers
> can only create relations of patches which are part of a project they
> maintain.
>
> I've significantly rewritten this from Mete's original submission based
> on how Stephen wants the API to work. In short:
>
> - all operations use PATCH to the 'related' field of a patch
>
> - to relate a patch to another patch, say 7 to 19, either:
> PATCH /api/patch/7 related := [19]
> PATCH /api/patch/19 related := [7]
>
> - to delete a patch from a relation, say 1, 21 and 42 are related but we
> only want it to be 1 and 42:
> PATCH /api/patch/21 related := []
>
> * You _cannot_ remove a patch from a relation by patching another
> patch in the relation: I'm trying to avoid read-modify-write loops.
>
> * Relations that would be left with just 1 patch are deleted. This is
> only ensured in the API - the admin interface will let you do this.
>
> - break-before-make: if you have [1, 12, 24] and [7, 15, 42] and you want
> to end up with [1, 12, 15, 42], you have to remove 15 from the second
> relation first:
> PATCH /api/patch/1 related := [15] will fail with 409 Conflict.
> Instead do:
> PATCH /api/patch/15 related := []
> PATCH /api/patch/1 related := [15]
> -> 200 OK, [1, 12, 15, 42] and [7, 42] are the resulting relations
>
> I think this allows you to do things in a reasonably sensible way.
>
> I've preserved the BMW copyright header and moved it into api/patch.py,
> because I have taken some chunks of code verbatim. I have dropped it from
> the tests because I have completely rewritten them in the process of redoing
> the API.
>
> Signed-off-by: Mete Polat <metepolat2000 at gmail.com>
> Signed-off-by: Daniel Axtens <dja at axtens.net>
>
> ---
>
> Todos:
> - j2 API docs
> - pep8, tox tests
> - optimise db queries: I haven't optimised the queries yet because I keep getting
> sidetracked by the random queries for a project linkname
Ok, so as I've mentioned, I can avoid the random queries by doing
prefetch_related('related__patches__project'), however, this does a full
fetch of related__patches, which includes the really big things like
content/diff/header that we try to avoid fetching when we don't have to.
Django won't let me do .defer('related__patches__diff') because patches
isn't actually a field of PatchRelation.
I wonder if we give up and drop to raw SQL at this point, if that's even
possible for a QS with prefetch_related? I guess we could maybe cheat
with another model over the same tables that doesn't include the big
fields? Or just accept the penalty for fetching all of related patches
and hope for the best? (bearing in mind that we don't limit the size of
relations so this could get really pathological)
Thoughts?
Daniel
> - general nitpickery
>
> ---
> patchwork/api/embedded.py | 23 ++
> patchwork/api/event.py | 3 +
> patchwork/api/patch.py | 112 ++++++-
> patchwork/tests/api/test_relation.py | 304 ++++++++++++++++++
> patchwork/tests/utils.py | 11 +
> .../add-patch-relations-c96bb6c567b416d8.yaml | 11 +
> 6 files changed, 461 insertions(+), 3 deletions(-)
> create mode 100644 patchwork/tests/api/test_relation.py
> create mode 100644 releasenotes/notes/add-patch-relations-c96bb6c567b416d8.yaml
>
> diff --git a/patchwork/api/embedded.py b/patchwork/api/embedded.py
> index de4f31165ee7..506873c4adb6 100644
> --- a/patchwork/api/embedded.py
> +++ b/patchwork/api/embedded.py
> @@ -14,6 +14,7 @@ from collections import OrderedDict
> from rest_framework.serializers import CharField
> from rest_framework.serializers import SerializerMethodField
> from rest_framework.serializers import PrimaryKeyRelatedField
> +from rest_framework.serializers import ValidationError
>
> from patchwork.api.base import BaseHyperlinkedModelSerializer
> from patchwork.api.base import CheckHyperlinkedIdentityField
> @@ -138,6 +139,28 @@ class PatchSerializer(SerializedRelatedField):
> }
>
>
> +class PatchRelationSerializer(BaseHyperlinkedModelSerializer):
> + """Hide the PatchRelation model, just show the list"""
> + patches = PatchSerializer(many=True)
> +
> + def to_internal_value(self, data):
> + if not isinstance(data, type([])):
> + raise ValidationError(
> + "Patch relations must be specified as a list of patch IDs"
> + )
> + result = super(PatchRelationSerializer, self).to_internal_value({'patches': data})
> + return result
> +
> + def to_representation(self, instance):
> + data = super(PatchRelationSerializer, self).to_representation(instance)
> + data = data['patches']
> + return data
> +
> + class Meta:
> + model = models.PatchRelation
> + fields = ('patches',)
> +
> +
> class PersonSerializer(SerializedRelatedField):
>
> class _Serializer(BaseHyperlinkedModelSerializer):
> diff --git a/patchwork/api/event.py b/patchwork/api/event.py
> index d7685f4c138c..b8b535040116 100644
> --- a/patchwork/api/event.py
> +++ b/patchwork/api/event.py
> @@ -13,6 +13,7 @@ from rest_framework.serializers import SlugRelatedField
> from patchwork.api.embedded import CheckSerializer
> from patchwork.api.embedded import CoverLetterSerializer
> from patchwork.api.embedded import PatchSerializer
> +from patchwork.api.embedded import PatchRelationSerializer
> from patchwork.api.embedded import ProjectSerializer
> from patchwork.api.embedded import SeriesSerializer
> from patchwork.api.embedded import UserSerializer
> @@ -33,6 +34,8 @@ class EventSerializer(ModelSerializer):
> current_delegate = UserSerializer()
> created_check = SerializerMethodField()
> created_check = CheckSerializer()
> + previous_relation = PatchRelationSerializer(read_only=True)
> + current_relation = PatchRelationSerializer(read_only=True)
>
> _category_map = {
> Event.CATEGORY_COVER_CREATED: ['cover'],
> diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
> index a29a1ab0eb71..996547be1e96 100644
> --- a/patchwork/api/patch.py
> +++ b/patchwork/api/patch.py
> @@ -1,5 +1,7 @@
> # Patchwork - automated patch tracking system
> # Copyright (C) 2016 Linaro Corporation
> +# Copyright (C) 2019, Bayerische Motoren Werke Aktiengesellschaft (BMW AG)
> +# Copyright (C) 2020, IBM Corporation
> #
> # SPDX-License-Identifier: GPL-2.0-or-later
>
> @@ -7,21 +9,25 @@ import email.parser
>
> from django.utils.text import slugify
> from django.utils.translation import ugettext_lazy as _
> +from rest_framework import status
> +from rest_framework.exceptions import APIException
> +from rest_framework.exceptions import PermissionDenied
> from rest_framework.generics import ListAPIView
> from rest_framework.generics import RetrieveUpdateAPIView
> from rest_framework.relations import RelatedField
> from rest_framework.reverse import reverse
> from rest_framework.serializers import SerializerMethodField
> -from rest_framework.serializers import ValidationError
>
> from patchwork.api.base import BaseHyperlinkedModelSerializer
> from patchwork.api.base import PatchworkPermission
> from patchwork.api.filters import PatchFilterSet
> +from patchwork.api.embedded import PatchRelationSerializer
> from patchwork.api.embedded import PersonSerializer
> from patchwork.api.embedded import ProjectSerializer
> from patchwork.api.embedded import SeriesSerializer
> from patchwork.api.embedded import UserSerializer
> from patchwork.models import Patch
> +from patchwork.models import PatchRelation
> from patchwork.models import State
> from patchwork.parser import clean_subject
>
> @@ -54,6 +60,13 @@ class StateField(RelatedField):
> return State.objects.all()
>
>
> +class PatchConflict(APIException):
> + status_code = status.HTTP_409_CONFLICT
> + default_detail = 'At least one patch is already part of another ' \
> + 'relation. You have to explicitly remove a patch ' \
> + 'from its existing relation before moving it to this one.'
> +
> +
> class PatchListSerializer(BaseHyperlinkedModelSerializer):
>
> web_url = SerializerMethodField()
> @@ -67,6 +80,7 @@ class PatchListSerializer(BaseHyperlinkedModelSerializer):
> check = SerializerMethodField()
> checks = SerializerMethodField()
> tags = SerializerMethodField()
> + related = PatchRelationSerializer()
>
> def get_web_url(self, instance):
> request = self.context.get('request')
> @@ -109,6 +123,15 @@ class PatchListSerializer(BaseHyperlinkedModelSerializer):
> # will be removed in API v2
> data = super(PatchListSerializer, self).to_representation(instance)
> data['series'] = [data['series']] if data['series'] else []
> +
> + # stop the related serializer returning this patch in the list of
> + # related patches. Also make it return an empty list, not null/None
> + if data['related']:
> + data['related'] = [p for p in data['related']
> + if p['id'] != instance.id]
> + else:
> + data['related'] = []
> +
> return data
>
> class Meta:
> @@ -116,13 +139,13 @@ class PatchListSerializer(BaseHyperlinkedModelSerializer):
> fields = ('id', 'url', 'web_url', 'project', 'msgid',
> 'list_archive_url', 'date', 'name', 'commit_ref', 'pull_url',
> 'state', 'archived', 'hash', 'submitter', 'delegate', 'mbox',
> - 'series', 'comments', 'check', 'checks', 'tags')
> + 'series', 'comments', 'check', 'checks', 'tags', 'related',)
> read_only_fields = ('web_url', 'project', 'msgid', 'list_archive_url',
> 'date', 'name', 'hash', 'submitter', 'mbox',
> 'series', 'comments', 'check', 'checks', 'tags')
> versioned_fields = {
> '1.1': ('comments', 'web_url'),
> - '1.2': ('list_archive_url',),
> + '1.2': ('list_archive_url', 'related',),
> }
> extra_kwargs = {
> 'url': {'view_name': 'api-patch-detail'},
> @@ -151,6 +174,89 @@ class PatchDetailSerializer(PatchListSerializer):
> 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')
> +
> + # Validation rules
> + # ----------------
> + #
> + # Permissions: to change a relation:
> + # for all patches in the relation, current and proposed,
> + # the user must be maintainer of the patch's project
> + # Note that this has a ratchet effect: if you add a cross-project
> + # relation, only you or another maintainer across both projects can
> + # modify that relationship in _any way_.
> + #
> + # Break before Make: a patch must be explicitly removed from a
> + # relation before being added to another
> + #
> + # No Read-Modify-Write for deletion:
> + # to delete a patch from a relation, clear _its_ related patch,
> + # don't modify one of the patches that are to remain.
> + #
> + # (As a consequence of those two, operations are additive:
> + # if 1 is in a relation with [1,2,3], then
> + # patching 1 with related=[2,4] gives related=[1,2,3,4])
> +
> + # Permissions:
> + # Because we're in a serializer, not a view, this is a bit clunky
> + user = self.context['request'].user.profile
> + # Must be maintainer of:
> + # - current patch
> + self.check_user_maintains_all(user, [instance])
> + # - all patches currently in relation
> + # - all patches proposed to be in relation
> + patches = set(related['patches']) if related else {}
> + if instance.related is not None:
> + patches = patches.union(instance.related.patches.all())
> + self.check_user_maintains_all(user, patches)
> +
> + # handle deletion
> + if not related['patches']:
> + # do not allow relations with a single patch
> + if instance.related and instance.related.patches.count() == 2:
> + instance.related.delete()
> + instance.related = None
> + return super(PatchDetailSerializer, self).update(instance, validated_data)
> +
> + # break before make
> + relations = {patch.related for patch in patches if patch.related}
> + if len(relations) > 1:
> + raise PatchConflict()
> + if relations:
> + relation = relations.pop()
> + else:
> + relation = None
> + if relation and instance.related is not None:
> + if instance.related != relation:
> + raise PatchConflict()
> +
> + # apply
> + if relation is None:
> + relation = PatchRelation()
> + relation.save()
> + for patch in patches:
> + patch.related = relation
> + patch.save()
> + instance.related = relation
> + instance.save()
> +
> + return super(PatchDetailSerializer, self).update(instance, validated_data)
> +
> + @staticmethod
> + def check_user_maintains_all(user, patches):
> + maintains = user.maintainer_projects.all()
> + if any(s.project not in maintains for s in patches):
> + detail = 'At least one patch is part of a project you are ' \
> + 'not maintaining.'
> + raise PermissionDenied(detail=detail)
> + return True
> +
> class Meta:
> model = Patch
> fields = PatchListSerializer.Meta.fields + (
> diff --git a/patchwork/tests/api/test_relation.py b/patchwork/tests/api/test_relation.py
> new file mode 100644
> index 000000000000..fae9eeef5ada
> --- /dev/null
> +++ b/patchwork/tests/api/test_relation.py
> @@ -0,0 +1,304 @@
> +# Patchwork - automated patch tracking system
> +# Copyright (C) 2020, IBM Corporation
> +# Author: Daniel Axtens
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +import unittest
> +
> +import six
> +from django.conf import settings
> +from django.urls import reverse
> +
> +from patchwork.models import Patch
> +from patchwork.models import PatchRelation
> +
> +from patchwork.tests.api import utils
> +from patchwork.tests.utils import create_maintainer
> +from patchwork.tests.utils import create_patch
> +from patchwork.tests.utils import create_patches
> +from patchwork.tests.utils import create_project
> +from patchwork.tests.utils import create_relation
> +from patchwork.tests.utils import create_user
> +
> +if settings.ENABLE_REST_API:
> + from rest_framework import status
> +
> +
> + at unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
> +class TestRelationSimpleAPI(utils.APITestCase):
> +
> + @staticmethod
> + def api_url(item=None, version=None):
> + kwargs = {}
> + if version:
> + kwargs['version'] = version
> +
> + if item is None:
> + return reverse('api-patch-list', kwargs=kwargs)
> + kwargs['pk'] = item
> + return reverse('api-patch-detail', kwargs=kwargs)
> +
> + def setUp(self):
> + self.project = create_project()
> + self.normal_user = create_user()
> + self.maintainer = create_maintainer(self.project)
> +
> + def test_no_relation(self):
> + patch = create_patch(project=self.project)
> + resp = self.client.get(self.api_url(item=patch.pk))
> + self.assertEqual(resp.status_code, status.HTTP_200_OK)
> + self.assertEqual(resp.data['related'], [])
> +
> + @utils.store_samples('relation-list')
> + def test_list_two_patch_relation(self):
> + relation = create_relation(2, project=self.project)
> + patches = relation.patches.all()
> +
> + # nobody
> + resp = self.client.get(self.api_url(item=patches[0].pk))
> + self.assertEqual(resp.status_code, status.HTTP_200_OK)
> +
> + self.assertIn('related', resp.data)
> + self.assertEqual(len(resp.data['related']), 1)
> + self.assertEqual(resp.data['related'][0]['id'], patches[1].pk)
> +
> + resp = self.client.get(self.api_url(item=patches[1].pk))
> + self.assertEqual(resp.status_code, status.HTTP_200_OK)
> +
> + self.assertIn('related', resp.data)
> + self.assertEqual(len(resp.data['related']), 1)
> + self.assertEqual(resp.data['related'][0]['id'], patches[0].pk)
> +
> + @utils.store_samples('relation-create-forbidden')
> + def test_create_two_patch_relation_nobody(self):
> + patches = create_patches(2, project=self.project)
> +
> + resp = self.client.patch(self.api_url(item=patches[0].pk),
> + {'related': [patches[1].pk]})
> + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
> +
> + def test_create_two_patch_relation_user(self):
> + patches = create_patches(2, project=self.project)
> +
> + self.client.force_authenticate(user=self.normal_user)
> + resp = self.client.patch(self.api_url(item=patches[0].pk),
> + {'related': [patches[1].pk]})
> + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
> +
> + @utils.store_samples('relation-create-maintainer')
> + def test_create_two_patch_relation_maintainer(self):
> + patches = create_patches(2, project=self.project)
> +
> + self.client.force_authenticate(user=self.maintainer)
> + resp = self.client.patch(self.api_url(item=patches[0].pk),
> + {'related': [patches[1].pk]})
> + self.assertEqual(resp.status_code, status.HTTP_200_OK)
> +
> + # reload and verify
> + patches = Patch.objects.all()
> + self.assertIsNotNone(patches[0].related)
> + self.assertIsNotNone(patches[1].related)
> + self.assertEqual(patches[1].related, patches[0].related)
> +
> + def test_delete_two_patch_relation_nobody(self):
> + relation = create_relation(project=self.project)
> + patch = relation.patches.all()[0]
> +
> + self.assertEqual(PatchRelation.objects.count(), 1)
> +
> + resp = self.client.patch(self.api_url(item=patch.pk),
> + {'related': []})
> + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
> + self.assertEqual(PatchRelation.objects.count(), 1)
> +
> + @utils.store_samples('relation-delete')
> + def test_delete_two_patch_relation_maintainer(self):
> + relation = create_relation(project=self.project)
> + patch = relation.patches.all()[0]
> +
> + self.assertEqual(PatchRelation.objects.count(), 1)
> +
> + self.client.force_authenticate(user=self.maintainer)
> + resp = self.client.patch(self.api_url(item=patch.pk),
> + {'related': []})
> + self.assertEqual(resp.status_code, status.HTTP_200_OK)
> +
> + self.assertEqual(PatchRelation.objects.count(), 0)
> + self.assertEqual(Patch.objects.filter(related__isnull=False).exists(),
> + False)
> +
> + def test_create_three_patch_relation(self):
> + patches = create_patches(3, project=self.project)
> +
> + self.client.force_authenticate(user=self.maintainer)
> + resp = self.client.patch(self.api_url(item=patches[0].pk),
> + {'related': [patches[1].pk, patches[2].pk]})
> + self.assertEqual(resp.status_code, status.HTTP_200_OK)
> +
> + # reload and verify
> + patches = Patch.objects.all()
> + self.assertIsNotNone(patches[0].related)
> + self.assertIsNotNone(patches[1].related)
> + self.assertIsNotNone(patches[2].related)
> + self.assertEqual(patches[0].related, patches[1].related)
> + self.assertEqual(patches[1].related, patches[2].related)
> +
> + def test_delete_from_three_patch_relation(self):
> + relation = create_relation(3, project=self.project)
> + patch = relation.patches.all()[0]
> +
> + self.assertEqual(PatchRelation.objects.count(), 1)
> +
> + self.client.force_authenticate(user=self.maintainer)
> + resp = self.client.patch(self.api_url(item=patch.pk),
> + {'related': []})
> + self.assertEqual(resp.status_code, status.HTTP_200_OK)
> + self.assertIsNone(Patch.objects.get(id=patch.pk).related)
> + self.assertEqual(PatchRelation.objects.count(), 1)
> + self.assertEqual(PatchRelation.objects.first().patches.count(), 2)
> +
> + @utils.store_samples('relation-extend-through-new')
> + def test_extend_relation_through_new(self):
> + relation = create_relation(project=self.project)
> + existing_patch_a = relation.patches.first()
> +
> + new_patch = create_patch(project=self.project)
> +
> + self.client.force_authenticate(user=self.maintainer)
> + resp = self.client.patch(self.api_url(item=new_patch.pk),
> + {'related': [existing_patch_a.pk]})
> + self.assertEqual(resp.status_code, status.HTTP_200_OK)
> + self.assertEqual(relation,
> + Patch.objects.get(pk=new_patch.pk).related)
> + self.assertEqual(relation.patches.count(), 3)
> +
> + def test_extend_relation_through_old(self):
> + relation = create_relation(project=self.project)
> + existing_patch_a = relation.patches.first()
> +
> + new_patch = create_patch(project=self.project)
> +
> + # maintainer
> + self.client.force_authenticate(user=self.maintainer)
> + resp = self.client.patch(self.api_url(item=existing_patch_a.pk),
> + {'related': [new_patch.pk]})
> + self.assertEqual(resp.status_code, status.HTTP_200_OK)
> + self.assertEqual(relation,
> + Patch.objects.get(id=new_patch.id).related)
> + self.assertEqual(relation.patches.count(), 3)
> +
> + def test_extend_relation_through_new_two(self):
> + relation = create_relation(project=self.project)
> + existing_patch_a = relation.patches.first()
> +
> + new_patch_a = create_patch(project=self.project)
> + new_patch_b = create_patch(project=self.project)
> +
> + self.client.force_authenticate(user=self.maintainer)
> + resp = self.client.patch(self.api_url(item=new_patch_a.pk),
> + {'related': [existing_patch_a.pk,
> + new_patch_b.pk]})
> + self.assertEqual(resp.status_code, status.HTTP_200_OK)
> + self.assertEqual(relation,
> + Patch.objects.get(id=new_patch_a.id).related)
> + self.assertEqual(relation,
> + Patch.objects.get(id=new_patch_b.id).related)
> + self.assertEqual(relation.patches.count(), 4)
> +
> + @utils.store_samples('relation-extend-through-old')
> + def test_extend_relation_through_old_two(self):
> + relation = create_relation(project=self.project)
> + existing_patch_a = relation.patches.first()
> +
> + new_patch_a = create_patch(project=self.project)
> + new_patch_b = create_patch(project=self.project)
> +
> + # maintainer
> + self.client.force_authenticate(user=self.maintainer)
> + resp = self.client.patch(self.api_url(item=existing_patch_a.pk),
> + {'related': [new_patch_a.pk, new_patch_b.pk]})
> + self.assertEqual(resp.status_code, status.HTTP_200_OK)
> + self.assertEqual(relation,
> + Patch.objects.get(id=new_patch_a.id).related)
> + self.assertEqual(relation,
> + Patch.objects.get(id=new_patch_b.id).related)
> + self.assertEqual(relation.patches.count(), 4)
> +
> + def test_remove_one_patch_from_relation_bad(self):
> + relation = create_relation(3, project=self.project)
> + keep_patch_a = relation.patches.all()[1]
> + keep_patch_b = relation.patches.all()[2]
> +
> + # this should do nothing - it is interpreted as
> + # _adding_ keep_patch_b again which is a no-op.
> +
> + # maintainer
> + self.client.force_authenticate(user=self.maintainer)
> + resp = self.client.patch(self.api_url(item=keep_patch_a.pk),
> + {'related': [keep_patch_b.pk]})
> + self.assertEqual(resp.status_code, status.HTTP_200_OK)
> + self.assertEqual(relation.patches.count(), 3)
> +
> + def test_remove_one_patch_from_relation_good(self):
> + relation = create_relation(3, project=self.project)
> + target_patch = relation.patches.all()[0]
> +
> + # maintainer
> + self.client.force_authenticate(user=self.maintainer)
> + resp = self.client.patch(self.api_url(item=target_patch.pk),
> + {'related': []})
> + self.assertEqual(resp.status_code, status.HTTP_200_OK)
> + self.assertIsNone(Patch.objects.get(id=target_patch.id).related)
> + self.assertEqual(relation.patches.count(), 2)
> +
> + @utils.store_samples('relation-forbid-moving-between-relations')
> + def test_forbid_moving_patch_between_relations(self):
> + """Test the break-before-make logic"""
> + relation_a = create_relation(project=self.project)
> + relation_b = create_relation(project=self.project)
> +
> + patch_a = relation_a.patches.first()
> + patch_b = relation_b.patches.first()
> +
> + self.client.force_authenticate(user=self.maintainer)
> + resp = self.client.patch(self.api_url(item=patch_a.pk),
> + {'related': [patch_b.pk]})
> + self.assertEqual(resp.status_code, status.HTTP_409_CONFLICT)
> +
> + resp = self.client.patch(self.api_url(item=patch_b.pk),
> + {'related': [patch_a.pk]})
> + self.assertEqual(resp.status_code, status.HTTP_409_CONFLICT)
> +
> + def test_cross_project_different_maintainers(self):
> + patch_a = create_patch(project=self.project)
> +
> + project_b = create_project()
> + patch_b = create_patch(project=project_b)
> +
> + # maintainer a, patch in own project
> + self.client.force_authenticate(user=self.maintainer)
> + resp = self.client.patch(self.api_url(item=patch_a.pk),
> + {'related': [patch_b.pk]})
> + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
> +
> + # maintainer a, patch in project b
> + resp = self.client.patch(self.api_url(item=patch_b.pk),
> + {'related': [patch_a.pk]})
> + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
> +
> + def test_cross_project_relation_super_maintainer(self):
> + patch_a = create_patch(project=self.project)
> +
> + project_b = create_project()
> + patch_b = create_patch(project=project_b)
> +
> + project_b.maintainer_project.add(self.maintainer.profile)
> + project_b.save()
> +
> + self.client.force_authenticate(user=self.maintainer)
> + resp = self.client.patch(self.api_url(item=patch_a.pk),
> + {'related': [patch_b.pk]})
> + self.assertEqual(resp.status_code, status.HTTP_200_OK)
> + self.assertEqual(Patch.objects.get(id=patch_a.id).related,
> + Patch.objects.get(id=patch_b.id).related)
> diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py
> index 5e6a1b130394..7759c8f37a30 100644
> --- a/patchwork/tests/utils.py
> +++ b/patchwork/tests/utils.py
> @@ -16,6 +16,7 @@ from patchwork.models import Check
> from patchwork.models import Comment
> from patchwork.models import CoverLetter
> from patchwork.models import Patch
> +from patchwork.models import PatchRelation
> from patchwork.models import Person
> from patchwork.models import Project
> from patchwork.models import Series
> @@ -352,3 +353,13 @@ def create_covers(count=1, **kwargs):
> kwargs (dict): Overrides for various cover letter fields
> """
> return _create_submissions(create_cover, count, **kwargs)
> +
> +
> +def create_relation(count_patches=2, **kwargs):
> + relation = PatchRelation.objects.create()
> + values = {
> + 'related': relation
> + }
> + values.update(kwargs)
> + create_patches(count_patches, **values)
> + return relation
> diff --git a/releasenotes/notes/add-patch-relations-c96bb6c567b416d8.yaml b/releasenotes/notes/add-patch-relations-c96bb6c567b416d8.yaml
> new file mode 100644
> index 000000000000..44e704c85c20
> --- /dev/null
> +++ b/releasenotes/notes/add-patch-relations-c96bb6c567b416d8.yaml
> @@ -0,0 +1,11 @@
> +---
> +features:
> + - |
> + Patches can now be related to other patches (e.g. to cross-reference
> + revisions). Relations can be set via the REST API by maintainers
> + (currently only for patches of projects they maintain). Patches can
> + belong to at most one relation at a time.
> +api:
> + - |
> + Relations are available via ``/patches/{patchID}/`` endpoint, in
> + the ``related`` field.
> --
> 2.20.1
More information about the Patchwork
mailing list