{"action":"created","comment":{"url":"https://api.github.com/repos/edx/edx-platform/pulls/comments/32229867","id":32229867,"diff_hunk":"@@ -25,43 +27,42 @@ def get(self, block, name, default):\n         \"\"\"\n         Just call the get_override_for_ccx method if there is a ccx\n         \"\"\"\n-        ccx = get_current_ccx()\n+        # The incoming block might be a CourseKey instance of some type, a\n+        # UsageKey instance of some type, or it might be something that has a\n+        # location attribute.  That location attribute will be a UsageKey\n+        ccx = course_id = None\n+        identifier = getattr(block, 'id', None)\n+        if isinstance(identifier, CourseKey):\n+            course_id = block.id\n+        elif isinstance(identifier, UsageKey):\n+            course_id = block.id.course_key\n+        elif hasattr(block, 'location'):\n+            course_id = block.location.course_key\n+        else:\n+            msg = \"Unable to get course id when calculating ccx overide for block type {}\"\n+            log.error(msg.format(type(block)))\n+        if course_id is not None:\n+            ccx = get_current_ccx(course_id)\n         if ccx:\n             return get_override_for_ccx(ccx, block, name, default)\n         return default\n \n \n-class _CcxContext(threading.local):\n+def get_current_ccx(course_id):\n     \"\"\"\n-    A threading local used to implement the `with_ccx` context manager, that\n-    keeps track of the CCX currently set as the context.\n+    Return the ccx that is active for this course.\n     \"\"\"\n-    ccx = None\n-    request = None\n-\n-\n-_CCX_CONTEXT = _CcxContext()\n-\n-\n-@contextmanager\n-def ccx_context(ccx):\n-    \"\"\"\n-    A context manager which can be used to explicitly set the CCX that is in\n-    play for field overrides.  This mechanism overrides the standard mechanism\n-    of looking in the user's session to see if they are enrolled in a CCX and\n-    viewing that CCX.\n-    \"\"\"\n-    prev = _CCX_CONTEXT.ccx\n-    _CCX_CONTEXT.ccx = ccx\n-    yield\n-    _CCX_CONTEXT.ccx = prev\n-\n+    # ensure that the ID passed in is a CourseKey instance of some type.\n+    if isinstance(course_id, CourseKey):\n+        course_key = course_id\n+    else:\n+        course_key = CourseKey.from_string(course_id)\n \n-def get_current_ccx():\n-    \"\"\"\n-    Return the ccx that is active for this request.\n-    \"\"\"\n-    return _CCX_CONTEXT.ccx\n+    ccx = None\n+    if isinstance(course_key, CCXLocator):\n+        ccx_id = course_key.ccx\n+        ccx = CustomCourseForEdX.objects.get(pk=ccx_id)\n+    return ccx","path":"lms/djangoapps/ccx/overrides.py","position":92,"original_position":92,"commit_id":"774ba17c30862884318b624f37b1dd358d6e3c69","original_commit_id":"cd7fc2e386710e7b555b2a6b1104ed66c935727c","user":{"login":"cpennington","id":30236,"avatar_url":"https://avatars.githubusercontent.com/u/30236?v=3","gravatar_id":"","url":"https://api.github.com/users/cpennington","html_url":"https://github.com/cpennington","followers_url":"https://api.github.com/users/cpennington/followers","following_url":"https://api.github.com/users/cpennington/following{/other_user}","gists_url":"https://api.github.com/users/cpennington/gists{/gist_id}","starred_url":"https://api.github.com/users/cpennington/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cpennington/subscriptions","organizations_url":"https://api.github.com/users/cpennington/orgs","repos_url":"https://api.github.com/users/cpennington/repos","events_url":"https://api.github.com/users/cpennington/events{/privacy}","received_events_url":"https://api.github.com/users/cpennington/received_events","type":"User","site_admin":false},"body":"I tend to prefer the other style, but that's definitely a personal preference, not an edx.org style requirement.","created_at":"2015-06-11T15:01:21Z","updated_at":"2015-06-11T15:01:21Z","html_url":"https://github.com/edx/edx-platform/pull/8395#discussion_r32229867","pull_request_url":"https://api.github.com/repos/edx/edx-platform/pulls/8395","_links":{"self":{"href":"https://api.github.com/repos/edx/edx-platform/pulls/comments/32229867"},"html":{"href":"https://github.com/edx/edx-platform/pull/8395#discussion_r32229867"},"pull_request":{"href":"https://api.github.com/repos/edx/edx-platform/pulls/8395"}}},"pull_request":{"url":"https://api.github.com/repos/edx/edx-platform/pulls/8395","id":37060643,"html_url":"https://github.com/edx/edx-platform/pull/8395","diff_url":"https://github.com/edx/edx-platform/pull/8395.diff","patch_url":"https://github.com/edx/edx-platform/pull/8395.patch","issue_url":"https://api.github.com/repos/edx/edx-platform/issues/8395","number":8395,"state":"open","locked":false,"title":"MIT CCX: Use CCX Keys","user":{"login":"cewing","id":417109,"avatar_url":"https://avatars.githubusercontent.com/u/417109?v=3","gravatar_id":"","url":"https://api.github.com/users/cewing","html_url":"https://github.com/cewing","followers_url":"https://api.github.com/users/cewing/followers","following_url":"https://api.github.com/users/cewing/following{/other_user}","gists_url":"https://api.github.com/users/cewing/gists{/gist_id}","starred_url":"https://api.github.com/users/cewing/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cewing/subscriptions","organizations_url":"https://api.github.com/users/cewing/orgs","repos_url":"https://api.github.com/users/cewing/repos","events_url":"https://api.github.com/users/cewing/events{/privacy}","received_events_url":"https://api.github.com/users/cewing/received_events","type":"User","site_admin":false},"body":"This pull request implements the use of ccx-specific opaque keys (`CCXLocator` and `CCXBlockUsageLocator` throughout the codebase.  \r\n\r\n### History\r\n\r\nThe request to implement custom opaque keys for ccx was first brought up in an architecture review of the PR referenced above.  It was felt that using such keys would allow ccx to better integrate with existing logging and analytics systems.  The outcome of this conversation was captured in jazkarta/edx-platform#43 \r\n\r\nA separate PR (https://github.com/edx/edx-platform/pull/8259) has been opened to add the dependency upon the [custom key implementation](https://github.com/jazkarta/ccx-keys).  That PR is under review and should be merged prior to this.\r\n\r\nDuring implementation of this code, @nedbat, @cpennington, @ormsbee, and @carsongee have all been pulled into discussions at various points.\r\n\r\nThe work has been overseen by @pdpinch and the team at MIT.\r\n\r\n### Purpose\r\n\r\nBy creating a custom opaque-key implementation, the mechanism by which a user views CCX content (as opposed to the course from which the ccx was created) is simplified, and made to fall in line with existing mechanisms for retrieving course modules, assets and so on.  In addition, because there is a \"locator\" for a ccx that is distinct from that for the course, logging and analytics can register the use of CCX content separately from the use of the same materials for the original course.\r\n\r\n### Impacts\r\n\r\nThis implementation adds a wrapper around the modulestore returned by `xmodule.modulestore.django.modulestore`.  When the CCX feature is enabled in settings, then wherever that method is called, a wrapped version of the modulestore will be returned.  This wrapper strips the ccx information out of locators and keys as queries are sent in to the modulestore and restores the information to the data that is returned.  Because of this, there is little in the system that is not impacted by this change.  For queries to the modulestore that use non-ccx keys or locations, this wrapper should be transparent, but it is present.\r\n\r\nAn additional implication of this work is that CCXs can not be created from legacy courses that use the pre-split deprecated form of course keys and locations. This restriction can be worked around by creating a re-run of such a course, which will convert it to the new form of storage and keys/locations.  It is therefore not considered a blocker.\r\n\r\nIn all other respects, the work should not have any direct impact on users.  CCX functionality continues to work as it did before this change.  There are no user-facing UI elements that have changed.\r\n\r\nThis work is integrated with previous CCX work, so it will be released initially only on edge, where CCX is already in use. \r\n\r\n### Manual Testing\r\n\r\nIn order to test these changes, you must begin with a course that uses the split-modulestore and has opaque-key course keys and locations (`course-v1:org+course+run...`).  Once that is in place, then you can follow these steps:\r\n\r\n1.  Log in to the LMS an admin for the course, view the \"instructor\" tab and select \"membership\"\r\n2.  In the course staff section, select the 'CCX Coach' role and add a user as a ccx coach.\r\n3.  Log in as the CCX Coach user and visit the CCX Coach tab.\r\n4.  Fill in a name for your ccx and submit to create it. You should be redirected to the Coach Dashboard view for the CCX.\r\n5.  verify that when viewing the coach tab, you see a url that contains `ccx-v1:` and has `+ccx@n` in the identifier (`n` is the integer primary key of the CCX that was created). \r\n6.  verify that when viewing any other top level tab, you see only the course id (it should start with `course-v1:` and *not* contain any mention of ccx).\r\n7.  From the coach tab schedule some course content, but not all the items from the original course (so you can easily tell which you are seeing).\r\n8.  From the coach tab, enroll a user in your ccx.\r\n9.  Log in to the LMS as the student just enrolled by the coach.  You should see both the original course and the CCX on your dashboard.  They should be visually distinct\r\n10.  Click on the CCX to go to the course info page.  It should look identical to the original course info page.  The URL for the page should contain `ccx-v1:` and `+ccx@n`.  (this verifies that you are viewing the ccx).\r\n11.  Click on the courseware tab, verify that you see *only* those course elements that were scheduled by the coach, and not any others.\r\n12.  View and interact with the courseware.  Verify that everything works as expected.\r\n13.  Check your progress report and verify that it operates as expected.\r\n14.  Verify that the student cannot see the discussion tab (it should be hidden for any ccx).\r\n14.  Log in as Coach and check the grade reports under `student admin` to verify that grades are being correctly calculated.  Update the grading policy and verify that it is applied to both the Coach grade views and the student progress view.\r\n\r\n### Concerns\r\n\r\nThe modulestore wrapper is not well covered with tests.  We are not comfortable that we understand how best to approach the testing of this element.  We would appreciate any pointers in that regard.  \r\n\r\nThe management of CCX keys and locations versus Course keys and locations is a bit tricky and there are many possible edge cases we have yet to uncover.  There are a few places where manually unwrapping a ccx key is required to get course access information directly.  We would appreciate attention paid to this aspect.  If there are other places we have not yet found or thought of that might need special attention, please let us know.\r\n\r\nIt should be noted that at the time of this PR, there are 5 test failures and one error happening in my local development environment.  All of these are in the tests for the sysadmin dashboard and have to do with importing XML course content from git.  I believe the problem to be in my development environment, but if anyone (like @carsongee) has suggestions about fixing them should they also appear in jenkins, I'd appreciate it.","created_at":"2015-06-05T19:06:32Z","updated_at":"2015-06-11T15:01:21Z","closed_at":null,"merged_at":null,"merge_commit_sha":"9e1bcd4084bfb6ebaae799c94ae6f7bfcc270157","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/edx/edx-platform/pulls/8395/commits","review_comments_url":"https://api.github.com/repos/edx/edx-platform/pulls/8395/comments","review_comment_url":"https://api.github.com/repos/edx/edx-platform/pulls/comments{/number}","comments_url":"https://api.github.com/repos/edx/edx-platform/issues/8395/comments","statuses_url":"https://api.github.com/repos/edx/edx-platform/statuses/774ba17c30862884318b624f37b1dd358d6e3c69","head":{"label":"jazkarta:ccx-custom-ids","ref":"ccx-custom-ids","sha":"774ba17c30862884318b624f37b1dd358d6e3c69","user":{"login":"jazkarta","id":974974,"avatar_url":"https://avatars.githubusercontent.com/u/974974?v=3","gravatar_id":"","url":"https://api.github.com/users/jazkarta","html_url":"https://github.com/jazkarta","followers_url":"https://api.github.com/users/jazkarta/followers","following_url":"https://api.github.com/users/jazkarta/following{/other_user}","gists_url":"https://api.github.com/users/jazkarta/gists{/gist_id}","starred_url":"https://api.github.com/users/jazkarta/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jazkarta/subscriptions","organizations_url":"https://api.github.com/users/jazkarta/orgs","repos_url":"https://api.github.com/users/jazkarta/repos","events_url":"https://api.github.com/users/jazkarta/events{/privacy}","received_events_url":"https://api.github.com/users/jazkarta/received_events","type":"Organization","site_admin":false},"repo":{"id":13279839,"name":"edx-platform","full_name":"jazkarta/edx-platform","owner":{"login":"jazkarta","id":974974,"avatar_url":"https://avatars.githubusercontent.com/u/974974?v=3","gravatar_id":"","url":"https://api.github.com/users/jazkarta","html_url":"https://github.com/jazkarta","followers_url":"https://api.github.com/users/jazkarta/followers","following_url":"https://api.github.com/users/jazkarta/following{/other_user}","gists_url":"https://api.github.com/users/jazkarta/gists{/gist_id}","starred_url":"https://api.github.com/users/jazkarta/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jazkarta/subscriptions","organizations_url":"https://api.github.com/users/jazkarta/orgs","repos_url":"https://api.github.com/users/jazkarta/repos","events_url":"https://api.github.com/users/jazkarta/events{/privacy}","received_events_url":"https://api.github.com/users/jazkarta/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/jazkarta/edx-platform","description":"the edX learning management system (LMS) and course authoring tool, Studio","fork":true,"url":"https://api.github.com/repos/jazkarta/edx-platform","forks_url":"https://api.github.com/repos/jazkarta/edx-platform/forks","keys_url":"https://api.github.com/repos/jazkarta/edx-platform/keys{/key_id}","collaborators_url":"https://api.github.com/repos/jazkarta/edx-platform/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/jazkarta/edx-platform/teams","hooks_url":"https://api.github.com/repos/jazkarta/edx-platform/hooks","issue_events_url":"https://api.github.com/repos/jazkarta/edx-platform/issues/events{/number}","events_url":"https://api.github.com/repos/jazkarta/edx-platform/events","assignees_url":"https://api.github.com/repos/jazkarta/edx-platform/assignees{/user}","branches_url":"https://api.github.com/repos/jazkarta/edx-platform/branches{/branch}","tags_url":"https://api.github.com/repos/jazkarta/edx-platform/tags","blobs_url":"https://api.github.com/repos/jazkarta/edx-platform/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/jazkarta/edx-platform/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/jazkarta/edx-platform/git/refs{/sha}","trees_url":"https://api.github.com/repos/jazkarta/edx-platform/git/trees{/sha}","statuses_url":"https://api.github.com/repos/jazkarta/edx-platform/statuses/{sha}","languages_url":"https://api.github.com/repos/jazkarta/edx-platform/languages","stargazers_url":"https://api.github.com/repos/jazkarta/edx-platform/stargazers","contributors_url":"https://api.github.com/repos/jazkarta/edx-platform/contributors","subscribers_url":"https://api.github.com/repos/jazkarta/edx-platform/subscribers","subscription_url":"https://api.github.com/repos/jazkarta/edx-platform/subscription","commits_url":"https://api.github.com/repos/jazkarta/edx-platform/commits{/sha}","git_commits_url":"https://api.github.com/repos/jazkarta/edx-platform/git/commits{/sha}","comments_url":"https://api.github.com/repos/jazkarta/edx-platform/comments{/number}","issue_comment_url":"https://api.github.com/repos/jazkarta/edx-platform/issues/comments{/number}","contents_url":"https://api.github.com/repos/jazkarta/edx-platform/contents/{+path}","compare_url":"https://api.github.com/repos/jazkarta/edx-platform/compare/{base}...{head}","merges_url":"https://api.github.com/repos/jazkarta/edx-platform/merges","archive_url":"https://api.github.com/repos/jazkarta/edx-platform/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/jazkarta/edx-platform/downloads","issues_url":"https://api.github.com/repos/jazkarta/edx-platform/issues{/number}","pulls_url":"https://api.github.com/repos/jazkarta/edx-platform/pulls{/number}","milestones_url":"https://api.github.com/repos/jazkarta/edx-platform/milestones{/number}","notifications_url":"https://api.github.com/repos/jazkarta/edx-platform/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/jazkarta/edx-platform/labels{/name}","releases_url":"https://api.github.com/repos/jazkarta/edx-platform/releases{/id}","created_at":"2013-10-02T18:52:14Z","updated_at":"2015-05-07T18:55:56Z","pushed_at":"2015-06-11T04:50:55Z","git_url":"git://github.com/jazkarta/edx-platform.git","ssh_url":"git@github.com:jazkarta/edx-platform.git","clone_url":"https://github.com/jazkarta/edx-platform.git","svn_url":"https://github.com/jazkarta/edx-platform","homepage":"http://code.edx.org/","size":484311,"stargazers_count":1,"watchers_count":1,"language":"Python","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":1,"mirror_url":null,"open_issues_count":24,"forks":1,"open_issues":24,"watchers":1,"default_branch":"master"}},"base":{"label":"edx:master","ref":"master","sha":"ce8550dba6b6d3967c0ace85282667e84c46a7dc","user":{"login":"edx","id":3179841,"avatar_url":"https://avatars.githubusercontent.com/u/3179841?v=3","gravatar_id":"","url":"https://api.github.com/users/edx","html_url":"https://github.com/edx","followers_url":"https://api.github.com/users/edx/followers","following_url":"https://api.github.com/users/edx/following{/other_user}","gists_url":"https://api.github.com/users/edx/gists{/gist_id}","starred_url":"https://api.github.com/users/edx/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/edx/subscriptions","organizations_url":"https://api.github.com/users/edx/orgs","repos_url":"https://api.github.com/users/edx/repos","events_url":"https://api.github.com/users/edx/events{/privacy}","received_events_url":"https://api.github.com/users/edx/received_events","type":"Organization","site_admin":false},"repo":{"id":10391073,"name":"edx-platform","full_name":"edx/edx-platform","owner":{"login":"edx","id":3179841,"avatar_url":"https://avatars.githubusercontent.com/u/3179841?v=3","gravatar_id":"","url":"https://api.github.com/users/edx","html_url":"https://github.com/edx","followers_url":"https://api.github.com/users/edx/followers","following_url":"https://api.github.com/users/edx/following{/other_user}","gists_url":"https://api.github.com/users/edx/gists{/gist_id}","starred_url":"https://api.github.com/users/edx/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/edx/subscriptions","organizations_url":"https://api.github.com/users/edx/orgs","repos_url":"https://api.github.com/users/edx/repos","events_url":"https://api.github.com/users/edx/events{/privacy}","received_events_url":"https://api.github.com/users/edx/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/edx/edx-platform","description":"The Open edX platform, the software that powers edX!","fork":false,"url":"https://api.github.com/repos/edx/edx-platform","forks_url":"https://api.github.com/repos/edx/edx-platform/forks","keys_url":"https://api.github.com/repos/edx/edx-platform/keys{/key_id}","collaborators_url":"https://api.github.com/repos/edx/edx-platform/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/edx/edx-platform/teams","hooks_url":"https://api.github.com/repos/edx/edx-platform/hooks","issue_events_url":"https://api.github.com/repos/edx/edx-platform/issues/events{/number}","events_url":"https://api.github.com/repos/edx/edx-platform/events","assignees_url":"https://api.github.com/repos/edx/edx-platform/assignees{/user}","branches_url":"https://api.github.com/repos/edx/edx-platform/branches{/branch}","tags_url":"https://api.github.com/repos/edx/edx-platform/tags","blobs_url":"https://api.github.com/repos/edx/edx-platform/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/edx/edx-platform/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/edx/edx-platform/git/refs{/sha}","trees_url":"https://api.github.com/repos/edx/edx-platform/git/trees{/sha}","statuses_url":"https://api.github.com/repos/edx/edx-platform/statuses/{sha}","languages_url":"https://api.github.com/repos/edx/edx-platform/languages","stargazers_url":"https://api.github.com/repos/edx/edx-platform/stargazers","contributors_url":"https://api.github.com/repos/edx/edx-platform/contributors","subscribers_url":"https://api.github.com/repos/edx/edx-platform/subscribers","subscription_url":"https://api.github.com/repos/edx/edx-platform/subscription","commits_url":"https://api.github.com/repos/edx/edx-platform/commits{/sha}","git_commits_url":"https://api.github.com/repos/edx/edx-platform/git/commits{/sha}","comments_url":"https://api.github.com/repos/edx/edx-platform/comments{/number}","issue_comment_url":"https://api.github.com/repos/edx/edx-platform/issues/comments{/number}","contents_url":"https://api.github.com/repos/edx/edx-platform/contents/{+path}","compare_url":"https://api.github.com/repos/edx/edx-platform/compare/{base}...{head}","merges_url":"https://api.github.com/repos/edx/edx-platform/merges","archive_url":"https://api.github.com/repos/edx/edx-platform/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/edx/edx-platform/downloads","issues_url":"https://api.github.com/repos/edx/edx-platform/issues{/number}","pulls_url":"https://api.github.com/repos/edx/edx-platform/pulls{/number}","milestones_url":"https://api.github.com/repos/edx/edx-platform/milestones{/number}","notifications_url":"https://api.github.com/repos/edx/edx-platform/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/edx/edx-platform/labels{/name}","releases_url":"https://api.github.com/repos/edx/edx-platform/releases{/id}","created_at":"2013-05-30T20:20:38Z","updated_at":"2015-06-11T13:11:50Z","pushed_at":"2015-06-11T14:57:44Z","git_url":"git://github.com/edx/edx-platform.git","ssh_url":"git@github.com:edx/edx-platform.git","clone_url":"https://github.com/edx/edx-platform.git","svn_url":"https://github.com/edx/edx-platform","homepage":"http://open.edx.org/","size":1885935,"stargazers_count":1971,"watchers_count":1971,"language":"Python","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":1135,"mirror_url":null,"open_issues_count":173,"forks":1135,"open_issues":173,"watchers":1971,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/edx/edx-platform/pulls/8395"},"html":{"href":"https://github.com/edx/edx-platform/pull/8395"},"issue":{"href":"https://api.github.com/repos/edx/edx-platform/issues/8395"},"comments":{"href":"https://api.github.com/repos/edx/edx-platform/issues/8395/comments"},"review_comments":{"href":"https://api.github.com/repos/edx/edx-platform/pulls/8395/comments"},"review_comment":{"href":"https://api.github.com/repos/edx/edx-platform/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/edx/edx-platform/pulls/8395/commits"},"statuses":{"href":"https://api.github.com/repos/edx/edx-platform/statuses/774ba17c30862884318b624f37b1dd358d6e3c69"}}}}