Merge pull request #147 from edx-solutions/cdodge/fix-workgroup-cohort-feature

create workgroup cohort if it does not exist when adding a user to a coh...
Conflicts:
	lms/djangoapps/projects/views.py

common/djangoapps/course_groups/cohorts.py

@@ -262,7 +262,7 @@ def add_user_to_cohort(cohort, username_or_email):
     """
     user = get_user_by_username_or_email(username_or_email)
     previous_cohort = None
-
+    
     course_cohorts = CourseUserGroup.objects.filter(
         course_id=cohort.course_id,
         users__id=user.id,
@@ -281,6 +281,16 @@ def add_user_to_cohort(cohort, username_or_email):
     return (user, previous_cohort)
 
 
+def is_user_in_cohort(cohort, user_id, group_type=CourseUserGroup.COHORT):
+    """
+    Returns True or False if a user is in a cohort
+    """
+    return CourseUserGroup.objects.filter(
+        course_id=cohort.course_id,
+        users__id=user_id,
+        group_type=group_type).exists()
+
+
 def get_course_cohort_names(course_key):
     """
     Return a list of the cohort names in a course.

lms/djangoapps/projects/models.py

@@ -37,6 +37,21 @@ class Workgroup(TimeStampedModel):
     users = models.ManyToManyField(User, related_name="workgroups", blank=True, null=True)
     groups = models.ManyToManyField(Group, related_name="workgroups", blank=True, null=True)
 
+    @property
+    def cohort_name(self):
+        return Workgroup.cohort_name_for_workgroup(
+            self.project.id,
+            self.id,
+            self.name
+        )
+
+    @classmethod
+    def cohort_name_for_workgroup(cls, project_id, workgroup_id, workgroup_name):
+        return 'Group Project {} Workgroup {} ({})'.format(
+            project_id,
+            workgroup_id,
+            workgroup_name
+        )
 
 class WorkgroupReview(TimeStampedModel):
     """

lms/djangoapps/projects/tests/test_workgroups.py

@@ -15,8 +15,11 @@ from django.test.utils import override_settings
 
 from api_manager.models import GroupProfile
 from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
-from projects.models import Project
+from projects.models import Project, Workgroup
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
+from course_groups.cohorts import (get_cohort_by_name, remove_user_from_cohort,
+                                   delete_empty_cohort, is_user_in_cohort, get_course_cohort_names)
+from course_groups.models import CourseUserGroup
 
 TEST_API_KEY = str(uuid.uuid4())
 
@@ -163,6 +166,15 @@ class WorkgroupsApiTests(TestCase):
         self.assertIsNotNone(response.data['created'])
         self.assertIsNotNone(response.data['modified'])
 
+        # make sure a discussion cohort was created
+        cohort_name = Workgroup.cohort_name_for_workgroup(
+            self.test_project.id,
+            response.data['id'],
+            self.test_workgroup_name
+        )
+        cohort = get_cohort_by_name(self.test_project.course_id, cohort_name, CourseUserGroup.WORKGROUP)
+        self.assertIsNotNone(cohort)
+
     def test_workgroups_detail_get(self):
         data = {
             'name': self.test_workgroup_name,
@@ -248,6 +260,65 @@ class WorkgroupsApiTests(TestCase):
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data['users'][0]['id'], self.test_user.id)
 
+        # make sure a discussion cohort was created
+        cohort_name = Workgroup.cohort_name_for_workgroup(
+            self.test_project.id,
+            response.data['id'],
+            self.test_workgroup_name
+        )
+        cohort = get_cohort_by_name(self.test_project.course_id, cohort_name, CourseUserGroup.WORKGROUP)
+        self.assertIsNotNone(cohort)
+        self.assertTrue(is_user_in_cohort(cohort, self.test_user.id, CourseUserGroup.WORKGROUP))
+
+    def test_workgroups_users_post_with_cohort_backfill(self):
+        """
+        This test asserts a case where a workgroup was created before the existence of a cohorted discussion
+        """
+        data = {
+            'name': self.test_workgroup_name,
+            'project': self.test_project.id
+        }
+        response = self.do_post(self.test_workgroups_uri, data)
+        self.assertEqual(response.status_code, 201)
+        test_uri = '{}{}/'.format(self.test_workgroups_uri, str(response.data['id']))
+        users_uri = '{}users/'.format(test_uri)
+        data = {"id": self.test_user.id}
+        response = self.do_post(users_uri, data)
+        self.assertEqual(response.status_code, 201)
+        response = self.do_get(test_uri)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.data['users'][0]['id'], self.test_user.id)
+
+        cohort_name = Workgroup.cohort_name_for_workgroup(
+            self.test_project.id,
+            response.data['id'],
+            self.test_workgroup_name
+        )
+
+        # now let's remove existing cohort users
+        cohort = get_cohort_by_name(self.test_project.course_id, cohort_name, CourseUserGroup.WORKGROUP)
+        self.assertTrue(is_user_in_cohort(cohort, self.test_user.id, CourseUserGroup.WORKGROUP))
+
+        remove_user_from_cohort(cohort, self.test_user.username, CourseUserGroup.WORKGROUP)
+        self.assertFalse(is_user_in_cohort(cohort, self.test_user.id, CourseUserGroup.WORKGROUP))
+
+        # delete cohort
+        delete_empty_cohort(self.test_project.course_id, cohort_name, CourseUserGroup.WORKGROUP)
+        self.assertEqual(0, len(get_course_cohort_names(self.test_project.course_id, CourseUserGroup.WORKGROUP)))
+
+        # 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']))
+        users_uri = '{}users/'.format(test_uri)
+        data = {"id": self.test_user2.id}
+        response = self.do_post(users_uri, data)
+        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_project.course_id, cohort_name, CourseUserGroup.WORKGROUP)
+        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))
+
     def test_workgroups_users_delete(self):
         data = {
             'name': self.test_workgroup_name,

lms/djangoapps/projects/views.py

@@ -160,6 +160,7 @@ class WorkgroupsViewSet(viewsets.ModelViewSet):
             workgroup = self.get_object()
             workgroup.users.add(user)
             workgroup.save()
+
             return Response({}, status=status.HTTP_201_CREATED)
         else:
             user_id = request.DATA.get('id')