initial layout

interim commit

clean up

Initial unit tests and pylint

pylint cleanup

Unit tests extended and passing. Pylint

WIP - fix Unit tests extended and passing. Pylint

Updated logic to validate before commit.

CR pass1

CR pass1, pep8

pep8 2

interim cr

interim cr2

refactor to using DictReader

removed uneeded mixin

pr comments

encoding fix and return value

Cache teams/membershipos

ncr

cr aed

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': <user_obj>}
+         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

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):

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'
+                       )

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):
         """