From: Stephen Finucane <stephen@that.guru>
Date: Wed, 5 Sep 2018 14:53:32 +0000 (+0100)
Subject: models: Remove 'latest_series'
X-Git-Tag: v2.2.0-rc1~290
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=84def8549f6fc624a523e4c45480305848f29cc9;p=thirdparty%2Fpatchwork.git

models: Remove 'latest_series'

This is only used in a single view (where it probably shouldn't be used)
and some tests. It's an anti-pattern that makes it too easy to shoot
yourself in the foot. Remove it.

Signed-off-by: Stephen Finucane <stephen@that.guru>
---

diff --git a/patchwork/models.py b/patchwork/models.py
index d2d8f343..8731bb2f 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -393,26 +393,7 @@ class Submission(FilenameMixin, EmailMixin, models.Model):
         ]
 
 
-class SeriesMixin(object):
-
-    @property
-    def latest_series(self):
-        """Get the latest series this is a member of.
-
-        Return the last series that (ordered by date) that this
-        submission is a member of.
-
-        .. warning::
-          Be judicious in your use of this. For example, do not use it
-          in list templates as doing so will result in a new query for
-          each item in the list.
-        """
-        # NOTE(stephenfin): We don't use 'latest()' here, as this can raise an
-        # exception if no series exist
-        return self.series.order_by('-date').first()
-
-
-class CoverLetter(SeriesMixin, Submission):
+class CoverLetter(Submission):
 
     def get_absolute_url(self):
         return reverse('cover-detail', kwargs={'cover_id': self.id})
@@ -422,7 +403,7 @@ class CoverLetter(SeriesMixin, Submission):
 
 
 @python_2_unicode_compatible
-class Patch(SeriesMixin, Submission):
+class Patch(Submission):
     # patch metadata
 
     diff = models.TextField(null=True, blank=True)
diff --git a/patchwork/templates/patchwork/download_buttons.html b/patchwork/templates/patchwork/download_buttons.html
index 4809db54..32acf26b 100644
--- a/patchwork/templates/patchwork/download_buttons.html
+++ b/patchwork/templates/patchwork/download_buttons.html
@@ -15,17 +15,19 @@
    class="btn btn-default" role="button" title="Download cover mbox"
    >mbox</a>
   {% endif %}
-  {% if submission.series.all|length == 1 %}
-  <a href="{% url 'series-mbox' series_id=submission.latest_series.id %}"
+  {% if all_series|length == 1 %}
+  {% with all_series|first as series %}
+  <a href="{% url 'series-mbox' series_id=series.id %}"
    class="btn btn-default" role="button"
    title="Download patch mbox with dependencies">series</a>
-  {% elif submission.series.all|length > 1 %}
+  {% endwith %}
+  {% elif all_series|length > 1 %}
   <button type="button" class="btn btn-default dropdown-toggle"
    data-toggle="dropdown">
    series <span class="caret"></span>
   </button>
   <ul class="dropdown-menu" role="menu">
-  {% for series in submission.series.all %}
+  {% for series in all_series %}
    <li><a href="{% url 'series-mbox' series_id=series.id %}"
     >{{ series }}</a></li>
   {% endfor %}
diff --git a/patchwork/tests/test_series.py b/patchwork/tests/test_series.py
index 9b5c0129..6892a615 100644
--- a/patchwork/tests/test_series.py
+++ b/patchwork/tests/test_series.py
@@ -87,7 +87,7 @@ class _BaseTestCase(TestCase):
 
             patches_ = patches[start_idx:end_idx]
             for patch in patches_:
-                self.assertEqual(patch.latest_series, series[idx])
+                self.assertEqual(patch.series.first(), series[idx])
 
             start_idx = end_idx
 
@@ -531,7 +531,7 @@ class SeriesTotalTest(_BaseTestCase):
         self.assertSerialized(patches, [1])
         self.assertSerialized(covers, [1])
 
-        series = patches[0].latest_series
+        series = patches[0].series.first()
         self.assertFalse(series.received_all)
 
     def test_complete(self):
@@ -551,7 +551,7 @@ class SeriesTotalTest(_BaseTestCase):
         self.assertSerialized(covers, [1])
         self.assertSerialized(patches, [2])
 
-        series = patches[0].latest_series
+        series = patches[0].series.first()
         self.assertTrue(series.received_all)
 
     def test_extra_patches(self):
@@ -572,7 +572,7 @@ class SeriesTotalTest(_BaseTestCase):
         self.assertSerialized(covers, [1])
         self.assertSerialized(patches, [3])
 
-        series = patches[0].latest_series
+        series = patches[0].series.first()
         self.assertTrue(series.received_all)
 
 
@@ -657,13 +657,13 @@ class SeriesNameTestCase(TestCase):
 
         cover = self._parse_mail(mbox[0])
         cover_name = self._format_name(cover)
-        self.assertEqual(cover.latest_series.name, cover_name)
+        self.assertEqual(cover.series.first().name, cover_name)
 
         self._parse_mail(mbox[1])
-        self.assertEqual(cover.latest_series.name, cover_name)
+        self.assertEqual(cover.series.first().name, cover_name)
 
         self._parse_mail(mbox[2])
-        self.assertEqual(cover.latest_series.name, cover_name)
+        self.assertEqual(cover.series.first().name, cover_name)
 
         mbox.close()
 
@@ -681,7 +681,7 @@ class SeriesNameTestCase(TestCase):
         mbox = self._get_mbox('base-no-cover-letter.mbox')
 
         patch = self._parse_mail(mbox[0])
-        series = patch.latest_series
+        series = patch.series.first()
         self.assertEqual(series.name, patch.name)
 
         self._parse_mail(mbox[1])
@@ -705,13 +705,13 @@ class SeriesNameTestCase(TestCase):
         mbox = self._get_mbox('base-out-of-order.mbox')
 
         patch = self._parse_mail(mbox[0])
-        self.assertIsNone(patch.latest_series.name)
+        self.assertIsNone(patch.series.first().name)
 
         patch = self._parse_mail(mbox[1])
-        self.assertEqual(patch.latest_series.name, patch.name)
+        self.assertEqual(patch.series.first().name, patch.name)
 
         cover = self._parse_mail(mbox[2])
-        self.assertEqual(cover.latest_series.name, self._format_name(cover))
+        self.assertEqual(cover.series.first().name, self._format_name(cover))
 
         mbox.close()
 
@@ -730,7 +730,7 @@ class SeriesNameTestCase(TestCase):
         """
         mbox = self._get_mbox('base-out-of-order.mbox')
 
-        series = self._parse_mail(mbox[0]).latest_series
+        series = self._parse_mail(mbox[0]).series.first()
         self.assertIsNone(series.name)
 
         series_name = 'My custom series name'
diff --git a/patchwork/views/utils.py b/patchwork/views/utils.py
index 2357ab86..3bf7af3c 100644
--- a/patchwork/views/utils.py
+++ b/patchwork/views/utils.py
@@ -143,7 +143,7 @@ def series_patch_to_mbox(patch, series_id):
         A string for the mbox file.
     """
     if series_id == '*':
-        series = patch.latest_series
+        series = patch.series.order_by('-date').first()
     else:
         try:
             series_id = int(series_id)