From: Stephen Finucane <stephen.finucane@intel.com>
Date: Sat, 6 Feb 2016 20:32:20 +0000 (+0000)
Subject: models: Merge patch and first comment
X-Git-Tag: v2.0.0-rc1~414
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=ef56359fb776;p=thirdparty%2Fpatchwork.git

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 b32ef0b2..9640ff30 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 00000000..926ef954
--- /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 00000000..63d57bae
--- /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 60556638..3e701fd7 100644
--- 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 4d463543..a05f00dc 100644
--- 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 2a4164d6..85ed201d 100644
--- 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 4711b4ad..85c02c11 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 e39e3194..fa5c8895 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 a0596dc2..eda51503 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 1139a348..bf009f9f 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 a2bee0d1..0bba9e21 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 b7dd359c..f4f8c51d 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 f3d9b6bc..1fba35c7 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 <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 70ca43d6..b57d5fde 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 <tagger@example.com>'
 
diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py
index 45eb2d3f..375f188c 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 00ff96e8..b12a1275 100644
--- 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 e739c908..aa0e9cc2 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 7ad34d80..1f489599 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 ''