From fa285e09a4ae92eefdf705a76591bb462bc3dd81 Mon Sep 17 00:00:00 2001 From: wajeeha-khalid Date: Mon, 28 Sep 2015 18:31:13 +0500 Subject: [PATCH 1/2] MA-1189 - Discussion API: implemented GET comment children --- lms/djangoapps/discussion_api/api.py | 46 +++++++++++- lms/djangoapps/discussion_api/serializers.py | 9 +++ .../discussion_api/tests/test_api.py | 2 - .../discussion_api/tests/test_views.py | 72 ++++++++++++++++++- lms/djangoapps/discussion_api/views.py | 32 ++++++++- 5 files changed, 154 insertions(+), 7 deletions(-) diff --git a/lms/djangoapps/discussion_api/api.py b/lms/djangoapps/discussion_api/api.py index 15b35d5ac0..0b54ec0ccf 100644 --- a/lms/djangoapps/discussion_api/api.py +++ b/lms/djangoapps/discussion_api/api.py @@ -415,7 +415,7 @@ def get_comment_list(request, thread_id, endorsed, page, page_size, mark_as_read raise Http404 num_pages = (resp_total + page_size - 1) / page_size if resp_total else 1 - results = [CommentSerializer(response, context=context).data for response in responses] + results = [CommentSerializer(response, remove_fields=["children"], context=context).data for response in responses] return get_paginated_data(request, results, page, num_pages) @@ -718,6 +718,50 @@ def get_thread(request, thread_id): return serializer.data +def get_response_comments(request, comment_id, page, page_size): + """ + Return the list of comments for the given thread response. + + Arguments: + + request: The django request object used for build_absolute_uri and + determining the requesting user. + + comment_id: The id of the comment/response to get child comments for. + + page: The page number (1-indexed) to retrieve + + page_size: The number of comments to retrieve per page + + Returns: + + A paginated result containing a list of comments + + """ + try: + cc_comment = Comment(id=comment_id).retrieve() + cc_thread, context = _get_thread_and_context(request, cc_comment["thread_id"]) + if cc_thread["thread_type"] == "question": + thread_responses = cc_thread["endorsed_responses"] + cc_thread["non_endorsed_responses"] + else: + thread_responses = cc_thread["children"] + response_comments = [] + for response in thread_responses: + if response["id"] == comment_id: + response_comments = response["children"] + break + + response_skip = page_size * (page - 1) + paged_response_comments = response_comments[response_skip:(response_skip + page_size)] + results = [CommentSerializer(comment, context=context).data for comment in paged_response_comments] + + comments_count = len(response_comments) + num_pages = (comments_count + page_size - 1) / page_size if comments_count else 1 + return get_paginated_data(request, results, page, num_pages) + except CommentClientRequestError: + raise Http404 + + def delete_thread(request, thread_id): """ Delete a thread. diff --git a/lms/djangoapps/discussion_api/serializers.py b/lms/djangoapps/discussion_api/serializers.py index 4309531c12..18a8989a40 100644 --- a/lms/djangoapps/discussion_api/serializers.py +++ b/lms/djangoapps/discussion_api/serializers.py @@ -290,6 +290,15 @@ class CommentSerializer(_ContentSerializer): non_updatable_fields = NON_UPDATABLE_COMMENT_FIELDS + def __init__(self, *args, **kwargs): + remove_fields = kwargs.pop('remove_fields', None) + super(CommentSerializer, self).__init__(*args, **kwargs) + + if remove_fields: + # for multiple fields in a list + for field_name in remove_fields: + self.fields.pop(field_name) + def get_endorsed_by(self, obj): """ Returns the username of the endorsing user, if the information is diff --git a/lms/djangoapps/discussion_api/tests/test_api.py b/lms/djangoapps/discussion_api/tests/test_api.py index acb06ff2fc..ca14dfbec9 100644 --- a/lms/djangoapps/discussion_api/tests/test_api.py +++ b/lms/djangoapps/discussion_api/tests/test_api.py @@ -1146,7 +1146,6 @@ class GetCommentListTest(CommentsServiceMockMixin, SharedModuleStoreTestCase): "abuse_flagged": False, "voted": False, "vote_count": 4, - "children": [], "editable_fields": ["abuse_flagged", "voted"], }, { @@ -1166,7 +1165,6 @@ class GetCommentListTest(CommentsServiceMockMixin, SharedModuleStoreTestCase): "abuse_flagged": True, "voted": False, "vote_count": 7, - "children": [], "editable_fields": ["abuse_flagged", "voted"], }, ] diff --git a/lms/djangoapps/discussion_api/tests/test_views.py b/lms/djangoapps/discussion_api/tests/test_views.py index 27287a8c7a..915cfc89ec 100644 --- a/lms/djangoapps/discussion_api/tests/test_views.py +++ b/lms/djangoapps/discussion_api/tests/test_views.py @@ -662,7 +662,6 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): "endorsed": False, "abuse_flaggers": [], "votes": {"up_count": 4}, - "children": [], }] expected_comments = [{ "id": "test_comment", @@ -681,7 +680,6 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): "abuse_flagged": False, "voted": True, "vote_count": 4, - "children": [], "editable_fields": ["abuse_flagged", "voted"], }] self.register_get_thread_response({ @@ -1025,3 +1023,73 @@ class ThreadViewSetRetrieveTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase) self.register_get_thread_error_response(self.thread_id, 404) response = self.client.get(self.url) self.assertEqual(response.status_code, 404) + + +@httpretty.activate +class CommentViewSetRetrieveTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): + """Tests for CommentViewSet Retrieve""" + def setUp(self): + super(CommentViewSetRetrieveTest, self).setUp() + self.url = reverse("comment-detail", kwargs={"comment_id": "test_comment"}) + self.thread_id = "test_thread" + self.comment_id = "test_comment" + + def make_comment_data(self, comment_id, parent_id=None, children=[]): # pylint: disable=W0102 + """ + Returns comment dict object as returned by comments service + """ + return make_minimal_cs_comment({ + "id": comment_id, + "parent_id": parent_id, + "course_id": unicode(self.course.id), + "thread_id": self.thread_id, + "thread_type": "discussion", + "username": self.user.username, + "user_id": str(self.user.id), + "created_at": "2015-06-03T00:00:00Z", + "updated_at": "2015-06-03T00:00:00Z", + "body": "Original body", + "children": children, + }) + + def test_basic(self): + self.register_get_user_response(self.user) + cs_comment_child = self.make_comment_data("test_child_comment", self.comment_id, children=[]) + cs_comment = self.make_comment_data(self.comment_id, None, [cs_comment_child]) + cs_thread = make_minimal_cs_thread({ + "id": self.thread_id, + "course_id": unicode(self.course.id), + "children": [cs_comment], + }) + self.register_get_thread_response(cs_thread) + self.register_get_comment_response(cs_comment) + + expected_response_data = { + "id": "test_child_comment", + "parent_id": self.comment_id, + "thread_id": self.thread_id, + "author": self.user.username, + "author_label": None, + "raw_body": "Original body", + "rendered_body": "

Original body

", + "created_at": "2015-06-03T00:00:00Z", + "updated_at": "2015-06-03T00:00:00Z", + "children": [], + "endorsed_at": None, + "endorsed": False, + "endorsed_by": None, + "endorsed_by_label": None, + "voted": False, + "vote_count": 0, + "abuse_flagged": False, + "editable_fields": ["abuse_flagged", "raw_body", "voted"] + } + + response = self.client.get(self.url) + self.assertEqual(response.status_code, 200) + self.assertEqual(json.loads(response.content)['results'][0], expected_response_data) + + def test_retrieve_nonexistent_comment(self): + self.register_get_comment_error_response(self.comment_id, 404) + response = self.client.get(self.url) + self.assertEqual(response.status_code, 404) diff --git a/lms/djangoapps/discussion_api/views.py b/lms/djangoapps/discussion_api/views.py index 7852a4abe8..13de43b16f 100644 --- a/lms/djangoapps/discussion_api/views.py +++ b/lms/djangoapps/discussion_api/views.py @@ -18,6 +18,7 @@ from discussion_api.api import ( delete_thread, delete_comment, get_comment_list, + get_response_comments, get_course, get_course_topics, get_thread, @@ -25,7 +26,7 @@ from discussion_api.api import ( update_comment, update_thread, ) -from discussion_api.forms import CommentListGetForm, ThreadListGetForm +from discussion_api.forms import CommentListGetForm, ThreadListGetForm, _PaginationForm from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin @@ -299,6 +300,8 @@ class CommentViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet): GET /api/discussion/v1/comments/?thread_id=0123456789abcdef01234567 + GET /api/discussion/v1/comments/2123456789abcdef01234555 + POST /api/discussion/v1/comments/ { "thread_id": "0123456789abcdef01234567", @@ -310,7 +313,7 @@ class CommentViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet): DELETE /api/discussion/v1/comments/comment_id - **GET Parameters**: + **GET Comment List Parameters**: * thread_id (required): The thread to retrieve comments for @@ -325,6 +328,15 @@ class CommentViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet): * mark_as_read: Will mark the thread of the comments as read. (default is False) + **GET Child Comment List Parameters**: + + * comment_id (required): The comment to retrieve child comments for + + * page: The (1-indexed) page to retrieve (default is 1) + + * page_size: The number of items per page (default is 10, max is 100) + + **POST Parameters**: * thread_id (required): The thread to post the comment in @@ -420,6 +432,22 @@ class CommentViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet): ) ) + def retrieve(self, request, comment_id=None): + """ + Implements the GET method for comments against response ID + """ + form = _PaginationForm(request.GET) + if not form.is_valid(): + raise ValidationError(form.errors) + return Response( + get_response_comments( + request, + comment_id, + form.cleaned_data["page"], + form.cleaned_data["page_size"] + ) + ) + def create(self, request): """ Implements the POST method for the list endpoint as described in the From c5afa44e35bdfe4bd006ff9a6b510fa0485273dc Mon Sep 17 00:00:00 2001 From: wajeeha-khalid Date: Wed, 21 Oct 2015 18:40:43 +0500 Subject: [PATCH 2/2] used recursive to optionally get response comments from comment service --- lms/djangoapps/discussion_api/api.py | 15 +++++++++++---- lms/djangoapps/discussion_api/tests/test_api.py | 4 +++- lms/djangoapps/discussion_api/tests/test_views.py | 6 ++++-- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/lms/djangoapps/discussion_api/api.py b/lms/djangoapps/discussion_api/api.py index 0b54ec0ccf..548a76c919 100644 --- a/lms/djangoapps/discussion_api/api.py +++ b/lms/djangoapps/discussion_api/api.py @@ -8,6 +8,7 @@ from urlparse import urlunparse from django.core.exceptions import ValidationError from django.core.urlresolvers import reverse from django.http import Http404 +import itertools from rest_framework.exceptions import PermissionDenied @@ -378,7 +379,7 @@ def get_comment_list(request, thread_id, endorsed, page, page_size, mark_as_read request, thread_id, retrieve_kwargs={ - "recursive": True, + "recursive": False, "user_id": request.user.id, "mark_as_read": mark_as_read, "response_skip": response_skip, @@ -415,7 +416,7 @@ def get_comment_list(request, thread_id, endorsed, page, page_size, mark_as_read raise Http404 num_pages = (resp_total + page_size - 1) / page_size if resp_total else 1 - results = [CommentSerializer(response, remove_fields=["children"], context=context).data for response in responses] + results = [CommentSerializer(response, context=context).data for response in responses] return get_paginated_data(request, results, page, num_pages) @@ -740,9 +741,15 @@ def get_response_comments(request, comment_id, page, page_size): """ try: cc_comment = Comment(id=comment_id).retrieve() - cc_thread, context = _get_thread_and_context(request, cc_comment["thread_id"]) + cc_thread, context = _get_thread_and_context( + request, + cc_comment["thread_id"], + retrieve_kwargs={ + "recursive": True, + } + ) if cc_thread["thread_type"] == "question": - thread_responses = cc_thread["endorsed_responses"] + cc_thread["non_endorsed_responses"] + thread_responses = itertools.chain(cc_thread["endorsed_responses"], cc_thread["non_endorsed_responses"]) else: thread_responses = cc_thread["children"] response_comments = [] diff --git a/lms/djangoapps/discussion_api/tests/test_api.py b/lms/djangoapps/discussion_api/tests/test_api.py index ca14dfbec9..7b0568cf6d 100644 --- a/lms/djangoapps/discussion_api/tests/test_api.py +++ b/lms/djangoapps/discussion_api/tests/test_api.py @@ -1085,7 +1085,7 @@ class GetCommentListTest(CommentsServiceMockMixin, SharedModuleStoreTestCase): self.assert_query_params_equal( httpretty.httpretty.latest_requests[-2], { - "recursive": ["True"], + "recursive": ["False"], "user_id": [str(self.user.id)], "mark_as_read": ["False"], "resp_skip": ["70"], @@ -1147,6 +1147,7 @@ class GetCommentListTest(CommentsServiceMockMixin, SharedModuleStoreTestCase): "voted": False, "vote_count": 4, "editable_fields": ["abuse_flagged", "voted"], + "children": [], }, { "id": "test_comment_2", @@ -1166,6 +1167,7 @@ class GetCommentListTest(CommentsServiceMockMixin, SharedModuleStoreTestCase): "voted": False, "vote_count": 7, "editable_fields": ["abuse_flagged", "voted"], + "children": [], }, ] actual_comments = self.get_comment_list( diff --git a/lms/djangoapps/discussion_api/tests/test_views.py b/lms/djangoapps/discussion_api/tests/test_views.py index 915cfc89ec..6f5f699827 100644 --- a/lms/djangoapps/discussion_api/tests/test_views.py +++ b/lms/djangoapps/discussion_api/tests/test_views.py @@ -681,6 +681,7 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): "voted": True, "vote_count": 4, "editable_fields": ["abuse_flagged", "voted"], + "children": [], }] self.register_get_thread_response({ "id": self.thread_id, @@ -704,7 +705,7 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): self.assert_query_params_equal( httpretty.httpretty.latest_requests[-2], { - "recursive": ["True"], + "recursive": ["False"], "resp_skip": ["0"], "resp_limit": ["10"], "user_id": [str(self.user.id)], @@ -738,7 +739,7 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): self.assert_query_params_equal( httpretty.httpretty.latest_requests[-2], { - "recursive": ["True"], + "recursive": ["False"], "resp_skip": ["68"], "resp_limit": ["4"], "user_id": [str(self.user.id)], @@ -1026,6 +1027,7 @@ class ThreadViewSetRetrieveTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase) @httpretty.activate +@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) class CommentViewSetRetrieveTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): """Tests for CommentViewSet Retrieve""" def setUp(self):