fix: fixed permissions for voted (openedx#34993)

* fix: fixed permissions for voted

* fix: fixed test cases

lms/djangoapps/discussion/rest_api/permissions.py

@@ -90,6 +90,7 @@ def get_editable_fields(cc_content: Union[Thread, Comment], context: Dict) -> Se
     # For closed thread:
     # no edits, except 'abuse_flagged' and 'read' are allowed for thread
     # no edits, except 'abuse_flagged' is allowed for comment
+
     is_thread = cc_content["type"] == "thread"
     is_comment = cc_content["type"] == "comment"
     has_moderation_privilege = context["has_moderation_privilege"]
@@ -120,7 +121,7 @@ def get_editable_fields(cc_content: Union[Thread, Comment], context: Dict) -> Se
 
     is_author = _is_author(cc_content, context)
     editable_fields.update({
-        "voted": True,
+        "voted": has_moderation_privilege or not is_author or is_staff_or_admin,
         "raw_body": has_moderation_privilege or is_author,
         "edit_reason_code": has_moderation_privilege and not is_author,
         "following": is_thread,

lms/djangoapps/discussion/rest_api/tests/test_api.py

@@ -2128,19 +2128,6 @@ def test_following(self):
         assert cs_request.method == 'POST'
         assert parsed_body(cs_request) == {'source_type': ['thread'], 'source_id': ['test_id']}
 
-    def test_voted(self):
-        self.register_post_thread_response({"id": "test_id", "username": self.user.username})
-        self.register_thread_votes_response("test_id")
-        data = self.minimal_data.copy()
-        data["voted"] = "True"
-        with self.assert_signal_sent(api, 'thread_voted', sender=None, user=self.user, exclude_args=('post',)):
-            result = create_thread(self.request, data)
-        assert result['voted'] is True
-        cs_request = httpretty.last_request()
-        assert urlparse(cs_request.path).path == '/api/v1/threads/test_id/votes'  # lint-amnesty, pylint: disable=no-member
-        assert cs_request.method == 'PUT'
-        assert parsed_body(cs_request) == {'user_id': [str(self.user.id)], 'value': ['up']}
-
     def test_abuse_flagged(self):
         self.register_post_thread_response({"id": "test_id", "username": self.user.username})
         self.register_thread_flag_response("test_id")
@@ -2278,7 +2265,7 @@ def test_success(self, parent_id, mock_emit):
             "voted": False,
             "vote_count": 0,
             "children": [],
-            "editable_fields": ["abuse_flagged", "anonymous", "raw_body", "voted"],
+            "editable_fields": ["abuse_flagged", "anonymous", "raw_body"],
             "child_count": 0,
             "can_delete": True,
             "anonymous": False,
@@ -2354,7 +2341,7 @@ def test_success_in_black_out_with_user_access(self, parent_id, mock_emit):
             "abuse_flagged",
             "anonymous",
             "raw_body",
-            "voted",
+            "voted"
         ]
         if parent_id:
             data["parent_id"] = parent_id
@@ -2485,19 +2472,6 @@ def test_endorsed(self, role_name, is_thread_author, thread_type):
         except ValidationError:
             assert expected_error
 
-    def test_voted(self):
-        self.register_post_comment_response({"id": "test_comment", "username": self.user.username}, "test_thread")
-        self.register_comment_votes_response("test_comment")
-        data = self.minimal_data.copy()
-        data["voted"] = "True"
-        with self.assert_signal_sent(api, 'comment_voted', sender=None, user=self.user, exclude_args=('post',)):
-            result = create_comment(self.request, data)
-        assert result['voted'] is True
-        cs_request = httpretty.last_request()
-        assert urlparse(cs_request.path).path == '/api/v1/comments/test_comment/votes'  # lint-amnesty, pylint: disable=no-member
-        assert cs_request.method == 'PUT'
-        assert parsed_body(cs_request) == {'user_id': [str(self.user.id)], 'value': ['up']}
-
     def test_abuse_flagged(self):
         self.register_post_comment_response({"id": "test_comment", "username": self.user.username}, "test_thread")
         self.register_comment_flag_response("test_comment")
@@ -2642,6 +2616,17 @@ def register_thread(self, overrides=None):
         self.register_get_thread_response(cs_data)
         self.register_put_thread_response(cs_data)
 
+    def create_user_with_request(self):
+        """
+        Create a user and an associated request for a specific course enrollment.
+        """
+        user = UserFactory.create()
+        self.register_get_user_response(user)
+        request = RequestFactory().get("/test_path")
+        request.user = user
+        CourseEnrollmentFactory.create(user=user, course_id=self.course.id)
+        return user, request
+
     def test_empty(self):
         """Check that an empty update does not make any modifying requests."""
         # Ensure that the default following value of False is not applied implicitly
@@ -2813,12 +2798,15 @@ def test_voted(self, current_vote_status, new_vote_status, mock_emit):
         are the same, no update should be made. Otherwise, a vote should be PUT
         or DELETEd according to the new_vote_status value.
         """
+        #setup
+        user1, request1 = self.create_user_with_request()
+
         if current_vote_status:
-            self.register_get_user_response(self.user, upvoted_ids=["test_thread"])
+            self.register_get_user_response(user1, upvoted_ids=["test_thread"])
         self.register_thread_votes_response("test_thread")
         self.register_thread()
         data = {"voted": new_vote_status}
-        result = update_thread(self.request, "test_thread", data)
+        result = update_thread(request1, "test_thread", data)
         assert result['voted'] == new_vote_status
         last_request_path = urlparse(httpretty.last_request().path).path  # lint-amnesty, pylint: disable=no-member
         votes_url = "/api/v1/threads/test_thread/votes"
@@ -2832,7 +2820,7 @@ def test_voted(self, current_vote_status, new_vote_status, mock_emit):
                 parse_qs(urlparse(httpretty.last_request().path).query)  # lint-amnesty, pylint: disable=no-member
             )
             actual_request_data.pop("request_id", None)
-            expected_request_data = {"user_id": [str(self.user.id)]}
+            expected_request_data = {"user_id": [str(user1.id)]}
             if new_vote_status:
                 expected_request_data["value"] = ["up"]
             assert actual_request_data == expected_request_data
@@ -2858,21 +2846,22 @@ def test_vote_count(self, current_vote_status, first_vote, second_vote):
         """
         #setup
         starting_vote_count = 0
+        user, request = self.create_user_with_request()
         if current_vote_status:
-            self.register_get_user_response(self.user, upvoted_ids=["test_thread"])
+            self.register_get_user_response(user, upvoted_ids=["test_thread"])
             starting_vote_count = 1
         self.register_thread_votes_response("test_thread")
         self.register_thread(overrides={"votes": {"up_count": starting_vote_count}})
 
         #first vote
         data = {"voted": first_vote}
-        result = update_thread(self.request, "test_thread", data)
+        result = update_thread(request, "test_thread", data)
         self.register_thread(overrides={"voted": first_vote})
         assert result['vote_count'] == (1 if first_vote else 0)
 
         #second vote
         data = {"voted": second_vote}
-        result = update_thread(self.request, "test_thread", data)
+        result = update_thread(request, "test_thread", data)
         assert result['vote_count'] == (1 if second_vote else 0)
 
     @ddt.data(*itertools.product([True, False], [True, False], [True, False], [True, False]))
@@ -2888,22 +2877,19 @@ def test_vote_count_two_users(
         Tests vote_count increases and decreases correctly from different users
         """
         #setup
-        user2 = UserFactory.create()
-        self.register_get_user_response(user2)
-        request2 = RequestFactory().get("/test_path")
-        request2.user = user2
-        CourseEnrollmentFactory.create(user=user2, course_id=self.course.id)
+        user1, request1 = self.create_user_with_request()
+        user2, request2 = self.create_user_with_request()
 
         vote_count = 0
         if current_user1_vote:
-            self.register_get_user_response(self.user, upvoted_ids=["test_thread"])
+            self.register_get_user_response(user1, upvoted_ids=["test_thread"])
             vote_count += 1
         if current_user2_vote:
             self.register_get_user_response(user2, upvoted_ids=["test_thread"])
             vote_count += 1
 
         for (current_vote, user_vote, request) in \
-                [(current_user1_vote, user1_vote, self.request),
+                [(current_user1_vote, user1_vote, request1),
                  (current_user2_vote, user2_vote, request2)]:
 
             self.register_thread_votes_response("test_thread")
@@ -3202,6 +3188,17 @@ def register_comment(self, overrides=None, thread_overrides=None, course=None):
         self.register_get_comment_response(cs_comment_data)
         self.register_put_comment_response(cs_comment_data)
 
+    def create_user_with_request(self):
+        """
+        Create a user and an associated request for a specific course enrollment.
+        """
+        user = UserFactory.create()
+        self.register_get_user_response(user)
+        request = RequestFactory().get("/test_path")
+        request.user = user
+        CourseEnrollmentFactory.create(user=user, course_id=self.course.id)
+        return user, request
+
     def test_empty(self):
         """Check that an empty update does not make any modifying requests."""
         self.register_comment()
@@ -3235,7 +3232,7 @@ def test_basic(self, parent_id):
             "voted": False,
             "vote_count": 0,
             "children": [],
-            "editable_fields": ["abuse_flagged", "anonymous", "raw_body", "voted"],
+            "editable_fields": ["abuse_flagged", "anonymous", "raw_body"],
             "child_count": 0,
             "can_delete": True,
             "last_edit": None,
@@ -3394,13 +3391,14 @@ def test_voted(self, current_vote_status, new_vote_status, mock_emit):
         or DELETEd according to the new_vote_status value.
         """
         vote_count = 0
+        user1, request1 = self.create_user_with_request()
         if current_vote_status:
-            self.register_get_user_response(self.user, upvoted_ids=["test_comment"])
+            self.register_get_user_response(user1, upvoted_ids=["test_comment"])
             vote_count = 1
         self.register_comment_votes_response("test_comment")
         self.register_comment(overrides={"votes": {"up_count": vote_count}})
         data = {"voted": new_vote_status}
-        result = update_comment(self.request, "test_comment", data)
+        result = update_comment(request1, "test_comment", data)
         assert result['vote_count'] == (1 if new_vote_status else 0)
         assert result['voted'] == new_vote_status
         last_request_path = urlparse(httpretty.last_request().path).path  # lint-amnesty, pylint: disable=no-member
@@ -3415,7 +3413,7 @@ def test_voted(self, current_vote_status, new_vote_status, mock_emit):
                 parse_qs(urlparse(httpretty.last_request().path).query)  # lint-amnesty, pylint: disable=no-member
             )
             actual_request_data.pop("request_id", None)
-            expected_request_data = {"user_id": [str(self.user.id)]}
+            expected_request_data = {"user_id": [str(user1.id)]}
             if new_vote_status:
                 expected_request_data["value"] = ["up"]
             assert actual_request_data == expected_request_data
@@ -3442,21 +3440,22 @@ def test_vote_count(self, current_vote_status, first_vote, second_vote):
         """
         #setup
         starting_vote_count = 0
+        user1, request1 = self.create_user_with_request()
         if current_vote_status:
-            self.register_get_user_response(self.user, upvoted_ids=["test_comment"])
+            self.register_get_user_response(user1, upvoted_ids=["test_comment"])
             starting_vote_count = 1
         self.register_comment_votes_response("test_comment")
         self.register_comment(overrides={"votes": {"up_count": starting_vote_count}})
 
         #first vote
         data = {"voted": first_vote}
-        result = update_comment(self.request, "test_comment", data)
+        result = update_comment(request1, "test_comment", data)
         self.register_comment(overrides={"voted": first_vote})
         assert result['vote_count'] == (1 if first_vote else 0)
 
         #second vote
         data = {"voted": second_vote}
-        result = update_comment(self.request, "test_comment", data)
+        result = update_comment(request1, "test_comment", data)
         assert result['vote_count'] == (1 if second_vote else 0)
 
     @ddt.data(*itertools.product([True, False], [True, False], [True, False], [True, False]))
@@ -3471,22 +3470,19 @@ def test_vote_count_two_users(
         """
         Tests vote_count increases and decreases correctly from different users
         """
-        user2 = UserFactory.create()
-        self.register_get_user_response(user2)
-        request2 = RequestFactory().get("/test_path")
-        request2.user = user2
-        CourseEnrollmentFactory.create(user=user2, course_id=self.course.id)
+        user1, request1 = self.create_user_with_request()
+        user2, request2 = self.create_user_with_request()
 
         vote_count = 0
         if current_user1_vote:
-            self.register_get_user_response(self.user, upvoted_ids=["test_comment"])
+            self.register_get_user_response(user1, upvoted_ids=["test_comment"])
             vote_count += 1
         if current_user2_vote:
             self.register_get_user_response(user2, upvoted_ids=["test_comment"])
             vote_count += 1
 
         for (current_vote, user_vote, request) in \
-                [(current_user1_vote, user1_vote, self.request),
+                [(current_user1_vote, user1_vote, request1),
                  (current_user2_vote, user2_vote, request2)]:
 
             self.register_comment_votes_response("test_comment")

lms/djangoapps/discussion/rest_api/tests/test_permissions.py

@@ -66,10 +66,10 @@ def test_thread(
         actual = get_initializable_thread_fields(context)
         expected = {
             "abuse_flagged", "copy_link", "course_id", "following", "raw_body",
-            "read", "title", "topic_id", "type", "voted"
+            "read", "title", "topic_id", "type"
         }
         if is_privileged:
-            expected |= {"closed", "pinned", "close_reason_code"}
+            expected |= {"closed", "pinned", "close_reason_code", "voted"}
         if is_privileged and is_cohorted:
             expected |= {"group_id"}
         if allow_anonymous:
@@ -88,8 +88,10 @@ def test_comment(self, is_thread_author, thread_type, is_privileged):
         )
         actual = get_initializable_comment_fields(context)
         expected = {
-            "anonymous", "abuse_flagged", "parent_id", "raw_body", "thread_id", "voted"
+            "anonymous", "abuse_flagged", "parent_id", "raw_body", "thread_id"
         }
+        if is_privileged:
+            expected |= {"voted"}
         if (is_thread_author and thread_type == "question") or is_privileged:
             expected |= {"endorsed"}
         assert actual == expected
@@ -119,11 +121,13 @@ def test_thread(
             is_staff_or_admin=is_staff_or_admin,
         )
         actual = get_editable_fields(thread, context)
-        expected = {"abuse_flagged", "copy_link", "following", "read", "voted"}
+        expected = {"abuse_flagged", "copy_link", "following", "read"}
         if has_moderation_privilege:
             expected |= {"closed", "close_reason_code"}
         if has_moderation_privilege or is_staff_or_admin:
             expected |= {"pinned"}
+        if has_moderation_privilege or not is_author or is_staff_or_admin:
+            expected |= {"voted"}
         if has_moderation_privilege and not is_author:
             expected |= {"edit_reason_code"}
         if is_author or has_moderation_privilege:
@@ -162,7 +166,9 @@ def test_comment(
             has_moderation_privilege=has_moderation_privilege,
         )
         actual = get_editable_fields(comment, context)
-        expected = {"abuse_flagged", "voted"}
+        expected = {"abuse_flagged"}
+        if has_moderation_privilege or not is_author:
+            expected |= {"voted"}
         if has_moderation_privilege and not is_author:
             expected |= {"edit_reason_code"}
         if is_author or has_moderation_privilege:

lms/djangoapps/discussion/rest_api/tests/test_serializers.py

@@ -143,6 +143,7 @@ def test_voted(self):
 @ddt.ddt
 class ThreadSerializerSerializationTest(SerializerTestMixin, SharedModuleStoreTestCase):
     """Tests for ThreadSerializer serialization."""
+
     def make_cs_content(self, overrides):
         """
         Create a thread with the given overrides, plus some useful test data.
@@ -279,6 +280,7 @@ def test_closed_by_label_field(self, role, visible):
         can_delete = role != FORUM_ROLE_STUDENT
         editable_fields = ["abuse_flagged", "copy_link", "following", "read", "voted"]
         if role == "author":
+            editable_fields.remove("voted")
             editable_fields.extend(['anonymous', 'raw_body', 'title', 'topic_id', 'type'])
         elif role == FORUM_ROLE_MODERATOR:
             editable_fields.extend(['close_reason_code', 'closed', 'edit_reason_code', 'pinned',
@@ -335,7 +337,9 @@ def test_edit_by_label_field(self, role, visible):
         editable_fields = ["abuse_flagged", "copy_link", "following", "read", "voted"]
 
         if role == "author":
+            editable_fields.remove("voted")
             editable_fields.extend(['anonymous', 'raw_body', 'title', 'topic_id', 'type'])
+
         elif role == FORUM_ROLE_MODERATOR:
             editable_fields.extend(['close_reason_code', 'closed', 'edit_reason_code', 'pinned',
                                     'raw_body', 'title', 'topic_id', 'type'])
@@ -375,6 +379,7 @@ def test_get_preview_body(self):
 @ddt.ddt
 class CommentSerializerTest(SerializerTestMixin, SharedModuleStoreTestCase):
     """Tests for CommentSerializer."""
+
     def setUp(self):
         super().setUp()
         self.endorser = UserFactory.create()
@@ -610,7 +615,7 @@ def test_create_minimal(self):
         self.register_post_thread_response({"id": "test_id", "username": self.user.username})
         saved = self.save_and_reserialize(self.minimal_data)
         assert urlparse(httpretty.last_request().path).path ==\
-               '/api/v1/test_topic/threads'  # lint-amnesty, pylint: disable=no-member
+            '/api/v1/test_topic/threads'  # lint-amnesty, pylint: disable=no-member
         assert parsed_body(httpretty.last_request()) == {
             'course_id': [str(self.course.id)],
             'commentable_id': ['test_topic'],