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

[Don Zickus]

On Wed, Aug 29, 2018 at 01:50:11PM +0100, Stephen Finucane wrote:
> 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.

With the kernel git tree being the culmination of about 100 different
mailing lists/projects, it is hard to map a msg-id found in the commit log
to the right project.  So the former could be difficult unless you have the
decoder ring for the mailing list it went through.

Now the msg-id could map to nothing to as there is no guarantee it is in
patchwork either.

Just a different perspective.

Cheers,
Don

> 
> > 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))
> 
> 
> _______________________________________________
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork