Merge pull request #13156 from edx/jia/MA-2675

MA-2675: Skip 'endorsed_by=null' in discussion requested_fields=profile_image

lms/djangoapps/discussion_api/api.py

@@ -379,7 +379,11 @@ def _get_users(discussion_entity_type, discussion_entity, username_profile_dict)
     if discussion_entity['author']:
         users[discussion_entity['author']] = _user_profile(username_profile_dict[discussion_entity['author']])
 
-    if discussion_entity_type == DiscussionEntity.comment and discussion_entity['endorsed']:
+    if (
+            discussion_entity_type == DiscussionEntity.comment
+            and discussion_entity['endorsed']
+            and discussion_entity['endorsed_by']
+    ):
         users[discussion_entity['endorsed_by']] = _user_profile(username_profile_dict[discussion_entity['endorsed_by']])
     return users
 
@@ -452,7 +456,8 @@ def _serialize_discussion_entities(request, context, discussion_entities, reques
                 usernames.append(serialized_entity['author'])
             if (
                     'endorsed' in serialized_entity and serialized_entity['endorsed'] and
-                    'endorsed_by' in serialized_entity and serialized_entity['endorsed_by'] not in usernames
+                    'endorsed_by' in serialized_entity and
+                    serialized_entity['endorsed_by'] and serialized_entity['endorsed_by'] not in usernames
             ):
                 usernames.append(serialized_entity['endorsed_by'])
 

lms/djangoapps/discussion_api/tests/test_views.py

@@ -1019,6 +1019,24 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pr
         self.thread_id = "test_thread"
         self.storage = get_profile_image_storage()
 
+    def create_source_comment(self, overrides=None):
+        """
+        Create a sample source cs_comment
+        """
+        comment = make_minimal_cs_comment({
+            "id": "test_comment",
+            "thread_id": self.thread_id,
+            "user_id": str(self.user.id),
+            "username": self.user.username,
+            "created_at": "2015-05-11T00:00:00Z",
+            "updated_at": "2015-05-11T11:11:11Z",
+            "body": "Test body",
+            "votes": {"up_count": 4},
+        })
+
+        comment.update(overrides or {})
+        return comment
+
     def make_minimal_cs_thread(self, overrides=None):
         """
         Create a thread with the given overrides, plus the course_id if not
@@ -1075,24 +1093,9 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pr
 
     def test_basic(self):
         self.register_get_user_response(self.user, upvoted_ids=["test_comment"])
-        source_comments = [{
-            "type": "comment",
-            "id": "test_comment",
-            "thread_id": self.thread_id,
-            "parent_id": None,
-            "user_id": str(self.author.id),
-            "username": self.author.username,
-            "anonymous": False,
-            "anonymous_to_peers": False,
-            "created_at": "2015-05-11T00:00:00Z",
-            "updated_at": "2015-05-11T11:11:11Z",
-            "body": "Test body",
-            "endorsed": False,
-            "abuse_flaggers": [],
-            "votes": {"up_count": 4},
-            "child_count": 0,
-            "children": [],
-        }]
+        source_comments = [
+            self.create_source_comment({"user_id": str(self.author.id), "username": self.author.username})
+        ]
         expected_comments = [self.expected_response_comment(overrides={
             "voted": True,
             "vote_count": 4,
@@ -1279,24 +1282,7 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pr
         """
         Tests all comments retrieved have user profile image details if called in requested_fields
         """
-        source_comments = [{
-            "type": "comment",
-            "id": "test_comment",
-            "thread_id": self.thread_id,
-            "parent_id": None,
-            "user_id": str(self.user.id),
-            "username": self.user.username,
-            "anonymous": False,
-            "anonymous_to_peers": False,
-            "created_at": "2015-05-11T00:00:00Z",
-            "updated_at": "2015-05-11T11:11:11Z",
-            "body": "Test body",
-            "endorsed": False,
-            "abuse_flaggers": [],
-            "votes": {"up_count": 4},
-            "child_count": 0,
-            "children": [],
-        }]
+        source_comments = [self.create_source_comment()]
         self.register_get_thread_response({
             "id": self.thread_id,
             "course_id": unicode(self.course.id),
@@ -1360,6 +1346,38 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pr
             self.assertEqual(expected_author_profile_data, response_users[response_comment['author']])
             self.assertEqual(expected_endorser_profile_data, response_users[response_comment['endorsed_by']])
 
+    def test_profile_image_request_for_null_endorsed_by(self):
+        """
+        Tests if 'endorsed' is True but 'endorsed_by' is null, the api does not crash.
+        This is the case for some old/stale data in prod/stage environments.
+        """
+        self.register_get_user_response(self.user)
+        thread = self.make_minimal_cs_thread({
+            "thread_type": "question",
+            "endorsed_responses": [make_minimal_cs_comment({
+                "id": "endorsed_comment",
+                "user_id": self.user.id,
+                "username": self.user.username,
+                "endorsed": True,
+            })],
+            "non_endorsed_resp_total": 0,
+        })
+        self.register_get_thread_response(thread)
+        self.create_profile_image(self.user, get_profile_image_storage())
+
+        response = self.client.get(self.url, {
+            "thread_id": thread["id"],
+            "endorsed": True,
+            "requested_fields": "profile_image",
+        })
+        self.assertEqual(response.status_code, 200)
+        response_comments = json.loads(response.content)['results']
+        for response_comment in response_comments:
+            expected_author_profile_data = self.get_expected_user_profile(response_comment['author'])
+            response_users = response_comment['users']
+            self.assertEqual(expected_author_profile_data, response_users[response_comment['author']])
+            self.assertNotIn(response_comment['endorsed_by'], response_users)
+
 
 @httpretty.activate
 @disable_signal(api, 'comment_deleted')