fix: discussion moderators can't create posts for specific cohorts (#30352)
fix: discussion moderators can't create posts for specific cohorts
This commit is contained in:
@@ -179,7 +179,7 @@ class CourseDiscussionSettingsForm(Form):
|
||||
course_id = self.cleaned_data['course_id']
|
||||
try:
|
||||
course_key = CourseKey.from_string(course_id)
|
||||
self.cleaned_data['course'] = get_course_with_access(self.request_user, 'staff', course_key)
|
||||
self.cleaned_data['course'] = get_course_with_access(self.request_user, 'load', course_key)
|
||||
self.cleaned_data['course_key'] = course_key
|
||||
return course_id
|
||||
except InvalidKeyError:
|
||||
|
||||
@@ -13,10 +13,14 @@ from common.djangoapps.student.roles import (
|
||||
GlobalStaff,
|
||||
)
|
||||
from lms.djangoapps.discussion.django_comment_client.utils import (
|
||||
get_user_role_names,
|
||||
has_discussion_privileges,
|
||||
)
|
||||
from openedx.core.djangoapps.django_comment_common.comment_client.comment import Comment
|
||||
from openedx.core.djangoapps.django_comment_common.comment_client.thread import Thread
|
||||
from openedx.core.djangoapps.django_comment_common.models import (
|
||||
FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_MODERATOR
|
||||
)
|
||||
|
||||
|
||||
def _is_author(cc_content, context):
|
||||
@@ -156,3 +160,24 @@ class IsStaffOrCourseTeamOrEnrolled(permissions.BasePermission):
|
||||
CourseEnrollment.is_enrolled(request.user, course_key) or
|
||||
has_discussion_privileges(request.user, course_key)
|
||||
)
|
||||
|
||||
|
||||
class IsStaffOrAdmin(permissions.BasePermission):
|
||||
"""
|
||||
Permission that checks if the user is staff or an admin.
|
||||
"""
|
||||
|
||||
def has_permission(self, request, view):
|
||||
"""Returns true if the user is admin or staff and request method is GET."""
|
||||
course_key = CourseKey.from_string(view.kwargs.get('course_id'))
|
||||
user_roles = get_user_role_names(request.user, course_key)
|
||||
is_user_staff = bool(user_roles & {
|
||||
FORUM_ROLE_ADMINISTRATOR,
|
||||
FORUM_ROLE_MODERATOR,
|
||||
FORUM_ROLE_COMMUNITY_TA,
|
||||
})
|
||||
return (
|
||||
GlobalStaff().has_user(request.user) or
|
||||
request.user.is_staff or
|
||||
is_user_staff and request.method == "GET"
|
||||
)
|
||||
|
||||
@@ -2282,6 +2282,12 @@ class CourseDiscussionSettingsAPIViewTest(APITestCase, UrlResetMixin, ModuleStor
|
||||
"""Log the client in as the staff."""
|
||||
self.client.login(username=self.user.username, password=self.password)
|
||||
|
||||
def _login_as_discussion_staff(self):
|
||||
user = UserFactory(username='abc', password='abc')
|
||||
role = Role.objects.create(name='Administrator', course_id=self.course.id)
|
||||
role.users.set([user])
|
||||
self.client.login(username=user.username, password='abc')
|
||||
|
||||
def _create_divided_discussions(self):
|
||||
"""Create some divided discussions for testing."""
|
||||
divided_inline_discussions = ['Topic A', ]
|
||||
@@ -2373,6 +2379,46 @@ class CourseDiscussionSettingsAPIViewTest(APITestCase, UrlResetMixin, ModuleStor
|
||||
)
|
||||
assert response.status_code == 404
|
||||
|
||||
def test_patch_request_by_discussion_staff(self):
|
||||
"""Test the response when patch request is sent by a user with discussions staff role."""
|
||||
self._login_as_discussion_staff()
|
||||
response = self.patch_request(
|
||||
{'always_divide_inline_discussions': True}
|
||||
)
|
||||
assert response.status_code == 403
|
||||
|
||||
def test_get_request_by_discussion_staff(self):
|
||||
"""Test the response when get request is sent by a user with discussions staff role."""
|
||||
self._login_as_discussion_staff()
|
||||
divided_inline_discussions, divided_course_wide_discussions = self._create_divided_discussions()
|
||||
response = self.client.get(self.path)
|
||||
assert response.status_code == 200
|
||||
expected_response = self._get_expected_response()
|
||||
expected_response['divided_course_wide_discussions'] = [
|
||||
topic_name_to_id(self.course, name) for name in divided_course_wide_discussions
|
||||
]
|
||||
expected_response['divided_inline_discussions'] = [
|
||||
topic_name_to_id(self.course, name) for name in divided_inline_discussions
|
||||
]
|
||||
content = json.loads(response.content.decode('utf-8'))
|
||||
assert content == expected_response
|
||||
|
||||
def test_get_request_by_non_staff_user(self):
|
||||
"""Test the response when get request is sent by a regular user with no staff role."""
|
||||
user = UserFactory(username='abc', password='abc')
|
||||
self.client.login(username=user.username, password='abc')
|
||||
response = self.client.get(self.path)
|
||||
assert response.status_code == 403
|
||||
|
||||
def test_patch_request_by_non_staff_user(self):
|
||||
"""Test the response when patch request is sent by a regular user with no staff role."""
|
||||
user = UserFactory(username='abc', password='abc')
|
||||
self.client.login(username=user.username, password='abc')
|
||||
response = self.patch_request(
|
||||
{'always_divide_inline_discussions': True}
|
||||
)
|
||||
assert response.status_code == 403
|
||||
|
||||
def test_get_settings(self):
|
||||
"""Test the current discussion settings against the expected response."""
|
||||
divided_inline_discussions, divided_course_wide_discussions = self._create_divided_discussions()
|
||||
|
||||
@@ -63,7 +63,7 @@ from ..rest_api.forms import (
|
||||
UserCommentListGetForm,
|
||||
UserOrdering,
|
||||
)
|
||||
from ..rest_api.permissions import IsStaffOrCourseTeamOrEnrolled
|
||||
from ..rest_api.permissions import IsStaffOrAdmin, IsStaffOrCourseTeamOrEnrolled
|
||||
from ..rest_api.serializers import (
|
||||
CourseMetadataSerailizer,
|
||||
DiscussionRolesListSerializer,
|
||||
@@ -1096,7 +1096,7 @@ class CourseDiscussionSettingsAPIView(DeveloperErrorViewMixin, APIView):
|
||||
SessionAuthenticationAllowInactiveUser,
|
||||
)
|
||||
parser_classes = (JSONParser, MergePatchParser,)
|
||||
permission_classes = (permissions.IsAuthenticated, permissions.IsAdminUser)
|
||||
permission_classes = (permissions.IsAuthenticated, IsStaffOrAdmin)
|
||||
|
||||
def _get_request_kwargs(self, course_id):
|
||||
return dict(course_id=course_id)
|
||||
|
||||
33
openedx/core/djangoapps/course_groups/permissions.py
Normal file
33
openedx/core/djangoapps/course_groups/permissions.py
Normal file
@@ -0,0 +1,33 @@
|
||||
"""
|
||||
Permissions for cohorts API
|
||||
"""
|
||||
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
from rest_framework import permissions
|
||||
|
||||
from openedx.core.djangoapps.django_comment_common.models import (
|
||||
FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_MODERATOR
|
||||
)
|
||||
from common.djangoapps.student.roles import GlobalStaff
|
||||
from lms.djangoapps.discussion.django_comment_client.utils import get_user_role_names
|
||||
|
||||
|
||||
class IsStaffOrAdmin(permissions.BasePermission):
|
||||
"""
|
||||
Permission that checks if the user is staff or an admin.
|
||||
"""
|
||||
|
||||
def has_permission(self, request, view):
|
||||
"""Returns true if the user is admin or staff and request method is GET."""
|
||||
course_key = CourseKey.from_string(view.kwargs.get('course_key_string'))
|
||||
user_roles = get_user_role_names(request.user, course_key)
|
||||
is_user_staff = bool(user_roles & {
|
||||
FORUM_ROLE_ADMINISTRATOR,
|
||||
FORUM_ROLE_MODERATOR,
|
||||
FORUM_ROLE_COMMUNITY_TA,
|
||||
})
|
||||
return (
|
||||
GlobalStaff().has_user(request.user) or
|
||||
request.user.is_staff or
|
||||
is_user_staff and request.method == "GET"
|
||||
)
|
||||
@@ -27,6 +27,7 @@ from rest_framework.serializers import Serializer
|
||||
from lms.djangoapps.courseware.courses import get_course, get_course_with_access
|
||||
from common.djangoapps.edxmako.shortcuts import render_to_response
|
||||
from openedx.core.djangoapps.course_groups.models import CohortMembership
|
||||
from openedx.core.djangoapps.course_groups.permissions import IsStaffOrAdmin
|
||||
from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser
|
||||
from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin
|
||||
from common.djangoapps.student.auth import has_course_author_access
|
||||
@@ -431,7 +432,7 @@ class APIPermissions(GenericAPIView):
|
||||
BearerAuthenticationAllowInactiveUser,
|
||||
SessionAuthenticationAllowInactiveUser,
|
||||
)
|
||||
permission_classes = (permissions.IsAuthenticated, permissions.IsAdminUser)
|
||||
permission_classes = (permissions.IsAuthenticated, IsStaffOrAdmin)
|
||||
serializer_class = Serializer
|
||||
|
||||
|
||||
@@ -505,7 +506,7 @@ class CohortHandler(DeveloperErrorViewMixin, APIPermissions):
|
||||
"""
|
||||
Endpoint to get either one or all cohorts.
|
||||
"""
|
||||
course_key, course = _get_course_with_access(request, course_key_string)
|
||||
course_key, course = _get_course_with_access(request, course_key_string, 'load')
|
||||
if not cohort_id:
|
||||
all_cohorts = cohorts.get_course_cohorts(course)
|
||||
paginator = NamespacedPageNumberPagination()
|
||||
|
||||
Reference in New Issue
Block a user