From 1d5fe0ed33cb23d52f867e071b1c1cdb72f2c22c Mon Sep 17 00:00:00 2001 From: wajeeha-khalid Date: Thu, 26 May 2016 11:52:47 +0500 Subject: [PATCH] MA-2419: make thread 'read' mutual exlusive to update fields --- lms/djangoapps/discussion_api/api.py | 11 +++++ lms/djangoapps/discussion_api/forms.py | 1 + lms/djangoapps/discussion_api/serializers.py | 2 +- .../discussion_api/tests/test_api.py | 1 - .../discussion_api/tests/test_serializers.py | 2 - .../discussion_api/tests/test_views.py | 40 ++----------------- lms/djangoapps/discussion_api/tests/utils.py | 12 ++++++ lms/lib/comment_client/user.py | 20 ++++++++++ 8 files changed, 48 insertions(+), 41 deletions(-) diff --git a/lms/djangoapps/discussion_api/api.py b/lms/djangoapps/discussion_api/api.py index 37646febac..6ebb8ce97c 100644 --- a/lms/djangoapps/discussion_api/api.py +++ b/lms/djangoapps/discussion_api/api.py @@ -514,6 +514,8 @@ def _do_extra_actions(api_content, cc_content, request_fields, actions_form, con _handle_abuse_flagged_field(form_value, context["cc_requester"], cc_content) elif field == "voted": _handle_voted_field(form_value, cc_content, api_content, request, context) + elif field == "read": + _handle_read_field(api_content, form_value, context["cc_requester"], cc_content) else: raise ValidationError({field: ["Invalid Key"]}) @@ -549,6 +551,15 @@ def _handle_voted_field(form_value, cc_content, api_content, request, context): ) +def _handle_read_field(api_content, form_value, user, cc_content): + """ + Marks thread as read for the user + """ + if form_value and not cc_content['read']: + user.read(cc_content) + api_content["unread_comment_count"] -= 1 + + def create_thread(request, thread_data): """ Create a thread. diff --git a/lms/djangoapps/discussion_api/forms.py b/lms/djangoapps/discussion_api/forms.py index ea9486bd3f..914bfbceae 100644 --- a/lms/djangoapps/discussion_api/forms.py +++ b/lms/djangoapps/discussion_api/forms.py @@ -98,6 +98,7 @@ class ThreadActionsForm(Form): following = BooleanField(required=False) voted = BooleanField(required=False) abuse_flagged = BooleanField(required=False) + read = BooleanField(required=False) class CommentListGetForm(_PaginationForm): diff --git a/lms/djangoapps/discussion_api/serializers.py b/lms/djangoapps/discussion_api/serializers.py index c88e9941e0..99016dedff 100644 --- a/lms/djangoapps/discussion_api/serializers.py +++ b/lms/djangoapps/discussion_api/serializers.py @@ -272,7 +272,7 @@ class ThreadSerializer(_ContentSerializer): def update(self, instance, validated_data): for key, val in validated_data.items(): instance[key] = val - instance.save(params={"requested_user_id": self.context["cc_requester"]["id"]}) + instance.save() return instance diff --git a/lms/djangoapps/discussion_api/tests/test_api.py b/lms/djangoapps/discussion_api/tests/test_api.py index de2bfa7644..df2d26f3e0 100644 --- a/lms/djangoapps/discussion_api/tests/test_api.py +++ b/lms/djangoapps/discussion_api/tests/test_api.py @@ -1992,7 +1992,6 @@ class UpdateThreadTest( "closed": ["False"], "pinned": ["False"], "read": ["False"], - "requested_user_id": [str(self.user.id)], } ) diff --git a/lms/djangoapps/discussion_api/tests/test_serializers.py b/lms/djangoapps/discussion_api/tests/test_serializers.py index 5b6c51ccc2..c359fbcc62 100644 --- a/lms/djangoapps/discussion_api/tests/test_serializers.py +++ b/lms/djangoapps/discussion_api/tests/test_serializers.py @@ -566,7 +566,6 @@ class ThreadSerializerDeserializationTest(CommentsServiceMockMixin, UrlResetMixi "pinned": ["False"], "user_id": [str(self.user.id)], "read": ["False"], - "requested_user_id": [str(self.user.id)], } ) @@ -595,7 +594,6 @@ class ThreadSerializerDeserializationTest(CommentsServiceMockMixin, UrlResetMixi "pinned": ["False"], "user_id": [str(self.user.id)], "read": [str(read)], - "requested_user_id": [str(self.user.id)], } ) for key in data: diff --git a/lms/djangoapps/discussion_api/tests/test_views.py b/lms/djangoapps/discussion_api/tests/test_views.py index 9b87c91fb8..a70b211bf7 100644 --- a/lms/djangoapps/discussion_api/tests/test_views.py +++ b/lms/djangoapps/discussion_api/tests/test_views.py @@ -730,7 +730,6 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest "closed": ["False"], "pinned": ["False"], "read": ["False"], - "requested_user_id": [str(self.user.id)], } ) @@ -787,7 +786,9 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest def test_patch_read_owner_user(self): self.register_get_user_response(self.user) self.register_thread() + self.register_read_response(self.user, "thread", "test_thread") request_data = {"read": True} + response = self.request_patch(request_data) self.assertEqual(response.status_code, 200) response_data = json.loads(response.content) @@ -802,30 +803,13 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest }) ) - self.assertEqual( - httpretty.last_request().parsed_body, - { - "course_id": [unicode(self.course.id)], - "commentable_id": ["original_topic"], - "thread_type": ["discussion"], - "title": ["Original Title"], - "body": ["Original body"], - "user_id": [str(self.user.id)], - "anonymous": ["False"], - "anonymous_to_peers": ["False"], - "closed": ["False"], - "pinned": ["False"], - "read": ["True"], - "requested_user_id": [str(self.user.id)], - } - ) - def test_patch_read_non_owner_user(self): self.register_get_user_response(self.user) thread_owner_user = UserFactory.create(password=self.password) CourseEnrollmentFactory.create(user=thread_owner_user, course_id=self.course.id) self.register_get_user_response(thread_owner_user) self.register_thread({"username": thread_owner_user.username, "user_id": str(thread_owner_user.id)}) + self.register_read_response(self.user, "thread", "test_thread") request_data = {"read": True} response = self.request_patch(request_data) @@ -843,24 +827,6 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest }) ) - self.assertEqual( - httpretty.last_request().parsed_body, - { - "course_id": [unicode(self.course.id)], - "commentable_id": ["original_topic"], - "thread_type": ["discussion"], - "title": ["Original Title"], - "body": ["Original body"], - "user_id": [str(thread_owner_user.id)], - "anonymous": ["False"], - "anonymous_to_peers": ["False"], - "closed": ["False"], - "pinned": ["False"], - "read": ["True"], - "requested_user_id": [str(self.user.id)], - } - ) - @httpretty.activate @disable_signal(api, 'thread_deleted') diff --git a/lms/djangoapps/discussion_api/tests/utils.py b/lms/djangoapps/discussion_api/tests/utils.py index e0f187fbf2..031982435b 100644 --- a/lms/djangoapps/discussion_api/tests/utils.py +++ b/lms/djangoapps/discussion_api/tests/utils.py @@ -260,6 +260,18 @@ class CommentsServiceMockMixin(object): status=200 ) + def register_read_response(self, user, content_type, content_id): + """ + Register a mock response for POST on the CS 'read' endpoint + """ + httpretty.register_uri( + httpretty.POST, + "http://localhost:4567/api/v1/users/{id}/read".format(id=user.id), + params={'source_type': content_type, 'source_id': content_id}, + body=json.dumps({}), # body is unused + status=200 + ) + def register_thread_flag_response(self, thread_id): """Register a mock response for PUT on the CS thread flag endpoints""" self.register_flag_response("thread", thread_id) diff --git a/lms/lib/comment_client/user.py b/lms/lib/comment_client/user.py index e88fe9ecd4..d43f563b57 100644 --- a/lms/lib/comment_client/user.py +++ b/lms/lib/comment_client/user.py @@ -30,6 +30,19 @@ class User(models.Model): external_id=str(user.id), username=user.username) + def read(self, source): + """ + Calls cs_comments_service to mark thread as read for the user + """ + params = {'source_type': source.type, 'source_id': source.id} + perform_request( + 'post', + _url_for_read(self.id), + params, + metric_action='user.read', + metric_tags=self._metric_tags + ['target.type:{}'.format(source.type)], + ) + def follow(self, source): params = {'source_type': source.type, 'source_id': source.id} response = perform_request( @@ -172,3 +185,10 @@ def _url_for_user_active_threads(user_id): def _url_for_user_subscribed_threads(user_id): return "{prefix}/users/{user_id}/subscribed_threads".format(prefix=settings.PREFIX, user_id=user_id) + + +def _url_for_read(user_id): + """ + Returns cs_comments_service url endpoint to mark thread as read for given user_id + """ + return "{prefix}/users/{user_id}/read".format(prefix=settings.PREFIX, user_id=user_id)