MA-199 Course Authorization framework in mobile API - #6398
Conversation
6835837 to
41ba3b2
Compare
There was a problem hiding this comment.
I wonder if there's a way we can make the names more obvious. Since it's not just enrollment, it's also beta users though IIRC I had the same problem and didn't come up with anything good.
There was a problem hiding this comment.
The can_load_mobile method checks enrollment in addition to the other checks.
The can_load_mobile_not_enrolled method does not check enrollment, but checks release date, beta user, and eventually cohorted content (once supported).
How about can_load_mobile and can_load_mobile_no_enrollment_check?
There was a problem hiding this comment.
Renamed load_mobile_not_enrolled -> load_mobile_no_enrollment_check in latest commit.
|
That's all from me. |
There was a problem hiding this comment.
Oops! Thanks. I'm surprised pylint doesn't raise this issue.
|
@BenjiLee @aleffert I believe I have addressed all your comments. Please see the latest commit edx@a221727. |
There was a problem hiding this comment.
More unused imports:
from rest_framework import permissions
from rest_framework.authentication import OAuth2Authentication, SessionAuthentication
from opaque_keys.edx.keys import CourseKey
from xmodule.modulestore.django import modulestore
|
👍 |
|
👍 |
bf7f076 to
63412f8
Compare
63412f8 to
5349b55
Compare
MA-199 Course Authorization framework in mobile API
3 participants
Please review. @aleffert @BenjiLee