Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fixed permissions for voted #34993

Merged
merged 2 commits into from
Jun 26, 2024
Merged

fix: fixed permissions for voted #34993

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1e8729a
fix: fixed permissions for voted
ayesha-waris Jun 14, 2024
d0d846e
fix: fixed test cases
ayesha-waris Jun 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lms/djangoapps/discussion/rest_api/permissions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down Expand Up @@ -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,
Expand Down
106 changes: 51 additions & 55 deletions lms/djangoapps/discussion/rest_api/tests/test_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand All @@ -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
Expand All @@ -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]))
Expand All @@ -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")
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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]))
Expand All @@ -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")
Expand Down
16 changes: 11 additions & 5 deletions lms/djangoapps/discussion/rest_api/tests/test_permissions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
7 changes: 6 additions & 1 deletion lms/djangoapps/discussion/rest_api/tests/test_serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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'])
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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'],
Expand Down
Loading
Loading