From fae695884b42582bc45cb02f5e53dd997f7f93d0 Mon Sep 17 00:00:00 2001 From: Michael Cornwell Date: Thu, 23 Nov 2017 09:47:46 -0600 Subject: [PATCH] Added urls and built endpoint. --- lms/djangoapps/completion/api/__init__.py | 0 lms/djangoapps/completion/api/urls.py | 8 + lms/djangoapps/completion/api/v1/__init__.py | 0 .../completion/api/v1/tests/__init__.py | 0 .../completion/api/v1/tests/test_views.py | 216 ++++++++++++++++++ lms/djangoapps/completion/api/v1/urls.py | 10 + lms/djangoapps/completion/api/v1/views.py | 136 +++++++++++ lms/djangoapps/completion/models.py | 47 +++- .../completion/tests/test_models.py | 49 +++- lms/urls.py | 3 + 10 files changed, 462 insertions(+), 7 deletions(-) create mode 100644 lms/djangoapps/completion/api/__init__.py create mode 100644 lms/djangoapps/completion/api/urls.py create mode 100644 lms/djangoapps/completion/api/v1/__init__.py create mode 100644 lms/djangoapps/completion/api/v1/tests/__init__.py create mode 100644 lms/djangoapps/completion/api/v1/tests/test_views.py create mode 100644 lms/djangoapps/completion/api/v1/urls.py create mode 100644 lms/djangoapps/completion/api/v1/views.py diff --git a/lms/djangoapps/completion/api/__init__.py b/lms/djangoapps/completion/api/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/completion/api/urls.py b/lms/djangoapps/completion/api/urls.py new file mode 100644 index 0000000000..276d4f949a --- /dev/null +++ b/lms/djangoapps/completion/api/urls.py @@ -0,0 +1,8 @@ +""" +Api URLs. +""" +from django.conf.urls import include, url + +urlpatterns = [ + url(r'^v1/', include('lms.djangoapps.completion.api.v1.urls', namespace='v1')), +] diff --git a/lms/djangoapps/completion/api/v1/__init__.py b/lms/djangoapps/completion/api/v1/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/completion/api/v1/tests/__init__.py b/lms/djangoapps/completion/api/v1/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/completion/api/v1/tests/test_views.py b/lms/djangoapps/completion/api/v1/tests/test_views.py new file mode 100644 index 0000000000..6b4fb08f47 --- /dev/null +++ b/lms/djangoapps/completion/api/v1/tests/test_views.py @@ -0,0 +1,216 @@ +# -*- coding: utf-8 -*- +""" +Test models, managers, and validators. +""" + +import ddt +from django.core.urlresolvers import reverse +from rest_framework.test import APIClient, force_authenticate + +from completion import waffle +from student.tests.factories import UserFactory, CourseEnrollmentFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from openedx.core.djangoapps.content.course_structures.tasks import update_course_structure + + +@ddt.ddt +class CompletionBatchTestCase(ModuleStoreTestCase): + """ + Test that BlockCompletion.objects.submit_batch_completion has the desired + semantics. + """ + ENROLLED_USERNAME = 'test_user' + UNENROLLED_USERNAME = 'unenrolled_user' + COURSE_KEY = 'TestX/101/Test' + BLOCK_KEY = 'i4x://TestX/101/problem/Test_Problem' + + def setUp(self): + """ + Create the test data. + """ + super(CompletionBatchTestCase, self).setUp() + self.url = reverse('completion_api:v1:completion-batch') + + # Enable the waffle flag for all tests + _overrider = waffle.waffle().override(waffle.ENABLE_COMPLETION_TRACKING, True) + _overrider.__enter__() + self.addCleanup(_overrider.__exit__, None, None, None) + + # Create course + self.course = CourseFactory.create(org='TestX', number='101', display_name='Test') + self.problem = ItemFactory.create( + parent=self.course, + category="problem", + display_name="Test Problem", + ) + update_course_structure(unicode(self.course.id)) + + # Create users + self.staff_user = UserFactory(is_staff=True) + self.enrolled_user = UserFactory(username=self.ENROLLED_USERNAME) + self.unenrolled_user = UserFactory(username=self.UNENROLLED_USERNAME) + + # Enrol one user in the course + CourseEnrollmentFactory.create(user=self.enrolled_user, course_id=self.course.id) + + # Login the enrolled user by for all tests + self.client = APIClient() + self.client.force_authenticate(user=self.enrolled_user) + + def test_enable_completion_tracking(self): + """ + Test response when the waffle switch is disabled (default). + """ + with waffle.waffle().override(waffle.ENABLE_COMPLETION_TRACKING, False): + response = self.client.post(self.url, {'username': self.ENROLLED_USERNAME}, format='json') + self.assertEqual(response.data, { + "detail": + "BlockCompletion.objects.submit_batch_completion should not be called when the feature is disabled." + }) + self.assertEqual(response.status_code, 400) + + @ddt.data( + # Valid submission + ( + { + 'username': ENROLLED_USERNAME, + 'course_key': COURSE_KEY, + 'blocks': { + BLOCK_KEY: 1.0, + } + }, 200, {'detail': 'ok'} + ), + # Blocks list can be empty, though it's a no-op + ( + { + 'username': ENROLLED_USERNAME, + 'course_key': COURSE_KEY, + 'blocks': [], + }, 200, {"detail": "ok"} + ), + # Course must be a valid key + ( + { + 'username': ENROLLED_USERNAME, + 'course_key': "not:a:course:key", + 'blocks': { + BLOCK_KEY: 1.0, + } + }, 400, {"detail": "Invalid course key: not:a:course:key"} + ), + # Block not in course + ( + { + 'username': ENROLLED_USERNAME, + 'course_key': COURSE_KEY, + 'blocks': { + 'some:other:block': 1.0, + } + }, 400, {"detail": "Block with key: 'some:other:block' is not in course {}".format(COURSE_KEY)} + ), + # Course key is required + ( + { + 'username': ENROLLED_USERNAME, + 'blocks': { + BLOCK_KEY: 1.0, + } + }, 400, {"detail": "Key 'course_key' not found."} + ), + # Blocks is required + ( + { + 'username': ENROLLED_USERNAME, + 'course_key': COURSE_KEY, + }, 400, {"detail": "Key 'blocks' not found."} + ), + # Ordinary users can only update their own completions + ( + { + 'username': UNENROLLED_USERNAME, + 'course_key': COURSE_KEY, + 'blocks': { + BLOCK_KEY: 1.0, + } + }, 403, {"detail": "You do not have permission to perform this action."} + ), + # Username is required + ( + { + 'course_key': COURSE_KEY, + 'blocks': { + BLOCK_KEY: 1.0, + } + }, 403, {"detail": 'You do not have permission to perform this action.'} + ), + # Course does not exist + ( + { + 'username': ENROLLED_USERNAME, + 'course_key': 'TestX/101/Test2', + 'blocks': { + BLOCK_KEY: 1.0, + } + }, 404, {"detail": "CourseStructure matching query does not exist."} + ), + ) + @ddt.unpack + def test_batch_submit(self, payload, expected_status, expected_data): + """ + Test the batch submission response for student users. + """ + response = self.client.post(self.url, payload, format='json') + self.assertEqual(response.data, expected_data) + self.assertEqual(response.status_code, expected_status) + + @ddt.data( + # Staff can submit completion on behalf of other users + ( + { + 'username': ENROLLED_USERNAME, + 'course_key': COURSE_KEY, + 'blocks': { + BLOCK_KEY: 1.0, + } + }, 200, {'detail': 'ok'} + ), + # User must be enrolled in the course + ( + { + 'username': UNENROLLED_USERNAME, + 'course_key': COURSE_KEY, + 'blocks': { + BLOCK_KEY: 1.0, + } + }, 400, {"detail": "User is not enrolled in course."} + ), + # Username is required + ( + { + 'course_key': COURSE_KEY, + 'blocks': { + BLOCK_KEY: 1.0, + } + }, 400, {"detail": "Key 'username' not found."} + ), + # User must not exist + ( + { + 'username': 'doesntexist', + 'course_key': COURSE_KEY, + 'blocks': { + BLOCK_KEY: 1.0, + } + }, 404, {"detail": 'User matching query does not exist.'} + ), + ) + @ddt.unpack + def test_batch_submit_staff(self, payload, expected_status, expected_data): + """ + Test the batch submission response when logged in as a staff user. + """ + self.client.force_authenticate(user=self.staff_user) + response = self.client.post(self.url, payload, format='json') + self.assertEqual(response.data, expected_data) + self.assertEqual(response.status_code, expected_status) diff --git a/lms/djangoapps/completion/api/v1/urls.py b/lms/djangoapps/completion/api/v1/urls.py new file mode 100644 index 0000000000..18d6a5a0a0 --- /dev/null +++ b/lms/djangoapps/completion/api/v1/urls.py @@ -0,0 +1,10 @@ +""" +API v1 URLs. +""" +from django.conf.urls import include, url + +from . import views + +urlpatterns = [ + url(r'^completion-batch', views.CompletionBatchView.as_view(), name='completion-batch'), +] diff --git a/lms/djangoapps/completion/api/v1/views.py b/lms/djangoapps/completion/api/v1/views.py new file mode 100644 index 0000000000..48096b3109 --- /dev/null +++ b/lms/djangoapps/completion/api/v1/views.py @@ -0,0 +1,136 @@ +""" API v1 views. """ +from django.contrib.auth.models import User +from django.core.exceptions import ValidationError, ObjectDoesNotExist +from django.utils.translation import ugettext as _ +from django.db import DatabaseError + +from rest_framework.views import APIView +from rest_framework.response import Response +from rest_framework import permissions +from rest_framework import status + +from opaque_keys.edx.keys import CourseKey, UsageKey +from opaque_keys import InvalidKeyError + +from lms.djangoapps.completion.models import BlockCompletion +from openedx.core.djangoapps.content.course_structures.models import CourseStructure +from openedx.core.lib.api.permissions import IsStaffOrOwner +from student.models import CourseEnrollment +from completion import waffle + + +class CompletionBatchView(APIView): + """ + Handles API requests to submit batch completions. + """ + permission_classes = (permissions.IsAuthenticated, IsStaffOrOwner,) + REQUIRED_KEYS = ['username', 'course_key', 'blocks'] + + def _validate_and_parse(self, batch_object): + """ + Performs validation on the batch object to make sure it is in the proper format. + + Parameters: + * batch_object: The data provided to a POST. The expected format is the following: + { + "username": "username", + "course_key": "course-key", + "blocks": { + "block_key1": 0.0, + "block_key2": 1.0, + "block_key3": 1.0, + } + } + + + Return Value: + * tuple: (User, CourseKey, List of tuples (UsageKey, completion_float) + + Raises: + + django.core.exceptions.ValidationError: + If any aspect of validation fails a ValidationError is raised. + + ObjectDoesNotExist: + If a database object cannot be found an ObjectDoesNotExist is raised. + """ + if not waffle.waffle().is_enabled(waffle.ENABLE_COMPLETION_TRACKING): + raise ValidationError( + _("BlockCompletion.objects.submit_batch_completion should not be called when the feature is disabled.") + ) + + for key in self.REQUIRED_KEYS: + if key not in batch_object: + raise ValidationError(_("Key '{key}' not found.".format(key=key))) + + username = batch_object['username'] + user = User.objects.get(username=username) + + course_key = batch_object['course_key'] + try: + course_key_obj = CourseKey.from_string(course_key) + except InvalidKeyError: + raise ValidationError(_("Invalid course key: {}").format(course_key)) + course_structure = CourseStructure.objects.get(course_id=course_key_obj) + + if not CourseEnrollment.is_enrolled(user, course_key_obj): + raise ValidationError(_('User is not enrolled in course.')) + + blocks = batch_object['blocks'] + block_objs = [] + for block_key in blocks: + if block_key not in course_structure.structure['blocks'].keys(): + raise ValidationError(_("Block with key: '{key}' is not in course {course}") + .format(key=block_key, course=course_key)) + + block_key_obj = UsageKey.from_string(block_key) + completion = float(blocks[block_key]) + block_objs.append((block_key_obj, completion)) + + return user, course_key_obj, block_objs + + def post(self, request, *args, **kwargs): + """ + Inserts a batch of completions. + + REST Endpoint Format: + { + "username": "username", + "course_key": "course-key", + "blocks": { + "block_key1": 0.0, + "block_key2": 1.0, + "block_key3": 1.0, + } + } + + **Returns** + + A Response object, with an appropriate status code. + + If successful, status code is 200. + { + "detail" : _("ok") + } + + Otherwise, a 400 or 404 may be returned, and the "detail" content will explain the error. + + """ + batch_object = request.data or {} + try: + user, course_key, blocks = self._validate_and_parse(batch_object) + BlockCompletion.objects.submit_batch_completion(user, course_key, blocks) + except (ValidationError, ValueError) as exc: + return Response({ + "detail": exc.message, + }, status=status.HTTP_400_BAD_REQUEST) + except ObjectDoesNotExist as exc: + return Response({ + "detail": exc.message, + }, status=status.HTTP_404_NOT_FOUND) + except DatabaseError as exc: + return Response({ + "detail": exc.message, + }, status=status.HTTP_500_INTERNAL_SERVER_ERROR) + + return Response({"detail": _("ok")}, status=status.HTTP_200_OK) diff --git a/lms/djangoapps/completion/models.py b/lms/djangoapps/completion/models.py index 528e20224e..c4bd5ab6fe 100644 --- a/lms/djangoapps/completion/models.py +++ b/lms/djangoapps/completion/models.py @@ -6,7 +6,7 @@ from __future__ import absolute_import, division, print_function, unicode_litera from django.contrib.auth.models import User from django.core.exceptions import ValidationError -from django.db import models +from django.db import models, transaction, connection from django.utils.translation import ugettext as _ from model_utils.models import TimeStampedModel from opaque_keys.edx.keys import CourseKey @@ -34,7 +34,7 @@ class BlockCompletionManager(models.Manager): """ Custom manager for BlockCompletion model. - Adds submit_completion method. + Adds submit_completion and submit_batch_completion methods. """ def submit_completion(self, user, course_key, block_key, completion): @@ -87,14 +87,14 @@ class BlockCompletionManager(models.Manager): ) if waffle.waffle().is_enabled(waffle.ENABLE_COMPLETION_TRACKING): - obj, isnew = self.get_or_create( + obj, is_new = self.get_or_create( user=user, course_key=course_key, block_type=block_type, block_key=block_key, defaults={'completion': completion}, ) - if not isnew and obj.completion != completion: + if not is_new and obj.completion != completion: obj.completion = completion obj.full_clean() obj.save() @@ -103,7 +103,44 @@ class BlockCompletionManager(models.Manager): raise RuntimeError( "BlockCompletion.objects.submit_completion should not be called when the feature is disabled." ) - return obj, isnew + return obj, is_new + + @transaction.atomic() + def submit_batch_completion(self, user, course_key, blocks): + """ + Performs a batch insertion of completion objects. + + Parameters: + * user (django.contrib.auth.models.User): The user for whom the + completions are being submitted. + * course_key (opaque_keys.edx.keys.CourseKey): The course in + which the submitted blocks are found. + * blocks: A list of tuples of UsageKey to float completion values. + (float in range [0.0, 1.0]): The fractional completion + value of the block (0.0 = incomplete, 1.0 = complete). + + Return Value: + Dict of (BlockCompletion, bool): A dictionary with a + BlockCompletion object key and a value of bool. The boolean value + indicates whether the object was newly created by this call. + + Raises: + + ValueError: + If the wrong type is passed for one of the parameters. + + django.core.exceptions.ValidationError: + If a float is passed that is not between 0.0 and 1.0. + + django.db.DatabaseError: + If there was a problem getting, creating, or updating the + BlockCompletion record in the database. + """ + block_completions = {} + for block, completion in blocks: + (block_completion, is_new) = self.submit_completion(user, course_key, block, completion) + block_completions[block_completion] = is_new + return block_completions class BlockCompletion(TimeStampedModel, models.Model): diff --git a/lms/djangoapps/completion/tests/test_models.py b/lms/djangoapps/completion/tests/test_models.py index 1664732874..e926177a41 100644 --- a/lms/djangoapps/completion/tests/test_models.py +++ b/lms/djangoapps/completion/tests/test_models.py @@ -6,9 +6,9 @@ from __future__ import absolute_import, division, print_function, unicode_litera from django.core.exceptions import ValidationError from django.test import TestCase -from opaque_keys.edx.keys import UsageKey +from opaque_keys.edx.keys import UsageKey, CourseKey -from student.tests.factories import UserFactory +from student.tests.factories import UserFactory, CourseEnrollmentFactory from .. import models from .. import waffle @@ -142,3 +142,48 @@ class CompletionDisabledTestCase(CompletionSetUpMixin, TestCase): completion=0.9, ) self.assertEqual(models.BlockCompletion.objects.count(), 1) + + +class SubmitBatchCompletionTestCase(TestCase): + """ + Test that BlockCompletion.objects.submit_batch_completion has the desired + semantics. + """ + + def setUp(self): + super(SubmitBatchCompletionTestCase, self).setUp() + _overrider = waffle.waffle().override(waffle.ENABLE_COMPLETION_TRACKING, True) + _overrider.__enter__() + self.addCleanup(_overrider.__exit__, None, None, None) + + self.block_key = UsageKey.from_string('block-v1:edx+test+run+type@video+block@doggos') + self.course_key_obj = CourseKey.from_string('course-v1:edx+test+run') + self.user = UserFactory() + CourseEnrollmentFactory.create(user=self.user, course_id=unicode(self.course_key_obj)) + + def test_submit_batch_completion(self): + blocks = [(self.block_key, 1.0)] + models.BlockCompletion.objects.submit_batch_completion(self.user, self.course_key_obj, blocks) + self.assertEqual(models.BlockCompletion.objects.count(), 1) + self.assertEqual(models.BlockCompletion.objects.last().completion, 1.0) + + def test_submit_batch_completion_without_waffle(self): + with waffle.waffle().override(waffle.ENABLE_COMPLETION_TRACKING, False): + with self.assertRaises(RuntimeError): + blocks = [(self.block_key, 1.0)] + models.BlockCompletion.objects.submit_batch_completion(self.user, self.course_key_obj, blocks) + + def test_submit_batch_completion_with_same_block_new_completion_value(self): + blocks = [(self.block_key, 0.0)] + self.assertEqual(models.BlockCompletion.objects.count(), 0) + models.BlockCompletion.objects.submit_batch_completion(self.user, self.course_key_obj, blocks) + self.assertEqual(models.BlockCompletion.objects.count(), 1) + model = models.BlockCompletion.objects.first() + self.assertEqual(model.completion, 0.0) + blocks = [ + (UsageKey.from_string('block-v1:edx+test+run+type@video+block@doggos'), 1.0), + ] + models.BlockCompletion.objects.submit_batch_completion(self.user, self.course_key_obj, blocks) + self.assertEqual(models.BlockCompletion.objects.count(), 1) + model = models.BlockCompletion.objects.first() + self.assertEqual(model.completion, 1.0) diff --git a/lms/urls.py b/lms/urls.py index 041d819d4a..7e58a2b418 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -100,6 +100,9 @@ urlpatterns = [ # Course API url(r'^api/courses/', include('course_api.urls')), + # Completion API + url(r'^api/completion/', include('completion.api.urls', namespace='completion_api')), + # User API endpoints url(r'^api/user/', include('openedx.core.djangoapps.user_api.urls')),