[PATCH] Find patches, cover letters and comments by msgid

[Daniel Axtens]

Hi Stephen,

>> /message/<msgid> does not require a project, but therefore if a mail
>> has been sent to multiple projects, the project that you will be
>> redirected to is arbitrary.
>
> Personally, I'm not a fan of the latter as I don't see what value it
> would bring. Is there any reason to keep it or could we drop it.

As Don points out, without this the feature is basically useless for
kernel work. Given that kernel work represents the bulk of both OzLabs
PW and kernel.org PW, it's reasonably critical.

>> These URLs also just redirect to the usual canonical patchwork URL -
>> it doesn't supplant or replace them. However, I'd be open to moving to
>> this scheme as the 'normal'/default URL in the future.
>
> Personally, I'd reverse the order of things and make these the new
> canonical URLs. There are issues with using the autoincrement ID
> column, as Konstantin has pointed out, but we could continue to use
> these for shortlinks until such a time as we come up with something
> better.
>
>> I also haven't attempted to do anything meaningful with series.
>
> I think this needs to be addressed too. At a minimum, we should do
> cover letters.

>> +    # message-id searches
>> +    url(r'^project/(?P<project_id>[^/]+)/message/(?P<msg_id>[^/]+)$',
>> +        patch_views.patch_by_project_msgid,
>> +        name='msgid-project-redirect'),
>
> Given my above comments, I wonder if we should split this into 'patch'
> and 'cover' again? I realize it's more work, but I really do think
> these should become the canonical links.

I'm not 100% sure what you had in mind here; I'm guessing you mean that
you want the ultimate URL scheme to be:

For patches:
 The canonical URL                         - /project/N/patch/<msg_id>
 Accepted, redirects you to canonical URL  - /project/N/patch/NNN
 Accepted, guesses a project and redirects - /patch/<msg_id>

For covers:
 The canonical URL                         - /project/N/cover/<msg_id>
 Accepted, redirects you to canonical URL  - /project/N/cover/NNN
 Accepted, guesses a project and redirects - /cover/<msg_id>

(And I suppose if you get patch vs cover wrong it'll just redirect you
to the right one.)

For series, I'm even less clear on what you would like. My guess would
be that we could have /project/N/series/<msg_id> just show you an
abbreviated version of the entire series that msg_id is a part of. I
think that would be super useful but would require some UI work
(designing a template to show you a whole series) and ideally for the
1:N series change to land. It might be better for a follow up.

Does that line up with what you had in mind?

Regards,
Daniel

>
> Stephen
>
>> +    url(r'^message/(?P<msg_id>[^/]+)$', patch_views.patch_by_msgid,
>> +        name='msgid-redirect'),
>> +
>>      # logged-in user stuff
>>      url(r'^user/$', user_views.profile, name='user-profile'),
>>      url(r'^user/todo/$', user_views.todo_lists,
>> diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py
>> index 6921882e71d1..53441b39cc57 100644
>> --- a/patchwork/views/patch.py
>> +++ b/patchwork/views/patch.py
>> @@ -28,6 +28,7 @@ from django.urls import reverse
>>  
>>  from patchwork.forms import CreateBundleForm
>>  from patchwork.forms import PatchForm
>> +from patchwork.models import Comment
>>  from patchwork.models import Bundle
>>  from patchwork.models import Patch
>>  from patchwork.models import Project
>> @@ -150,3 +151,51 @@ def patch_mbox(request, patch_id):
>>          patch.filename)
>>  
>>      return response
>> +
>> +
>> +def patch_by_msgid(request, msg_id):
>> +    msgid = ('<%s>' % msg_id)
>> +
>> +    patches = Patch.objects.filter(msgid=msgid)
>> +    if patches:
>> +        return HttpResponseRedirect(
>> +                reverse('patch-detail', kwargs={'patch_id': patches.first().id}))
>> +
>> +    subs = Submission.objects.filter(msgid=msgid)
>> +    if subs:
>> +        return HttpResponseRedirect(
>> +                reverse('cover-detail', kwargs={'cover_id': subs.first().id}))
>> +
>> +    comments = Comment.objects.filter(msgid=msgid)
>> +    if comments:
>> +        return HttpResponseRedirect(comments.first().get_absolute_url())
>> +
>> +    raise Http404("No patch, cover letter of comment matching %s found." % msg_id)
>> +
>> +
>> +def patch_by_project_msgid(request, project_id, msg_id):
>> +    project = get_object_or_404(Project, linkname=project_id)
>> +    project_id = project.id
>> +    msgid = ('<%s>' % msg_id)
>> +
>> +    try:
>> +        patch = Patch.objects.get(patch_project_id=project_id, msgid=msgid)
>> +        return HttpResponseRedirect(
>> +                reverse('patch-detail', kwargs={'patch_id': patch.id}))
>> +    except Patch.DoesNotExist:
>> +        pass
>> +
>> +    try:
>> +        sub = Submission.objects.get(project_id=project_id, msgid=msgid)
>> +        return HttpResponseRedirect(
>> +                reverse('cover-detail', kwargs={'cover_id': sub.id}))
>> +    except Submission.DoesNotExist:
>> +        pass
>> +
>> +    try:
>> +        comment = Comment.objects.get(submission__project_id=project_id, msgid=msgid)
>> +        return HttpResponseRedirect(comment.get_absolute_url())
>> +    except Comment.DoesNotExist:
>> +        pass
>> +
>> +    raise Http404("No patch, cover letter or comment matching %s found in %s" % (msg_id, project.name))