Merge pull request #258 from aboudreault/cohorts-rebase

Cohorts rebase

common/djangoapps/course_groups/cohorts.py

@@ -291,6 +291,36 @@ def is_user_in_cohort(cohort, user_id, group_type=CourseUserGroup.COHORT):
         group_type=group_type).exists()
 
 
+def remove_user_from_cohort(cohort, username_or_email):
+    """
+    Look up the given user, and if successful, remove them to the specified cohort.
+
+    Arguments:
+        cohort: CourseUserGroup
+        username_or_email: string.  Treated as email if has '@'
+
+    Returns:
+        User object that has been removed from the cohort
+
+    Raises:
+        User.DoesNotExist if can't find user.
+
+        ValueError if user is not in this cohort.
+    """
+
+    user = get_user_by_username_or_email(username_or_email)
+
+    course_cohorts = CourseUserGroup.objects.filter(
+        course_id=cohort.course_id,
+        users__id=user.id,
+        group_type=CourseUserGroup.COHORT,
+    )
+    if course_cohorts.exists():
+        cohort.users.remove(user)
+
+    return user
+
+
 def get_course_cohort_names(course_key):
     """
     Return a list of the cohort names in a course.

common/djangoapps/course_groups/management/commands/revert_workgroup.py

+from django.core.management.base import BaseCommand, CommandError
+from django.core.exceptions import MultipleObjectsReturned
+from optparse import make_option
+
+from django.contrib.auth.models import User
+
+from xmodule.modulestore.django import modulestore
+from xmodule.course_module import CourseDescriptor
+from course_groups.models import CourseUserGroup
+from course_groups.cohorts import (
+    get_cohort,
+    get_cohort_by_name,
+    add_cohort,
+    add_user_to_cohort,
+    remove_user_from_cohort
+)
+from student.models import CourseEnrollment
+
+
+class Command(BaseCommand):
+    option_list = BaseCommand.option_list + (
+        make_option('--fix',
+            action='store_true',
+            dest='fix',
+            default=False,
+            help='Apply possible fixes automatically'),
+    )
+    help = 'Revert the multiple cohorts feature.'
+
+    def handle(self, *args, **options):
+
+        self.stdout.write('### Checking CourseUserGroup group types\n')
+        error = False
+        for course_group in CourseUserGroup.objects.all():
+            if course_group.group_type != CourseUserGroup.COHORT:
+                if options['fix']:
+                    self.stdout.write(
+                        'Fixed: CourseUserGroup with an invalid group_type found: {} (type: {})\n'.format(
+                            course_group.name, course_group.group_type)
+                    )
+                    course_group.group_type = CourseUserGroup.COHORT
+                    course_group.save()
+                else:
+                    error = True
+                    self.stdout.write(
+                        'CourseUserGroup with an invalid group_type found: {} (type: {})\n'.format(
+                            course_group.name, course_group.group_type)
+                    )
+
+        if not error:
+            self.stdout.write('Ok.\n')
+
+        self.stdout.write('\n### Checking user cohorts\n')
+        error = False
+        users = User.objects.all()
+        _courses = modulestore().get_courses()
+        courses = [c for c in _courses if isinstance(c, CourseDescriptor)]
+        # for each course, check if users are in atleast and only 1 cohort
+        for course in courses:
+            for user in users:
+                if not CourseEnrollment.is_enrolled(user, course.id):
+                    continue
+                try:
+                    CourseUserGroup.objects.get(course_id=course.id,
+                                                users__id=user.id)
+                except CourseUserGroup.DoesNotExist:
+                    if options['fix']:
+                        # create a "default_cohort" is it doesn't already exist
+                        try:
+                            default_cohort = get_cohort_by_name(course.id, CourseUserGroup.default_cohort_name)
+                        except CourseUserGroup.DoesNotExist:
+                            default_cohort = add_cohort(course.id, CourseUserGroup.default_cohort_name)
+                            self.stdout.write('Default cohort "{}" created for course "{}"'.format(
+                                default_cohort.name, course.display_name)
+                            )
+                        add_user_to_cohort(default_cohort, user.username)
+                        self.stdout.write(
+                            'Fixed: User "{}" is not in a cohort in course "{}". Added in "{}" cohort\n'.format(
+                                user.username, course.display_name, default_cohort.name)
+                        )
+                    else:
+                        error = True
+                        self.stdout.write(
+                            'User "{}" is not in a cohort in course "{}".\n'.format(
+                                user.username, course.display_name)
+                        )
+                except MultipleObjectsReturned:
+                    self.stdout.write(
+                        'User "{}" is in multiple cohorts in course "{}".\n'.format(
+                            user.username, course.display_name)
+                    )
+                    if options['fix']:
+                        user_cohorts = CourseUserGroup.objects.filter(course_id=course.id,
+                                                                      users__id=user.id).all()
+                        user_cohort = user_cohorts[0]
+                        for cohort in user_cohorts[1:]:
+                            remove_user_from_cohort(cohort, user.username)
+                            self.stdout.write("User '{}' has been removed from cohort '{}' in course '{}'.\n".format(
+                                user.username, cohort.name, course.display_name)
+                            )
+
+                        self.stdout.write("User '{}' is now only in cohort '{}' in course '{}'.\n".format(
+                            user.username, cohort.name, course.display_name)
+                        )
+                    else:
+                        error = True
+
+        if not error:
+            self.stdout.write('Ok.\n')
+
+        self.stdout.write('\nTo fix issues, run the script with the "--fix" option.\n')

common/djangoapps/course_groups/models.py

@@ -13,6 +13,8 @@ class CourseUserGroup(models.Model):
     which may be treated specially.  For example, a user can be in at most one cohort per
     course, and cohorts are used to split up the forums by group.
     """
+    default_cohort_name = "default_cohort"
+
     class Meta:
         unique_together = (('name', 'course_id'), )
 

common/djangoapps/course_groups/views.py

@@ -228,7 +228,7 @@ def remove_user_from_cohort(request, course_key_string, cohort_id):
     cohort = cohorts.get_cohort_by_id(course_key, cohort_id)
     try:
         user = User.objects.get(username=username)
-        cohort.users.remove(user)
+        cohorts.remove_user_from_cohort(cohort, user.username)
         return json_http_response({'success': True})
     except User.DoesNotExist:
         log.debug('no user')

lms/djangoapps/api_manager/users/views.py

@@ -18,6 +18,12 @@ from rest_framework.response import Response
 from courseware import grades, module_render
 from courseware.model_data import FieldDataCache
 from courseware.views import get_module_for_descriptor, save_child_position, get_current_child
+from course_groups.models import CourseUserGroup
+from course_groups.cohorts import (
+    get_cohort_by_name,
+    add_cohort,
+    add_user_to_cohort
+)
 from django_comment_common.models import Role, FORUM_ROLE_MODERATOR
 from gradebook.models import StudentGradebook
 from instructor.access import revoke_access, update_forum_role
@@ -716,6 +722,15 @@ class UsersCoursesList(SecureAPIView):
             return Response({}, status=status.HTTP_404_NOT_FOUND)
         base_uri = generate_base_uri(request)
         course_enrollment = CourseEnrollment.enroll(user, course_key)
+        # Ensure the user is in a cohort. Add it explicitly in the default_cohort
+        try:
+            default_cohort = get_cohort_by_name(course_id, CourseUserGroup.default_cohort_name)
+        except CourseUserGroup.DoesNotExist:
+            default_cohort = add_cohort(course_id, CourseUserGroup.default_cohort_name)
+        add_user_to_cohort(default_cohort, user.username)
+        log.debug('User "{}" has been automatically added in cohort "{}" for course "{}"'.format(
+            user.username, default_cohort.name, course_descriptor.display_name)
+        )
         response_data['uri'] = '{}/{}'.format(base_uri, course_id)
         response_data['id'] = unicode(course_key)
         response_data['name'] = course_descriptor.display_name

lms/djangoapps/projects/models.py

@@ -2,6 +2,7 @@
 
 from django.contrib.auth.models import Group, User
 from django.db import models
+from django.core.exceptions import ValidationError
 
 from model_utils.models import TimeStampedModel
 
@@ -34,7 +35,8 @@ class Workgroup(TimeStampedModel):
     """
     name = models.CharField(max_length=255, null=True, blank=True)
     project = models.ForeignKey(Project, related_name="workgroups")
-    users = models.ManyToManyField(User, related_name="workgroups", blank=True, null=True)
+    users = models.ManyToManyField(User, related_name="workgroups",
+                                   through="WorkgroupUser", blank=True, null=True)
     groups = models.ManyToManyField(Group, related_name="workgroups", blank=True, null=True)
 
     @property
@@ -53,6 +55,35 @@ class Workgroup(TimeStampedModel):
             workgroup_name
         )
 
+    def add_user(self, user):
+        workgroup_user = WorkgroupUser(workgroup=self, user=user)
+        workgroup_user.save()
+
+    def remove_user(self, user):
+        workgroup_user = WorkgroupUser.objects.get(workgroup=self, user=user)
+        workgroup_user.delete()
+
+
+class WorkgroupUser(models.Model):
+    """A Folder to store some data between a client and its insurance"""
+
+    workgroup = models.ForeignKey(Workgroup, null=False)
+    user = models.ForeignKey(User, null=False)
+
+    class Meta:
+        db_table = 'projects_workgroup_users'
+
+    def clean(self):
+        # Ensure the user is not already assigned to a workgroup for this course
+        existing_workgroups = Workgroup.objects.filter(users=self.user).filter(project__course_id=self.workgroup.project.course_id)
+        if len(existing_workgroups):
+            raise ValidationError('User {} is already assigned to a workgroup for this course'.format(self.user.username))
+
+    def save(self, **kwargs):
+        self.clean()
+        return super(WorkgroupUser, self).save(**kwargs)
+
+
 class WorkgroupReview(TimeStampedModel):
     """
     Model representing the Workgroup Review concept.  A Workgroup Review is

lms/djangoapps/projects/tests/test_peer_reviews.py

@@ -84,7 +84,7 @@ class PeerReviewsApiTests(TestCase):
             name="Test Workgroup",
             project=self.test_project,
         )
-        self.test_workgroup.users.add(self.test_peer_user)
+        self.test_workgroup.add_user(self.test_peer_user)
         self.test_workgroup.save()
 
         self.client = SecureClient()

lms/djangoapps/projects/tests/test_projects.py

@@ -58,7 +58,7 @@ class ProjectsApiTests(TestCase):
             name="Test Workgroup",
             project=self.test_project,
         )
-        self.test_workgroup.users.add(self.test_user)
+        self.test_workgroup.add_user(self.test_user)
         self.test_workgroup.save()
 
         self.client = SecureClient()

lms/djangoapps/projects/tests/test_submission_reviews.py

@@ -77,7 +77,7 @@ class SubmissionReviewsApiTests(TestCase):
             name="Test Workgroup",
             project=self.test_project,
         )
-        self.test_workgroup.users.add(self.test_user)
+        self.test_workgroup.add_user(self.test_user)
         self.test_workgroup.save()
 
         self.test_submission = WorkgroupSubmission.objects.create(

lms/djangoapps/projects/tests/test_workgroup_reviews.py

@@ -77,7 +77,7 @@ class WorkgroupReviewsApiTests(TestCase):
             name="Test Workgroup",
             project=self.test_project,
         )
-        self.test_workgroup.users.add(self.test_user)
+        self.test_workgroup.add_user(self.test_user)
         self.test_workgroup.save()
 
         self.test_submission = WorkgroupSubmission.objects.create(

lms/djangoapps/projects/tests/test_workgroup_submissions.py

@@ -64,7 +64,7 @@ class SubmissionsApiTests(TestCase):
             name="Test Workgroup",
             project=self.test_project,
         )
-        self.test_workgroup.users.add(self.test_user)
+        self.test_workgroup.add_user(self.test_user)
         self.test_workgroup.save()
 
         self.client = SecureClient()

lms/djangoapps/projects/tests/test_workgroups.py

@@ -182,7 +182,7 @@ class WorkgroupsApiTests(TestCase):
             response.data['id'],
             self.test_workgroup_name
         )
-        cohort = get_cohort_by_name(self.test_course.id, cohort_name, CourseUserGroup.WORKGROUP)
+        cohort = get_cohort_by_name(self.test_course.id, cohort_name)
         self.assertIsNotNone(cohort)
 
     def test_workgroups_detail_get(self):
@@ -276,9 +276,9 @@ class WorkgroupsApiTests(TestCase):
             response.data['id'],
             self.test_workgroup_name
         )
-        cohort = get_cohort_by_name(self.test_course.id, cohort_name, CourseUserGroup.WORKGROUP)
+        cohort = get_cohort_by_name(self.test_course.id, cohort_name)
         self.assertIsNotNone(cohort)
-        self.assertTrue(is_user_in_cohort(cohort, self.test_user.id, CourseUserGroup.WORKGROUP))
+        self.assertTrue(is_user_in_cohort(cohort, self.test_user.id))
 
 
     def test_workgroups_users_post_preexisting_workgroup(self):
@@ -360,15 +360,15 @@ class WorkgroupsApiTests(TestCase):
         )
 
         # now let's remove existing cohort users
-        cohort = get_cohort_by_name(self.test_course.id, cohort_name, CourseUserGroup.WORKGROUP)
-        self.assertTrue(is_user_in_cohort(cohort, self.test_user.id, CourseUserGroup.WORKGROUP))
+        cohort = get_cohort_by_name(self.test_course.id, cohort_name)
+        self.assertTrue(is_user_in_cohort(cohort, self.test_user.id))
 
-        remove_user_from_cohort(cohort, self.test_user.username, CourseUserGroup.WORKGROUP)
-        self.assertFalse(is_user_in_cohort(cohort, self.test_user.id, CourseUserGroup.WORKGROUP))
+        remove_user_from_cohort(cohort, self.test_user.username)
+        self.assertFalse(is_user_in_cohort(cohort, self.test_user.id))
 
         # delete cohort
-        delete_empty_cohort(self.test_course.id, cohort_name, CourseUserGroup.WORKGROUP)
-        self.assertEqual(0, len(get_course_cohort_names(self.test_course.id, CourseUserGroup.WORKGROUP)))
+        delete_empty_cohort(self.test_course.id, cohort_name)
+        self.assertEqual(0, len(get_course_cohort_names(self.test_course.id)))
 
         # add a 2nd user and make sure a discussion cohort was created and users were backfilled
         test_uri = '{}{}/'.format(self.test_workgroups_uri, str(response.data['id']))
@@ -378,10 +378,10 @@ class WorkgroupsApiTests(TestCase):
         self.assertEqual(response.status_code, 201)
 
         # now inspect cohort and assert that things are as we anticipate (i.e. both users are in there)
-        cohort = get_cohort_by_name(self.test_course.id, cohort_name, CourseUserGroup.WORKGROUP)
+        cohort = get_cohort_by_name(self.test_course.id, cohort_name)
         self.assertIsNotNone(cohort)
-        self.assertTrue(is_user_in_cohort(cohort, self.test_user.id, CourseUserGroup.WORKGROUP))
-        self.assertTrue(is_user_in_cohort(cohort, self.test_user2.id, CourseUserGroup.WORKGROUP))
+        self.assertTrue(is_user_in_cohort(cohort, self.test_user.id))
+        self.assertTrue(is_user_in_cohort(cohort, self.test_user2.id))
 
     def test_workgroups_users_delete(self):
         data = {

lms/djangoapps/projects/views.py

@@ -3,7 +3,7 @@
 
 """ WORKGROUPS API VIEWS """
 from django.contrib.auth.models import Group, User
-from django.core.exceptions import ObjectDoesNotExist
+from django.core.exceptions import ObjectDoesNotExist, ValidationError
 
 from rest_framework import viewsets
 from rest_framework.decorators import action, link
@@ -22,6 +22,8 @@ from opaque_keys.edx.keys import CourseKey, UsageKey
 from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location
 from xmodule.modulestore import Location, InvalidLocationError
 from xmodule.modulestore.django import modulestore
+from course_groups.cohorts import (add_cohort, add_user_to_cohort, get_cohort_by_name,
+                                   remove_user_from_cohort)
 
 from .models import Project, Workgroup, WorkgroupSubmission
 from .models import WorkgroupReview, WorkgroupSubmissionReview, WorkgroupPeerReview
@@ -111,6 +113,20 @@ class WorkgroupsViewSet(viewsets.ModelViewSet):
     serializer_class = WorkgroupSerializer
     model = Workgroup
 
+    def create(self, request):
+        """
+        Create a new workgroup and its cohort.
+        """
+        response = super(WorkgroupsViewSet, self).create(request)
+
+        if response.status_code == status.HTTP_201_CREATED:
+            # create the workgroup cohort
+            workgroup = self.object
+            course_descriptor, course_key, course_content = _get_course(self.request, self.request.user, workgroup.project.course_id)  # pylint: disable=W0612
+            add_cohort(course_key, workgroup.cohort_name)
+
+        return response
+
     @action(methods=['get', 'post'])
     def groups(self, request, pk):
         """
@@ -134,7 +150,6 @@ class WorkgroupsViewSet(viewsets.ModelViewSet):
             workgroup = self.get_object()
             workgroup.groups.add(group)
             workgroup.save()
-            print workgroup.groups.all()
             return Response({}, status=status.HTTP_201_CREATED)
 
     @action(methods=['get', 'post', 'delete'])
@@ -160,21 +175,31 @@ class WorkgroupsViewSet(viewsets.ModelViewSet):
 
             workgroup = self.get_object()
 
-            # Ensure the user is not already assigned to a workgroup for this project
-            existing_workgroups = Workgroup.objects.filter(users=user).filter(project=workgroup.project)
-            if len(existing_workgroups):
-                message = 'User {} already assigned to a workgroup for this project'.format(user_id)
-                return Response({"detail": message}, status.HTTP_400_BAD_REQUEST)
-
             # Ensure the user is not already assigned to a project for this course
             existing_projects = Project.objects.filter(course_id=workgroup.project.course_id).filter(workgroups__users__id=user.id)
             if len(existing_projects):
                 message = 'User {} already assigned to a project for this course'.format(user_id)
                 return Response({"detail": message}, status.HTTP_400_BAD_REQUEST)
 
-            workgroup.users.add(user)
+            try:
+                workgroup.add_user(user)
+            except ValidationError as e:
+                return Response({"detail": unicode(e)}, status.HTTP_400_BAD_REQUEST)
+
             workgroup.save()
 
+            # add user to the workgroup cohort, create it if it doesn't exist (for cases where there is a legacy
+            # workgroup)
+            course_descriptor, course_key, course_content = _get_course(self.request, user, workgroup.project.course_id)  # pylint: disable=W0612
+            try:
+                cohort = get_cohort_by_name(course_key, workgroup.cohort_name)
+                add_user_to_cohort(cohort, user.username)
+            except ObjectDoesNotExist:
+                # This use case handles cases where a workgroup might have been created before
+                # the notion of a cohorted discussion. So we need to backfill in the data
+                cohort = add_cohort(course_key, workgroup.cohort_name)
+                for workgroup_user in workgroup.users.all():
+                    add_user_to_cohort(cohort, workgroup_user.username)
             return Response({}, status=status.HTTP_201_CREATED)
         else:
             user_id = request.DATA.get('id')
@@ -184,7 +209,11 @@ class WorkgroupsViewSet(viewsets.ModelViewSet):
                 message = 'User {} does not exist'.format(user_id)
                 return Response({"detail": message}, status.HTTP_400_BAD_REQUEST)
             workgroup = self.get_object()
-            workgroup.users.remove(user)
+            course_descriptor, course_key, course_content = _get_course(self.request, user, workgroup.project.course_id)  # pylint: disable=W0612
+            cohort = get_cohort_by_name(course_key,
+                                        workgroup.cohort_name)
+            workgroup.remove_user(user)
+            remove_user_from_cohort(cohort, user.username)
             return Response({}, status=status.HTTP_204_NO_CONTENT)
 
     @link()