From patchwork Sun Oct 4 11:10:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Finucane X-Patchwork-Id: 1376395 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4C41Hb40zqz9sRR for ; Sun, 4 Oct 2020 22:10:51 +1100 (AEDT) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=that.guru Authentication-Results: ozlabs.org; dkim=fail reason="key not found in DNS" header.d=that.guru header.i=@that.guru header.a=rsa-sha256 header.s=x header.b=nPBYe43J; dkim-atps=neutral Received: from bilbo.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 4C41Hb1fBRzDqHf for ; Sun, 4 Oct 2020 22:10:51 +1100 (AEDT) X-Original-To: patchwork@lists.ozlabs.org Delivered-To: patchwork@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=that.guru (client-ip=172.82.139.39; helo=qrelay39.mxroute.com; envelope-from=stephen@that.guru; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=that.guru Authentication-Results: lists.ozlabs.org; dkim=fail reason="key not found in DNS" header.d=that.guru header.i=@that.guru header.a=rsa-sha256 header.s=x header.b=nPBYe43J; dkim-atps=neutral Received: from qrelay39.mxroute.com (qrelay39.mxroute.com [172.82.139.39]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4C41Gt5QVCzDqD8 for ; Sun, 4 Oct 2020 22:10:13 +1100 (AEDT) Received: from filter003.mxroute.com ([168.235.111.26] 168-235-111-26.cloud.ramnode.com) (Authenticated sender: mN4UYu2MZsgR) by qrelay39.mxroute.com (ZoneMTA) with ESMTPA id 174f34dfb9d000354c.001 for ; Sun, 04 Oct 2020 11:10:08 +0000 X-Zone-Loop: 8ff491defbbab93be551b42fb9f9af4c583b8782f910 X-Originating-IP: [168.235.111.26] Received: from sunfire.mxrouting.net (sunfire.mxrouting.net [49.12.120.198]) by filter003.mxroute.com (Postfix) with ESMTPS id 7551360035 for ; Sun, 4 Oct 2020 11:10:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=that.guru; s=x; h=Content-Transfer-Encoding:MIME-Version:Message-Id:Date:Subject:Cc:To: From:Sender:Reply-To:Content-Type:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To: References:List-Id:List-Help:List-Unsubscribe:List-Subscribe:List-Post: List-Owner:List-Archive; bh=OuXafIkw+4zsu+mHNFOYyy1wID+3hBmklwlXNcZeKK8=; b=n PBYe43J6og7fPFDISTsMrez53oyASM1gMUxh1lVV9E6++tlDNWgMf3XkP6bra8gtrdyS8xL6QL+ke hIl09Fe8jr0GS7y7cMJX8jm6gLfKbVQMHkIHXwn0COZTF22AiyFPteGCR+eXx8Au3SPoiGwJ2adcY EK0DeqBqM89a5RB3AbKvRR5V9/JQYp+Qfb2ZTnGRBi8RRf7Y1yESH+cgD5Q6TcfWgSn2xo8q+AbhA Kv2L6yCn+1Qz8rz+jYPu/4cPfCxIGPDfSi8DZCtCP2VFaKtCc3FCqo/R+ZXK3Iar9Sw5Ugb71GGJd FJT4x8Rr/UYFWwC6+SZzH1d17YQKps1Hw==; From: Stephen Finucane To: patchwork@lists.ozlabs.org Subject: [PATCH 1/3] tests: Rework 'create_relation' helper Date: Sun, 4 Oct 2020 12:10:00 +0100 Message-Id: <20201004111002.234895-1-stephen@that.guru> X-Mailer: git-send-email 2.25.4 MIME-Version: 1.0 X-AuthUser: stephen@that.guru X-BeenThere: patchwork@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Patchwork development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Patchwork" This wasn't actually creating just a patch relation object - it was also creating patches, which is something we already have an explicit helper for. Clean this thing up. Signed-off-by: Stephen Finucane --- patchwork/tests/api/test_relation.py | 54 ++++++++++++++++------------ patchwork/tests/utils.py | 15 +++----- 2 files changed, 36 insertions(+), 33 deletions(-) diff --git patchwork/tests/api/test_relation.py patchwork/tests/api/test_relation.py index d48c62bc..5f8048f2 100644 --- patchwork/tests/api/test_relation.py +++ patchwork/tests/api/test_relation.py @@ -48,8 +48,8 @@ class TestRelationSimpleAPI(utils.APITestCase): @utils.store_samples('relation-list') def test_list_two_patch_relation(self): - relation = create_relation(2, project=self.project) - patches = relation.patches.all() + relation = create_relation() + patches = create_patches(2, project=self.project, related=relation) # nobody resp = self.client.get(self.api_url(item=patches[0].pk)) @@ -101,8 +101,8 @@ class TestRelationSimpleAPI(utils.APITestCase): 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] + relation = create_relation() + patch = create_patches(2, project=self.project, related=relation)[0] self.assertEqual(PatchRelation.objects.count(), 1) @@ -112,8 +112,8 @@ class TestRelationSimpleAPI(utils.APITestCase): @utils.store_samples('relation-delete') def test_delete_two_patch_relation_maintainer(self): - relation = create_relation(project=self.project) - patch = relation.patches.all()[0] + relation = create_relation() + patch = create_patches(2, project=self.project, related=relation)[0] self.assertEqual(PatchRelation.objects.count(), 1) @@ -145,8 +145,8 @@ class TestRelationSimpleAPI(utils.APITestCase): 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] + relation = create_relation() + patch = create_patches(3, project=self.project, related=relation)[0] self.assertEqual(PatchRelation.objects.count(), 1) @@ -159,8 +159,9 @@ class TestRelationSimpleAPI(utils.APITestCase): @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() + relation = create_relation() + existing_patch_a = create_patches( + 2, project=self.project, related=relation)[0] new_patch = create_patch(project=self.project) @@ -173,8 +174,9 @@ class TestRelationSimpleAPI(utils.APITestCase): 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() + relation = create_relation() + existing_patch_a = create_patches( + 2, project=self.project, related=relation)[0] new_patch = create_patch(project=self.project) @@ -188,8 +190,9 @@ class TestRelationSimpleAPI(utils.APITestCase): 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() + relation = create_relation() + existing_patch_a = create_patches( + 2, project=self.project, related=relation)[0] new_patch_a = create_patch(project=self.project) new_patch_b = create_patch(project=self.project) @@ -210,8 +213,9 @@ class TestRelationSimpleAPI(utils.APITestCase): @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() + relation = create_relation() + existing_patch_a = create_patches( + 2, project=self.project, related=relation)[0] new_patch_a = create_patch(project=self.project) new_patch_b = create_patch(project=self.project) @@ -232,9 +236,10 @@ class TestRelationSimpleAPI(utils.APITestCase): 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] + relation = create_relation() + patches = create_patches(3, project=self.project, related=relation) + keep_patch_a = patches[1] + keep_patch_b = patches[1] # this should do nothing - it is interpreted as # _adding_ keep_patch_b again which is a no-op. @@ -248,8 +253,9 @@ class TestRelationSimpleAPI(utils.APITestCase): 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] + relation = create_relation() + target_patch = create_patches( + 3, project=self.project, related=relation)[0] # maintainer self.client.force_authenticate(user=self.maintainer) @@ -263,8 +269,10 @@ class TestRelationSimpleAPI(utils.APITestCase): @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) + relation_a = create_relation() + create_patches(2, project=self.project, related=relation_a) + relation_b = create_relation() + create_patches(2, project=self.project, related=relation_b) patch_a = relation_a.patches.first() patch_b = relation_b.patches.first() diff --git patchwork/tests/utils.py patchwork/tests/utils.py index c464979f..17dc3fcb 100644 --- patchwork/tests/utils.py +++ patchwork/tests/utils.py @@ -308,6 +308,11 @@ def create_series_reference(**kwargs): return SeriesReference.objects.create(**values) +def create_relation(**kwargs): + """Create 'PatchRelation' object.""" + return PatchRelation.objects.create(**kwargs) + + def _create_submissions(create_func, count=1, **kwargs): """Create 'count' SubmissionMixin-based objects. @@ -364,13 +369,3 @@ 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 From patchwork Sun Oct 4 11:10:01 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Finucane X-Patchwork-Id: 1376394 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4C41H26FT8z9sRR for ; Sun, 4 Oct 2020 22:10:22 +1100 (AEDT) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=that.guru Authentication-Results: ozlabs.org; dkim=fail reason="key not found in DNS" header.d=that.guru header.i=@that.guru header.a=rsa-sha256 header.s=x header.b=b/QuSFeC; dkim-atps=neutral Received: from bilbo.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 4C41H2149bzDqHR for ; Sun, 4 Oct 2020 22:10:22 +1100 (AEDT) X-Original-To: patchwork@lists.ozlabs.org Delivered-To: patchwork@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=that.guru (client-ip=172.82.139.4; helo=qrelay4.mxroute.com; envelope-from=stephen@that.guru; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=that.guru Authentication-Results: lists.ozlabs.org; dkim=fail reason="key not found in DNS" header.d=that.guru header.i=@that.guru header.a=rsa-sha256 header.s=x header.b=b/QuSFeC; dkim-atps=neutral Received: from qrelay4.mxroute.com (qrelay4.mxroute.com [172.82.139.4]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4C41Gt4TrSzDqD5 for ; Sun, 4 Oct 2020 22:10:14 +1100 (AEDT) Received: from filter004.mxroute.com ([149.28.56.236] 149.28.56.236.vultr.com) (Authenticated sender: mN4UYu2MZsgR) by qrelay4.mxroute.com (ZoneMTA) with ESMTPA id 174f34dfb02000354c.001 for ; Sun, 04 Oct 2020 11:10:08 +0000 X-Zone-Loop: 7d9bcf70ee28471e05ececa71143215002c80030f5c4 X-Originating-IP: [149.28.56.236] Received: from sunfire.mxrouting.net (sunfire.mxrouting.net [49.12.120.198]) by filter004.mxroute.com (Postfix) with ESMTPS id 7FC003ED9C for ; Sun, 4 Oct 2020 11:10:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=that.guru; s=x; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=qIcRm6034qKQ8BOdAvhlkjA+yFiuQ5cyJtXuwyZFdeo=; b=b/QuSFeCCrW9it2fvThnEHvBbL gOLvIEGuTz8zVxOUrK410PjUHMVEsE7kyCDcwlXVhfds4aKxbM8/9o7rmHoJAI+v8nUEkKZ1fCXGY F5/taxo4lKA63+4eI/7hVSbT/CmmZogXgNp5m6/9xlIEtDRY5HIOLpf1zzKMOrTn8QcJhPZzNeXaO EpRPUm4HwGt6owP2mcqE9roLhO0I3s/xhI0ViPR5KPl4ordxnMpPiw9HsKNO+ukMT36L73cdw7vqI xQtMWmtxFiUCW5vtW7lMWKo2vJD2Yf0MgzyN2iiZXTyQzNRrr9UiFq5OhB+LK+tYUQ03pk44RxzT6 PuZwkNcw==; From: Stephen Finucane To: patchwork@lists.ozlabs.org Subject: [PATCH 2/3] tests: Add tests for 'patch-relation-changed' events Date: Sun, 4 Oct 2020 12:10:01 +0100 Message-Id: <20201004111002.234895-2-stephen@that.guru> X-Mailer: git-send-email 2.25.4 In-Reply-To: <20201004111002.234895-1-stephen@that.guru> References: <20201004111002.234895-1-stephen@that.guru> MIME-Version: 1.0 X-AuthUser: stephen@that.guru X-BeenThere: patchwork@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Patchwork development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Patchwork" This event is rather odd. If you have two patches then the way a relation is created is by creating a 'PatchRelation' instance and then setting the 'related' attribute on the first patch followed by the second patch. Because the event uses the 'Patch' model's 'pre_save' signal, we'll only see events for the patch being currently saved. This means no event will be raised for the first patch and only one event, the one for the second patch, will be raised when the second patch is being added to the relationship. In hindsight, the structure of the event is off. We should have had something like a 'patch-added-to-relationship' and a 'patch-removed-from-relationship' event, both with the same fields: 'project', 'actor', 'patch' and 'related', the latter of which would have listed all of the _other_ patches in the relationship. Sadly, this is an API change which means we can't do it now. We may well wish to do so in the future though. Signed-off-by: Stephen Finucane --- patchwork/tests/test_events.py | 47 ++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git patchwork/tests/test_events.py patchwork/tests/test_events.py index 415237f9..5bac7789 100644 --- patchwork/tests/test_events.py +++ patchwork/tests/test_events.py @@ -172,6 +172,53 @@ class PatchChangedTest(_BaseTestCase): Event.CATEGORY_PATCH_DELEGATED) self.assertEventFields(events[3], previous_delegate=delegate_b) + def test_patch_relations_changed(self): + # purposefully setting series to None to minimize additional events + relation = utils.create_relation() + patches = utils.create_patches(3, series=None) + + # mark the first two patches as related; the second patch should be the + # one that the event is raised for + + patches[0].related = relation + patches[0].save() + patches[1].related = relation + patches[1].save() + + events = _get_events(patch=patches[1]) + self.assertEqual(events.count(), 2) + self.assertEqual( + events[1].category, Event.CATEGORY_PATCH_RELATION_CHANGED) + self.assertEqual(events[1].project, patches[1].project) + self.assertEqual(events[1].previous_relation, None) + self.assertEqual(events[1].current_relation, relation) + + # add the third patch + + patches[2].related = relation + patches[2].save() + + events = _get_events(patch=patches[2]) + self.assertEqual(events.count(), 2) + self.assertEqual( + events[1].category, Event.CATEGORY_PATCH_RELATION_CHANGED) + self.assertEqual(events[1].project, patches[1].project) + self.assertEqual(events[1].previous_relation, None) + self.assertEqual(events[1].current_relation, relation) + + # drop the third patch + + patches[2].related = None + patches[2].save() + + events = _get_events(patch=patches[2]) + self.assertEqual(events.count(), 3) + self.assertEqual( + events[2].category, Event.CATEGORY_PATCH_RELATION_CHANGED) + self.assertEqual(events[2].project, patches[1].project) + self.assertEqual(events[2].previous_relation, relation) + self.assertEqual(events[2].current_relation, None) + class CheckCreatedTest(_BaseTestCase): From patchwork Sun Oct 4 11:10:02 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Finucane X-Patchwork-Id: 1376398 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4C41J55GBLz9sSG for ; Sun, 4 Oct 2020 22:11:17 +1100 (AEDT) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=that.guru Authentication-Results: ozlabs.org; dkim=fail reason="key not found in DNS" header.d=that.guru header.i=@that.guru header.a=rsa-sha256 header.s=x header.b=ZIlFXnrd; dkim-atps=neutral Received: from bilbo.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 4C41J54dZvzDqHX for ; Sun, 4 Oct 2020 22:11:17 +1100 (AEDT) X-Original-To: patchwork@lists.ozlabs.org Delivered-To: patchwork@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=that.guru (client-ip=64.40.27.174; helo=dal3relay174.mxroute.com; envelope-from=stephen@that.guru; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=that.guru Authentication-Results: lists.ozlabs.org; dkim=fail reason="key not found in DNS" header.d=that.guru header.i=@that.guru header.a=rsa-sha256 header.s=x header.b=ZIlFXnrd; dkim-atps=neutral Received: from dal3relay174.mxroute.com (dal3relay174.mxroute.com [64.40.27.174]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4C41Gx3jGszDqD5 for ; Sun, 4 Oct 2020 22:10:16 +1100 (AEDT) Received: from filter003.mxroute.com ([168.235.111.26] 168-235-111-26.cloud.ramnode.com) (Authenticated sender: mN4UYu2MZsgR) by dal3relay174.mxroute.com (ZoneMTA) with ESMTPSA id 174f34dfeed000af84.003 for (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES128-GCM-SHA256); Sun, 04 Oct 2020 11:10:09 +0000 X-Zone-Loop: 57e95da3e982fa8af27095d240f40865053973fa01bb X-Originating-IP: [168.235.111.26] Received: from sunfire.mxrouting.net (sunfire.mxrouting.net [49.12.120.198]) by filter003.mxroute.com (Postfix) with ESMTPS id 096F560044; Sun, 4 Oct 2020 11:10:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=that.guru; s=x; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=arUZjNCak1OMARnmLFDo2OzonZq4E8WN19SZHIMeTqM=; b=ZIlFXnrdPdnyBue3/JEXE3kb4T X+oN5hReNIBx2hTrz+WQRuaES/Z/S+pUes8fW7pUy5F6xkFiFM0h2F3Q89NyDbaBBDyaghlgycLOS cHt3GRBhfWCuWvCMCt16oTRq3EtX9J8TPtrqqFeKmi3rHH/av9T8xfrz7ZDaG1xgiG1bOOMdrN9J+ 0qR2u9XvTBchKK49JCllk+kmlj+XsTO9wEdntSaA9GWoNS39ExhzsrXetiCYCxZ9xUI8DJp82mhhh pkhvvP1d256KVWcYKjmY7hLUbnBRPvM+3OjNiRmIpMm6wRVanZh/eynQ1AMWjXPza/MXEagoeTZpf Pm+hnjVA==; From: Stephen Finucane To: patchwork@lists.ozlabs.org Subject: [PATCH 3/3] Remove 'PatchRelationSerializer' Date: Sun, 4 Oct 2020 12:10:02 +0100 Message-Id: <20201004111002.234895-3-stephen@that.guru> X-Mailer: git-send-email 2.25.4 In-Reply-To: <20201004111002.234895-1-stephen@that.guru> References: <20201004111002.234895-1-stephen@that.guru> MIME-Version: 1.0 X-AuthUser: stephen@that.guru X-BeenThere: patchwork@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Patchwork development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Ralf Ramsauer Errors-To: patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Patchwork" This wasn't writeable for reasons I haven't been able to figure out. However, it's not actually needed: the 'PatchSerializer' can do the job just fine, given enough information. This exposes a bug in DRF, which has been reported upstream [1]. While we wait for that fix, or some variant of it, to be merged, we must monkey patch the library. [1] https://github.com/encode/django-rest-framework/issues/7550 [2] https://github.com/encode/django-rest-framework/pull/7574 Signed-off-by: Stephen Finucane Reported-by: Ralf Ramsauer Closes: #379 Cc: Daniel Axtens Cc: Rohit Sarkar --- patchwork/api/__init__.py | 50 +++++++++++++++++++++++++++++++++++++++ patchwork/api/embedded.py | 28 +--------------------- patchwork/api/event.py | 7 +++--- patchwork/api/patch.py | 22 ++++++++--------- 4 files changed, 65 insertions(+), 42 deletions(-) diff --git patchwork/api/__init__.py patchwork/api/__init__.py index e69de29b..efc0dd89 100644 --- patchwork/api/__init__.py +++ patchwork/api/__init__.py @@ -0,0 +1,50 @@ +# Patchwork - automated patch tracking system +# Copyright (C) 2020, Stephen Finucane +# +# SPDX-License-Identifier: GPL-2.0-or-later + +from rest_framework.fields import empty +from rest_framework.fields import get_attribute +from rest_framework.fields import SkipField +from rest_framework.relations import ManyRelatedField + + +# monkey patch django-rest-framework to work around issue #7550 [1] until #7574 +# [2] or some other variant lands +# +# [1] https://github.com/encode/django-rest-framework/issues/7550 +# [2] https://github.com/encode/django-rest-framework/pull/7574 + +def _get_attribute(self, instance): + # Can't have any relationships if not created + if hasattr(instance, 'pk') and instance.pk is None: + return [] + + try: + relationship = get_attribute(instance, self.source_attrs) + except (KeyError, AttributeError) as exc: + if self.default is not empty: + return self.get_default() + if self.allow_null: + return None + if not self.required: + raise SkipField() + msg = ( + 'Got {exc_type} when attempting to get a value for field ' + '`{field}` on serializer `{serializer}`.\nThe serializer ' + 'field might be named incorrectly and not match ' + 'any attribute or key on the `{instance}` instance.\n' + 'Original exception text was: {exc}.'.format( + exc_type=type(exc).__name__, + field=self.field_name, + serializer=self.parent.__class__.__name__, + instance=instance.__class__.__name__, + exc=exc + ) + ) + raise type(exc)(msg) + + return relationship.all() if hasattr(relationship, 'all') else relationship + + +ManyRelatedField.get_attribute = _get_attribute diff --git patchwork/api/embedded.py patchwork/api/embedded.py index 3f32bd42..78316979 100644 --- patchwork/api/embedded.py +++ patchwork/api/embedded.py @@ -12,9 +12,8 @@ nested fields. 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 rest_framework.serializers import SerializerMethodField from patchwork.api.base import BaseHyperlinkedModelSerializer from patchwork.api.base import CheckHyperlinkedIdentityField @@ -139,31 +138,6 @@ class PatchSerializer(SerializedRelatedField): } -class PatchRelationSerializer(BaseHyperlinkedModelSerializer): - """Hide the PatchRelation model, just show the list""" - patches = PatchSerializer(many=True, - style={'base_template': 'input.html'}) - - 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 patchwork/api/event.py patchwork/api/event.py index 7ed9efb1..75bf8708 100644 --- patchwork/api/event.py +++ patchwork/api/event.py @@ -13,7 +13,6 @@ from rest_framework.serializers import SlugRelatedField from patchwork.api.embedded import CheckSerializer from patchwork.api.embedded import CoverSerializer 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 @@ -34,8 +33,10 @@ class EventSerializer(ModelSerializer): current_delegate = UserSerializer() created_check = SerializerMethodField() created_check = CheckSerializer() - previous_relation = PatchRelationSerializer(read_only=True) - current_relation = PatchRelationSerializer(read_only=True) + previous_relation = PatchSerializer( + source='previous_relation.patches', many=True, default=None) + current_relation = PatchSerializer( + source='current_relation.patches', many=True, default=None) _category_map = { Event.CATEGORY_COVER_CREATED: ['cover'], diff --git patchwork/api/patch.py patchwork/api/patch.py index d881504f..f6cb276d 100644 --- patchwork/api/patch.py +++ patchwork/api/patch.py @@ -10,7 +10,6 @@ import email.parser from django.core.exceptions import ValidationError from django.utils.text import slugify from django.utils.translation import gettext_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 @@ -18,15 +17,16 @@ 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 import status 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 PatchSerializer 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.api.filters import PatchFilterSet from patchwork.models import Patch from patchwork.models import PatchRelation from patchwork.models import State @@ -83,7 +83,8 @@ class PatchListSerializer(BaseHyperlinkedModelSerializer): check = SerializerMethodField() checks = SerializerMethodField() tags = SerializerMethodField() - related = PatchRelationSerializer() + related = PatchSerializer( + source='related.patches', many=True, default=[]) def get_web_url(self, instance): request = self.context.get('request') @@ -127,14 +128,11 @@ class PatchListSerializer(BaseHyperlinkedModelSerializer): 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'] = [] + # Remove this patch from 'related' + if 'related' in data and data['related']: + data['related'] = [ + x for x in data['related'] if x['id'] != data['id'] + ] return data