Merge pull request #10877 from mitocw/enhancement/aq/add_master_course_staff_in_ccx

Added staff and instructor roles on ccx to all the staff and instructors of the master course plus fixed view as student masquerade and display name of ccx on coach dashboard 

cms/djangoapps/contentstore/views/course.py

@@ -27,6 +27,7 @@ from .component import (
 )
 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 _accessible_courses_list(request):
         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/migrations/0002_add_master_course_staff_in_ccx.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from ccx_keys.locator import CCXLocator
+from django.db import migrations
+
+from lms.djangoapps.ccx.models import CustomCourseForEdX
+from lms.djangoapps.ccx.utils import (
+    add_master_course_staff_to_ccx,
+    reverse_add_master_course_staff_to_ccx
+)
+
+
+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).
+    """
+    list_ccx = CustomCourseForEdX.objects.all()
+    for ccx in list_ccx:
+        ccx_locator = CCXLocator.from_course_locator(ccx.course_id, unicode(ccx.id))
+        add_master_course_staff_to_ccx(ccx.course, 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).
+    """
+    list_ccx = CustomCourseForEdX.objects.all()
+    for ccx in list_ccx:
+        ccx_locator = CCXLocator.from_course_locator(ccx.course_id, unicode(ccx.id))
+        reverse_add_master_course_staff_to_ccx(ccx.course, ccx_locator, ccx.display_name)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('ccx', '0001_initial'),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            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

@@ -3,17 +3,30 @@ test utils
 """
 from nose.plugins.attrib import attr
 
-from lms.djangoapps.ccx.tests.factories import CcxFactory
-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,
+    SharedModuleStoreTestCase,
     TEST_DATA_SPLIT_MODULESTORE)
 from xmodule.modulestore.tests.factories import CourseFactory
+from xmodule.modulestore.django import modulestore
 
-from ccx_keys.locator import CCXLocator
+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.ccx.tests.factories import CcxFactory
+from lms.djangoapps.ccx.tests.utils import CcxTestCase
 
 
 @attr('shard_1')
@@ -47,3 +60,51 @@ class TestGetCCXFromCCXLocator(ModuleStoreTestCase):
         course_key = CCXLocator.from_course_locator(self.course.id, ccx.id)
         result = self.call_fut(course_key)
         self.assertEqual(result, ccx)
+
+
+class TestStaffOnCCX(CcxTestCase, SharedModuleStoreTestCase):
+    """
+    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 = UserFactory()
+        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))
+
+    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)
+
+        # 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:
+            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)

lms/djangoapps/ccx/tests/test_views.py

@@ -15,6 +15,8 @@ from courseware.courses import get_course_by_id
 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 django.conf import settings
 from django.core.urlresolvers import reverse, resolve
 from django.utils.timezone import UTC
@@ -23,7 +25,11 @@ from django.test import RequestFactory
 from edxmako.shortcuts import render_to_response
 from request_cache.middleware import RequestCache
 from opaque_keys.edx.keys import CourseKey
-from student.roles import CourseCcxCoachRole
+from student.roles import (
+    CourseCcxCoachRole,
+    CourseInstructorRole,
+    CourseStaffRole,
+)
 from student.models import (
     CourseEnrollment,
     CourseEnrollmentAllowed,
@@ -49,6 +55,7 @@ from ccx_keys.locator import CCXLocator
 
 from lms.djangoapps.ccx.models import CustomCourseForEdX
 from lms.djangoapps.ccx.overrides import get_override_for_ccx, override_field_for_ccx
+from lms.djangoapps.ccx.views import ccx_course
 from lms.djangoapps.ccx.tests.factories import CcxFactory
 from lms.djangoapps.ccx.tests.utils import (
     CcxTestCase,
@@ -136,6 +143,16 @@ class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase):
         # Login with the instructor account
         self.client.login(username=self.coach.username, password="test")
 
+        # adding staff to master course.
+        staff = UserFactory()
+        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))
+
     def assert_elements_in_schedule(self, url, n_chapters=2, n_sequentials=4, n_verticals=8):
         """
         Helper function to count visible elements in the schedule
@@ -222,6 +239,19 @@ class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase):
         role = CourseCcxCoachRole(course_key)
         self.assertTrue(role.has_user(self.coach))
 
+        # 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(course_key) 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_get_date(self):
         """
         Assert that get_date returns valid date.

lms/djangoapps/ccx/utils.py

@@ -18,17 +18,18 @@ from courseware.model_data import FieldDataCache
 from courseware.module_render import get_module_for_descriptor
 from instructor.enrollment import (
     enroll_email,
+    get_email_params,
     unenroll_email,
 )
-from instructor.access import allow_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
 from student.roles import CourseCcxCoachRole
 
-from lms.djangoapps.ccx.models import CustomCourseForEdX
 from lms.djangoapps.ccx.overrides import get_override_for_ccx
 from lms.djangoapps.ccx.custom_exception import CCXUserValidationException
+from lms.djangoapps.ccx.models import CustomCourseForEdX
 
 log = logging.getLogger("edx.ccx")
 
@@ -260,3 +261,83 @@ def is_email(identifier):
     except ValidationError:
         return False
     return True
+
+
+def add_master_course_staff_to_ccx(master_course, ccx_key, display_name):
+    """
+    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)
+        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,
+            )
+
+        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,
+            )
+
+
+def reverse_add_master_course_staff_to_ccx(master_course, ccx_key, display_name):
+    """
+    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, display_name=display_name)
+        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=True,
+                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=True,
+                email_params=email_params,
+            )

lms/djangoapps/ccx/views.py

@@ -52,6 +52,7 @@ from lms.djangoapps.ccx.overrides import (
     bulk_delete_ccx_override_fields,
 )
 from lms.djangoapps.ccx.utils import (
+    add_master_course_staff_to_ccx,
     assign_coach_role_to_ccx,
     ccx_course,
     ccx_students_enrolling_center,
@@ -146,6 +147,9 @@ def dashboard(request, course, ccx=None):
         context['grading_policy_url'] = reverse(
             'ccx_set_grading_policy', kwargs={'course_id': ccx_locator})
 
+        with ccx_course(ccx_locator) as course:
+            context['course'] = course
+
     else:
         context['create_ccx_url'] = reverse(
             'create_ccx', kwargs={'course_id': course.id})
@@ -208,7 +212,7 @@ def create_ccx(request, course, ccx=None):
     )
 
     assign_coach_role_to_ccx(ccx_id, request.user, course.id)
-
+    add_master_course_staff_to_ccx(course, ccx_id, ccx.display_name)
     return redirect(url)
 
 

lms/djangoapps/courseware/access.py

@@ -132,9 +132,6 @@ def has_access(user, action, obj, course_key=None):
     if not user:
         user = AnonymousUser()
 
-    if isinstance(course_key, CCXLocator):
-        course_key = course_key.to_course_locator()
-
     # delegate the work to type-specific functions.
     # (start with more specific types, then get more general)
     if isinstance(obj, CourseDescriptor):