[PATCH v2 2/3] models: Merge patch and first comment
Stephen Finucane
stephen.finucane at intel.com
Mon Mar 14 07:50:50 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 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 at intel.com>
---
v2:
- Handle patches without an attached comment in the migration
- Remove unused import
- Update missed references of patch.content -> patch.diff
---
patchwork/bin/parsemail.py | 28 ++---
patchwork/migrations/0006_add_patch_diff.py | 31 ++++++
.../0007_move_comment_content_to_patch_content.py | 63 +++++++++++
patchwork/models.py | 41 ++++----
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, 228 insertions(+), 167 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 b32ef0b..9640ff3 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..c320d28
--- /dev/null
+++ b/patchwork/migrations/0007_move_comment_content_to_patch_content.py
@@ -0,0 +1,63 @@
+# -*- 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):
+ 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():
+ 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 ec0f4e1..275938c 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:
@@ -374,11 +372,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
@@ -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)
</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 2a4164d..85ed201 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 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 at 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 at 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 at example.com>\n' +
'Reviewed-by: Test User <test at 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 at 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 <tagger at example.com>'
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 ''
--
2.0.0
More information about the Patchwork
mailing list