diff --git a/lms/djangoapps/teams/tests/test_views.py b/lms/djangoapps/teams/tests/test_views.py index 2ea39ee66a..791ae3bad3 100644 --- a/lms/djangoapps/teams/tests/test_views.py +++ b/lms/djangoapps/teams/tests/test_views.py @@ -1491,19 +1491,19 @@ class TestListTopicsAPI(TeamAPITestCase): """Test cases for the topic listing endpoint.""" @ddt.data( - (None, 401), - ('student_inactive', 401), - ('student_unenrolled', 403), - ('student_enrolled', 200), - ('staff', 200), - ('course_staff', 200), - ('community_ta', 200), + (None, 401, None), + ('student_inactive', 401, None), + ('student_unenrolled', 403, None), + ('student_enrolled', 200, 4), + ('staff', 200, 6), + ('course_staff', 200, 6), + ('community_ta', 200, 4), ) @ddt.unpack - def test_access(self, user, status): + def test_access(self, user, status, expected_topics_count): topics = self.get_topics_list(status, {'course_id': str(self.test_course_1.id)}, user=user) if status == 200: - self.assertEqual(topics['count'], self.topics_count) + self.assertEqual(topics['count'], expected_topics_count) @ddt.data('A+BOGUS+COURSE', 'A/BOGUS/COURSE') def test_invalid_course_key(self, course_id): @@ -1513,14 +1513,11 @@ class TestListTopicsAPI(TeamAPITestCase): self.get_topics_list(400) @ddt.data( - (None, 200, ['Coal Power', 'Nuclear Power', 'private_topic_1_name', 'private_topic_2_name', - u'Sólar power', 'Wind Power'], 'name'), - ('name', 200, ['Coal Power', 'Nuclear Power', 'private_topic_1_name', 'private_topic_2_name', - u'Sólar power', 'Wind Power'], 'name'), + (None, 200, ['Coal Power', 'Nuclear Power', u'Sólar power', 'Wind Power'], 'name'), + ('name', 200, ['Coal Power', 'Nuclear Power', u'Sólar power', 'Wind Power'], 'name'), # Note that "Nuclear Power" will have 2 teams. "Coal Power" "Wind Power" and "Solar Power" # all have 1 team. The secondary sort is alphabetical by name. - ('team_count', 200, ['Nuclear Power', 'Coal Power', u'Sólar power', 'Wind Power', - 'private_topic_1_name', 'private_topic_2_name'], 'team_count'), + ('team_count', 200, ['Nuclear Power', 'Coal Power', u'Sólar power', 'Wind Power'], 'team_count'), ('no_such_field', 400, [], None), ) @ddt.unpack @@ -1620,10 +1617,10 @@ class TestListTopicsAPI(TeamAPITestCase): @ddt.unpack @ddt.data( - ('student_enrolled', 2), - ('student_on_team_1_private_set_1', 2), - ('student_on_team_2_private_set_1', 2), - ('student_masters', 2), + ('student_enrolled', 0), + ('student_on_team_1_private_set_1', 1), + ('student_on_team_2_private_set_1', 1), + ('student_masters', 0), ('staff', 2) ) def test_teamset_type(self, requesting_user, expected_private_teamsets): @@ -1632,9 +1629,6 @@ class TestListTopicsAPI(TeamAPITestCase): Staff should be able to see both teamsets, and anyone enrolled in a private teamset should see that and only that teamset - - TODO: student_enrolled and student_masters should see no private teamsets. - sot1ps1 and sot2ps3 should only see their teamset """ topics = self.get_topics_list( data={'course_id': str(self.test_course_1.id)}, @@ -1645,6 +1639,23 @@ class TestListTopicsAPI(TeamAPITestCase): ] self.assertEqual(len(private_teamsets_returned), expected_private_teamsets) + @ddt.unpack + @ddt.data( + ('student_on_team_1_private_set_1', 2), + ('student_on_team_2_private_set_1', 2), + ('staff', 2) + ) + def test_private_teamset_team_count(self, requesting_user, expected_team_count): + """ + TODO: the two students should probably not see that there's another team that they don't see + """ + topics = self.get_topics_list( + data={'course_id': str(self.test_course_1.id)}, + user=requesting_user + ) + private_teamset_1 = [topic for topic in topics['results'] if topic['name'] == 'private_topic_1_name'][0] + self.assertEqual(private_teamset_1['team_count'], expected_team_count) + @ddt.ddt class TestDetailTopicAPI(TeamAPITestCase): diff --git a/lms/djangoapps/teams/views.py b/lms/djangoapps/teams/views.py index 1611bc94ae..a5c1f200ce 100644 --- a/lms/djangoapps/teams/views.py +++ b/lms/djangoapps/teams/views.py @@ -930,6 +930,8 @@ class TopicListView(GenericAPIView): # in the case of "team_count". organization_protection_status = user_organization_protection_status(request.user, course_id) topics = get_alphabetical_topics(course_module) + topics = self._filter_hidden_private_teamsets(topics, course_module) + if ordering == 'team_count': add_team_count(topics, course_id, organization_protection_status) topics.sort(key=lambda t: t['team_count'], reverse=True) @@ -956,6 +958,26 @@ class TopicListView(GenericAPIView): return response + def _filter_hidden_private_teamsets(self, teamsets, course_module): + """ + Return a filtered list of teamsets, removing any private teamsets that a user doesn't have access to. + Follows the same logic as `has_specific_teamset_access` but in bulk rather than for one teamset at a time + """ + if has_course_staff_privileges(self.request.user, course_module.id): + return teamsets + private_teamset_ids = [teamset.teamset_id for teamset in course_module.teamsets if teamset.is_private_managed] + teamset_ids_user_has_access_to = set( + CourseTeam.objects.filter( + course_id=course_module.id, + topic_id__in=private_teamset_ids, + membership__user=self.request.user + ).values_list('topic_id', flat=True) + ) + return [ + teamset for teamset in teamsets + if teamset['type'] != TeamsetType.private_managed.value or teamset['id'] in teamset_ids_user_has_access_to + ] + def get_alphabetical_topics(course_module): """Return a list of team topics sorted alphabetically.