From 8533fd97348c24ffc2f598477c21b88a2b8cd5fb Mon Sep 17 00:00:00 2001 From: Jesse Shapiro Date: Fri, 29 Jul 2016 19:18:11 -0400 Subject: [PATCH 1/3] Adding change to staff role for new users and draft migration --- common/djangoapps/student/roles.py | 4 +- lms/djangoapps/ccx/api/v0/views.py | 6 +- .../0004_change_ccx_coach_to_staff.py | 67 +++++++++++++++++++ lms/djangoapps/ccx/utils.py | 12 ++-- lms/djangoapps/ccx/views.py | 7 +- 5 files changed, 82 insertions(+), 14 deletions(-) create mode 100644 lms/djangoapps/ccx/migrations/0004_change_ccx_coach_to_staff.py diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index c8b8796420..15d60afa84 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -153,7 +153,7 @@ class RoleBase(AccessRole): # legit get updated. from student.models import CourseAccessRole for user in users: - if user.is_authenticated and user.is_active and not self.has_user(user): + if user.is_authenticated() and user.is_active and not self.has_user(user): entry = CourseAccessRole(user=user, role=self._role_name, course_id=self.course_key, org=self.org) entry.save() if hasattr(user, '_roles'): @@ -349,7 +349,7 @@ class UserBasedRole(object): """ Grant this object's user the object's role for the supplied courses """ - if self.user.is_authenticated and self.user.is_active: + if self.user.is_authenticated() and self.user.is_active: for course_key in course_keys: entry = CourseAccessRole(user=self.user, role=self.role, course_id=course_key, org=course_key.org) entry.save() diff --git a/lms/djangoapps/ccx/api/v0/views.py b/lms/djangoapps/ccx/api/v0/views.py index 7c9106d0dd..8525519154 100644 --- a/lms/djangoapps/ccx/api/v0/views.py +++ b/lms/djangoapps/ccx/api/v0/views.py @@ -37,7 +37,7 @@ from lms.djangoapps.ccx.overrides import ( ) from lms.djangoapps.ccx.utils import ( add_master_course_staff_to_ccx, - assign_coach_role_to_ccx, + assign_staff_role_to_ccx, is_email, get_course_chapters, ) @@ -507,8 +507,8 @@ class CCXListView(GenericAPIView): email_students=True, email_params=email_params, ) - # 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 the coach to the newly created ccx + assign_staff_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, 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 new file mode 100644 index 0000000000..43754c88ba --- /dev/null +++ b/lms/djangoapps/ccx/migrations/0004_change_ccx_coach_to_staff.py @@ -0,0 +1,67 @@ +# -*- 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/utils.py b/lms/djangoapps/ccx/utils.py index 66170ddd6c..abb3b6a318 100644 --- a/lms/djangoapps/ccx/utils.py +++ b/lms/djangoapps/ccx/utils.py @@ -292,9 +292,9 @@ def ccx_course(ccx_locator): yield course -def assign_coach_role_to_ccx(ccx_locator, user, master_course_id): +def assign_staff_role_to_ccx(ccx_locator, user, master_course_id): """ - Check if user has ccx_coach role on master course then assign him coach role on ccx only + Check if user has ccx_coach role on master course then assign him staff role on ccx only if role is not already assigned. Because of this coach can open dashboard from master course as well as ccx. :param ccx_locator: CCX key @@ -304,12 +304,12 @@ def assign_coach_role_to_ccx(ccx_locator, user, master_course_id): coach_role_on_master_course = CourseCcxCoachRole(master_course_id) # check if user has coach role on master course if coach_role_on_master_course.has_user(user): - # Check if user has coach role on ccx. - role = CourseCcxCoachRole(ccx_locator) + # Check if user has staff role on ccx. + role = CourseStaffRole(ccx_locator) if not role.has_user(user): - # assign user role coach on ccx + # assign user the staff role on ccx with ccx_course(ccx_locator) as course: - allow_access(course, user, "ccx_coach", send_email=False) + allow_access(course, user, "staff", send_email=False) def is_email(identifier): diff --git a/lms/djangoapps/ccx/views.py b/lms/djangoapps/ccx/views.py index 76f30fdaf5..9e1136c0f7 100644 --- a/lms/djangoapps/ccx/views.py +++ b/lms/djangoapps/ccx/views.py @@ -56,7 +56,7 @@ from lms.djangoapps.ccx.overrides import ( ) from lms.djangoapps.ccx.utils import ( add_master_course_staff_to_ccx, - assign_coach_role_to_ccx, + assign_staff_role_to_ccx, ccx_course, ccx_students_enrolling_center, get_ccx_for_coach, @@ -147,7 +147,8 @@ def dashboard(request, course, ccx=None): if ccx: ccx_locator = CCXLocator.from_course_locator(course.id, unicode(ccx.id)) - + # At this point we are done with verification that current user is ccx coach. + assign_staff_role_to_ccx(ccx_locator, request.user, course.id) schedule = get_ccx_schedule(course, ccx) grading_policy = get_override_for_ccx( ccx, course, 'grading_policy', course.grading_policy) @@ -239,7 +240,7 @@ def create_ccx(request, course, ccx=None): email_params=email_params, ) - assign_coach_role_to_ccx(ccx_id, request.user, course.id) + assign_staff_role_to_ccx(ccx_id, request.user, course.id) add_master_course_staff_to_ccx(course, ccx_id, ccx.display_name) # using CCX object as sender here. From 8c3224b4f070f6614fbebd07c1e2802e1365b42e Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Thu, 1 Sep 2016 09:51:28 -0400 Subject: [PATCH 2/3] Finish implementing CCX coach as staff --- lms/djangoapps/ccx/api/v0/tests/test_views.py | 11 ++- lms/djangoapps/ccx/api/v0/views.py | 4 +- .../0004_change_ccx_coach_to_staff.py | 67 --------------- .../0005_change_ccx_coach_to_staff.py | 83 +++++++++++++++++++ lms/djangoapps/ccx/models.py | 14 +++- lms/djangoapps/ccx/tests/test_models.py | 20 ++++- lms/djangoapps/ccx/tests/test_views.py | 12 ++- lms/djangoapps/certificates/queue.py | 14 ++++ .../instructor/views/instructor_dashboard.py | 21 ++++- lms/envs/common.py | 5 ++ 10 files changed, 167 insertions(+), 84 deletions(-) delete mode 100644 lms/djangoapps/ccx/migrations/0004_change_ccx_coach_to_staff.py create mode 100644 lms/djangoapps/ccx/migrations/0005_change_ccx_coach_to_staff.py diff --git a/lms/djangoapps/ccx/api/v0/tests/test_views.py b/lms/djangoapps/ccx/api/v0/tests/test_views.py index a306433656..3a5e683209 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 8525519154..74523bce71 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 43754c88ba..0000000000 --- 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 0000000000..711d493f94 --- /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 5c6e8dd103..00c660d792 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 c756b61de0..f4091e4210 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 16fe73aee7..6b8e7bc2e7 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 638b34d4ec..2018a88574 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 0cce726355..261e19456c 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("").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 47adc782f9..23dde90842 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 From 9563b4d2fe41517cc1c0f2e382663446292c2192 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Fri, 7 Oct 2016 11:49:15 -0400 Subject: [PATCH 3/3] Change content groups message when in a CCX --- lms/djangoapps/instructor/views/instructor_dashboard.py | 2 ++ lms/static/js/groups/views/cohort_form.js | 3 ++- lms/static/js/groups/views/cohorts_dashboard_factory.js | 3 ++- .../instructor_dashboard_2/cohort-form.underscore | 7 +++++-- .../instructor_dashboard_2/cohort_management.html | 1 + 5 files changed, 12 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index 261e19456c..e20f1e9934 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -486,10 +486,12 @@ def _section_membership(course, access, is_white_label): def _section_cohort_management(course, access): """ Provide data for the corresponding cohort management section """ course_key = course.id + ccx_enabled = hasattr(course_key, 'ccx') section_data = { 'section_key': 'cohort_management', 'section_display_name': _('Cohorts'), 'access': access, + 'ccx_is_enabled': ccx_enabled, 'course_cohort_settings_url': reverse( 'course_cohort_settings', kwargs={'course_key_string': unicode(course_key)} diff --git a/lms/static/js/groups/views/cohort_form.js b/lms/static/js/groups/views/cohort_form.js index 1d7c987232..307ee2ac85 100644 --- a/lms/static/js/groups/views/cohort_form.js +++ b/lms/static/js/groups/views/cohort_form.js @@ -35,7 +35,8 @@ cohort: this.model, isDefaultCohort: this.isDefault(this.model.get('name')), contentGroups: this.contentGroups, - studioGroupConfigurationsUrl: this.context.studioGroupConfigurationsUrl + studioGroupConfigurationsUrl: this.context.studioGroupConfigurationsUrl, + isCcxEnabled: this.context.isCcxEnabled })); return this; }, diff --git a/lms/static/js/groups/views/cohorts_dashboard_factory.js b/lms/static/js/groups/views/cohorts_dashboard_factory.js index af63d0c676..e85ade7bdf 100644 --- a/lms/static/js/groups/views/cohorts_dashboard_factory.js +++ b/lms/static/js/groups/views/cohorts_dashboard_factory.js @@ -31,7 +31,8 @@ discussionTopicsSettingsModel: discussionTopicsSettings, uploadCohortsCsvUrl: cohortManagementElement.data('upload_cohorts_csv_url'), verifiedTrackCohortingUrl: cohortManagementElement.data('verified_track_cohorting_url'), - studioGroupConfigurationsUrl: studioGroupConfigurationsUrl + studioGroupConfigurationsUrl: studioGroupConfigurationsUrl, + isCcxEnabled: cohortManagementElement.data('is_ccx_enabled') } }); diff --git a/lms/templates/instructor/instructor_dashboard_2/cohort-form.underscore b/lms/templates/instructor/instructor_dashboard_2/cohort-form.underscore index 9f41c248ad..9321e29f15 100644 --- a/lms/templates/instructor/instructor_dashboard_2/cohort-form.underscore +++ b/lms/templates/instructor/instructor_dashboard_2/cohort-form.underscore @@ -17,7 +17,6 @@ var cohort_name = cohort.get('name'); var cohort_name_value = isNewCohort ? '' : cohort_name; var placeholder_value = isNewCohort ? gettext('Enter the name of the cohort') : ''; - %>
@@ -126,7 +125,11 @@ } ) %> - <%- gettext("Create a content group") %> + <% if (isCcxEnabled) { %> + <%- gettext("Only the parent course staff of a CCX can create content groups.") %> + <% } else { %> + <%- gettext("Create a content group") %> + <% } %>

diff --git a/lms/templates/instructor/instructor_dashboard_2/cohort_management.html b/lms/templates/instructor/instructor_dashboard_2/cohort_management.html index adf74640de..8e7bad3439 100644 --- a/lms/templates/instructor/instructor_dashboard_2/cohort_management.html +++ b/lms/templates/instructor/instructor_dashboard_2/cohort_management.html @@ -15,6 +15,7 @@ from openedx.core.djangoapps.course_groups.partition_scheme import get_cohorted_ data-course_cohort_settings_url="${section_data['course_cohort_settings_url']}" data-discussion-topics-url="${section_data['discussion_topics_url']}" data-verified_track_cohorting_url="${section_data['verified_track_cohorting_url']}" + data-is_ccx_enabled="${'true' if section_data['ccx_is_enabled'] else 'false'}" >