From 3f453e892de91b8ba133545e47a301ee4263cb74 Mon Sep 17 00:00:00 2001 From: wajeeha-khalid Date: Thu, 10 Sep 2015 17:42:58 +0500 Subject: [PATCH] added response count in thread MA-1212 - added response count field in tests removed unneccessary function "MA-1212 - added response count in thread retrieval service and updated tests" updated serializerz test for response count MA-1212 exclude response count from thread get list removed response_count field from update and create thread services made resp_total dynamic for cs thread in test_serielizer quality fixed improved quality add TODO JIRA ticket on removed fields --- lms/djangoapps/discussion_api/api.py | 7 ++++--- lms/djangoapps/discussion_api/serializers.py | 8 ++++++++ lms/djangoapps/discussion_api/tests/test_api.py | 12 ++++++++---- .../discussion_api/tests/test_serializers.py | 15 ++++++++++++--- lms/djangoapps/discussion_api/tests/test_views.py | 9 +++++---- 5 files changed, 37 insertions(+), 14 deletions(-) diff --git a/lms/djangoapps/discussion_api/api.py b/lms/djangoapps/discussion_api/api.py index b16d85216c..d8153ff486 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, context=context).data for thread in threads] + results = [ThreadSerializer(thread, remove_fields=['response_count'], 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, context=context) + serializer = ThreadSerializer(data=thread_data, remove_fields=['response_count'], 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,7 +642,8 @@ 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, data=update_data, partial=True, context=context) + serializer = ThreadSerializer(cc_thread, remove_fields=['response_count'], 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 f0c414274b..f6b9d36d0a 100644 --- a/lms/djangoapps/discussion_api/serializers.py +++ b/lms/djangoapps/discussion_api/serializers.py @@ -187,10 +187,13 @@ class ThreadSerializer(_ContentSerializer): non_endorsed_comment_list_url = serializers.SerializerMethodField("get_non_endorsed_comment_list_url") 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) 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) # type is an invalid class attribute name, so we must declare a # different name above and modify it here @@ -200,6 +203,11 @@ class ThreadSerializer(_ContentSerializer): if self.object and self.object.get("pinned") is None: self.object["pinned"] = False + if remove_fields: + # for multiple fields in a list + for field_name in remove_fields: + self.fields.pop(field_name) + def get_group_name(self, obj): """Returns the name of the group identified by the thread's group_id.""" return self.context["group_ids_to_names"].get(obj["group_id"]) diff --git a/lms/djangoapps/discussion_api/tests/test_api.py b/lms/djangoapps/discussion_api/tests/test_api.py index 0549af17ad..a2df7abd36 100644 --- a/lms/djangoapps/discussion_api/tests/test_api.py +++ b/lms/djangoapps/discussion_api/tests/test_api.py @@ -2967,7 +2967,9 @@ class RetrieveThreadTest( "title": "Test Title", "body": "Test body", "created_at": "2015-05-29T00:00:00Z", - "updated_at": "2015-05-29T00:00:00Z" + "updated_at": "2015-05-29T00:00:00Z", + "resp_total": 0, + }) cs_data.update(overrides or {}) self.register_get_thread_response(cs_data) @@ -3000,9 +3002,10 @@ class RetrieveThreadTest( "read": False, "has_endorsed": False, "id": "test_thread", - "type": "discussion" + "type": "discussion", + "response_count": 2, } - self.register_thread() + self.register_thread({"resp_total": 2}) self.assertEqual(get_thread(self.request, self.thread_id), expected_response_data) self.assertEqual(httpretty.last_request().method, "GET") @@ -3039,7 +3042,8 @@ class RetrieveThreadTest( "read": False, "has_endorsed": False, "id": "test_thread", - "type": "discussion" + "type": "discussion", + "response_count": 0, } non_author_user = UserFactory.create() # pylint: disable=attribute-defined-outside-init self.register_get_user_response(non_author_user) diff --git a/lms/djangoapps/discussion_api/tests/test_serializers.py b/lms/djangoapps/discussion_api/tests/test_serializers.py index b46149a8ee..6d090d3965 100644 --- a/lms/djangoapps/discussion_api/tests/test_serializers.py +++ b/lms/djangoapps/discussion_api/tests/test_serializers.py @@ -144,7 +144,8 @@ class ThreadSerializerSerializationTest(SerializerTestMixin, SharedModuleStoreTe "user_id": str(self.author.id), "username": self.author.username, "read": True, - "endorsed": True + "endorsed": True, + "resp_total": 0, } merged_overrides.update(overrides) return make_minimal_cs_thread(merged_overrides) @@ -179,7 +180,8 @@ class ThreadSerializerSerializationTest(SerializerTestMixin, SharedModuleStoreTe "comments_count": 5, "unread_comments_count": 3, "read": False, - "endorsed": False + "endorsed": False, + "response_count": None, } expected = { "id": "test_thread", @@ -208,7 +210,8 @@ class ThreadSerializerSerializationTest(SerializerTestMixin, SharedModuleStoreTe "non_endorsed_comment_list_url": None, "editable_fields": ["abuse_flagged", "following", "voted"], "read": False, - "has_endorsed": False + "has_endorsed": False, + "response_count": None, } self.assertEqual(self.serialize(thread), expected) @@ -248,6 +251,12 @@ class ThreadSerializerSerializationTest(SerializerTestMixin, SharedModuleStoreTe serialized = self.serialize(self.make_cs_content({"id": thread_id})) self.assertEqual(serialized["following"], True) + def test_response_count(self): + thread_data = self.make_cs_content({"resp_total": 2}) + self.register_get_thread_response(thread_data) + serialized = self.serialize(Thread(id=thread_data["id"])) + self.assertEqual(serialized["response_count"], 2) + @ddt.ddt class CommentSerializerTest(SerializerTestMixin, SharedModuleStoreTestCase): diff --git a/lms/djangoapps/discussion_api/tests/test_views.py b/lms/djangoapps/discussion_api/tests/test_views.py index 960ce4f638..ce1df0a85a 100644 --- a/lms/djangoapps/discussion_api/tests/test_views.py +++ b/lms/djangoapps/discussion_api/tests/test_views.py @@ -215,7 +215,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): "non_endorsed_comment_list_url": None, "editable_fields": ["abuse_flagged", "following", "voted"], "read": False, - "has_endorsed": False + "has_endorsed": False, }] self.register_get_threads_response(source_threads, page=1, num_pages=2) response = self.client.get(self.url, {"course_id": unicode(self.course.id)}) @@ -436,7 +436,7 @@ class ThreadViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): "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 = self.client.post( self.url, @@ -530,7 +530,7 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest "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 = self.client.patch( # pylint: disable=no-member self.url, @@ -1001,7 +1001,8 @@ class ThreadViewSetRetrieveTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase) "read": False, "has_endorsed": False, "id": "test_thread", - "type": "discussion" + "type": "discussion", + "response_count": 0, } self.register_get_thread_response(cs_thread) response = self.client.get(self.url)