Rebase of workgroup cohorts change

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

@@ -83,7 +83,7 @@ class PeerReviewsApiTests(ModuleStoreTestCase):
             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

@@ -76,7 +76,7 @@ class SubmissionReviewsApiTests(ModuleStoreTestCase):
             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(ModuleStoreTestCase):
             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(ModuleStoreTestCase):
             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(ModuleStoreTestCase):
             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(ModuleStoreTestCase):
         )
 
         # 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(ModuleStoreTestCase):
         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,9 @@ 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 openedx.core.djangoapps.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 +114,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 +151,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 +176,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 +210,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()

openedx/core/djangoapps/course_groups/cohorts.py

@@ -420,6 +420,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_group_info_for_cohort(cohort, use_cached=False):
     """
     Get the ids of the group and partition to which this cohort has been linked

openedx/core/djangoapps/course_groups/views.py

@@ -387,7 +387,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')