Adds staff and instructor users of the master course to CCX

cms/djangoapps/contentstore/views/course.py

@@ -27,6 +27,7 @@
 )
 from .item import create_xblock_info
 from .library import LIBRARIES_ENABLED
+from ccx_keys.locator import CCXLocator
 from contentstore import utils
 from contentstore.course_group_config import (
     COHORT_SCHEME,
@@ -389,6 +390,11 @@ def course_filter(course):
         if isinstance(course, ErrorDescriptor):
             return False
 
+        # Custom Courses for edX (CCX) is an edX feature for re-using course content.
+        # CCXs cannot be edited in Studio (aka cms) and should not be shown in this dashboard.
+        if isinstance(course, CCXLocator):
+            return False
+
         # pylint: disable=fixme
         # TODO remove this condition when templates purged from db
         if course.location.course == 'templates':
@@ -433,8 +439,11 @@ def _accessible_courses_list_from_groups(request):
             except ItemNotFoundError:
                 # If a user has access to a course that doesn't exist, don't do anything with that course
                 pass
-            if course is not None and not isinstance(course, ErrorDescriptor):
-                # ignore deleted or errored courses
+
+            # Custom Courses for edX (CCX) is an edX feature for re-using course content.
+            # CCXs cannot be edited in Studio (aka cms) and should not be shown in this dashboard.
+            if course is not None and not isinstance(course, ErrorDescriptor) and not isinstance(course.id, CCXLocator):
+                # ignore deleted, errored or ccx courses
                 courses_list[course_key] = course
 
     return courses_list.values(), in_process_course_actions

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

@@ -34,6 +34,7 @@
     override_field_for_ccx,
 )
 from lms.djangoapps.ccx.utils import (
+    add_master_course_staff_to_ccx,
     assign_coach_role_to_ccx,
     is_email,
     get_course_chapters,
@@ -506,6 +507,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/migrations/0003_add_master_course_staff_in_ccx.py

@@ -0,0 +1,161 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from ccx_keys.locator import CCXLocator
+from courseware.courses import get_course_by_id
+from contextlib import contextmanager
+from django.db import migrations
+
+from lms.djangoapps.instructor.enrollment import (
+    enroll_email,
+    get_email_params,
+    unenroll_email,
+)
+from lms.djangoapps.instructor.access import (
+    allow_access,
+    list_with_level,
+    revoke_access,
+)
+
+
+@contextmanager
+def ccx_course(ccx_locator):
+    """Create a context in which the course identified by course_locator exists
+    """
+    course = get_course_by_id(ccx_locator)
+    yield course
+
+
+def add_master_course_staff_to_ccx_for_existing_ccx(apps, schema_editor):
+    """
+    Add all staff and admin of master course to respective CCX(s).
+    """
+    CustomCourseForEdX = apps.get_model("ccx", "CustomCourseForEdX")
+    list_ccx = CustomCourseForEdX.objects.all()
+    for ccx in list_ccx:
+        if ccx.course_id.deprecated:
+            # prevent migration for deprecated course ids.
+            continue
+        ccx_locator = CCXLocator.from_course_locator(ccx.course_id, unicode(ccx.id))
+        add_master_course_staff_to_ccx(
+            get_course_by_id(ccx.course_id),
+            ccx_locator,
+            ccx.display_name
+        )
+
+
+def reverse_add_master_course_staff_to_ccx_for_existing_ccx(apps, schema_editor):
+    """
+    Add all staff and admin of master course to respective CCX(s).
+    """
+    CustomCourseForEdX = apps.get_model("ccx", "CustomCourseForEdX")
+    list_ccx = CustomCourseForEdX.objects.all()
+    for ccx in list_ccx:
+        if ccx.course_id.deprecated:
+            # prevent migration for deprecated course ids.
+            continue
+        ccx_locator = CCXLocator.from_course_locator(ccx.course_id, unicode(ccx.id))
+        reverse_add_master_course_staff_to_ccx(
+            get_course_by_id(ccx.course_id),
+            ccx_locator
+        )
+
+
+def add_master_course_staff_to_ccx(master_course, ccx_key, display_name, send_email=False):
+    """
+    Added staff role on ccx to all the staff members of master course.
+    Arguments:
+        master_course (CourseDescriptorWithMixins): Master course instance
+        ccx_key (CCXLocator): CCX course key
+        display_name (str): ccx display name for email
+    """
+    list_staff = list_with_level(master_course, 'staff')
+    list_instructor = list_with_level(master_course, 'instructor')
+
+    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:
+            # 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:
+            # 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, send_email=False):
+    """
+    Remove staff of ccx.
+
+    Arguments:
+        master_course (CourseDescriptorWithMixins): Master course instance
+        ccx_key (CCXLocator): CCX course key
+        display_name (str): ccx display name for email
+    """
+    list_staff = list_with_level(master_course, 'staff')
+    list_instructor = list_with_level(master_course, 'instructor')
+
+    with ccx_course(ccx_key) as course_ccx:
+        email_params = get_email_params(course_ccx, auto_enroll=True, course_key=ccx_key)
+        for staff in list_staff:
+            # allow 'staff' access on ccx to staff of master course
+            revoke_access(course_ccx, staff, 'staff')
+
+            # Enroll the staff in the ccx
+            unenroll_email(
+                course_id=ccx_key,
+                student_email=staff.email,
+                email_students=send_email,
+                email_params=email_params,
+            )
+
+        for instructor in list_instructor:
+            # allow 'instructor' access on ccx to instructor of master course
+            revoke_access(course_ccx, instructor, 'instructor')
+
+            # Enroll the instructor in the ccx
+            unenroll_email(
+                course_id=ccx_key,
+                student_email=instructor.email,
+                email_students=send_email,
+                email_params=email_params,
+            )
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('ccx', '0001_initial'),
+        ('ccx', '0002_customcourseforedx_structure_json'),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            code=add_master_course_staff_to_ccx_for_existing_ccx,
+            reverse_code=reverse_add_master_course_staff_to_ccx_for_existing_ccx
+        )
+    ]

lms/djangoapps/ccx/tests/test_utils.py

@@ -2,22 +2,41 @@
 test utils
 """
 import mock
+import uuid
 from nose.plugins.attrib import attr
 
-from student.roles import CourseCcxCoachRole
+from ccx_keys.locator import CCXLocator
+from student.roles import (
+    CourseCcxCoachRole,
+    CourseInstructorRole,
+    CourseStaffRole,
+)
 from student.tests.factories import (
     AdminFactory,
+    CourseEnrollmentFactory,
+    UserFactory
 )
 from xmodule.modulestore.tests.django_utils import (
     ModuleStoreTestCase,
-    TEST_DATA_SPLIT_MODULESTORE)
+    SharedModuleStoreTestCase,
+    TEST_DATA_SPLIT_MODULESTORE
+)
 from xmodule.modulestore.tests.factories import CourseFactory
 from opaque_keys.edx.keys import CourseKey
+from xmodule.modulestore.django import modulestore
 
 from lms.djangoapps.ccx import utils
+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,
+    reverse_add_master_course_staff_to_ccx
+)
 from lms.djangoapps.ccx.tests.factories import CcxFactory
 from lms.djangoapps.ccx.tests.utils import CcxTestCase
-from ccx_keys.locator import CCXLocator
 
 
 @attr('shard_1')
@@ -93,3 +112,135 @@ def test_get_chapters(self):
             sorted(course_chapters),
             sorted([unicode(child) for child in self.course.children])
         )
+
+
+class TestStaffOnCCX(CcxTestCase):
+    """
+    Tests for staff on ccx courses.
+    """
+    MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
+
+    def setUp(self):
+        super(TestStaffOnCCX, self).setUp()
+
+        # Create instructor account
+        self.client.login(username=self.coach.username, password="test")
+
+        # create an instance of modulestore
+        self.mstore = modulestore()
+
+        # adding staff to master course.
+        staff = AdminFactory()
+        allow_access(self.course, staff, 'staff')
+        self.assertTrue(CourseStaffRole(self.course.id).has_user(staff))
+
+        # adding instructor to master course.
+        instructor = UserFactory()
+        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
+        """
+        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(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_reverse_add_master_course_staff_to_ccx(self):
+        """
+        Test add staff of master course to ccx course
+        """
+        reverse_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(self.ccx_locator) as course_ccx:
+            list_staff_ccx_course = list_with_level(course_ccx, 'staff')
+            self.assertNotEqual(len(list_staff_master_course), len(list_staff_ccx_course))
+
+            list_instructor_ccx_course = list_with_level(course_ccx, 'instructor')
+            self.assertNotEqual(len(list_instructor_ccx_course), len(list_instructor_master_course))
+
+    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)