> 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.lesp...@intel.com>
> Acked-by: Stephen Finucane <stephen.finuc...@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@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
_______________________________________________
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Reply via email to