[PATCH] patch-list: Link to delegate's assigned patches

[Sean Anderson]

Hi Stephen,

On 11/29/21 12:58 PM, Stephen Finucane wrote:
> On Mon, 2021-11-22 at 18:20 -0500, Sean Anderson wrote:
>> This adds a link to all patches currently delegated to a user. This can
>> be much easier than going through the delegate filter, especially if you
>> already have a patch delegated to that user. This also replaces username
>> text with their real name or email (if they are populated). This can
>> help submitters figure out who their patches are assigned to (in cases
>> where the username and real name of the delegate significantly diverge).
> 
> Thanks for the patch. Some comments below. If you can address these, I'd be
> happy to apply this.
> 
> Stephen
> 
> PS: This didn't appear on Patchwork. I've no idea why...

Hm, looks like my original message isn't in the archive. I've just subscribed
to the list, so perhaps that will fix it.

> 
>> Signed-off-by: Sean Anderson <sean.anderson at seco.com>
>> ---
>> 
>>  .../templates/patchwork/partials/patch-list.html    |  2 +-
>>  patchwork/templatetags/person.py                    | 13 ++++++++++++-
>>  .../notes/link-to-delegates-366d5b18f1367fd6.yaml   |  5 +++++
>>  3 files changed, 18 insertions(+), 2 deletions(-)
>>  create mode 100644 releasenotes/notes/link-to-delegates-366d5b18f1367fd6.yaml
>> 
>> diff --git a/patchwork/templates/patchwork/partials/patch-list.html b/patchwork/templates/patchwork/partials/patch-list.html
>> index 02d6dff..4918aeb 100644
>> --- a/patchwork/templates/patchwork/partials/patch-list.html
>> +++ b/patchwork/templates/patchwork/partials/patch-list.html
>> @@ -204,7 +204,7 @@ $(document).ready(function() {
>>     <td class="text-nowrap">{{ patch|patch_checks }}</td>
>>     <td class="text-nowrap">{{ patch.date|date:"Y-m-d" }}</td>
>>     <td>{{ patch.submitter|personify:project }}</td>
>> -   <td>{{ patch.delegate.username }}</td>
>> +   <td>{{ patch.delegate|delegatify:project }}</td>
>>     <td>{{ patch.state }}</td>
>>    </tr>
>>   {% empty %}
>> diff --git a/patchwork/templatetags/person.py b/patchwork/templatetags/person.py
>> index 61937d9..433d713 100644
>> --- a/patchwork/templatetags/person.py
>> +++ b/patchwork/templatetags/person.py
>> @@ -8,7 +8,7 @@ from django.urls import reverse
>>  from django.utils.html import escape
>>  from django.utils.safestring import mark_safe
>>  
>> -from patchwork.filters import SubmitterFilter
>> +from patchwork.filters import SubmitterFilter, DelegateFilter
>>  
>>  
>>  register = template.Library()
>> @@ -28,3 +28,14 @@ def personify(person, project):
>>          url, SubmitterFilter.param, escape(person.id), linktext)
>>  
>>      return mark_safe(out)
>> +
>> + at register.filter
>> +def delegatify(delegate, project):
>> +
>> +    linktext = escape(delegate.name or delegate.email or delegate.username)
> 
> delegate will be None if the patch hasn't been delegated to anyone. You need to
> handle this.

Ah, I forgot about that. Will fix.

>> +    url = reverse('patch-list',
>> +                  kwargs={'project_id': project.linkname})
> 
> style nit: I think this would all fit on one line?

This is the same formatting as personify above. But the whole thing is under 80
chars, so I will unwrap it.

>> +    out = '<a href="%s?%s=%s">%s</a>' % (
>> +        url, DelegateFilter.param, escape(delegate.id), linktext)
>> +
>> +    return mark_safe(out)
> 
> It would be great if there was some unit tests for this filter to ensure things
> are working as expected. I don't think we have any tests for this stuff
> currently but it should be pretty trivial to add, either in
> 'patchwork/tests/views/test_patch.py' or in a new
> 'patchwork/tests/templatetags/test_person.py' file. Heck, why not both, since
> the former are really more functional test'y than the latter. Note that you can
> build on an existing test for the former and refer to this StackOverflow answer
> for a guide on how to do the latter [1].

Ok, I will look into this. I was trying to avoid setting up a test environment >.>
But it looks like you very recently added github actions, so perhaps I can just
fork and push for CI

--Sean

>> diff --git a/releasenotes/notes/link-to-delegates-366d5b18f1367fd6.yaml b/releasenotes/notes/link-to-delegates-366d5b18f1367fd6.yaml
>> new file mode 100644
>> index 0000000..aa69df3
>> --- /dev/null
>> +++ b/releasenotes/notes/link-to-delegates-366d5b18f1367fd6.yaml
>> @@ -0,0 +1,5 @@
>> +---
>> +features:
>> +  - |
>> +    The delegate column in the patch list now links to all patches delegated to
>> +    that user.
> 
> [1] https://stackoverflow.com/a/49604862/613428
>