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 <ra...@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 <step...@that.guru>
I'll hold off applying until Daniel has had a chance to look also.
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@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
