diff --git a/lms/djangoapps/teams/csv.py b/lms/djangoapps/teams/csv.py index a5a399f489..fa90d622fd 100644 --- a/lms/djangoapps/teams/csv.py +++ b/lms/djangoapps/teams/csv.py @@ -6,6 +6,7 @@ import csv from django.contrib.auth.models import User +from lms.djangoapps.teams.api import OrganizationProtectionStatus, user_organization_protection_status from lms.djangoapps.teams.models import CourseTeam, CourseTeamMembership from student.models import CourseEnrollment from .utils import emit_team_event @@ -289,11 +290,13 @@ class TeamMembershipImportManager(object): if not team_name: continue if (team_name, teamset_id) not in self.existing_course_teams: + protection_status = user_organization_protection_status(user, self.course.id) team = CourseTeam.create( name=team_name, course_id=self.course.id, description='Import from csv', - topic_id=teamset_id + topic_id=teamset_id, + organization_protected=protection_status == OrganizationProtectionStatus.protected ) team.save() self.existing_course_teams[(team_name, teamset_id)] = team diff --git a/lms/djangoapps/teams/errors.py b/lms/djangoapps/teams/errors.py index 63f4ec0c5d..dc9e735760 100644 --- a/lms/djangoapps/teams/errors.py +++ b/lms/djangoapps/teams/errors.py @@ -18,6 +18,14 @@ class AlreadyOnTeamInCourse(TeamAPIRequestError): pass +class AddToIncompatibleTeamError(TeamAPIRequestError): + """ + User is enrolled in a mode that is incompatible with this team type. + e.g. Masters learners cannot be placed in a team with audit learners + """ + pass + + class ElasticSearchConnectionError(TeamAPIRequestError): """The system was unable to connect to the configured elasticsearch instance.""" pass diff --git a/lms/djangoapps/teams/models.py b/lms/djangoapps/teams/models.py index c9af94bc39..ca1ba55697 100644 --- a/lms/djangoapps/teams/models.py +++ b/lms/djangoapps/teams/models.py @@ -35,7 +35,12 @@ from openedx.core.djangoapps.django_comment_common.signals import ( ) from student.models import CourseEnrollment, LanguageField -from .errors import AlreadyOnTeamInCourse, ImmutableMembershipFieldException, NotEnrolledInCourseForTeam +from .errors import ( + AlreadyOnTeamInCourse, + ImmutableMembershipFieldException, + NotEnrolledInCourseForTeam, + AddToIncompatibleTeamError +) @receiver(thread_voted) @@ -195,10 +200,14 @@ class CourseTeam(models.Model): def add_user(self, user): """Adds the given user to the CourseTeam.""" + from lms.djangoapps.teams.api import has_specific_team_access + if not CourseEnrollment.is_enrolled(user, self.course_id): raise NotEnrolledInCourseForTeam if CourseTeamMembership.user_in_team_for_course(user, self.course_id, self.topic_id): raise AlreadyOnTeamInCourse + if not has_specific_team_access(user, self): + raise AddToIncompatibleTeamError return CourseTeamMembership.objects.create( user=user, team=self diff --git a/lms/djangoapps/teams/tests/test_csv.py b/lms/djangoapps/teams/tests/test_csv.py index e046257661..b639f7aefa 100644 --- a/lms/djangoapps/teams/tests/test_csv.py +++ b/lms/djangoapps/teams/tests/test_csv.py @@ -3,6 +3,7 @@ from io import StringIO from lms.djangoapps.teams import csv +from lms.djangoapps.teams.models import CourseTeam from lms.djangoapps.teams.tests.factories import CourseTeamFactory from student.tests.factories import CourseEnrollmentFactory, UserFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase @@ -33,7 +34,13 @@ class TeamMembershipCsvTests(SharedModuleStoreTestCase): team2_1 = CourseTeamFactory(course_id=cls.course.id, name='team_2_1', topic_id='teamset_2') team2_2 = CourseTeamFactory(course_id=cls.course.id, name='team_2_2', topic_id='teamset_2') team3_1 = CourseTeamFactory(course_id=cls.course.id, name='team_3_1', topic_id='teamset_3') - team3_2 = CourseTeamFactory(course_id=cls.course.id, name='team_3_2', topic_id='teamset_3') + # protected team + team3_2 = CourseTeamFactory( + course_id=cls.course.id, + name='team_3_2', + topic_id='teamset_3', + organization_protected=True + ) # No teams in teamset 4 user1 = UserFactory.create(username='user1') @@ -57,9 +64,8 @@ class TeamMembershipCsvTests(SharedModuleStoreTestCase): team3_1.add_user(user2) team2_1.add_user(user3) - team3_2.add_user(user3) + team3_1.add_user(user3) - team1_1.add_user(user4) team3_2.add_user(user4) def setUp(self): @@ -89,8 +95,8 @@ class TeamMembershipCsvTests(SharedModuleStoreTestCase): self.assertEqual(len(data), 5) self.assert_teamset_membership(data[0], 'user1', 'audit', 'team_1_1', 'team_2_2', 'team_3_1') self.assert_teamset_membership(data[1], 'user2', 'verified', 'team_1_1', 'team_2_2', 'team_3_1') - self.assert_teamset_membership(data[2], 'user3', 'honors', None, 'team_2_1', 'team_3_2') - self.assert_teamset_membership(data[3], 'user4', 'masters', 'team_1_1', None, 'team_3_2') + self.assert_teamset_membership(data[2], 'user3', 'honors', None, 'team_2_1', 'team_3_1') + self.assert_teamset_membership(data[3], 'user4', 'masters', None, None, 'team_3_2') self.assert_teamset_membership(data[4], 'user5', 'masters', None, None, None) def assert_teamset_membership( @@ -118,8 +124,53 @@ class TeamMembershipCsvTests(SharedModuleStoreTestCase): expected_csv_output = ('user,mode,teamset_1,teamset_2,teamset_3,teamset_4\r\n' 'user1,audit,team_1_1,team_2_2,team_3_1,\r\n' 'user2,verified,team_1_1,team_2_2,team_3_1,\r\n' - 'user3,honors,,team_2_1,team_3_2,\r\n' - 'user4,masters,team_1_1,,team_3_2,\r\n' + 'user3,honors,,team_2_1,team_3_1,\r\n' + 'user4,masters,,,team_3_2,\r\n' 'user5,masters,,,,\r\n') csv.load_team_membership_csv(self.course, self.buf) self.assertEqual(expected_csv_output, self.buf.getvalue()) + + +class TeamMembershipImportManagerTests(SharedModuleStoreTestCase): + """ Tests for TeamMembershipImportManager """ + @classmethod + def setUpClass(cls): + super(TeamMembershipImportManagerTests, cls).setUpClass() + teams_config = TeamsConfig({ + 'team_sets': [{ + 'id': 'teamset_1', + 'name': 'teamset_name', + 'description': 'teamset_desc', + }] + }) + cls.course = CourseFactory(teams_configuration=teams_config) + + # initialize import manager + cls.import_manager = csv.TeamMembershipImportManager(cls.course) + cls.import_manager.teamset_ids = {ts.teamset_id for ts in cls.course.teamsets} + + def test_add_user_to_new_protected_team(self): + """Adding a masters learner to a new team should create a team with organization protected status""" + masters_learner = UserFactory.create(username='masters_learner') + CourseEnrollmentFactory.create(user=masters_learner, course_id=self.course.id, mode='masters') + row = { + 'mode': 'masters', + 'teamset_1': 'new_protected_team', + 'user': masters_learner + } + + self.import_manager.add_user_to_team(row) + self.assertTrue(CourseTeam.objects.get(team_id__startswith='new_protected_team').organization_protected) + + def test_add_user_to_new_unprotected_team(self): + """Adding a non-masters learner to a new team should create a team with no organization protected status""" + audit_learner = UserFactory.create(username='audit_learner') + CourseEnrollmentFactory.create(user=audit_learner, course_id=self.course.id, mode='audit') + row = { + 'mode': 'audit', + 'teamset_1': 'new_unprotected_team', + 'user': audit_learner + } + + self.import_manager.add_user_to_team(row) + self.assertFalse(CourseTeam.objects.get(team_id__startswith='new_unprotected_team').organization_protected) diff --git a/lms/djangoapps/teams/tests/test_models.py b/lms/djangoapps/teams/tests/test_models.py index 65bd1511ea..5851220515 100644 --- a/lms/djangoapps/teams/tests/test_models.py +++ b/lms/djangoapps/teams/tests/test_models.py @@ -14,7 +14,9 @@ import six from mock import Mock from opaque_keys.edx.keys import CourseKey +from course_modes.models import CourseMode from lms.djangoapps.teams import TEAM_DISCUSSION_CONTEXT +from lms.djangoapps.teams.errors import AddToIncompatibleTeamError from lms.djangoapps.teams.models import CourseTeam, CourseTeamMembership from lms.djangoapps.teams.tests.factories import CourseTeamFactory, CourseTeamMembershipFactory from openedx.core.djangoapps.django_comment_common.signals import ( @@ -82,6 +84,47 @@ class TestModelStrings(SharedModuleStoreTestCase): ) +class CourseTeamTest(SharedModuleStoreTestCase): + """Tests for the CourseTeam model.""" + + @classmethod + def setUpClass(cls): + super(CourseTeamTest, cls).setUpClass() + + cls.audit_learner = UserFactory.create(username="audit") + CourseEnrollmentFactory.create(user=cls.audit_learner, course_id="edx/the-course/1", mode=CourseMode.AUDIT) + cls.audit_team = CourseTeamFactory( + course_id="edx/the-course/1", + team_id="audit-team", + topic_id=TEAMSET_1_ID, + name="The Team" + ) + + cls.masters_learner = UserFactory.create(username="masters") + CourseEnrollmentFactory.create(user=cls.masters_learner, course_id="edx/the-course/1", mode=CourseMode.MASTERS) + cls.masters_team = CourseTeamFactory( + course_id="edx/the-course/1", + team_id="masters-team", + topic_id=TEAMSET_1_ID, + name="The Team", + organization_protected=True + ) + + def test_add_user(self): + """Test that we can add users with correct protection status to a team""" + self.assertIsNotNone(self.masters_team.add_user(self.masters_learner)) + self.assertIsNotNone(self.audit_team.add_user(self.audit_learner)) + + def test_add_user_bad_team_access(self): + """Test that we are blocked from adding a user to a team of mixed enrollment types""" + + with self.assertRaises(AddToIncompatibleTeamError): + self.audit_team.add_user(self.masters_learner) + + with self.assertRaises(AddToIncompatibleTeamError): + self.masters_team.add_user(self.audit_learner) + + @ddt.ddt class TeamMembershipTest(SharedModuleStoreTestCase): """Tests for the TeamMembership model."""