Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: update editable action item for different discussion and course roles

Browse Source
This commit is contained in:
SaadYousaf
2022-08-31 19:11:16 +05:00
committed by Saad Yousaf
parent bdb0e4d8ad
commit b8e466b55c
8 changed files with 107 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
lms/djangoapps/discussion/rest_api/api.py
View File

@@ -198,12 +198,12 @@ def _get_thread_and_context(request, thread_id, retrieve_kwargs=None):
course = _get_course(course_key, request.user)
context = get_context(course, request, cc_thread)
if retrieve_kwargs.get("flagged_comments") and not context["is_requester_privileged"]:
if retrieve_kwargs.get("flagged_comments") and not context["has_moderation_privilege"]:
raise ValidationError("Only privileged users can request flagged comments")
course_discussion_settings = CourseDiscussionSettings.get(course_key)
if (
not context["is_requester_privileged"] and
not context["has_moderation_privilege"] and
cc_thread["group_id"] and
is_commentable_divided(course.id, cc_thread["commentable_id"], course_discussion_settings)
):
@@ -241,7 +241,7 @@ def _is_user_author_or_privileged(cc_content, context):
Boolean
"""
return (
context["is_requester_privileged"] or
context["has_moderation_privilege"] or
context["cc_requester"]["id"] == cc_content["user_id"]
)
@@ -819,7 +819,7 @@ def get_thread_list(
"text_search_rewrite": None,
})
if count_flagged and not context["is_requester_privileged"]:
if count_flagged and not context["has_moderation_privilege"]:
raise PermissionDenied("`count_flagged` can only be set by users with moderator access or higher.")
group_id = None
@@ -836,7 +836,7 @@ def get_thread_list(
except ValueError:
pass
if (group_id is None) and (not context["is_requester_privileged"]):
if (group_id is None) and not context["has_moderation_privilege"]:
group_id = get_group_id_for_user(request.user, CourseDiscussionSettings.get(course.id))
query_params = {
@@ -1559,7 +1559,7 @@ def get_user_comments(
course = _get_course(course_key, request.user)
context = get_context(course, request)
if flagged and not context["is_requester_privileged"]:
if flagged and not context["has_moderation_privilege"]:
raise ValidationError("Only privileged users can filter comments by flagged status")
try:

24
lms/djangoapps/discussion/rest_api/permissions.py
View File

@@ -35,7 +35,7 @@ def _is_author_or_privileged(cc_content, context):
Return True if the requester authored the given content or is a privileged
user, False otherwise
"""
return context["is_requester_privileged"] or _is_author(cc_content, context)
return context["has_moderation_privilege"] or _is_author(cc_content, context)
NON_UPDATABLE_THREAD_FIELDS = {"course_id"}
@@ -92,7 +92,7 @@ def get_editable_fields(cc_content: Union[Thread, Comment], context: Dict) -> Se
# no edits, except 'abuse_flagged' is allowed for comment
is_thread = cc_content["type"] == "thread"
is_comment = cc_content["type"] == "comment"
is_privileged = context["is_requester_privileged"]
has_moderation_privilege = context["has_moderation_privilege"]
if is_thread:
is_thread_closed = cc_content["closed"]
@@ -105,9 +105,9 @@ def get_editable_fields(cc_content: Union[Thread, Comment], context: Dict) -> Se
# Map each field to the condition in which it's editable.
editable_fields = {
"abuse_flagged": True,
"closed": is_thread and is_privileged,
"close_reason_code": is_thread and is_privileged,
"pinned": is_thread and is_privileged,
"closed": is_thread and has_moderation_privilege,
"close_reason_code": is_thread and has_moderation_privilege,
"pinned": is_thread and has_moderation_privilege,
"read": is_thread,
}
if is_thread:
@@ -120,16 +120,16 @@ def get_editable_fields(cc_content: Union[Thread, Comment], context: Dict) -> Se
is_author = _is_author(cc_content, context)
editable_fields.update({
"voted": True,
"raw_body": is_privileged or is_author,
"edit_reason_code": is_privileged,
"raw_body": has_moderation_privilege or is_author,
"edit_reason_code": has_moderation_privilege and not is_author,
"following": is_thread,
"topic_id": is_thread and (is_author or is_privileged),
"type": is_thread and (is_author or is_privileged),
"title": is_thread and (is_author or is_privileged),
"group_id": is_thread and is_privileged and context["discussion_division_enabled"],
"topic_id": is_thread and (is_author or has_moderation_privilege),
"type": is_thread and (is_author or has_moderation_privilege),
"title": is_thread and (is_author or has_moderation_privilege),
"group_id": is_thread and has_moderation_privilege and context["discussion_division_enabled"],
"endorsed": (
(is_comment and cc_content.get("parent_id", None) is None) and
(is_privileged or
(has_moderation_privilege or
(_is_author(context["thread"], context) and context["thread"]["thread_type"] == "question"))
),
"anonymous": is_author and context["course"].allow_anonymous,

33
lms/djangoapps/discussion/rest_api/serializers.py
View File

@@ -14,6 +14,7 @@ from django.utils.text import Truncator
from rest_framework import serializers
from common.djangoapps.student.models import get_user_by_username_or_email
from common.djangoapps.student.roles import GlobalStaff
from lms.djangoapps.discussion.django_comment_client.base.views import track_thread_lock_unlock_event
from lms.djangoapps.discussion.django_comment_client.utils import (
course_discussion_division_enabled,
@@ -28,7 +29,11 @@ from lms.djangoapps.discussion.rest_api.permissions import (
get_editable_fields,
)
from lms.djangoapps.discussion.rest_api.render import render_body
from lms.djangoapps.discussion.rest_api.utils import get_course_staff_users_list, get_course_ta_users_list
from lms.djangoapps.discussion.rest_api.utils import (
get_course_staff_users_list,
get_moderator_users_list,
get_course_ta_users_list,
)
from openedx.core.djangoapps.discussions.models import DiscussionTopicLink
from openedx.core.djangoapps.discussions.utils import get_group_names_by_id
from openedx.core.djangoapps.django_comment_common.comment_client.comment import Comment
@@ -37,7 +42,6 @@ from openedx.core.djangoapps.django_comment_common.comment_client.user import Us
from openedx.core.djangoapps.django_comment_common.comment_client.utils import CommentClientRequestError
from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings
from openedx.core.lib.api.serializers import CourseKeyField
from common.djangoapps.student.roles import GlobalStaff
User = get_user_model()
@@ -59,23 +63,26 @@ def get_context(course, request, thread=None):
Returns a context appropriate for use with ThreadSerializer or
(if thread is provided) CommentSerializer.
"""
staff_user_ids = get_course_staff_users_list(course.id)
course_staff_user_ids = get_course_staff_users_list(course.id)
moderator_user_ids = get_moderator_users_list(course.id)
ta_user_ids = get_course_ta_users_list(course.id)
requester = request.user
cc_requester = CommentClientUser.from_django_user(requester).retrieve()
cc_requester["course_id"] = course.id
course_discussion_settings = CourseDiscussionSettings.get(course.id)
is_global_staff = GlobalStaff().has_user(requester)
has_moderation_privilege = requester.id in moderator_user_ids or requester.id in ta_user_ids or is_global_staff
return {
"course": course,
"request": request,
"thread": thread,
"discussion_division_enabled": course_discussion_division_enabled(course_discussion_settings),
"group_ids_to_names": get_group_names_by_id(course_discussion_settings),
"is_requester_privileged": requester.id in staff_user_ids or requester.id in ta_user_ids or is_global_staff,
"staff_user_ids": staff_user_ids,
"moderator_user_ids": moderator_user_ids,
"course_staff_user_ids": course_staff_user_ids,
"ta_user_ids": ta_user_ids,
"cc_requester": cc_requester,
"has_moderation_privilege": has_moderation_privilege,
}
@@ -123,7 +130,7 @@ def _validate_privileged_access(context: Dict) -> bool:
bool: Course exists and the user has privileged access.
"""
course = context.get('course', None)
is_requester_privileged = context.get('is_requester_privileged')
is_requester_privileged = context.get('has_moderation_privilege')
return course and is_requester_privileged
@@ -169,7 +176,7 @@ class _ContentSerializer(serializers.Serializer):
Returns a boolean indicating whether the given user_id identifies a
privileged user.
"""
return user_id in self.context["staff_user_ids"] or user_id in self.context["ta_user_ids"]
return user_id in self.context["moderator_user_ids"] or user_id in self.context["ta_user_ids"]
def _is_anonymous(self, obj):
"""
@@ -177,7 +184,8 @@ class _ContentSerializer(serializers.Serializer):
the requester.
"""
user_id = self.context["request"].user.id
is_user_staff = user_id in self.context["staff_user_ids"]
is_user_staff = user_id in self.context["moderator_user_ids"] or user_id in self.context["ta_user_ids"]
return (
obj["anonymous"] or
obj["anonymous_to_peers"] and not is_user_staff
@@ -194,9 +202,12 @@ class _ContentSerializer(serializers.Serializer):
Returns the role label (i.e. "Staff" or "Community TA") for the user
with the given id.
"""
is_staff = user_id in self.context["course_staff_user_ids"] or user_id in self.context["moderator_user_ids"]
is_ta = user_id in self.context["ta_user_ids"]
return (
"Staff" if user_id in self.context["staff_user_ids"] else
"Community TA" if user_id in self.context["ta_user_ids"] else
"Staff" if is_staff else
"Community TA" if is_ta else
None
)
@@ -225,7 +236,7 @@ class _ContentSerializer(serializers.Serializer):
"""
total_abuse_flaggers = len(obj.get("abuse_flaggers", []))
return (
self.context["is_requester_privileged"] and total_abuse_flaggers > 0 or
self.context["has_moderation_privilege"] and total_abuse_flaggers > 0 or
self.context["cc_requester"]["id"] in obj.get("abuse_flaggers", [])
)

11
lms/djangoapps/discussion/rest_api/tests/test_api.py
View File

@@ -1894,7 +1894,6 @@ class CreateThreadTest(
"close_reason_code",
"closed",
"copy_link",
"edit_reason_code",
"following",
"pinned",
"raw_body",
@@ -2249,14 +2248,13 @@ class CreateCommentTest(
editable_fields = [
"abuse_flagged",
"anonymous",
"edit_reason_code",
"raw_body",
"voted",
]
if parent_id:
data["parent_id"] = parent_id
else:
editable_fields.insert(3, "endorsed")
editable_fields.insert(2, "endorsed")
_set_course_discussion_blackout(course=self.course, user_id=self.user.id)
_assign_role_to_user(user=self.user, course_id=self.course.id, role=FORUM_ROLE_MODERATOR)
@@ -2871,7 +2869,7 @@ class UpdateThreadTest(
Test editing comments, specifying and retrieving edit reason codes.
"""
_assign_role_to_user(user=self.user, course_id=self.course.id, role=role_name)
self.register_thread()
self.register_thread({"user_id": str(self.user.id + 1)})
try:
result = update_thread(self.request, "test_thread", {
"raw_body": "Edited body",
@@ -2888,7 +2886,8 @@ class UpdateThreadTest(
assert request_body["edit_reason_code"] == ["test-edit-reason"]
except ValidationError as error:
assert role_name == FORUM_ROLE_STUDENT
assert error.message_dict == {"edit_reason_code": ["This field is not editable."]}
assert error.message_dict == {"edit_reason_code": ["This field is not editable."],
"raw_body": ["This field is not editable."]}
@ddt.data(
*itertools.product(
@@ -3371,7 +3370,7 @@ class UpdateCommentTest(
Test editing comments, specifying and retrieving edit reason codes.
"""
_assign_role_to_user(user=self.user, course_id=self.course.id, role=role_name)
self.register_comment()
self.register_comment({"user_id": str(self.user.id + 1)})
try:
result = update_comment(self.request, "test_comment", {
"raw_body": "Edited body",

47
lms/djangoapps/discussion/rest_api/tests/test_permissions.py
View File

@@ -22,16 +22,15 @@ from openedx.core.djangoapps.django_comment_common.comment_client.user import Us
def _get_context(
requester_id,
is_requester_privileged,
is_cohorted=False,
thread=None,
allow_anonymous=True,
allow_anonymous_to_peers=False,
has_moderation_privilege=False,
):
"""Return a context suitable for testing the permissions module"""
return {
"cc_requester": User(id=requester_id),
"is_requester_privileged": is_requester_privileged,
"course": CourseFactory(
cohort_config={"cohorted": is_cohorted},
allow_anonymous=allow_anonymous,
@@ -39,6 +38,7 @@ def _get_context(
),
"discussion_division_enabled": is_cohorted,
"thread": thread,
"has_moderation_privilege": has_moderation_privilege,
}
@@ -56,7 +56,7 @@ class GetInitializableFieldsTest(ModuleStoreTestCase):
):
context = _get_context(
requester_id="5",
is_requester_privileged=is_privileged,
has_moderation_privilege=is_privileged,
is_cohorted=is_cohorted,
allow_anonymous=allow_anonymous,
allow_anonymous_to_peers=allow_anonymous_to_peers,
@@ -67,7 +67,7 @@ class GetInitializableFieldsTest(ModuleStoreTestCase):
"read", "title", "topic_id", "type", "voted"
}
if is_privileged:
expected |= {"closed", "pinned", "close_reason_code", "edit_reason_code"}
expected |= {"closed", "pinned", "close_reason_code"}
if is_privileged and is_cohorted:
expected |= {"group_id"}
if allow_anonymous:
@@ -81,15 +81,13 @@ class GetInitializableFieldsTest(ModuleStoreTestCase):
def test_comment(self, is_thread_author, thread_type, is_privileged):
context = _get_context(
requester_id="5",
is_requester_privileged=is_privileged,
has_moderation_privilege=is_privileged,
thread=Thread(user_id="5" if is_thread_author else "6", thread_type=thread_type)
)
actual = get_initializable_comment_fields(context)
expected = {
"anonymous", "abuse_flagged", "parent_id", "raw_body", "thread_id", "voted"
}
if is_privileged:
expected |= {"edit_reason_code"}
if (is_thread_author and thread_type == "question") or is_privileged:
expected |= {"endorsed"}
assert actual == expected
@@ -103,31 +101,34 @@ class GetEditableFieldsTest(ModuleStoreTestCase):
def test_thread(
self,
is_author,
is_privileged,
is_cohorted,
allow_anonymous,
allow_anonymous_to_peers
allow_anonymous_to_peers,
has_moderation_privilege,
):
thread = Thread(user_id="5" if is_author else "6", type="thread")
context = _get_context(
requester_id="5",
is_requester_privileged=is_privileged,
is_cohorted=is_cohorted,
allow_anonymous=allow_anonymous,
allow_anonymous_to_peers=allow_anonymous_to_peers,
has_moderation_privilege=has_moderation_privilege,
)
actual = get_editable_fields(thread, context)
expected = {"abuse_flagged", "copy_link", "following", "read", "voted"}
if is_privileged:
expected |= {"closed", "pinned", "close_reason_code", "edit_reason_code"}
if is_author or is_privileged:
expected |= {"topic_id", "type", "title", "raw_body"}
if is_privileged and is_cohorted:
if has_moderation_privilege:
expected |= {"closed", "pinned", "close_reason_code"}
if has_moderation_privilege and not is_author:
expected |= {"edit_reason_code"}
if is_author or has_moderation_privilege:
expected |= {"raw_body", "topic_id", "type", "title"}
if has_moderation_privilege and is_cohorted:
expected |= {"group_id"}
if is_author and allow_anonymous:
expected |= {"anonymous"}
if is_author and allow_anonymous_to_peers:
expected |= {"anonymous_to_peers"}
assert actual == expected
@ddt.data(*itertools.product(*[[True, False] for _ in range(6)], ["question", "discussion"]))
@@ -136,11 +137,11 @@ class GetEditableFieldsTest(ModuleStoreTestCase):
self,
is_author,
is_thread_author,
is_privileged,
allow_anonymous,
allow_anonymous_to_peers,
has_parent,
thread_type
has_moderation_privilege,
thread_type,
):
comment = Comment(
user_id="5" if is_author else "6",
@@ -149,18 +150,18 @@ class GetEditableFieldsTest(ModuleStoreTestCase):
)
context = _get_context(
requester_id="5",
is_requester_privileged=is_privileged,
thread=Thread(user_id="5" if is_thread_author else "6", thread_type=thread_type),
allow_anonymous=allow_anonymous,
allow_anonymous_to_peers=allow_anonymous_to_peers,
has_moderation_privilege=has_moderation_privilege,
)
actual = get_editable_fields(comment, context)
expected = {"abuse_flagged", "voted"}
if is_privileged:
if has_moderation_privilege and not is_author:
expected |= {"edit_reason_code"}
if is_author or is_privileged:
if is_author or has_moderation_privilege:
expected |= {"raw_body"}
if not has_parent and ((is_thread_author and thread_type == "question") or is_privileged):
if not has_parent and ((is_thread_author and thread_type == "question") or has_moderation_privilege):
expected |= {"endorsed"}
if is_author and allow_anonymous:
expected |= {"anonymous"}
@@ -176,7 +177,7 @@ class CanDeleteTest(ModuleStoreTestCase):
@ddt.unpack
def test_thread(self, is_author, is_privileged):
thread = Thread(user_id="5" if is_author else "6")
context = _get_context(requester_id="5", is_requester_privileged=is_privileged)
context = _get_context(requester_id="5", has_moderation_privilege=is_privileged)
assert can_delete(thread, context) == (is_author or is_privileged)
@ddt.data(*itertools.product([True, False], [True, False], [True, False]))
@@ -185,7 +186,7 @@ class CanDeleteTest(ModuleStoreTestCase):
comment = Comment(user_id="5" if is_author else "6")
context = _get_context(
requester_id="5",
is_requester_privileged=is_privileged,
has_moderation_privilege=is_privileged,
thread=Thread(user_id="5" if is_thread_author else "6")
)
assert can_delete(comment, context) == (is_author or is_privileged)

2
lms/djangoapps/discussion/rest_api/tests/test_serializers.py
View File

@@ -73,7 +73,7 @@ class SerializerTestMixin(ForumsEnableMixin, CommentsServiceMockMixin, UrlResetM
(FORUM_ROLE_MODERATOR, True, False, True),
(FORUM_ROLE_MODERATOR, False, True, False),
(FORUM_ROLE_COMMUNITY_TA, True, False, True),
(FORUM_ROLE_COMMUNITY_TA, False, True, True),
(FORUM_ROLE_COMMUNITY_TA, False, True, False),
(FORUM_ROLE_STUDENT, True, False, True),
(FORUM_ROLE_STUDENT, False, True, True),
)

11
lms/djangoapps/discussion/rest_api/tests/test_utils.py
View File

@@ -14,8 +14,9 @@ from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, U
from lms.djangoapps.discussion.django_comment_client.tests.factories import RoleFactory
from lms.djangoapps.discussion.rest_api.utils import (
discussion_open_for_user,
get_course_staff_users_list,
get_course_ta_users_list,
get_course_staff_users_list,
get_moderator_users_list,
)
@@ -62,7 +63,11 @@ class DiscussionAPIUtilsTestCase(ModuleStoreTestCase):
self.assertTrue(discussion_open_for_user(self.course, self.community_ta))
def test_course_staff_users_list(self):
assert len(get_course_staff_users_list(self.course.id)) == 3
assert len(get_course_staff_users_list(self.course.id)) == 2
def test_course_moderator_users_list(self):
assert len(get_moderator_users_list(self.course.id)) == 1
def test_course_ta_users_list(self):
assert len(get_course_ta_users_list(self.course.id)) == 2
ta_user_list = get_course_ta_users_list(self.course.id)
assert len(ta_user_list) == 2

48
lms/djangoapps/discussion/rest_api/utils.py
View File

@@ -118,22 +118,15 @@ def add_stats_for_users_with_no_discussion_content(course_stats, users_in_course
def get_course_staff_users_list(course_id):
"""
Gets user ids for Staff roles for course discussions.
Roles include Discussion Administrator, Discussion Moderator, Course Instructor and Course Staff.
Roles Course Instructor and Course Staff.
"""
# TODO: cache staff_user_ids if we need to improve perf
staff_user_ids = [
user.id
for role in Role.objects.filter(
name__in=[FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR],
course_id=course_id
)
for user in role.users.all()
]
# TODO: cache course_staff_user_ids if we need to improve perf
course_staff_user_ids = []
staff = list(CourseStaffRole(course_id).users_with_role().values_list('id', flat=True))
admins = list(CourseInstructorRole(course_id).users_with_role().values_list('id', flat=True))
staff_user_ids.extend(staff)
staff_user_ids.extend(admins)
return set(staff_user_ids)
course_staff_user_ids.extend(staff)
course_staff_user_ids.extend(admins)
return list(set(course_staff_user_ids))
def get_course_ta_users_list(course_id):
@@ -141,11 +134,28 @@ def get_course_ta_users_list(course_id):
Gets user ids for TA roles for course discussions.
Roles include Community TA and Group Community TA.
"""
# TODO: cache ta_user_ids if we need to improve perf
ta_user_ids = {
# TODO: cache ta_users_ids if we need to improve perf
ta_users_ids = [
user.id
for role in Role.objects.filter(name__in=[FORUM_ROLE_COMMUNITY_TA,
FORUM_ROLE_GROUP_MODERATOR], course_id=course_id)
for role in Role.objects.filter(name__in=[FORUM_ROLE_GROUP_MODERATOR,
FORUM_ROLE_COMMUNITY_TA], course_id=course_id)
for user in role.users.all()
}
return ta_user_ids
]
return ta_users_ids
def get_moderator_users_list(course_id):
"""
Gets user ids for Moderator roles for course discussions.
Roles include Discussion Administrator and Discussion Moderator.
"""
# TODO: cache moderator_user_ids if we need to improve perf
moderator_user_ids = [
user.id
for role in Role.objects.filter(
name__in=[FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR],
course_id=course_id
)
for user in role.users.all()
]
return moderator_user_ids