jazkarta / edx-platform

the edX learning management system (LMS) and course authoring tool, Studio
http://code.edx.org/
GNU Affero General Public License v3.0
1 stars 0 forks source link

Resolve issue of student views when using CCXLocator urls #97

Closed cewing closed 9 years ago

cewing commented 9 years ago

from an email to @cpennington:

In the original version of CCX (and the one currently deployed) we used a threadlocal to store the ID of the ccx a student was viewing. This meant that the URL they visited to get to any view of the course (for example /courses//info) was the same for the CCX as it was for the course from which the CCX had been made.

What this meant, practically speaking is that when a URL was loaded, checks for enrollment and other relationships between the student and the course were done against the course itself, and only at the time when field lookups happened did the override come into play, finding the current ccx from the threadlocal value stored and then providing overridden data instead of the core data of the course.

Now we are loading URLs with ids that belong to the CCX, and so in a view like the course_info view (https://github.com/jazkarta/edx-platform/blob/master/lms/djangoapps/courseware/views.py#L656) when get_course_with_access is called and the ID derived from the URL is used, it is the CCX that is found. The thing is, of course, that for a CCX most of the data that is important to the student views is not on the CCX, but on the course from which it was built.

My question is, given that we want to be able to use these new CCXLocators as the source of URLs for student views, how best should we handle this difference. When we look up the course and most of its data and assets, we want to look up the course. When we get field data, we want to use the course, but wrap it with the current ccx, and when we render URLs for other views, we want to use the CCXLocator

cewing commented 9 years ago

from the email thread:

Looking at get_course_with_access (for instance), I see a couple of places where we probably need to work to avoid issues: 1) The modulestore. Our existing course storage is based purely on the non-CCX key, so we'll need to strip off the CCX-ness of the key before passing to the existing modulestores. My ideal solution here would be to add a wrapper that implements the modulestore APIs, removes the CCX-ness of the key on the way in, and adds it back in on the way out. However, thinking about how that will play out, I'm a bit concerned about the "add it back in" part, because it would require inspecting all of the objects coming out of the modulestores. What we'd really want here is to have a separate notion of course-local and absolute keys, so that the data coming out of the modulestore could just be course-local (and thus abstract from the notion of CCX). We do have one existing bit of prior-art as far as manipulating keys coming out of the modulestore, which is

https://github.com/edx/edx-platform/blob/master/common/lib/xmodule/xmodule/modulestore/mixed.py#L35-L95.

2) has_access. This should be relatively simple: inside _has_access_to_course, you can remove the CCX-ness from the key, and then check it against our existing roles.

cewing commented 9 years ago

@pdpinch, made good progress on this. Most student views now work. Some assets go missing due to incorrect urls built from ccx-aware locators.

cewing commented 9 years ago

This is now resolved. All student views are working correctly.