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

[Stephen Finucane]

On Mon, 2018-08-27 at 00:14 +1000, Daniel Axtens wrote:
> Add /message/<msgid> and /project/<project>/message/<msgid> URLs.
> 
> /project/<project>/message/<msgid> finds the patch, cover-letter or
> comment in <project> that matches <msgid> and redirects you to it.
> This is guaranteed to be unique.
> 
> /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.

> This patch doesn't expose these URLs in the UI anywhere. I'd love to
> know where people would like them to go. I hesitate to make the
> message-id clickable in the patch detail view, as I think that would
> break workflows for people who want to copy the msgid to the clipboard.
>
> 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.

Stephen

> Partially-closes: #106
> Reported-by: Konstantin Ryabitsev <konstantin at linuxfoundation.org>
> Reported-by: Linus Torvalds <torvalds at linux-foundation.org>
> Reported-by: Stephen Finucane <stephen at that.guru>
> Signed-off-by: Daniel Axtens <dja at axtens.net>
> ---
>  patchwork/urls.py        |  7 ++++++
>  patchwork/views/patch.py | 49 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/patchwork/urls.py b/patchwork/urls.py
> index 6b1ef511ef15..90391b90e413 100644
> --- a/patchwork/urls.py
> +++ b/patchwork/urls.py
> @@ -74,6 +74,13 @@ urlpatterns = [
>      url(r'^series/(?P<series_id>\d+)/mbox/$', series_views.series_mbox,
>          name='series-mbox'),
>  
> +    # 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.

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))