On 16/10/16 23:50, Stephen Finucane wrote:
Add a series revision model. This model is expected to act like a
collection for patches, similar to bundles but thread-orientated.
Signed-off-by: Stephen Finucane <step...@that.guru>
Looking pretty good! A few minor things below. I mostly won't repeat
stuff that Daniel pointed out.
Otherwise, with Daniel's comments fixed:
Reviewed-by: Andrew Donnellan <andrew.donnel...@au1.ibm.com>
diff --git a/patchwork/models.py b/patchwork/models.py
index d0ef44d..49572ec 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -292,7 +292,7 @@ class EmailMixin(models.Model):
@python_2_unicode_compatible
class Submission(EmailMixin, models.Model):
- # parent
+ # parents
What's this for?
project = models.ForeignKey(Project)
@@ -317,11 +317,27 @@ class Submission(EmailMixin, models.Model):
class CoverLetter(Submission):
- pass
+
+ @property
+ def series(self):
+ """Get a simple series reference.
+
+ Return the last series revision 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.
+ """
I vote for keeping the warning, personally - it looks like the kind of
thing which someone somewhere down the track might stuff up rather easily.
+ # NOTE(stephenfin): We don't use 'latest()' here, as this can raise an
+ # exception if no series revisions exist
+ return self.series_revisions.order_by('-date').first()
@python_2_unicode_compatible
class Patch(Submission):
+
Whitespace code churn
def is_editable(self, user):
if not user.is_authenticated():
return False
@@ -439,6 +444,22 @@ class Patch(Submission):
return self.project.is_editable(user)
@property
+ def series(self):
+ """Get a simple series reference.
+
+ Return the last series revision 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 revisions exist
+ return self.series_revisions.order_by('-date').first()
+
When versioning gets added later on, are you planning on calling the
series group model "Series" as per v2? (FWIW after thinking about it for
a bit I think I like the v1 naming better than the v2 naming, but I
digress...)
If so, calling this method "series()" and having it return a
SeriesRevision is a bit confusing.
class Meta:
verbose_name_plural = 'Patches'
@@ -568,6 +600,134 @@ class Comment(EmailMixin, models.Model):
unique_together = [('msgid', 'submission')]
+@python_2_unicode_compatible
+class SeriesRevision(models.Model):
+ """An individual revision of a series."""
+
+ # content
+ cover_letter = models.ForeignKey(CoverLetter,
+ related_name='series_revisions',
+ null=True, blank=True)
+ patches = models.ManyToManyField(Patch, through='SeriesRevisionPatch',
+ related_name='series_revisions',
+ related_query_name='series_revision')
+
+ # metadata
+ name = models.CharField(max_length=255, blank=True, null=True,
+ help_text='An optional name to associate with '
+ 'the series, e.g. "John\'s PCI series".')
+ date = models.DateTimeField()
+ submitter = models.ForeignKey(Person)
+ version = models.IntegerField(default=1,
+ help_text='Version of series revision as '
+ 'indicated by the subject prefix(es)')
+ total = models.IntegerField(help_text='Number of patches in series as '
+ 'indicated by the subject prefix(es)')
+
+ @property
+ def actual_total(self):
+ return self.patches.count()
+
+ @property
+ def complete(self):
+ return self.total == self.actual_total
+
+ def add_cover_letter(self, cover):
+ """Add a cover letter to the series revision.
+
+ Helper method so we can use the same pattern to add both
+ patches and cover letters.
+ """
+
+ def _format_name(obj):
+ return obj.name.split(']')[-1]
+
+ if self.cover_letter:
+ # TODO(stephenfin): We may wish to raise an exception here in the
+ # future
+ return
+
+ self.cover_letter = cover
+
+ # don't override user-defined names
+ if not self.name:
+ self.name = _format_name(cover)
+ # ...but give cover letter-based names precedence over patch-based
+ # names
+ else:
+ try:
+ name = SeriesRevisionPatch.objects.get(revision=self,
+ number=1).patch.name
+ except SeriesRevisionPatch.DoesNotExist:
+ name = None
+
+ if self.name == name:
+ self.name = _format_name(cover)
+
+ self.save()
+
+ def add_patch(self, patch, number):
+ """Add a patch to the series revision."""
+ # see if the patch is already in this series
+ if SeriesRevisionPatch.objects.filter(revision=self,
+ patch=patch).count():
+ # TODO(stephenfin): We may wish to raise an exception here in the
+ # future
+ return
+
+ # both user defined names and cover letter-based names take precedence
+ if not self.name and number == 1:
+ self.name = patch.name # keep the prefixes for patch-based names
+ self.save()
+
+ return SeriesRevisionPatch.objects.create(patch=patch,
+ revision=self,
+ number=number)
+
+ def __str__(self):
+ return self.name if self.name else 'Untitled series #%d' % self.id
Just to add to Daniel's working-through of the precedence of names...
"Untitled series X" should only be printed if there is no user defined
name, no cover letter received yet, and the only patch that has been
received is not numbered patch 1?
+@python_2_unicode_compatible
+class SeriesReference(models.Model):
+ """A reference found in a series.
+
+ Message IDs should be created for all patches in a series,
+ including those of patches that have not yet been received. This is
+ required to handle the case whereby one or more patches are
+ received before the cover letter.
+ """
+ series = models.ForeignKey(SeriesRevision, related_name='references',
+ related_query_name='reference')
See earlier comment about "series" and SeriesRevision.
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnel...@au1.ibm.com IBM Australia Limited
_______________________________________________
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
