Merge pull request #8395 from jazkarta/ccx-custom-ids

MIT CCX: Use CCX Keys

common/djangoapps/student/models.py

@@ -1195,6 +1195,13 @@ class CourseEnrollment(models.Model):
         """
         if not user.is_authenticated():
             return False
+
+        # unwrap CCXLocators so that we use the course as the access control
+        # source
+        from ccx_keys.locator import CCXLocator
+        if isinstance(course_key, CCXLocator):
+            course_key = course_key.to_course_locator()
+
         try:
             record = CourseEnrollment.objects.get(user=user, course_id=course_key)
             return record.is_active

common/djangoapps/student/views.py

@@ -618,14 +618,10 @@ def dashboard(request):
 
     ccx_membership_triplets = []
     if settings.FEATURES.get('CUSTOM_COURSES_EDX', False):
-        from ccx import ACTIVE_CCX_KEY
         from ccx.utils import get_ccx_membership_triplets
         ccx_membership_triplets = get_ccx_membership_triplets(
             user, course_org_filter, org_filter_out_set
         )
-        # should we deselect any active CCX at this time so that we don't have
-        # to change the URL for viewing a course?  I think so.
-        request.session[ACTIVE_CCX_KEY] = None
 
     context = {
         'enrollment_message': enrollment_message,

common/lib/xmodule/xmodule/modulestore/django.py

@@ -193,6 +193,15 @@ def modulestore():
             settings.MODULESTORE['default'].get('OPTIONS', {})
         )
 
+        if settings.FEATURES.get('CUSTOM_COURSES_EDX'):
+            # TODO: This import prevents a circular import issue, but is
+            # symptomatic of a lib having a dependency on code in lms.  This
+            # should be updated to have a setting that enumerates modulestore
+            # wrappers and then uses that setting to wrap the modulestore in
+            # appropriate wrappers depending on enabled features.
+            from ccx.modulestore import CCXModulestoreWrapper  # pylint: disable=import-error
+            _MIXED_MODULESTORE = CCXModulestoreWrapper(_MIXED_MODULESTORE)
+
     return _MIXED_MODULESTORE
 
 

common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py

@@ -72,6 +72,9 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
         )
         self.modulestore = modulestore
         self.course_entry = course_entry
+        # set course_id attribute to avoid problems with subsystems that expect
+        # it here. (grading, for example)
+        self.course_id = course_entry.course_key
         self.lazy = lazy
         self.module_data = module_data
         self.default_class = default_class

lms/djangoapps/ccx/__init__.py

-"""
-we use this to mark the active ccx, for use by ccx middleware and some views
-"""
-ACTIVE_CCX_KEY = '_ccx_id'

lms/djangoapps/ccx/overrides.py

@@ -3,16 +3,18 @@ API related to providing field overrides for individual students.  This is used
 by the individual custom courses feature.
 """
 import json
-import threading
-
-from contextlib import contextmanager
+import logging
 
 from django.db import transaction, IntegrityError
 
 from courseware.field_overrides import FieldOverrideProvider  # pylint: disable=import-error
-from ccx import ACTIVE_CCX_KEY  # pylint: disable=import-error
+from opaque_keys.edx.keys import CourseKey, UsageKey
+from ccx_keys.locator import CCXLocator, CCXBlockUsageLocator
+
+from .models import CcxFieldOverride, CustomCourseForEdX
+
 
-from .models import CcxMembership, CcxFieldOverride
+log = logging.getLogger(__name__)
 
 
 class CustomCoursesForEdxOverrideProvider(FieldOverrideProvider):
@@ -25,43 +27,41 @@ class CustomCoursesForEdxOverrideProvider(FieldOverrideProvider):
         """
         Just call the get_override_for_ccx method if there is a ccx
         """
-        ccx = get_current_ccx()
+        # The incoming block might be a CourseKey instance of some type, a
+        # UsageKey instance of some type, or it might be something that has a
+        # location attribute.  That location attribute will be a UsageKey
+        ccx = course_key = None
+        identifier = getattr(block, 'id', None)
+        if isinstance(identifier, CourseKey):
+            course_key = block.id
+        elif isinstance(identifier, UsageKey):
+            course_key = block.id.course_key
+        elif hasattr(block, 'location'):
+            course_key = block.location.course_key
+        else:
+            msg = "Unable to get course id when calculating ccx overide for block type %r"
+            log.error(msg, type(block))
+        if course_key is not None:
+            ccx = get_current_ccx(course_key)
         if ccx:
             return get_override_for_ccx(ccx, block, name, default)
         return default
 
 
-class _CcxContext(threading.local):
+def get_current_ccx(course_key):
     """
-    A threading local used to implement the `with_ccx` context manager, that
-    keeps track of the CCX currently set as the context.
-    """
-    ccx = None
-    request = None
-
-
-_CCX_CONTEXT = _CcxContext()
+    Return the ccx that is active for this course.
 
-
-@contextmanager
-def ccx_context(ccx):
-    """
-    A context manager which can be used to explicitly set the CCX that is in
-    play for field overrides.  This mechanism overrides the standard mechanism
-    of looking in the user's session to see if they are enrolled in a CCX and
-    viewing that CCX.
+    course_key is expected to be an instance of an opaque CourseKey, a
+    ValueError is raised if this expectation is not met.
     """
-    prev = _CCX_CONTEXT.ccx
-    _CCX_CONTEXT.ccx = ccx
-    yield
-    _CCX_CONTEXT.ccx = prev
+    if not isinstance(course_key, CourseKey):
+        raise ValueError("get_current_ccx requires a CourseKey instance")
 
+    if not isinstance(course_key, CCXLocator):
+        return None
 
-def get_current_ccx():
-    """
-    Return the ccx that is active for this request.
-    """
-    return _CCX_CONTEXT.ccx
+    return CustomCourseForEdX.objects.get(pk=course_key.ccx)
 
 
 def get_override_for_ccx(ccx, block, name, default=None):
@@ -85,9 +85,14 @@ def _get_overrides_for_ccx(ccx, block):
     overrides set on this block for this CCX.
     """
     overrides = {}
+    # block as passed in may have a location specific to a CCX, we must strip
+    # that for this query
+    location = block.location
+    if isinstance(block.location, CCXBlockUsageLocator):
+        location = block.location.to_block_locator()
     query = CcxFieldOverride.objects.filter(
         ccx=ccx,
-        location=block.location
+        location=location
     )
     for override in query:
         field = block.fields[override.field]
@@ -141,35 +146,3 @@ def clear_override_for_ccx(ccx, block, name):
 
     except CcxFieldOverride.DoesNotExist:
         pass
-
-
-class CcxMiddleware(object):
-    """
-    Checks to see if current session is examining a CCX and sets the CCX as
-    the current CCX for the override machinery if so.
-    """
-    def process_request(self, request):
-        """
-        Do the check.
-        """
-        ccx_id = request.session.get(ACTIVE_CCX_KEY, None)
-        if ccx_id is not None:
-            try:
-                membership = CcxMembership.objects.get(
-                    student=request.user, active=True, ccx__id__exact=ccx_id
-                )
-                _CCX_CONTEXT.ccx = membership.ccx
-            except CcxMembership.DoesNotExist:
-                # if there is no membership, be sure to unset the active ccx
-                _CCX_CONTEXT.ccx = None
-                request.session.pop(ACTIVE_CCX_KEY)
-
-        _CCX_CONTEXT.request = request
-
-    def process_response(self, request, response):  # pylint: disable=unused-argument
-        """
-        Clean up afterwards.
-        """
-        _CCX_CONTEXT.ccx = None
-        _CCX_CONTEXT.request = None
-        return response

lms/djangoapps/ccx/tests/test_ccx_modulestore.py

+"""
+Test the CCXModulestoreWrapper
+"""
+from collections import deque
+from ccx_keys.locator import CCXLocator
+import datetime
+from itertools import izip_longest, chain
+import pytz
+from student.tests.factories import AdminFactory
+from xmodule.modulestore.tests.django_utils import (
+    ModuleStoreTestCase,
+    TEST_DATA_SPLIT_MODULESTORE
+)
+from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
+
+from ..models import CustomCourseForEdX
+
+
+class TestCCXModulestoreWrapper(ModuleStoreTestCase):
+    """tests for a modulestore wrapped by CCXModulestoreWrapper
+    """
+    MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
+
+    def setUp(self):
+        """
+        Set up tests
+        """
+        super(TestCCXModulestoreWrapper, self).setUp()
+        self.course = course = CourseFactory.create()
+
+        # Create instructor account
+        coach = AdminFactory.create()
+
+        # Create a course outline
+        self.mooc_start = start = datetime.datetime(
+            2010, 5, 12, 2, 42, tzinfo=pytz.UTC)
+        self.mooc_due = due = datetime.datetime(
+            2010, 7, 7, 0, 0, tzinfo=pytz.UTC)
+        self.chapters = chapters = [
+            ItemFactory.create(start=start, parent=course) for _ in xrange(2)
+        ]
+        self.sequentials = sequentials = [
+            ItemFactory.create(parent=c) for _ in xrange(2) for c in chapters
+        ]
+        self.verticals = verticals = [
+            ItemFactory.create(
+                due=due, parent=s, graded=True, format='Homework'
+            ) for _ in xrange(2) for s in sequentials
+        ]
+        self.blocks = [
+            ItemFactory.create(parent=v) for _ in xrange(2) for v in verticals
+        ]
+
+        self.ccx = ccx = CustomCourseForEdX(
+            course_id=course.id,
+            display_name='Test CCX',
+            coach=coach
+        )
+        ccx.save()
+
+        self.ccx_locator = CCXLocator.from_course_locator(course.id, ccx.id)  # pylint: disable=no-member
+
+    def get_all_children_bf(self, block):
+        """traverse the children of block in a breadth-first order"""
+        queue = deque([block])
+        while queue:
+            item = queue.popleft()
+            yield item
+            queue.extend(item.get_children())
+
+    def get_course(self, key):
+        """get a course given a key"""
+        with self.store.bulk_operations(key):
+            course = self.store.get_course(key)
+        return course
+
+    def test_get_course(self):
+        """retrieving a course with a ccx key works"""
+        expected = self.get_course(self.ccx_locator.to_course_locator())
+        actual = self.get_course(self.ccx_locator)
+        self.assertEqual(
+            expected.location.course_key,
+            actual.location.course_key.to_course_locator())
+        self.assertEqual(expected.display_name, actual.display_name)
+
+    def test_get_children(self):
+        """the children of retrieved courses should be the same with course and ccx keys
+        """
+        course_key = self.ccx_locator.to_course_locator()
+        course = self.get_course(course_key)
+        ccx = self.get_course(self.ccx_locator)
+        test_fodder = izip_longest(
+            self.get_all_children_bf(course), self.get_all_children_bf(ccx)
+        )
+        for expected, actual in test_fodder:
+            if expected is None:
+                self.fail('course children exhausted before ccx children')
+            if actual is None:
+                self.fail('ccx children exhausted before course children')
+            self.assertEqual(expected.display_name, actual.display_name)
+            self.assertEqual(expected.location.course_key, course_key)
+            self.assertEqual(actual.location.course_key, self.ccx_locator)
+
+    def test_has_item(self):
+        """can verify that a location exists, using ccx block usage key"""
+        for item in chain(self.chapters, self.sequentials, self.verticals, self.blocks):
+            block_key = self.ccx_locator.make_usage_key(
+                item.location.block_type, item.location.block_id
+            )
+            self.assertTrue(self.store.has_item(block_key))
+
+    def test_get_item(self):
+        """can retrieve an item by a location key, using a ccx block usage key
+
+        the retrieved item should be the same as the the one read without ccx
+        info
+        """
+        for expected in chain(self.chapters, self.sequentials, self.verticals, self.blocks):
+            block_key = self.ccx_locator.make_usage_key(
+                expected.location.block_type, expected.location.block_id
+            )
+            actual = self.store.get_item(block_key)
+            self.assertEqual(expected.display_name, actual.display_name)
+            self.assertEqual(expected.location, actual.location.to_block_locator())
+
+    def test_publication_api(self):
+        """verify that we can correctly discern a published item by ccx key"""
+        for expected in self.blocks:
+            block_key = self.ccx_locator.make_usage_key(
+                expected.location.block_type, expected.location.block_id
+            )
+            self.assertTrue(self.store.has_published_version(expected))
+            self.store.unpublish(block_key, self.user.id)
+            self.assertFalse(self.store.has_published_version(expected))
+            self.store.publish(block_key, self.user.id)
+            self.assertTrue(self.store.has_published_version(expected))

lms/djangoapps/ccx/tests/test_overrides.py

@@ -9,7 +9,9 @@ from nose.plugins.attrib import attr
 from courseware.field_overrides import OverrideFieldData  # pylint: disable=import-error
 from django.test.utils import override_settings
 from student.tests.factories import AdminFactory  # pylint: disable=import-error
-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, ItemFactory
 
 from ..models import CustomCourseForEdX
@@ -25,6 +27,8 @@ class TestFieldOverrides(ModuleStoreTestCase):
     """
     Make sure field overrides behave in the expected manner.
     """
+    MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
+
     def setUp(self):
         """
         Set up tests
@@ -64,7 +68,7 @@ class TestFieldOverrides(ModuleStoreTestCase):
         # sure if there's a way to poke the test harness to do so.  So, we'll
         # just inject the override field storage in this brute force manner.
         OverrideFieldData.provider_classes = None
-        for block in iter_blocks(course):
+        for block in iter_blocks(ccx.course):
             block._field_data = OverrideFieldData.wrap(   # pylint: disable=protected-access
                 AdminFactory.create(), block._field_data)   # pylint: disable=protected-access
 
@@ -81,7 +85,7 @@ class TestFieldOverrides(ModuleStoreTestCase):
         Test that overriding start date on a chapter works.
         """
         ccx_start = datetime.datetime(2014, 12, 25, 00, 00, tzinfo=pytz.UTC)
-        chapter = self.course.get_children()[0]
+        chapter = self.ccx.course.get_children()[0]
         override_field_for_ccx(self.ccx, chapter, 'start', ccx_start)
         self.assertEquals(chapter.start, ccx_start)
 
@@ -90,7 +94,7 @@ class TestFieldOverrides(ModuleStoreTestCase):
         Test that overriding and accessing a field produce same number of queries.
         """
         ccx_start = datetime.datetime(2014, 12, 25, 00, 00, tzinfo=pytz.UTC)
-        chapter = self.course.get_children()[0]
+        chapter = self.ccx.course.get_children()[0]
         with self.assertNumQueries(4):
             override_field_for_ccx(self.ccx, chapter, 'start', ccx_start)
             dummy = chapter.start
@@ -100,7 +104,7 @@ class TestFieldOverrides(ModuleStoreTestCase):
         Test no extra queries when accessing an overriden field more than once.
         """
         ccx_start = datetime.datetime(2014, 12, 25, 00, 00, tzinfo=pytz.UTC)
-        chapter = self.course.get_children()[0]
+        chapter = self.ccx.course.get_children()[0]
         with self.assertNumQueries(4):
             override_field_for_ccx(self.ccx, chapter, 'start', ccx_start)
             dummy1 = chapter.start
@@ -112,7 +116,7 @@ class TestFieldOverrides(ModuleStoreTestCase):
         Test that sequentials inherit overridden start date from chapter.
         """
         ccx_start = datetime.datetime(2014, 12, 25, 00, 00, tzinfo=pytz.UTC)
-        chapter = self.course.get_children()[0]
+        chapter = self.ccx.course.get_children()[0]
         override_field_for_ccx(self.ccx, chapter, 'start', ccx_start)
         self.assertEquals(chapter.get_children()[0].start, ccx_start)
         self.assertEquals(chapter.get_children()[1].start, ccx_start)
@@ -124,7 +128,7 @@ class TestFieldOverrides(ModuleStoreTestCase):
         the mooc.
         """
         ccx_due = datetime.datetime(2015, 1, 1, 00, 00, tzinfo=pytz.UTC)
-        chapter = self.course.get_children()[0]
+        chapter = self.ccx.course.get_children()[0]
         chapter.display_name = 'itsme!'
         override_field_for_ccx(self.ccx, chapter, 'due', ccx_due)
         vertical = chapter.get_children()[0].get_children()[0]

lms/djangoapps/ccx/tests/test_utils.py

@@ -16,11 +16,12 @@ from student.roles import CourseCcxCoachRole  # pylint: disable=import-error
 from student.tests.factories import (  # pylint: disable=import-error
     AdminFactory,
     UserFactory,
-    CourseEnrollmentFactory,
-    AnonymousUserFactory,
 )
-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
+from ccx_keys.locator import CCXLocator
 
 
 @attr('shard_1')
@@ -68,9 +69,9 @@ class TestEmailEnrollmentState(ModuleStoreTestCase):
         """verify behavior for non-user email address
         """
         ee_state = self.create_one(email='nobody@nowhere.com')
-        for attr in ['user', 'member', 'full_name', 'in_ccx']:
-            value = getattr(ee_state, attr, 'missing attribute')
-            self.assertFalse(value, "{}: {}".format(value, attr))
+        for attribute in ['user', 'member', 'full_name', 'in_ccx']:
+            value = getattr(ee_state, attribute, 'missing attribute')
+            self.assertFalse(value, "{}: {}".format(value, attribute))
 
     def test_enrollment_state_for_non_member_user(self):
         """verify behavior for email address of user who is not a ccx memeber
@@ -88,10 +89,10 @@ class TestEmailEnrollmentState(ModuleStoreTestCase):
         self.create_user()
         self.register_user_in_ccx()
         ee_state = self.create_one()
-        for attr in ['user', 'in_ccx']:
+        for attribute in ['user', 'in_ccx']:
             self.assertTrue(
-                getattr(ee_state, attr, False),
-                "attribute {} is missing or False".format(attr)
+                getattr(ee_state, attribute, False),
+                "attribute {} is missing or False".format(attribute)
             )
         self.assertEqual(ee_state.member, self.user)
         self.assertEqual(ee_state.full_name, self.user.profile.name)
@@ -128,6 +129,8 @@ class TestEmailEnrollmentState(ModuleStoreTestCase):
 class TestGetEmailParams(ModuleStoreTestCase):
     """tests for ccx.utils.get_email_params
     """
+    MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
+
     def setUp(self):
         """
         Set up tests
@@ -157,7 +160,7 @@ class TestGetEmailParams(ModuleStoreTestCase):
         self.assertFalse(set(params.keys()) - set(self.all_keys))
 
     def test_ccx_id_in_params(self):
-        expected_course_id = self.ccx.course_id.to_deprecated_string()
+        expected_course_id = unicode(CCXLocator.from_course_locator(self.ccx.course_id, self.ccx.id))
         params = self.call_fut()
         self.assertEqual(params['course'], self.ccx)
         for url_key in self.url_keys:
@@ -185,6 +188,8 @@ class TestGetEmailParams(ModuleStoreTestCase):
 class TestEnrollEmail(ModuleStoreTestCase):
     """tests for the enroll_email function from ccx.utils
     """
+    MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
+
     def setUp(self):
         super(TestEnrollEmail, self).setUp()
         # unbind the user created by the parent, so we can create our own when
@@ -365,6 +370,8 @@ class TestEnrollEmail(ModuleStoreTestCase):
 # TODO: deal with changes in behavior for auto_enroll
 class TestUnenrollEmail(ModuleStoreTestCase):
     """Tests for the unenroll_email function from ccx.utils"""
+    MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
+
     def setUp(self):
         super(TestUnenrollEmail, self).setUp()
         # unbind the user created by the parent, so we can create our own when
@@ -511,67 +518,60 @@ class TestUnenrollEmail(ModuleStoreTestCase):
 
 
 @attr('shard_1')
-class TestUserCCXList(ModuleStoreTestCase):
-    """Unit tests for ccx.utils.get_all_ccx_for_user"""
+class TestGetMembershipTriplets(ModuleStoreTestCase):
+    """Verify that get_ccx_membership_triplets functions properly"""
+    MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
 
     def setUp(self):
-        """Create required infrastructure for tests"""
-        super(TestUserCCXList, self).setUp()
+        """Set up a course, coach, ccx and user"""
+        super(TestGetMembershipTriplets, self).setUp()
         self.course = CourseFactory.create()
         coach = AdminFactory.create()
         role = CourseCcxCoachRole(self.course.id)
         role.add_users(coach)
         self.ccx = CcxFactory(course_id=self.course.id, coach=coach)
-        enrollment = CourseEnrollmentFactory.create(course_id=self.course.id)
-        self.user = enrollment.user
-        self.anonymous = AnonymousUserFactory.create()
 
-    def register_user_in_ccx(self, active=False):
+    def make_ccx_membership(self, active=True):
         """create registration of self.user in self.ccx
 
-        registration will be inactive unless active=True
+        registration will be inactive
         """
-        CcxMembershipFactory(ccx=self.ccx, student=self.user, active=active)
+        CcxMembershipFactory.create(ccx=self.ccx, student=self.user, active=active)
 
-    def get_course_title(self):
-        """Get course title"""
-        from courseware.courses import get_course_about_section  # pylint: disable=import-error
-        return get_course_about_section(self.course, 'title')
-
-    def call_fut(self, user):
-        """Call function under test"""
-        from ccx.utils import get_all_ccx_for_user  # pylint: disable=import-error
-        return get_all_ccx_for_user(user)
+    def call_fut(self, org_filter=None, org_filter_out=()):
+        """call the function under test in this test case"""
+        from ccx.utils import get_ccx_membership_triplets
+        return list(
+            get_ccx_membership_triplets(self.user, org_filter, org_filter_out)
+        )
 
-    def test_anonymous_sees_no_ccx(self):
-        memberships = self.call_fut(self.anonymous)
-        self.assertEqual(memberships, [])
+    def test_no_membership(self):
+        """verify that no triplets are returned if there are no memberships
+        """
+        triplets = self.call_fut()
+        self.assertEqual(len(triplets), 0)
 
-    def test_unenrolled_sees_no_ccx(self):
-        memberships = self.call_fut(self.user)
-        self.assertEqual(memberships, [])
+    def test_has_membership(self):
+        """verify that a triplet is returned when a membership exists
+        """
+        self.make_ccx_membership()
+        triplets = self.call_fut()
+        self.assertEqual(len(triplets), 1)
+        ccx, membership, course = triplets[0]
+        self.assertEqual(ccx.id, self.ccx.id)
+        self.assertEqual(unicode(course.id), unicode(self.course.id))
+        self.assertEqual(membership.student, self.user)
+
+    def test_has_membership_org_filtered(self):
+        """verify that microsite org filter prevents seeing microsite ccx"""
+        self.make_ccx_membership()
+        bad_org = self.course.location.org + 'foo'
+        triplets = self.call_fut(org_filter=bad_org)
+        self.assertEqual(len(triplets), 0)
 
-    def test_enrolled_inactive_sees_no_ccx(self):
-        self.register_user_in_ccx()
-        memberships = self.call_fut(self.user)
-        self.assertEqual(memberships, [])
-
-    def test_enrolled_sees_a_ccx(self):
-        self.register_user_in_ccx(active=True)
-        memberships = self.call_fut(self.user)
-        self.assertEqual(len(memberships), 1)
-
-    def test_data_structure(self):
-        self.register_user_in_ccx(active=True)
-        memberships = self.call_fut(self.user)
-        this_membership = memberships[0]
-        self.assertTrue(this_membership)
-        # structure contains the expected keys
-        for key in ['ccx_name', 'ccx_url']:
-            self.assertTrue(key in this_membership.keys())
-        url_parts = [self.course.id.to_deprecated_string(), str(self.ccx.id)]
-        # all parts of the ccx url are present
-        for part in url_parts:
-            self.assertTrue(part in this_membership['ccx_url'])
-        actual_name = self.ccx.display_name
-        self.assertEqual(actual_name, this_membership['ccx_name'])
+    def test_has_membership_org_filtered_out(self):
+        """verify that microsite ccxs not seen in non-microsite view"""
+        self.make_ccx_membership()
+        filter_list = [self.course.location.org]
+        triplets = self.call_fut(org_filter_out=filter_list)
+        self.assertEqual(len(triplets), 0)

lms/djangoapps/ccx/urls.py

@@ -24,6 +24,4 @@ urlpatterns = patterns(
         'ccx.views.ccx_grades_csv', name='ccx_grades_csv'),
     url(r'^ccx_set_grading_policy$',
         'ccx.views.set_grading_policy', name='ccx_set_grading_policy'),
-    url(r'^switch_ccx(?:/(?P<ccx_id>[\d]+))?$',
-        'ccx.views.switch_active_ccx', name='switch_active_ccx'),
 )

lms/djangoapps/ccx/utils.py

@@ -4,8 +4,6 @@ CCX Enrollment operations for use by Coach APIs.
 Does not include any access control, be sure to check access before calling.
 """
 import logging
-from courseware.courses import get_course_about_section  # pylint: disable=import-error
-from courseware.courses import get_course_by_id  # pylint: disable=import-error
 from django.contrib.auth.models import User
 from django.conf import settings
 from django.core.urlresolvers import reverse
@@ -14,12 +12,12 @@ from edxmako.shortcuts import render_to_string  # pylint: disable=import-error
 from microsite_configuration import microsite  # pylint: disable=import-error
 from xmodule.modulestore.django import modulestore
 from xmodule.error_module import ErrorDescriptor
+from ccx_keys.locator import CCXLocator
 
 from .models import (
     CcxMembership,
     CcxFutureMembership,
 )
-from .overrides import get_current_ccx
 
 
 log = logging.getLogger("edx.ccx")
@@ -138,7 +136,6 @@ def get_email_params(ccx, auto_enroll, secure=True):
     get parameters for enrollment emails
     """
     protocol = 'https' if secure else 'http'
-    course_id = ccx.course_id
 
     stripped_site_name = microsite.get_value(
         'SITE_NAME',
@@ -154,7 +151,7 @@ def get_email_params(ccx, auto_enroll, secure=True):
         site=stripped_site_name,
         path=reverse(
             'course_root',
-            kwargs={'course_id': course_id.to_deprecated_string()}
+            kwargs={'course_id': CCXLocator.from_course_locator(ccx.course_id, ccx.id)}
         )
     )
 
@@ -165,7 +162,7 @@ def get_email_params(ccx, auto_enroll, secure=True):
             site=stripped_site_name,
             path=reverse(
                 'about_course',
-                kwargs={'course_id': course_id.to_deprecated_string()}
+                kwargs={'course_id': CCXLocator.from_course_locator(ccx.course_id, ccx.id)}
             )
         )
 
@@ -241,44 +238,6 @@ def send_mail_to_student(student, param_dict):
         )
 
 
-def get_all_ccx_for_user(user):
-    """return all CCXS to which the user is registered
-
-    Returns a list of dicts: {
-        ccx_name: <formatted title of CCX course>
-        ccx_url: <url to view this CCX>
-        ccx_active: True if this ccx is currently the 'active' one
-        mooc_name: <formatted title of the MOOC course for this CCX>
-        mooc_url: <url to view this MOOC>
-    }
-    """
-    if user.is_anonymous():
-        return []
-    current_active_ccx = get_current_ccx()
-    memberships = []
-    for membership in CcxMembership.memberships_for_user(user):
-        course = get_course_by_id(membership.ccx.course_id)
-        ccx = membership.ccx
-        ccx_title = ccx.display_name
-        mooc_title = get_course_about_section(course, 'title')
-        url = reverse(
-            'switch_active_ccx',
-            args=[course.id.to_deprecated_string(), membership.ccx.id]
-        )
-        mooc_url = reverse(
-            'switch_active_ccx',
-            args=[course.id.to_deprecated_string(), ]
-        )
-        memberships.append({
-            'ccx_name': ccx_title,
-            'ccx_url': url,
-            'active': membership.ccx == current_active_ccx,
-            'mooc_name': mooc_title,
-            'mooc_url': mooc_url,
-        })
-    return memberships
-
-
 def get_ccx_membership_triplets(user, course_org_filter, org_filter_out_set):
     """
     Get the relevant set of (CustomCourseForEdX, CcxMembership, Course)
@@ -300,6 +259,17 @@ def get_ccx_membership_triplets(user, course_org_filter, org_filter_out_set):
                 elif course.location.org in org_filter_out_set:
                     continue
 
+                # If, somehow, we've got a ccx that has been created for a
+                # course with a deprecated ID, we must filter it out. Emit a
+                # warning to the log so we can clean up.
+                if course.location.deprecated:
+                    log.warning(
+                        "CCX %s exists for course %s with deprecated id",
+                        ccx,
+                        ccx.course_id
+                    )
+                    continue
+
                 yield (ccx, membership, course)
             else:
                 log.error("User {0} enrolled in {2} course {1}".format(  # pylint: disable=logging-format-interpolation

lms/djangoapps/courseware/access.py

@@ -43,6 +43,7 @@ from util.milestones_helpers import (
     get_pre_requisite_courses_not_completed,
     any_unfulfilled_milestones,
 )
+from ccx_keys.locator import CCXLocator
 
 import dogstats_wrapper as dog_stats_api
 
@@ -91,6 +92,9 @@ 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):
@@ -106,6 +110,9 @@ def has_access(user, action, obj, course_key=None):
     if isinstance(obj, XBlock):
         return _has_access_descriptor(user, action, obj, course_key)
 
+    if isinstance(obj, CCXLocator):
+        return _has_access_ccx_key(user, action, obj)
+
     if isinstance(obj, CourseKey):
         return _has_access_course_key(user, action, obj)
 
@@ -488,6 +495,16 @@ def _has_access_course_key(user, action, course_key):
     return _dispatch(checkers, action, user, course_key)
 
 
+def _has_access_ccx_key(user, action, ccx_key):
+    """Check if user has access to the course for this ccx_key
+
+    Delegates checking to _has_access_course_key
+    Valid actions: same as for that function
+    """
+    course_key = ccx_key.to_course_locator()
+    return _has_access_course_key(user, action, course_key)
+
+
 def _has_access_string(user, action, perm):
     """
     Check if user has certain special access, specified as string.  Valid strings:

lms/djangoapps/dashboard/sysadmin.py

@@ -41,7 +41,6 @@ from student.models import CourseEnrollment, UserProfile, Registration
 import track.views
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
-from xmodule.modulestore.xml import XMLModuleStore
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 
 
@@ -60,8 +59,9 @@ class SysadminDashboardView(TemplateView):
         """
 
         self.def_ms = modulestore()
+
         self.is_using_mongo = True
-        if isinstance(self.def_ms, XMLModuleStore):
+        if self.def_ms.get_modulestore_type(None) == 'xml':
             self.is_using_mongo = False
         self.msg = u''
         self.datatable = []

lms/djangoapps/dashboard/tests/test_sysadmin.py

@@ -32,7 +32,6 @@ from student.tests.factories import UserFactory
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST
-from xmodule.modulestore.xml import XMLModuleStore
 
 
 TEST_MONGODB_LOG = {
@@ -316,7 +315,8 @@ class TestSysadmin(SysadminBaseTestCase):
         response = self._add_edx4edx()
 
         def_ms = modulestore()
-        self.assertIn('xml', str(def_ms.__class__))
+
+        self.assertEqual('xml', def_ms.get_modulestore_type(None))
         course = def_ms.courses.get('{0}/edx4edx_lite'.format(
             os.path.abspath(settings.DATA_DIR)), None)
         self.assertIsNotNone(course)
@@ -460,7 +460,7 @@ class TestSysAdminMongoCourseImport(SysadminBaseTestCase):
         self._mkdir(getattr(settings, 'GIT_REPO_DIR'))
 
         def_ms = modulestore()
-        self.assertFalse(isinstance(def_ms, XMLModuleStore))
+        self.assertFalse('xml' == def_ms.get_modulestore_type(None))
 
         self._add_edx4edx()
         course = def_ms.get_course(SlashSeparatedCourseKey('MITx', 'edx4edx', 'edx4edx'))

lms/djangoapps/django_comment_client/forum/views.py

@@ -65,7 +65,7 @@ class DiscussionTab(EnrolledTab):
             return False
 
         if settings.FEATURES.get('CUSTOM_COURSES_EDX', False):
-            if get_current_ccx():
+            if get_current_ccx(course.id):
                 return False
         return settings.FEATURES.get('ENABLE_DISCUSSION_SERVICE')
 

lms/envs/aws.py

@@ -612,7 +612,6 @@ ECOMMERCE_API_TIMEOUT = ENV_TOKENS.get('ECOMMERCE_API_TIMEOUT', ECOMMERCE_API_TI
 ##### Custom Courses for EdX #####
 if FEATURES.get('CUSTOM_COURSES_EDX'):
     INSTALLED_APPS += ('ccx',)
-    MIDDLEWARE_CLASSES += ('ccx.overrides.CcxMiddleware',)
     FIELD_OVERRIDE_PROVIDERS += (
         'ccx.overrides.CustomCoursesForEdxOverrideProvider',
     )

lms/envs/test.py

@@ -475,7 +475,6 @@ FEATURES['CERTIFICATES_HTML_VIEW'] = True
 
 ######### custom courses #########
 INSTALLED_APPS += ('ccx',)
-MIDDLEWARE_CLASSES += ('ccx.overrides.CcxMiddleware',)
 FEATURES['CUSTOM_COURSES_EDX'] = True
 
 # Set dummy values for profile image settings.

lms/envs/yaml_config.py

@@ -307,7 +307,6 @@ GRADES_DOWNLOAD_ROUTING_KEY = HIGH_MEM_QUEUE
 ##### Custom Courses for EdX #####
 if FEATURES.get('CUSTOM_COURSES_EDX'):
     INSTALLED_APPS += ('ccx',)
-    MIDDLEWARE_CLASSES += ('ccx.overrides.CcxMiddleware',)
     FIELD_OVERRIDE_PROVIDERS += (
         'ccx.overrides.CustomCoursesForEdxOverrideProvider',
     )

lms/templates/ccx/_dashboard_ccx_listing.html

@@ -4,9 +4,10 @@
 from django.utils.translation import ugettext as _
 from django.core.urlresolvers import reverse
 from courseware.courses import course_image_url, get_course_about_section
+from ccx_keys.locator import CCXLocator
 %>
 <%
-  ccx_switch_target = reverse('switch_active_ccx', args=[course.id.to_deprecated_string(), ccx.id])
+  ccx_target = reverse('info', args=[CCXLocator.from_course_locator(course.id, ccx.id)])
 %>
 <li class="course-item">
   <article class="course">
@@ -14,7 +15,7 @@ from courseware.courses import course_image_url, get_course_about_section
       <div class="wrapper-course-image" aria-hidden="true">
         % if show_courseware_link:
           % if not is_course_blocked:
-              <a href="${ccx_switch_target}" class="cover">
+              <a href="${ccx_target}" class="cover">
                 <img src="${course_image_url(course)}" class="course-image" alt="${_('{course_number} {ccx_name} Cover Image').format(course_number=course.number, ccx_name=ccx.display_name) |h}" />
               </a>
           % else:
@@ -32,7 +33,7 @@ from courseware.courses import course_image_url, get_course_about_section
         <h3 class="course-title">
           % if show_courseware_link:
             % if not is_course_blocked:
-              <a href="${ccx_switch_target}">${ccx.display_name}</a>
+              <a href="${ccx_target}">${ccx.display_name}</a>
             % else:
               <a class="disable-look">${ccx.display_name}</a>
             % endif
@@ -58,13 +59,13 @@ from courseware.courses import course_image_url, get_course_about_section
             <div class="course-actions">
               % if ccx.has_ended():
                 % if not is_course_blocked:
-                  <a href="${ccx_switch_target}" class="enter-course archived">${_('View Archived Custom Course')}<span class="sr">&nbsp;${ccx.display_name}</span></a>
+                  <a href="${ccx_target}" class="enter-course archived">${_('View Archived Custom Course')}<span class="sr">&nbsp;${ccx.display_name}</span></a>
                 % else:
                   <a class="enter-course-blocked archived">${_('View Archived Custom Course')}<span class="sr">&nbsp;${ccx.display_name}</span></a>
                 % endif
               % else:
                 % if not is_course_blocked:
-                  <a href="${ccx_switch_target}" class="enter-course">${_('View Custom Course')}<span class="sr">&nbsp;${ccx.display_name}</span></a>
+                  <a href="${ccx_target}" class="enter-course">${_('View Custom Course')}<span class="sr">&nbsp;${ccx.display_name}</span></a>
                 % else:
                   <a class="enter-course-blocked">${_('View Custom Course')}<span class="sr">&nbsp;${ccx.display_name}</span></a>
                 % endif

lms/templates/ccx/coach_dashboard.html

@@ -22,6 +22,18 @@ from django.core.urlresolvers import reverse
     <section class="instructor-dashboard-content-2" id="ccx-coach-dashboard-content">
       <h1>${_("CCX Coach Dashboard")}</h1>
 
+      % if messages:
+      <ul class="messages">
+        % for message in messages:
+          % if message.tags:
+          <li class="${message.tags}">${message}</li>
+          % else:
+          <li>${message}</li>
+          % endif
+        % endfor
+        </ul>
+      % endif
+
       %if not ccx:
       <section>
         <form action="${create_ccx_url}" method="POST">

lms/templates/navigation.html

@@ -53,7 +53,7 @@ site_status_msg = get_site_status_msg(course_id)
       <%
         display_name = course.display_name_with_default
         if settings.FEATURES.get('CUSTOM_COURSES_EDX', False):
-          ccx = get_current_ccx()
+          ccx = get_current_ccx(course.id)
           if ccx:
             display_name = ccx.display_name
       %>