From a3c6faefc5a1752b5fda9fe566a288641d9bee3b Mon Sep 17 00:00:00 2001 From: wajeeha-khalid Date: Tue, 13 Oct 2015 17:45:28 +0500 Subject: [PATCH] MA-1359 & MA-1457 DiscussionAPI: return response_count for thread GET/POST --- lms/djangoapps/discussion_api/api.py | 7 +++---- lms/djangoapps/discussion_api/serializers.py | 12 ++---------- lms/djangoapps/discussion_api/tests/test_api.py | 7 +++++-- .../discussion_api/tests/test_serializers.py | 3 +-- lms/djangoapps/discussion_api/tests/test_views.py | 5 ++++- lms/djangoapps/discussion_api/tests/utils.py | 2 +- lms/djangoapps/discussion_api/views.py | 2 ++ 7 files changed, 18 insertions(+), 20 deletions(-) diff --git a/lms/djangoapps/discussion_api/api.py b/lms/djangoapps/discussion_api/api.py index 15b35d5ac0..d01d7357c7 100644 --- a/lms/djangoapps/discussion_api/api.py +++ b/lms/djangoapps/discussion_api/api.py @@ -341,7 +341,7 @@ def get_thread_list( if result_page != page: raise Http404 - results = [ThreadSerializer(thread, remove_fields=['response_count'], context=context).data for thread in threads] + results = [ThreadSerializer(thread, context=context).data for thread in threads] ret = get_paginated_data(request, results, page, num_pages) ret["text_search_rewrite"] = text_search_rewrite return ret @@ -555,7 +555,7 @@ def create_thread(request, thread_data): ): thread_data = thread_data.copy() thread_data["group_id"] = get_cohort_id(user, course_key) - serializer = ThreadSerializer(data=thread_data, remove_fields=['response_count'], context=context) + serializer = ThreadSerializer(data=thread_data, context=context) actions_form = ThreadActionsForm(thread_data) if not (serializer.is_valid() and actions_form.is_valid()): raise ValidationError(dict(serializer.errors.items() + actions_form.errors.items())) @@ -642,8 +642,7 @@ def update_thread(request, thread_id, update_data): """ cc_thread, context = _get_thread_and_context(request, thread_id) _check_editable_fields(cc_thread, update_data, context) - serializer = ThreadSerializer(cc_thread, remove_fields=['response_count'], data=update_data, partial=True, - context=context) + serializer = ThreadSerializer(cc_thread, data=update_data, partial=True, context=context) actions_form = ThreadActionsForm(update_data) if not (serializer.is_valid() and actions_form.is_valid()): raise ValidationError(dict(serializer.errors.items() + actions_form.errors.items())) diff --git a/lms/djangoapps/discussion_api/serializers.py b/lms/djangoapps/discussion_api/serializers.py index 4309531c12..c9c73aa93c 100644 --- a/lms/djangoapps/discussion_api/serializers.py +++ b/lms/djangoapps/discussion_api/serializers.py @@ -197,24 +197,16 @@ class ThreadSerializer(_ContentSerializer): non_endorsed_comment_list_url = serializers.SerializerMethodField() read = serializers.BooleanField(read_only=True) has_endorsed = serializers.BooleanField(read_only=True, source="endorsed") - response_count = serializers.IntegerField(source="resp_total", read_only=True) + response_count = serializers.IntegerField(source="resp_total", read_only=True, required=False) non_updatable_fields = NON_UPDATABLE_THREAD_FIELDS - # TODO: https://openedx.atlassian.net/browse/MA-1359 def __init__(self, *args, **kwargs): - remove_fields = kwargs.pop('remove_fields', None) super(ThreadSerializer, self).__init__(*args, **kwargs) # Compensate for the fact that some threads in the comments service do # not have the pinned field set if self.instance and self.instance.get("pinned") is None: self.instance["pinned"] = False - if self.instance and self.instance.get("resp_total") is None: - self.instance["resp_total"] = None - if remove_fields: - # for multiple fields in a list - for field_name in remove_fields: - self.fields.pop(field_name) def get_pinned(self, obj): """ @@ -286,7 +278,7 @@ class CommentSerializer(_ContentSerializer): endorsed_by = serializers.SerializerMethodField() endorsed_by_label = serializers.SerializerMethodField() endorsed_at = serializers.SerializerMethodField() - children = serializers.SerializerMethodField() + children = serializers.SerializerMethodField(required=False) non_updatable_fields = NON_UPDATABLE_COMMENT_FIELDS diff --git a/lms/djangoapps/discussion_api/tests/test_api.py b/lms/djangoapps/discussion_api/tests/test_api.py index acb06ff2fc..ff85470852 100644 --- a/lms/djangoapps/discussion_api/tests/test_api.py +++ b/lms/djangoapps/discussion_api/tests/test_api.py @@ -1368,12 +1368,13 @@ class CreateThreadTest( @mock.patch("eventtracking.tracker.emit") def test_basic(self, mock_emit): - self.register_post_thread_response({ + cs_thread = make_minimal_cs_thread({ "id": "test_id", "username": self.user.username, "created_at": "2015-05-19T00:00:00Z", "updated_at": "2015-05-19T00:00:00Z", }) + self.register_post_thread_response(cs_thread) with self.assert_signal_sent(api, 'thread_created', sender=None, user=self.user, exclude_args=('post',)): actual = create_thread(self.request, self.minimal_data) expected = { @@ -1403,7 +1404,8 @@ class CreateThreadTest( "non_endorsed_comment_list_url": None, "editable_fields": ["abuse_flagged", "following", "raw_body", "title", "topic_id", "type", "voted"], 'read': False, - 'has_endorsed': False + 'has_endorsed': False, + 'response_count': 0, } self.assertEqual(actual, expected) self.assertEqual( @@ -1949,6 +1951,7 @@ class UpdateThreadTest( "editable_fields": ["abuse_flagged", "following", "raw_body", "title", "topic_id", "type", "voted"], 'read': False, 'has_endorsed': False, + 'response_count': 0 } self.assertEqual(actual, expected) self.assertEqual( diff --git a/lms/djangoapps/discussion_api/tests/test_serializers.py b/lms/djangoapps/discussion_api/tests/test_serializers.py index 7ed1087544..b299d89814 100644 --- a/lms/djangoapps/discussion_api/tests/test_serializers.py +++ b/lms/djangoapps/discussion_api/tests/test_serializers.py @@ -211,7 +211,6 @@ class ThreadSerializerSerializationTest(SerializerTestMixin, SharedModuleStoreTe "editable_fields": ["abuse_flagged", "following", "voted"], "read": False, "has_endorsed": False, - "response_count": None, } self.assertEqual(self.serialize(thread), expected) @@ -262,7 +261,7 @@ class ThreadSerializerSerializationTest(SerializerTestMixin, SharedModuleStoreTe del thread_data["resp_total"] self.register_get_thread_response(thread_data) serialized = self.serialize(Thread(id=thread_data["id"])) - self.assertIsNone(serialized["response_count"], None) + self.assertNotIn("response_count", serialized) @ddt.ddt diff --git a/lms/djangoapps/discussion_api/tests/test_views.py b/lms/djangoapps/discussion_api/tests/test_views.py index 27287a8c7a..acbb441785 100644 --- a/lms/djangoapps/discussion_api/tests/test_views.py +++ b/lms/djangoapps/discussion_api/tests/test_views.py @@ -400,12 +400,13 @@ class ThreadViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): def test_basic(self): self.register_get_user_response(self.user) - self.register_post_thread_response({ + cs_thread = make_minimal_cs_thread({ "id": "test_thread", "username": self.user.username, "created_at": "2015-05-19T00:00:00Z", "updated_at": "2015-05-19T00:00:00Z", }) + self.register_post_thread_response(cs_thread) request_data = { "course_id": unicode(self.course.id), "topic_id": "test_topic", @@ -441,6 +442,7 @@ class ThreadViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): "editable_fields": ["abuse_flagged", "following", "raw_body", "title", "topic_id", "type", "voted"], "read": False, "has_endorsed": False, + "response_count": 0, } response = self.client.post( self.url, @@ -536,6 +538,7 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest "editable_fields": ["abuse_flagged", "following", "raw_body", "title", "topic_id", "type", "voted"], "read": False, "has_endorsed": False, + "response_count": 0, } response = self.client.patch( # pylint: disable=no-member self.url, diff --git a/lms/djangoapps/discussion_api/tests/utils.py b/lms/djangoapps/discussion_api/tests/utils.py index 5e72528833..6a7da90aaa 100644 --- a/lms/djangoapps/discussion_api/tests/utils.py +++ b/lms/djangoapps/discussion_api/tests/utils.py @@ -329,9 +329,9 @@ def make_minimal_cs_thread(overrides=None): "comments_count": 0, "unread_comments_count": 0, "children": [], - "resp_total": 0, "read": False, "endorsed": False, + "resp_total": 0, } ret.update(overrides or {}) return ret diff --git a/lms/djangoapps/discussion_api/views.py b/lms/djangoapps/discussion_api/views.py index 7852a4abe8..8893abe9f8 100644 --- a/lms/djangoapps/discussion_api/views.py +++ b/lms/djangoapps/discussion_api/views.py @@ -229,6 +229,8 @@ class ThreadViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet): * has_endorsed: Boolean indicating whether this thread has been answered + * response_count: The number of direct responses for a thread + **DELETE response values: No content is returned for a DELETE request