[PATCH v2 2/2] REST: Add patch relations
Stephen Finucane
stephen at that.guru
Fri Feb 28 10:29:32 AEDT 2020
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. Because this is a writable many-many nested relationship, it
behaves a little unusually. 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
Signed-off-by: Mete Polat <metepolat2000 at gmail.com>
Signed-off-by: Daniel Axtens <dja at axtens.net>
Signed-off-by: Stephen Finucane <stephen at that.guru>
---
docs/api/schemas/latest/patchwork.yaml | 44 +++
docs/api/schemas/patchwork.j2 | 60 ++++
docs/api/schemas/v1.1/patchwork.yaml | 18 +
docs/api/schemas/v1.2/patchwork.yaml | 44 +++
patchwork/api/embedded.py | 25 ++
patchwork/api/event.py | 3 +
patchwork/api/patch.py | 121 ++++++-
patchwork/tests/api/test_relation.py | 319 ++++++++++++++++++
patchwork/tests/utils.py | 11 +
.../add-patch-relations-c96bb6c567b416d8.yaml | 11 +
10 files changed, 653 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/docs/api/schemas/latest/patchwork.yaml b/docs/api/schemas/latest/patchwork.yaml
index 4cee1513..13cdc9cd 100644
--- a/docs/api/schemas/latest/patchwork.yaml
+++ b/docs/api/schemas/latest/patchwork.yaml
@@ -352,6 +352,7 @@ paths:
- patch-created
- patch-completed
- patch-state-changed
+ - patch-relation-changed
- patch-delegated
- check-created
- series-created
@@ -390,6 +391,7 @@ paths:
- $ref: '#/components/schemas/EventPatchCreated'
- $ref: '#/components/schemas/EventPatchCompleted'
- $ref: '#/components/schemas/EventPatchStateChanged'
+ - $ref: '#/components/schemas/EventPatchRelationChanged'
- $ref: '#/components/schemas/EventPatchDelegated'
- $ref: '#/components/schemas/EventCheckCreated'
- $ref: '#/components/schemas/EventSeriesCreated'
@@ -403,6 +405,8 @@ paths:
'#/components/schemas/EventPatchCompleted'
patch-state-changed: >
'#/components/schemas/EventPatchStateChanged'
+ patch-relation-changed: >
+ '#/components/schemas/EventPatchRelationChanged'
patch-delegated: >
'#/components/schemas/EventPatchDelegated'
check-created: '#/components/schemas/EventCheckCreated'
@@ -552,6 +556,12 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/Error'
+ '409':
+ description: Conflict
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Error'
tags:
- patches
put:
@@ -595,6 +605,12 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/Error'
+ '409':
+ description: Conflict
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Error'
tags:
- patches
/api/patches/{id}/comments/:
@@ -1721,6 +1737,24 @@ components:
current_state:
title: Current state
type: string
+ EventPatchRelationChanged:
+ allOf:
+ - $ref: '#/components/schemas/EventBase'
+ - type: object
+ properties:
+ category:
+ enum:
+ - patch-relation-changed
+ payload:
+ properties:
+ patch:
+ $ref: '#/components/schemas/PatchEmbedded'
+ previous_relation:
+ title: Previous relation
+ type: string
+ current_relation:
+ title: Current relation
+ type: string
EventPatchDelegated:
allOf:
- $ref: '#/components/schemas/EventBase'
@@ -1893,6 +1927,11 @@ components:
items:
type: string
readOnly: true
+ related:
+ title: Relations
+ type: array
+ items:
+ type: string
PatchDetail:
allOf:
- $ref: '#/components/schemas/PatchList'
@@ -1943,6 +1982,11 @@ components:
title: Delegate
type: integer
nullable: true
+ related:
+ title: Relations
+ type: array
+ items:
+ type: string
Person:
type: object
properties:
diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2
index c2f5ea6a..bd714d5e 100644
--- a/docs/api/schemas/patchwork.j2
+++ b/docs/api/schemas/patchwork.j2
@@ -359,6 +359,9 @@ paths:
- patch-created
- patch-completed
- patch-state-changed
+{% if version >= (1, 2) %}
+ - patch-relation-changed
+{% endif %}
- patch-delegated
- check-created
- series-created
@@ -397,6 +400,9 @@ paths:
- $ref: '#/components/schemas/EventPatchCreated'
- $ref: '#/components/schemas/EventPatchCompleted'
- $ref: '#/components/schemas/EventPatchStateChanged'
+{% if version >= (1, 2) %}
+ - $ref: '#/components/schemas/EventPatchRelationChanged'
+{% endif %}
- $ref: '#/components/schemas/EventPatchDelegated'
- $ref: '#/components/schemas/EventCheckCreated'
- $ref: '#/components/schemas/EventSeriesCreated'
@@ -410,6 +416,10 @@ paths:
'#/components/schemas/EventPatchCompleted'
patch-state-changed: >
'#/components/schemas/EventPatchStateChanged'
+{% if version >= (1, 2) %}
+ patch-relation-changed: >
+ '#/components/schemas/EventPatchRelationChanged'
+{% endif %}
patch-delegated: >
'#/components/schemas/EventPatchDelegated'
check-created: '#/components/schemas/EventCheckCreated'
@@ -561,6 +571,14 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/Error'
+{% if version >= (1, 2) %}
+ '409':
+ description: Conflict
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Error'
+{% endif %}
tags:
- patches
put:
@@ -604,6 +622,14 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/Error'
+{% if version >= (1, 2) %}
+ '409':
+ description: Conflict
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Error'
+{% endif %}
tags:
- patches
/api/{{ version_url }}patches/{id}/comments/:
@@ -1777,6 +1803,26 @@ components:
current_state:
title: Current state
type: string
+{% if version >= (1, 1) %}
+ EventPatchRelationChanged:
+ allOf:
+ - $ref: '#/components/schemas/EventBase'
+ - type: object
+ properties:
+ category:
+ enum:
+ - patch-relation-changed
+ payload:
+ properties:
+ patch:
+ $ref: '#/components/schemas/PatchEmbedded'
+ previous_relation:
+ title: Previous relation
+ type: string
+ current_relation:
+ title: Current relation
+ type: string
+{% endif %}
EventPatchDelegated:
allOf:
- $ref: '#/components/schemas/EventBase'
@@ -1955,6 +2001,13 @@ components:
items:
type: string
readOnly: true
+{% if version >= (1, 2) %}
+ related:
+ title: Relations
+ type: array
+ items:
+ type: string
+{% endif %}
PatchDetail:
allOf:
- $ref: '#/components/schemas/PatchList'
@@ -2005,6 +2058,13 @@ components:
title: Delegate
type: integer
nullable: true
+{% if version >= (1, 2) %}
+ related:
+ title: Relations
+ type: array
+ items:
+ type: string
+{% endif %}
Person:
type: object
properties:
diff --git a/docs/api/schemas/v1.1/patchwork.yaml b/docs/api/schemas/v1.1/patchwork.yaml
index babc972a..6b497aba 100644
--- a/docs/api/schemas/v1.1/patchwork.yaml
+++ b/docs/api/schemas/v1.1/patchwork.yaml
@@ -1551,6 +1551,24 @@ components:
current_state:
title: Current state
type: string
+ EventPatchRelationChanged:
+ allOf:
+ - $ref: '#/components/schemas/EventBase'
+ - type: object
+ properties:
+ category:
+ enum:
+ - patch-relation-changed
+ payload:
+ properties:
+ patch:
+ $ref: '#/components/schemas/PatchEmbedded'
+ previous_relation:
+ title: Previous relation
+ type: string
+ current_relation:
+ title: Current relation
+ type: string
EventPatchDelegated:
allOf:
- $ref: '#/components/schemas/EventBase'
diff --git a/docs/api/schemas/v1.2/patchwork.yaml b/docs/api/schemas/v1.2/patchwork.yaml
index be79e386..db2ed122 100644
--- a/docs/api/schemas/v1.2/patchwork.yaml
+++ b/docs/api/schemas/v1.2/patchwork.yaml
@@ -352,6 +352,7 @@ paths:
- patch-created
- patch-completed
- patch-state-changed
+ - patch-relation-changed
- patch-delegated
- check-created
- series-created
@@ -390,6 +391,7 @@ paths:
- $ref: '#/components/schemas/EventPatchCreated'
- $ref: '#/components/schemas/EventPatchCompleted'
- $ref: '#/components/schemas/EventPatchStateChanged'
+ - $ref: '#/components/schemas/EventPatchRelationChanged'
- $ref: '#/components/schemas/EventPatchDelegated'
- $ref: '#/components/schemas/EventCheckCreated'
- $ref: '#/components/schemas/EventSeriesCreated'
@@ -403,6 +405,8 @@ paths:
'#/components/schemas/EventPatchCompleted'
patch-state-changed: >
'#/components/schemas/EventPatchStateChanged'
+ patch-relation-changed: >
+ '#/components/schemas/EventPatchRelationChanged'
patch-delegated: >
'#/components/schemas/EventPatchDelegated'
check-created: '#/components/schemas/EventCheckCreated'
@@ -552,6 +556,12 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/Error'
+ '409':
+ description: Conflict
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Error'
tags:
- patches
put:
@@ -595,6 +605,12 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/Error'
+ '409':
+ description: Conflict
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/Error'
tags:
- patches
/api/1.2/patches/{id}/comments/:
@@ -1721,6 +1737,24 @@ components:
current_state:
title: Current state
type: string
+ EventPatchRelationChanged:
+ allOf:
+ - $ref: '#/components/schemas/EventBase'
+ - type: object
+ properties:
+ category:
+ enum:
+ - patch-relation-changed
+ payload:
+ properties:
+ patch:
+ $ref: '#/components/schemas/PatchEmbedded'
+ previous_relation:
+ title: Previous relation
+ type: string
+ current_relation:
+ title: Current relation
+ type: string
EventPatchDelegated:
allOf:
- $ref: '#/components/schemas/EventBase'
@@ -1893,6 +1927,11 @@ components:
items:
type: string
readOnly: true
+ related:
+ title: Relations
+ type: array
+ items:
+ type: string
PatchDetail:
allOf:
- $ref: '#/components/schemas/PatchList'
@@ -1943,6 +1982,11 @@ components:
title: Delegate
type: integer
nullable: true
+ related:
+ title: Relations
+ type: array
+ items:
+ type: string
Person:
type: object
properties:
diff --git a/patchwork/api/embedded.py b/patchwork/api/embedded.py
index de4f3116..85a30cae 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,30 @@ 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 44c34520..d7a99c7d 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 1a3ce905..9983bb62 100644
--- a/patchwork/api/patch.py
+++ b/patchwork/api/patch.py
@@ -1,27 +1,34 @@
# 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
import email.parser
+from django.core.exceptions import ValidationError
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 +61,15 @@ 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 +83,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 +126,16 @@ 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 'related' in data:
+ 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 +143,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 +178,94 @@ 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 00000000..d48c62bc
--- /dev/null
+++ b/patchwork/tests/api/test_relation.py
@@ -0,0 +1,319 @@
+# Patchwork - automated patch tracking system
+# Copyright (C) 2020, IBM Corporation
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+import unittest
+
+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 5e6a1b13..7759c8f3 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 00000000..44e704c8
--- /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.24.1
More information about the Patchwork
mailing list