Merge pull request #23983 from edx/aakbar/PROD-1362
fix admin unable to delete course team
This commit is contained in:
@@ -325,7 +325,8 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase):
|
||||
cls.topics_count = 6
|
||||
cls.users = {
|
||||
'staff': AdminFactory.create(password=cls.test_password),
|
||||
'course_staff': StaffFactory.create(course_key=cls.test_course_1.id, password=cls.test_password)
|
||||
'course_staff': StaffFactory.create(course_key=cls.test_course_1.id, password=cls.test_password),
|
||||
'admin': AdminFactory.create(password=cls.test_password)
|
||||
}
|
||||
cls.create_and_enroll_student(username='student_enrolled')
|
||||
cls.create_and_enroll_student(username='student_on_team_1_private_set_1', mode=CourseMode.MASTERS)
|
||||
@@ -1339,28 +1340,54 @@ class TestDeleteTeamAPI(EventTestMixin, TeamAPITestCase):
|
||||
super(TestDeleteTeamAPI, self).setUp('lms.djangoapps.teams.utils.tracker')
|
||||
|
||||
@ddt.data(
|
||||
(None, 401),
|
||||
('student_inactive', 401),
|
||||
('student_unenrolled', 403),
|
||||
('student_enrolled', 403),
|
||||
('staff', 204),
|
||||
('course_staff', 204),
|
||||
('community_ta', 204)
|
||||
('community_ta', 204),
|
||||
('admin', 204)
|
||||
)
|
||||
@ddt.unpack
|
||||
def test_access(self, user, status):
|
||||
team_list = self.get_teams_list(user='course_staff', expected_status=200)
|
||||
previous_count = team_list['count']
|
||||
self.assertIn(self.solar_team.team_id, [result['id'] for result in team_list.get('results')])
|
||||
self.delete_team(self.solar_team.team_id, status, user=user)
|
||||
|
||||
team_list = self.get_teams_list(user='course_staff', expected_status=200)
|
||||
self.assertEqual(team_list['count'], previous_count - 1)
|
||||
self.assertNotIn(self.solar_team.team_id, [result['id'] for result in team_list.get('results')])
|
||||
self.assert_event_emitted(
|
||||
'edx.team.deleted',
|
||||
team_id=self.solar_team.team_id,
|
||||
)
|
||||
self.assert_event_emitted(
|
||||
'edx.team.learner_removed',
|
||||
team_id=self.solar_team.team_id,
|
||||
remove_method='team_deleted',
|
||||
user_id=self.users['student_enrolled'].id
|
||||
)
|
||||
|
||||
@ddt.data(
|
||||
('student_unenrolled', 403),
|
||||
('student_enrolled', 403),
|
||||
)
|
||||
@ddt.unpack
|
||||
def test_access_forbidden(self, user, status):
|
||||
team_list = self.get_teams_list(user='course_staff', expected_status=200)
|
||||
previous_count = team_list['count']
|
||||
self.assertIn(self.solar_team.team_id, [result['id'] for result in team_list.get('results')])
|
||||
self.delete_team(self.solar_team.team_id, status, user=user)
|
||||
|
||||
team_list = self.get_teams_list(user='course_staff', expected_status=200)
|
||||
self.assertEqual(team_list['count'], previous_count)
|
||||
self.assertIn(self.solar_team.team_id, [result['id'] for result in team_list.get('results')])
|
||||
|
||||
@ddt.data(
|
||||
(None, 401),
|
||||
('student_inactive', 401),
|
||||
)
|
||||
@ddt.unpack
|
||||
def test_access_unauthorized(self, user, status):
|
||||
self.delete_team(self.solar_team.team_id, status, user=user)
|
||||
if status == 204:
|
||||
self.assert_event_emitted(
|
||||
'edx.team.deleted',
|
||||
team_id=self.solar_team.team_id,
|
||||
)
|
||||
self.assert_event_emitted(
|
||||
'edx.team.learner_removed',
|
||||
team_id=self.solar_team.team_id,
|
||||
remove_method='team_deleted',
|
||||
user_id=self.users['student_enrolled'].id
|
||||
)
|
||||
|
||||
def test_does_not_exist(self):
|
||||
self.delete_team('nonexistent', 404)
|
||||
|
||||
@@ -34,7 +34,7 @@ from lms.djangoapps.discussion.django_comment_client.utils import has_discussion
|
||||
from lms.djangoapps.teams.models import CourseTeam, CourseTeamMembership
|
||||
from openedx.core.lib.teams_config import TeamsetType
|
||||
from openedx.core.lib.api.parsers import MergePatchParser
|
||||
from openedx.core.lib.api.permissions import IsStaffOrReadOnly
|
||||
from openedx.core.lib.api.permissions import IsCourseStaffInstructor, IsStaffOrReadOnly
|
||||
from openedx.core.lib.api.view_utils import (
|
||||
ExpandableFieldViewMixin,
|
||||
RetrievePatchAPIView,
|
||||
@@ -664,13 +664,14 @@ class IsEnrolledOrIsStaff(permissions.BasePermission):
|
||||
class IsStaffOrPrivilegedOrReadOnly(IsStaffOrReadOnly):
|
||||
"""
|
||||
Permission that checks to see if the user is global staff, course
|
||||
staff, or has discussion privileges. If none of those conditions are
|
||||
staff, course admin, or has discussion privileges. If none of those conditions are
|
||||
met, only read access will be granted.
|
||||
"""
|
||||
|
||||
def has_object_permission(self, request, view, obj):
|
||||
return (
|
||||
has_discussion_privileges(request.user, obj.course_id) or
|
||||
IsCourseStaffInstructor.has_object_permission(self, request, view, obj) or
|
||||
super(IsStaffOrPrivilegedOrReadOnly, self).has_object_permission(request, view, obj)
|
||||
)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user