From 32d4b2dae604cb61e329619847f0e2a59f3ac559 Mon Sep 17 00:00:00 2001
From: Ali-D-Akbar <aliakbarit019@gmail.com>
Date: Wed, 13 May 2020 15:29:36 +0500
Subject: [PATCH] fix admin unable to delete course team
modify delete team unit test for admin
test improvements
test improvements
test improvements
test improvements
add team unauthorized access test
add team unauthorized and forbidden access test
---
lms/djangoapps/teams/tests/test_views.py | 61 +++++++++++++++++-------
lms/djangoapps/teams/views.py | 5 +-
2 files changed, 47 insertions(+), 19 deletions(-)
diff --git a/lms/djangoapps/teams/tests/test_views.py b/lms/djangoapps/teams/tests/test_views.py
index 20aedd435b5..49548820a04 100644
--- a/lms/djangoapps/teams/tests/test_views.py
+++ b/lms/djangoapps/teams/tests/test_views.py
@@ -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)
@@ -1293,28 +1294,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)
diff --git a/lms/djangoapps/teams/views.py b/lms/djangoapps/teams/views.py
index fe2f9fa9054..ed0a895035e 100644
--- a/lms/djangoapps/teams/views.py
+++ b/lms/djangoapps/teams/views.py
@@ -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,
@@ -663,13 +663,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)
)
--
GitLab