From 2c922dd795ced9282d729fd22b9e5ba59243e0ba Mon Sep 17 00:00:00 2001 From: atesker Date: Fri, 6 Mar 2020 12:39:42 -0500 Subject: [PATCH] EDUCATOR-4941 cr1 cr2 cr3 cr3-remove unused var --- lms/djangoapps/teams/csv.py | 39 ++++++++++++++++-------- lms/djangoapps/teams/tests/test_views.py | 26 ++++++++++++++-- 2 files changed, 51 insertions(+), 14 deletions(-) diff --git a/lms/djangoapps/teams/csv.py b/lms/djangoapps/teams/csv.py index feccfad440..657b8c7e35 100644 --- a/lms/djangoapps/teams/csv.py +++ b/lms/djangoapps/teams/csv.py @@ -3,6 +3,7 @@ CSV processing and generation utilities for Teams LMS app. """ import csv +from collections import Counter from django.contrib.auth.models import User @@ -107,6 +108,7 @@ class TeamMembershipImportManager(object): self.max_errors = 0 self.existing_course_team_memberships = {} self.existing_course_teams = {} + self.user_count_by_team = Counter() self.number_of_learners_assigned = 0 @property @@ -282,8 +284,15 @@ class TeamMembershipImportManager(object): # 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)] + if (teamset_id, team_name) not in self.user_count_by_team: + self.user_count_by_team[(teamset_id, team_name)] = team.users.count() + if (user.id, team.topic_id) in self.existing_course_team_memberships: + error_message = 'User {0} is already on a team in teamset {1}.'.format( + user.username, team.topic_id + ) + if self.add_error_and_check_if_max_exceeded(error_message): + return False 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 = 'User {0} is already on a team in teamset {1}.'.format( user.username, teamset_id @@ -292,18 +301,24 @@ class TeamMembershipImportManager(object): 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 full.'): - return False + if not self.validate_proposed_team_size_wont_exceed_maximum(team_name, teamset_id): + return False + return True - if (user.id, team.topic_id) in self.existing_course_team_memberships: - error_message = 'User {0} is already on a team in teamset {1}.'.format( - user.username, team.topic_id - ) - if self.add_error_and_check_if_max_exceeded(error_message): - return False + def validate_proposed_team_size_wont_exceed_maximum(self, team_name, teamset_id): + """ + Validates that the number of users we want to add to a team won't exceed maximum team size. + """ + if self.course.teams_configuration.teamsets_by_id[teamset_id].max_team_size is None: + max_team_size = self.course.teams_configuration.default_max_team_size + else: + max_team_size = self.course.teams_configuration.teamsets_by_id[teamset_id].max_team_size + + if max_team_size is not None: + if self.user_count_by_team[(teamset_id, team_name)] + 1 > max_team_size: + self.add_error_and_check_if_max_exceeded('Team ' + team_name + ' is full.') + return False + self.user_count_by_team[(teamset_id, team_name)] += 1 return True def add_error_and_check_if_max_exceeded(self, error_message): diff --git a/lms/djangoapps/teams/tests/test_views.py b/lms/djangoapps/teams/tests/test_views.py index e4c12c761d..7ac4e9622a 100644 --- a/lms/djangoapps/teams/tests/test_views.py +++ b/lms/djangoapps/teams/tests/test_views.py @@ -258,7 +258,8 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase): with super(TeamAPITestCase, cls).setUpClassAndTestData(): base_topics = [{ 'id': 'topic_{}'.format(i), 'name': name, - 'description': u'Description for topic {}.'.format(i) + 'description': u'Description for topic {}.'.format(i), + 'max_team_size': 3 } for i, name in enumerate([u'Sólar power', 'Wind Power', 'Nuclear Power', 'Coal Power'])] base_topics.append( { @@ -277,7 +278,8 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase): } ) teams_configuration_1 = TeamsConfig({ - 'topics': base_topics + 'topics': base_topics, + 'max_team_size': 5 }) cls.test_course_1 = CourseFactory.create( @@ -1968,3 +1970,23 @@ class TestBulkMembershipManagement(TeamAPITestCase): 400, method='post', data={'csv': csv_file}, user='staff' ) + + def test_upload_learners_exceed_max_team_size(self): + csv_content = 'user,mode,topic_0,topic_1' + '\n' + team1 = 'team wind power' + team2 = 'team 2' + for name_enum in enumerate(['a', 'b', 'c', 'd', 'e', 'f', 'g']): + username = 'user_{}'.format(name_enum[1]) + self.create_and_enroll_student(username=username, mode=CourseMode.MASTERS) + csv_content += '{},masters,{},{}'.format(username, team1, team2) + '\n' + + 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) + response = self.make_call(reverse( + 'team_membership_bulk_management', + args=[self.good_course_id]), + 400, method='post', + data={'csv': csv_file}, user='staff' + ) + response_text = json.loads(response.content.decode('utf-8')) + self.assertEqual(response_text['errors'][0], 'Team {} is full.'.format(team1))