Staff and instructor users of the master course are added to CCX created via REST APIs

lms/djangoapps/ccx/api/v0/tests/test_views.py

@@ -8,6 +8,7 @@
 import string
 import urllib
 import urlparse
+from itertools import izip
 
 import ddt
 import mock
@@ -30,6 +31,7 @@
 from courseware import courses
 from ccx_keys.locator import CCXLocator
 from student.models import CourseEnrollment
+from instructor.access import list_with_level
 from instructor.enrollment import (
     enroll_email,
     get_email_params,
@@ -38,6 +40,7 @@
 from lms.djangoapps.ccx.models import CcxFieldOverride, CustomCourseForEdX
 from lms.djangoapps.ccx.overrides import override_field_for_ccx
 from lms.djangoapps.ccx.tests.utils import CcxTestCase
+from lms.djangoapps.ccx.utils import ccx_course as ccx_course_cm
 from opaque_keys.edx.keys import CourseKey
 from student.roles import CourseCcxCoachRole
 from student.tests.factories import AdminFactory
@@ -459,6 +462,38 @@ def test_post_list(self):
         self.assertEqual(len(outbox), 1)
         self.assertIn(self.coach.email, outbox[0].recipients())  # pylint: disable=no-member
 
+    def test_post_list_staff_master_course_in_ccx(self):
+        """
+        Specific test to check that the staff and instructor of the master
+        course are assigned to the CCX.
+        """
+        outbox = self.get_outbox()
+        data = {
+            'master_course_id': self.master_course_key_str,
+            'max_students_allowed': 111,
+            'display_name': 'CCX Test Title',
+            'coach_email': self.coach.email
+        }
+        resp = self.client.post(self.list_url, data, format='json', HTTP_AUTHORIZATION=self.auth)
+        self.assertEqual(resp.status_code, status.HTTP_201_CREATED)
+        # check that only one email has been sent and it is to to the coach
+        self.assertEqual(len(outbox), 1)
+        self.assertIn(self.coach.email, outbox[0].recipients())  # pylint: disable=no-member
+
+        list_staff_master_course = list_with_level(self.course, 'staff')
+        list_instructor_master_course = list_with_level(self.course, 'instructor')
+        course_key = CourseKey.from_string(resp.data.get('ccx_course_id'))  # pylint: disable=no-member
+        with ccx_course_cm(course_key) as course_ccx:
+            list_staff_ccx_course = list_with_level(course_ccx, 'staff')
+            list_instructor_ccx_course = list_with_level(course_ccx, 'instructor')
+
+        self.assertEqual(len(list_staff_master_course), len(list_staff_ccx_course))
+        for course_user, ccx_user in izip(sorted(list_staff_master_course), sorted(list_staff_ccx_course)):
+            self.assertEqual(course_user, ccx_user)
+        self.assertEqual(len(list_instructor_master_course), len(list_instructor_ccx_course))
+        for course_user, ccx_user in izip(sorted(list_instructor_master_course), sorted(list_instructor_ccx_course)):
+            self.assertEqual(course_user, ccx_user)
+
 
 @attr('shard_1')
 @ddt.ddt

lms/djangoapps/ccx/api/v0/views.py

@@ -33,6 +33,7 @@
     override_field_for_ccx,
 )
 from lms.djangoapps.ccx.utils import (
+    add_master_course_staff_to_ccx,
     assign_coach_role_to_ccx,
     is_email,
 )
@@ -433,6 +434,13 @@ def post(self, request):
             )
             # assign coach role for the coach to the newly created ccx
             assign_coach_role_to_ccx(ccx_course_key, coach, master_course_object.id)
+            # assign staff role for all the staff and instructor of the master course to the newly created ccx
+            add_master_course_staff_to_ccx(
+                master_course_object,
+                ccx_course_key,
+                ccx_course_object.display_name,
+                send_email=False
+            )
 
         serializer = self.get_serializer(ccx_course_object)
         return Response(

lms/djangoapps/ccx/tests/test_utils.py

@@ -1,6 +1,7 @@
 """
 test utils
 """
+import uuid
 from nose.plugins.attrib import attr
 
 from ccx_keys.locator import CCXLocator
@@ -21,10 +22,14 @@
 from xmodule.modulestore.tests.factories import CourseFactory
 from xmodule.modulestore.django import modulestore
 
-from lms.djangoapps.instructor.access import list_with_level, allow_access
-
-from lms.djangoapps.ccx.utils import add_master_course_staff_to_ccx
-from lms.djangoapps.ccx.views import ccx_course
+from lms.djangoapps.instructor.access import (
+    allow_access,
+    list_with_level,
+)
+from lms.djangoapps.ccx.utils import (
+    add_master_course_staff_to_ccx,
+    ccx_course,
+)
 from lms.djangoapps.ccx.tests.factories import CcxFactory
 from lms.djangoapps.ccx.tests.utils import CcxTestCase
 
@@ -62,7 +67,7 @@ def test_ccx_locator(self):
         self.assertEqual(result, ccx)
 
 
-class TestStaffOnCCX(CcxTestCase, SharedModuleStoreTestCase):
+class TestStaffOnCCX(CcxTestCase):
     """
     Tests for staff on ccx courses.
     """
@@ -87,24 +92,91 @@ def setUp(self):
         allow_access(self.course, instructor, 'instructor')
         self.assertTrue(CourseInstructorRole(self.course.id).has_user(instructor))
 
+        self.make_coach()
+        self.ccx = self.make_ccx()
+        self.ccx_locator = CCXLocator.from_course_locator(self.course.id, self.ccx.id)
+
     def test_add_master_course_staff_to_ccx(self):
         """
         Test add staff of master course to ccx course
         """
-        self.make_coach()
-        ccx = self.make_ccx()
-        ccx_locator = CCXLocator.from_course_locator(self.course.id, ccx.id)
-        add_master_course_staff_to_ccx(self.course, ccx_locator, ccx.display_name)
+        add_master_course_staff_to_ccx(self.course, self.ccx_locator, self.ccx.display_name)
 
         # assert that staff and instructors of master course has staff and instructor roles on ccx
         list_staff_master_course = list_with_level(self.course, 'staff')
         list_instructor_master_course = list_with_level(self.course, 'instructor')
 
-        with ccx_course(ccx_locator) as course_ccx:
+        with ccx_course(self.ccx_locator) as course_ccx:
             list_staff_ccx_course = list_with_level(course_ccx, 'staff')
             self.assertEqual(len(list_staff_master_course), len(list_staff_ccx_course))
             self.assertEqual(list_staff_master_course[0].email, list_staff_ccx_course[0].email)
 
             list_instructor_ccx_course = list_with_level(course_ccx, 'instructor')
             self.assertEqual(len(list_instructor_ccx_course), len(list_instructor_master_course))
             self.assertEqual(list_instructor_ccx_course[0].email, list_instructor_master_course[0].email)
+
+    def test_add_master_course_staff_to_ccx_display_name(self):
+        """
+        Test add staff of master course to ccx course.
+        Specific test to check that a passed display name is in the
+        subject of the email sent to the enrolled users.
+        """
+        outbox = self.get_outbox()
+        # create a unique display name
+        display_name = 'custom_display_{}'.format(uuid.uuid4())
+        list_staff_master_course = list_with_level(self.course, 'staff')
+        list_instructor_master_course = list_with_level(self.course, 'instructor')
+        self.assertEqual(len(outbox), 0)
+        # give access to the course staff/instructor
+        add_master_course_staff_to_ccx(self.course, self.ccx_locator, display_name)
+        self.assertEqual(len(outbox), len(list_staff_master_course) + len(list_instructor_master_course))
+        for email in outbox:
+            self.assertIn(display_name, email.subject)
+
+    def test_add_master_course_staff_to_ccx_idempotent(self):
+        """
+        Test add staff of master course to ccx course multiple time will
+        not result in multiple enrollments.
+        """
+        outbox = self.get_outbox()
+        list_staff_master_course = list_with_level(self.course, 'staff')
+        list_instructor_master_course = list_with_level(self.course, 'instructor')
+        self.assertEqual(len(outbox), 0)
+
+        # run the assignment the first time
+        add_master_course_staff_to_ccx(self.course, self.ccx_locator, self.ccx.display_name)
+        self.assertEqual(len(outbox), len(list_staff_master_course) + len(list_instructor_master_course))
+        with ccx_course(self.ccx_locator) as course_ccx:
+            list_staff_ccx_course = list_with_level(course_ccx, 'staff')
+            list_instructor_ccx_course = list_with_level(course_ccx, 'instructor')
+        self.assertEqual(len(list_staff_master_course), len(list_staff_ccx_course))
+        for user in list_staff_master_course:
+            self.assertIn(user, list_staff_ccx_course)
+        self.assertEqual(len(list_instructor_master_course), len(list_instructor_ccx_course))
+        for user in list_instructor_master_course:
+            self.assertIn(user, list_instructor_ccx_course)
+
+        # run the assignment again
+        add_master_course_staff_to_ccx(self.course, self.ccx_locator, self.ccx.display_name)
+        # there are no new duplicated email
+        self.assertEqual(len(outbox), len(list_staff_master_course) + len(list_instructor_master_course))
+        # there are no duplicated staffs
+        with ccx_course(self.ccx_locator) as course_ccx:
+            list_staff_ccx_course = list_with_level(course_ccx, 'staff')
+            list_instructor_ccx_course = list_with_level(course_ccx, 'instructor')
+        self.assertEqual(len(list_staff_master_course), len(list_staff_ccx_course))
+        for user in list_staff_master_course:
+            self.assertIn(user, list_staff_ccx_course)
+        self.assertEqual(len(list_instructor_master_course), len(list_instructor_ccx_course))
+        for user in list_instructor_master_course:
+            self.assertIn(user, list_instructor_ccx_course)
+
+    def test_add_master_course_staff_to_ccx_no_email(self):
+        """
+        Test add staff of master course to ccx course without
+        sending enrollment email.
+        """
+        outbox = self.get_outbox()
+        self.assertEqual(len(outbox), 0)
+        add_master_course_staff_to_ccx(self.course, self.ccx_locator, self.ccx.display_name, send_email=False)
+        self.assertEqual(len(outbox), 0)

lms/djangoapps/ccx/tests/test_views.py

@@ -15,7 +15,10 @@
 from courseware.tests.factories import StudentModuleFactory
 from courseware.tests.helpers import LoginEnrollmentTestCase
 from courseware.tabs import get_course_tab_list
-from instructor.access import list_with_level, allow_access
+from instructor.access import (
+    allow_access,
+    list_with_level,
+)
 
 from django.conf import settings
 from django.core.urlresolvers import reverse, resolve

lms/djangoapps/ccx/utils.py

@@ -21,7 +21,11 @@
     get_email_params,
     unenroll_email,
 )
-from instructor.access import allow_access, list_with_level, revoke_access
+from instructor.access import (
+    allow_access,
+    list_with_level,
+    revoke_access,
+)
 from instructor.views.tools import get_student_from_identifier
 from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from student.models import CourseEnrollment
@@ -263,7 +267,7 @@ def is_email(identifier):
     return True
 
 
-def add_master_course_staff_to_ccx(master_course, ccx_key, display_name):
+def add_master_course_staff_to_ccx(master_course, ccx_key, display_name, send_email=True):
     """
     Added staff role on ccx to all the staff members of master course.
 
@@ -277,31 +281,37 @@ def add_master_course_staff_to_ccx(master_course, ccx_key, display_name):
 
     with ccx_course(ccx_key) as course_ccx:
         email_params = get_email_params(course_ccx, auto_enroll=True, course_key=ccx_key, display_name=display_name)
+        list_staff_ccx = list_with_level(course_ccx, 'staff')
+        list_instructor_ccx = list_with_level(course_ccx, 'instructor')
         for staff in list_staff:
-            # allow 'staff' access on ccx to staff of master course
-            allow_access(course_ccx, staff, 'staff')
-
-            # Enroll the staff in the ccx
-            enroll_email(
-                course_id=ccx_key,
-                student_email=staff.email,
-                auto_enroll=True,
-                email_students=True,
-                email_params=email_params,
-            )
+            # this call should be idempotent
+            if staff not in list_staff_ccx:
+                # allow 'staff' access on ccx to staff of master course
+                allow_access(course_ccx, staff, 'staff')
+
+                # Enroll the staff in the ccx
+                enroll_email(
+                    course_id=ccx_key,
+                    student_email=staff.email,
+                    auto_enroll=True,
+                    email_students=send_email,
+                    email_params=email_params,
+                )
 
         for instructor in list_instructor:
-            # allow 'instructor' access on ccx to instructor of master course
-            allow_access(course_ccx, instructor, 'instructor')
-
-            # Enroll the instructor in the ccx
-            enroll_email(
-                course_id=ccx_key,
-                student_email=instructor.email,
-                auto_enroll=True,
-                email_students=True,
-                email_params=email_params,
-            )
+            # this call should be idempotent
+            if instructor not in list_instructor_ccx:
+                # allow 'instructor' access on ccx to instructor of master course
+                allow_access(course_ccx, instructor, 'instructor')
+
+                # Enroll the instructor in the ccx
+                enroll_email(
+                    course_id=ccx_key,
+                    student_email=instructor.email,
+                    auto_enroll=True,
+                    email_students=send_email,
+                    email_params=email_params,
+                )
 
 
 def reverse_add_master_course_staff_to_ccx(master_course, ccx_key, display_name):