Merge pull request #13954 from edx/jia/MA-2554

MA-2554: DiscussionAPI - fixed unread_comment_count

lms/djangoapps/discussion_api/serializers.py

@@ -76,7 +76,9 @@ def validate_not_blank(value):
 
 
 class _ContentSerializer(serializers.Serializer):
-    """A base class for thread and comment serializers."""
+    """
+    A base class for thread and comment serializers.
+    """
     id = serializers.CharField(read_only=True)  # pylint: disable=invalid-name
     author = serializers.SerializerMethodField()
     author_label = serializers.SerializerMethodField()
@@ -121,7 +123,9 @@ class _ContentSerializer(serializers.Serializer):
         )
 
     def get_author(self, obj):
-        """Returns the author's username, or None if the content is anonymous."""
+        """
+        Returns the author's username, or None if the content is anonymous.
+        """
         return None if self._is_anonymous(obj) else obj["username"]
 
     def _get_user_label(self, user_id):
@@ -136,7 +140,9 @@ class _ContentSerializer(serializers.Serializer):
         )
 
     def get_author_label(self, obj):
-        """Returns the role label for the content author."""
+        """
+        Returns the role label for the content author.
+        """
         if self._is_anonymous(obj) or obj["user_id"] is None:
             return None
         else:
@@ -144,7 +150,9 @@ class _ContentSerializer(serializers.Serializer):
             return self._get_user_label(user_id)
 
     def get_rendered_body(self, obj):
-        """Returns the rendered body content."""
+        """
+        Returns the rendered body content.
+        """
         return render_body(obj["body"])
 
     def get_abuse_flagged(self, obj):
@@ -162,11 +170,15 @@ class _ContentSerializer(serializers.Serializer):
         return obj["id"] in self.context["cc_requester"]["upvoted_ids"]
 
     def get_vote_count(self, obj):
-        """Returns the number of votes for the content."""
+        """
+        Returns the number of votes for the content.
+        """
         return obj.get("votes", {}).get("up_count", 0)
 
     def get_editable_fields(self, obj):
-        """Return the list of the fields the requester can edit"""
+        """
+        Return the list of the fields the requester can edit
+        """
         return sorted(get_editable_fields(obj, self.context))
 
 
@@ -196,7 +208,7 @@ class ThreadSerializer(_ContentSerializer):
     endorsed_comment_list_url = serializers.SerializerMethodField()
     non_endorsed_comment_list_url = serializers.SerializerMethodField()
     read = serializers.BooleanField(required=False)
-    has_endorsed = serializers.BooleanField(read_only=True, source="endorsed")
+    has_endorsed = serializers.BooleanField(source="endorsed", read_only=True)
     response_count = serializers.IntegerField(source="resp_total", read_only=True, required=False)
 
     non_updatable_fields = NON_UPDATABLE_THREAD_FIELDS
@@ -216,7 +228,9 @@ class ThreadSerializer(_ContentSerializer):
         return bool(obj["pinned"])
 
     def get_group_name(self, obj):
-        """Returns the name of the group identified by the thread's group_id."""
+        """
+        Returns the name of the group identified by the thread's group_id.
+        """
         return self.context["group_ids_to_names"].get(obj["group_id"])
 
     def get_following(self, obj):
@@ -245,22 +259,31 @@ class ThreadSerializer(_ContentSerializer):
         )
 
     def get_endorsed_comment_list_url(self, obj):
-        """Returns the URL to retrieve the thread's endorsed comments."""
+        """
+        Returns the URL to retrieve the thread's endorsed comments.
+        """
         return self.get_comment_list_url(obj, endorsed=True)
 
     def get_non_endorsed_comment_list_url(self, obj):
-        """Returns the URL to retrieve the thread's non-endorsed comments."""
+        """
+        Returns the URL to retrieve the thread's non-endorsed comments.
+        """
         return self.get_comment_list_url(obj, endorsed=False)
 
     def get_comment_count(self, obj):
-        """Increments comment count to include post and returns total count of
-        contributions (i.e. post + responses + comments) for the thread"""
+        """
+        Increments comment count to include post and returns total count of
+        contributions (i.e. post + responses + comments) for the thread
+        """
         return obj["comments_count"] + 1
 
     def get_unread_comment_count(self, obj):
-        """Increments comment count to include post if thread is unread and returns
-        total count of unread contributions (i.e. post + responses + comments) for the thread"""
-        if not obj["read"]:
+        """
+        Returns the number of unread comments. If the thread has never been read,
+        this additionally includes 1 for the post itself, in addition to its responses and
+        comments.
+        """
+        if not obj["read"] and obj["comments_count"] == obj["unread_comments_count"]:
             return obj["unread_comments_count"] + 1
         return obj["unread_comments_count"]
 
@@ -333,7 +356,9 @@ class CommentSerializer(_ContentSerializer):
             return None
 
     def get_endorsed_at(self, obj):
-        """Returns the timestamp for the endorsement, if available."""
+        """
+        Returns the timestamp for the endorsement, if available.
+        """
         endorsement = obj.get("endorsement")
         return endorsement["time"] if endorsement else None
 

lms/djangoapps/discussion_api/tests/test_serializers.py

@@ -161,60 +161,27 @@ class ThreadSerializerSerializationTest(SerializerTestMixin, SharedModuleStoreTe
         return ThreadSerializer(thread, context=get_context(self.course, self.request)).data
 
     def test_basic(self):
-        thread = {
-            "type": "thread",
+        thread = make_minimal_cs_thread({
             "id": "test_thread",
             "course_id": unicode(self.course.id),
             "commentable_id": "test_topic",
-            "group_id": None,
             "user_id": str(self.author.id),
             "username": self.author.username,
-            "anonymous": False,
-            "anonymous_to_peers": False,
-            "created_at": "2015-04-28T00:00:00Z",
-            "updated_at": "2015-04-28T11:11:11Z",
-            "thread_type": "discussion",
             "title": "Test Title",
             "body": "Test body",
             "pinned": True,
-            "closed": False,
-            "abuse_flaggers": [],
             "votes": {"up_count": 4},
             "comments_count": 5,
             "unread_comments_count": 3,
-            "read": False,
-            "endorsed": False,
-            "response_count": None,
-        }
-        expected = {
-            "id": "test_thread",
-            "course_id": unicode(self.course.id),
-            "topic_id": "test_topic",
-            "group_id": None,
-            "group_name": None,
+        })
+        expected = self.expected_thread_data({
             "author": self.author.username,
-            "author_label": None,
-            "created_at": "2015-04-28T00:00:00Z",
-            "updated_at": "2015-04-28T11:11:11Z",
-            "type": "discussion",
-            "title": "Test Title",
-            "raw_body": "Test body",
-            "rendered_body": "<p>Test body</p>",
-            "pinned": True,
-            "closed": False,
-            "following": False,
-            "abuse_flagged": False,
-            "voted": False,
             "vote_count": 4,
             "comment_count": 6,
-            "unread_comment_count": 4,
-            "comment_list_url": "http://testserver/api/discussion/v1/comments/?thread_id=test_thread",
-            "endorsed_comment_list_url": None,
-            "non_endorsed_comment_list_url": None,
+            "unread_comment_count": 3,
+            "pinned": True,
             "editable_fields": ["abuse_flagged", "following", "read", "voted"],
-            "read": False,
-            "has_endorsed": False,
-        }
+        })
         self.assertEqual(self.serialize(thread), expected)
 
         thread["thread_type"] = "question"

lms/djangoapps/discussion_api/tests/utils.py

@@ -335,6 +335,43 @@ class CommentsServiceMockMixin(object):
             content_type="application/merge-patch+json"
         )
 
+    def expected_thread_data(self, overrides=None):
+        """
+        Returns expected thread data in API response
+        """
+        response_data = {
+            "author": self.user.username,
+            "author_label": None,
+            "created_at": "1970-01-01T00:00:00Z",
+            "updated_at": "1970-01-01T00:00:00Z",
+            "raw_body": "Test body",
+            "rendered_body": "<p>Test body</p>",
+            "abuse_flagged": False,
+            "voted": False,
+            "vote_count": 0,
+            "editable_fields": ["abuse_flagged", "following", "raw_body", "read", "title", "topic_id", "type", "voted"],
+            "course_id": unicode(self.course.id),
+            "topic_id": "test_topic",
+            "group_id": None,
+            "group_name": None,
+            "title": "Test Title",
+            "pinned": False,
+            "closed": False,
+            "following": False,
+            "comment_count": 1,
+            "unread_comment_count": 0,
+            "comment_list_url": "http://testserver/api/discussion/v1/comments/?thread_id=test_thread",
+            "endorsed_comment_list_url": None,
+            "non_endorsed_comment_list_url": None,
+            "read": False,
+            "has_endorsed": False,
+            "id": "test_thread",
+            "type": "discussion",
+            "response_count": 0,
+        }
+        response_data.update(overrides or {})
+        return response_data
+
 
 def make_minimal_cs_thread(overrides=None):
     """