Badges mobile api

[Kelketek]

Description: Provides read-only API views for Badges.
Discussion/Spec: https://docs.google.com/document/d/1ob-leRHV97agPGw5ys9uASXrcsU8qf51bzYlKH3mBEM/edit?usp=sharing
Dependencies: https://github.com/edx/edx-platform/pull/10498 https://github.com/edx/edx-platform/pull/10612
Partner information: 3rd party-hosted open edX instance, for an edX solutions client.

NOTES: The API endpoint design ended up being slightly different in implementation. The URLs have changed to fit within the existing mobile API structure and the functionality was tweaked slightly. The most notable change is that because we're allowing a badge class to be awarded multiple times, searching for a user having a particular badge class yields a list of badge assertions rather than just one. The handling of permissions is not quite as the spec says-- rather, we re-use the existing mobile API permission handling for consistency via its existing decorator functions.

To test:

1. Log in as a user which has some badges (For instance, honor@example.com), and visit the API endpoints:

http://pr10556.sandbox.opencraft.com/api/badges/v1/assertions/user/honor/
http://pr10556.sandbox.opencraft.com/api/badges/v1/assertions/user/honor/course-v1:OpenCraftX+1+2015/
http://pr10556.sandbox.opencraft.com/api/badges/v1/assertions/user/honor/*/?slug=bloo&issuing_component=colorize
http://pr10556.sandbox.opencraft.com/api/badges/v1/assertions/user/honor/?issuing_component=edx__course&slug=robin
http://pr10556.sandbox.opencraft.com/api/badges/v1/classes/edx__course/batman/
http://pr10556.sandbox.opencraft.com/api/badges/v1/classes/colorize/bloo/course-v1:OpenCraftX+1+2015/

[openedx-webhooks]

Thanks for the pull request, @Kelketek! It looks like you're a member of a company that does contract work for edX. If you're doing this work as part of a paid contract with edX, you should talk to edX about who will review this pull request. If this work is not part of a paid contract with edX, then you should ensure that there is an OSPR issue to track this work in JIRA, so that we don't lose track of your pull request.

To automatically create an OSPR issue for this pull request, just visit this link: https://openedx-webhooks.herokuapp.com/github/process_pr?repo=edx%2Fedx-platform&number=10732

[Kelketek]

@smarnach This is ready for your review. Only the last commit is relevant-- the rest are dependencies.

[smarnach]

How about using string.islower() instead?

[smarnach]

@Kelketek Looks good so far. I've added a bunch of minor comments, and the tests aren't passing yet.

[Kelketek]

@smarnach Is there anything else you need to give this a +1?

[smarnach]

Is this change supposed to be included in the final merge?

[Kelketek]

No, it will be removed as part of the final rebase.

[smarnach]

@Kelketek Looks all good now; 👍 after replying to the question about the django-wiki requirement change.

My original review comments have all disappeared because I commented on the commit and the PR was rebased. I'd prefer if we don't rebase during reviews (and merge instead). This would make it much easier to see what changed since the last review.

[Kelketek]

@nedbat This is ready for your review.

[Kelketek]

@nasthagiri This is ready for your review, as well.

[nasthagiri]

@wajeeha-khalid FYI.

[nasthagiri]

@Kelketek Why is this API included as part of the mobile_api django project? The mobile_api project and its APIs are deprecated. Per the design spec, isn't the Badges API a generic API that is available for wider use?

[Kelketek]

@nasthagiri When I set out to start developing the API, I noticed that several API endpoints existed there. I felt it would be silly to fragment how things were, but apparently not. Would you prefer the API endpoints be migrated into the user_api app, or just placed as a separate set of endpoints within the badging app?

[nasthagiri]

@Kelketek Since you already have a badges djangoapp, it is better for it to live in lms/djangoapps/badges.

[nasthagiri]

This can be moved to lms/urls.py.

[nasthagiri]

What are classes?

[Kelketek]

They refer to Badge Classes, which are the types of badges that may be awarded to users, but not the badge assertions themselves.

[nasthagiri]

@Kelketek Thanks for this work. Overall, it looks good. My main requirements are:

1. To move the API into the badges djangoapp rather than within the deprecated mobile_api project
2. Simplify/reduce the multiple URLs that have been created.

[Kelketek]

@nasthagiri Thank you. Although it isn't complete, could you please look at https://github.com/edx/edx-platform/pull/10861 's last commit to see how it modifies the user API and see if that approach handles your needs?

I should have this ready relatively shortly since the notes aren't too difficult overall. However, I am a bit concerned about trying to prune the URLs, as the reason they grew was because I realized I made a bad assumption when trying to create the ones in the spec about how badges would need to be specified and made, and had to go back to the other Pull requests and modify them before returning to this one. See my comment above about the additional URLs.

[Kelketek]

@nasthagiri I have pushed some updates to the PR. I don't think I'm going to be able to prune any URLs, but I did simplify them, and will be updating the sandbox and its example links to demonstrate.

[Kelketek]

@nasthagiri Updated, and I managed to prune down the URLs a bit after all by using query params in a few places.

[nasthagiri]

Can we remove the interdependency between the badges app and the mobile_api one? Since this app is generic enough and shouldn't be tied to mobile.
If there are items in the mobile_api project that are generally useful, feel free to refactor it and move it to a more common place that both apps can then use.

[Kelketek]

@nasthagiri This is ready for another look.

[nasthagiri]

Please update docstring with new URL format.

[nasthagiri]

Particularly, course_id should be moved into the "Params" section below.

[nasthagiri]

This looks good to me. Thanks for updating the PR. I have an outstanding issue with the user field.

[nasthagiri]

Nit: The convention in python is to use __ for dummy, unused variables.

[smarnach]

This isn't mentioned in the list of unused variable patterns, while dummy is.

[nasthagiri]

The first character listed in that link is _. So that's what I'm referring to. And since we want to distinguish between the _ that refers to ugettext, I usually use double underscore instead.

But yeah, dummy also works as an old-style convention.

[nasthagiri]

Great job! Thanks.

👍 Assuming quality, coverage, and jenkins are happy.
Please remember to squash.