[RFC 2/2] models: Merge patch and first comment

Stephen Finucane stephen.finucane at intel.com
Sun Feb 7 10:19:38 AEDT 2016


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 achieve 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 at intel.com>
---
 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                                | 22 ++-------
 patchwork/templates/patchwork/patch.html           | 14 ++----
 patchwork/templatetags/syntax.py                   | 14 +++---
 6 files changed, 114 insertions(+), 52 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..00ae177 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=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..dbcb49a 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -317,6 +317,7 @@ class Patch(models.Model):
 
     submitter = models.ForeignKey(Person)
     name = models.CharField(max_length=255)
+    diff = models.TextField(null=True, blank=True)
     content = models.TextField(null=True, blank=True)
 
     # patch metadata
@@ -334,24 +335,6 @@ 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))
-
-    def comments(self):
-        """Retrieves all comments of this patch.
-
-        This includes the commit message and all other replies.
-        """
-        return Comment.objects.filter(patch=self)
-
     def _set_tag(self, tag, count):
         if count == 0:
             self.patchtag_set.filter(tag=tag).delete()
@@ -479,7 +462,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 f379317..ecdc4b7 100644
--- a/patchwork/templates/patchwork/patch.html
+++ b/patchwork/templates/patchwork/patch.html
@@ -197,28 +197,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>
@@ -230,7 +226,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 1f7e718..585c04a 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)
+    diff = escape(patch.diff)
 
     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)
-- 
2.0.0



More information about the Patchwork mailing list