models: Merge patch and first comment

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 <stephen.finucane@intel.com>
Reviewed-by: Andy Doan <andy.doan@linaro.org>

diff --git a/patchwork/bin/parsemail.py b/patchwork/bin/parsemail.py
index b32ef0b2c447e720f0000b21ac97a3cae46a197a..9640ff306fe0c22c4541995709f28c8c40f7489a 100755 (executable)
--- 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 (file)
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 (file)
index 0000000..63d57ba
--- /dev/null
+++ b/patchwork/migrations/0007_move_comment_content_to_patch_content.py
@@ -0,0 +1,68 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+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():
+        try:
+            # when available, this can only return one entry due to the
+            # unique_together constraint
+            comment = Comment.objects.get(patch=patch, msgid=patch.msgid)
+        except Comment.DoesNotExist:
+            # though there's no requirement to actually have a comment
+            continue
+
+        patch.content = comment.content
+        patch.save()
+
+
+def uncopy_comment_field(apps, schema_editor):
+    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():
+        try:
+            # when available, this can only return one entry due to the
+            # unique_together constraint
+            comment = Comment.objects.get(patch=patch, msgid=patch.msgid)
+            comment.delete()
+        except Comment.DoesNotExist:
+            # though there's no requirement to actually have a comment
+            continue
+
+
+def recreate_comments(apps, schema_editor):
+    Comment = apps.get_model('patchwork', 'Comment')
+    Patch = apps.get_model('patchwork', 'Patch')
+
+    for patch in Patch.objects.all():
+        if patch.content:
+            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 60556638ca316831d1e02990c7b449633bdc2c8d..3e701fd78423b966a2a3f77d81f1ca82520e606f 100644 (file)
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -30,7 +30,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.six.moves import filter
@@ -278,6 +277,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
 
@@ -294,23 +294,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:
@@ -324,7 +318,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:
@@ -334,11 +332,13 @@ class Patch(models.Model):
         if not hasattr(self, 'state') or not self.state:
             self.state = get_default_initial_patch_state()
 
-        if self.hash is None and self.content is not None:
-            self.hash = hash_patch(self.content).hexdigest()
+        if self.hash is None and self.diff is not None:
+            self.hash = hash_patch(self.diff).hexdigest()
 
         super(Patch, self).save()
 
+        self.refresh_tag_counts()
+
     def is_editable(self, user):
         if not user.is_authenticated():
             return False
@@ -439,7 +439,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 4d4635439c606e5748e72ec75b76e3c5da3b51b9..a05f00dcafc6a14e261d7edc17b5e2ab682f3866 100644 (file)
--- a/patchwork/templates/patchwork/patch.html
+++ b/patchwork/templates/patchwork/patch.html
@@ -191,28 +191,24 @@ function toggle_headers(link_id, headers_id)
 </table>
 {% endif %}
 
-{% for item in patch.commit_message %}
 <h2>Commit Message</h2>
 <div class="comment">
 <div class="meta">
- <span>{{ item.submitter|personify:project }}</span>
- <span class="pull-right">{{ item.date }}</span>
+ <span>{{ patch.submitter|personify:project }}</span>
+ <span class="pull-right">{{ patch.date }}</span>
 </div>
 <pre class="content">
-{{ item|commentsyntax }}
+{{ patch|commentsyntax }}
 </pre>
 </div>
-{% endfor %}
 
-{% if patch.content %}
+{% if patch.diff %}
 <h2>
  Patch
  <a href="javascript:toggle_headers('hide-patch', 'patch')" id="hide-patch">hide</a></span>
-{% if patch.content %}
  <span>|</span>
  <a href="{% url 'patch-raw' patch_id=patch.id %}"
    >download patch</a>
-{% endif %}
  <span>|</span>
  <a href="{% url 'patch-mbox' patch_id=patch.id %}"
    >download mbox</a>
@@ -224,7 +220,7 @@ function toggle_headers(link_id, headers_id)
 </div>
 {% endif %}
 
-{% for item in patch.answers %}
+{% for item in patch.comments.all %}
 {% if forloop.first %}
 <h2>Comments</h2>
 {% endif %}

diff --git a/patchwork/templatetags/syntax.py b/patchwork/templatetags/syntax.py
index 2a4164d6e9b244587bc09d13ca223f8479626593..85ed201d6e3b215235b48507b27ccf8b929668e8 100644 (file)
--- a/patchwork/templatetags/syntax.py
+++ b/patchwork/templatetags/syntax.py
@@ -59,23 +59,23 @@ _span = '<span class="%s">%s</span>'
 
 @register.filter
 def patchsyntax(patch):
-    content = escape(patch.content).replace('\r\n', '\n')
+    diff = escape(patch.diff).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 4711b4adf36bf1039441ce8482e7ec5152806d15..85c02c118c6bc2c3bb4ec9da2d9d051820398b68 100644 (file)
--- 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 e39e31940b7ba67f98c1fe25751f66aa577ae543..fa5c88955504eb98973af09c4bd7ab68ed09105e 100644 (file)
--- 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 a0596dc21a30201ff767ad92e99d1b3ad084d8ef..eda515031abc9b159ea5d701bab5445e2493a2ff 100644 (file)
--- 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 1139a3485a660186381daab2f96bf475fd6aeaf0..bf009f9ff0b2d5cf66b18da5c63c98048ddf5950 100644 (file)
--- 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 a2bee0d1e8729b7383c41f11c5e4fb58cfbc77da..0bba9e21a73f63a40bd87ed29d537600dff156fb 100644 (file)
--- 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 b7dd359ca685bd0c868560a576a8558d416fc592..f4f8c51d5962c48a6d1ef43aa2c72d354fd34838 100644 (file)
--- 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 f3d9b6bcd11fec5dc735d0587df1700cee28fe0f..1fba35c701bed3f02ff04ca7cb5f066073051c59 100644 (file)
--- 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 <test@example.com>\n' +
                     'Reviewed-by: Test User <test@example.com>\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 70ca43d64b6c8b2d92d6f74593575da9aadd626f..b57d5fde4743151721e961cc50907bf94315ba38 100644 (file)
--- 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 <tagger@example.com>'
 

diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py
index 45eb2d3f9d4649ccce92574c0a8b9ecc76abd1db..375f188c45237f156113d8532a629fd6224e9954 100644 (file)
--- 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 00ff96e84f9a86fecca2c0059f2078ccbf4869fe..b12a1275b388d19e7b9899c586f8a003a3371876 100644 (file)
--- a/patchwork/views/__init__.py
+++ b/patchwork/views/__init__.py
@@ -255,9 +255,9 @@ def generic_list(request, project, view,
     if not editable_order:
         patches = order.apply(patches)
 
-    # we don't need the content or headers for a list; they're text fields
-    # that can potentially contain a lot of data
-    patches = patches.defer('content', 'headers')
+    # we don't need the content, diff or headers for a list; they're text
+    # fields that can potentially contain a lot of data
+    patches = patches.defer('content', 'diff', 'headers')
 
     # but we will need to follow the state and submitter relations for
     # rendering the list template
@@ -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 e739c9082818b148e56b49b6b7321ddc1cd0d315..aa0e9cc28a56e5bdd420dc3d6ae4da3d496ee0ce 100644 (file)
--- 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 7ad34d809400c0ed1fd19bf7f9b44624cacd0dce..1f48959942e7a8706badc3a7e393ad9fcfc429dc 100644 (file)
--- 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 ''