From 54f6cbdec9a17dc66a35d33086689f34b5aee99a Mon Sep 17 00:00:00 2001 From: Jansen Kantor Date: Thu, 10 Sep 2020 12:34:57 -0400 Subject: [PATCH] EDUCATOR-5312: Learners unable to create new Teams if 0 teams currently exist (#24948) * reproduce * Prevent 404 for empty non-private teamset * add another teamset to teams view tests * don't return 404 for staff querying empty private teamset --- lms/djangoapps/teams/tests/test_views.py | 76 ++++++++++++++++++++++-- lms/djangoapps/teams/views.py | 21 ++++--- 2 files changed, 83 insertions(+), 14 deletions(-) diff --git a/lms/djangoapps/teams/tests/test_views.py b/lms/djangoapps/teams/tests/test_views.py index 34cb409be1..073b6b5cbf 100644 --- a/lms/djangoapps/teams/tests/test_views.py +++ b/lms/djangoapps/teams/tests/test_views.py @@ -327,6 +327,14 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase): 'type': u'private_managed' } ) + base_topics.append( + { + 'id': 'private_topic_no_teams', + 'name': 'private_topic_no_teams_name', + 'description': u'Description for topic private_topic_no_teams.', + 'type': u'private_managed' + } + ) teams_configuration_1 = TeamsConfig({ 'topics': base_topics, 'max_team_size': 5 @@ -1730,8 +1738,8 @@ class TestListTopicsAPI(TeamAPITestCase): ('student_inactive', 401, None), ('student_unenrolled', 403, None), ('student_enrolled', 200, 4), - ('staff', 200, 6), - ('course_staff', 200, 6), + ('staff', 200, 7), + ('course_staff', 200, 7), ('community_ta', 200, 4), ) @ddt.unpack @@ -1868,13 +1876,13 @@ class TestListTopicsAPI(TeamAPITestCase): ('student_on_team_1_private_set_1', 1), ('student_on_team_2_private_set_1', 1), ('student_masters', 0), - ('staff', 2) + ('staff', 3) ) def test_teamset_type(self, requesting_user, expected_private_teamsets): """ As different users, request course_1's list of topics, and see what private_managed teamsets are returned - Staff should be able to see both teamsets, and anyone enrolled in a private teamset should see that and + Staff should be able to see all teamsets, and anyone enrolled in a private teamset should see that and only that teamset """ topics = self.get_topics_list( @@ -2227,7 +2235,7 @@ class TestListMembershipAPI(TeamAPITestCase): ('student_masters', 404, None), ('staff', 200, {'student_on_team_1_private_set_1', 'student_on_team_2_private_set_1'}) ) - def test_access_filter_teamset(self, user, expected_response, expected_users): + def test_access_filter_teamset__private_teamset(self, user, expected_response, expected_users): memberships = self.get_membership_list( expected_response, { @@ -2240,6 +2248,64 @@ class TestListMembershipAPI(TeamAPITestCase): returned_users = {membership['user']['username'] for membership in memberships['results']} self.assertEqual(returned_users, expected_users) + @ddt.unpack + @ddt.data( + ('student_enrolled', 404), + ('student_on_team_1_private_set_1', 404), + ('student_on_team_2_private_set_1', 404), + ('student_masters', 404), + ('staff', 200) + ) + def test_access_filter_teamset__private_teamset__no_teams(self, user, expected_response): + """ + private_topic_no_teams has no teams in it, but staff should still get a 200 when + requesting teamset memberships + """ + self.get_membership_list( + expected_response, + { + 'teamset_id': 'private_topic_no_teams', + 'course_id': str(self.test_course_1.id), + }, + user=user + ) + + @ddt.unpack + @ddt.data( + ('student_unenrolled', 404, {}), + ('student_enrolled_not_on_team', 200, {'student_enrolled'}), + ('student_enrolled', 200, {'student_enrolled'}), + ('student_masters', 200, {'student_masters'}), + ('staff', 200, {'student_enrolled', 'student_masters'}) + ) + def test_access_filter_teamset__open_teamset(self, user, expected_response, expected_usernames): + # topic_3 has no teams + self.assertFalse(CourseTeam.objects.filter(topic_id='topic_3').exists()) + memberships = self.get_membership_list( + expected_response, + { + 'teamset_id': 'topic_3', + 'course_id': str(self.test_course_1.id), + }, + user=user + ) + if expected_response == 200: + self.assertEqual(memberships['count'], 0) + + # topic_0 has teams + self.assertTrue(CourseTeam.objects.filter(topic_id='topic_0').exists()) + memberships = self.get_membership_list( + expected_response, + { + 'teamset_id': 'topic_0', + 'course_id': str(self.test_course_1.id), + }, + user=user + ) + if expected_response == 200: + returned_users = {membership['user']['username'] for membership in memberships['results']} + self.assertEqual(returned_users, expected_usernames) + @ddt.ddt class TestCreateMembershipAPI(EventTestMixin, TeamAPITestCase): diff --git a/lms/djangoapps/teams/views.py b/lms/djangoapps/teams/views.py index c77521adee..dad93ed400 100644 --- a/lms/djangoapps/teams/views.py +++ b/lms/djangoapps/teams/views.py @@ -1360,15 +1360,18 @@ class MembershipListView(ExpandableFieldViewMixin, GenericAPIView): status=status.HTTP_404_NOT_FOUND ) teamset_teams = CourseTeam.objects.filter(course_id=requested_course_key, topic_id=teamset_id) - teams_with_access = [ - team for team in teamset_teams - if has_specific_team_access(request.user, team) - ] - if not teams_with_access: - return Response( - build_api_error(ugettext_noop("No teamset found in given course with given id")), - status=status.HTTP_404_NOT_FOUND - ) + if has_course_staff_privileges(request.user, requested_course_key): + teams_with_access = list(teamset_teams) + else: + teams_with_access = [ + team for team in teamset_teams + if has_specific_team_access(request.user, team) + ] + if teamset.is_private_managed and not teams_with_access: + return Response( + build_api_error(ugettext_noop("No teamset found in given course with given id")), + status=status.HTTP_404_NOT_FOUND + ) team_ids = [team.team_id for team in teams_with_access] if 'username' in request.query_params: