[PATCH 6/6] Burninate CoverLetter
Daniel Axtens
dja at axtens.net
Wed Sep 18 16:17:31 AEST 2019
A patch must have a diff or a pull_url.
If a Submission does not have a diff or a pull_url, it must be
a CoverLetter.
This is enough to get rid of the CoverLetter model. We do have some
work to do around the interactions with Series, which is currently
super ugly, but it's good enough for a proof of concept.
Signed-off-by: Daniel Axtens <dja at axtens.net>
---
patchwork/admin.py | 3 --
patchwork/api/cover.py | 16 +++++-----
patchwork/api/embedded.py | 2 +-
patchwork/api/filters.py | 13 +++++++--
patchwork/management/commands/parsearchive.py | 4 +--
patchwork/migrations/0041_drop_coverletter.py | 29 +++++++++++++++++++
patchwork/models.py | 19 +++++-------
patchwork/parser.py | 9 +++---
patchwork/signals.py | 4 +--
patchwork/templates/patchwork/submission.html | 10 +++----
patchwork/tests/api/test_cover.py | 5 ++--
patchwork/tests/test_series.py | 14 ++++-----
patchwork/tests/utils.py | 6 ++--
patchwork/views/cover.py | 25 +++++++++++-----
patchwork/views/patch.py | 1 +
15 files changed, 100 insertions(+), 60 deletions(-)
create mode 100644 patchwork/migrations/0041_drop_coverletter.py
diff --git a/patchwork/admin.py b/patchwork/admin.py
index f9a94c6f5c07..48fe43b93c99 100644
--- a/patchwork/admin.py
+++ b/patchwork/admin.py
@@ -11,7 +11,6 @@ from django.db.models import Prefetch
from patchwork.models import Bundle
from patchwork.models import Check
from patchwork.models import Comment
-from patchwork.models import CoverLetter
from patchwork.models import DelegationRule
from patchwork.models import Patch
from patchwork.models import Person
@@ -83,8 +82,6 @@ class SubmissionAdmin(admin.ModelAdmin):
admin.site.register(Submission, SubmissionAdmin)
-admin.site.register(CoverLetter, SubmissionAdmin)
-
class PatchAdmin(admin.ModelAdmin):
list_display = ('name', 'submitter', 'project', 'state', 'date',
diff --git a/patchwork/api/cover.py b/patchwork/api/cover.py
index caf9a386efa5..8eb65f8cb942 100644
--- a/patchwork/api/cover.py
+++ b/patchwork/api/cover.py
@@ -15,7 +15,7 @@ from patchwork.api.filters import CoverLetterFilterSet
from patchwork.api.embedded import PersonSerializer
from patchwork.api.embedded import ProjectSerializer
from patchwork.api.embedded import SeriesSerializer
-from patchwork.models import CoverLetter
+from patchwork.models import Submission
class CoverLetterListSerializer(BaseHyperlinkedModelSerializer):
@@ -24,7 +24,7 @@ class CoverLetterListSerializer(BaseHyperlinkedModelSerializer):
project = ProjectSerializer(read_only=True)
submitter = PersonSerializer(read_only=True)
mbox = SerializerMethodField()
- series = SeriesSerializer(read_only=True)
+ series = SeriesSerializer(source='cl_series', read_only=True)
comments = SerializerMethodField()
def get_web_url(self, instance):
@@ -49,7 +49,7 @@ class CoverLetterListSerializer(BaseHyperlinkedModelSerializer):
return data
class Meta:
- model = CoverLetter
+ model = Submission
fields = ('id', 'url', 'web_url', 'project', 'msgid',
'list_archive_url', 'date', 'name', 'submitter', 'mbox',
'series', 'comments')
@@ -82,7 +82,7 @@ class CoverLetterDetailSerializer(CoverLetterListSerializer):
return headers
class Meta:
- model = CoverLetter
+ model = Submission
fields = CoverLetterListSerializer.Meta.fields + (
'headers', 'content')
read_only_fields = fields
@@ -100,8 +100,8 @@ class CoverLetterList(ListAPIView):
ordering = 'id'
def get_queryset(self):
- return CoverLetter.objects.all()\
- .select_related('project', 'submitter', 'series')\
+ return Submission.objects.filter(new_diff__isnull=True, new_pull_url__isnull=True)\
+ .select_related('project', 'submitter', 'cl_series')\
.defer('content', 'headers')
@@ -111,5 +111,5 @@ class CoverLetterDetail(RetrieveAPIView):
serializer_class = CoverLetterDetailSerializer
def get_queryset(self):
- return CoverLetter.objects.all()\
- .select_related('project', 'submitter', 'series')
+ return Submission.objects.filter(new_diff__isnull=True, new_pull_url__isnull=True)\
+ .select_related('project', 'submitter', 'cl_series')
diff --git a/patchwork/api/embedded.py b/patchwork/api/embedded.py
index de4f31165ee7..a0196128bad4 100644
--- a/patchwork/api/embedded.py
+++ b/patchwork/api/embedded.py
@@ -107,7 +107,7 @@ class CoverLetterSerializer(SerializedRelatedField):
class _Serializer(MboxMixin, WebURLMixin, BaseHyperlinkedModelSerializer):
class Meta:
- model = models.CoverLetter
+ model = models.Submission
fields = ('id', 'url', 'web_url', 'msgid', 'list_archive_url',
'date', 'name', 'mbox')
read_only_fields = fields
diff --git a/patchwork/api/filters.py b/patchwork/api/filters.py
index 37aca82d9ab2..4690fcab7479 100644
--- a/patchwork/api/filters.py
+++ b/patchwork/api/filters.py
@@ -15,13 +15,13 @@ from django.forms.widgets import MultipleHiddenInput
from patchwork.compat import NAME_FIELD
from patchwork.models import Bundle
from patchwork.models import Check
-from patchwork.models import CoverLetter
from patchwork.models import Event
from patchwork.models import Patch
from patchwork.models import Person
from patchwork.models import Project
from patchwork.models import Series
from patchwork.models import State
+from patchwork.models import Submission
# custom fields, filters
@@ -161,8 +161,13 @@ class CoverLetterFilterSet(TimestampMixin, FilterSet):
widget=MultipleHiddenInput)
submitter = PersonFilter(queryset=Person.objects.all())
+ @property
+ def qs(self):
+ parent = super(CoverLetterFilterSet, self).qs
+ return parent.filter(new_diff__isnull=True, new_pull_url__isnull=True)
+
class Meta:
- model = CoverLetter
+ model = Submission
fields = ('project', 'series', 'submitter')
@@ -191,6 +196,8 @@ class CheckFilterSet(TimestampMixin, FilterSet):
model = Check
fields = ('user', 'state', 'context')
+def get_coverletter_qs(request):
+ return Submission.objects.all().filter(new_diff__isnull=True, new_pull_url__isnull=True)
class EventFilterSet(TimestampMixin, FilterSet):
@@ -203,7 +210,7 @@ class EventFilterSet(TimestampMixin, FilterSet):
widget=MultipleHiddenInput)
patch = BaseFilter(queryset=Patch.objects.all(),
widget=MultipleHiddenInput)
- cover = BaseFilter(queryset=CoverLetter.objects.all(),
+ cover = BaseFilter(queryset=get_coverletter_qs,
widget=MultipleHiddenInput)
class Meta:
diff --git a/patchwork/management/commands/parsearchive.py b/patchwork/management/commands/parsearchive.py
index b7f1ea7313c2..39c640ccee8b 100644
--- a/patchwork/management/commands/parsearchive.py
+++ b/patchwork/management/commands/parsearchive.py
@@ -32,7 +32,7 @@ class Command(BaseCommand):
def handle(self, *args, **options):
results = {
models.Patch: 0,
- models.CoverLetter: 0,
+ models.Submission: 0,
models.Comment: 0,
}
duplicates = 0
@@ -118,7 +118,7 @@ class Command(BaseCommand):
' %(errors)4d errors\n'
'Total: %(new)s new entries' % {
'total': count,
- 'covers': results[models.CoverLetter],
+ 'covers': results[models.Submission],
'patches': results[models.Patch],
'comments': results[models.Comment],
'duplicates': duplicates,
diff --git a/patchwork/migrations/0041_drop_coverletter.py b/patchwork/migrations/0041_drop_coverletter.py
new file mode 100644
index 000000000000..d504676fd7fc
--- /dev/null
+++ b/patchwork/migrations/0041_drop_coverletter.py
@@ -0,0 +1,29 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.24 on 2019-09-17 17:56
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('patchwork', '0040_drop_old_diff_pull_url'),
+ ]
+
+ operations = [
+ migrations.AlterField(
+ model_name='event',
+ name='cover',
+ field=models.ForeignKey(blank=True, help_text=b'The cover letter that this event was created for.', null=True, on_delete=django.db.models.deletion.CASCADE, related_name='+', to='patchwork.Submission'),
+ ),
+ migrations.AlterField(
+ model_name='series',
+ name='cover_letter',
+ field=models.OneToOneField(null=True, on_delete=django.db.models.deletion.CASCADE, related_name='cl_series', to='patchwork.Submission'),
+ ),
+ migrations.DeleteModel(
+ name='CoverLetter',
+ ),
+ ]
diff --git a/patchwork/models.py b/patchwork/models.py
index a96018004f75..4b6466614fd3 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -389,6 +389,12 @@ class Submission(FilenameMixin, EmailMixin, models.Model):
def __str__(self):
return self.name
+ def get_absolute_url(self):
+ return reverse('cover-detail', kwargs={'cover_id': self.id})
+
+ def get_mbox_url(self):
+ return reverse('cover-mbox', kwargs={'cover_id': self.id})
+
class Meta:
ordering = ['date']
unique_together = [('msgid', 'project')]
@@ -402,15 +408,6 @@ class Submission(FilenameMixin, EmailMixin, models.Model):
]
-class CoverLetter(Submission):
-
- def get_absolute_url(self):
- return reverse('cover-detail', kwargs={'cover_id': self.id})
-
- def get_mbox_url(self):
- return reverse('cover-mbox', kwargs={'cover_id': self.id})
-
-
@python_2_unicode_compatible
class Patch(Submission):
# patch metadata
@@ -739,7 +736,7 @@ class Series(FilenameMixin, models.Model):
blank=True, on_delete=models.CASCADE)
# content
- cover_letter = models.OneToOneField(CoverLetter, related_name='series',
+ cover_letter = models.OneToOneField(Submission, related_name='cl_series',
null=True,
on_delete=models.CASCADE)
@@ -1030,7 +1027,7 @@ class Event(models.Model):
on_delete=models.CASCADE,
help_text='The series that this event was created for.')
cover = models.ForeignKey(
- CoverLetter, related_name='+', null=True, blank=True,
+ Submission, related_name='+', null=True, blank=True,
on_delete=models.CASCADE,
help_text='The cover letter that this event was created for.')
diff --git a/patchwork/parser.py b/patchwork/parser.py
index 7dc66bc05a5b..a122839f3f73 100644
--- a/patchwork/parser.py
+++ b/patchwork/parser.py
@@ -19,7 +19,6 @@ from django.db.utils import IntegrityError
from django.utils import six
from patchwork.models import Comment
-from patchwork.models import CoverLetter
from patchwork.models import DelegationRule
from patchwork.models import get_default_initial_patch_state
from patchwork.models import Patch
@@ -1088,11 +1087,11 @@ def parse_mail(mail, list_id=None):
if not is_comment:
if not refs == []:
try:
- CoverLetter.objects.all().get(name=name)
- except CoverLetter.DoesNotExist:
+ Submission.objects.get(name=name, new_diff__isnull=True, new_pull_url__isnull=True)
+ except Submission.DoesNotExist:
# if no match, this is a new cover letter
is_cover_letter = True
- except CoverLetter.MultipleObjectsReturned:
+ except Submission.MultipleObjectsReturned:
# if multiple cover letters are found, just ignore
pass
else:
@@ -1135,7 +1134,7 @@ def parse_mail(mail, list_id=None):
" in project %s!" % (msgid, project.name))
try:
- cover_letter = CoverLetter.objects.create(
+ cover_letter = Submission.objects.create(
msgid=msgid,
project=project,
name=name[:255],
diff --git a/patchwork/signals.py b/patchwork/signals.py
index 666199b68161..bbfdc0106fd7 100644
--- a/patchwork/signals.py
+++ b/patchwork/signals.py
@@ -10,11 +10,11 @@ from django.db.models.signals import pre_save
from django.dispatch import receiver
from patchwork.models import Check
-from patchwork.models import CoverLetter
from patchwork.models import Event
from patchwork.models import Patch
from patchwork.models import PatchChangeNotification
from patchwork.models import Series
+from patchwork.models import Submission
@receiver(pre_save, sender=Patch)
@@ -54,7 +54,7 @@ def patch_change_callback(sender, instance, raw, **kwargs):
notification.save()
- at receiver(post_save, sender=CoverLetter)
+ at receiver(post_save, sender=Submission)
def create_cover_created_event(sender, instance, created, raw, **kwargs):
def create_event(cover):
diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html
index e79dd92497a4..c4175a30e9c5 100644
--- a/patchwork/templates/patchwork/submission.html
+++ b/patchwork/templates/patchwork/submission.html
@@ -68,12 +68,12 @@ function toggle_div(link_id, headers_id)
</div>
</td>
</tr>
-{% if submission.series %}
+{% if series %}
<tr>
<th>Series</th>
<td>
- <a href="{% url 'patch-list' project_id=project.linkname %}?series={{ submission.series.id }}">
- {{ submission.series }}
+ <a href="{% url 'patch-list' project_id=project.linkname %}?series={{ series.id }}">
+ {{ series }}
</a>
</td>
</tr>
@@ -85,7 +85,7 @@ function toggle_div(link_id, headers_id)
>show</a>
<div id="patchrelations" class="patchrelations" style="display:none;">
<ul>
- {% with submission.series.cover_letter as cover %}
+ {% with series.cover_letter as cover %}
<li>
{% if cover %}
{% if cover == submission %}
@@ -98,7 +98,7 @@ function toggle_div(link_id, headers_id)
{% endif %}
</li>
{% endwith %}
- {% for sibling in submission.series.patches.all %}
+ {% for sibling in series.patches.all %}
<li>
{% if sibling == submission %}
{{ sibling.name|default:"[no subject]"|truncatechars:100 }}
diff --git a/patchwork/tests/api/test_cover.py b/patchwork/tests/api/test_cover.py
index 0a0bf041abc5..faaeb9f30093 100644
--- a/patchwork/tests/api/test_cover.py
+++ b/patchwork/tests/api/test_cover.py
@@ -44,10 +44,9 @@ class TestCoverLetterAPI(utils.APITestCase):
self.assertEqual(cover_obj.submitter.id,
cover_json['submitter']['id'])
-
- if hasattr(cover_obj, 'series'):
+ if hasattr(cover_obj, 'cl_series'):
self.assertEqual(1, len(cover_json['series']))
- self.assertEqual(cover_obj.series.id,
+ self.assertEqual(cover_obj.cl_series.id,
cover_json['series'][0]['id'])
else:
self.assertEqual([], cover_json['series'])
diff --git a/patchwork/tests/test_series.py b/patchwork/tests/test_series.py
index deb93043d295..69798acf9624 100644
--- a/patchwork/tests/test_series.py
+++ b/patchwork/tests/test_series.py
@@ -36,7 +36,7 @@ class _BaseTestCase(TestCase):
mbox = mailbox.mbox(os.path.join(TEST_SERIES_DIR, name), create=False)
for msg in mbox:
obj = parser.parse_mail(msg, project.listid)
- if type(obj) == models.CoverLetter:
+ if type(obj) == models.Submission:
results[0].append(obj)
elif type(obj) == models.Patch:
results[1].append(obj)
@@ -75,14 +75,14 @@ class _BaseTestCase(TestCase):
patches_ = patches[start_idx:end_idx]
for patch in patches_:
- self.assertEqual(patch.series, series[idx])
-
# TODO(stephenfin): Rework this function into two different
# functions - we're clearly not always testing patches here
if isinstance(patch, models.Patch):
+ self.assertEqual(patch.series, series[idx])
self.assertEqual(series[idx].patches.get(id=patch.id),
patch)
else:
+ self.assertEqual(patch.cl_series, series[idx])
self.assertEqual(series[idx].cover_letter, patch)
start_idx = end_idx
@@ -660,13 +660,13 @@ class SeriesNameTestCase(TestCase):
cover = self._parse_mail(mbox[0])
cover_name = 'A sample series'
- self.assertEqual(cover.series.name, cover_name)
+ self.assertEqual(cover.cl_series.name, cover_name)
self._parse_mail(mbox[1])
- self.assertEqual(cover.series.name, cover_name)
+ self.assertEqual(cover.cl_series.name, cover_name)
self._parse_mail(mbox[2])
- self.assertEqual(cover.series.name, cover_name)
+ self.assertEqual(cover.cl_series.name, cover_name)
mbox.close()
@@ -714,7 +714,7 @@ class SeriesNameTestCase(TestCase):
self.assertEqual(patch.series.name, patch.name)
cover = self._parse_mail(mbox[2])
- self.assertEqual(cover.series.name, 'A sample series')
+ self.assertEqual(cover.cl_series.name, 'A sample series')
mbox.close()
diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py
index 4ac9afe0de01..3934f6674f78 100644
--- a/patchwork/tests/utils.py
+++ b/patchwork/tests/utils.py
@@ -14,13 +14,13 @@ from django.contrib.auth.models import User
from patchwork.models import Bundle
from patchwork.models import Check
from patchwork.models import Comment
-from patchwork.models import CoverLetter
from patchwork.models import Patch
from patchwork.models import Person
from patchwork.models import Project
from patchwork.models import Series
from patchwork.models import SeriesReference
from patchwork.models import State
+from patchwork.models import Submission
from patchwork.tests import TEST_PATCH_DIR
SAMPLE_DIFF = """--- /dev/null 2011-01-01 00:00:00.000000000 +0800
@@ -202,7 +202,7 @@ def create_patch(**kwargs):
def create_cover(**kwargs):
"""Create 'CoverLetter' object."""
- num = CoverLetter.objects.count()
+ num = Submission.objects.count() - Patch.objects.count()
# NOTE(stephenfin): Despite first appearances, passing 'series' to the
# 'create' function doesn't actually cause the relationship to be created.
@@ -230,7 +230,7 @@ def create_cover(**kwargs):
}
values.update(kwargs)
- cover = CoverLetter.objects.create(**values)
+ cover = Submission.objects.create(**values)
if series:
series.add_cover_letter(cover)
diff --git a/patchwork/views/cover.py b/patchwork/views/cover.py
index 08b633a11f78..5ff1023f1a7e 100644
--- a/patchwork/views/cover.py
+++ b/patchwork/views/cover.py
@@ -10,7 +10,7 @@ from django.shortcuts import get_object_or_404
from django.shortcuts import render
from django.urls import reverse
-from patchwork.models import CoverLetter
+from patchwork.models import Patch
from patchwork.models import Submission
from patchwork.views.utils import cover_to_mbox
@@ -18,13 +18,14 @@ from patchwork.views.utils import cover_to_mbox
def cover_detail(request, cover_id):
# redirect to patches where necessary
try:
- cover = get_object_or_404(CoverLetter, id=cover_id)
- except Http404 as exc:
- submissions = Submission.objects.filter(id=cover_id)
- if submissions:
+ patch = get_object_or_404(Patch, id=cover_id)
+ if patch:
return HttpResponseRedirect(
reverse('patch-detail', kwargs={'patch_id': cover_id}))
- raise exc
+ except Http404:
+ pass
+
+ cover = get_object_or_404(Submission, id=cover_id)
context = {
'submission': cover,
@@ -36,12 +37,22 @@ def cover_detail(request, cover_id):
comments = comments.only('submitter', 'date', 'id', 'content',
'submission')
context['comments'] = comments
+ context['series'] = cover.cl_series
return render(request, 'patchwork/submission.html', context)
def cover_mbox(request, cover_id):
- cover = get_object_or_404(CoverLetter, id=cover_id)
+ # check if we should redirect to patch
+ try:
+ patch = get_object_or_404(Patch, id=cover_id)
+ if patch:
+ return HttpResponseRedirect(
+ reverse('patch-mbox', kwargs={'patch_id': cover_id}))
+ except Http404:
+ pass
+
+ cover = get_object_or_404(Submission, id=cover_id)
response = HttpResponse(content_type='text/plain')
response.write(cover_to_mbox(cover))
diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py
index 277b2816e31e..6c5125ee5681 100644
--- a/patchwork/views/patch.py
+++ b/patchwork/views/patch.py
@@ -110,6 +110,7 @@ def patch_detail(request, patch_id):
context['patchform'] = form
context['createbundleform'] = createbundleform
context['project'] = patch.project
+ context['series'] = patch.series
return render(request, 'patchwork/submission.html', context)
--
2.20.1
More information about the Patchwork
mailing list