diff --git a/lms/djangoapps/ccx/api/v0/tests/test_views.py b/lms/djangoapps/ccx/api/v0/tests/test_views.py
index a30643365655e647bbeb6eb9070f6dd1986ef209..3a5e683209435a3e37f2c509be60530aa3147a9f 100644
--- a/lms/djangoapps/ccx/api/v0/tests/test_views.py
+++ b/lms/djangoapps/ccx/api/v0/tests/test_views.py
@@ -647,9 +647,14 @@ class CcxListTest(CcxRestApiTest):
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)
+ # The "Coach" in the parent course becomes "Staff" on the CCX, so the CCX should have 1 "Staff"
+ # user more than the parent course
+ self.assertEqual(len(list_staff_master_course) + 1, len(list_staff_ccx_course))
+ # Make sure all of the existing course staff are passed to the CCX
+ for course_user in list_staff_master_course:
+ self.assertIn(course_user, list_staff_ccx_course)
+ # Make sure the "Coach" on the parent course is "Staff" on the CCX
+ self.assertIn(self.coach, list_staff_ccx_course)
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)
diff --git a/lms/djangoapps/ccx/api/v0/views.py b/lms/djangoapps/ccx/api/v0/views.py
index 85255191542a0a0358b7be2c70eb9fbe2c97b08b..74523bce71eba59bc83640e44b439f7ed0f302c5 100644
--- a/lms/djangoapps/ccx/api/v0/views.py
+++ b/lms/djangoapps/ccx/api/v0/views.py
@@ -766,8 +766,8 @@ class CCXDetailView(GenericAPIView):
email_students=True,
email_params=email_params,
)
- # enroll the coach to the newly created ccx
- assign_coach_role_to_ccx(ccx_course_key, coach, master_course_object.id)
+ # make the new coach staff on the CCX
+ assign_staff_role_to_ccx(ccx_course_key, coach, master_course_object.id)
# using CCX object as sender here.
responses = SignalHandler.course_published.send(
diff --git a/lms/djangoapps/ccx/migrations/0004_change_ccx_coach_to_staff.py b/lms/djangoapps/ccx/migrations/0004_change_ccx_coach_to_staff.py
deleted file mode 100644
index 43754c88ba8470b97c8352ad2731b139c16510b7..0000000000000000000000000000000000000000
--- a/lms/djangoapps/ccx/migrations/0004_change_ccx_coach_to_staff.py
+++ /dev/null
@@ -1,67 +0,0 @@
-# -*- coding: utf-8 -*-
-from __future__ import unicode_literals
-
-from django.db import migrations, models
-
-# We're doing something awful here, but it's necessary for the greater good:
-from django.contrib.auth.models import User
-
-from instructor.access import allow_access, revoke_access
-
-from ccx_keys.locator import CCXLocator
-from lms.djangoapps.ccx.utils import ccx_course
-
-def change_existing_ccx_coaches_to_staff(apps, schema_editor):
- """
- Modify all coaches of CCX courses so that they have the staff role on the
- CCX course they coach, but retain the CCX Coach role on the parent course.
-
- Arguments:
- apps (Applications): Apps in edX platform.
- schema_editor (SchemaEditor): For editing database schema (unused)
-
- """
- CustomCourseForEdX = apps.get_model('ccx', 'CustomCourseForEdX')
- db_alias = schema_editor.connection.alias
- list_ccx = CustomCourseForEdX.objects.using(db_alias).all()
- for ccx in list_ccx:
- ccx_locator = CCXLocator.from_course_locator(ccx.course_id, unicode(ccx.id))
- with ccx_course(ccx_locator) as course:
- coach = User.objects.get(id=ccx.coach.id)
- allow_access(course, coach, 'staff', send_email=False)
- revoke_access(course, coach, 'ccx_coach', send_email=False)
-
-def revert_ccx_staff_to_coaches(apps, schema_editor):
- """
- Modify all staff on CCX courses so that they no longer have the staff role
- on the course that they coach.
-
- Arguments:
- apps (Applications): Apps in edX platform.
- schema_editor (SchemaEditor): For editing database schema (unused)
-
- """
- CustomCourseForEdX = apps.get_model('ccx', 'CustomCourseForEdX')
- db_alias = schema_editor.connection.alias
- list_ccx = CustomCourseForEdX.objects.using(db_alias).all()
- for ccx in list_ccx:
- ccx_locator = CCXLocator.from_course_locator(ccx.course_id, unicode(ccx.id))
- with ccx_course(ccx_locator) as course:
- coach = User.objects.get(id=ccx.coach.id)
- allow_access(course, coach, 'ccx_coach', send_email=False)
- revoke_access(course, coach, 'staff', send_email=False)
-
-class Migration(migrations.Migration):
-
- dependencies = [
- ('ccx', '0001_initial'),
- ('ccx', '0002_customcourseforedx_structure_json'),
- ('ccx', '0003_add_master_course_staff_in_ccx'),
- ]
-
- operations = [
- migrations.RunPython(
- code=change_existing_ccx_coaches_to_staff,
- reverse_code=revert_ccx_staff_to_coaches
- )
- ]
diff --git a/lms/djangoapps/ccx/migrations/0005_change_ccx_coach_to_staff.py b/lms/djangoapps/ccx/migrations/0005_change_ccx_coach_to_staff.py
new file mode 100644
index 0000000000000000000000000000000000000000..711d493f945ce4b999be9fb9d04153817fa19fc6
--- /dev/null
+++ b/lms/djangoapps/ccx/migrations/0005_change_ccx_coach_to_staff.py
@@ -0,0 +1,83 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+import logging
+
+from django.contrib.auth.models import User
+from django.db import migrations
+
+from ccx_keys.locator import CCXLocator
+from instructor.access import allow_access, revoke_access
+from lms.djangoapps.ccx.utils import ccx_course
+
+
+log = logging.getLogger("edx.ccx")
+
+def change_existing_ccx_coaches_to_staff(apps, schema_editor):
+ """
+ Modify all coaches of CCX courses so that they have the staff role on the
+ CCX course they coach, but retain the CCX Coach role on the parent course.
+
+ Arguments:
+ apps (Applications): Apps in edX platform.
+ schema_editor (SchemaEditor): For editing database schema (unused)
+
+ """
+ CustomCourseForEdX = apps.get_model('ccx', 'CustomCourseForEdX')
+ db_alias = schema_editor.connection.alias
+ if not db_alias == 'default':
+ # This migration is not intended to run against the student_module_history database and
+ # will fail if it does. Ensure that it'll only run against the default database.
+ return
+ list_ccx = CustomCourseForEdX.objects.using(db_alias).all()
+ for ccx in list_ccx:
+ ccx_locator = CCXLocator.from_course_locator(ccx.course_id, unicode(ccx.id))
+ with ccx_course(ccx_locator) as course:
+ coach = User.objects.get(id=ccx.coach.id)
+ allow_access(course, coach, 'staff', send_email=False)
+ revoke_access(course, coach, 'ccx_coach', send_email=False)
+ log.info(
+ 'The CCX coach of CCX %s has been switched from "CCX Coach" to "Staff".',
+ unicode(ccx_locator)
+ )
+
+def revert_ccx_staff_to_coaches(apps, schema_editor):
+ """
+ Modify all staff on CCX courses so that they no longer have the staff role
+ on the course that they coach.
+
+ Arguments:
+ apps (Applications): Apps in edX platform.
+ schema_editor (SchemaEditor): For editing database schema (unused)
+
+ """
+ CustomCourseForEdX = apps.get_model('ccx', 'CustomCourseForEdX')
+ db_alias = schema_editor.connection.alias
+ if not db_alias == 'default':
+ return
+ list_ccx = CustomCourseForEdX.objects.using(db_alias).all()
+ for ccx in list_ccx:
+ ccx_locator = CCXLocator.from_course_locator(ccx.course_id, unicode(ccx.id))
+ with ccx_course(ccx_locator) as course:
+ coach = User.objects.get(id=ccx.coach.id)
+ allow_access(course, coach, 'ccx_coach', send_email=False)
+ revoke_access(course, coach, 'staff', send_email=False)
+ log.info(
+ 'The CCX coach of CCX %s has been switched from "Staff" to "CCX Coach".',
+ unicode(ccx_locator)
+ )
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('ccx', '0001_initial'),
+ ('ccx', '0002_customcourseforedx_structure_json'),
+ ('ccx', '0003_add_master_course_staff_in_ccx'),
+ ('ccx', '0004_seed_forum_roles_in_ccx_courses'),
+ ]
+
+ operations = [
+ migrations.RunPython(
+ code=change_existing_ccx_coaches_to_staff,
+ reverse_code=revert_ccx_staff_to_coaches
+ )
+ ]
diff --git a/lms/djangoapps/ccx/models.py b/lms/djangoapps/ccx/models.py
index 5c6e8dd103264893a7dd72c7d6eeec0815c13805..00c660d7925cbd8e5d2894670c62017af90ffd6e 100644
--- a/lms/djangoapps/ccx/models.py
+++ b/lms/djangoapps/ccx/models.py
@@ -1,6 +1,7 @@
"""
Models for the custom course feature
"""
+from __future__ import unicode_literals
import json
import logging
from datetime import datetime
@@ -8,8 +9,9 @@ from datetime import datetime
from django.contrib.auth.models import User
from django.db import models
from pytz import utc
-
from lazy import lazy
+
+from ccx_keys.locator import CCXLocator
from openedx.core.lib.time_zone_utils import get_time_zone_abbr
from xmodule_django.models import CourseKeyField, LocationKeyField
from xmodule.error_module import ErrorDescriptor
@@ -121,6 +123,16 @@ class CustomCourseForEdX(models.Model):
return json.loads(self.structure_json)
return None
+ @property
+ def locator(self):
+ """
+ Helper property that gets a corresponding CCXLocator for this CCX.
+
+ Returns:
+ The CCXLocator corresponding to this CCX.
+ """
+ return CCXLocator.from_course_locator(self.course_id, unicode(self.id))
+
class CcxFieldOverride(models.Model):
"""
diff --git a/lms/djangoapps/ccx/tests/test_models.py b/lms/djangoapps/ccx/tests/test_models.py
index c756b61de00b0424dcf2a011e92460b5dc81ebfe..f4091e4210b201f95fbb10196542abe77494c41e 100644
--- a/lms/djangoapps/ccx/tests/test_models.py
+++ b/lms/djangoapps/ccx/tests/test_models.py
@@ -12,7 +12,10 @@ from student.tests.factories import (
AdminFactory,
)
from util.tests.test_date_utils import fake_ugettext
-from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore.tests.django_utils import (
+ ModuleStoreTestCase,
+ TEST_DATA_SPLIT_MODULESTORE
+)
from xmodule.modulestore.tests.factories import (
CourseFactory,
check_mongo_calls
@@ -30,6 +33,8 @@ class TestCCX(ModuleStoreTestCase):
"""Unit tests for the CustomCourseForEdX model
"""
+ MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
+
def setUp(self):
"""common setup for all tests"""
super(TestCCX, self).setUp()
@@ -51,7 +56,7 @@ class TestCCX(ModuleStoreTestCase):
def test_ccx_course_caching(self):
"""verify that caching the propery works to limit queries"""
- with check_mongo_calls(1):
+ with check_mongo_calls(3):
# these statements are used entirely to demonstrate the
# instance-level caching of these values on CCX objects. The
# check_mongo_calls context is the point here.
@@ -77,7 +82,7 @@ class TestCCX(ModuleStoreTestCase):
"""verify that caching the start property works to limit queries"""
now = datetime.now(utc)
self.set_ccx_override('start', now)
- with check_mongo_calls(1):
+ with check_mongo_calls(3):
# these statements are used entirely to demonstrate the
# instance-level caching of these values on CCX objects. The
# check_mongo_calls context is the point here.
@@ -102,7 +107,7 @@ class TestCCX(ModuleStoreTestCase):
"""verify that caching the due property works to limit queries"""
expected = datetime.now(utc)
self.set_ccx_override('due', expected)
- with check_mongo_calls(1):
+ with check_mongo_calls(3):
# these statements are used entirely to demonstrate the
# instance-level caching of these values on CCX objects. The
# check_mongo_calls context is the point here.
@@ -269,3 +274,10 @@ class TestCCX(ModuleStoreTestCase):
)
self.assertEqual(ccx.structure_json, json_struct) # pylint: disable=no-member
self.assertEqual(ccx.structure, dummy_struct) # pylint: disable=no-member
+
+ def test_locator_property(self):
+ """
+ Verify that the locator helper property returns a correct CCXLocator
+ """
+ locator = self.ccx.locator # pylint: disable=no-member
+ self.assertEqual(self.ccx.id, long(locator.ccx))
diff --git a/lms/djangoapps/ccx/tests/test_views.py b/lms/djangoapps/ccx/tests/test_views.py
index 16fe73aee7d442e4bb9401a4d87632599edc522e..6b8e7bc2e7215e7f13126ee72c7f313d1d7160d4 100644
--- a/lms/djangoapps/ccx/tests/test_views.py
+++ b/lms/djangoapps/ccx/tests/test_views.py
@@ -418,8 +418,8 @@ class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase):
course_enrollments = get_override_for_ccx(ccx, self.course, 'max_student_enrollments_allowed')
self.assertEqual(course_enrollments, settings.CCX_MAX_STUDENTS_ALLOWED)
- # assert ccx creator has role=ccx_coach
- role = CourseCcxCoachRole(course_key)
+ # assert ccx creator has role=staff
+ role = CourseStaffRole(course_key)
self.assertTrue(role.has_user(self.coach))
# assert that staff and instructors of master course has staff and instructor roles on ccx
@@ -432,8 +432,12 @@ class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase):
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)
+ # The "Coach" in the parent course becomes "Staff" on the CCX, so the CCX should have 1 "Staff"
+ # user more than the parent course
+ self.assertEqual(len(list_staff_master_course) + 1, len(list_staff_ccx_course))
+ self.assertIn(list_staff_master_course[0].email, [ccx_staff.email for ccx_staff in list_staff_ccx_course])
+ # Make sure the "Coach" on the parent course is "Staff" on the CCX
+ self.assertIn(self.coach, list_staff_ccx_course)
list_instructor_ccx_course = list_with_level(course_ccx, 'instructor')
self.assertEqual(len(list_instructor_ccx_course), len(list_instructor_master_course))
diff --git a/lms/djangoapps/certificates/queue.py b/lms/djangoapps/certificates/queue.py
index 638b34d4ecc7d220ce9f0767221821c39b25e9eb..2018a88574b79c3538d3757e00012d2eb9ea323c 100644
--- a/lms/djangoapps/certificates/queue.py
+++ b/lms/djangoapps/certificates/queue.py
@@ -199,6 +199,8 @@ class XQueueCertInterface(object):
Will change the certificate status to 'generating' or
`downloadable` in case of web view certificates.
+ The course must not be a CCX.
+
Certificate must be in the 'unavailable', 'error',
'deleted' or 'generating' state.
@@ -214,6 +216,18 @@ class XQueueCertInterface(object):
Returns the newly created certificate instance
"""
+ if hasattr(course_id, 'ccx'):
+ LOGGER.warning(
+ (
+ u"Cannot create certificate generation task for user %s "
+ u"in the course '%s'; "
+ u"certificates are not allowed for CCX courses."
+ ),
+ student.id,
+ unicode(course_id)
+ )
+ return None
+
valid_statuses = [
status.generating,
status.unavailable,
diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py
index 0cce7263555cbeed361019f036e92cc413a9cffd..261e19456cf70b1d464b5c5698d4591bd6e04c84 100644
--- a/lms/djangoapps/instructor/views/instructor_dashboard.py
+++ b/lms/djangoapps/instructor/views/instructor_dashboard.py
@@ -77,6 +77,20 @@ class InstructorDashboardTab(CourseTab):
return bool(user and has_access(user, 'staff', course, course.id))
+def show_analytics_dashboard_message(course_key):
+ """
+ Defines whether or not the analytics dashboard URL should be displayed.
+
+ Arguments:
+ course_key (CourseLocator): The course locator to display the analytics dashboard message on.
+ """
+ if hasattr(course_key, 'ccx'):
+ ccx_analytics_enabled = settings.FEATURES.get('ENABLE_CCX_ANALYTICS_DASHBOARD_URL', False)
+ return settings.ANALYTICS_DASHBOARD_URL and ccx_analytics_enabled
+
+ return settings.ANALYTICS_DASHBOARD_URL
+
+
@ensure_csrf_cookie
@cache_control(no_cache=True, no_store=True, must_revalidate=True)
def instructor_dashboard_2(request, course_id):
@@ -112,7 +126,7 @@ def instructor_dashboard_2(request, course_id):
]
analytics_dashboard_message = None
- if settings.ANALYTICS_DASHBOARD_URL:
+ if show_analytics_dashboard_message(course_key):
# Construct a URL to the external analytics dashboard
analytics_dashboard_url = '{0}/courses/{1}'.format(settings.ANALYTICS_DASHBOARD_URL, unicode(course_key))
link_start = HTML("<a href=\"{}\" target=\"_blank\">").format(analytics_dashboard_url)
@@ -169,7 +183,8 @@ def instructor_dashboard_2(request, course_id):
# Certificates panel
# This is used to generate example certificates
# and enable self-generated certificates for a course.
- certs_enabled = CertificateGenerationConfiguration.current().enabled
+ # Note: This is hidden for all CCXs
+ certs_enabled = CertificateGenerationConfiguration.current().enabled and not hasattr(course_key, 'ccx')
if certs_enabled and access['admin']:
sections.append(_section_certificates(course))
@@ -421,7 +436,7 @@ def _section_course_info(course, access):
if settings.FEATURES.get('DISPLAY_ANALYTICS_ENROLLMENTS'):
section_data['enrollment_count'] = CourseEnrollment.objects.enrollment_counts(course_key)
- if settings.ANALYTICS_DASHBOARD_URL:
+ if show_analytics_dashboard_message(course_key):
# dashboard_link is already made safe in _get_dashboard_link
dashboard_link = _get_dashboard_link(course_key)
# so we can use Text() here so it's not double-escaped and rendering HTML on the front-end
diff --git a/lms/envs/common.py b/lms/envs/common.py
index 47adc782f98420a5e2a7845135f101464dbd5f89..23dde90842636d087ced2b8ed7fb94a86e2867ac 100644
--- a/lms/envs/common.py
+++ b/lms/envs/common.py
@@ -356,6 +356,11 @@ FEATURES = {
# lives in the Extended table, saving the frontend from
# making multiple queries.
'ENABLE_READING_FROM_MULTIPLE_HISTORY_TABLES': True,
+
+ # Display the 'Analytics' tab in the instructor dashboard for CCX courses.
+ # Note: This has no effect unless ANALYTICS_DASHBOARD_URL is already set,
+ # because without that setting, the tab does not show up for any courses.
+ 'ENABLE_CCX_ANALYTICS_DASHBOARD_URL': False,
}
# Ignore static asset files on import which match this pattern