Before adding user to a team, check the organization protection status (#23134)
* Check learner enrollment status to avoid mixing learners from incompatible tracks (e.g. masters/audit) to the same team * When a new team is encountered on a team management CSV, create it with the protection status of the user
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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."""
|
||||
|
||||
Reference in New Issue
Block a user