[PATCH v3 01/10] api: change <pk> parameter to <patch_id> for comments endpoint

[Stephen Finucane]

On Fri, 2021-08-13 at 09:56 +0100, Stephen Finucane wrote:
> On Fri, 2021-08-13 at 05:31 +0000, Raxel Gutierrez wrote:
> > Refactor patch lookup parameter `pk` to `patch_id` for the comments
> > list
> > endpoint to disambiguate from the lookup parameter `comment_id` in an
> > upcoming patch which introduces the comments detail endpoint. This
> > doesn't affect the user-facing API.
> > 
> > Signed-off-by: Raxel Gutierrez <raxel at google.com>
> 
> This looks sensible to me. I'd like it if we could also update the
> routes for cover letter comments, but that's a nice-to-have based on
> the current design of this feature and could be done in a separate
> change if you had time.
> 
> Reviewed-by: Stephen Finucane <stephen at that.guru>
> 
> I'll hold off applying until Daniel has had a chance to look also.

Tell a lie. I've gone and applied this since the next few patches look sane and
it seems Daniel was pretty happy with this in v2 once it was split out.

@Daniel: If you've any outstanding concerns, feel free to revert or ask for a
follow-up (I'd still like to see the cover comments routes updated too)

Stephen

> Stephen
> 
> PS: Feel free to include the 'Reviewed-by' tag in future revisions if
> this isn't applied before then and assuming the patch itself is
> unchanged.
> 
> > ---
> >  patchwork/api/comment.py            | 6 +++---
> >  patchwork/api/patch.py              | 2 +-
> >  patchwork/tests/api/test_comment.py | 4 ++--
> >  patchwork/urls.py                   | 2 +-
> >  4 files changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/patchwork/api/comment.py b/patchwork/api/comment.py
> > index 43b26c6..0c578b4 100644
> > --- a/patchwork/api/comment.py
> > +++ b/patchwork/api/comment.py
> > @@ -102,12 +102,12 @@ class PatchCommentList(ListAPIView):
> >      search_fields = ('subject',)
> >      ordering_fields = ('id', 'subject', 'date', 'submitter')
> >      ordering = 'id'
> > -    lookup_url_kwarg = 'pk'
> > +    lookup_url_kwarg = 'patch_id'
> >  
> >      def get_queryset(self):
> > -        if not Patch.objects.filter(pk=self.kwargs['pk']).exists():
> > +        if not
> > Patch.objects.filter(id=self.kwargs['patch_id']).exists():
> >              raise Http404
> >  
> >          return PatchComment.objects.filter(
> > -            patch=self.kwargs['pk']
> > +            patch=self.kwargs['patch_id']
> >          ).select_related('submitter')
> > diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
> > index 9d22275..a97a882 100644
> > --- a/patchwork/api/patch.py
> > +++ b/patchwork/api/patch.py
> > @@ -97,7 +97,7 @@ class
> > PatchListSerializer(BaseHyperlinkedModelSerializer):
> >  
> >      def get_comments(self, patch):
> >          return self.context.get('request').build_absolute_uri(
> > -            reverse('api-patch-comment-list', kwargs={'pk':
> > patch.id}))
> > +            reverse('api-patch-comment-list', kwargs={'patch_id':
> > patch.id}))
> >  
> >      def get_check(self, instance):
> >          return instance.combined_check_state
> > diff --git a/patchwork/tests/api/test_comment.py
> > b/patchwork/tests/api/test_comment.py
> > index 5bbebf2..59450d8 100644
> > --- a/patchwork/tests/api/test_comment.py
> > +++ b/patchwork/tests/api/test_comment.py
> > @@ -90,7 +90,7 @@ class TestPatchComments(utils.APITestCase):
> >          kwargs = {}
> >          if version:
> >              kwargs['version'] = version
> > -        kwargs['pk'] = patch.id
> > +        kwargs['patch_id'] = patch.id
> >  
> >          return reverse('api-patch-comment-list', kwargs=kwargs)
> >  
> > @@ -142,5 +142,5 @@ class TestPatchComments(utils.APITestCase):
> >      def test_list_invalid_patch(self):
> >          """Ensure we get a 404 for a non-existent patch."""
> >          resp = self.client.get(
> > -            reverse('api-patch-comment-list', kwargs={'pk':
> > '99999'}))
> > +            reverse('api-patch-comment-list', kwargs={'patch_id':
> > '99999'}))
> >          self.assertEqual(status.HTTP_404_NOT_FOUND,
> > resp.status_code)
> > diff --git a/patchwork/urls.py b/patchwork/urls.py
> > index 6ac9b81..1e6c12a 100644
> > --- a/patchwork/urls.py
> > +++ b/patchwork/urls.py
> > @@ -332,7 +332,7 @@ if settings.ENABLE_REST_API:
> >  
> >      api_1_1_patterns = [
> >          path(
> > -            'patches/<pk>/comments/',
> > +            'patches/<patch_id>/comments/',
> >              api_comment_views.PatchCommentList.as_view(),
> >              name='api-patch-comment-list',
> >          ),
> 
> 
> _______________________________________________
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork