[2/4] models, templates: Add submission relations

Introduces the ability to add relations between submissions. Relations
are displayed in the details page of a submission under 'Related'.
Related submissions located in another projects can be viewed as well.

Signed-off-by: Mete Polat <metepolat2000@gmail.com>
---
 .../migrations/0038_submission_relations.py   | 30 +++++++++++++++
 patchwork/models.py                           | 10 +++++
 patchwork/templates/patchwork/submission.html | 37 +++++++++++++++++++
 patchwork/views/patch.py                      |  7 ++++
 4 files changed, 84 insertions(+)
 create mode 100644 patchwork/migrations/0038_submission_relations.py

On Sat, 2019-12-07 at 17:46 +0100, Mete Polat wrote:
> Introduces the ability to add relations between submissions. Relations
> are displayed in the details page of a submission under 'Related'.
> Related submissions located in another projects can be viewed as well.

Apologies for the delay /o\ I finally had a chance to look at this and
went ahead and rebased this this evening. I've a lot of notes below.

> Signed-off-by: Mete Polat <metepolat2000@gmail.com>
> ---
>  .../migrations/0038_submission_relations.py   | 30 +++++++++++++++
>  patchwork/models.py                           | 10 +++++
>  patchwork/templates/patchwork/submission.html | 37 +++++++++++++++++++
>  patchwork/views/patch.py                      |  7 ++++
>  4 files changed, 84 insertions(+)
>  create mode 100644 patchwork/migrations/0038_submission_relations.py
> 
> diff --git a/patchwork/migrations/0038_submission_relations.py b/patchwork/migrations/0038_submission_relations.py
> new file mode 100644
> index 000000000000..4c5274c64c09
> --- /dev/null
> +++ b/patchwork/migrations/0038_submission_relations.py
> @@ -0,0 +1,30 @@
> +# Generated by Django 2.2.6 on 2019-12-08 03:00
> +
> +import datetime
> +from django.conf import settings
> +from django.db import migrations, models
> +import django.db.models.deletion
> +import patchwork.models
> +
> +
> +class Migration(migrations.Migration):
> +
> +    dependencies = [
> +        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
> +        ('patchwork', '0037_event_actor'),
> +    ]
> +
> +    operations = [
> +        migrations.CreateModel(
> +            name='SubmissionRelation',
> +            fields=[
> +                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
> +                ('by', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)),
> +            ],
> +        ),
> +        migrations.AddField(
> +            model_name='submission',
> +            name='related',
> +            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='submissions', related_query_name='submission', to='patchwork.SubmissionRelation'),
> +        ),
> +    ]
> diff --git a/patchwork/models.py b/patchwork/models.py
> index 7f0efd489ae3..a92203b24ff2 100644
> --- a/patchwork/models.py
> +++ b/patchwork/models.py
> @@ -374,6 +374,9 @@ class Submission(FilenameMixin, EmailMixin, models.Model):
>      # submission metadata
>  
>      name = models.CharField(max_length=255)
> +    related = models.ForeignKey(
> +        'SubmissionRelation', null=True, blank=True, on_delete=models.SET_NULL,
> +        related_name='submissions', related_query_name='submission')

So I know Daniel suggested moving this out of Patch into Submission,
but I don't think that's the right thing to do. I've a few alternative
designs for resolving the performance issues caused by the Submission
base class and merging everything back into one is only one of them
(the others being a complete separation, and merging the CoverLetter
class into the Series class). Given how you plan to use this, and the
oddness of a cover letter relationship, I'd be in favour of reverting
to version 1 of the series and keeping this as a field on 'Patch'.

>      @property
>      def list_archive_url(self):
> @@ -863,6 +866,13 @@ class BundlePatch(models.Model):
>          ordering = ['order']
>  
>  
> +class SubmissionRelation(models.Model):

You'd need the '@python_2_unicode_compatible' decorator for this if it
were to be included in 2.2. If it ends up in 3.0, that shouldn't be the
case.

> +    by = models.ForeignKey(User, on_delete=models.CASCADE)

Again, I'm not a fan of this /o\ We already have a mechanism for
tracking who does stuff - 'Events'. Instead of doing this, could we
create a new event type and create these instead? That would provide a
far more detailed audit trail to boot, since we could track removals as
well as additions (assuming the former is allowed).

> +
> +    def __str__(self):
> +        return ', '.join(s.name for s in self.submissions.all()) or '<Empty>'
> +
> +
>  @python_2_unicode_compatible
>  class Check(models.Model):
>  
> diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html
> index 77a2711ab5b4..bb0391f98ff4 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 submission.related.submissions.all %}
> +      <li>
> +       {% if sibling.id != submission.id and sibling.project_id == project.id %}
> +        <a href="{% url 'patch-detail' project_id=project.linkname msgid=sibling.url_msgid %}">
> +         {{ sibling.name|default:"[no subject]"|truncatechars:100 }}
> +        </a>
> +       {% endif %}

You need to preload these here too, like you do in the REST API, to
prevent the database getting hammered.

> +      </li>
> +     {% endfor %}
> +     {% if related_outside %}
> +      <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..0480614614ad 100644
> --- a/patchwork/views/patch.py
> +++ b/patchwork/views/patch.py
> @@ -110,12 +110,19 @@ def patch_detail(request, project_id, msgid):
>      comments = comments.only('submitter', 'date', 'id', 'content',
>                               'submission')
>  
> +    if patch.related:
> +        related_outside = patch.related.submissions \
> +            .exclude(project=patch.project)
> +    else:
> +        related_outside = []
> +

I'm not sure why this exists. Could you add some additional context to
the commit message? In any case, I think it would be more performant to
just prefetch the related patches and do this filtering in the view.

>      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_outside'] = related_outside
>  
>      return render(request, 'patchwork/submission.html', context)
>  

I was going to ask why you did patch relations rather than series
relations, but I'm guessing PaStA works on a patch level? No issues
with that, but it would be nice to reuse the relationship information
to link series somehow. I've no idea how that would happen though, so
we can probably think about that later.

Stephen

Hi Stephen,

On Fr., 27. Dez. 2019 at 18:09, Stephen Finucane <stephen@that.guru> wrote:

>
> I was going to ask why you did patch relations rather than series
> relations, but I'm guessing PaStA works on a patch level? No issues
> with that, but it would be nice to reuse the relationship information
> to link series somehow. I've no idea how that would happen though, so
> we can probably think about that later.
>

Pasta works on relating patches to each other. Historically, it was
intended to relate patches and to identify their evolution in out-of-tree
developments. So, these developments never had the notion of proper series,
but only of versions of sets of patches. Ralf Ramsauer et al. then
recognized that this could be used on mailing lists as well, relating
patches on mailing lists.

I agree that computing relations on series would be really nice. We already
recorded the task as future work here:

https://github.com/lfd/PaStA/issues/34

Given the possible solution space to compute the relationship, i.e., which
factors to account and how to weigh them, this task is probably  a bachelor
thesis or even a master thesis work, including a proper evaluation. If you
know anyone interested, let us know. We will look for students interested
in working on this topic in 2020.

I hope we can see all of Mete's patches included in the final 2.2.0. We
want to make the current heuristics from Pasta useful to others, and the
integration into patchwork is an important step for that.

Lukas


>

Hi Stephen,

On 27.12.19 19:05, Stephen Finucane wrote:
> On Sat, 2019-12-07 at 17:46 +0100, Mete Polat wrote:
>> Introduces the ability to add relations between submissions. Relations
>> are displayed in the details page of a submission under 'Related'.
>> Related submissions located in another projects can be viewed as well.
> 
> Apologies for the delay /o\ I finally had a chance to look at this and
> went ahead and rebased this this evening. I've a lot of notes below.
> 

Unfortunately I won't be able to make bigger changes on this patch
anymore as my employment contract just ended (and exams coming up).
Nevertheless I think most of the work is done and both Patchwork and
PaStA could really benefit from this. Feel free to base you work on this
if you are interested to further work on it.

>> Signed-off-by: Mete Polat <metepolat2000@gmail.com>
>> ---
>>  .../migrations/0038_submission_relations.py   | 30 +++++++++++++++
>>  patchwork/models.py                           | 10 +++++
>>  patchwork/templates/patchwork/submission.html | 37 +++++++++++++++++++
>>  patchwork/views/patch.py                      |  7 ++++
>>  4 files changed, 84 insertions(+)
>>  create mode 100644 patchwork/migrations/0038_submission_relations.py
>>
>> diff --git a/patchwork/migrations/0038_submission_relations.py b/patchwork/migrations/0038_submission_relations.py
>> new file mode 100644
>> index 000000000000..4c5274c64c09
>> --- /dev/null
>> +++ b/patchwork/migrations/0038_submission_relations.py
>> @@ -0,0 +1,30 @@
>> +# Generated by Django 2.2.6 on 2019-12-08 03:00
>> +
>> +import datetime
>> +from django.conf import settings
>> +from django.db import migrations, models
>> +import django.db.models.deletion
>> +import patchwork.models
>> +
>> +
>> +class Migration(migrations.Migration):
>> +
>> +    dependencies = [
>> +        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
>> +        ('patchwork', '0037_event_actor'),
>> +    ]
>> +
>> +    operations = [
>> +        migrations.CreateModel(
>> +            name='SubmissionRelation',
>> +            fields=[
>> +                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
>> +                ('by', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)),
>> +            ],
>> +        ),
>> +        migrations.AddField(
>> +            model_name='submission',
>> +            name='related',
>> +            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='submissions', related_query_name='submission', to='patchwork.SubmissionRelation'),
>> +        ),
>> +    ]
>> diff --git a/patchwork/models.py b/patchwork/models.py
>> index 7f0efd489ae3..a92203b24ff2 100644
>> --- a/patchwork/models.py
>> +++ b/patchwork/models.py
>> @@ -374,6 +374,9 @@ class Submission(FilenameMixin, EmailMixin, models.Model):
>>      # submission metadata
>>  
>>      name = models.CharField(max_length=255)
>> +    related = models.ForeignKey(
>> +        'SubmissionRelation', null=True, blank=True, on_delete=models.SET_NULL,
>> +        related_name='submissions', related_query_name='submission')
> 
> So I know Daniel suggested moving this out of Patch into Submission,
> but I don't think that's the right thing to do. I've a few alternative
> designs for resolving the performance issues caused by the Submission
> base class and merging everything back into one is only one of them
> (the others being a complete separation, and merging the CoverLetter
> class into the Series class). Given how you plan to use this, and the
> oddness of a cover letter relationship, I'd be in favour of reverting
> to version 1 of the series and keeping this as a field on 'Patch'.
>>>      @property
>>      def list_archive_url(self):
>> @@ -863,6 +866,13 @@ class BundlePatch(models.Model):
>>          ordering = ['order']
>>  
>>  
>> +class SubmissionRelation(models.Model):
> 
> You'd need the '@python_2_unicode_compatible' decorator for this if it
> were to be included in 2.2. If it ends up in 3.0, that shouldn't be the
> case.
> 

Ahh I always forget the python2 compatibility /.\

>> +    by = models.ForeignKey(User, on_delete=models.CASCADE)
> 
> Again, I'm not a fan of this /o\ We already have a mechanism for
> tracking who does stuff - 'Events'. Instead of doing this, could we
> create a new event type and create these instead? That would provide a
> far more detailed audit trail to boot, since we could track removals as
> well as additions (assuming the former is allowed).
> >> +
>> +    def __str__(self):
>> +        return ', '.join(s.name for s in self.submissions.all()) or '<Empty>'
>> +
>> +
>>  @python_2_unicode_compatible
>>  class Check(models.Model):
>>  
>> diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html
>> index 77a2711ab5b4..bb0391f98ff4 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 submission.related.submissions.all %}
>> +      <li>
>> +       {% if sibling.id != submission.id and sibling.project_id == project.id %}
>> +        <a href="{% url 'patch-detail' project_id=project.linkname msgid=sibling.url_msgid %}">
>> +         {{ sibling.name|default:"[no subject]"|truncatechars:100 }}
>> +        </a>
>> +       {% endif %}
> 
> You need to preload these here too, like you do in the REST API, to
> prevent the database getting hammered.
> 
>> +      </li>
>> +     {% endfor %}
>> +     {% if related_outside %}
>> +      <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..0480614614ad 100644
>> --- a/patchwork/views/patch.py
>> +++ b/patchwork/views/patch.py
>> @@ -110,12 +110,19 @@ def patch_detail(request, project_id, msgid):
>>      comments = comments.only('submitter', 'date', 'id', 'content',
>>                               'submission')
>>  
>> +    if patch.related:
>> +        related_outside = patch.related.submissions \
>> +            .exclude(project=patch.project)
>> +    else:
>> +        related_outside = []
>> +
> 
> I'm not sure why this exists. Could you add some additional context to
> the commit message? In any case, I think it would be more performant to
> just prefetch the related patches and do this filtering in the view.
> 

I had some problems while trying to do the filtering in django's
template engine but to be honest I can't remember anymore what exactly
the problem was.

>>      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_outside'] = related_outside
>>  
>>      return render(request, 'patchwork/submission.html', context)
>>  
> 
> I was going to ask why you did patch relations rather than series
> relations, but I'm guessing PaStA works on a patch level? No issues
> with that, but it would be nice to reuse the relationship information
> to link series somehow. I've no idea how that would happen though, so
> we can probably think about that later.
> 
> Stephen
> 

Best regards and a happy new year full of patches!

Mete

On Mo., 30. Dez. 2019 at 12:11, Mete Polat <metepolat2000@gmail.com> wrote:

> Hi Stephen,
>
> On 27.12.19 19:05, Stephen Finucane wrote:
> > On Sat, 2019-12-07 at 17:46 +0100, Mete Polat wrote:
> >> Introduces the ability to add relations between submissions. Relations
> >> are displayed in the details page of a submission under 'Related'.
> >> Related submissions located in another projects can be viewed as well.
> >
> > Apologies for the delay /o\ I finally had a chance to look at this and
> > went ahead and rebased this this evening. I've a lot of notes below.
> >
>
> Unfortunately I won't be able to make bigger changes on this patch
> anymore as my employment contract just ended (and exams coming up).
> Nevertheless I think most of the work is done and both Patchwork and
> PaStA could really benefit from this. Feel free to base you work on this
> if you are interested to further work on it.


With no contract for Mete (due to my company's internal policies), I fear
this state of the patch needs to be picked up by someone else than Mete to
address the rework due to the review comments.

I will try to find a student to continue this work or rather other work
around patchwork mentioned on the kernel's workflows mailing list, but this
will probably take me a few months. So, hopefully someone on this list can
push a bit further; after a few turns, it seems that just a few steps short
of final integration.

Best regards,

Lukas

>

Lukas Bulwahn <lukas.bulwahn@gmail.com> writes:

> On Mo., 30. Dez. 2019 at 12:11, Mete Polat <metepolat2000@gmail.com> wrote:
>
>> Hi Stephen,
>>
>> On 27.12.19 19:05, Stephen Finucane wrote:
>> > On Sat, 2019-12-07 at 17:46 +0100, Mete Polat wrote:
>> >> Introduces the ability to add relations between submissions. Relations
>> >> are displayed in the details page of a submission under 'Related'.
>> >> Related submissions located in another projects can be viewed as well.
>> >
>> > Apologies for the delay /o\ I finally had a chance to look at this and
>> > went ahead and rebased this this evening. I've a lot of notes below.
>> >
>>
>> Unfortunately I won't be able to make bigger changes on this patch
>> anymore as my employment contract just ended (and exams coming up).
>> Nevertheless I think most of the work is done and both Patchwork and
>> PaStA could really benefit from this. Feel free to base you work on this
>> if you are interested to further work on it.
>
>
> With no contract for Mete (due to my company's internal policies), I fear
> this state of the patch needs to be picked up by someone else than Mete to
> address the rework due to the review comments.
>
> I will try to find a student to continue this work or rather other work
> around patchwork mentioned on the kernel's workflows mailing list, but this
> will probably take me a few months. So, hopefully someone on this list can
> push a bit further; after a few turns, it seems that just a few steps short
> of final integration.

Right, I'm sorry we weren't able to get this landed earlier. I'll own
this - I think it's an important part of the long-term evolution of
Patchwork.

Mete, best of luck with your exams and your future endeavours.

Regards,
Daniel

>
> Best regards,
>
> Lukas
>
>>
> _______________________________________________
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork

Stephen Finucane <stephen@that.guru> writes:

> On Sat, 2019-12-07 at 17:46 +0100, Mete Polat wrote:
>> Introduces the ability to add relations between submissions. Relations
>> are displayed in the details page of a submission under 'Related'.
>> Related submissions located in another projects can be viewed as well.
>
> Apologies for the delay /o\ I finally had a chance to look at this and
> went ahead and rebased this this evening. I've a lot of notes below.
>
>> Signed-off-by: Mete Polat <metepolat2000@gmail.com>
>> ---
>>  .../migrations/0038_submission_relations.py   | 30 +++++++++++++++
>>  patchwork/models.py                           | 10 +++++
>>  patchwork/templates/patchwork/submission.html | 37 +++++++++++++++++++
>>  patchwork/views/patch.py                      |  7 ++++
>>  4 files changed, 84 insertions(+)
>>  create mode 100644 patchwork/migrations/0038_submission_relations.py
>> 
>> diff --git a/patchwork/migrations/0038_submission_relations.py b/patchwork/migrations/0038_submission_relations.py
>> new file mode 100644
>> index 000000000000..4c5274c64c09
>> --- /dev/null
>> +++ b/patchwork/migrations/0038_submission_relations.py
>> @@ -0,0 +1,30 @@
>> +# Generated by Django 2.2.6 on 2019-12-08 03:00
>> +
>> +import datetime
>> +from django.conf import settings
>> +from django.db import migrations, models
>> +import django.db.models.deletion
>> +import patchwork.models
>> +
>> +
>> +class Migration(migrations.Migration):
>> +
>> +    dependencies = [
>> +        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
>> +        ('patchwork', '0037_event_actor'),
>> +    ]
>> +
>> +    operations = [
>> +        migrations.CreateModel(
>> +            name='SubmissionRelation',
>> +            fields=[
>> +                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
>> +                ('by', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)),
>> +            ],
>> +        ),
>> +        migrations.AddField(
>> +            model_name='submission',
>> +            name='related',
>> +            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='submissions', related_query_name='submission', to='patchwork.SubmissionRelation'),
>> +        ),
>> +    ]
>> diff --git a/patchwork/models.py b/patchwork/models.py
>> index 7f0efd489ae3..a92203b24ff2 100644
>> --- a/patchwork/models.py
>> +++ b/patchwork/models.py
>> @@ -374,6 +374,9 @@ class Submission(FilenameMixin, EmailMixin, models.Model):
>>      # submission metadata
>>  
>>      name = models.CharField(max_length=255)
>> +    related = models.ForeignKey(
>> +        'SubmissionRelation', null=True, blank=True, on_delete=models.SET_NULL,
>> +        related_name='submissions', related_query_name='submission')
>
> So I know Daniel suggested moving this out of Patch into Submission,
> but I don't think that's the right thing to do. I've a few alternative
> designs for resolving the performance issues caused by the Submission
> base class and merging everything back into one is only one of them
> (the others being a complete separation, and merging the CoverLetter
> class into the Series class). Given how you plan to use this, and the
> oddness of a cover letter relationship, I'd be in favour of reverting
> to version 1 of the series and keeping this as a field on 'Patch'.
>

What's odd about a cover-letter relationship? The CL of v1 is related to
the CL of v2, surely?

If we moved CLs into series rather than making them their own thing,
we'd need to just find a way to relate series as whole things. I wonder
if that's the way to go and I'd be interested in seeing what that looked
like. You can make a very good logical argument that a CL is much more
conceptually linked to a series than to a patch... yeah, I'd be really
interested in seeing that.

I still think I'm right wrt the code as it stands now, but I'm far more
interested in seeing at least the first step of this in 2.2 because I
think otherwise it will get lost in the push to 3.0, so I'll respin with
this reverted back to the orignal version. (Sorry Mete!!)

>>      @property
>>      def list_archive_url(self):
>> @@ -863,6 +866,13 @@ class BundlePatch(models.Model):
>>          ordering = ['order']
>>  
>>  
>> +class SubmissionRelation(models.Model):
>
> You'd need the '@python_2_unicode_compatible' decorator for this if it
> were to be included in 2.2. If it ends up in 3.0, that shouldn't be the
> case.

Fixed.

>
>> +    by = models.ForeignKey(User, on_delete=models.CASCADE)
>
> Again, I'm not a fan of this /o\ We already have a mechanism for
> tracking who does stuff - 'Events'. Instead of doing this, could we
> create a new event type and create these instead? That would provide a
> far more detailed audit trail to boot, since we could track removals as
> well as additions (assuming the former is allowed).

Fixed. Btw I notice we're using on_delete=CASCADE for event fields:
shouldn't we be using SET_NULL or better yet creating some blah_invalid
entries and referring to them? Events showing that something happened
even if we don't know all of what it is any more is better than no
events at all.

I'm going to leave it here for now because I'm really trying to not stay
up late hacking on stuff so much in 2020, but I hope to carve out time
next week to polish these two patches off.

Regards,
Daniel

>
>> +
>> +    def __str__(self):
>> +        return ', '.join(s.name for s in self.submissions.all()) or '<Empty>'
>> +
>> +
>>  @python_2_unicode_compatible
>>  class Check(models.Model):
>>  
>> diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html
>> index 77a2711ab5b4..bb0391f98ff4 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 submission.related.submissions.all %}
>> +      <li>
>> +       {% if sibling.id != submission.id and sibling.project_id == project.id %}
>> +        <a href="{% url 'patch-detail' project_id=project.linkname msgid=sibling.url_msgid %}">
>> +         {{ sibling.name|default:"[no subject]"|truncatechars:100 }}
>> +        </a>
>> +       {% endif %}
>
> You need to preload these here too, like you do in the REST API, to
> prevent the database getting hammered.
>
>> +      </li>
>> +     {% endfor %}
>> +     {% if related_outside %}
>> +      <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..0480614614ad 100644
>> --- a/patchwork/views/patch.py
>> +++ b/patchwork/views/patch.py
>> @@ -110,12 +110,19 @@ def patch_detail(request, project_id, msgid):
>>      comments = comments.only('submitter', 'date', 'id', 'content',
>>                               'submission')
>>  
>> +    if patch.related:
>> +        related_outside = patch.related.submissions \
>> +            .exclude(project=patch.project)
>> +    else:
>> +        related_outside = []
>> +
>
> I'm not sure why this exists. Could you add some additional context to
> the commit message? In any case, I think it would be more performant to
> just prefetch the related patches and do this filtering in the view.
>
>>      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_outside'] = related_outside
>>  
>>      return render(request, 'patchwork/submission.html', context)
>>  
>
> I was going to ask why you did patch relations rather than series
> relations, but I'm guessing PaStA works on a patch level? No issues
> with that, but it would be nice to reuse the relationship information
> to link series somehow. I've no idea how that would happen though, so
> we can probably think about that later.
>
> Stephen
>
> _______________________________________________
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork

On Sa., 11. Jan. 2020 at 11:47, Daniel Axtens <dja@axtens.net> wrote:

> Lukas Bulwahn <lukas.bulwahn@gmail.com> writes:
>
> > On Mo., 30. Dez. 2019 at 12:11, Mete Polat <metepolat2000@gmail.com>
> wrote:
> >
> >> Hi Stephen,
> >>
> >> On 27.12.19 19:05, Stephen Finucane wrote:
> >> > On Sat, 2019-12-07 at 17:46 +0100, Mete Polat wrote:
> >> >> Introduces the ability to add relations between submissions.
> Relations
> >> >> are displayed in the details page of a submission under 'Related'.
> >> >> Related submissions located in another projects can be viewed as
> well.
> >> >
> >> > Apologies for the delay /o\ I finally had a chance to look at this and
> >> > went ahead and rebased this this evening. I've a lot of notes below.
> >> >
> >>
> >> Unfortunately I won't be able to make bigger changes on this patch
> >> anymore as my employment contract just ended (and exams coming up).
> >> Nevertheless I think most of the work is done and both Patchwork and
> >> PaStA could really benefit from this. Feel free to base you work on this
> >> if you are interested to further work on it.
> >
> >
> > With no contract for Mete (due to my company's internal policies), I fear
> > this state of the patch needs to be picked up by someone else than Mete
> to
> > address the rework due to the review comments.
> >
> > I will try to find a student to continue this work or rather other work
> > around patchwork mentioned on the kernel's workflows mailing list, but
> this
> > will probably take me a few months. So, hopefully someone on this list
> can
> > push a bit further; after a few turns, it seems that just a few steps
> short
> > of final integration.
>
> Right, I'm sorry we weren't able to get this landed earlier. I'll own
> this - I think it's an important part of the long-term evolution of
> Patchwork.
>

Daniel, Thanks for taking further ownership of this feature.

If I find new students in 2020 with the appropriate background, I will
motivate them to continue contributions to Patchwork and inform you on the
mailing list about it.

Lukas


> Mete, best of luck with your exams and your future endeavours.
>
> Regards,
> Daniel
>
> >
> > Best regards,
> >
> > Lukas
> >
> >>
> > _______________________________________________
> > Patchwork mailing list
> > Patchwork@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/patchwork
>

On Sa., 11. Jan. 2020 at 13:08, Daniel Axtens <dja@axtens.net> wrote:

> Stephen Finucane <stephen@that.guru> writes:
>
> > On Sat, 2019-12-07 at 17:46 +0100, Mete Polat wrote:
> >> Introduces the ability to add relations between submissions. Relations
> >> are displayed in the details page of a submission under 'Related'.
> >> Related submissions located in another projects can be viewed as well.
> >
> > Apologies for the delay /o\ I finally had a chance to look at this and
> > went ahead and rebased this this evening. I've a lot of notes below.
> >
> >> Signed-off-by: Mete Polat <metepolat2000@gmail.com>
> >> ---
> >>  .../migrations/0038_submission_relations.py   | 30 +++++++++++++++
> >>  patchwork/models.py                           | 10 +++++
> >>  patchwork/templates/patchwork/submission.html | 37 +++++++++++++++++++
> >>  patchwork/views/patch.py                      |  7 ++++
> >>  4 files changed, 84 insertions(+)
> >>  create mode 100644 patchwork/migrations/0038_submission_relations.py
> >>
> >> diff --git a/patchwork/migrations/0038_submission_relations.py
> b/patchwork/migrations/0038_submission_relations.py
> >> new file mode 100644
> >> index 000000000000..4c5274c64c09
> >> --- /dev/null
> >> +++ b/patchwork/migrations/0038_submission_relations.py
> >> @@ -0,0 +1,30 @@
> >> +# Generated by Django 2.2.6 on 2019-12-08 03:00
> >> +
> >> +import datetime
> >> +from django.conf import settings
> >> +from django.db import migrations, models
> >> +import django.db.models.deletion
> >> +import patchwork.models
> >> +
> >> +
> >> +class Migration(migrations.Migration):
> >> +
> >> +    dependencies = [
> >> +        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
> >> +        ('patchwork', '0037_event_actor'),
> >> +    ]
> >> +
> >> +    operations = [
> >> +        migrations.CreateModel(
> >> +            name='SubmissionRelation',
> >> +            fields=[
> >> +                ('id', models.AutoField(auto_created=True,
> primary_key=True, serialize=False, verbose_name='ID')),
> >> +                ('by',
> models.ForeignKey(on_delete=django.db.models.deletion.CASCADE,
> to=settings.AUTH_USER_MODEL)),
> >> +            ],
> >> +        ),
> >> +        migrations.AddField(
> >> +            model_name='submission',
> >> +            name='related',
> >> +            field=models.ForeignKey(blank=True, null=True,
> on_delete=django.db.models.deletion.SET_NULL, related_name='submissions',
> related_query_name='submission', to='patchwork.SubmissionRelation'),
> >> +        ),
> >> +    ]
> >> diff --git a/patchwork/models.py b/patchwork/models.py
> >> index 7f0efd489ae3..a92203b24ff2 100644
> >> --- a/patchwork/models.py
> >> +++ b/patchwork/models.py
> >> @@ -374,6 +374,9 @@ class Submission(FilenameMixin, EmailMixin,
> models.Model):
> >>      # submission metadata
> >>
> >>      name = models.CharField(max_length=255)
> >> +    related = models.ForeignKey(
> >> +        'SubmissionRelation', null=True, blank=True,
> on_delete=models.SET_NULL,
> >> +        related_name='submissions', related_query_name='submission')
> >
> > So I know Daniel suggested moving this out of Patch into Submission,
> > but I don't think that's the right thing to do. I've a few alternative
> > designs for resolving the performance issues caused by the Submission
> > base class and merging everything back into one is only one of them
> > (the others being a complete separation, and merging the CoverLetter
> > class into the Series class). Given how you plan to use this, and the
> > oddness of a cover letter relationship, I'd be in favour of reverting
> > to version 1 of the series and keeping this as a field on 'Patch'.
> >
>
> What's odd about a cover-letter relationship? The CL of v1 is related to
> the CL of v2, surely?
>
> If we moved CLs into series rather than making them their own thing,
> we'd need to just find a way to relate series as whole things. I wonder
> if that's the way to go and I'd be interested in seeing what that looked
> like. You can make a very good logical argument that a CL is much more
> conceptually linked to a series than to a patch... yeah, I'd be really
> interested in seeing that.


> I still think I'm right wrt the code as it stands now, but I'm far more
> interested in seeing at least the first step of this in 2.2 because I
> think otherwise it will get lost in the push to 3.0, so I'll respin with
> this reverted back to the orignal version. (Sorry Mete!!)
>

In the end version, we will probably need to have both concepts in
patchwork, relations among patches and relations among series. Just
consider the evolution of the series and patches for this feature:

Patches appear at a certain version and integrated into git after another
version.
The series lives on while some patches stabilize, some are integrated and
some simply disappear.

Also the algorithms to find relations between series and between patches
are different; of course, these algorithms for patches and series can
mutually use the computed relationships of the other to compute its own
relation.

Let us get the relationship on patches into 2.2, and continue to look into
this topic for 3.0.

Lukas



> >>      @property
> >>      def list_archive_url(self):
> >> @@ -863,6 +866,13 @@ class BundlePatch(models.Model):
> >>          ordering = ['order']
> >>
> >>
> >> +class SubmissionRelation(models.Model):
> >
> > You'd need the '@python_2_unicode_compatible' decorator for this if it
> > were to be included in 2.2. If it ends up in 3.0, that shouldn't be the
> > case.
>
> Fixed.
>
> >
> >> +    by = models.ForeignKey(User, on_delete=models.CASCADE)
> >
> > Again, I'm not a fan of this /o\ We already have a mechanism for
> > tracking who does stuff - 'Events'. Instead of doing this, could we
> > create a new event type and create these instead? That would provide a
> > far more detailed audit trail to boot, since we could track removals as
> > well as additions (assuming the former is allowed).
>
> Fixed. Btw I notice we're using on_delete=CASCADE for event fields:
> shouldn't we be using SET_NULL or better yet creating some blah_invalid
> entries and referring to them? Events showing that something happened
> even if we don't know all of what it is any more is better than no
> events at all.
>
> I'm going to leave it here for now because I'm really trying to not stay
> up late hacking on stuff so much in 2020, but I hope to carve out time
> next week to polish these two patches off.
>
> Regards,
> Daniel
>
> >
> >> +
> >> +    def __str__(self):
> >> +        return ', '.join(s.name for s in self.submissions.all()) or
> '<Empty>'
> >> +
> >> +
> >>  @python_2_unicode_compatible
> >>  class Check(models.Model):
> >>
> >> diff --git a/patchwork/templates/patchwork/submission.html
> b/patchwork/templates/patchwork/submission.html
> >> index 77a2711ab5b4..bb0391f98ff4 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 submission.related.submissions.all %}
> >> +      <li>
> >> +       {% if sibling.id != submission.id and sibling.project_id ==
> project.id %}
> >> +        <a href="{% url 'patch-detail' project_id=project.linkname
> msgid=sibling.url_msgid %}">
> >> +         {{ sibling.name|default:"[no subject]"|truncatechars:100 }}
> >> +        </a>
> >> +       {% endif %}
> >
> > You need to preload these here too, like you do in the REST API, to
> > prevent the database getting hammered.
> >
> >> +      </li>
> >> +     {% endfor %}
> >> +     {% if related_outside %}
> >> +      <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..0480614614ad 100644
> >> --- a/patchwork/views/patch.py
> >> +++ b/patchwork/views/patch.py
> >> @@ -110,12 +110,19 @@ def patch_detail(request, project_id, msgid):
> >>      comments = comments.only('submitter', 'date', 'id', 'content',
> >>                               'submission')
> >>
> >> +    if patch.related:
> >> +        related_outside = patch.related.submissions \
> >> +            .exclude(project=patch.project)
> >> +    else:
> >> +        related_outside = []
> >> +
> >
> > I'm not sure why this exists. Could you add some additional context to
> > the commit message? In any case, I think it would be more performant to
> > just prefetch the related patches and do this filtering in the view.
> >
> >>      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_outside'] = related_outside
> >>
> >>      return render(request, 'patchwork/submission.html', context)
> >>
> >
> > I was going to ask why you did patch relations rather than series
> > relations, but I'm guessing PaStA works on a patch level? No issues
> > with that, but it would be nice to reuse the relationship information
> > to link series somehow. I've no idea how that would happen though, so
> > we can probably think about that later.
> >
> > Stephen
> >
> > _______________________________________________
> > 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
>

On Sat, 2020-01-11 at 17:12 +0000, Lukas Bulwahn wrote:
> 
> 
> On Sa., 11. Jan. 2020 at 13:08, Daniel Axtens <dja@axtens.net> wrote:
> > Stephen Finucane <stephen@that.guru> writes:
> > 
> > > On Sat, 2019-12-07 at 17:46 +0100, Mete Polat wrote:
> > >> Introduces the ability to add relations between submissions. Relations
> > >> are displayed in the details page of a submission under 'Related'.
> > >> Related submissions located in another projects can be viewed as well.
> > >
> > > Apologies for the delay /o\ I finally had a chance to look at this and
> > > went ahead and rebased this this evening. I've a lot of notes below.
> > >
> > >> Signed-off-by: Mete Polat <metepolat2000@gmail.com>
> > >> ---
> > >>  .../migrations/0038_submission_relations.py   | 30 +++++++++++++++
> > >>  patchwork/models.py                           | 10 +++++
> > >>  patchwork/templates/patchwork/submission.html | 37 +++++++++++++++++++
> > >>  patchwork/views/patch.py                      |  7 ++++
> > >>  4 files changed, 84 insertions(+)
> > >>  create mode 100644 patchwork/migrations/0038_submission_relations.py
> > >> 
> > >> diff --git a/patchwork/migrations/0038_submission_relations.py b/patchwork/migrations/0038_submission_relations.py
> > >> new file mode 100644
> > >> index 000000000000..4c5274c64c09
> > >> --- /dev/null
> > >> +++ b/patchwork/migrations/0038_submission_relations.py
> > >> @@ -0,0 +1,30 @@
> > >> +# Generated by Django 2.2.6 on 2019-12-08 03:00
> > >> +
> > >> +import datetime
> > >> +from django.conf import settings
> > >> +from django.db import migrations, models
> > >> +import django.db.models.deletion
> > >> +import patchwork.models
> > >> +
> > >> +
> > >> +class Migration(migrations.Migration):
> > >> +
> > >> +    dependencies = [
> > >> +        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
> > >> +        ('patchwork', '0037_event_actor'),
> > >> +    ]
> > >> +
> > >> +    operations = [
> > >> +        migrations.CreateModel(
> > >> +            name='SubmissionRelation',
> > >> +            fields=[
> > >> +                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
> > >> +                ('by', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)),
> > >> +            ],
> > >> +        ),
> > >> +        migrations.AddField(
> > >> +            model_name='submission',
> > >> +            name='related',
> > >> +            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='submissions', related_query_name='submission', to='patchwork.SubmissionRelation'),
> > >> +        ),
> > >> +    ]
> > >> diff --git a/patchwork/models.py b/patchwork/models.py
> > >> index 7f0efd489ae3..a92203b24ff2 100644
> > >> --- a/patchwork/models.py
> > >> +++ b/patchwork/models.py
> > >> @@ -374,6 +374,9 @@ class Submission(FilenameMixin, EmailMixin, models.Model):
> > >>      # submission metadata
> > >>  
> > >>      name = models.CharField(max_length=255)
> > >> +    related = models.ForeignKey(
> > >> +        'SubmissionRelation', null=True, blank=True, on_delete=models.SET_NULL,
> > >> +        related_name='submissions', related_query_name='submission')
> > >
> > > So I know Daniel suggested moving this out of Patch into Submission,
> > > but I don't think that's the right thing to do. I've a few alternative
> > > designs for resolving the performance issues caused by the Submission
> > > base class and merging everything back into one is only one of them
> > > (the others being a complete separation, and merging the CoverLetter
> > > class into the Series class). Given how you plan to use this, and the
> > > oddness of a cover letter relationship, I'd be in favour of reverting
> > > to version 1 of the series and keeping this as a field on 'Patch'.
> > >
> > 
> > What's odd about a cover-letter relationship? The CL of v1 is related to
> > the CL of v2, surely?
> > 
> > If we moved CLs into series rather than making them their own thing,
> > we'd need to just find a way to relate series as whole things. I wonder
> > if that's the way to go and I'd be interested in seeing what that looked
> > like. You can make a very good logical argument that a CL is much more
> > conceptually linked to a series than to a patch... yeah, I'd be really
> > interested in seeing that.
> > I still think I'm right wrt the code as it stands now, but I'm far more
> > interested in seeing at least the first step of this in 2.2 because I
> > think otherwise it will get lost in the push to 3.0, so I'll respin with
> > this reverted back to the orignal version. (Sorry Mete!!)

Let me know if you need any help with this refactor, Daniel. I was
going to attempt this myself this week (just back from PTO) but if
you're doing it, I'll wait and review instead.

> 
> In the end version, we will probably need to have both concepts in
> patchwork, relations among patches and relations among series. Just
> consider the evolution of the series and patches for this feature:
> 
> Patches appear at a certain version and integrated into git after
> another version.
> The series lives on while some patches stabilize, some are integrated
> and some simply disappear.
> 
> Also the algorithms to find relations between series and between
> patches are different; of course, these algorithms for patches and
> series can mutually use the computed relationships of the other to
> compute its own relation.
> 
> Let us get the relationship on patches into 2.2, and continue to look
> into this topic for 3.0.

++ this is a sane approach, IMO.

Stephen

> Lukas
> 
> 
> > >>      @property
> > >>      def list_archive_url(self):
> > >> @@ -863,6 +866,13 @@ class BundlePatch(models.Model):
> > >>          ordering = ['order']
> > >>  
> > >>  
> > >> +class SubmissionRelation(models.Model):
> > >
> > > You'd need the '@python_2_unicode_compatible' decorator for this if it
> > > were to be included in 2.2. If it ends up in 3.0, that shouldn't be the
> > > case.
> > 
> > Fixed.
> > 
> > >
> > >> +    by = models.ForeignKey(User, on_delete=models.CASCADE)
> > >
> > > Again, I'm not a fan of this /o\ We already have a mechanism for
> > > tracking who does stuff - 'Events'. Instead of doing this, could we
> > > create a new event type and create these instead? That would provide a
> > > far more detailed audit trail to boot, since we could track removals as
> > > well as additions (assuming the former is allowed).
> > 
> > Fixed. Btw I notice we're using on_delete=CASCADE for event fields:
> > shouldn't we be using SET_NULL or better yet creating some blah_invalid
> > entries and referring to them? Events showing that something happened
> > even if we don't know all of what it is any more is better than no
> > events at all.
> > 
> > I'm going to leave it here for now because I'm really trying to not stay
> > up late hacking on stuff so much in 2020, but I hope to carve out time
> > next week to polish these two patches off.
> > 
> > Regards,
> > Daniel
> > 
> > >
> > >> +
> > >> +    def __str__(self):
> > >> +        return ', '.join(s.name for s in self.submissions.all()) or '<Empty>'
> > >> +
> > >> +
> > >>  @python_2_unicode_compatible
> > >>  class Check(models.Model):
> > >>  
> > >> diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html
> > >> index 77a2711ab5b4..bb0391f98ff4 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 submission.related.submissions.all %}
> > >> +      <li>
> > >> +       {% if sibling.id != submission.id and sibling.project_id == project.id %}
> > >> +        <a href="{% url 'patch-detail' project_id=project.linkname msgid=sibling.url_msgid %}">
> > >> +         {{ sibling.name|default:"[no subject]"|truncatechars:100 }}
> > >> +        </a>
> > >> +       {% endif %}
> > >
> > > You need to preload these here too, like you do in the REST API, to
> > > prevent the database getting hammered.
> > >
> > >> +      </li>
> > >> +     {% endfor %}
> > >> +     {% if related_outside %}
> > >> +      <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..0480614614ad 100644
> > >> --- a/patchwork/views/patch.py
> > >> +++ b/patchwork/views/patch.py
> > >> @@ -110,12 +110,19 @@ def patch_detail(request, project_id, msgid):
> > >>      comments = comments.only('submitter', 'date', 'id', 'content',
> > >>                               'submission')
> > >>  
> > >> +    if patch.related:
> > >> +        related_outside = patch.related.submissions \
> > >> +            .exclude(project=patch.project)
> > >> +    else:
> > >> +        related_outside = []
> > >> +
> > >
> > > I'm not sure why this exists. Could you add some additional context to
> > > the commit message? In any case, I think it would be more performant to
> > > just prefetch the related patches and do this filtering in the view.
> > >
> > >>      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_outside'] = related_outside
> > >>  
> > >>      return render(request, 'patchwork/submission.html', context)
> > >>  
> > >
> > > I was going to ask why you did patch relations rather than series
> > > relations, but I'm guessing PaStA works on a patch level? No issues
> > > with that, but it would be nice to reuse the relationship information
> > > to link series somehow. I've no idea how that would happen though, so
> > > we can probably think about that later.
> > >
> > > Stephen

Picking this up again...
>> +    by = models.ForeignKey(User, on_delete=models.CASCADE)
>
> Again, I'm not a fan of this /o\ We already have a mechanism for
> tracking who does stuff - 'Events'. Instead of doing this, could we
> create a new event type and create these instead? That would provide a
> far more detailed audit trail to boot, since we could track removals as
> well as additions (assuming the former is allowed).

I have to bump up the size of the category field because
'patch-relation-changed' is longer than 20 characters.
>
>> +
>> +    def __str__(self):
>> +        return ', '.join(s.name for s in self.submissions.all()) or '<Empty>'

I've made this cut off after 60 chars (and only consider the first 10
patches) so as not to overwhelm the admin view. Oh, I also added an
admin view.

>> +
>> +
>>  @python_2_unicode_compatible
>>  class Check(models.Model):
>>  
>> diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html
>> index 77a2711ab5b4..bb0391f98ff4 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 submission.related.submissions.all %}
>> +      <li>
>> +       {% if sibling.id != submission.id and sibling.project_id == project.id %}
>> +        <a href="{% url 'patch-detail' project_id=project.linkname msgid=sibling.url_msgid %}">
>> +         {{ sibling.name|default:"[no subject]"|truncatechars:100 }}
>> +        </a>
>> +       {% endif %}
>
> You need to preload these here too, like you do in the REST API, to
> prevent the database getting hammered.
>
AFAICT the refactored (patch only) one doesn't require this, but with
some tomfoolery I can reduce the load on the database even more with
judicious use of .only, and by not going out to the database again for
related_outside (which I have renamed related_different_project).

>> +      </li>
>> +     {% endfor %}
>> +     {% if related_outside %}
>> +      <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..0480614614ad 100644
>> --- a/patchwork/views/patch.py
>> +++ b/patchwork/views/patch.py
>> @@ -110,12 +110,19 @@ def patch_detail(request, project_id, msgid):
>>      comments = comments.only('submitter', 'date', 'id', 'content',
>>                               'submission')
>>  
>> +    if patch.related:
>> +        related_outside = patch.related.submissions \
>> +            .exclude(project=patch.project)
>> +    else:
>> +        related_outside = []
>> +
>
> I'm not sure why this exists. Could you add some additional context to
> the commit message? In any case, I think it would be more performant to
> just prefetch the related patches and do this filtering in the view.
>

As per above, this is now related_same_project and
related_different_project, it exists on my suggestion so that relations
across different projects show up separately. This means that if you're
looking at linuxppc-dev at a series cross-posted to the pci list, the
relations created in different projects are visible if you care to
explore them, but are not forced on you.

>>      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_outside'] = related_outside
>>  
>>      return render(request, 'patchwork/submission.html', context)
>>  
>
> I was going to ask why you did patch relations rather than series
> relations, but I'm guessing PaStA works on a patch level? No issues
> with that, but it would be nice to reuse the relationship information
> to link series somehow. I've no idea how that would happen though, so
> we can probably think about that later.
>
> Stephen
>
> _______________________________________________
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork