From f4dc90b34899476c1d93abf4f30090d18a689140 Mon Sep 17 00:00:00 2001 From: Akiva Leffert Date: Mon, 17 Nov 2014 16:24:48 -0500 Subject: [PATCH] Add an endpoint for syncing a user's course status metadata JIRA: MA-75 --- lms/djangoapps/courseware/views.py | 20 ++ lms/djangoapps/mobile_api/errors.py | 15 ++ lms/djangoapps/mobile_api/users/tests.py | 181 +++++++++++++++++- lms/djangoapps/mobile_api/users/urls.py | 12 +- lms/djangoapps/mobile_api/users/views.py | 173 ++++++++++++++++- .../mobile_api/video_outlines/serializers.py | 1 + .../mobile_api/video_outlines/tests.py | 1 + .../mobile_api/video_outlines/views.py | 8 +- 8 files changed, 399 insertions(+), 12 deletions(-) create mode 100644 lms/djangoapps/mobile_api/errors.py diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 54936d972b..48ceced78d 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -220,6 +220,26 @@ def save_child_position(seq_module, child_name): seq_module.save() +def save_positions_recursively_up(user, request, field_data_cache, xmodule): + """ + Recurses up the course tree starting from a leaf + Saving the position property based on the previous node as it goes + """ + current_module = xmodule + + while current_module: + parent_location = modulestore().get_parent_location(current_module.location) + parent = None + if parent_location: + parent_descriptor = modulestore().get_item(parent_location) + parent = get_module_for_descriptor(user, request, parent_descriptor, field_data_cache, current_module.location.course_key) + + if parent and hasattr(parent, 'position'): + save_child_position(parent, current_module.location.name) + + current_module = parent + + def chat_settings(course, user): """ Returns a dict containing the settings required to connect to a diff --git a/lms/djangoapps/mobile_api/errors.py b/lms/djangoapps/mobile_api/errors.py new file mode 100644 index 0000000000..a76e84076c --- /dev/null +++ b/lms/djangoapps/mobile_api/errors.py @@ -0,0 +1,15 @@ +""" +List of errors that can be returned by the mobile api +""" + + +def format_error(error_code, message): + """ + Converts an error_code and message into a response body + """ + return {"errors": [{"code": error_code, "message": message}]} + +ERROR_INVALID_COURSE_ID = format_error("invalid-course-id", "Could not find course for course_id") +ERROR_INVALID_MODIFICATION_DATE = format_error("invalid-modification-date", "Could not parse modification_date") +ERROR_INVALID_MODULE_ID = format_error("invalid-module-id", "Could not find module for module_id") +ERROR_INVALID_USER_ID = format_error("invalid-user-id", "Could not find user for user_id") diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 714422840a..18cf519837 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -2,17 +2,20 @@ Tests for users API """ +import datetime import ddt +import json + from rest_framework.test import APITestCase -from unittest import skip -from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.django import modulestore from courseware.tests.factories import UserFactory from django.core.urlresolvers import reverse +from django.utils import timezone from mobile_api.users.serializers import CourseEnrollmentSerializer +from mobile_api import errors from student.models import CourseEnrollment -from student import auth from mobile_api.tests import ROLE_CASES @@ -135,3 +138,175 @@ class TestUserApi(ModuleStoreTestCase, APITestCase): serialized = CourseEnrollmentSerializer(CourseEnrollment.enrollments_for_user(self.user)[0]).data # pylint: disable=E1101 self.assertEqual(serialized['course']['number'], self.course.display_coursenumber) self.assertEqual(serialized['course']['org'], self.course.display_organization) + +# Tests for user-course-status + + def _course_status_url(self): + """ + Convenience to fetch the url for our user and course + """ + return reverse('user-course-status', kwargs={'username': self.username, 'course_id': unicode(self.course.id)}) + + def _setup_course_skeleton(self): + """ + Creates a basic course structure for our course + """ + section = ItemFactory.create( + parent_location=self.course.location, + ) + sub_section = ItemFactory.create( + parent_location=section.location, + ) + unit = ItemFactory.create( + parent_location=sub_section.location, + ) + other_unit = ItemFactory.create( + parent_location=sub_section.location, + ) + + return section, sub_section, unit, other_unit + + def test_course_status_course_not_found(self): + self.client.login(username=self.username, password=self.password) + url = reverse('user-course-status', kwargs={'username': self.username, 'course_id': 'a/b/c'}) + response = self.client.get(url) + json_data = json.loads(response.content) + self.assertEqual(response.status_code, 404) + self.assertEqual(json_data, errors.ERROR_INVALID_COURSE_ID) + + def test_course_status_wrong_user(self): + url = reverse('user-course-status', kwargs={'username': 'other_user', 'course_id': unicode(self.course.id)}) + self.client.login(username=self.username, password=self.password) + response = self.client.get(url) + self.assertEqual(response.status_code, 403) + + def test_course_status_no_auth(self): + url = self._course_status_url() + response = self.client.get(url) + self.assertEqual(response.status_code, 401) + + def test_default_value(self): + (__, __, unit, __) = self._setup_course_skeleton() + self.client.login(username=self.username, password=self.password) + + url = self._course_status_url() + result = self.client.get(url) + json_data = json.loads(result.content) + + self.assertEqual(result.status_code, 200) + self.assertEqual(json_data["last_visited_module_id"], unicode(unit.location)) + + def test_course_update_no_args(self): + self.client.login(username=self.username, password=self.password) + + url = self._course_status_url() + result = self.client.patch(url) # pylint: disable=no-member + self.assertEqual(result.status_code, 200) + + def test_course_update(self): + (__, __, __, other_unit) = self._setup_course_skeleton() + self.client.login(username=self.username, password=self.password) + + url = self._course_status_url() + result = self.client.patch( # pylint: disable=no-member + url, + {"last_visited_module_id": unicode(other_unit.location)} + ) + self.assertEqual(result.status_code, 200) + result = self.client.get(url) + json_data = json.loads(result.content) + self.assertEqual(result.status_code, 200) + self.assertEqual(json_data["last_visited_module_id"], unicode(other_unit.location)) + + def test_course_update_bad_module(self): + self.client.login(username=self.username, password=self.password) + + url = self._course_status_url() + result = self.client.patch( # pylint: disable=no-member + url, + {"last_visited_module_id": "abc"}, + ) + json_data = json.loads(result.content) + self.assertEqual(result.status_code, 400) + self.assertEqual(json_data, errors.ERROR_INVALID_MODULE_ID) + + def test_course_update_no_timezone(self): + (__, __, __, other_unit) = self._setup_course_skeleton() + self.client.login(username=self.username, password=self.password) + url = self._course_status_url() + past_date = datetime.datetime.now() + result = self.client.patch( # pylint: disable=no-member + url, + { + "last_visited_module_id": unicode(other_unit.location), + "modification_date": past_date.isoformat() # pylint: disable=maybe-no-member + }, + ) + + json_data = json.loads(result.content) + self.assertEqual(result.status_code, 400) + self.assertEqual(json_data, errors.ERROR_INVALID_MODIFICATION_DATE) + + def _test_course_update_date_sync(self, date, initial_unit, update_unit, expected_unit): + """ + Helper for test cases that use a modification to decide whether + to update the course status + """ + self.client.login(username=self.username, password=self.password) + url = self._course_status_url() + # save something so we have an initial date + self.client.patch( # pylint: disable=no-member + url, + {"last_visited_module_id": unicode(initial_unit.location)} + ) + + # now actually update it + result = self.client.patch( # pylint: disable=no-member + url, + { + "last_visited_module_id": unicode(update_unit.location), + "modification_date": date.isoformat() + }, + ) + + json_data = json.loads(result.content) + self.assertEqual(result.status_code, 200) + self.assertEqual(json_data["last_visited_module_id"], unicode(expected_unit.location)) + + def test_course_update_old_date(self): + (__, __, unit, other_unit) = self._setup_course_skeleton() + date = timezone.now() + datetime.timedelta(days=-100) + self._test_course_update_date_sync(date, unit, other_unit, unit) + + def test_course_update_new_date(self): + (__, __, unit, other_unit) = self._setup_course_skeleton() + + date = timezone.now() + datetime.timedelta(days=100) + self._test_course_update_date_sync(date, unit, other_unit, other_unit) + + def test_course_update_no_initial_date(self): + (__, __, _, other_unit) = self._setup_course_skeleton() + self.client.login(username=self.username, password=self.password) + url = self._course_status_url() + result = self.client.patch( # pylint: disable=no-member + url, + { + "last_visited_module_id": unicode(other_unit.location), + "modification_date": timezone.now().isoformat() + } + ) + json_data = json.loads(result.content) + self.assertEqual(result.status_code, 200) + self.assertEqual(json_data["last_visited_module_id"], unicode(other_unit.location)) + + def test_course_update_invalid_date(self): + self.client.login(username=self.username, password=self.password) + + url = self._course_status_url() + result = self.client.patch( # pylint: disable=no-member + url, + {"modification_date": "abc"} + ) + json_data = json.loads(result.content) + self.assertEqual(result.status_code, 400) + self.assertEqual(json_data, errors.ERROR_INVALID_MODIFICATION_DATE) diff --git a/lms/djangoapps/mobile_api/users/urls.py b/lms/djangoapps/mobile_api/users/urls.py index f9ba58f2b5..093f1f65ac 100644 --- a/lms/djangoapps/mobile_api/users/urls.py +++ b/lms/djangoapps/mobile_api/users/urls.py @@ -2,15 +2,21 @@ URLs for user API """ from django.conf.urls import patterns, url +from django.conf import settings -from .views import UserDetail, UserCourseEnrollmentsList +from .views import UserDetail, UserCourseEnrollmentsList, UserCourseStatus + +USERNAME_PATTERN = r'(?P[\w.+-]+)' urlpatterns = patterns( 'mobile_api.users.views', - url(r'^(?P[\w.+-]+)$', UserDetail.as_view(), name='user-detail'), + url('^' + USERNAME_PATTERN + '$', UserDetail.as_view(), name='user-detail'), url( - r'^(?P[\w.+-]+)/course_enrollments/$', + '^' + USERNAME_PATTERN + '/course_enrollments/$', UserCourseEnrollmentsList.as_view(), name='courseenrollment-detail' ), + url('^{}/course_status_info/{}'.format(USERNAME_PATTERN, settings.COURSE_ID_PATTERN), + UserCourseStatus.as_view(), + name='user-course-status') ) diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index 23223dc9fb..e18a62c4c3 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -1,18 +1,36 @@ """ Views for user API """ -from django.shortcuts import redirect -from rest_framework import generics, permissions +from courseware.model_data import FieldDataCache +from courseware.module_render import get_module_for_descriptor + +from django.shortcuts import redirect +from django.utils import dateparse + +from rest_framework import generics, permissions, views from rest_framework.authentication import OAuth2Authentication, SessionAuthentication from rest_framework.decorators import api_view, authentication_classes, permission_classes + from rest_framework.permissions import IsAuthenticated +from rest_framework.response import Response + +from courseware.views import get_current_child, save_positions_recursively_up + +from opaque_keys.edx.keys import CourseKey, UsageKey +from opaque_keys import InvalidKeyError from student.models import CourseEnrollment, User from mobile_api.utils import mobile_available_when_enrolled +from xblock.fields import Scope +from xblock.runtime import KeyValueStore +from xmodule.modulestore.django import modulestore + + from .serializers import CourseEnrollmentSerializer, UserSerializer +from mobile_api import errors class IsUser(permissions.BasePermission): @@ -62,6 +80,157 @@ class UserDetail(generics.RetrieveAPIView): lookup_field = 'username' +@authentication_classes((OAuth2Authentication, SessionAuthentication)) +@permission_classes((IsAuthenticated,)) +class UserCourseStatus(views.APIView): + """ + Endpoints for getting and setting meta data + about a user's status within a given course. + """ + + http_method_names = ["get", "patch"] + + def _last_visited_module_id(self, request, course): + """ + Returns the id of the last module visited by the current user in the given course. + If there is no such visit returns the default (the first item deep enough down the course tree) + """ + field_data_cache = FieldDataCache.cache_for_descriptor_descendents( + course.id, request.user, course, depth=2) + + course_module = get_module_for_descriptor(request.user, request, course, field_data_cache, course.id) + current = course_module + + child = current + while child: + child = get_current_child(current) + if child: + current = child + + return current + + def _process_arguments(self, request, username, course_id, course_handler): + """ + Checks and processes the arguments to our endpoint + then passes the processed and verified arguments on to something that + does the work specific to the individual case + """ + if username != request.user.username: + return Response(errors.ERROR_INVALID_USER_ID, status=403) + + course = None + try: + course_key = CourseKey.from_string(course_id) + course = modulestore().get_course(course_key, depth=None) + except InvalidKeyError: + pass + + if not course: + return Response(errors.ERROR_INVALID_COURSE_ID, status=404) # pylint: disable=lost-exception + + return course_handler(course) + + def get_course_info(self, request, course): + """ + Returns the course status + """ + current_module = self._last_visited_module_id(request, course) + return Response({"last_visited_module_id": unicode(current_module.location)}) + + def get(self, request, username, course_id): + """ + **Use Case** + + Get meta data about user's status within a specific course + + **Example request**: + + GET /api/mobile/v0.5/users/{username}/course_status_info/{course_id} + + **Response Values** + + * last_visited_module_id: The id of the last module visited by the user in the given course + + """ + + return self._process_arguments(request, username, course_id, lambda course: self.get_course_info(request, course)) + + def _update_last_visited_module_id(self, request, course, module_key, modification_date): + """ + Saves the module id if the found modification_date is less recent than the passed modification date + """ + field_data_cache = FieldDataCache.cache_for_descriptor_descendents( + course.id, request.user, course, depth=2) + module_descriptor = modulestore().get_item(module_key) + module = get_module_for_descriptor(request.user, request, module_descriptor, field_data_cache, course.id) + + if modification_date: + key = KeyValueStore.Key( + scope=Scope.user_state, + user_id=request.user.id, + block_scope_id=course.location, + field_name=None + ) + student_module = field_data_cache.find(key) + if student_module: + original_store_date = student_module.modified + if modification_date < original_store_date: + # old modification date so skip update + return self.get_course_info(request, course) + + if module: + save_positions_recursively_up(request.user, request, field_data_cache, module) + return self.get_course_info(request, course) + else: + return Response(errors.ERROR_INVALID_MODULE_ID, status=400) + + def patch(self, request, username, course_id): + """ + **Use Case** + + Update meta data about user's status within a specific course + + **Example request**: + + PATCH /api/mobile/v0.5/users/{username}/course_status_info/{course_id} + body: + last_visited_module_id={module_id} + modification_date={date} + + modification_date is optional. If it is present, the update will only take effect + if modification_date is later than the modification_date saved on the server + + **Response Values** + + The same as doing a GET on this path + + """ + def handle_course(course): + """ + Updates the course_status once the arguments are checked + """ + module_id = request.DATA.get("last_visited_module_id") + modification_date_string = request.DATA.get("modification_date") + modification_date = None + if modification_date_string: + modification_date = dateparse.parse_datetime(modification_date_string) + if not modification_date or not modification_date.tzinfo: + return Response(errors.ERROR_INVALID_MODIFICATION_DATE, status=400) + + if module_id: + try: + module_key = UsageKey.from_string(module_id) + except InvalidKeyError: + return Response(errors.ERROR_INVALID_MODULE_ID, status=400) + + return self._update_last_visited_module_id(request, course, module_key, modification_date) + else: + # The arguments are optional, so if there's no argument just succeed + return self.get_course_info(request, course) + + return self._process_arguments(request, username, course_id, handle_course) + + class UserCourseEnrollmentsList(generics.ListAPIView): """ **Use Case** diff --git a/lms/djangoapps/mobile_api/video_outlines/serializers.py b/lms/djangoapps/mobile_api/video_outlines/serializers.py index 029e72a7e7..30a75a46be 100644 --- a/lms/djangoapps/mobile_api/video_outlines/serializers.py +++ b/lms/djangoapps/mobile_api/video_outlines/serializers.py @@ -43,6 +43,7 @@ class BlockOutline(object): # to be consistent with other edx-platform clients, return the defaulted display name 'name': block.display_name_with_default, 'category': block.category, + 'id': unicode(block.location) }) return reversed(block_path) diff --git a/lms/djangoapps/mobile_api/video_outlines/tests.py b/lms/djangoapps/mobile_api/video_outlines/tests.py index e76473a8f9..15ab7b4ab8 100644 --- a/lms/djangoapps/mobile_api/video_outlines/tests.py +++ b/lms/djangoapps/mobile_api/video_outlines/tests.py @@ -188,6 +188,7 @@ class TestVideoOutline(ModuleStoreTestCase, APITestCase): self.assertEqual(course_outline[1]['summary']['video_url'], self.html5_video_url) self.assertEqual(course_outline[1]['summary']['size'], 0) self.assertEqual(course_outline[1]['path'][2]['name'], self.other_unit.display_name) + self.assertEqual(course_outline[1]['path'][2]['id'], unicode(self.other_unit.location)) self.assertEqual(course_outline[2]['summary']['video_url'], self.html5_video_url) self.assertEqual(course_outline[2]['summary']['size'], 0) diff --git a/lms/djangoapps/mobile_api/video_outlines/views.py b/lms/djangoapps/mobile_api/video_outlines/views.py index bc8483a3d3..3ed9ea4994 100644 --- a/lms/djangoapps/mobile_api/video_outlines/views.py +++ b/lms/djangoapps/mobile_api/video_outlines/views.py @@ -41,15 +41,15 @@ class VideoSummaryList(generics.ListAPIView): An array of videos in the course. For each video: * section_url: The URL to the first page of the section that - contains the video in the Learning Managent System. + contains the video in the Learning Management System. - * path: An array containing category and name values specifying the - complete path the the video in the courseware hierarcy. The + * path: An array containing category, name, and id values specifying the + complete path the the video in the courseware hierarchy. The following categories values are included: "chapter", "sequential", and "vertical". The name value is the display name for that object. * unit_url: The URL to the unit contains the video in the Learning - Managent System. + Management System. * named_path: An array consisting of the display names of the courseware objects in the path to the video.