From patchwork Wed Mar 4 11:54:56 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Finucane X-Patchwork-Id: 1248973 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (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 48XXSB0RXfz9sQt for ; Wed, 4 Mar 2020 22:57:30 +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=default header.b=ITx7PtFE; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 48XXS96gMQzDqcR for ; Wed, 4 Mar 2020 22:57:29 +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.177; helo=qrelay177.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=default header.b=ITx7PtFE; dkim-atps=neutral Received: from qrelay177.mxroute.com (qrelay177.mxroute.com [172.82.139.177]) (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 48XXPc17NdzDqVc for ; Wed, 4 Mar 2020 22:55:15 +1100 (AEDT) Received: from filter003.mxroute.com ([168.235.111.26] 168-235-111-26.cloud.ramnode.com) (Authenticated sender: mN4UYu2MZsgR) by qrelay177.mxroute.com (ZoneMTA) with ESMTPA id 170a5664d7b000f8af.002 for ; Wed, 04 Mar 2020 11:55:10 +0000 X-Zone-Loop: 4289ad1632f63d4126d4fc66eedaaf0cc86f48eb068e X-Originating-IP: [168.235.111.26] Received: from one.mxroute.com (one.mxroute.com [195.201.59.211]) by filter003.mxroute.com (Postfix) with ESMTPS id 8BB8E6003F; Wed, 4 Mar 2020 11:55:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=that.guru; s=default; 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=OxSOGWAMgyLy6spSg8CXGgPaGjJ23pWO2NkVfVeJREo=; b=ITx7PtFErTtrYYT33cXeGuI9YZ lu5vuvBlJGLc83MtPtVmemZUaLwttOrR2BS0TEQ992vMGMJnHAKOwtT48hivjy0HCLv2piydrzC1+ +T1L+J1DXD7Ma79GueilJt9WtiklBLhf3Cs1a2zli2rh8SEp9/IeSIfIOdgk7/NGGez/gHZcn5R8a jVSuMd4xDW4TgMFni9AqbPxn/l0FbFg8HPRFWUhIyLqLaJZ/5AfOFhxiFZjKTfVsN2ZZi4thpF9XA mD9r0vsD8HS8M1t/23McaoOOY+qe4G6nlzh2TqDakzaLFye3DmhJ1DMlq9ZfCJlJ5Gi3C+juUSSjW fCULx6KQ==; From: Stephen Finucane To: patchwork@lists.ozlabs.org Subject: [PATCH 4/5] models: Split 'CoverLetter' from 'Submission' Date: Wed, 4 Mar 2020 11:54:56 +0000 Message-Id: <20200304115457.32300-5-stephen@that.guru> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200304115457.32300-1-stephen@that.guru> References: <20200304115457.32300-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" We want to get rid of the split between 'Patch' and 'Submission' because of the cost of using JOINs basically everywhere we use 'Patch'. Before we do that, we need to move the other users of 'Submission' to other models and other models that rely on these users sharing the common 'Submission' base. For the former, there is only one user, 'CoverLetter', while for the latter there is only the 'Comment' model. As a result, we must do the following: - Create a new 'Cover' model - Create a new 'CoverComment' model - Move everything from 'CoverLetter' to 'Cover' and all entries associated with a 'CoverLetter' from 'Comment' to 'CoverComment' - Delete the 'CoverLetter' model - Rename the 'Comment' model to 'PatchComment' This means our model "hierarchy" goes from: Submission Patch CoverLetter Comment To: Submission Patch PatchComment Cover CoverComment A future change will flatten the 'Submission' and 'Patch' model. Note that this actually highlighted a bug in Django, which has since been reported upstream [1]. As noted there, the issue stems from MySQL's refusal to remove an index from a foreign key when DB constraints are used and the workaround is to remove the foreign key constraint before altering the indexes and then re-add the constraint after. [1] https://code.djangoproject.com/ticket/31335 Signed-off-by: Stephen Finucane --- patchwork/admin.py | 26 +- patchwork/api/comment.py | 52 +++- patchwork/api/cover.py | 10 +- patchwork/api/embedded.py | 2 +- patchwork/api/filters.py | 6 +- patchwork/management/commands/parsearchive.py | 11 +- patchwork/migrations/0040_add_cover_model.py | 249 ++++++++++++++++++ patchwork/models.py | 120 ++++++--- patchwork/parser.py | 77 ++++-- patchwork/signals.py | 4 +- patchwork/tests/api/test_comment.py | 11 +- patchwork/tests/test_detail.py | 31 ++- patchwork/tests/test_mboxviews.py | 14 +- patchwork/tests/test_parser.py | 4 +- patchwork/tests/test_series.py | 2 +- patchwork/tests/test_tags.py | 6 +- patchwork/tests/utils.py | 32 ++- patchwork/urls.py | 4 +- patchwork/views/comment.py | 43 ++- patchwork/views/cover.py | 12 +- patchwork/views/patch.py | 7 +- patchwork/views/utils.py | 17 +- 22 files changed, 597 insertions(+), 143 deletions(-) create mode 100644 patchwork/migrations/0040_add_cover_model.py diff --git a/patchwork/admin.py b/patchwork/admin.py index d4ab109c..6b6acbf1 100644 --- a/patchwork/admin.py +++ b/patchwork/admin.py @@ -10,10 +10,11 @@ from django.db.models import Prefetch from patchwork.models import Bundle from patchwork.models import Check -from patchwork.models import Comment -from patchwork.models import CoverLetter +from patchwork.models import Cover +from patchwork.models import CoverComment from patchwork.models import DelegationRule from patchwork.models import Patch +from patchwork.models import PatchComment from patchwork.models import Person from patchwork.models import Project from patchwork.models import Series @@ -74,14 +75,14 @@ class StateAdmin(admin.ModelAdmin): admin.site.register(State, StateAdmin) -class CoverLetterAdmin(admin.ModelAdmin): +class CoverAdmin(admin.ModelAdmin): list_display = ('name', 'submitter', 'project', 'date') list_filter = ('project', ) search_fields = ('name', 'submitter__name', 'submitter__email') date_hierarchy = 'date' -admin.site.register(CoverLetter, CoverLetterAdmin) +admin.site.register(Cover, CoverAdmin) class PatchAdmin(admin.ModelAdmin): @@ -103,13 +104,22 @@ class PatchAdmin(admin.ModelAdmin): admin.site.register(Patch, PatchAdmin) -class CommentAdmin(admin.ModelAdmin): - list_display = ('submission', 'submitter', 'date') - search_fields = ('submission__name', 'submitter__name', 'submitter__email') +class CoverCommentAdmin(admin.ModelAdmin): + list_display = ('cover', 'submitter', 'date') + search_fields = ('cover__name', 'submitter__name', 'submitter__email') date_hierarchy = 'date' -admin.site.register(Comment, CommentAdmin) +admin.site.register(CoverComment, CoverCommentAdmin) + + +class PatchCommentAdmin(admin.ModelAdmin): + list_display = ('patch', 'submitter', 'date') + search_fields = ('patch__name', 'submitter__name', 'submitter__email') + date_hierarchy = 'date' + + +admin.site.register(PatchComment, PatchCommentAdmin) class PatchInline(admin.StackedInline): diff --git a/patchwork/api/comment.py b/patchwork/api/comment.py index 290b9cd3..3802dab9 100644 --- a/patchwork/api/comment.py +++ b/patchwork/api/comment.py @@ -12,11 +12,13 @@ from rest_framework.serializers import SerializerMethodField from patchwork.api.base import BaseHyperlinkedModelSerializer from patchwork.api.base import PatchworkPermission from patchwork.api.embedded import PersonSerializer -from patchwork.models import Comment +from patchwork.models import Cover +from patchwork.models import CoverComment from patchwork.models import Submission +from patchwork.models import PatchComment -class CommentListSerializer(BaseHyperlinkedModelSerializer): +class BaseCommentListSerializer(BaseHyperlinkedModelSerializer): web_url = SerializerMethodField() subject = SerializerMethodField() @@ -46,7 +48,6 @@ class CommentListSerializer(BaseHyperlinkedModelSerializer): return headers class Meta: - model = Comment fields = ('id', 'web_url', 'msgid', 'list_archive_url', 'date', 'subject', 'submitter', 'content', 'headers') read_only_fields = fields @@ -56,11 +57,48 @@ class CommentListSerializer(BaseHyperlinkedModelSerializer): } -class CommentList(ListAPIView): +class CoverCommentListSerializer(BaseCommentListSerializer): + + class Meta: + model = CoverComment + fields = BaseCommentListSerializer.Meta.fields + read_only_fields = fields + versioned_fields = BaseCommentListSerializer.Meta.versioned_fields + + +class PatchCommentListSerializer(BaseCommentListSerializer): + + class Meta: + model = PatchComment + fields = BaseCommentListSerializer.Meta.fields + read_only_fields = fields + versioned_fields = BaseCommentListSerializer.Meta.versioned_fields + + +class CoverCommentList(ListAPIView): + """List cover comments""" + + permission_classes = (PatchworkPermission,) + serializer_class = CoverCommentListSerializer + search_fields = ('subject',) + ordering_fields = ('id', 'subject', 'date', 'submitter') + ordering = 'id' + lookup_url_kwarg = 'pk' + + def get_queryset(self): + if not Cover.objects.filter(pk=self.kwargs['pk']).exists(): + raise Http404 + + return CoverComment.objects.filter( + cover=self.kwargs['pk'] + ).select_related('submitter') + + +class PatchCommentList(ListAPIView): """List comments""" permission_classes = (PatchworkPermission,) - serializer_class = CommentListSerializer + serializer_class = PatchCommentListSerializer search_fields = ('subject',) ordering_fields = ('id', 'subject', 'date', 'submitter') ordering = 'id' @@ -70,6 +108,6 @@ class CommentList(ListAPIView): if not Submission.objects.filter(pk=self.kwargs['pk']).exists(): raise Http404 - return Comment.objects.filter( - submission=self.kwargs['pk'] + return PatchComment.objects.filter( + patch=self.kwargs['pk'] ).select_related('submitter') diff --git a/patchwork/api/cover.py b/patchwork/api/cover.py index 8dc2f9ce..b7ced0f9 100644 --- a/patchwork/api/cover.py +++ b/patchwork/api/cover.py @@ -15,7 +15,7 @@ from patchwork.api.filters import CoverFilterSet from patchwork.api.embedded import PersonSerializer from patchwork.api.embedded import ProjectSerializer from patchwork.api.embedded import SeriesSerializer -from patchwork.models import CoverLetter +from patchwork.models import Cover class CoverListSerializer(BaseHyperlinkedModelSerializer): @@ -49,7 +49,7 @@ class CoverListSerializer(BaseHyperlinkedModelSerializer): return data class Meta: - model = CoverLetter + model = Cover fields = ('id', 'url', 'web_url', 'project', 'msgid', 'list_archive_url', 'date', 'name', 'submitter', 'mbox', 'series', 'comments') @@ -82,7 +82,7 @@ class CoverDetailSerializer(CoverListSerializer): return headers class Meta: - model = CoverLetter + model = Cover fields = CoverListSerializer.Meta.fields + ( 'headers', 'content') read_only_fields = fields @@ -100,7 +100,7 @@ class CoverList(ListAPIView): ordering = 'id' def get_queryset(self): - return CoverLetter.objects.all()\ + return Cover.objects.all()\ .select_related('project', 'submitter', 'series__project')\ .defer('content', 'headers') @@ -111,5 +111,5 @@ class CoverDetail(RetrieveAPIView): serializer_class = CoverDetailSerializer def get_queryset(self): - return CoverLetter.objects.all()\ + return Cover.objects.all()\ .select_related('project', 'submitter', 'series') diff --git a/patchwork/api/embedded.py b/patchwork/api/embedded.py index bcf4f7ce..61f6ab69 100644 --- a/patchwork/api/embedded.py +++ b/patchwork/api/embedded.py @@ -107,7 +107,7 @@ class CoverSerializer(SerializedRelatedField): class _Serializer(MboxMixin, WebURLMixin, BaseHyperlinkedModelSerializer): class Meta: - model = models.CoverLetter + model = models.Cover fields = ('id', 'url', 'web_url', 'msgid', 'list_archive_url', 'date', 'name', 'mbox') read_only_fields = fields diff --git a/patchwork/api/filters.py b/patchwork/api/filters.py index 2d4b6355..c030f058 100644 --- a/patchwork/api/filters.py +++ b/patchwork/api/filters.py @@ -17,7 +17,7 @@ from patchwork.api import utils from patchwork.compat import NAME_FIELD from patchwork.models import Bundle from patchwork.models import Check -from patchwork.models import CoverLetter +from patchwork.models import Cover from patchwork.models import Event from patchwork.models import Patch from patchwork.models import Person @@ -182,7 +182,7 @@ class CoverFilterSet(TimestampMixin, BaseFilterSet): submitter = PersonFilter(queryset=Person.objects.all()) class Meta: - model = CoverLetter + model = Cover fields = ('project', 'series', 'submitter') @@ -231,7 +231,7 @@ class EventFilterSet(TimestampMixin, BaseFilterSet): widget=MultipleHiddenInput) patch = BaseFilter(queryset=Patch.objects.all(), widget=MultipleHiddenInput) - cover = BaseFilter(queryset=CoverLetter.objects.all(), + cover = BaseFilter(queryset=Cover.objects.all(), widget=MultipleHiddenInput) class Meta: diff --git a/patchwork/management/commands/parsearchive.py b/patchwork/management/commands/parsearchive.py index b7f1ea73..f53b8db4 100644 --- a/patchwork/management/commands/parsearchive.py +++ b/patchwork/management/commands/parsearchive.py @@ -32,8 +32,9 @@ class Command(BaseCommand): def handle(self, *args, **options): results = { models.Patch: 0, - models.CoverLetter: 0, - models.Comment: 0, + models.Cover: 0, + models.PatchComment: 0, + models.CoverComment: 0, } duplicates = 0 dropped = 0 @@ -118,9 +119,11 @@ class Command(BaseCommand): ' %(errors)4d errors\n' 'Total: %(new)s new entries' % { 'total': count, - 'covers': results[models.CoverLetter], + 'covers': results[models.Cover], 'patches': results[models.Patch], - 'comments': results[models.Comment], + 'comments': ( + results[models.CoverComment] + results[models.PatchComment] + ), 'duplicates': duplicates, 'dropped': dropped, 'errors': errors, diff --git a/patchwork/migrations/0040_add_cover_model.py b/patchwork/migrations/0040_add_cover_model.py new file mode 100644 index 00000000..aa0ff5ef --- /dev/null +++ b/patchwork/migrations/0040_add_cover_model.py @@ -0,0 +1,249 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +import datetime +from django.db import migrations, models +import django.db.models.deletion +import patchwork.models + + +class Migration(migrations.Migration): + + dependencies = [ + ('patchwork', '0039_unique_series_references'), + ] + + operations = [ + # create a new, separate cover (letter) model + + migrations.CreateModel( + name='Cover', + fields=[ + ( + 'id', + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name='ID', + ), + ), + ('msgid', models.CharField(max_length=255)), + ( + 'date', + models.DateTimeField(default=datetime.datetime.utcnow), + ), + ('headers', models.TextField(blank=True)), + ('content', models.TextField(blank=True, null=True)), + ('name', models.CharField(max_length=255)), + ( + 'project', + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to='patchwork.Project', + ), + ), + ( + 'submitter', + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to='patchwork.Person', + ), + ), + ], + options={'ordering': ['date']}, + bases=(patchwork.models.FilenameMixin, models.Model), + ), + migrations.AddIndex( + model_name='cover', + index=models.Index( + fields=[b'date', b'project', b'submitter', b'name'], + name=b'cover_covering_idx', + ), + ), + migrations.AlterUniqueTogether( + name='cover', unique_together=set([('msgid', 'project')]), + ), + + # create a new, separate cover (letter) comment model + + migrations.CreateModel( + name='CoverComment', + fields=[ + ( + 'id', + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name='ID', + ), + ), + ('msgid', models.CharField(max_length=255)), + ( + 'date', + models.DateTimeField(default=datetime.datetime.utcnow), + ), + ('headers', models.TextField(blank=True)), + ('content', models.TextField(blank=True, null=True)), + ( + 'cover', + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name='comments', + related_query_name='comment', + to='patchwork.Cover', + ), + ), + ( + 'submitter', + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to='patchwork.Person', + ), + ), + ], + options={'ordering': ['date']}, + ), + migrations.AddIndex( + model_name='covercomment', + index=models.Index( + fields=[b'cover', b'date'], name=b'cover_date_idx' + ), + ), + migrations.AlterUniqueTogether( + name='covercomment', unique_together=set([('msgid', 'cover')]), + ), + + # copy all entries from the 'CoverLetter' model to the new 'Cover' + # model; note that it's not possible to reverse this since we can't + # guarantee IDs will be unique after the split + + migrations.RunSQL( + """ + INSERT INTO patchwork_cover + (id, msgid, name, date, headers, project_id, submitter_id, + content) + SELECT s.id, s.msgid, s.name, s.date, s.headers, s.project_id, + s.submitter_id, s.content + FROM patchwork_coverletter c + INNER JOIN patchwork_submission s ON s.id = c.submission_ptr_id + """, + None, + ), + + # copy all 'CoverLetter'-related comments to the new 'CoverComment' + # table; as above, this is non-reversible + + migrations.RunSQL( + """ + INSERT INTO patchwork_covercomment + (id, msgid, date, headers, content, cover_id, submitter_id) + SELECT c.id, c.msgid, c.date, c.headers, c.content, + c.submission_id, c.submitter_id + FROM patchwork_comment c + INNER JOIN patchwork_coverletter p + ON p.submission_ptr_id = c.submission_id + """, + None, + ), + + # update all references to the 'CoverLetter' model to point to the new + # 'Cover' model instead + + migrations.AlterField( + model_name='event', + name='cover', + field=models.ForeignKey( + blank=True, + help_text=b'The cover letter that this event was created for.', + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name='+', + to='patchwork.Cover', + ), + ), + migrations.AlterField( + model_name='series', + name='cover_letter', + field=models.OneToOneField( + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name='series', + to='patchwork.Cover' + ), + ), + + # remove all the old 'CoverLetter'-related entries from the 'Comment' + # table + + migrations.RunSQL( + """ + DELETE patchwork_comment FROM + patchwork_comment + INNER JOIN patchwork_coverletter + ON patchwork_coverletter.submission_ptr_id = patchwork_comment.submission_id + """, # noqa + None + ), + + # delete the old 'CoverLetter' model + + migrations.DeleteModel( + name='CoverLetter', + ), + + # rename the 'Comment.submission' field to 'Comment.patch'; note our + # use of 'AlterField' before and after to work around bug #31335 + # + # https://code.djangoproject.com/ticket/31335 + + migrations.AlterField( + model_name='comment', + name='submission', + field=models.ForeignKey( + db_constraint=False, + on_delete=django.db.models.deletion.CASCADE, + related_name='comments', + related_query_name='comment', + to='patchwork.Submission', + ), + ), + migrations.RemoveIndex( + model_name='comment', + name='submission_date_idx', + ), + migrations.RenameField( + model_name='comment', + old_name='submission', + new_name='patch', + ), + migrations.AlterUniqueTogether( + name='comment', + unique_together=set([('msgid', 'patch')]), + ), + migrations.AddIndex( + model_name='comment', + index=models.Index( + fields=[b'patch', b'date'], + name=b'patch_date_idx', + ), + ), + migrations.AlterField( + model_name='comment', + name='patch', + field=models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name='comments', + related_query_name='comment', + to='patchwork.Submission', + ), + ), + + # rename the 'Comment' model to 'PatchComment' + + migrations.RenameModel( + old_name='Comment', + new_name='PatchComment', + ), + ] diff --git a/patchwork/models.py b/patchwork/models.py index 08ee341e..37cbce32 100644 --- a/patchwork/models.py +++ b/patchwork/models.py @@ -367,7 +367,7 @@ class FilenameMixin(object): @python_2_unicode_compatible -class Submission(FilenameMixin, EmailMixin, models.Model): +class SubmissionMixin(models.Model): # parent project = models.ForeignKey(Project, on_delete=models.CASCADE) @@ -394,19 +394,10 @@ class Submission(FilenameMixin, EmailMixin, models.Model): return self.name class Meta: - ordering = ['date'] - unique_together = [('msgid', 'project')] - indexes = [ - # This is a covering index for the /list/ query - # Like what we have for Patch, but used for displaying what we want - # rather than for working out the count (of course, this all - # depends on the SQL optimiser of your db engine) - models.Index(fields=['date', 'project', 'submitter', 'name'], - name='submission_covering_idx'), - ] + abstract = True -class CoverLetter(Submission): +class Cover(FilenameMixin, EmailMixin, SubmissionMixin): def get_absolute_url(self): return reverse('cover-detail', @@ -418,6 +409,35 @@ class CoverLetter(Submission): kwargs={'project_id': self.project.linkname, 'msgid': self.url_msgid}) + class Meta: + ordering = ['date'] + unique_together = [('msgid', 'project')] + indexes = [ + # This is a covering index for the /list/ query + # Like what we have for Patch, but used for displaying what we want + # rather than for working out the count (of course, this all + # depends on the SQL optimiser of your DB engine) + models.Index( + fields=['date', 'project', 'submitter', 'name'], + name='cover_covering_idx', + ), + ] + + +class Submission(SubmissionMixin, FilenameMixin, EmailMixin): + + class Meta: + ordering = ['date'] + unique_together = [('msgid', 'project')] + indexes = [ + # This is a covering index for the /list/ query + # Like what we have for Patch, but used for displaying what we want + # rather than for working out the count (of course, this all + # depends on the SQL optimiser of your db engine) + models.Index(fields=['date', 'project', 'submitter', 'name'], + name='submission_covering_idx'), + ] + @python_2_unicode_compatible class Patch(Submission): @@ -623,44 +643,79 @@ class Patch(Submission): ] -class Comment(EmailMixin, models.Model): +class CoverComment(EmailMixin, models.Model): + + cover = models.ForeignKey( + Cover, + related_name='comments', + related_query_name='comment', + on_delete=models.CASCADE, + ) + + @property + def list_archive_url(self): + if not self.cover.project.list_archive_url_format: + return None + if not self.msgid: + return None + return self.project.list_archive_url_format.format(self.url_msgid) + + def get_absolute_url(self): + return reverse('comment-redirect', kwargs={'comment_id': self.id}) + + def is_editable(self, user): + return False + + class Meta: + ordering = ['date'] + unique_together = [('msgid', 'cover')] + indexes = [ + models.Index(name='cover_date_idx', fields=['cover', 'date']), + ] + + +class PatchComment(EmailMixin, models.Model): # parent - submission = models.ForeignKey(Submission, related_name='comments', - related_query_name='comment', - on_delete=models.CASCADE) + patch = models.ForeignKey( + Submission, + related_name='comments', + related_query_name='comment', + on_delete=models.CASCADE, + ) @property def list_archive_url(self): - if not self.submission.project.list_archive_url_format: + if not self.patch.project.list_archive_url_format: return None if not self.msgid: return None - return self.project.list_archive_url_format.format( + return self.patch.list_archive_url_format.format( self.url_msgid) def get_absolute_url(self): return reverse('comment-redirect', kwargs={'comment_id': self.id}) def save(self, *args, **kwargs): - super(Comment, self).save(*args, **kwargs) - if hasattr(self.submission, 'patch'): - self.submission.patch.refresh_tag_counts() + super(PatchComment, self).save(*args, **kwargs) + # TODO(stephenfin): Update this once patch is flattened + if hasattr(self.patch, 'patch'): + self.patch.patch.refresh_tag_counts() def delete(self, *args, **kwargs): - super(Comment, self).delete(*args, **kwargs) - if hasattr(self.submission, 'patch'): - self.submission.patch.refresh_tag_counts() + super(PatchComment, self).delete(*args, **kwargs) + # TODO(stephenfin): Update this once patch is flattened + if hasattr(self.patch, 'patch'): + self.patch.patch.refresh_tag_counts() def is_editable(self, user): return False class Meta: ordering = ['date'] - unique_together = [('msgid', 'submission')] + unique_together = [('msgid', 'patch')] indexes = [ - models.Index(name='submission_date_idx', - fields=['submission', 'date']) + models.Index(name='patch_date_idx', fields=['patch', 'date']), ] @@ -673,9 +728,12 @@ class Series(FilenameMixin, models.Model): blank=True, on_delete=models.CASCADE) # content - cover_letter = models.OneToOneField(CoverLetter, related_name='series', - null=True, - on_delete=models.CASCADE) + cover_letter = models.OneToOneField( + Cover, + related_name='series', + null=True, + on_delete=models.CASCADE + ) # metadata name = models.CharField(max_length=255, blank=True, null=True, @@ -980,7 +1038,7 @@ class Event(models.Model): on_delete=models.CASCADE, help_text='The series that this event was created for.') cover = models.ForeignKey( - CoverLetter, related_name='+', null=True, blank=True, + Cover, related_name='+', null=True, blank=True, on_delete=models.CASCADE, help_text='The cover letter that this event was created for.') diff --git a/patchwork/parser.py b/patchwork/parser.py index 0e88d934..cd82f3be 100644 --- a/patchwork/parser.py +++ b/patchwork/parser.py @@ -19,11 +19,12 @@ from django.db.utils import IntegrityError from django.db import transaction from django.utils import six -from patchwork.models import Comment -from patchwork.models import CoverLetter +from patchwork.models import Cover +from patchwork.models import CoverComment from patchwork.models import DelegationRule from patchwork.models import get_default_initial_patch_state from patchwork.models import Patch +from patchwork.models import PatchComment from patchwork.models import Person from patchwork.models import Project from patchwork.models import Series @@ -658,10 +659,12 @@ def find_submission_for_comment(project, refs): # see if we have comments that refer to a patch try: - comment = Comment.objects.get(submission__project=project, - msgid=ref) + comment = PatchComment.objects.get( + patch__project=project, + msgid=ref, + ) return comment.submission - except Comment.MultipleObjectsReturned: + except PatchComment.MultipleObjectsReturned: # NOTE(stephenfin): This is a artifact of prior lack of support # for cover letters in Patchwork. Previously all replies to # patches were saved as comments. However, it's possible that @@ -675,11 +678,36 @@ def find_submission_for_comment(project, refs): # apply the comment to the cover letter. Note that this only # happens when running 'parsearchive' or similar, so it should not # affect every day use in any way. - comments = Comment.objects.filter(submission__project=project, - msgid=ref) + comments = PatchComment.objects.filter( + patch__project=project, + msgid=ref, + ) # The latter item will be the cover letter return comments.reverse()[0].submission - except Comment.DoesNotExist: + except PatchComment.DoesNotExist: + pass + + return None + + +def find_cover_for_comment(project, refs): + for ref in refs: + ref = ref[:255] + # first, check for a direct reply + try: + cover = Cover.objects.get(project=project, msgid=ref) + return cover + except Cover.DoesNotExist: + pass + + # see if we have comments that refer to a patch + try: + comment = CoverComment.objects.get( + cover__project=project, + msgid=ref, + ) + return comment.cover + except CoverComment.DoesNotExist: pass return None @@ -1187,11 +1215,11 @@ def parse_mail(mail, list_id=None): if not is_comment: if not refs == []: try: - CoverLetter.objects.all().get(name=name) - except CoverLetter.DoesNotExist: + Cover.objects.all().get(name=name) + except Cover.DoesNotExist: # if no match, this is a new cover letter is_cover_letter = True - except CoverLetter.MultipleObjectsReturned: + except Cover.MultipleObjectsReturned: # if multiple cover letters are found, just ignore pass else: @@ -1225,7 +1253,7 @@ def parse_mail(mail, list_id=None): msgid=msgid, project=project, series=series) try: - cover_letter = CoverLetter.objects.create( + cover_letter = Cover.objects.create( msgid=msgid, project=project, name=name[:255], @@ -1246,14 +1274,33 @@ def parse_mail(mail, list_id=None): # we only save comments if we have the parent email submission = find_submission_for_comment(project, refs) - if not submission: + if submission: + author = get_or_create_author(mail, project) + + try: + comment = PatchComment.objects.create( + patch=submission, + msgid=msgid, + date=date, + headers=headers, + submitter=author, + content=message) + except IntegrityError: + raise DuplicateMailError(msgid=msgid) + + logger.debug('Comment saved') + + return comment + + cover = find_cover_for_comment(project, refs) + if not cover: return author = get_or_create_author(mail, project) try: - comment = Comment.objects.create( - submission=submission, + comment = CoverComment.objects.create( + cover=cover, msgid=msgid, date=date, headers=headers, diff --git a/patchwork/signals.py b/patchwork/signals.py index 73ddfa5e..4080b367 100644 --- a/patchwork/signals.py +++ b/patchwork/signals.py @@ -10,7 +10,7 @@ from django.db.models.signals import pre_save from django.dispatch import receiver from patchwork.models import Check -from patchwork.models import CoverLetter +from patchwork.models import Cover from patchwork.models import Event from patchwork.models import Patch from patchwork.models import PatchChangeNotification @@ -54,7 +54,7 @@ def patch_change_callback(sender, instance, raw, **kwargs): notification.save() -@receiver(post_save, sender=CoverLetter) +@receiver(post_save, sender=Cover) def create_cover_created_event(sender, instance, created, raw, **kwargs): def create_event(cover): diff --git a/patchwork/tests/api/test_comment.py b/patchwork/tests/api/test_comment.py index f48bfce1..dfbf9049 100644 --- a/patchwork/tests/api/test_comment.py +++ b/patchwork/tests/api/test_comment.py @@ -10,9 +10,10 @@ from django.urls import NoReverseMatch from django.urls import reverse from patchwork.tests.api import utils -from patchwork.tests.utils import create_comment from patchwork.tests.utils import create_cover +from patchwork.tests.utils import create_cover_comment from patchwork.tests.utils import create_patch +from patchwork.tests.utils import create_patch_comment from patchwork.tests.utils import SAMPLE_CONTENT if settings.ENABLE_REST_API: @@ -47,7 +48,7 @@ class TestCoverComments(utils.APITestCase): def test_list(self): """List cover letter comments.""" cover = create_cover() - comment = create_comment(submission=cover) + comment = create_cover_comment(cover=cover) resp = self.client.get(self.api_url(cover)) self.assertEqual(status.HTTP_200_OK, resp.status_code) @@ -57,7 +58,7 @@ class TestCoverComments(utils.APITestCase): def test_list_version_1_0(self): """List cover letter comments using API v1.0.""" cover = create_cover() - create_comment(submission=cover) + create_cover_comment(cover=cover) # check we can't access comments using the old version of the API with self.assertRaises(NoReverseMatch): @@ -98,7 +99,7 @@ class TestPatchComments(utils.APITestCase): def test_list(self): """List patch comments.""" patch = create_patch() - comment = create_comment(submission=patch) + comment = create_patch_comment(patch=patch) resp = self.client.get(self.api_url(patch)) self.assertEqual(status.HTTP_200_OK, resp.status_code) @@ -108,7 +109,7 @@ class TestPatchComments(utils.APITestCase): def test_list_version_1_0(self): """List patch comments using API v1.0.""" patch = create_patch() - create_comment(submission=patch) + create_patch_comment(patch=patch) # check we can't access comments using the old version of the API with self.assertRaises(NoReverseMatch): diff --git a/patchwork/tests/test_detail.py b/patchwork/tests/test_detail.py index 0384e7ae..d97c6ddd 100644 --- a/patchwork/tests/test_detail.py +++ b/patchwork/tests/test_detail.py @@ -6,9 +6,10 @@ from django.test import TestCase from django.urls import reverse -from patchwork.tests.utils import create_comment from patchwork.tests.utils import create_cover +from patchwork.tests.utils import create_cover_comment from patchwork.tests.utils import create_patch +from patchwork.tests.utils import create_patch_comment class CoverViewTest(TestCase): @@ -124,24 +125,32 @@ class PatchViewTest(TestCase): class CommentRedirectTest(TestCase): - def _test_redirect(self, submission, submission_url): - comment_id = create_comment(submission=submission).id + def test_patch_redirect(self): + patch = create_patch() + comment_id = create_patch_comment(patch=patch).id requested_url = reverse('comment-redirect', kwargs={'comment_id': comment_id}) redirect_url = '%s#%d' % ( - reverse(submission_url, - kwargs={'project_id': submission.project.linkname, - 'msgid': submission.url_msgid}), + reverse('patch-detail', + kwargs={'project_id': patch.project.linkname, + 'msgid': patch.url_msgid}), comment_id) response = self.client.get(requested_url) self.assertRedirects(response, redirect_url) - def test_patch_redirect(self): - patch = create_patch() - self._test_redirect(patch, 'patch-detail') - def test_cover_redirect(self): cover = create_cover() - self._test_redirect(cover, 'cover-detail') + comment_id = create_cover_comment(cover=cover).id + + requested_url = reverse('comment-redirect', + kwargs={'comment_id': comment_id}) + redirect_url = '%s#%d' % ( + reverse('cover-detail', + kwargs={'project_id': cover.project.linkname, + 'msgid': cover.url_msgid}), + comment_id) + + response = self.client.get(requested_url) + self.assertRedirects(response, redirect_url) diff --git a/patchwork/tests/test_mboxviews.py b/patchwork/tests/test_mboxviews.py index 1e7bfb01..a7b0186a 100644 --- a/patchwork/tests/test_mboxviews.py +++ b/patchwork/tests/test_mboxviews.py @@ -13,8 +13,8 @@ import email from django.test import TestCase from django.urls import reverse -from patchwork.tests.utils import create_comment from patchwork.tests.utils import create_patch +from patchwork.tests.utils import create_patch_comment from patchwork.tests.utils import create_project from patchwork.tests.utils import create_person from patchwork.tests.utils import create_series @@ -34,8 +34,8 @@ class MboxPatchResponseTest(TestCase): project=self.project, submitter=self.person, content='comment 1 text\nAcked-by: 1\n') - create_comment( - submission=patch, + create_patch_comment( + patch=patch, submitter=self.person, content='comment 2 text\nAcked-by: 2\n') response = self.client.get( @@ -48,8 +48,8 @@ class MboxPatchResponseTest(TestCase): project=self.project, submitter=self.person, content='patch text\n') - create_comment( - submission=patch, + create_patch_comment( + patch=patch, submitter=self.person, content=u'comment\nAcked-by:\u00A0 foo') response = self.client.get( @@ -71,8 +71,8 @@ class MboxPatchSplitResponseTest(TestCase): submitter=self.person, diff='', content='comment 1 text\nAcked-by: 1\n---\nupdate\n') - self.comment = create_comment( - submission=self.patch, + self.comment = create_patch_comment( + patch=self.patch, submitter=self.person, content='comment 2 text\nAcked-by: 2\n') diff --git a/patchwork/tests/test_parser.py b/patchwork/tests/test_parser.py index 6fbc9da9..45ff3333 100644 --- a/patchwork/tests/test_parser.py +++ b/patchwork/tests/test_parser.py @@ -16,8 +16,8 @@ from django.test import TestCase from django.test import TransactionTestCase from django.utils import six -from patchwork.models import Comment from patchwork.models import Patch +from patchwork.models import PatchComment from patchwork.models import Person from patchwork.models import State from patchwork.parser import clean_subject @@ -529,7 +529,7 @@ class MultipleProjectPatchCommentTest(MultipleProjectPatchTest): patch = Patch.objects.filter(project=project)[0] # we should see the reply comment only self.assertEqual( - Comment.objects.filter(submission=patch).count(), 1) + PatchComment.objects.filter(patch=patch).count(), 1) class ListIdHeaderTest(TestCase): diff --git a/patchwork/tests/test_series.py b/patchwork/tests/test_series.py index 295a049d..5d750ce5 100644 --- a/patchwork/tests/test_series.py +++ b/patchwork/tests/test_series.py @@ -35,7 +35,7 @@ class _BaseTestCase(TestCase): mbox = mailbox.mbox(os.path.join(TEST_SERIES_DIR, name), create=False) for msg in mbox: obj = parser.parse_mail(msg, project.listid) - if type(obj) == models.CoverLetter: + if type(obj) == models.Cover: results[0].append(obj) elif type(obj) == models.Patch: results[1].append(obj) diff --git a/patchwork/tests/test_tags.py b/patchwork/tests/test_tags.py index 97afba0a..6c13687f 100644 --- a/patchwork/tests/test_tags.py +++ b/patchwork/tests/test_tags.py @@ -9,8 +9,8 @@ from django.test import TransactionTestCase from patchwork.models import Patch from patchwork.models import PatchTag from patchwork.models import Tag -from patchwork.tests.utils import create_comment from patchwork.tests.utils import create_patch +from patchwork.tests.utils import create_patch_comment class ExtractTagsTest(TestCase): @@ -107,8 +107,8 @@ class PatchTagsTest(TransactionTestCase): return '%s-by: Test Tagger \n' % tags[tagtype] def create_tag_comment(self, patch, tagtype=None): - comment = create_comment( - submission=patch, + comment = create_patch_comment( + patch=patch, content=self.create_tag(tagtype)) return comment diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py index 5e6a1b13..aeeb14d7 100644 --- a/patchwork/tests/utils.py +++ b/patchwork/tests/utils.py @@ -13,9 +13,10 @@ from django.contrib.auth.models import User from patchwork.models import Bundle from patchwork.models import Check -from patchwork.models import Comment -from patchwork.models import CoverLetter +from patchwork.models import Cover +from patchwork.models import CoverComment from patchwork.models import Patch +from patchwork.models import PatchComment from patchwork.models import Person from patchwork.models import Project from patchwork.models import Series @@ -202,8 +203,8 @@ def create_patch(**kwargs): def create_cover(**kwargs): - """Create 'CoverLetter' object.""" - num = CoverLetter.objects.count() + """Create 'Cover' object.""" + num = Cover.objects.count() # NOTE(stephenfin): Despite first appearances, passing 'series' to the # 'create' function doesn't actually cause the relationship to be created. @@ -231,7 +232,7 @@ def create_cover(**kwargs): } values.update(kwargs) - cover = CoverLetter.objects.create(**values) + cover = Cover.objects.create(**values) if series: series.add_cover_letter(cover) @@ -239,17 +240,30 @@ def create_cover(**kwargs): return cover -def create_comment(**kwargs): - """Create 'Comment' object.""" +def create_cover_comment(**kwargs): + """Create 'CoverComment' object.""" values = { 'submitter': create_person() if 'submitter' not in kwargs else None, - 'submission': create_patch() if 'submission' not in kwargs else None, + 'cover': create_cover() if 'cover' not in kwargs else None, 'msgid': make_msgid(), 'content': SAMPLE_CONTENT, } values.update(kwargs) - return Comment.objects.create(**values) + return CoverComment.objects.create(**values) + + +def create_patch_comment(**kwargs): + """Create 'PatchComment' object.""" + values = { + 'submitter': create_person() if 'submitter' not in kwargs else None, + 'patch': create_patch() if 'patch' not in kwargs else None, + 'msgid': make_msgid(), + 'content': SAMPLE_CONTENT, + } + values.update(kwargs) + + return PatchComment.objects.create(**values) def create_check(**kwargs): diff --git a/patchwork/urls.py b/patchwork/urls.py index 9c3b85f0..7d888d4a 100644 --- a/patchwork/urls.py +++ b/patchwork/urls.py @@ -249,10 +249,10 @@ if settings.ENABLE_REST_API: api_1_1_patterns = [ url(r'^patches/(?P[^/]+)/comments/$', - api_comment_views.CommentList.as_view(), + api_comment_views.PatchCommentList.as_view(), name='api-patch-comment-list'), url(r'^covers/(?P[^/]+)/comments/$', - api_comment_views.CommentList.as_view(), + api_comment_views.CoverCommentList.as_view(), name='api-cover-comment-list'), ] diff --git a/patchwork/views/comment.py b/patchwork/views/comment.py index 7dee8dc4..4f699224 100644 --- a/patchwork/views/comment.py +++ b/patchwork/views/comment.py @@ -4,20 +4,41 @@ # SPDX-License-Identifier: GPL-2.0-or-later from django import http -from django import shortcuts from django.urls import reverse from patchwork import models def comment(request, comment_id): - submission = shortcuts.get_object_or_404(models.Comment, - id=comment_id).submission - if models.Patch.objects.filter(id=submission.id).exists(): - url = 'patch-detail' - else: - url = 'cover-detail' - - return http.HttpResponseRedirect('%s#%s' % ( - reverse(url, kwargs={'project_id': submission.project.linkname, - 'msgid': submission.url_msgid}), comment_id)) + patch = None + cover = None + + try: + patch = models.PatchComment.objects.get(id=comment_id).patch + except models.PatchComment.DoesNotExist: + try: + cover = models.CoverComment.objects.get(id=comment_id).cover + except models.CoverComment.DoesNotExist: + pass + + if not patch and not cover: + raise http.Http404('No comment matches the given query.') + + if patch: + url = reverse( + 'patch-detail', + kwargs={ + 'project_id': patch.project.linkname, + 'msgid': patch.url_msgid, + }, + ) + else: # cover + url = reverse( + 'cover-detail', + kwargs={ + 'project_id': cover.project.linkname, + 'msgid': cover.url_msgid, + }, + ) + + return http.HttpResponseRedirect('%s#%s' % (url, comment_id)) diff --git a/patchwork/views/cover.py b/patchwork/views/cover.py index e90b7373..8ab0ba99 100644 --- a/patchwork/views/cover.py +++ b/patchwork/views/cover.py @@ -10,7 +10,7 @@ from django.shortcuts import get_object_or_404 from django.shortcuts import render from django.urls import reverse -from patchwork.models import CoverLetter +from patchwork.models import Cover from patchwork.models import Patch from patchwork.models import Project from patchwork.views.utils import cover_to_mbox @@ -22,7 +22,7 @@ def cover_detail(request, project_id, msgid): # redirect to patches where necessary try: - cover = get_object_or_404(CoverLetter, project_id=project.id, + cover = get_object_or_404(Cover, project_id=project.id, msgid=db_msgid) except Http404 as exc: patches = Patch.objects.filter( @@ -44,7 +44,7 @@ def cover_detail(request, project_id, msgid): comments = cover.comments.all() comments = comments.select_related('submitter') comments = comments.only('submitter', 'date', 'id', 'content', - 'submission') + 'cover') context['comments'] = comments return render(request, 'patchwork/submission.html', context) @@ -53,7 +53,7 @@ def cover_detail(request, project_id, msgid): def cover_mbox(request, project_id, msgid): db_msgid = ('<%s>' % msgid) project = get_object_or_404(Project, linkname=project_id) - cover = get_object_or_404(CoverLetter, project_id=project.id, + cover = get_object_or_404(Cover, project_id=project.id, msgid=db_msgid) response = HttpResponse(content_type='text/plain') @@ -65,7 +65,7 @@ def cover_mbox(request, project_id, msgid): def cover_by_id(request, cover_id): - cover = get_object_or_404(CoverLetter, id=cover_id) + cover = get_object_or_404(Cover, id=cover_id) url = reverse('cover-detail', kwargs={'project_id': cover.project.linkname, 'msgid': cover.url_msgid}) @@ -74,7 +74,7 @@ def cover_by_id(request, cover_id): def cover_mbox_by_id(request, cover_id): - cover = get_object_or_404(CoverLetter, id=cover_id) + cover = get_object_or_404(Cover, id=cover_id) url = reverse('cover-mbox', kwargs={'project_id': cover.project.linkname, 'msgid': cover.url_msgid}) diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py index bc508bee..3460b9ec 100644 --- a/patchwork/views/patch.py +++ b/patchwork/views/patch.py @@ -15,7 +15,7 @@ from django.urls import reverse from patchwork.forms import CreateBundleForm from patchwork.forms import PatchForm from patchwork.models import Bundle -from patchwork.models import CoverLetter +from patchwork.models import Cover from patchwork.models import Patch from patchwork.models import Project from patchwork.views import generic_list @@ -42,7 +42,7 @@ def patch_detail(request, project_id, msgid): try: patch = Patch.objects.get(project_id=project.id, msgid=db_msgid) except Patch.DoesNotExist as exc: - covers = CoverLetter.objects.filter( + covers = Cover.objects.filter( project_id=project.id, msgid=db_msgid, ) @@ -109,8 +109,7 @@ def patch_detail(request, project_id, msgid): comments = patch.comments.all() comments = comments.select_related('submitter') - comments = comments.only('submitter', 'date', 'id', 'content', - 'submission') + comments = comments.only('submitter', 'date', 'id', 'content', 'patch') context['comments'] = comments context['checks'] = patch.check_set.all().select_related('user') diff --git a/patchwork/views/utils.py b/patchwork/views/utils.py index 90580793..875edf45 100644 --- a/patchwork/views/utils.py +++ b/patchwork/views/utils.py @@ -16,8 +16,9 @@ from django.conf import settings from django.http import Http404 from django.utils import six -from patchwork.models import Comment +from patchwork.models import CoverComment from patchwork.models import Patch +from patchwork.models import PatchComment from patchwork.parser import split_from_header if settings.ENABLE_REST_API: @@ -35,12 +36,12 @@ class PatchMbox(MIMENonMultipart): def _submission_to_mbox(submission): - """Get an mbox representation of a single Submission. + """Get an mbox representation of a single submission. - Handles both Patch and CoverLetter objects. + Handles both Patch and Cover objects. Arguments: - submission: The Patch object to convert. + submission: The Patch or Cover object to convert. Returns: A string for the mbox file. @@ -62,8 +63,12 @@ def _submission_to_mbox(submission): postscript = '' # TODO(stephenfin): Make this use the tags infrastructure - for comment in Comment.objects.filter(submission=submission): - body += comment.patch_responses + if is_patch: + for comment in PatchComment.objects.filter(patch=submission): + body += comment.patch_responses + else: + for comment in CoverComment.objects.filter(cover=submission): + body += comment.patch_responses if postscript: body += '---\n' + postscript + '\n'