Merge pull request #12568 from edx/jia/MA-2419

MA-2419: make thread 'read' mutual exlusive to update fields

lms/djangoapps/discussion_api/api.py

@@ -739,6 +739,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"]})
 
@@ -774,6 +776,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.

lms/djangoapps/discussion_api/forms.py

@@ -99,6 +99,7 @@ class ThreadActionsForm(Form):
     following = BooleanField(required=False)
     voted = BooleanField(required=False)
     abuse_flagged = BooleanField(required=False)
+    read = BooleanField(required=False)
 
 
 class CommentListGetForm(_PaginationForm):

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
 
 

lms/djangoapps/discussion_api/tests/test_api.py

@@ -2042,7 +2042,6 @@ class UpdateThreadTest(
                 "closed": ["False"],
                 "pinned": ["False"],
                 "read": ["False"],
-                "requested_user_id": [str(self.user.id)],
             }
         )
 

lms/djangoapps/discussion_api/tests/test_serializers.py

@@ -568,7 +568,6 @@ class ThreadSerializerDeserializationTest(CommentsServiceMockMixin, UrlResetMixi
                 "pinned": ["False"],
                 "user_id": [str(self.user.id)],
                 "read": ["False"],
-                "requested_user_id": [str(self.user.id)],
             }
         )
 
@@ -597,7 +596,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:

lms/djangoapps/discussion_api/tests/test_views.py

@@ -886,7 +886,6 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest
                 "closed": ["False"],
                 "pinned": ["False"],
                 "read": ["False"],
-                "requested_user_id": [str(self.user.id)],
             }
         )
 
@@ -943,7 +942,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)
@@ -958,30 +959,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)
@@ -999,24 +983,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')

lms/djangoapps/discussion_api/tests/utils.py

@@ -269,6 +269,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)

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)