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

CCX crashes mobile api #126

Closed pdpinch closed 9 years ago

pdpinch commented 9 years ago

If you call the mobile api with a URL like /api/mobile/v0.5/users/clee/course_status_info/ccx-block-v1:edX+CCX100-RERUN+RERUN+ccx@14+type@sequential+block@simulations

It will cause a stack trace like this gist.

In this example, and is enrolled in the ccx course by clee

In the mobile enrollment api we check for any of the following:


  1. beta user 
2. staff user 
3. mobile enabled course clee qualifies for 2 so I was able to view it on my device
pdpinch commented 9 years ago

I updated the issue description with some more detail about the circumstances that caused the stack trace.

cewing commented 9 years ago

It would appear from the gist that the problem is that the API view is making the assumption that all the course_ids it gets will be CourseLocators, which means that it cannot handle locators for our custom keys. The code here needs to be made robust to creating keys of any kind from the incoming course_id string.

BenjiLee commented 9 years ago

@cewing Is there an example of a more robust custom key locator? Seems like we may be making this assumption in multiple places.

cpennington commented 9 years ago

CourseKey.from_string is the correct way to parse an OpaqueKey. Is it possible that the ccx key package isn't installed?

cewing commented 9 years ago

that would certainly explain it. Sorry for the red herring. Since it requires the entry point registered in ccx-keys in order to pick up the ccx-block-v1 key type that package being missing would potentially explain the issue. Is the API installed in a different virtualenv than the rest of the software? Did we miss adding the ccx-keys package to a requirements.txt file?

BenjiLee commented 9 years ago

The mobile API is an django app in the edx-platform repository so it should have access to that library.

cpennington commented 9 years ago

The other thing that @ormsbee pointed out is that ccx-block-v1: is a UsageKey, not a CourseKey. So, that's the immediate cause of the parse failure.

@BenjiLee tried to use a course-key (ccx-v1) instead, but ran into a different error. @BenjiLee, can you post that stack trace? Separately, there's the question of what client is sending a UsageKey to that url, which expects a CourseKey.

BenjiLee commented 9 years ago

@cpennington ccx-v1 did work (I forgot to put "courses" in courses.edx.stage).

BenjiLee commented 9 years ago

I believe the client passes the the course_id that we get from the enrollment api https://github.com/edx/edx-platform/blob/master/common/djangoapps/enrollment/serializers.py#L51

Update: Looks like the enrollment API is returning the correct course_id. Looking at the client code now.

BenjiLee commented 9 years ago

Figured it out. The android app was passing in a module_id for a course_id and vice versa. Everything seemed to have worked on the surface but last accessed feature was actually broken. Thanks all.