diff mbox

[02/10] models: Add 'Series' model and related models

Message ID 1465814502-14108-3-git-send-email-stephen.finucane@intel.com
State Superseded
Headers show

Commit Message

Stephen Finucane June 13, 2016, 10:41 a.m. UTC
Add a series model. This model is intentionally very minimal to allow
as much dynaminism as possible. It is expected that patches will be
migrated between series as new data is provided.

Signed-off-by: Stephen Finucane <stephen.finucane@intel.com>
---
v2:
- Rename 'SeriesGroup' to 'Series'
- Rename 'Series' to 'SeriesRevision'
---
 patchwork/admin.py                             |   55 +++++++++++-
 patchwork/migrations/0013_add_series_models.py |   56 ++++++++++++
 patchwork/models.py                            |  110 +++++++++++++++++++++++-
 3 files changed, 216 insertions(+), 5 deletions(-)
 create mode 100644 patchwork/migrations/0013_add_series_models.py

Comments

Andy Doan June 23, 2016, 6:22 p.m. UTC | #1
On 06/13/2016 05:41 AM, Stephen Finucane wrote:
> Add a series model. This model is intentionally very minimal to allow
> as much dynaminism as possible. It is expected that patches will be
> migrated between series as new data is provided.
>
> Signed-off-by: Stephen Finucane <stephen.finucane@intel.com>
> ---
> v2:
> - Rename 'SeriesGroup' to 'Series'
> - Rename 'Series' to 'SeriesRevision'
> ---

It was a little subtle, but it made this easier to mentally picture for me.

Review-by: Andy Doan <andy.doan@linaro.org>
Russell Currey July 18, 2016, 3:31 a.m. UTC | #2
On Mon, 2016-06-13 at 11:41 +0100, Stephen Finucane wrote:
> Add a series model. This model is intentionally very minimal to allow
> as much dynaminism as possible. It is expected that patches will be
> migrated between series as new data is provided.
> 
> Signed-off-by: Stephen Finucane <stephen.finucane@intel.com>
> ---

<snip>

> diff --git a/patchwork/models.py b/patchwork/models.py
> index 9ad2bcb..67ea012 100644
> --- a/patchwork/models.py
> +++ b/patchwork/models.py
> @@ -37,6 +37,9 @@ from django.utils.six.moves import filter
>  from patchwork.fields import HashField
>  from patchwork.parser import extract_tags, hash_patch
>  
> +SERIES_REVISION_DEFAULT_NAME = 'Unnamed Series Revision'
> +SERIES_DEFAULT_NAME = 'Unnamed Series'
> +
>  

Having multiple series revisions without a cover letter appear the same is a
problem.  If there are multiple series next to each other in the patch view,
it's visually difficult to tell them apart, even though they link to different
places. Perhaps series without a cover letter could instead be named after the
first patch in the series, prefixed with "Series starting with" or "[Missing
cover letter]", or something?
Russell Currey July 19, 2016, 12:50 a.m. UTC | #3
On Mon, 2016-06-13 at 11:41 +0100, Stephen Finucane wrote:
> Add a series model. This model is intentionally very minimal to allow
> as much dynaminism as possible. It is expected that patches will be
> migrated between series as new data is provided.
> 
> Signed-off-by: Stephen Finucane <stephen.finucane@intel.com>
> ---

I'm pretty sure this broke the patches API, though I'm not exactly a Django
expert.  Here's what I'm seeing:

Exception Type: ImproperlyConfigured at /api/1.0/patches/
Exception Value: Could not resolve URL for hyperlinked relationship using view
name "seriesrevision-detail". You may have failed to include the related model
in your API, or incorrectly configured the `lookup_field` attribute on this
field.

Full log here: http://dpaste.com/0P3C63K
diff mbox

Patch

diff --git a/patchwork/admin.py b/patchwork/admin.py
index 0f217d7..afb4af6 100644
--- a/patchwork/admin.py
+++ b/patchwork/admin.py
@@ -23,7 +23,8 @@  from django.contrib import admin
 
 from patchwork.models import (Project, Person, UserProfile, State, Submission,
                               Patch, CoverLetter, Comment, Bundle, Tag, Check,
-                              DelegationRule)
+                              DelegationRule, Series, SeriesRevision,
+                              SeriesReference)
 
 
 class DelegationRuleInline(admin.TabularInline):
@@ -76,8 +77,8 @@  admin.site.register(CoverLetter, CoverLetterAdmin)
 
 class PatchAdmin(admin.ModelAdmin):
     list_display = ('name', 'submitter', 'project', 'state', 'date',
-                    'archived', 'is_pull_request')
-    list_filter = ('project', 'state', 'archived')
+                    'archived', 'is_pull_request', 'series')
+    list_filter = ('project', 'state', 'archived', 'series')
     search_fields = ('name', 'submitter__name', 'submitter__email')
     date_hierarchy = 'date'
 
@@ -97,6 +98,54 @@  class CommentAdmin(admin.ModelAdmin):
 admin.site.register(Comment, CommentAdmin)
 
 
+class CoverLetterInline(admin.StackedInline):
+    model = CoverLetter
+    extra = 0
+
+
+class PatchInline(admin.StackedInline):
+    model = Patch
+    extra = 0
+
+
+class SeriesRevisionAdmin(admin.ModelAdmin):
+    list_display = ('name', 'group', 'date', 'submitter', 'version', 'total',
+                    'actual_total', 'complete')
+    readonly_fields = ('actual_total', 'complete')
+    search_fields = ('submitter_name', 'submitter_email')
+    inlines = [CoverLetterInline, PatchInline]
+
+    def complete(self, series):
+        return series.complete
+    complete.boolean = True
+admin.site.register(SeriesRevision, SeriesRevisionAdmin)
+
+
+class SeriesRevisionInline(admin.StackedInline):
+    model = SeriesRevision
+    readonly_fields = ('date', 'submitter', 'version', 'total',
+                       'actual_total', 'complete')
+    ordering = ('-date', )
+    show_change_link = True
+    extra = 0
+
+    def complete(self, series):
+        return series.complete
+    complete.boolean = True
+
+
+class SeriesAdmin(admin.ModelAdmin):
+    list_display = ('name', )
+    readonly_fields = ('name', )
+    inlines = [SeriesRevisionInline]
+admin.site.register(Series, SeriesAdmin)
+
+
+class SeriesReferenceAdmin(admin.ModelAdmin):
+    model = SeriesReference
+admin.site.register(SeriesReference, SeriesReferenceAdmin)
+
+
 class CheckAdmin(admin.ModelAdmin):
     list_display = ('patch', 'user', 'state', 'target_url',
                     'description', 'context')
diff --git a/patchwork/migrations/0013_add_series_models.py b/patchwork/migrations/0013_add_series_models.py
new file mode 100644
index 0000000..03b7a22
--- /dev/null
+++ b/patchwork/migrations/0013_add_series_models.py
@@ -0,0 +1,56 @@ 
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('patchwork', '0012_add_coverletter_model'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='Series',
+            fields=[
+                ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
+            ],
+            options={
+                'verbose_name_plural': 'Series',
+            },
+        ),
+        migrations.CreateModel(
+            name='SeriesReference',
+            fields=[
+                ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
+                ('msgid', models.CharField(unique=True, max_length=255)),
+            ],
+        ),
+        migrations.CreateModel(
+            name='SeriesRevision',
+            fields=[
+                ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
+                ('date', models.DateTimeField()),
+                ('version', models.IntegerField(default=1, help_text=b'Version of series revision as indicated by the subject prefix(es)')),
+                ('total', models.IntegerField(help_text=b'Number of patches in series as indicated by the subject prefix(es)')),
+                ('group', models.ForeignKey(related_query_name=b'revision', related_name='revisions', blank=True, to='patchwork.Series', null=True)),
+                ('submitter', models.ForeignKey(to='patchwork.Person')),
+            ],
+        ),
+        migrations.AddField(
+            model_name='seriesreference',
+            name='series',
+            field=models.ForeignKey(related_query_name=b'reference', related_name='references', to='patchwork.SeriesRevision'),
+        ),
+        migrations.AddField(
+            model_name='coverletter',
+            name='series',
+            field=models.OneToOneField(related_name='cover_letter', null=True, blank=True, to='patchwork.SeriesRevision'),
+        ),
+        migrations.AddField(
+            model_name='patch',
+            name='series',
+            field=models.ForeignKey(related_query_name=b'patch', related_name='unique_patches', blank=True, to='patchwork.SeriesRevision', null=True),
+        ),
+    ]
diff --git a/patchwork/models.py b/patchwork/models.py
index 9ad2bcb..67ea012 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -37,6 +37,9 @@  from django.utils.six.moves import filter
 from patchwork.fields import HashField
 from patchwork.parser import extract_tags, hash_patch
 
+SERIES_REVISION_DEFAULT_NAME = 'Unnamed Series Revision'
+SERIES_DEFAULT_NAME = 'Unnamed Series'
+
 
 @python_2_unicode_compatible
 class Person(models.Model):
@@ -182,6 +185,100 @@  models.signals.post_save.connect(_user_saved_callback, sender=User)
 
 
 @python_2_unicode_compatible
+class Series(models.Model):
+    """A collection of series.
+
+    Provides a way to group series revisions such that finding a series
+    revisions won't require a large amount of computation each time.
+    The model itself should never be exposed to users - instead expose
+    the individual revisions.
+    """
+
+    @property
+    def name(self):
+        # TODO(stephenfin) If there is a non-empty revision name we
+        # should use that instead
+        if self.latest_revision:
+            return self.latest_revision.name
+        return SERIES_DEFAULT_NAME
+
+    @property
+    def latest_revision(self):
+        # TODO(stephenfin) Can this raise an exception?
+        return self.revisions.latest('date')
+
+    def __str__(self):
+        return self.name
+
+    class Meta:
+        verbose_name_plural = 'Series'
+
+
+@python_2_unicode_compatible
+class SeriesRevision(models.Model):
+
+    group = models.ForeignKey(Series, related_name='revisions',
+                              related_query_name='revision', null=True,
+                              blank=True)
+    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)')
+
+    @cached_property
+    def name(self):
+        try:
+            return self.cover_letter.name
+        except CoverLetter.DoesNotExist:
+            return SERIES_REVISION_DEFAULT_NAME
+
+    @property
+    def actual_total(self):
+        return Patch.objects.filter(series=self).count()
+
+    @property
+    def complete(self):
+        return self.total == self.actual_total
+
+    @property
+    def patches(self):
+        """Return patches associated with this series .
+
+        Eventually this will "autofill" a series revision by pulling in
+        missing patches from prior revisions, where possible. For now,
+        however, this just lets us retrieve the patches created for
+        this given revision.
+
+        Returns:
+            The patches in the revision.
+        """
+        return self.unique_patches.all()
+
+    def __str__(self):
+        return self.name
+
+
+@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')
+    msgid = models.CharField(max_length=255, unique=True)
+
+    def __str__(self):
+        return self.msgid
+
+
+@python_2_unicode_compatible
 class State(models.Model):
     name = models.CharField(max_length=100)
     ordering = models.IntegerField(unique=True)
@@ -295,7 +392,7 @@  class EmailMixin(models.Model):
 
 @python_2_unicode_compatible
 class Submission(EmailMixin, models.Model):
-    # parent
+    # parents
 
     project = models.ForeignKey(Project)
 
@@ -320,11 +417,20 @@  class Submission(EmailMixin, models.Model):
 
 
 class CoverLetter(Submission):
-    pass
+    # parents
+
+    series = models.OneToOneField(SeriesRevision, related_name='cover_letter',
+                                  null=True, blank=True)
 
 
 @python_2_unicode_compatible
 class Patch(Submission):
+    # parents
+
+    series = models.ForeignKey(SeriesRevision, related_name='unique_patches',
+                               related_query_name='patch',
+                               null=True, blank=True)
+
     # patch metadata
 
     diff = models.TextField(null=True, blank=True)