[PATCH 3/4] models, templates: Add patch relations

Daniel Axtens dja at axtens.net
Tue Feb 11 15:07:49 AEDT 2020


Daniel Axtens <dja at axtens.net> writes:

> From: Mete Polat <metepolat2000 at gmail.com>
>
> Introduces the ability to add relations between patches. Relations are
> displayed in the details page of a patch under 'Related'. Related
> patches located in another projects can be viewed as well.
>
> Changes to relations are tracked in events. Currently the display of
> this is very bare in the API but that will be fixed in a subsequent patch:
> this is the minimum required to avoid throwing errors when you view the
> events feed.
>
> Signed-off-by: Mete Polat <metepolat2000 at gmail.com>
> [dja: address some review comments from Stephen, add an admin view,
>       move to using Events, misc tidy-ups.]
> Signed-off-by: Daniel Axtens <dja at axtens.net>
>
> ---
>
> Todos:
>  - don't display these events for API < v1.2. Mete I think your suggestion
>    here will work well - thanks. d-r-f is really not my thing so I do miss
>    obvious stuff from time to time!
>  - events, what should happen if a relation goes away
>  - pep8 and general nitpickery
>
> ---
>  patchwork/admin.py                            |  8 ++++
>  patchwork/api/event.py                        |  5 ++-
>  .../migrations/0040_add_related_patches.py    | 42 +++++++++++++++++++
>  patchwork/models.py                           | 32 +++++++++++++-
>  patchwork/signals.py                          | 24 +++++++++++
>  patchwork/templates/patchwork/submission.html | 37 ++++++++++++++++
>  patchwork/views/patch.py                      | 13 ++++++
>  7 files changed, 159 insertions(+), 2 deletions(-)
>  create mode 100644 patchwork/migrations/0040_add_related_patches.py
>
> diff --git a/patchwork/admin.py b/patchwork/admin.py
> index f9a94c6f5c07..c3d45240f1eb 100644
> --- a/patchwork/admin.py
> +++ b/patchwork/admin.py
> @@ -14,6 +14,7 @@ from patchwork.models import Comment
>  from patchwork.models import CoverLetter
>  from patchwork.models import DelegationRule
>  from patchwork.models import Patch
> +from patchwork.models import PatchRelation
>  from patchwork.models import Person
>  from patchwork.models import Project
>  from patchwork.models import Series
> @@ -174,3 +175,10 @@ class TagAdmin(admin.ModelAdmin):
>  
>  
>  admin.site.register(Tag, TagAdmin)
> +
> +
> +class PatchRelationAdmin(admin.ModelAdmin):
> +    model = PatchRelation
> +
> +
> +admin.site.register(PatchRelation, PatchRelationAdmin)
> diff --git a/patchwork/api/event.py b/patchwork/api/event.py
> index a066faaec63b..d7685f4c138c 100644
> --- a/patchwork/api/event.py
> +++ b/patchwork/api/event.py
> @@ -42,6 +42,8 @@ class EventSerializer(ModelSerializer):
>                                               'current_state'],
>          Event.CATEGORY_PATCH_DELEGATED: ['patch', 'previous_delegate',
>                                           'current_delegate'],
> +        Event.CATEGORY_PATCH_RELATION_CHANGED: ['patch', 'previous_relation',
> +                                                'current_relation'],
>          Event.CATEGORY_CHECK_CREATED: ['patch', 'created_check'],
>          Event.CATEGORY_SERIES_CREATED: ['series'],
>          Event.CATEGORY_SERIES_COMPLETED: ['series'],
> @@ -68,7 +70,8 @@ class EventSerializer(ModelSerializer):
>          model = Event
>          fields = ('id', 'category', 'project', 'date', 'actor', 'patch',
>                    'series', 'cover', 'previous_state', 'current_state',
> -                  'previous_delegate', 'current_delegate', 'created_check')
> +                  'previous_delegate', 'current_delegate', 'created_check',
> +                  'previous_relation', 'current_relation',)
>          read_only_fields = fields
>          versioned_fields = {
>              '1.2': ('actor', ),
> diff --git a/patchwork/migrations/0040_add_related_patches.py b/patchwork/migrations/0040_add_related_patches.py
> new file mode 100644
> index 000000000000..f4d811fedddd
> --- /dev/null
> +++ b/patchwork/migrations/0040_add_related_patches.py
> @@ -0,0 +1,42 @@
> +# -*- coding: utf-8 -*-
> +# Generated by Django 1.11.27 on 2020-01-15 23:15
> +from __future__ import unicode_literals
> +
> +from django.db import migrations, models
> +import django.db.models.deletion
> +
> +
> +class Migration(migrations.Migration):
> +
> +    dependencies = [
> +        ('patchwork', '0039_unique_series_references'),
> +    ]
> +
> +    operations = [
> +        migrations.CreateModel(
> +            name='PatchRelation',
> +            fields=[
> +                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
> +            ],
> +        ),
> +        migrations.AlterField(
> +            model_name='event',
> +            name='category',
> +            field=models.CharField(choices=[(b'cover-created', b'Cover Letter Created'), (b'patch-created', b'Patch Created'), (b'patch-completed', b'Patch Completed'), (b'patch-state-changed', b'Patch State Changed'), (b'patch-delegated', b'Patch Delegate Changed'), (b'patch-relation-changed', b'Patch Relation Changed'), (b'check-created', b'Check Created'), (b'series-created', b'Series Created'), (b'series-completed', b'Series Completed')], db_index=True, help_text=b'The category of the event.', max_length=25),
> +        ),
> +        migrations.AddField(
> +            model_name='event',
> +            name='current_relation',
> +            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='+', to='patchwork.PatchRelation'),
> +        ),
> +        migrations.AddField(
> +            model_name='event',
> +            name='previous_relation',
> +            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='+', to='patchwork.PatchRelation'),
> +        ),
> +        migrations.AddField(
> +            model_name='patch',
> +            name='related',
> +            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='patches', related_query_name='patch', to='patchwork.PatchRelation'),
> +        ),
> +    ]
> diff --git a/patchwork/models.py b/patchwork/models.py
> index be71d4077892..4f7186d3ec10 100644
> --- a/patchwork/models.py
> +++ b/patchwork/models.py
> @@ -447,6 +447,12 @@ class Patch(Submission):
>          default=None, null=True,
>          help_text='The number assigned to this patch in the series')
>  
> +    # related patches metadata
> +
> +    related = models.ForeignKey(
> +        'PatchRelation', null=True, blank=True, on_delete=models.SET_NULL,
> +        related_name='patches', related_query_name='patch')
> +

I wonder if we need to make this a many-to-many to patch through
patchrelation? I'm struggling to get this to prefetch_related or
select_related properly: the best I can do atm is to prefetch_related on
'related__patches__project' which is super gross (and might be fetching
every project?)

Regards,
Daniel

>      objects = PatchManager()
>  
>      @staticmethod
> @@ -863,6 +869,19 @@ class BundlePatch(models.Model):
>          ordering = ['order']
>  
>  
> + at python_2_unicode_compatible
> +class PatchRelation(models.Model):
> +
> +    def __str__(self):
> +        patches = self.patches.all()
> +        if not patches:
> +            return '<Empty>'
> +        name = ', '.join(patch.name for patch in patches[:10])
> +        if len(name) > 60:
> +            name = name[:60] + '...'
> +        return name
> +
> +
>  @python_2_unicode_compatible
>  class Check(models.Model):
>  
> @@ -928,6 +947,7 @@ class Event(models.Model):
>      CATEGORY_PATCH_COMPLETED = 'patch-completed'
>      CATEGORY_PATCH_STATE_CHANGED = 'patch-state-changed'
>      CATEGORY_PATCH_DELEGATED = 'patch-delegated'
> +    CATEGORY_PATCH_RELATION_CHANGED = 'patch-relation-changed'
>      CATEGORY_CHECK_CREATED = 'check-created'
>      CATEGORY_SERIES_CREATED = 'series-created'
>      CATEGORY_SERIES_COMPLETED = 'series-completed'
> @@ -937,6 +957,7 @@ class Event(models.Model):
>          (CATEGORY_PATCH_COMPLETED, 'Patch Completed'),
>          (CATEGORY_PATCH_STATE_CHANGED, 'Patch State Changed'),
>          (CATEGORY_PATCH_DELEGATED, 'Patch Delegate Changed'),
> +        (CATEGORY_PATCH_RELATION_CHANGED, 'Patch Relation Changed'),
>          (CATEGORY_CHECK_CREATED, 'Check Created'),
>          (CATEGORY_SERIES_CREATED, 'Series Created'),
>          (CATEGORY_SERIES_COMPLETED, 'Series Completed'),
> @@ -952,7 +973,7 @@ class Event(models.Model):
>      # event metadata
>  
>      category = models.CharField(
> -        max_length=20,
> +        max_length=25,
>          choices=CATEGORY_CHOICES,
>          db_index=True,
>          help_text='The category of the event.')
> @@ -1000,6 +1021,15 @@ class Event(models.Model):
>          User, related_name='+', null=True, blank=True,
>          on_delete=models.CASCADE)
>  
> +    # fields for 'patch-relation-changed-changed' events
> +
> +    previous_relation = models.ForeignKey(
> +        PatchRelation, related_name='+', null=True, blank=True,
> +        on_delete=models.CASCADE)
> +    current_relation = models.ForeignKey(
> +        PatchRelation, related_name='+', null=True, blank=True,
> +        on_delete=models.CASCADE)
> +
>      # fields or 'patch-check-created' events
>  
>      created_check = models.ForeignKey(
> diff --git a/patchwork/signals.py b/patchwork/signals.py
> index 73ddfa5e35ee..3a2f0fbdd3a4 100644
> --- a/patchwork/signals.py
> +++ b/patchwork/signals.py
> @@ -134,6 +134,30 @@ def create_patch_delegated_event(sender, instance, raw, **kwargs):
>      create_event(instance, orig_patch.delegate, instance.delegate)
>  
>  
> + at receiver(pre_save, sender=Patch)
> +def create_patch_relation_changed_event(sender, instance, raw, **kwargs):
> +
> +    def create_event(patch, before, after):
> +        return Event.objects.create(
> +            category=Event.CATEGORY_PATCH_RELATION_CHANGED,
> +            project=patch.project,
> +            actor=getattr(patch, '_edited_by', None),
> +            patch=patch,
> +            previous_relation=before,
> +            current_relation=after)
> +
> +    # don't trigger for items loaded from fixtures or new items
> +    if raw or not instance.pk:
> +        return
> +
> +    orig_patch = Patch.objects.get(pk=instance.pk)
> +
> +    if orig_patch.related == instance.related:
> +        return
> +
> +    create_event(instance, orig_patch.related, instance.related)
> +
> +
>  @receiver(pre_save, sender=Patch)
>  def create_patch_completed_event(sender, instance, raw, **kwargs):
>  
> diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html
> index 77a2711ab5b4..978559b8726b 100644
> --- a/patchwork/templates/patchwork/submission.html
> +++ b/patchwork/templates/patchwork/submission.html
> @@ -110,6 +110,43 @@ function toggle_div(link_id, headers_id, label_show, label_hide)
>    </td>
>   </tr>
>  {% endif %}
> +{% if submission.related %}
> + <tr>
> +  <th>Related</th>
> +  <td>
> +   <a id="togglerelated"
> +      href="javascript:toggle_div('togglerelated', 'related')"
> +   >show</a>
> +   <div id="related" class="submissionlist" style="display:none;">
> +    <ul>
> +     {% for sibling in related_same_project %}
> +      <li>
> +       {% if sibling.id != submission.id %}
> +        <a href="{% url 'patch-detail' project_id=project.linkname msgid=sibling.url_msgid %}">
> +         {{ sibling.name|default:"[no subject]"|truncatechars:100 }}
> +        </a>
> +       {% endif %}
> +      </li>
> +     {% endfor %}
> +     {% if related_different_project %}
> +      <a id="togglerelatedoutside"
> +         href="javascript:toggle_div('togglerelatedoutside', 'relatedoutside', 'show from other projects')"
> +      >show from other projects</a>
> +      <div id="relatedoutside" class="submissionlist" style="display:none;">
> +       {% for sibling in related_outside %}
> +        <li>
> +         <a href="{% url 'patch-detail' project_id=sibling.project.linkname msgid=sibling.url_msgid %}">
> +          {{ sibling.name|default:"[no subject]"|truncatechars:100 }}
> +         </a> (in {{ sibling.project }})
> +        </li>
> +       {% endfor %}
> +      </div>
> +     {% endif %}
> +    </ul>
> +   </div>
> +  </td>
> + </tr>
> +{% endif %}
>  </table>
>  
>  <div class="patchforms">
> diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py
> index f34053ce57da..e32ff0bff5f5 100644
> --- a/patchwork/views/patch.py
> +++ b/patchwork/views/patch.py
> @@ -110,12 +110,25 @@ def patch_detail(request, project_id, msgid):
>      comments = comments.only('submitter', 'date', 'id', 'content',
>                               'submission')
>  
> +    if patch.related:
> +        related_same_project = \
> +            patch.related.patches.only('name', 'msgid', 'project', 'related')
> +        # avoid a second trip out to the db for info we already have
> +        related_different_project = \
> +            [related_patch for related_patch in related_same_project
> +             if related_patch.project_id != patch.project_id]
> +    else:
> +        related_same_project = []
> +        related_different_project = []
> +
>      context['comments'] = comments
>      context['checks'] = patch.check_set.all().select_related('user')
>      context['submission'] = patch
>      context['patchform'] = form
>      context['createbundleform'] = createbundleform
>      context['project'] = patch.project
> +    context['related_same_project'] = related_same_project
> +    context['related_different_project'] = related_different_project
>  
>      return render(request, 'patchwork/submission.html', context)
>  
> -- 
> 2.20.1


More information about the Patchwork mailing list