diff --git a/lms/djangoapps/teams/admin.py b/lms/djangoapps/teams/admin.py index 41f44625c1..6b5e931472 100644 --- a/lms/djangoapps/teams/admin.py +++ b/lms/djangoapps/teams/admin.py @@ -13,7 +13,7 @@ class CourseTeamAdmin(admin.ModelAdmin): """ Admin config for course teams. """ - list_display = ('course_id', 'topic_id', 'team_id', 'name', 'team_size') + list_display = ('course_id', 'topic_id', 'team_id', 'name', 'team_size', 'organization_protected') search_fields = ('course_id', 'topic_id', 'team_id', 'name', 'description') ordering = ('course_id', 'topic_id', 'team_id') readonly_fields = ('team_size',) diff --git a/lms/djangoapps/teams/api.py b/lms/djangoapps/teams/api.py index 73b5dbd3fb..84edc3368f 100644 --- a/lms/djangoapps/teams/api.py +++ b/lms/djangoapps/teams/api.py @@ -3,7 +3,39 @@ The Python API other app should use to work with Teams feature """ from __future__ import absolute_import, unicode_literals +import logging +from enum import Enum + +from django.db.models import Count + +from course_modes.models import CourseMode +from lms.djangoapps.discussion.django_comment_client.utils import has_discussion_privileges from lms.djangoapps.teams.models import CourseTeam +from student.models import CourseEnrollment +from student.roles import CourseInstructorRole, CourseStaffRole + +logger = logging.getLogger(__name__) + + +class OrganizationProtectionStatus(Enum): + """ + Enum for the different protection status a user can be in related to their + course enrollment mode. + + "protection_exempt" means the user is a course or org staff that do not need to be protected. + + "protected" means the learner is part of an organization and should be in an exclusive team + + "unprotected" means the learner is part of the general edX learner in audit or verified tracks + """ + protected = 'org_protected' + protection_exempt = 'org_protection_exempt' + unprotected = 'org_unprotected' + + +ORGANIZATION_PROTECTED_MODES = ( + CourseMode.MASTERS, +) def get_team_by_discussion(discussion_id): @@ -50,3 +82,110 @@ def discussion_visible_by_user(discussion_id, user): """ team = get_team_by_discussion(discussion_id) return not is_team_discussion_private(team) or user_is_a_team_member(user, team) + + +def _has_course_staff_privileges(user, course_key): + """ + Returns True if the user is an admin for the course, else returns False + """ + if user.is_staff: + return True + if CourseStaffRole(course_key).has_user(user) or CourseInstructorRole(course_key).has_user(user): + return True + return False + + +def has_team_api_access(user, course_key, access_username=None): + """Returns True if the user has access to the Team API for the course + given by `course_key`. The user must either be enrolled in the course, + be course staff, be global staff, or have discussion privileges. + + Args: + user (User): The user to check access for. + course_key (CourseKey): The key to the course which we are checking access to. + access_username (string): If provided, access_username must match user.username for non staff access. + + Returns: + bool: True if the user has access, False otherwise. + """ + if _has_course_staff_privileges(user, course_key): + return True + if has_discussion_privileges(user, course_key): + return True + if not access_username or access_username == user.username: + return CourseEnrollment.is_enrolled(user, course_key) + return False + + +def user_organization_protection_status(user, course_key): + """ + Returns the organization protection status of the user related to this course + If the user is in the Masters track of the course, we return the protected status. + If the user is a staff of the course, we return the protection_exempt status + else, we return the unprotected status + """ + if _has_course_staff_privileges(user, course_key): + return OrganizationProtectionStatus.protection_exempt + enrollment = CourseEnrollment.get_enrollment(user, course_key) + if enrollment and enrollment.is_active: + if enrollment.mode in ORGANIZATION_PROTECTED_MODES: + return OrganizationProtectionStatus.protected + else: + return OrganizationProtectionStatus.unprotected + else: + raise ValueError( + 'Cannot check the org_protection status on a student [%s] not enrolled in course [%s]', + user.id, + course_key + ) + + +def has_specific_team_access(user, team): + """ + Check whether the user have access to the specific team. + The user can be of a different organization protection bubble with the team in question. + If user is not in the same organization protection bubble with the team, return False. + Else, return True. If the user is a course admin, also return true + """ + protection_status = user_organization_protection_status(user, team.course_id) + if protection_status == OrganizationProtectionStatus.protection_exempt: + return True + if team.organization_protected: + return OrganizationProtectionStatus.protected == protection_status + else: + return OrganizationProtectionStatus.unprotected == protection_status + + +def get_team_count_query_set(topic_id_set, course_id, organization_protection_status): + """ Helper function to get the team count query set based on the filters provided """ + + filter_query = {'course_id': course_id} + if len(topic_id_set) == 1: + filter_query.update({'topic_id': topic_id_set[0]}) + else: + filter_query.update({'topic_id__in': topic_id_set}) + + if organization_protection_status != OrganizationProtectionStatus.protection_exempt: + filter_query.update( + {'organization_protected': organization_protection_status == OrganizationProtectionStatus.protected} + ) + return CourseTeam.objects.filter(**filter_query) + + +def add_team_count(topics, course_id, organization_protection_status): + """ + Helper method to add team_count for a list of topics. + This allows for a more efficient single query. + """ + topic_ids = [topic['id'] for topic in topics] + teams_query_set = get_team_count_query_set( + topic_ids, + course_id, + organization_protection_status + ) + + teams_per_topic = teams_query_set.values('topic_id').annotate(team_count=Count('topic_id')) + + topics_to_team_count = {d['topic_id']: d['team_count'] for d in teams_per_topic} + for topic in topics: + topic['team_count'] = topics_to_team_count.get(topic['id'], 0) diff --git a/lms/djangoapps/teams/migrations/0003_courseteam_organization_protected.py b/lms/djangoapps/teams/migrations/0003_courseteam_organization_protected.py new file mode 100644 index 0000000000..5fb87f0ca8 --- /dev/null +++ b/lms/djangoapps/teams/migrations/0003_courseteam_organization_protected.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.25 on 2019-11-04 19:15 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('teams', '0002_slug_field_ids'), + ] + + operations = [ + migrations.AddField( + model_name='courseteam', + name='organization_protected', + field=models.BooleanField(default=False), + ), + ] diff --git a/lms/djangoapps/teams/models.py b/lms/djangoapps/teams/models.py index aa3afe3025..e9cb9cc278 100644 --- a/lms/djangoapps/teams/models.py +++ b/lms/djangoapps/teams/models.py @@ -137,11 +137,26 @@ class CourseTeam(models.Model): field_tracker = FieldTracker() + # This field would divide the teams into two mutually exclusive groups + # If the team is org protected, the members in a team is enrolled into a degree bearing institution + # If the team is not org protected, the members in a team is part of the general edX learning community + # We need this exclusion for learner privacy protection + organization_protected = models.BooleanField(default=False) + # Don't emit changed events when these fields change. FIELD_BLACKLIST = ['last_activity_at', 'team_size'] @classmethod - def create(cls, name, course_id, description, topic_id=None, country=None, language=None): + def create( + cls, + name, + course_id, + description, + topic_id=None, + country=None, + language=None, + organization_protected=False + ): """Create a complete CourseTeam object. Args: @@ -155,6 +170,8 @@ class CourseTeam(models.Model): is based, as ISO 3166-1 code. language (str, optional): An optional language which the team uses, as ISO 639-1 code. + organization_protected (bool, optional): specifies whether the team should only + contain members who are in a organization context, or not """ unique_id = uuid4().hex @@ -170,7 +187,8 @@ class CourseTeam(models.Model): description=description, country=country if country else '', language=language if language else '', - last_activity_at=datetime.utcnow().replace(tzinfo=pytz.utc) + last_activity_at=datetime.utcnow().replace(tzinfo=pytz.utc), + organization_protected=organization_protected ) return course_team diff --git a/lms/djangoapps/teams/serializers.py b/lms/djangoapps/teams/serializers.py index 0aad1d0b0f..78efa17c29 100644 --- a/lms/djangoapps/teams/serializers.py +++ b/lms/djangoapps/teams/serializers.py @@ -8,10 +8,10 @@ from copy import deepcopy import six from django.conf import settings from django.contrib.auth.models import User -from django.db.models import Count from django_countries import countries from rest_framework import serializers +from lms.djangoapps.teams.api import add_team_count, get_team_count_query_set from lms.djangoapps.teams.models import CourseTeam, CourseTeamMembership from openedx.core.djangoapps.user_api.accounts.serializers import UserReadOnlySerializer from openedx.core.lib.api.fields import ExpandableField @@ -91,6 +91,7 @@ class CourseTeamSerializer(serializers.ModelSerializer): "language", "last_activity_at", "membership", + "organization_protected", ) read_only_fields = ("course_id", "date_created", "discussion_topic_id", "last_activity_at") @@ -109,6 +110,7 @@ class CourseTeamCreationSerializer(serializers.ModelSerializer): "topic_id", "country", "language", + "organization_protected", ) def create(self, validated_data): @@ -119,6 +121,7 @@ class CourseTeamCreationSerializer(serializers.ModelSerializer): topic_id=validated_data.get("topic_id", ''), country=validated_data.get("country", ''), language=validated_data.get("language", ''), + organization_protected=validated_data.get("organization_protected", False) ) team.save() return team @@ -189,7 +192,11 @@ class TopicSerializer(BaseTopicSerializer): # pylint: disable=abstract-method if 'team_count' in topic: return topic['team_count'] else: - return CourseTeam.objects.filter(course_id=self.context['course_id'], topic_id=topic['id']).count() + return get_team_count_query_set( + [topic['id']], + self.context['course_id'], + self.context.get('organization_protection_status') + ).count() class BulkTeamCountTopicListSerializer(serializers.ListSerializer): # pylint: disable=abstract-method @@ -200,7 +207,7 @@ class BulkTeamCountTopicListSerializer(serializers.ListSerializer): # pylint: d def to_representation(self, obj): # pylint: disable=arguments-differ """Adds team_count to each topic. """ data = super(BulkTeamCountTopicListSerializer, self).to_representation(obj) - add_team_count(data, self.context["course_id"]) + add_team_count(data, self.context['course_id'], self.context.get('organization_protection_status')) return data @@ -211,19 +218,3 @@ class BulkTeamCountTopicSerializer(BaseTopicSerializer): # pylint: disable=abst """ class Meta(object): list_serializer_class = BulkTeamCountTopicListSerializer - - -def add_team_count(topics, course_id): - """ - Helper method to add team_count for a list of topics. - This allows for a more efficient single query. - """ - topic_ids = [topic['id'] for topic in topics] - teams_per_topic = CourseTeam.objects.filter( - course_id=course_id, - topic_id__in=topic_ids - ).values('topic_id').annotate(team_count=Count('topic_id')) - - topics_to_team_count = {d['topic_id']: d['team_count'] for d in teams_per_topic} - for topic in topics: - topic['team_count'] = topics_to_team_count.get(topic['id'], 0) diff --git a/lms/djangoapps/teams/tests/test_api.py b/lms/djangoapps/teams/tests/test_api.py index 80208a7d26..c1386148da 100644 --- a/lms/djangoapps/teams/tests/test_api.py +++ b/lms/djangoapps/teams/tests/test_api.py @@ -7,11 +7,15 @@ from __future__ import absolute_import, unicode_literals import unittest from uuid import uuid4 +import ddt from opaque_keys.edx.keys import CourseKey +from course_modes.models import CourseMode from lms.djangoapps.teams import api as teams_api from lms.djangoapps.teams.tests.factories import CourseTeamFactory from student.tests.factories import CourseEnrollmentFactory, UserFactory +from student.models import CourseEnrollment +from student.roles import CourseStaffRole from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase COURSE_KEY1 = CourseKey.from_string('edx/history/1') @@ -76,3 +80,180 @@ class PythonAPITests(SharedModuleStoreTestCase): self.assertTrue(teams_api.discussion_visible_by_user(self.team2.discussion_topic_id, self.user1)) self.assertTrue(teams_api.discussion_visible_by_user(self.team2.discussion_topic_id, self.user2)) self.assertTrue(teams_api.discussion_visible_by_user('DO_NOT_EXISTS', self.user3)) + + +@ddt.ddt +class TeamAccessTests(SharedModuleStoreTestCase): + """ + The set of tests for API endpoints related to access of a team based on the users + """ + @classmethod + def setUpClass(cls): + super(TeamAccessTests, cls).setUpClass() + cls.user_audit = UserFactory.create(username='user_audit') + cls.user_staff = UserFactory.create(username='user_staff') + cls.user_masters = UserFactory.create(username='user_masters') + cls.user_unenrolled = UserFactory.create(username='user_unenrolled') + cls.users = { + 'user_audit': cls.user_audit, + 'user_staff': cls.user_staff, + 'user_masters': cls.user_masters, + 'user_unenrolled': cls.user_unenrolled, + } + + for user in (cls.user_audit, cls.user_staff): + CourseEnrollmentFactory.create(user=user, course_id=COURSE_KEY1) + CourseEnrollmentFactory.create(user=cls.user_masters, course_id=COURSE_KEY1, mode=CourseMode.MASTERS) + + CourseStaffRole(COURSE_KEY1).add_users(cls.user_staff) + + cls.topic_id = 'RANDOM TOPIC' + cls.team_unprotected_1 = CourseTeamFactory( + course_id=COURSE_KEY1, + topic_id=cls.topic_id, + team_id='team_unprotected_1' + ) + cls.team_unprotected_2 = CourseTeamFactory( + course_id=COURSE_KEY1, + topic_id=cls.topic_id, + team_id='team_unprotected_2' + ) + cls.team_unprotected_3 = CourseTeamFactory( + course_id=COURSE_KEY1, + topic_id=cls.topic_id, + team_id='team_unprotected_3' + ) + cls.team_protected_1 = CourseTeamFactory( + course_id=COURSE_KEY1, + team_id='team_protected_1', + topic_id=cls.topic_id, + organization_protected=True + ) + cls.team_protected_2 = CourseTeamFactory( + course_id=COURSE_KEY1, + team_id='team_protected_2', + topic_id=cls.topic_id, + organization_protected=True + ) + + @ddt.data( + ('user_audit', True), + ('user_masters', True), + ('user_staff', True), + ('user_unenrolled', False), + ) + @ddt.unpack + def test_has_team_api_access(self, username, expected_have_access): + user = self.users[username] + self.assertEqual( + expected_have_access, + teams_api.has_team_api_access(user, COURSE_KEY1) + ) + + @ddt.data( + ('user_audit', teams_api.OrganizationProtectionStatus.unprotected), + ('user_masters', teams_api.OrganizationProtectionStatus.protected), + ('user_staff', teams_api.OrganizationProtectionStatus.protection_exempt), + ('user_unenrolled', None), + ) + @ddt.unpack + def test_user_organization_protection_status(self, username, expected_protection_status): + user = self.users[username] + try: + self.assertEqual( + expected_protection_status, + teams_api.user_organization_protection_status(user, COURSE_KEY1) + ) + except ValueError: + self.assertFalse(CourseEnrollment.is_enrolled(user, COURSE_KEY1)) + + @ddt.data( + ('user_audit', True), + ('user_masters', False), + ('user_staff', True), + ('user_unenrolled', False), + ) + @ddt.unpack + def test_has_specific_team_access_unprotected_team(self, username, expected_return): + user = self.users[username] + try: + self.assertEqual( + expected_return, + teams_api.has_specific_team_access(user, self.team_unprotected_1) + ) + except ValueError: + self.assertFalse(CourseEnrollment.is_enrolled(user, self.team_unprotected_1.course_id)) + + @ddt.data( + ('user_audit', False), + ('user_masters', True), + ('user_staff', True), + ('user_unenrolled', False), + ) + @ddt.unpack + def test_has_specific_team_access_protected_team(self, username, expected_return): + user = self.users[username] + try: + self.assertEqual( + expected_return, + teams_api.has_specific_team_access(user, self.team_protected_1) + ) + except ValueError: + self.assertFalse(CourseEnrollment.is_enrolled(user, self.team_protected_1.course_id)) + + @ddt.data( + ('user_audit', 3), + ('user_masters', 2), + ('user_staff', 5), + ('user_unenrolled', 3), + ) + @ddt.unpack + def test_team_counter_get_team_count_query_set(self, username, expected_count): + user = self.users[username] + try: + organization_protection_status = teams_api.user_organization_protection_status( + user, + COURSE_KEY1 + ) + except ValueError: + self.assertFalse(CourseEnrollment.is_enrolled(user, COURSE_KEY1)) + return + teams_query_set = teams_api.get_team_count_query_set( + [self.topic_id], + COURSE_KEY1, + organization_protection_status + ) + self.assertEqual( + expected_count, + teams_query_set.count() + ) + + @ddt.data( + ('user_audit', 3), + ('user_masters', 2), + ('user_staff', 5), + ('user_unenrolled', 3), + ) + @ddt.unpack + def test_team_counter_add_team_count(self, username, expected_team_count): + user = self.users[username] + try: + organization_protection_status = teams_api.user_organization_protection_status( + user, + COURSE_KEY1 + ) + except ValueError: + self.assertFalse(CourseEnrollment.is_enrolled(user, COURSE_KEY1)) + return + topic = { + 'id': self.topic_id + } + teams_api.add_team_count( + [topic], + COURSE_KEY1, + organization_protection_status + ) + self.assertEqual( + expected_team_count, + topic.get('team_count') + ) diff --git a/lms/djangoapps/teams/tests/test_views.py b/lms/djangoapps/teams/tests/test_views.py index 5d6ec0300f..ca9546a89b 100644 --- a/lms/djangoapps/teams/tests/test_views.py +++ b/lms/djangoapps/teams/tests/test_views.py @@ -22,6 +22,7 @@ from search.search_engine_base import SearchEngine from six.moves import range from common.test.utils import skip_signal +from course_modes.models import CourseMode from lms.djangoapps.courseware.tests.factories import StaffFactory from openedx.core.djangoapps.django_comment_common.models import FORUM_ROLE_COMMUNITY_TA, Role from openedx.core.djangoapps.django_comment_common.utils import seed_permissions_roles @@ -291,6 +292,13 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase): username='student_enrolled_other_course_not_on_team' ) + # This is a Masters student who should be in the organization protected bubble + cls.create_and_enroll_student( + courses=[cls.test_course_1, cls.test_course_2], + username='student_masters', + mode=CourseMode.MASTERS + ) + with skip_signal( post_save, receiver=course_team_post_save_callback, @@ -327,6 +335,16 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase): topic_id='topic_7' ) + cls.masters_only_team = CourseTeamFactory.create( + name='masters_course_1', + description='masters student group', + country='US', + language='EN', + course_id=cls.test_course_1.id, + topic_id='topic_0', + organization_protected=True + ) + cls.test_team_name_id_map = {team.name: team for team in ( cls.solar_team, cls.wind_team, @@ -335,6 +353,7 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase): cls.public_profile_team, cls.search_team, cls.chinese_team, + cls.masters_only_team, )} for user, course in [('staff', cls.test_course_1), ('course_staff', cls.test_course_1)]: @@ -349,6 +368,7 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase): cls.nuclear_team.add_user(cls.users['student_enrolled_both_courses_other_team']) cls.another_team.add_user(cls.users['student_enrolled_both_courses_other_team']) cls.public_profile_team.add_user(cls.users['student_enrolled_public_profile']) + cls.masters_only_team.add_user(cls.users['student_masters']) def build_membership_data_raw(self, username, team): """Assembles a membership creation payload based on the raw values provided.""" @@ -359,7 +379,7 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase): return self.build_membership_data_raw(self.users[username].username, team.team_id) @classmethod - def create_and_enroll_student(cls, courses=None, username=None): + def create_and_enroll_student(cls, courses=None, username=None, mode=None): """ Creates a new student and enrolls that student in the course. Adds the new user to the cls.users dictionary with the username as the key. @@ -372,7 +392,7 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase): user = UserFactory.create(password=cls.test_password) courses = courses if courses is not None else [cls.test_course_1] for course in courses: - CourseEnrollment.enroll(user, course.id, check_access=True) + CourseEnrollment.enroll(user, course.id, mode=mode, check_access=True) cls.users[user.username] = user return user.username @@ -542,6 +562,12 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase): for field in ['id', 'name', 'course_id', 'topic_id', 'date_created', 'description']: self.assertIn(field, team) + def reset_search_index(self): + """Clear out the search index and reindex the teams.""" + CourseTeamIndexer.engine().destroy() + for team in self.test_team_name_id_map.values(): + CourseTeamIndexer.index(team) + @ddt.ddt class TestListTeamsAPI(EventTestMixin, TeamAPITestCase): @@ -554,16 +580,17 @@ class TestListTeamsAPI(EventTestMixin, TeamAPITestCase): (None, 401), ('student_inactive', 401), ('student_unenrolled', 403), - ('student_enrolled', 200), - ('staff', 200), - ('course_staff', 200), - ('community_ta', 200), + ('student_enrolled', 200, 3), + ('staff', 200, 4), + ('course_staff', 200, 4), + ('community_ta', 200, 3), + ('student_masters', 200, 1) ) @ddt.unpack - def test_access(self, user, status): + def test_access(self, user, status, expected_teams_count=0): teams = self.get_teams_list(user=user, expected_status=status) if status == 200: - self.assertEqual(3, teams['count']) + self.assertEqual(expected_teams_count, teams['count']) def test_missing_course_id(self): self.get_teams_list(400, no_course_id=True) @@ -669,13 +696,7 @@ class TestListTeamsAPI(EventTestMixin, TeamAPITestCase): ) @ddt.unpack def test_text_search(self, text_search, expected_team_names): - def reset_search_index(): - """Clear out the search index and reindex the teams.""" - CourseTeamIndexer.engine().destroy() - for team in self.test_team_name_id_map.values(): - CourseTeamIndexer.index(team) - - reset_search_index() + self.reset_search_index() self.verify_names( {'course_id': self.test_course_2.id, 'text_search': text_search}, 200, @@ -692,7 +713,7 @@ class TestListTeamsAPI(EventTestMixin, TeamAPITestCase): # Verify that the searches still work for a user from a different locale with translation.override('ar'): - reset_search_index() + self.reset_search_index() self.verify_names( {'course_id': self.test_course_2.id, 'text_search': text_search}, 200, @@ -700,6 +721,23 @@ class TestListTeamsAPI(EventTestMixin, TeamAPITestCase): user='student_enrolled_public_profile' ) + @ddt.data( + ('masters', ['masters_course_1']), + ('group', ['masters_course_1']), + ('Sólar', []), + ('Wind', []), + ) + @ddt.unpack + def test_text_search_masters(self, text_search, expected_team_names): + # Verify that the search is working with Masters learner + self.reset_search_index() + self.verify_names( + {'course_id': self.test_course_1.id, 'text_search': text_search}, + 200, + expected_team_names, + user='student_masters' + ) + def test_delete_removed_from_search(self): team = CourseTeamFactory.create( name=u'zoinks', @@ -877,7 +915,8 @@ class TestCreateTeamAPI(EventTestMixin, TeamAPITestCase): 'country': 'CA', 'topic_id': 'great-topic', 'course_id': str(self.test_course_1.id), - 'description': 'Another fantastic team' + 'description': 'Another fantastic team', + 'organization_protected': False, }) @ddt.data('staff', 'course_staff', 'community_ta') diff --git a/lms/djangoapps/teams/views.py b/lms/djangoapps/teams/views.py index 4c329a5d05..2c5d80c5e5 100644 --- a/lms/djangoapps/teams/views.py +++ b/lms/djangoapps/teams/views.py @@ -28,6 +28,13 @@ from rest_framework_oauth.authentication import OAuth2Authentication from lms.djangoapps.courseware.courses import get_course_with_access, has_access from lms.djangoapps.discussion.django_comment_client.utils import has_discussion_privileges +from lms.djangoapps.teams.api import ( + OrganizationProtectionStatus, + has_team_api_access, + user_organization_protection_status, + has_specific_team_access, + add_team_count, +) from lms.djangoapps.teams.models import CourseTeam, CourseTeamMembership from openedx.core.lib.api.parsers import MergePatchParser from openedx.core.lib.api.permissions import IsStaffOrReadOnly @@ -38,7 +45,6 @@ from openedx.core.lib.api.view_utils import ( build_api_error ) from student.models import CourseAccessRole, CourseEnrollment -from student.roles import CourseStaffRole from util.model_utils import truncate_fields from xmodule.modulestore.django import modulestore @@ -50,8 +56,7 @@ from .serializers import ( CourseTeamCreationSerializer, CourseTeamSerializer, MembershipSerializer, - TopicSerializer, - add_team_count + TopicSerializer ) from .utils import emit_team_event @@ -122,6 +127,7 @@ class TeamsDashboardView(GenericAPIView): # to the serializer so that the paginated results indicate how they were sorted. sort_order = 'name' topics = get_alphabetical_topics(course) + organization_protection_status = user_organization_protection_status(request.user, course_key) # Paginate and serialize topic data # BulkTeamCountPaginatedTopicSerializer will add team counts to the topics in a single @@ -131,13 +137,22 @@ class TeamsDashboardView(GenericAPIView): topics, request, BulkTeamCountTopicSerializer, - {'course_id': course.id}, + { + 'course_id': course.id, + 'organization_protection_status': organization_protection_status + }, ) topics_data["sort_order"] = sort_order - user = request.user + filter_query = { + 'membership__user': user, + 'course_id': course.id, + } + if organization_protection_status != OrganizationProtectionStatus.protection_exempt: + is_user_org_protected = organization_protection_status == OrganizationProtectionStatus.protected + filter_query['organization_protected'] = is_user_org_protected - user_teams = CourseTeam.objects.filter(membership__user=user, course_id=course.id) + user_teams = CourseTeam.objects.filter(**filter_query) user_teams_data = self._serialize_and_paginate( MyTeamsPagination, user_teams, @@ -208,30 +223,6 @@ class TeamsDashboardView(GenericAPIView): return paginator.get_paginated_response(serializer.data).data -def has_team_api_access(user, course_key, access_username=None): - """Returns True if the user has access to the Team API for the course - given by `course_key`. The user must either be enrolled in the course, - be course staff, be global staff, or have discussion privileges. - - Args: - user (User): The user to check access for. - course_key (CourseKey): The key to the course which we are checking access to. - access_username (string): If provided, access_username must match user.username for non staff access. - - Returns: - bool: True if the user has access, False otherwise. - """ - if user.is_staff: - return True - if CourseStaffRole(course_key).has_user(user): - return True - if has_discussion_privileges(user, course_key): - return True - if not access_username or access_username == user.username: - return CourseEnrollment.is_enrolled(user, course_key) - return False - - class TeamsListView(ExpandableFieldViewMixin, GenericAPIView): """ **Use Cases** @@ -320,6 +311,9 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView): * membership: A list of the users that are members of the team. See membership endpoint for more detail. + * organization_protected: Whether the team consists of organization-protected + learners + For all text fields, clients rendering the values should take care to HTML escape them to avoid script injections, as the data is stored exactly as specified. The intention is that plain text is @@ -340,6 +334,10 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView): If the server is unable to connect to Elasticsearch, and the text_search parameter is supplied, a 503 error is returned. + If the requesting user is a learner, the learner would only see organization + protected set of teams if the learner is enrolled in a degree bearing institution. + Otherwise, the learner will only see organization unprotected set of teams. + **Response Values for POST** Any logged in user who has verified their email address can create @@ -411,6 +409,15 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView): ) return Response(error, status=status.HTTP_400_BAD_REQUEST) result_filter.update({'topic_id': topic_id}) + + organization_protection_status = user_organization_protection_status(request.user, course_key) + if organization_protection_status != OrganizationProtectionStatus.protection_exempt: + result_filter.update( + { + 'organization_protected': organization_protection_status == OrganizationProtectionStatus.protected + } + ) + if text_search and CourseTeamIndexer.search_is_enabled(): try: search_engine = CourseTeamIndexer.engine() @@ -508,6 +515,10 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView): data = request.data.copy() data['course_id'] = six.text_type(course_key) + organization_protection_status = user_organization_protection_status(request.user, course_key) + if organization_protection_status != OrganizationProtectionStatus.protection_exempt: + data['organization_protected'] = organization_protection_status == OrganizationProtectionStatus.protected + serializer = CourseTeamCreationSerializer(data=data) add_serializer_errors(serializer, data, field_errors) @@ -619,6 +630,9 @@ class TeamsDetailView(ExpandableFieldViewMixin, RetrievePatchAPIView): * last_activity_at: The date of the last activity of any team member within the team. + * organization_protected: Whether the team consists of organization-protected + learners + For all text fields, clients rendering the values should take care to HTML escape them to avoid script injections, as the data is stored exactly as specified. The intention is that plain text is @@ -749,6 +763,11 @@ class TopicListView(GenericAPIView): * description: A description of the topic. + * team_count: Number of teams created under the topic. If the requesting user + is enrolled into a degree bearing institution, the count only include the teams + with organization_protected attribute true. If the requesting user is not + affiliated with any institutions, the teams included in the count would only be + those teams whose members are outside of institutions affliation. """ authentication_classes = (OAuth2Authentication, SessionAuthentication) @@ -794,9 +813,10 @@ class TopicListView(GenericAPIView): # Always sort alphabetically, as it will be used as secondary sort # in the case of "team_count". + organization_protection_status = user_organization_protection_status(request.user, course_id) topics = get_alphabetical_topics(course_module) if ordering == 'team_count': - add_team_count(topics, course_id) + add_team_count(topics, course_id, organization_protection_status) topics.sort(key=lambda t: t['team_count'], reverse=True) page = self.paginate_queryset(topics) serializer = TopicSerializer( @@ -807,7 +827,14 @@ class TopicListView(GenericAPIView): else: page = self.paginate_queryset(topics) # Use the serializer that adds team_count in a bulk operation per page. - serializer = BulkTeamCountTopicSerializer(page, context={'course_id': course_id}, many=True) + serializer = BulkTeamCountTopicSerializer( + page, + context={ + 'course_id': course_id, + 'organization_protection_status': organization_protection_status + }, + many=True + ) response = self.get_paginated_response(serializer.data) response.data['sort_order'] = ordering @@ -867,6 +894,11 @@ class TopicDetailView(APIView): * description: A description of the topic. + * team_count: Number of teams created under the topic. If the requesting user + is enrolled into a degree bearing institution, the count only include the teams + with organization_protected attribute true. If the requesting user is not + affiliated with any institutions, the teams included in the count would only be + those teams whose members are outside of institutions affliation. """ authentication_classes = (OAuth2Authentication, SessionAuthentication) @@ -892,8 +924,13 @@ class TopicDetailView(APIView): except KeyError: return Response(status=status.HTTP_404_NOT_FOUND) + organization_protection_status = user_organization_protection_status(request.user, course_id) serializer = TopicSerializer( - topic.cleaned_data_old_format, context={'course_id': course_id} + topic.cleaned_data_old_format, + context={ + 'course_id': course_id, + 'organization_protection_status': organization_protection_status + } ) return Response(serializer.data) @@ -1055,6 +1092,8 @@ class MembershipListView(ExpandableFieldViewMixin, GenericAPIView): return Response(status=status.HTTP_400_BAD_REQUEST) if not has_team_api_access(request.user, team.course_id): return Response(status=status.HTTP_404_NOT_FOUND) + if not has_specific_team_access(request.user, team): + return Response(status=status.HTTP_403_FORBIDDEN) if 'username' in request.query_params: specified_username_or_team = True @@ -1111,6 +1150,9 @@ class MembershipListView(ExpandableFieldViewMixin, GenericAPIView): if not has_team_api_access(request.user, team.course_id, access_username=username): return Response(status=status.HTTP_404_NOT_FOUND) + if not has_specific_team_access(request.user, team): + return Response(status=status.HTTP_403_FORBIDDEN) + try: user = User.objects.get(username=username) except User.DoesNotExist: @@ -1251,6 +1293,9 @@ class MembershipDetailView(ExpandableFieldViewMixin, GenericAPIView): if not has_team_api_access(request.user, team.course_id): return Response(status=status.HTTP_404_NOT_FOUND) + if not has_specific_team_access(request.user, team): + return Response(status=status.HTTP_403_FORBIDDEN) + membership = self.get_membership(username, team) serializer = self.get_serializer(instance=membership) @@ -1259,21 +1304,24 @@ class MembershipDetailView(ExpandableFieldViewMixin, GenericAPIView): def delete(self, request, team_id, username): """DELETE /api/team/v0/team_membership/{team_id},{username}""" team = self.get_team(team_id) - if has_team_api_access(request.user, team.course_id, access_username=username): - membership = self.get_membership(username, team) - removal_method = 'self_removal' - if 'admin' in request.query_params: - removal_method = 'removed_by_admin' - membership.delete() - emit_team_event( - 'edx.team.learner_removed', - team.course_id, - { - 'team_id': team.team_id, - 'user_id': membership.user.id, - 'remove_method': removal_method - } - ) - return Response(status=status.HTTP_204_NO_CONTENT) - else: + if not has_team_api_access(request.user, team.course_id, access_username=username): return Response(status=status.HTTP_404_NOT_FOUND) + + if not has_specific_team_access(request.user, team): + return Response(status=status.HTTP_403_FORBIDDEN) + + membership = self.get_membership(username, team) + removal_method = 'self_removal' + if 'admin' in request.query_params: + removal_method = 'removed_by_admin' + membership.delete() + emit_team_event( + 'edx.team.learner_removed', + team.course_id, + { + 'team_id': team.team_id, + 'user_id': membership.user.id, + 'remove_method': removal_method + } + ) + return Response(status=status.HTTP_204_NO_CONTENT)