From patchwork Tue Mar 8 21:54:03 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Finucane X-Patchwork-Id: 594357 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 9553D14032B for ; Wed, 9 Mar 2016 08:54:35 +1100 (AEDT) Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 813041A01ED for ; Wed, 9 Mar 2016 08:54:35 +1100 (AEDT) X-Original-To: patchwork@lists.ozlabs.org Delivered-To: patchwork@lists.ozlabs.org Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by lists.ozlabs.org (Postfix) with ESMTP id BC73E1A006D for ; Wed, 9 Mar 2016 08:54:19 +1100 (AEDT) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga101.jf.intel.com with ESMTP; 08 Mar 2016 13:54:17 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,558,1449561600"; d="scan'208";a="62403553" Received: from irvmail001.ir.intel.com ([163.33.26.43]) by fmsmga004.fm.intel.com with ESMTP; 08 Mar 2016 13:54:16 -0800 Received: from sivswdev01.ir.intel.com (sivswdev01.ir.intel.com [10.237.217.45]) by irvmail001.ir.intel.com (8.14.3/8.13.6/MailSET/Hub) with ESMTP id u28LsFrc026768; Tue, 8 Mar 2016 21:54:15 GMT Received: from sivswdev01.ir.intel.com (localhost [127.0.0.1]) by sivswdev01.ir.intel.com with ESMTP id u28LsFiE032181; Tue, 8 Mar 2016 21:54:15 GMT Received: (from sfinucan@localhost) by sivswdev01.ir.intel.com with id u28LsFxS032176; Tue, 8 Mar 2016 21:54:15 GMT From: Stephen Finucane To: patchwork@lists.ozlabs.org Subject: [PATCH 2/3] models: Merge patch and first comment Date: Tue, 8 Mar 2016 21:54:03 +0000 Message-Id: <1457474044-32132-2-git-send-email-stephen.finucane@intel.com> X-Mailer: git-send-email 2.0.0 In-Reply-To: <1457474044-32132-1-git-send-email-stephen.finucane@intel.com> References: <1457474044-32132-1-git-send-email-stephen.finucane@intel.com> X-BeenThere: patchwork@lists.ozlabs.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Patchwork development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Patchwork" At the moment a patch is split into two model entries: a Patch and a linked Comment. The original rationale for this was that a Patch is really a sub-class of Comment. A comment is a record of the text content of an incoming mail, while a patch is that, plus the patch content too. Hence the separation and a one-to-one relationship when a patch is present. However, this decision was made before Django added support for model inheritance and is no longer necessary. This change flatten the models in preparation for some email subclassing work. This is achieved by copying over the non-duplicated fields from the Comment to the linked Patch, then deleting the Comment. The migrations are broken into two steps: a schema migration and a data migration, per the recommendations of the Django documentation [1]. SQL migration scripts, where necessary, will need to be created manually as there appears to be no way to do this in a way that is RDBMS-independant [2][3]. [1] https://docs.djangoproject.com/en/1.9/topics/migrations/#data-migrations [2] https://stackoverflow.com/q/6856849/ [3] https://stackoverflow.com/q/224732/ Signed-off-by: Stephen Finucane Reviewed-by: Andy Doan --- patchwork/bin/parsemail.py | 28 ++--- patchwork/migrations/0006_add_patch_diff.py | 31 ++++++ .../0007_move_comment_content_to_patch_content.py | 57 ++++++++++ patchwork/models.py | 37 +++---- patchwork/templates/patchwork/patch.html | 14 +-- patchwork/templatetags/syntax.py | 14 +-- patchwork/tests/test_checks.py | 2 +- patchwork/tests/test_encodings.py | 8 +- patchwork/tests/test_expiry.py | 2 +- patchwork/tests/test_list.py | 2 +- patchwork/tests/test_mboxviews.py | 37 +++---- patchwork/tests/test_notifications.py | 8 +- patchwork/tests/test_patchparser.py | 115 +++++++++------------ patchwork/tests/test_tags.py | 2 +- patchwork/tests/utils.py | 2 +- patchwork/views/__init__.py | 22 ++-- patchwork/views/patch.py | 2 +- patchwork/views/xmlrpc.py | 2 +- 18 files changed, 220 insertions(+), 165 deletions(-) create mode 100644 patchwork/migrations/0006_add_patch_diff.py create mode 100644 patchwork/migrations/0007_move_comment_content_to_patch_content.py diff --git a/patchwork/bin/parsemail.py b/patchwork/bin/parsemail.py index d93c623..8bb924a 100755 --- a/patchwork/bin/parsemail.py +++ b/patchwork/bin/parsemail.py @@ -257,23 +257,17 @@ def find_content(project, mail): if pullurl or patchbuf: name = clean_subject(mail.get('Subject'), [project.linkname]) - patch = Patch(name=name, pull_url=pullurl, content=patchbuf, - date=mail_date(mail), headers=mail_headers(mail)) - - if commentbuf: - # If this is a new patch, we defer setting comment.patch until - # patch has been saved by the caller - if patch: - comment = Comment(date=mail_date(mail), - content=clean_content(commentbuf), - headers=mail_headers(mail)) - else: - cpatch = find_patch_for_comment(project, mail) - if not cpatch: - return (None, None, None) - comment = Comment(patch=cpatch, date=mail_date(mail), - content=clean_content(commentbuf), - headers=mail_headers(mail)) + patch = Patch(name=name, pull_url=pullurl, diff=patchbuf, + content=clean_content(commentbuf), date=mail_date(mail), + headers=mail_headers(mail)) + + if commentbuf and not patch: + cpatch = find_patch_for_comment(project, mail) + if not cpatch: + return (None, None, None) + comment = Comment(patch=cpatch, date=mail_date(mail), + content=clean_content(commentbuf), + headers=mail_headers(mail)) return (patch, comment, filenames) diff --git a/patchwork/migrations/0006_add_patch_diff.py b/patchwork/migrations/0006_add_patch_diff.py new file mode 100644 index 0000000..926ef95 --- /dev/null +++ b/patchwork/migrations/0006_add_patch_diff.py @@ -0,0 +1,31 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('patchwork', '0005_unselectable_maintainer_projects'), + ] + + operations = [ + migrations.RenameField( + model_name='patch', + old_name='content', + new_name='diff', + ), + migrations.AddField( + model_name='patch', + name='content', + field=models.TextField(null=True, blank=True), + ), + migrations.AlterField( + model_name='comment', + name='patch', + field=models.ForeignKey(related_query_name=b'comment', + related_name='comments', + to='patchwork.Patch'), + ), + ] diff --git a/patchwork/migrations/0007_move_comment_content_to_patch_content.py b/patchwork/migrations/0007_move_comment_content_to_patch_content.py new file mode 100644 index 0000000..066af9b --- /dev/null +++ b/patchwork/migrations/0007_move_comment_content_to_patch_content.py @@ -0,0 +1,57 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +def copy_comment_field(apps, schema_editor): + Comment = apps.get_model('patchwork', 'Comment') + Patch = apps.get_model('patchwork', 'Patch') + + for patch in Patch.objects.all(): + # this can only return one entry due to the unique_together + # constraint + comment = Comment.objects.get(patch=patch, msgid=patch.msgid) + patch.content = comment.content + patch.save() + + +def uncopy_comment_field(apps, schema_editor): + Comment = apps.get_model('patchwork', 'Comment') + Patch = apps.get_model('patchwork', 'Patch') + + for patch in Patch.objects.all(): + patch.content = None + patch.save() + + +def remove_duplicate_comments(apps, schema_editor): + Comment = apps.get_model('patchwork', 'Comment') + Patch = apps.get_model('patchwork', 'Patch') + + for patch in Patch.objects.all(): + comment = Comment.objects.get(patch=patch, msgid=patch.msgid) + comment.delete() + + +def recreate_comments(apps, schema_editor): + Comment = apps.get_model('patchwork', 'Comment') + Patch = apps.get_model('patchwork', 'Patch') + + for patch in Patch.objects.all(): + comment = Comment(patch=patch, msgid=patch.msgid, date=patch.date, + headers=patch.headers, submitter=patch.submitter, + content=patch.content) + comment.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ('patchwork', '0006_add_patch_diff'), + ] + + operations = [ + migrations.RunPython(copy_comment_field, uncopy_comment_field), + migrations.RunPython(remove_duplicate_comments, recreate_comments), + ] diff --git a/patchwork/models.py b/patchwork/models.py index ec0f4e1..c481ece 100644 --- a/patchwork/models.py +++ b/patchwork/models.py @@ -31,7 +31,6 @@ from django.conf import settings from django.contrib.sites.models import Site from django.core.urlresolvers import reverse from django.db import models -from django.db.models import Q from django.utils.encoding import python_2_unicode_compatible from django.utils.functional import cached_property from django.utils import six @@ -318,6 +317,7 @@ class Patch(models.Model): submitter = models.ForeignKey(Person) name = models.CharField(max_length=255) content = models.TextField(null=True, blank=True) + diff = models.TextField(null=True, blank=True) # patch metadata @@ -334,23 +334,17 @@ class Patch(models.Model): objects = PatchManager() - def commit_message(self): - """Retrieve the commit message.""" - return Comment.objects.filter(patch=self, msgid=self.msgid) - - def answers(self): - """Retrieve replies. - - This includes all repliess but not the commit message) - """ - return Comment.objects.filter(Q(patch=self) & ~Q(msgid=self.msgid)) + response_re = re.compile( + r'^(Tested|Reviewed|Acked|Signed-off|Nacked|Reported)-by: .*$', + re.M | re.I) - def comments(self): - """Retrieves all comments of this patch. + # TODO(stephenfin) Drag this out into a mixin + def patch_responses(self): + if not self.content: + return '' - This includes the commit message and all other replies. - """ - return Comment.objects.filter(patch=self) + return ''.join([match.group(0) + '\n' for match in + self.response_re.finditer(self.content)]) def _set_tag(self, tag, count): if count == 0: @@ -364,7 +358,11 @@ class Patch(models.Model): def refresh_tag_counts(self): tags = self.project.tags counter = Counter() - for comment in self.comment_set.all(): + + if self.content: + counter += extract_tags(self.content, tags) + + for comment in self.comments.all(): counter = counter + extract_tags(comment.content, tags) for tag in tags: @@ -379,6 +377,8 @@ class Patch(models.Model): super(Patch, self).save() + self.refresh_tag_counts() + def is_editable(self, user): if not user.is_authenticated(): return False @@ -479,7 +479,8 @@ class Patch(models.Model): class Comment(models.Model): # parent - patch = models.ForeignKey(Patch) + patch = models.ForeignKey(Patch, related_name='comments', + related_query_name='comment') # email metadata diff --git a/patchwork/templates/patchwork/patch.html b/patchwork/templates/patchwork/patch.html index 4d46354..a05f00d 100644 --- a/patchwork/templates/patchwork/patch.html +++ b/patchwork/templates/patchwork/patch.html @@ -191,28 +191,24 @@ function toggle_headers(link_id, headers_id) {% endif %} -{% for item in patch.commit_message %}

Commit Message

- {{ item.submitter|personify:project }} - {{ item.date }} + {{ patch.submitter|personify:project }} + {{ patch.date }} 
-{{ item|commentsyntax }}
+{{ patch|commentsyntax }}
-{% endfor %} -{% if patch.content %} +{% if patch.diff %}

Patch hide -{% if patch.content %} | download patch -{% endif %} | download mbox @@ -224,7 +220,7 @@ function toggle_headers(link_id, headers_id) {% endif %} -{% for item in patch.answers %} +{% for item in patch.comments.all %} {% if forloop.first %}

Comments

{% endif %} diff --git a/patchwork/templatetags/syntax.py b/patchwork/templatetags/syntax.py index 2a4164d..1e4d292 100644 --- a/patchwork/templatetags/syntax.py +++ b/patchwork/templatetags/syntax.py @@ -59,23 +59,23 @@ _span = '%s' @register.filter def patchsyntax(patch): - content = escape(patch.content).replace('\r\n', '\n') + diff = escape(patch.content).replace('\r\n', '\n') for (r, cls) in _patch_span_res: - content = r.sub(lambda x: _span % (cls, x.group(0)), content) + diff = r.sub(lambda x: _span % (cls, x.group(0)), diff) - content = _patch_chunk_re.sub( + diff = _patch_chunk_re.sub( lambda x: _span % ('p_chunk', x.group(1)) + ' ' + _span % ('p_context', x.group(2)), - content) + diff) - return mark_safe(content) + return mark_safe(diff) @register.filter -def commentsyntax(comment): - content = escape(comment.content) +def commentsyntax(patch): + content = escape(patch.content) for (r, cls) in _comment_span_res: content = r.sub(lambda x: _span % (cls, x.group(0)), content) diff --git a/patchwork/tests/test_checks.py b/patchwork/tests/test_checks.py index 4711b4a..85c02c1 100644 --- a/patchwork/tests/test_checks.py +++ b/patchwork/tests/test_checks.py @@ -36,7 +36,7 @@ class PatchChecksTest(TransactionTestCase): self.patch = Patch(project=project, msgid='x', name=defaults.patch_name, submitter=defaults.patch_author_person, - content='') + diff='') self.patch.save() self.user = create_user() diff --git a/patchwork/tests/test_encodings.py b/patchwork/tests/test_encodings.py index e39e319..fa5c889 100644 --- a/patchwork/tests/test_encodings.py +++ b/patchwork/tests/test_encodings.py @@ -37,7 +37,7 @@ class UTF8PatchViewTest(TestCase): self.patch = Patch(project=defaults.project, msgid='x', name=defaults.patch_name, submitter=defaults.patch_author_person, - content=self.patch_content) + diff=self.patch_content) self.patch.save() self.client = Client() @@ -48,14 +48,14 @@ class UTF8PatchViewTest(TestCase): def testMboxView(self): response = self.client.get('/patch/%d/mbox/' % self.patch.id) self.assertEqual(response.status_code, 200) - self.assertTrue(self.patch.content in + self.assertTrue(self.patch.diff in response.content.decode(self.patch_encoding)) def testRawView(self): response = self.client.get('/patch/%d/raw/' % self.patch.id) self.assertEqual(response.status_code, 200) self.assertEqual(response.content.decode(self.patch_encoding), - self.patch.content) + self.patch.diff) def tearDown(self): self.patch.delete() @@ -79,7 +79,7 @@ class UTF8HeaderPatchViewTest(UTF8PatchViewTest): self.patch = Patch(project=defaults.project, msgid='x', name=defaults.patch_name, submitter=self.patch_author, - content=self.patch_content) + diff=self.patch_content) self.patch.save() self.client = Client() diff --git a/patchwork/tests/test_expiry.py b/patchwork/tests/test_expiry.py index a0596dc..eda5150 100644 --- a/patchwork/tests/test_expiry.py +++ b/patchwork/tests/test_expiry.py @@ -87,7 +87,7 @@ class TestRegistrationExpiry(TestCase): patch = Patch(project=defaults.project, msgid='test@example.com', name='test patch', submitter=defaults.patch_author_person, - content=defaults.patch) + diff=defaults.patch) patch.save() # ... then starts registration... diff --git a/patchwork/tests/test_list.py b/patchwork/tests/test_list.py index 1139a34..bf009f9 100644 --- a/patchwork/tests/test_list.py +++ b/patchwork/tests/test_list.py @@ -77,7 +77,7 @@ class PatchOrderTest(TestCase): person = Person(name=name, email=email) person.save() patch = Patch(project=defaults.project, msgid=patch_name, - submitter=person, content='', date=date) + submitter=person, diff='', date=date) patch.save() def _extract_patch_ids(self, response): diff --git a/patchwork/tests/test_mboxviews.py b/patchwork/tests/test_mboxviews.py index a2bee0d..0bba9e2 100644 --- a/patchwork/tests/test_mboxviews.py +++ b/patchwork/tests/test_mboxviews.py @@ -43,14 +43,12 @@ class MboxPatchResponseTest(TestCase): self.patch = Patch(project=defaults.project, msgid='p1', name='testpatch', - submitter=self.person, content='') + submitter=self.person, diff='', + content='comment 1 text\nAcked-by: 1\n') self.patch.save() - comment = Comment(patch=self.patch, msgid='p1', - submitter=self.person, - content='comment 1 text\nAcked-by: 1\n') - comment.save() - comment = Comment(patch=self.patch, msgid='p2', + comment = Comment(patch=self.patch, + msgid='p2', submitter=self.person, content='comment 2 text\nAcked-by: 2\n') comment.save() @@ -73,16 +71,15 @@ class MboxPatchSplitResponseTest(TestCase): self.person = defaults.patch_author_person self.person.save() - self.patch = Patch(project=defaults.project, - msgid='p1', name='testpatch', - submitter=self.person, content='') + self.patch = Patch( + project=defaults.project, + msgid='p1', name='testpatch', + submitter=self.person, diff='', + content='comment 1 text\nAcked-by: 1\n---\nupdate\n') self.patch.save() - comment = Comment(patch=self.patch, msgid='p1', - submitter=self.person, - content='comment 1 text\nAcked-by: 1\n---\nupdate\n') - comment.save() - comment = Comment(patch=self.patch, msgid='p2', + comment = Comment(patch=self.patch, + msgid='p2', submitter=self.person, content='comment 2 text\nAcked-by: 2\n') comment.save() @@ -240,18 +237,14 @@ class MboxCommentPostcriptUnchangedTest(TestCase): self.person = defaults.patch_author_person self.person.save() + self.txt = 'some comment\n---\n some/file | 1 +\n' + self.patch = Patch(project=defaults.project, msgid='p1', name='testpatch', - submitter=self.person, content='') + submitter=self.person, diff='', + content=self.txt) self.patch.save() - self.txt = 'some comment\n---\n some/file | 1 +\n' - - comment = Comment(patch=self.patch, msgid='p1', - submitter=self.person, - content=self.txt) - comment.save() - def testCommentUnchanged(self): response = self.client.get('/patch/%d/mbox/' % self.patch.id) self.assertContains(response, self.txt) diff --git a/patchwork/tests/test_notifications.py b/patchwork/tests/test_notifications.py index b7dd359..f4f8c51 100644 --- a/patchwork/tests/test_notifications.py +++ b/patchwork/tests/test_notifications.py @@ -40,7 +40,7 @@ class PatchNotificationModelTest(TestCase): self.submitter = defaults.patch_author_person self.submitter.save() self.patch = Patch(project=self.project, msgid='testpatch', - name='testpatch', content='', + name='testpatch', diff='', submitter=self.submitter) def tearDown(self): @@ -133,7 +133,7 @@ class PatchNotificationEmailTest(TestCase): self.submitter = defaults.patch_author_person self.submitter.save() self.patch = Patch(project=self.project, msgid='testpatch', - name='testpatch', content='', + name='testpatch', diff='', submitter=self.submitter) self.patch.save() @@ -205,7 +205,7 @@ class PatchNotificationEmailTest(TestCase): def testNotificationMerge(self): patches = [self.patch, Patch(project=self.project, msgid='testpatch-2', - name='testpatch 2', content='', + name='testpatch 2', diff='', submitter=self.submitter)] for patch in patches: @@ -228,7 +228,7 @@ class PatchNotificationEmailTest(TestCase): are held""" patches = [self.patch, Patch(project=self.project, msgid='testpatch-2', - name='testpatch 2', content='', + name='testpatch 2', diff='', submitter=self.submitter)] for patch in patches: diff --git a/patchwork/tests/test_patchparser.py b/patchwork/tests/test_patchparser.py index f3d9b6b..1fba35c 100644 --- a/patchwork/tests/test_patchparser.py +++ b/patchwork/tests/test_patchparser.py @@ -41,26 +41,19 @@ class PatchTest(TestCase): class InlinePatchTest(PatchTest): patch_filename = '0001-add-line.patch' - test_comment = 'Test for attached patch' + test_content = 'Test for attached patch' expected_filenames = ['meep.text'] def setUp(self): self.orig_patch = read_patch(self.patch_filename) - email = create_email(self.test_comment + '\n' + self.orig_patch) - self.patch, self.comment, self.filenames = find_content( - self.project, email) - - def testPatchPresence(self): - self.assertTrue(self.patch is not None) + email = create_email(self.test_content + '\n' + self.orig_patch) + self.patch, _, self.filenames = find_content(self.project, email) def testPatchContent(self): - self.assertEqual(self.patch.content, self.orig_patch) - - def testCommentPresence(self): - self.assertTrue(self.comment is not None) + self.assertEqual(self.patch.diff, self.orig_patch) def testCommentContent(self): - self.assertEqual(self.comment.content, self.test_comment) + self.assertEqual(self.patch.content, self.test_content) def testFilenames(self): self.assertEqual(self.filenames, self.expected_filenames) @@ -73,11 +66,10 @@ class AttachmentPatchTest(InlinePatchTest): def setUp(self): self.orig_patch = read_patch(self.patch_filename) - email = create_email(self.test_comment, multipart=True) + email = create_email(self.test_content, multipart=True) attachment = MIMEText(self.orig_patch, _subtype=self.content_subtype) email.attach(attachment) - self.patch, self.comment, self.filenames = find_content( - self.project, email) + self.patch, _, self.filenames = find_content(self.project, email) class AttachmentXDiffPatchTest(AttachmentPatchTest): @@ -90,10 +82,9 @@ class UTF8InlinePatchTest(InlinePatchTest): def setUp(self): self.orig_patch = read_patch(self.patch_filename, self.patch_encoding) - email = create_email(self.test_comment + '\n' + self.orig_patch, + email = create_email(self.test_content + '\n' + self.orig_patch, content_encoding=self.patch_encoding) - self.patch, self.comment, self.filenames = find_content( - self.project, email) + self.patch, _, self.filenames = find_content(self.project, email) class NoCharsetInlinePatchTest(InlinePatchTest): @@ -103,43 +94,40 @@ class NoCharsetInlinePatchTest(InlinePatchTest): def setUp(self): self.orig_patch = read_patch(self.patch_filename) - email = create_email(self.test_comment + '\n' + self.orig_patch) + email = create_email(self.test_content + '\n' + self.orig_patch) del email['Content-Type'] del email['Content-Transfer-Encoding'] - self.patch, self.comment, self.filenames = find_content( - self.project, email) + self.patch, _, self.filenames = find_content(self.project, email) class SignatureCommentTest(InlinePatchTest): patch_filename = '0001-add-line.patch' - test_comment = 'Test comment\nmore comment' + test_content = 'Test comment\nmore comment' def setUp(self): self.orig_patch = read_patch(self.patch_filename) email = create_email( - self.test_comment + '\n' + + self.test_content + '\n' + '-- \nsig\n' + self.orig_patch) - self.patch, self.comment, self.filenames = find_content( - self.project, email) + self.patch, _, self.filenames = find_content(self.project, email) class ListFooterTest(InlinePatchTest): patch_filename = '0001-add-line.patch' - test_comment = 'Test comment\nmore comment' + test_content = 'Test comment\nmore comment' def setUp(self): self.orig_patch = read_patch(self.patch_filename) email = create_email( - self.test_comment + '\n' + + self.test_content + '\n' + '_______________________________________________\n' + 'Linuxppc-dev mailing list\n' + self.orig_patch) - self.patch, self.comment, self.filenames = find_content( - self.project, email) + self.patch, _, self.filenames = find_content(self.project, email) class DiffWordInCommentTest(InlinePatchTest): - test_comment = 'Lines can start with words beginning in "diff"\n' + \ + test_content = 'Lines can start with words beginning in "diff"\n' + \ 'difficult\nDifferent' @@ -147,14 +135,14 @@ class UpdateCommentTest(InlinePatchTest): """ Test for '---\nUpdate: v2' style comments to patches. """ patch_filename = '0001-add-line.patch' - test_comment = 'Test comment\nmore comment\n---\nUpdate: test update' + test_content = 'Test comment\nmore comment\n---\nUpdate: test update' class UpdateSigCommentTest(SignatureCommentTest): """ Test for '---\nUpdate: v2' style comments to patches, with a sig """ patch_filename = '0001-add-line.patch' - test_comment = 'Test comment\nmore comment\n---\nUpdate: test update' + test_content = 'Test comment\nmore comment\n---\nUpdate: test update' class SenderEncodingTest(TestCase): @@ -217,7 +205,7 @@ class SubjectEncodingTest(PatchTest): self.email = message_from_string(mail) def testSubjectEncoding(self): - patch, comment, _ = find_content(self.project, self.email) + patch, _, _ = find_content(self.project, self.email) self.assertEqual(patch.name, self.subject) @@ -280,7 +268,7 @@ class MultipleProjectPatchTest(TestCase): handled correctly """ fixtures = ['default_states'] - test_comment = 'Test Comment' + test_content = 'Test Comment' patch_filename = '0001-add-line.patch' msgid = '<1@example.com>' @@ -294,7 +282,7 @@ class MultipleProjectPatchTest(TestCase): self.p2.save() patch = read_patch(self.patch_filename) - email = create_email(self.test_comment + '\n' + patch) + email = create_email(self.test_content + '\n' + patch) del email['Message-Id'] email['Message-Id'] = self.msgid @@ -338,9 +326,8 @@ class MultipleProjectPatchCommentTest(MultipleProjectPatchTest): def testParsedComment(self): for project in [self.p1, self.p2]: patch = Patch.objects.filter(project=project)[0] - # we should see two comments now - the original mail with the - # patch, and the one we parsed in setUp() - self.assertEqual(Comment.objects.filter(patch=patch).count(), 2) + # we should see the reply comment only + self.assertEqual(Comment.objects.filter(patch=patch).count(), 1) class ListIdHeaderTest(TestCase): @@ -407,8 +394,8 @@ class GitPullTest(MBoxPatchTest): patch, comment, _ = find_content(self.project, self.mail) self.assertTrue(patch is not None) self.assertTrue(patch.pull_url is not None) - self.assertTrue(patch.content is None) - self.assertTrue(comment is not None) + self.assertTrue(patch.diff is None) + self.assertTrue(patch.content is not None) class GitPullWrappedTest(GitPullTest): @@ -419,16 +406,16 @@ class GitPullWithDiffTest(MBoxPatchTest): mail_file = '0003-git-pull-request-with-diff.mbox' def testGitPullWithDiff(self): - patch, comment, _ = find_content(self.project, self.mail) + patch, _, _ = find_content(self.project, self.mail) self.assertTrue(patch is not None) self.assertEqual('git://git.kernel.org/pub/scm/linux/kernel/git/tip/' 'linux-2.6-tip.git x86-fixes-for-linus', patch.pull_url) self.assertTrue( - patch.content.startswith( + patch.diff.startswith( 'diff --git a/arch/x86/include/asm/smp.h'), - patch.content) - self.assertTrue(comment is not None) + patch.diff) + self.assertTrue(patch.content is not None) class GitPullGitSSHUrlTest(GitPullTest): @@ -447,23 +434,22 @@ class GitRenameOnlyTest(MBoxPatchTest): mail_file = '0008-git-rename.mbox' def testGitRename(self): - patch, comment, _ = find_content(self.project, self.mail) + patch, _, _ = find_content(self.project, self.mail) self.assertTrue(patch is not None) - self.assertTrue(comment is not None) - self.assertEqual(patch.content.count("\nrename from "), 2) - self.assertEqual(patch.content.count("\nrename to "), 2) + self.assertEqual(patch.diff.count("\nrename from "), 2) + self.assertEqual(patch.diff.count("\nrename to "), 2) class GitRenameWithDiffTest(MBoxPatchTest): mail_file = '0009-git-rename-with-diff.mbox' def testGitRename(self): - patch, comment, _ = find_content(self.project, self.mail) + patch, _, _ = find_content(self.project, self.mail) self.assertTrue(patch is not None) - self.assertTrue(comment is not None) - self.assertEqual(patch.content.count("\nrename from "), 2) - self.assertEqual(patch.content.count("\nrename to "), 2) - self.assertEqual(patch.content.count('\n-a\n+b'), 1) + self.assertTrue(patch.content is not None) + self.assertEqual(patch.diff.count("\nrename from "), 2) + self.assertEqual(patch.diff.count("\nrename to "), 2) + self.assertEqual(patch.diff.count('\n-a\n+b'), 1) class CVSFormatPatchTest(MBoxPatchTest): @@ -472,8 +458,8 @@ class CVSFormatPatchTest(MBoxPatchTest): def testPatch(self): patch, comment, _ = find_content(self.project, self.mail) self.assertTrue(patch is not None) - self.assertTrue(comment is not None) - self.assertTrue(patch.content.startswith('Index')) + self.assertTrue(comment is None) + self.assertTrue(patch.diff.startswith('Index')) class CharsetFallbackPatchTest(MBoxPatchTest): @@ -485,8 +471,9 @@ class CharsetFallbackPatchTest(MBoxPatchTest): def testPatch(self): patch, comment, _ = find_content(self.project, self.mail) - self.assertTrue(patch is not None) - self.assertTrue(comment is not None) + self.assertTrue(comment is None) + self.assertTrue(patch.content is not None) + self.assertTrue(patch.diff is not None) class NoNewlineAtEndOfFilePatchTest(MBoxPatchTest): @@ -495,17 +482,17 @@ class NoNewlineAtEndOfFilePatchTest(MBoxPatchTest): def testPatch(self): patch, comment, _ = find_content(self.project, self.mail) self.assertTrue(patch is not None) - self.assertTrue(comment is not None) - self.assertTrue(patch.content.startswith( + self.assertTrue(comment is None) + self.assertTrue(patch.diff.startswith( 'diff --git a/tools/testing/selftests/powerpc/Makefile')) # Confirm the trailing no newline marker doesn't end up in the comment self.assertFalse( - comment.content.rstrip().endswith('\ No newline at end of file')) + patch.content.rstrip().endswith('\ No newline at end of file')) # Confirm it's instead at the bottom of the patch self.assertTrue( - patch.content.rstrip().endswith('\ No newline at end of file')) + patch.diff.rstrip().endswith('\ No newline at end of file')) # Confirm we got both markers - self.assertEqual(2, patch.content.count('\ No newline at end of file')) + self.assertEqual(2, patch.diff.count('\ No newline at end of file')) class DelegateRequestTest(TestCase): @@ -619,7 +606,7 @@ class InitialPatchStateTest(TestCase): class ParseInitialTagsTest(PatchTest): patch_filename = '0001-add-line.patch' - test_comment = ('test comment\n\n' + + test_content = ('test comment\n\n' + 'Tested-by: Test User \n' + 'Reviewed-by: Test User \n') fixtures = ['default_tags', 'default_states'] @@ -629,7 +616,7 @@ class ParseInitialTagsTest(PatchTest): project.listid = 'test.example.com' project.save() self.orig_patch = read_patch(self.patch_filename) - email = create_email(self.test_comment + '\n' + self.orig_patch, + email = create_email(self.test_content + '\n' + self.orig_patch, project=project) email['Message-Id'] = '<1@example.com>' parse_mail(email) diff --git a/patchwork/tests/test_tags.py b/patchwork/tests/test_tags.py index 70ca43d..b57d5fd 100644 --- a/patchwork/tests/test_tags.py +++ b/patchwork/tests/test_tags.py @@ -129,7 +129,7 @@ class PatchTagsTest(TransactionTestCase): self.patch = Patch(project=project, msgid='x', name=defaults.patch_name, submitter=defaults.patch_author_person, - content='') + diff='') self.patch.save() self.tagger = 'Test Tagger ' diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py index 45eb2d3..375f188 100644 --- a/patchwork/tests/utils.py +++ b/patchwork/tests/utils.py @@ -98,7 +98,7 @@ def create_patches(count=1): submitter=defaults.patch_author_person, msgid=make_msgid(), name='testpatch%d' % (i + 1), - content=defaults.patch) + diff=defaults.patch) patch.save() patches.append(patch) diff --git a/patchwork/views/__init__.py b/patchwork/views/__init__.py index 00ff96e..7d31a06 100644 --- a/patchwork/views/__init__.py +++ b/patchwork/views/__init__.py @@ -316,16 +316,10 @@ class PatchMbox(MIMENonMultipart): def patch_to_mbox(patch): postscript_re = re.compile('\n-{2,3} ?\n') - - comment = None - try: - comment = Comment.objects.get(patch=patch, msgid=patch.msgid) - except Exception: - pass - body = '' - if comment: - body = comment.content.strip() + "\n" + + if patch.content: + body = patch.content.strip() + "\n" parts = postscript_re.split(body, 1) if len(parts) == 2: @@ -335,15 +329,17 @@ def patch_to_mbox(patch): else: postscript = '' - for comment in Comment.objects.filter(patch=patch) \ - .exclude(msgid=patch.msgid): + # TODO(stephenfin): Make this use the tags infrastructure + body += patch.patch_responses() + + for comment in Comment.objects.filter(patch=patch): body += comment.patch_responses() if postscript: body += '---\n' + postscript + '\n' - if patch.content: - body += '\n' + patch.content + if patch.diff: + body += '\n' + patch.diff delta = patch.date - datetime.datetime.utcfromtimestamp(0) utc_timestamp = delta.seconds + delta.days * 24 * 3600 diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py index e739c90..aa0e9cc 100644 --- a/patchwork/views/patch.py +++ b/patchwork/views/patch.py @@ -91,7 +91,7 @@ def patch(request, patch_id): def content(request, patch_id): patch = get_object_or_404(Patch, id=patch_id) response = HttpResponse(content_type="text/x-patch") - response.write(patch.content) + response.write(patch.diff) response['Content-Disposition'] = 'attachment; filename=' + \ patch.filename().replace(';', '').replace('\n', '') return response diff --git a/patchwork/views/xmlrpc.py b/patchwork/views/xmlrpc.py index 7ad34d8..1f48959 100644 --- a/patchwork/views/xmlrpc.py +++ b/patchwork/views/xmlrpc.py @@ -716,7 +716,7 @@ def patch_get_diff(patch_id): """ try: patch = Patch.objects.filter(id=patch_id)[0] - return patch.content + return patch.diff except Patch.DoesNotExist: return ''