Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions cms/djangoapps/contentstore/tests/test_course_listing.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import ddt

from django.conf import settings
from ccx_keys.locator import CCXLocator
from django.test import RequestFactory
from django.test.client import Client

Expand All @@ -26,6 +27,7 @@
from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls
from xmodule.modulestore import ModuleStoreEnum
from opaque_keys.edx.locations import CourseLocator
from opaque_keys.edx.keys import CourseKey
from xmodule.error_module import ErrorDescriptor
from course_action_state.models import CourseRerunState

Expand Down Expand Up @@ -131,6 +133,39 @@ def test_get_course_list(self):
# check both course lists have same courses
self.assertEqual(courses_list, courses_list_by_groups)

def test_get_course_list_when_ccx(self):
"""
Assert that courses with CCXLocator are filter in course listing.
"""
course_location = self.store.make_course_key('Org1', 'Course1', 'Run1')
self._create_course_with_access_groups(course_location, self.user)

# get courses through iterating all courses
courses_list, __ = _accessible_courses_list(self.request)
self.assertEqual(len(courses_list), 1)

# get courses by reversing group name formats
courses_list_by_groups, __ = _accessible_courses_list_from_groups(self.request)
self.assertEqual(len(courses_list_by_groups), 1)

# assert no course in listing with ccx id
ccx_course = Mock()
course_key = CourseKey.from_string('course-v1:FakeOrg+CN1+CR-FALLNEVER1')
ccx_course.id = CCXLocator.from_course_locator(course_key, u"1")

with patch(
'xmodule.modulestore.mixed.MixedModuleStore.get_course',
return_value=ccx_course
), patch(
'xmodule.modulestore.mixed.MixedModuleStore.get_courses',
Mock(return_value=[ccx_course])
):
courses_list, __ = _accessible_courses_list_from_groups(self.request)
self.assertEqual(len(courses_list), 0)

courses_list, __ = _accessible_courses_list(self.request)
self.assertEqual(len(courses_list), 0)

@ddt.data(
(ModuleStoreEnum.Type.split, 'xmodule.modulestore.split_mongo.split_mongo_kvs.SplitMongoKVS'),
(ModuleStoreEnum.Type.mongo, 'xmodule.modulestore.mongo.base.MongoKeyValueStore')
Expand Down
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 @@ -390,6 +391,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.id, CCXLocator):
return False

# pylint: disable=fixme
# TODO remove this condition when templates purged from db
if course.location.course == 'templates':
Expand Down Expand Up @@ -434,8 +440,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
53 changes: 49 additions & 4 deletions lms/djangoapps/ccx/api/v0/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import string
import urllib
import urlparse
from itertools import izip

import ddt
import mock
Expand All @@ -30,6 +31,8 @@
from courseware import courses
from ccx_keys.locator import CCXLocator
from student.models import CourseEnrollment
from student.tests.factories import UserFactory
from instructor.access import allow_access, list_with_level
from instructor.enrollment import (
enroll_email,
get_email_params,
Expand All @@ -39,6 +42,7 @@
from lms.djangoapps.ccx.overrides import override_field_for_ccx
from lms.djangoapps.ccx.tests.utils import CcxTestCase
from lms.djangoapps.ccx.utils import get_course_chapters
from lms.djangoapps.ccx.utils import ccx_course as ccx_course_cm
from opaque_keys.edx.keys import CourseKey
from student.roles import (
CourseInstructorRole,
Expand Down Expand Up @@ -66,9 +70,14 @@ def setUp(self):
self.master_course_key_str = unicode(self.master_course_key)
# OAUTH2 setup
# create a specific user for the application
app_user = User.objects.create_user('test_app_user', 'test_app_user@openedx.org', 'test')
app_user = UserFactory(username='test_app_user', email='test_app_user@openedx.org', password='test')
# add staff role to the app user
CourseStaffRole(self.master_course_key).add_users(app_user)

# adding instructor to master course.
instructor = UserFactory()
allow_access(self.course, instructor, 'instructor')

# create an oauth client app entry
self.app_client = Client.objects.create(
user=app_user,
Expand Down Expand Up @@ -172,7 +181,7 @@ def test_authorization_no_oauth_staff(self):
Check authorization for staff users logged in without oauth
"""
# create a staff user
staff_user = User.objects.create_user('test_staff_user', 'test_staff_user@openedx.org', 'test')
staff_user = UserFactory(username='test_staff_user', email='test_staff_user@openedx.org', password='test')
# add staff role to the staff user
CourseStaffRole(self.master_course_key).add_users(staff_user)

Expand All @@ -194,7 +203,9 @@ def test_authorization_no_oauth_instructor(self):
Check authorization for instructor users logged in without oauth
"""
# create an instructor user
instructor_user = User.objects.create_user('test_instructor_user', 'test_instructor_user@openedx.org', 'test')
instructor_user = UserFactory(
username='test_instructor_user', email='test_instructor_user@openedx.org', password='test'
)
# add instructor role to the instructor user
CourseInstructorRole(self.master_course_key).add_users(instructor_user)

Expand All @@ -217,7 +228,9 @@ def test_authorization_no_oauth(self):
Check authorization for coach users logged in without oauth
"""
# create an coach user
coach_user = User.objects.create_user('test_coach_user', 'test_coach_user@openedx.org', 'test')
coach_user = UserFactory(
username='test_coach_user', email='test_coach_user@openedx.org', password='test'
)
# add coach role to the coach user
CourseCcxCoachRole(self.master_course_key).add_users(coach_user)

Expand Down Expand Up @@ -607,6 +620,38 @@ def test_post_list_duplicated_modules(self):
self.assertEqual(resp.status_code, status.HTTP_201_CREATED)
self.assertEqual(resp.data.get('course_modules'), chapters) # 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
Expand Down
8 changes: 8 additions & 0 deletions lms/djangoapps/ccx/api/v0/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from ccx_keys.locator import CCXLocator
from courseware.courses import get_course_by_id
from django.db import migrations

from lms.djangoapps.ccx.utils import (
add_master_course_staff_to_ccx,
remove_master_course_staff_from_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).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


Arguments:
apps (Applications): Apps in edX platform.
schema_editor (SchemaEditor): For editing database schema i.e create, delete field (column)

"""
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,
send_email=False
)


def remove_master_course_staff_from_ccx_for_existing_ccx(apps, schema_editor):
"""
Remove all staff and instructors of master course from respective CCX(s).

Arguments:
apps (Applications): Apps in edX platform.
schema_editor (SchemaEditor): For editing database schema i.e create, delete field (column)

"""
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))
remove_master_course_staff_from_ccx(
get_course_by_id(ccx.course_id),
ccx_locator,
ccx.display_name,
send_email=False
)


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=remove_master_course_staff_from_ccx_for_existing_ccx
)
]
Loading