[PATCH 06/14] series: Add a Series model

[Finucane, Stephen]

> v2: Add 'order' metadata to revisions as they do have a natural
>     ordering.
> v3: Add a couple of utility functions to Series and SeriesRevision
> v4: Provide a get_absolute_url() method on Series
> v5: Add a dump() method to series, handy for debugging
> v6: Use 'Series without cover letter' as default series name instead
>     of 'Untitled series' to clearly indicated what's hapening to the
>     user (Belen Pena)
> v7: Squash in the migration (Stephen Finucane)
> v8: Add (untested) manual SQL migration paths (Stephen Finucane)
> 
> Signed-off-by: Damien Lespiau <damien.lespiau at intel.com>
> Acked-by: Stephen Finucane <stephen.finucane at intel.com>

I've spotted a few issues I missed first go around when I was attempting to merge this. Can you address them?

Stephen

PS: I stripped the migrations section as these may/may not be able to go away now.

> --- a/patchwork/models.py
> +++ b/patchwork/models.py
> @@ -410,6 +410,91 @@ class BundlePatch(models.Model):
>          unique_together = [('bundle', 'patch')]
>          ordering = ['order']
> 
> +SERIES_DEFAULT_NAME = "Series without cover letter"
> +
> +# This Model represents the "top level" Series, an object that doesn't
> change
> +# with the various versions of patches sent to the mailing list.
> +class Series(models.Model):
> +    project = models.ForeignKey(Project)
> +    name = models.CharField(max_length=200, default=SERIES_DEFAULT_NAME)

I really am not happy with this. I understand your rationale and I realize that this means people need to account for this in the interfaces (API + UI), but this is still better than storing lies, figuratively speaking. Similarly i18n might not be an issue now but I'd like to keep the doors open for that down the line. Can we please allow 'blank' and 'null' instead?

> +    submitter = models.ForeignKey(Person, related_name='submitters')
> +    reviewer = models.ForeignKey(User, related_name='reviewers',
> null=True,
> +                                 blank=True)
> +    submitted = models.DateTimeField(default=datetime.datetime.now)
> +    last_updated = models.DateTimeField(auto_now=True)
> +    # Caches the latest version so we can display it without looking at
> the max
> +    # of all SeriesRevision.version
> +    version = models.IntegerField(default=1)
> +    # This is the number of patches of the latest version.
> +    n_patches = models.IntegerField(default=0)
> +
> +    def __unicode__(self):
> +        return self.name
> +
> +    def revisions(self):
> +        return SeriesRevision.objects.filter(series=self)
> +
> +    def latest_revision(self):
> +        return self.revisions().reverse()[0]
> +
> +    def get_absolute_url(self):
> +        return reverse('series', kwargs={ 'series': self.pk })
> +
> +    def dump(self):
> +        print('')
> +        print('===')
> +        print('Series: %s' % self)
> +        print('    version %d' % self.version)
> +        for rev in self.revisions():
> +            print('    rev %d:' % rev.version)
> +            i = 1
> +            for patch in rev.ordered_patches():
> +                print('        patch %d:' % i)
> +                print('            subject: %s' % patch.name)
> +                print('            msgid  : %s' % patch.msgid)
> +                i += 1
> +
> +# A 'revision' of a series. Resending a new version of a patch or a full
> new
> +# iteration of a series will create a new revision.
> +class SeriesRevision(models.Model):
> +    series = models.ForeignKey(Series)
> +    version = models.IntegerField(default=1)
> +    root_msgid = models.CharField(max_length=255)
> +    cover_letter = models.TextField(null = True, blank = True)
> +    patches = models.ManyToManyField(Patch, through =
> 'SeriesRevisionPatch')

Is it possible for a patch to belong to more than one Series(Revision)?
 
To be consistent, can you make the following changes:

	CHANGE root_msgid -> msgid
	CHANGE cover_letter -> content
	ADD    submitter  (it's possible that a series revision could come from a different person, yes?)
	ADD    date       (if there's a cover letter, we should store the date IMO. Even if not, knowing the date/time a revision was created is helpful)
	ADD    headers    (if there's a cover letter, we should store these headers for consistency)

This would allow us to treat SeriesRevision, Patch and Comment as Email-type objects, which most of the time they are. If we do this, we can start pulling out common behaviors into mixins.

	https://github.com/stephenfin/patchwork/commit/4912b19790da69e5cddd081119e34f5a194a63c0

> +
> +    class Meta:
> +        unique_together = [('series', 'version')]
> +        ordering = ['version']
> +
> +    def ordered_patches(self):
> +        return self.patches.order_by('seriesrevisionpatch__order')
> +
> +    def add_patch(self, patch, order):
> +        # see if the patch is already in this revision
> +        if SeriesRevisionPatch.objects.filter(revision=self,
> +                                              patch=patch).count():
> +            raise Exception("patch is already in revision")
> +
> +        sp = SeriesRevisionPatch.objects.create(revision=self,
> patch=patch,
> +                                                order=order)
> +        sp.save()
> +
> +    def __unicode__(self):
> +        if hasattr(self, 'series'):
> +            return self.series.name + " (rev " + str(self.version) + ")"
> +        else:
> +            return "New revision" + " (rev " + str(self.version) + ")"
> +
> +class SeriesRevisionPatch(models.Model):
> +    patch = models.ForeignKey(Patch)
> +    revision = models.ForeignKey(SeriesRevision)
> +    order = models.IntegerField()
> +
> +    class Meta:
> +        unique_together = [('revision', 'patch'), ('revision', 'order')]
> +        ordering = ['order']
> +
>  class EmailConfirmation(models.Model):
>      validity = datetime.timedelta(days =
> settings.CONFIRMATION_VALIDITY_DAYS)
>      type = models.CharField(max_length = 20, choices = [
> --
> 2.4.3
> 
> _______________________________________________
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork