Skip to content

Commit

Permalink
Added staff role on ccx to staff of master course and added a data mi…
Browse files Browse the repository at this point in the history
…gration for existing ccx courses.
  • Loading branch information
amir-qayyum-khan committed Feb 17, 2016
1 parent 6cc8bbf commit 91bf48f
Show file tree
Hide file tree
Showing 7 changed files with 239 additions and 12 deletions.
13 changes: 11 additions & 2 deletions cms/djangoapps/contentstore/views/course.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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':
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# -*- 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
)
]
67 changes: 64 additions & 3 deletions lms/djangoapps/ccx/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,30 @@
"""
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')
Expand Down Expand Up @@ -47,3 +60,51 @@ def test_ccx_locator(self):
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)
32 changes: 31 additions & 1 deletion lms/djangoapps/ccx/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
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
Expand All @@ -23,7 +25,11 @@
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,
Expand All @@ -49,6 +55,7 @@

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,
Expand Down Expand Up @@ -136,6 +143,16 @@ def setUp(self):
# 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
Expand Down Expand Up @@ -222,6 +239,19 @@ def test_create_ccx(self):
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.
Expand Down
85 changes: 83 additions & 2 deletions lms/djangoapps/ccx/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,18 @@
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")

Expand Down Expand Up @@ -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,
)
6 changes: 5 additions & 1 deletion lms/djangoapps/ccx/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
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,
Expand Down Expand Up @@ -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})
Expand Down Expand Up @@ -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)


Expand Down
3 changes: 0 additions & 3 deletions lms/djangoapps/courseware/access.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down

0 comments on commit 91bf48f

Please sign in to comment.