diff --git a/lms/djangoapps/teams/csv.py b/lms/djangoapps/teams/csv.py index 1418a4dee4..627c79e802 100644 --- a/lms/djangoapps/teams/csv.py +++ b/lms/djangoapps/teams/csv.py @@ -2,6 +2,13 @@ CSV processing and generation utilities for Teams LMS app. """ +import csv +from django.contrib.auth.models import User +from student.models import CourseEnrollment + +from lms.djangoapps.teams.models import CourseTeam, CourseTeamMembership +from .utils import emit_team_event + def load_team_membership_csv(course, response): """ @@ -19,3 +26,231 @@ def load_team_membership_csv(course, response): "Team membership CSV download is not yet implemented." ) response.write(not_implemented_message + "\n") + + +class TeamMembershipImportManager(object): + """ + A manager class that is responsible the import process of csv file including validation and creation of + team_courseteam and teams_courseteammembership objects. + """ + + def __init__(self, course): + self.validation_errors = [] + self.teamset_ids = [] + self.user_ids_by_teamset_id = {} + self.number_of_records_added = 0 + self.course = course + self.max_errors = 0 + self.existing_course_team_memberships = {} + self.existing_course_teams = {} + + @property + def import_succeeded(self): + """ + Helper wrapper that tells us the status of the import + """ + return not self.validation_errors + + def set_team_membership_from_csv(self, input_file): + """ + Assigns team membership based on the content of an uploaded CSV file. + Returns true if there were no issues. + """ + reader = csv.DictReader((line.decode('utf-8-sig').strip() for line in input_file.readlines())) + self.teamset_ids = reader.fieldnames[2:] + row_dictionaries = [] + csv_usernames = set() + if not self.validate_teamsets(): + return False + self.load_user_ids_by_teamset_id() + self.load_course_team_memberships() + self.load_course_teams() + # process student rows: + for row in reader: + username = row['user'] + if not username: + continue + if not self.is_username_unique(username, csv_usernames): + return False + csv_usernames.add(username) + user = self.get_user(username) + if user is None: + continue + if not self.validate_user_enrolled_in_course(user): + row['user'] = None + continue + row['user'] = user + + if not self.validate_user_assignment_to_team_and_teamset(row): + return False + row_dictionaries.append(row) + + if not self.validation_errors: + for row in row_dictionaries: + self.add_user_to_team(row) + return True + else: + return False + + def load_course_team_memberships(self): + """ + Caches existing team memberships by (user_id, teamset_id) + """ + for membership in CourseTeamMembership.get_memberships(course_ids=[self.course.id]): + user_id = membership.user_id + teamset_id = membership.team.topic_id + self.existing_course_team_memberships[(user_id, teamset_id)] = membership.team.id + + def load_course_teams(self): + """ + Caches existing course teams by (team_name, topic_id) + """ + for team in CourseTeam.objects.filter(course_id=self.course.id): + self.existing_course_teams[(team.name, team.topic_id)] = team + + def validate_teamsets(self): + """ + Validates team set ids. Returns true if there are no errors. + The following conditions result in errors: + Teamset does not exist + Teamset id is duplicated + """ + valid_teamset_ids = {ts.teamset_id for ts in self.course.teams_configuration.teamsets} + dupe_set = set() + for teamset_id in self.teamset_ids: + if teamset_id in dupe_set: + self.validation_errors.append("Teamset with id " + teamset_id + " is duplicated.") + return False + dupe_set.add(teamset_id) + if teamset_id not in valid_teamset_ids: + self.validation_errors.append("Teamset with id " + teamset_id + " does not exist.") + return False + return True + + def load_user_ids_by_teamset_id(self): + for teamset_id in self.teamset_ids: + self.user_ids_by_teamset_id[teamset_id] = { + membership.user_id for membership in + CourseTeamMembership.objects.filter( + team__course_id=self.course.id, team__topic_id=teamset_id + ) + } + + def validate_user_enrolled_in_course(self, user): + """ + Invalid states: + user not enrolled in course + """ + if not CourseEnrollment.is_enrolled(user, self.course.id): + self.validation_errors.append('User ' + user.username + ' is not enrolled in this course.') + return False + + return True + + def is_username_unique(self, username, usernames_found_so_far): + """ + Ensures that username exists only once in an input file + """ + if username in usernames_found_so_far: + error_message = 'Username {} was found more than once in input file.'.format(username) + if self.add_error_and_check_if_max_exceeded(error_message): + return False + return True + + def validate_user_assignment_to_team_and_teamset(self, row): + """ + Validates a user entry relative to an existing team. + row is a dictionary where key is column name and value is the row value + [andrew],masters,team1,,team3 + [joe],masters,,team2,team3 + """ + user = row['user'] + for teamset_id in self.teamset_ids: + team_name = row[teamset_id] + if not team_name: + continue + try: + # checks for a team inside a specific team set. This way team names can be duplicated across + # teamsets + team = self.existing_course_teams[(team_name, teamset_id)] + except KeyError: + # if a team doesn't exists, the validation doesn't apply to it. + all_teamset_user_ids = self.user_ids_by_teamset_id[teamset_id] + error_message = 'The user {0} is already a member of a team inside teamset {1} in this course.'.format( + user.username, teamset_id + ) + if user.id in all_teamset_user_ids and self.add_error_and_check_if_max_exceeded(error_message): + return False + else: + self.user_ids_by_teamset_id[teamset_id].add(user.id) + continue + max_team_size = self.course.teams_configuration.default_max_team_size + if max_team_size is not None and team.users.count() >= max_team_size: + if self.add_error_and_check_if_max_exceeded('Team ' + team.team_id + ' is already full.'): + return False + + if (user.id, team.topic_id) in self.existing_course_team_memberships: + error_message = 'The user {0} is already a member of a team inside teamset {1} in this course.'.format( + user.username, team.topic_id + ) + if self.add_error_and_check_if_max_exceeded(error_message): + return False + return True + + def add_error_and_check_if_max_exceeded(self, error_message): + """ + Adds an error to the error collection. + :param error_message: + :return: True if maximum error threshold is exceeded and processing must stop + False if maximum error threshold is NOT exceeded and processing can continue + """ + self.validation_errors.append(error_message) + return len(self.validation_errors) >= self.max_errors + + def add_user_to_team(self, user_row): + """ + Creates a CourseTeamMembership entry - i.e: a relationship between a user and a team. + user_row is a dictionary where key is column name and value is the row value. + {'mode': ' masters','topic_0': '','topic_1': 'team 2','topic_2': None,'user': } + andrew,masters,team1,,team3 + joe,masters,,team2,team3 + """ + user = user_row['user'] + for teamset_id in self.teamset_ids: + team_name = user_row[teamset_id] + if not team_name: + continue + if (team_name, teamset_id) not in self.existing_course_teams: + team = CourseTeam.create( + name=team_name, + course_id=self.course.id, + description='Import from csv', + topic_id=teamset_id + ) + team.save() + team.add_user(user) + emit_team_event( + 'edx.team.learner_added', + team.course_id, + { + 'team_id': team.team_id, + 'user_id': user.id, + 'add_method': 'team_csv_import' + } + ) + self.number_of_records_added += 1 + + def get_user(self, user_name): + """ + Gets the user object from user_name/email/locator + user_name: the user_name/email/user locator + """ + try: + return User.objects.get(username=user_name) + except User.DoesNotExist: + try: + return User.objects.get(email=user_name) + except User.DoesNotExist: + self.validation_errors.append('Username or email ' + user_name + ' does not exist.') + return None + # TODO - handle user key case diff --git a/lms/djangoapps/teams/models.py b/lms/djangoapps/teams/models.py index aebd6a80fe..c9af94bc39 100644 --- a/lms/djangoapps/teams/models.py +++ b/lms/djangoapps/teams/models.py @@ -197,7 +197,7 @@ class CourseTeam(models.Model): """Adds the given user to the CourseTeam.""" if not CourseEnrollment.is_enrolled(user, self.course_id): raise NotEnrolledInCourseForTeam - if CourseTeamMembership.user_in_team_for_course(user, self.course_id): + if CourseTeamMembership.user_in_team_for_course(user, self.course_id, self.topic_id): raise AlreadyOnTeamInCourse return CourseTeamMembership.objects.create( user=user, @@ -305,19 +305,25 @@ class CourseTeamMembership(models.Model): return queryset @classmethod - def user_in_team_for_course(cls, user, course_id): + def user_in_team_for_course(cls, user, course_id, topic_id=None): """ - Checks whether or not a user is already in a team in the given course. + Checks user membership in two ways: + if topic_id is None, checks to see if a user is assigned to any team in the course + if topic_id (teamset) is provided, checks to see if a user is assigned to a specific team in the course. Args: user: the user that we want to query on course_id: the course_id of the course we're interested in + topic_id: optional the topic_id (teamset) of the course we are interested in Returns: True if the user is on a team in the course already False if not """ - return cls.objects.filter(user=user, team__course_id=course_id).exists() + if topic_id is None: + return cls.objects.filter(user=user, team__course_id=course_id).exists() + else: + return cls.objects.filter(user=user, team__course_id=course_id, team__topic_id=topic_id).exists() @classmethod def update_last_activity(cls, user, discussion_topic_id): diff --git a/lms/djangoapps/teams/tests/test_views.py b/lms/djangoapps/teams/tests/test_views.py index 9ae398d108..d3bde8f072 100644 --- a/lms/djangoapps/teams/tests/test_views.py +++ b/lms/djangoapps/teams/tests/test_views.py @@ -13,6 +13,7 @@ import pytz from dateutil import parser from django.conf import settings from django.db.models.signals import post_save +from django.core.files.uploadedfile import SimpleUploadedFile from django.urls import reverse from django.utils import translation from elasticsearch.exceptions import ConnectionError @@ -425,7 +426,6 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase): response = func(url, data=data, content_type=content_type) else: response = func(url, data=data) - self.assertEqual( expected_status, response.status_code, @@ -1458,12 +1458,15 @@ class TestCreateMembershipAPI(EventTestMixin, TeamAPITestCase): self.assertIn('already a member', json.loads(response.content.decode('utf-8'))['developer_message']) def test_join_second_team_in_course(self): - response = self.post_create_membership( - 400, + """ + Behavior allows the same student to be enrolled in multiple teams, as long as they belong to different + topics (teamsets) + """ + self.post_create_membership( + 200, self.build_membership_data('student_enrolled_both_courses_other_team', self.solar_team), user='student_enrolled_both_courses_other_team' ) - self.assertIn('already a member', json.loads(response.content.decode('utf-8'))['developer_message']) @ddt.data('staff', 'course_staff') def test_not_enrolled_in_team_course(self, user): @@ -1679,10 +1682,7 @@ class TestBulkMembershipManagement(TeamAPITestCase): ('GET', good_course_id, deny_username, 403), ('GET', fake_course_id, allow_username, 404), ('GET', fake_course_id, deny_username, 404), - ('POST', good_course_id, allow_username, 501), # TODO MST-31 ('POST', good_course_id, deny_username, 403), - ('POST', fake_course_id, allow_username, 404), - ('POST', fake_course_id, deny_username, 404), ) @ddt.unpack def test_error_statuses(self, method, course_id, username, expected_status): @@ -1708,3 +1708,60 @@ class TestBulkMembershipManagement(TeamAPITestCase): def get_url(course_id): # This strategy allows us to test with invalid course IDs return reverse('team_membership_bulk_management', args=[course_id]) + + def test_upload_valid_csv_simple(self): + self.create_and_enroll_student(username='a_user') + csv_content = 'user,mode,topic_0' + '\n' + csv_content += 'a_user, masters, team wind power' + csv_file = SimpleUploadedFile('test_file.csv', csv_content.encode('utf8'), content_type='text/csv') + self.client.login(username=self.users['course_staff'].username, password=self.users['course_staff'].password) + self.make_call(reverse('team_membership_bulk_management', args=[self.good_course_id]), + 201, method='post', + data={'csv': csv_file}, user='staff' + ) + + def test_upload_invalid_teamset(self): + self.create_and_enroll_student(username='a_user') + csv_content = 'user,mode,topic_0_bad' + '\n' + csv_content += 'a_user, masters, team wind power' + csv_file = SimpleUploadedFile('test_file.csv', csv_content.encode('utf8'), content_type='text/csv') + self.client.login(username=self.users['course_staff'].username, password=self.users['course_staff'].password) + self.make_call(reverse('team_membership_bulk_management', args=[self.good_course_id]), + 400, method='post', + data={'csv': csv_file}, user='staff' + ) + + def test_upload_assign_user_twice_to_same_teamset(self): + csv_content = 'user,mode,topic_0' + '\n' + csv_content += 'student_enrolled, masters, team wind power' + csv_file = SimpleUploadedFile('test_file.csv', csv_content.encode('utf8'), content_type='text/csv') + self.client.login(username=self.users['course_staff'].username, password=self.users['course_staff'].password) + self.make_call(reverse('team_membership_bulk_management', args=[self.good_course_id]), + 400, method='post', + data={'csv': csv_file}, user='staff' + ) + + def test_upload_assign_one_user_to_different_teamsets(self): + self.create_and_enroll_student(username='a_user') + self.create_and_enroll_student(username='b_user') + self.create_and_enroll_student(username='c_user') + csv_content = 'user,mode,topic_0,topic_1,topic_2' + '\n' + csv_content += 'a_user, masters,team wind power,team 2' + '\n' + csv_content += 'b_user, masters,,team 2' + '\n' + csv_content += 'c_user, masters,,,team 3' + csv_file = SimpleUploadedFile('test_file.csv', csv_content.encode('utf8'), content_type='text/csv') + self.client.login(username=self.users['course_staff'].username, password=self.users['course_staff'].password) + self.make_call(reverse('team_membership_bulk_management', args=[self.good_course_id]), + 201, method='post', + data={'csv': csv_file}, user='staff' + ) + + def test_upload_non_existing_user(self): + csv_content = 'user,mode,topic_0' + '\n' + csv_content += 'missing_user, masters, team wind power' + csv_file = SimpleUploadedFile('test_file.csv', csv_content.encode('utf8'), content_type='text/csv') + self.client.login(username=self.users['course_staff'].username, password=self.users['course_staff'].password) + self.make_call(reverse('team_membership_bulk_management', args=[self.good_course_id]), + 400, method='post', + data={'csv': csv_file}, user='staff' + ) diff --git a/lms/djangoapps/teams/views.py b/lms/djangoapps/teams/views.py index 9a1b8827bb..5451defe31 100644 --- a/lms/djangoapps/teams/views.py +++ b/lms/djangoapps/teams/views.py @@ -55,7 +55,7 @@ from .api import ( has_team_api_access, user_organization_protection_status ) -from .csv import load_team_membership_csv +from .csv import load_team_membership_csv, TeamMembershipImportManager from .errors import AlreadyOnTeamInCourse, ElasticSearchConnectionError, NotEnrolledInCourseForTeam from .search_indexes import CourseTeamIndexer from .serializers import ( @@ -87,6 +87,7 @@ def team_post_save_callback(sender, instance, **kwargs): # pylint: disable=unus six.text_type(getattr(instance, field)) ) truncated_fields['team_id'] = instance.team_id + truncated_fields['team_id'] = instance.team_id truncated_fields['field'] = field emit_team_event( @@ -1360,11 +1361,9 @@ class MembershipDetailView(ExpandableFieldViewMixin, GenericAPIView): return Response(status=status.HTTP_204_NO_CONTENT) -class MembershipBulkManagementView(View): +class MembershipBulkManagementView(GenericAPIView): """ - Partially-implemented view for uploading and downloading team membership CSVs. - - TODO MST-31 + View for uploading and downloading team membership CSVs. """ def get(self, request, **_kwargs): """ @@ -1384,7 +1383,15 @@ class MembershipBulkManagementView(View): Process uploaded CSV to modify team memberships for given course run. """ self.check_access() - return HttpResponse(status=status.HTTP_501_NOT_IMPLEMENTED) + inputfile_handle = request.FILES['csv'] + team_import_manager = TeamMembershipImportManager(self.course) + team_import_manager.set_team_membership_from_csv(inputfile_handle) + if team_import_manager.import_succeeded: + return Response(str(team_import_manager.number_of_records_added), status=status.HTTP_201_CREATED) + else: + return Response({ + 'errors': team_import_manager.validation_errors + }, status=status.HTTP_400_BAD_REQUEST) def check_access(self): """