From 65e330e8b5ed272f585d20ed6a1ab9e289162ae5 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Mon, 18 May 2015 12:50:42 -0400 Subject: [PATCH 1/4] Make RequestCache reusable --- .../xmodule/xmodule/modulestore/__init__.py | 41 +----------- .../xmodule/xmodule/modulestore/mongo/base.py | 3 + .../xmodule/modulestore/mongo/draft.py | 3 +- .../xmodule/modulestore/split_mongo/split.py | 3 + openedx/core/lib/cache_utils.py | 49 ++++++++++++++ openedx/core/lib/tests/test_cache_utils.py | 64 +++++++++++++++++++ 6 files changed, 122 insertions(+), 41 deletions(-) create mode 100644 openedx/core/lib/cache_utils.py create mode 100644 openedx/core/lib/tests/test_cache_utils.py diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 6ed562ce34..e35cabc149 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -7,13 +7,11 @@ import logging import re import json import datetime -from uuid import uuid4 from pytz import UTC -from collections import namedtuple, defaultdict +from collections import defaultdict import collections from contextlib import contextmanager -import functools import threading from operator import itemgetter from sortedcontainers import SortedListWithKey @@ -27,8 +25,6 @@ from xmodule.errortracker import make_error_tracker from xmodule.assetstore import AssetMetadata from opaque_keys.edx.keys import CourseKey, UsageKey, AssetKey from opaque_keys.edx.locations import Location # For import backwards compatibility -from opaque_keys import InvalidKeyError -from opaque_keys.edx.locations import SlashSeparatedCourseKey from xblock.runtime import Mixologist from xblock.core import XBlock @@ -1195,41 +1191,6 @@ class ModuleStoreReadBase(BulkOperationsMixin, ModuleStoreRead): raise ValueError(u"Cannot set default store to type {}".format(store_type)) yield - @staticmethod - def memoize_request_cache(func): - """ - Memoize a function call results on the request_cache if there's one. Creates the cache key by - joining the unicode of all the args with &; so, if your arg may use the default &, it may - have false hits - """ - @functools.wraps(func) - def wrapper(self, *args, **kwargs): - """ - Wraps a method to memoize results. - """ - if self.request_cache: - cache_key = '&'.join([hashvalue(arg) for arg in args]) - if cache_key in self.request_cache.data.setdefault(func.__name__, {}): - return self.request_cache.data[func.__name__][cache_key] - - result = func(self, *args, **kwargs) - - self.request_cache.data[func.__name__][cache_key] = result - return result - else: - return func(self, *args, **kwargs) - return wrapper - - -def hashvalue(arg): - """ - If arg is an xblock, use its location. otherwise just turn it into a string - """ - if isinstance(arg, XBlock): - return unicode(arg.location) - else: - return unicode(arg) - # pylint: disable=abstract-method class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite): diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 9f62ee5ef4..dd37d47feb 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -915,6 +915,9 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo services["user"] = self.user_service services["settings"] = SettingsService() + if self.request_cache: + services["request_cache"] = self.request_cache + system = CachingDescriptorSystem( modulestore=self, course_key=course_key, diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index 9c8f327ad9..814058d145 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -10,6 +10,7 @@ import pymongo import logging from opaque_keys.edx.locations import Location +from openedx.core.lib.cache_utils import memoize_in_request_cache from xmodule.exceptions import InvalidVersionError from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.exceptions import ( @@ -634,7 +635,7 @@ class DraftModuleStore(MongoModuleStore): bulk_record.dirty = True self.collection.remove({'_id': {'$in': to_be_deleted}}, safe=self.collection.safe) - @MongoModuleStore.memoize_request_cache + @memoize_in_request_cache('request_cache') def has_changes(self, xblock): """ Check if the subtree rooted at xblock has any drafts and thus may possibly have changes diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 722dbf8dcf..4da2282c8c 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -671,6 +671,9 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): if user_service is not None: self.services["user"] = user_service + if self.request_cache is not None: + self.services["request_cache"] = self.request_cache + self.signal_handler = signal_handler def close_connections(self): diff --git a/openedx/core/lib/cache_utils.py b/openedx/core/lib/cache_utils.py new file mode 100644 index 0000000000..85fc56d638 --- /dev/null +++ b/openedx/core/lib/cache_utils.py @@ -0,0 +1,49 @@ +""" +Utilities related to caching. +""" + +import functools +from xblock.core import XBlock + + +def memoize_in_request_cache(request_cache_attr_name=None): + """ + Memoize a method call's results in the request_cache if there's one. Creates the cache key by + joining the unicode of all the args with &; so, if your arg may use the default &, it may + have false hits. + + Arguments: + request_cache_attr_name - The name of the field or property in this method's containing + class that stores the request_cache. + """ + def _decorator(func): + """Outer method decorator.""" + @functools.wraps(func) + def _wrapper(self, *args, **kwargs): + """ + Wraps a method to memoize results. + """ + request_cache = getattr(self, request_cache_attr_name, None) + if request_cache: + cache_key = '&'.join([hashvalue(arg) for arg in args]) + if cache_key in request_cache.data.setdefault(func.__name__, {}): + return request_cache.data[func.__name__][cache_key] + + result = func(self, *args, **kwargs) + + request_cache.data[func.__name__][cache_key] = result + return result + else: + return func(self, *args, **kwargs) + return _wrapper + return _decorator + + +def hashvalue(arg): + """ + If arg is an xblock, use its location. otherwise just turn it into a string + """ + if isinstance(arg, XBlock): + return unicode(arg.location) + else: + return unicode(arg) diff --git a/openedx/core/lib/tests/test_cache_utils.py b/openedx/core/lib/tests/test_cache_utils.py new file mode 100644 index 0000000000..97e973baa9 --- /dev/null +++ b/openedx/core/lib/tests/test_cache_utils.py @@ -0,0 +1,64 @@ +""" +Tests for cache_utils.py +""" +import ddt +from mock import MagicMock +from unittest import TestCase + +from openedx.core.lib.cache_utils import memoize_in_request_cache + + +@ddt.ddt +class TestMemoizeInRequestCache(TestCase): + """ + Test the memoize_in_request_cache helper function. + """ + class TestCache(object): + """ + A test cache that provides a data dict for caching values, analogous to the request_cache. + """ + def __init__(self): + self.data = {} + + def setUp(self): + super(TestMemoizeInRequestCache, self).setUp() + self.request_cache = self.TestCache() + + @memoize_in_request_cache('request_cache') + def func_to_memoize(self, param): + """ + A test function whose results are to be memoized in the request_cache. + """ + return self.func_to_count(param) + + @memoize_in_request_cache('request_cache') + def multi_param_func_to_memoize(self, param1, param2): + """ + A test function with multiple parameters whose results are to be memoized in the request_cache. + """ + return self.func_to_count(param1, param2) + + def test_memoize_in_request_cache(self): + """ + Tests the memoize_in_request_cache decorator for both single-param and multiple-param functions. + """ + funcs_to_test = ( + (self.func_to_memoize, ['foo'], ['bar']), + (self.multi_param_func_to_memoize, ['foo', 'foo2'], ['foo', 'foo3']), + ) + + for func_to_memoize, arg_list1, arg_list2 in funcs_to_test: + self.func_to_count = MagicMock() # pylint: disable=attribute-defined-outside-init + self.assertFalse(self.func_to_count.called) + + func_to_memoize(*arg_list1) + self.func_to_count.assert_called_once_with(*arg_list1) + + func_to_memoize(*arg_list1) + self.func_to_count.assert_called_once_with(*arg_list1) + + for _ in range(10): + func_to_memoize(*arg_list1) + func_to_memoize(*arg_list2) + + self.assertEquals(self.func_to_count.call_count, 2) From 037ef3be77f2e9620384754b40717bcac3151ee9 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Thu, 4 Jun 2015 22:08:07 -0400 Subject: [PATCH 2/4] Video module support for student_view_json. --- .../xmodule/video_module/video_handlers.py | 3 +- .../xmodule/video_module/video_module.py | 80 +++++++++++- .../courseware/tests/test_video_mongo.py | 117 +++++++++++++++++- 3 files changed, 196 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/video_module/video_handlers.py b/common/lib/xmodule/xmodule/video_module/video_handlers.py index 4ed6e16e18..0438c15f71 100644 --- a/common/lib/xmodule/xmodule/video_module/video_handlers.py +++ b/common/lib/xmodule/xmodule/video_module/video_handlers.py @@ -250,9 +250,10 @@ class VideoStudentViewHandlers(object): response.content_type = Transcript.mime_types['sjson'] elif dispatch == 'download': + lang = request.GET.get('lang', None) try: transcript_content, transcript_filename, transcript_mime_type = self.get_transcript( - transcripts, transcript_format=self.transcript_download_format + transcripts, transcript_format=self.transcript_download_format, lang=lang ) except (NotFoundError, ValueError, KeyError, UnicodeDecodeError): log.debug("Video@download exception") diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py index 574a4ffcf9..cffc8ad13c 100644 --- a/common/lib/xmodule/xmodule/video_module/video_module.py +++ b/common/lib/xmodule/xmodule/video_module/video_module.py @@ -20,12 +20,12 @@ import logging import random from collections import OrderedDict from operator import itemgetter - from lxml import etree from pkg_resources import resource_string from django.conf import settings +from openedx.core.lib.cache_utils import memoize_in_request_cache from xblock.core import XBlock from xblock.fields import ScopeIds from xblock.runtime import KvsFieldData @@ -329,6 +329,7 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers, return self.system.render_template('video.html', context) +@XBlock.wants("request_cache") @XBlock.wants("settings") class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandlers, TabsEditingDescriptor, EmptyDataRawDescriptor): @@ -722,7 +723,7 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler if self.sub: _update_transcript_for_index() - # check to see if there are transcripts in other languages besides default transcript + # Check to see if there are transcripts in other languages besides default transcript if self.transcripts: for language in self.transcripts.keys(): _update_transcript_for_index(language) @@ -734,3 +735,78 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler xblock_body["content_type"] = "Video" return xblock_body + + @property + def request_cache(self): + """ + Returns the request_cache from the runtime. + """ + return self.runtime.service(self, "request_cache") + + @memoize_in_request_cache('request_cache') + def get_cached_val_data_for_course(self, video_profile_names, course_id): + """ + Returns the VAL data for the requested video profiles for the given course. + """ + return edxval_api.get_video_info_for_course_and_profiles(unicode(course_id), video_profile_names) + + def student_view_json(self, context): + """ + Returns a JSON representation of the student_view of this XModule. + The contract of the JSON content is between the caller and the particular XModule. + """ + # Honor only_on_web + if self.only_on_web: + return {"only_on_web": True} + + encoded_videos = {} + val_video_data = {} + + # Check in VAL data first if edx_video_id exists + if self.edx_video_id: + video_profile_names = context.get("profiles", []) + + # get and cache bulk VAL data for course + val_course_data = self.get_cached_val_data_for_course(video_profile_names, self.location.course_key) + val_video_data = val_course_data.get(self.edx_video_id, {}) + + # Get the encoded videos if data from VAL is found + if val_video_data: + encoded_videos = val_video_data.get('profiles', {}) + + # If information for this edx_video_id is not found in the bulk course data, make a + # separate request for this individual edx_video_id, unless cache misses are disabled. + # This is useful/required for videos that don't have a course designated, such as the introductory video + # that is shared across many courses. However, this results in a separate database request so watch + # out for any performance hit if many such videos exist in a course. Set the 'allow_cache_miss' parameter + # to False to disable this fall back. + elif context.get("allow_cache_miss", "True").lower() == "true": + try: + val_video_data = edxval_api.get_video_info(self.edx_video_id) + # Unfortunately, the VAL API is inconsistent in how it returns the encodings, so remap here. + for enc_vid in val_video_data.pop('encoded_videos'): + encoded_videos[enc_vid['profile']] = {key: enc_vid[key] for key in ["url", "file_size"]} + except edxval_api.ValVideoNotFoundError: + pass + + # Fall back to other video URLs in the video module if not found in VAL + if not encoded_videos: + video_url = self.html5_sources[0] if self.html5_sources else self.source + if video_url: + encoded_videos["fallback"] = { + "url": video_url, + "file_size": 0, # File size is unknown for fallback URLs + } + + transcripts_info = self.get_transcripts_info() + transcripts = { + lang: self.runtime.handler_url(self, 'transcript', 'download', query="lang=" + lang, thirdparty=True) + for lang in self.available_translations(transcripts_info, verify_assets=False) + } + + return { + "only_on_web": self.only_on_web, + "duration": val_video_data.get('duration', None), + "transcripts": transcripts, + "encoded_videos": encoded_videos, + } diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index 36c1eacb2e..cb06a2cbed 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -1,5 +1,7 @@ # -*- coding: utf-8 -*- """Video xmodule tests in mongo.""" +import ddt +import itertools import json from collections import OrderedDict @@ -13,7 +15,7 @@ from django.test.utils import override_settings from xmodule.video_module import VideoDescriptor, bumper_utils, video_utils from xmodule.x_module import STUDENT_VIEW -from xmodule.tests.test_video import VideoDescriptorTestBase +from xmodule.tests.test_video import VideoDescriptorTestBase, instantiate_descriptor from xmodule.tests.test_import import DummySystem from edxval.api import ( @@ -861,6 +863,119 @@ class TestVideoDescriptorInitialization(BaseTestXmodule): self.assertFalse(self.item_descriptor.download_video) +@ddt.ddt +class TestVideoDescriptorStudentViewJson(TestCase): + """ + Tests for the student_view_json method on VideoDescriptor. + """ + TEST_DURATION = 111.0 + TEST_PROFILE = "mobile" + TEST_SOURCE_URL = "http://www.example.com/source.mp4" + TEST_LANGUAGE = "ge" + TEST_ENCODED_VIDEO = { + 'profile': TEST_PROFILE, + 'bitrate': 333, + 'url': 'http://example.com/video', + 'file_size': 222, + } + TEST_EDX_VIDEO_ID = 'test_edx_video_id' + + def setUp(self): + super(TestVideoDescriptorStudentViewJson, self).setUp() + sample_xml = ( + "" + ) + self.transcript_url = "transcript_url" + self.video = instantiate_descriptor(data=sample_xml) + self.video.runtime.handler_url = Mock(return_value=self.transcript_url) + + def setup_val_video(self, associate_course_in_val=False): + """ + Creates a video entry in VAL. + Arguments: + associate_course - If True, associates the test course with the video in VAL. + """ + create_profile('mobile') + create_video({ + 'edx_video_id': self.TEST_EDX_VIDEO_ID, + 'client_video_id': 'test_client_video_id', + 'duration': self.TEST_DURATION, + 'status': 'dummy', + 'encoded_videos': [self.TEST_ENCODED_VIDEO], + 'courses': [self.video.location.course_key] if associate_course_in_val else [], + }) + self.val_video = get_video_info(self.TEST_EDX_VIDEO_ID) # pylint: disable=attribute-defined-outside-init + + def get_result(self, allow_cache_miss=True): + """ + Returns the result from calling the video's student_view_json method. + Arguments: + allow_cache_miss is passed in the context to the student_view_json method. + """ + context = { + "profiles": [self.TEST_PROFILE], + "allow_cache_miss": "True" if allow_cache_miss else "False" + } + return self.video.student_view_json(context) + + def verify_result_with_fallback_url(self, result): + """ + Verifies the result is as expected when returning "fallback" video data (not from VAL). + """ + self.assertDictEqual( + result, + { + "only_on_web": False, + "duration": None, + "transcripts": {self.TEST_LANGUAGE: self.transcript_url}, + "encoded_videos": {"fallback": {"url": self.TEST_SOURCE_URL, "file_size": 0}}, + } + ) + + def verify_result_with_val_profile(self, result): + """ + Verifies the result is as expected when returning video data from VAL. + """ + self.assertDictContainsSubset( + result.pop("encoded_videos")[self.TEST_PROFILE], + self.TEST_ENCODED_VIDEO, + ) + self.assertDictEqual( + result, + { + "only_on_web": False, + "duration": self.TEST_DURATION, + "transcripts": {self.TEST_LANGUAGE: self.transcript_url}, + } + ) + + def test_only_on_web(self): + self.video.only_on_web = True + result = self.get_result() + self.assertDictEqual(result, {"only_on_web": True}) + + def test_no_edx_video_id(self): + result = self.get_result() + self.verify_result_with_fallback_url(result) + + @ddt.data( + *itertools.product([True, False], [True, False], [True, False]) + ) + @ddt.unpack + def test_with_edx_video_id(self, allow_cache_miss, video_exists_in_val, associate_course_in_val): + self.video.edx_video_id = self.TEST_EDX_VIDEO_ID + if video_exists_in_val: + self.setup_val_video(associate_course_in_val) + result = self.get_result(allow_cache_miss) + if video_exists_in_val and (associate_course_in_val or allow_cache_miss): + self.verify_result_with_val_profile(result) + else: + self.verify_result_with_fallback_url(result) + + @attr('shard_1') class VideoDescriptorTest(TestCase, VideoDescriptorTestBase): """ From 4921ec48c8a17dfc1df59ce8dd87120b62ab84af Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Fri, 5 Jun 2015 14:52:02 -0400 Subject: [PATCH 3/4] MA-792 Course Blocks and Navigation API (user-specific) --- common/djangoapps/util/module_utils.py | 12 +- .../xmodule/video_module/video_module.py | 3 +- .../course_structure_api/v0/tests.py | 262 +++++++++++- .../course_structure_api/v0/urls.py | 30 +- .../course_structure_api/v0/views.py | 397 +++++++++++++++++- lms/djangoapps/courseware/entrance_exams.py | 5 +- lms/djangoapps/courseware/grades.py | 10 +- .../mobile_api/video_outlines/serializers.py | 1 + lms/envs/common.py | 3 + lms/envs/test.py | 2 + 10 files changed, 691 insertions(+), 34 deletions(-) diff --git a/common/djangoapps/util/module_utils.py b/common/djangoapps/util/module_utils.py index 15a5e0f003..97f8fdaf81 100644 --- a/common/djangoapps/util/module_utils.py +++ b/common/djangoapps/util/module_utils.py @@ -3,7 +3,7 @@ Utility library containing operations used/shared by multiple courseware modules """ -def yield_dynamic_descriptor_descendents(descriptor, module_creator): # pylint: disable=invalid-name +def yield_dynamic_descriptor_descendants(descriptor, user_id, module_creator): # pylint: disable=invalid-name """ This returns all of the descendants of a descriptor. If the descriptor has dynamic children, the module will be created using module_creator @@ -13,17 +13,21 @@ def yield_dynamic_descriptor_descendents(descriptor, module_creator): # pylint: while len(stack) > 0: next_descriptor = stack.pop() - stack.extend(get_dynamic_descriptor_children(next_descriptor, module_creator)) + stack.extend(get_dynamic_descriptor_children(next_descriptor, user_id, module_creator)) yield next_descriptor -def get_dynamic_descriptor_children(descriptor, module_creator, usage_key_filter=None): +def get_dynamic_descriptor_children(descriptor, user_id, module_creator=None, usage_key_filter=None): """ Returns the children of the given descriptor, while supporting descriptors with dynamic children. """ module_children = [] if descriptor.has_dynamic_children(): - module = module_creator(descriptor) + # do not rebind the module if it's already bound to a user. + if descriptor.scope_ids.user_id and user_id == descriptor.scope_ids.user_id: + module = descriptor + else: + module = module_creator(descriptor) if module is not None: module_children = module.get_child_descriptors() else: diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py index cffc8ad13c..72c8c82aba 100644 --- a/common/lib/xmodule/xmodule/video_module/video_module.py +++ b/common/lib/xmodule/xmodule/video_module/video_module.py @@ -755,7 +755,8 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler Returns a JSON representation of the student_view of this XModule. The contract of the JSON content is between the caller and the particular XModule. """ - # Honor only_on_web + # If the "only_on_web" field is set on this video, do not return the rest of the video's data + # in this json view, since this video is to be accessed only through its web view." if self.only_on_web: return {"only_on_web": True} diff --git a/lms/djangoapps/course_structure_api/v0/tests.py b/lms/djangoapps/course_structure_api/v0/tests.py index 3545479a37..291b329428 100644 --- a/lms/djangoapps/course_structure_api/v0/tests.py +++ b/lms/djangoapps/course_structure_api/v0/tests.py @@ -3,20 +3,26 @@ Run these tests @ Devstack: paver test_system -s lms --fasttest --verbose --test_id=lms/djangoapps/course_structure_api """ # pylint: disable=missing-docstring,invalid-name,maybe-no-member,attribute-defined-outside-init +from abc import ABCMeta from datetime import datetime +from mock import patch, Mock +from itertools import product from django.core.urlresolvers import reverse from django.test.utils import override_settings -from mock import patch, Mock + +from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory from oauth2_provider.tests.factories import AccessTokenFactory, ClientFactory from opaque_keys.edx.locator import CourseLocator from xmodule.error_module import ErrorDescriptor from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls from xmodule.modulestore.xml import CourseLocationManager from xmodule.tests import get_test_system +from student.tests.factories import UserFactory, CourseEnrollmentFactory from courseware.tests.factories import GlobalStaffFactory, StaffFactory from openedx.core.djangoapps.content.course_structures.models import CourseStructure from openedx.core.djangoapps.content.course_structures.tasks import update_course_structure @@ -36,8 +42,11 @@ class CourseViewTestsMixin(object): self.create_test_data() self.create_user_and_access_token() - def create_user_and_access_token(self): + def create_user(self): self.user = GlobalStaffFactory.create() + + def create_user_and_access_token(self): + self.create_user() self.oauth_client = ClientFactory.create() self.access_token = AccessTokenFactory.create(user=self.user, client=self.oauth_client).token @@ -61,7 +70,7 @@ class CourseViewTestsMixin(object): ]) self.course_id = unicode(self.course.id) - sequential = ItemFactory.create( + self.sequential = ItemFactory.create( category="sequential", parent_location=self.course.location, display_name="Lesson 1", @@ -69,11 +78,21 @@ class CourseViewTestsMixin(object): graded=True ) + factory = MultipleChoiceResponseXMLFactory() + args = {'choices': [False, True, False]} + problem_xml = factory.build_xml(**args) ItemFactory.create( category="problem", - parent_location=sequential.location, + parent_location=self.sequential.location, display_name="Problem 1", - format="Homework" + format="Homework", + data=problem_xml, + ) + + self.video = ItemFactory.create( + category="video", + parent_location=self.sequential.location, + display_name="Video 1", ) self.empty_course = CourseFactory.create( @@ -120,6 +139,14 @@ class CourseViewTestsMixin(object): response = self.client.get(uri, follow=True, **default_headers) return response + def http_get_for_course(self, course_id=None, **headers): + """Submit an HTTP GET request to the view for the given course""" + + return self.http_get( + reverse(self.view, kwargs={'course_id': course_id or self.course_id}), + **headers + ) + def test_not_authenticated(self): """ Verify that access is denied to non-authenticated users. @@ -133,23 +160,24 @@ class CourseViewTestsMixin(object): raise NotImplementedError -class CourseDetailMixin(object): +class CourseDetailTestMixin(object): """ Mixin for views utilizing only the course_id kwarg. """ + view_supports_debug_mode = True def test_get_invalid_course(self): """ The view should return a 404 if the course ID is invalid. """ - response = self.http_get(reverse(self.view, kwargs={'course_id': self.invalid_course_id})) + response = self.http_get_for_course(self.invalid_course_id) self.assertEqual(response.status_code, 404) def test_get(self): """ The view should return a 200 if the course ID is valid. """ - response = self.http_get(reverse(self.view, kwargs={'course_id': self.course_id})) + response = self.http_get_for_course() self.assertEqual(response.status_code, 200) # Return the response so child classes do not have to repeat the request. @@ -158,7 +186,7 @@ class CourseDetailMixin(object): def test_not_authenticated(self): """ The view should return HTTP status 401 if no user is authenticated. """ # HTTP 401 should be returned if the user is not authenticated. - response = self.http_get(reverse(self.view, kwargs={'course_id': self.course_id}), HTTP_AUTHORIZATION=None) + response = self.http_get_for_course(HTTP_AUTHORIZATION=None) self.assertEqual(response.status_code, 401) def test_not_authorized(self): @@ -167,14 +195,12 @@ class CourseDetailMixin(object): auth_header = 'Bearer ' + access_token # Access should be granted if the proper access token is supplied. - response = self.http_get(reverse(self.view, kwargs={'course_id': self.course_id}), - HTTP_AUTHORIZATION=auth_header) + response = self.http_get_for_course(HTTP_AUTHORIZATION=auth_header) self.assertEqual(response.status_code, 200) # Access should be denied if the user is not course staff. - response = self.http_get(reverse(self.view, kwargs={'course_id': unicode(self.empty_course.id)}), - HTTP_AUTHORIZATION=auth_header) - self.assertEqual(response.status_code, 403) + response = self.http_get_for_course(course_id=unicode(self.empty_course.id), HTTP_AUTHORIZATION=auth_header) + self.assertEqual(response.status_code, 404) class CourseListTests(CourseViewTestsMixin, ModuleStoreTestCase): @@ -268,7 +294,7 @@ class CourseListTests(CourseViewTestsMixin, ModuleStoreTestCase): self.test_get() -class CourseDetailTests(CourseDetailMixin, CourseViewTestsMixin, ModuleStoreTestCase): +class CourseDetailTests(CourseDetailTestMixin, CourseViewTestsMixin, ModuleStoreTestCase): view = 'course_structure_api:v0:detail' def test_get(self): @@ -276,7 +302,7 @@ class CourseDetailTests(CourseDetailMixin, CourseViewTestsMixin, ModuleStoreTest self.assertValidResponseCourse(response.data, self.course) -class CourseStructureTests(CourseDetailMixin, CourseViewTestsMixin, ModuleStoreTestCase): +class CourseStructureTests(CourseDetailTestMixin, CourseViewTestsMixin, ModuleStoreTestCase): view = 'course_structure_api:v0:structure' def setUp(self): @@ -294,13 +320,13 @@ class CourseStructureTests(CourseDetailMixin, CourseViewTestsMixin, ModuleStoreT # Attempt to retrieve data for a course without stored structure CourseStructure.objects.all().delete() self.assertFalse(CourseStructure.objects.filter(course_id=self.course.id).exists()) - response = self.http_get(reverse(self.view, kwargs={'course_id': self.course_id})) + response = self.http_get_for_course() self.assertEqual(response.status_code, 503) self.assertEqual(response['Retry-After'], '120') # Course structure generation shouldn't take long. Generate the data and try again. self.assertTrue(CourseStructure.objects.filter(course_id=self.course.id).exists()) - response = self.http_get(reverse(self.view, kwargs={'course_id': self.course_id})) + response = self.http_get_for_course() self.assertEqual(response.status_code, 200) blocks = {} @@ -331,7 +357,7 @@ class CourseStructureTests(CourseDetailMixin, CourseViewTestsMixin, ModuleStoreT self.assertDictEqual(response.data, expected) -class CourseGradingPolicyTests(CourseDetailMixin, CourseViewTestsMixin, ModuleStoreTestCase): +class CourseGradingPolicyTests(CourseDetailTestMixin, CourseViewTestsMixin, ModuleStoreTestCase): view = 'course_structure_api:v0:grading_policy' def test_get(self): @@ -355,3 +381,199 @@ class CourseGradingPolicyTests(CourseDetailMixin, CourseViewTestsMixin, ModuleSt } ] self.assertListEqual(response.data, expected) + + +##################################################################################### +# +# The following Mixins/Classes collectively test the CourseBlocksAndNavigation view. +# +# The class hierarchy is: +# +# -----------------> CourseBlocksOrNavigationTestMixin <-------------- +# | ^ | +# | | | +# | CourseNavigationTestMixin | CourseBlocksTestMixin | +# | ^ ^ | ^ ^ | +# | | | | | | | +# | | | | | | | +# CourseNavigationTests CourseBlocksAndNavigationTests CourseBlocksTests +# +# +# Each Test Mixin is an abstract class that implements tests specific to its +# corresponding functionality. +# +# The concrete Test classes are expected to define the following class fields: +# +# block_navigation_view_type - The view's name as it should be passed to the django +# reverse method. +# container_fields - A list of fields that are expected to be included in the view's +# response for all container block types. +# block_fields - A list of fields that are expected to be included in the view's +# response for all block types. +# +###################################################################################### + + +class CourseBlocksOrNavigationTestMixin(CourseDetailTestMixin, CourseViewTestsMixin): + """ + A Mixin class for testing all views related to Course blocks and/or navigation. + """ + __metaclass__ = ABCMeta + + view_supports_debug_mode = False + + def setUp(self): + """ + Override the base `setUp` method to enroll the user in the course, since these views + require enrollment for non-staff users. + """ + super(CourseBlocksOrNavigationTestMixin, self).setUp() + CourseEnrollmentFactory(user=self.user, course_id=self.course.id) + + def create_user(self): + """ + Override the base `create_user` method to test with non-staff users for these views. + """ + self.user = UserFactory.create() + + @property + def view(self): + """ + Returns the name of the view for testing to use in the django `reverse` call. + """ + return 'course_structure_api:v0:' + self.block_navigation_view_type + + def test_get(self): + with check_mongo_calls(3): + response = super(CourseBlocksOrNavigationTestMixin, self).test_get() + + # verify root element + self.assertIn('root', response.data) + root_string = unicode(self.course.location) + self.assertEquals(response.data['root'], root_string) + + # verify ~blocks element + self.assertTrue(self.block_navigation_view_type in response.data) + blocks = response.data[self.block_navigation_view_type] + + # verify number of blocks + self.assertEquals(len(blocks), 4) + + # verify fields in blocks + for field, block in product(self.block_fields, blocks.values()): + self.assertIn(field, block) + + # verify container fields in container blocks + for field in self.container_fields: + self.assertIn(field, blocks[root_string]) + + def test_parse_error(self): + """ + Verifies the view returns a 400 when a query parameter is incorrectly formatted. + """ + response = self.http_get_for_course(data={'block_json': 'incorrect'}) + self.assertEqual(response.status_code, 400) + + def test_no_access_to_block(self): + """ + Verifies the view returns only the top-level course block, excluding the sequential block + and its descendants when the user does not have access to the sequential. + """ + self.sequential.visible_to_staff_only = True + modulestore().update_item(self.sequential, self.user.id) + + response = super(CourseBlocksOrNavigationTestMixin, self).test_get() + self.assertEquals(len(response.data[self.block_navigation_view_type]), 1) + + +class CourseBlocksTestMixin(object): + """ + A Mixin class for testing all views related to Course blocks. + """ + __metaclass__ = ABCMeta + + view_supports_debug_mode = False + block_fields = ['id', 'type', 'display_name', 'web_url', 'block_url', 'graded', 'format'] + + def test_block_json(self): + """ + Verifies the view's response when the block_json data is requested. + """ + response = self.http_get_for_course( + data={'block_json': '{"video":{"profiles":["mobile_low"]}}'} + ) + self.assertEquals(response.status_code, 200) + video_block = response.data[self.block_navigation_view_type][unicode(self.video.location)] + self.assertIn('block_json', video_block) + + def test_block_count(self): + """ + Verifies the view's response when the block_count data is requested. + """ + response = self.http_get_for_course( + data={'block_count': 'problem'} + ) + self.assertEquals(response.status_code, 200) + root_block = response.data[self.block_navigation_view_type][unicode(self.course.location)] + self.assertIn('block_count', root_block) + self.assertIn('problem', root_block['block_count']) + self.assertEquals(root_block['block_count']['problem'], 1) + + +class CourseNavigationTestMixin(object): + """ + A Mixin class for testing all views related to Course navigation. + """ + __metaclass__ = ABCMeta + + def test_depth_zero(self): + """ + Tests that all descendants are bundled into the root block when the navigation_depth is set to 0. + """ + response = self.http_get_for_course( + data={'navigation_depth': '0'} + ) + root_block = response.data[self.block_navigation_view_type][unicode(self.course.location)] + self.assertIn('descendants', root_block) + self.assertEquals(len(root_block['descendants']), 3) + + def test_depth(self): + """ + Tests that all container blocks have descendants listed in their data. + """ + response = self.http_get_for_course() + + container_descendants = ( + (self.course.location, 1), + (self.sequential.location, 2), + ) + for container_location, expected_num_descendants in container_descendants: + block = response.data[self.block_navigation_view_type][unicode(container_location)] + self.assertIn('descendants', block) + self.assertEquals(len(block['descendants']), expected_num_descendants) + + +class CourseBlocksTests(CourseBlocksOrNavigationTestMixin, CourseBlocksTestMixin, ModuleStoreTestCase): + """ + A Test class for testing the Course 'blocks' view. + """ + block_navigation_view_type = 'blocks' + container_fields = ['children'] + + +class CourseNavigationTests(CourseBlocksOrNavigationTestMixin, CourseNavigationTestMixin, ModuleStoreTestCase): + """ + A Test class for testing the Course 'navigation' view. + """ + block_navigation_view_type = 'navigation' + container_fields = ['descendants'] + block_fields = [] + + +class CourseBlocksAndNavigationTests(CourseBlocksOrNavigationTestMixin, CourseBlocksTestMixin, + CourseNavigationTestMixin, ModuleStoreTestCase): + """ + A Test class for testing the Course 'blocks+navigation' view. + """ + block_navigation_view_type = 'blocks+navigation' + container_fields = ['children', 'descendants'] diff --git a/lms/djangoapps/course_structure_api/v0/urls.py b/lms/djangoapps/course_structure_api/v0/urls.py index 2ccc7db7b1..035b1a023a 100644 --- a/lms/djangoapps/course_structure_api/v0/urls.py +++ b/lms/djangoapps/course_structure_api/v0/urls.py @@ -14,5 +14,33 @@ urlpatterns = patterns( url(r'^courses/$', views.CourseList.as_view(), name='list'), url(r'^courses/{}/$'.format(COURSE_ID_PATTERN), views.CourseDetail.as_view(), name='detail'), url(r'^course_structures/{}/$'.format(COURSE_ID_PATTERN), views.CourseStructure.as_view(), name='structure'), - url(r'^grading_policies/{}/$'.format(COURSE_ID_PATTERN), views.CourseGradingPolicy.as_view(), name='grading_policy') + url( + r'^grading_policies/{}/$'.format(COURSE_ID_PATTERN), + views.CourseGradingPolicy.as_view(), + name='grading_policy' + ), ) + +if settings.FEATURES.get('ENABLE_COURSE_BLOCKS_NAVIGATION_API'): + # TODO (MA-789) This endpoint still needs to be approved by the arch council. + # TODO (MA-704) This endpoint still needs to be made performant. + urlpatterns += ( + url( + r'^courses/{}/blocks/$'.format(COURSE_ID_PATTERN), + views.CourseBlocksAndNavigation.as_view(), + {'return_blocks': True, 'return_nav': False}, + name='blocks' + ), + url( + r'^courses/{}/navigation/$'.format(COURSE_ID_PATTERN), + views.CourseBlocksAndNavigation.as_view(), + {'return_blocks': False, 'return_nav': True}, + name='navigation' + ), + url( + r'^courses/{}/blocks\+navigation/$'.format(COURSE_ID_PATTERN), + views.CourseBlocksAndNavigation.as_view(), + {'return_blocks': True, 'return_nav': True}, + name='blocks+navigation' + ), + ) diff --git a/lms/djangoapps/course_structure_api/v0/views.py b/lms/djangoapps/course_structure_api/v0/views.py index d9468b2c1c..d53757a1b3 100644 --- a/lms/djangoapps/course_structure_api/v0/views.py +++ b/lms/djangoapps/course_structure_api/v0/views.py @@ -1,14 +1,17 @@ """ API implementation for course-oriented interactions. """ +from collections import namedtuple +import json import logging from django.conf import settings from django.http import Http404 from rest_framework.authentication import OAuth2Authentication, SessionAuthentication -from rest_framework.exceptions import PermissionDenied, AuthenticationFailed +from rest_framework.exceptions import AuthenticationFailed, ParseError from rest_framework.generics import RetrieveAPIView, ListAPIView from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response +from rest_framework.reverse import reverse from xmodule.modulestore.django import modulestore from opaque_keys.edx.keys import CourseKey @@ -16,8 +19,12 @@ from course_structure_api.v0 import api, serializers from course_structure_api.v0.errors import CourseNotFoundError, CourseStructureNotAvailableError from courseware import courses from courseware.access import has_access +from courseware.model_data import FieldDataCache +from courseware.module_render import get_module_for_descriptor +from openedx.core.lib.api.view_utils import view_course_access, view_auth_classes from openedx.core.lib.api.serializers import PaginationSerializer from student.roles import CourseInstructorRole, CourseStaffRole +from util.module_utils import get_dynamic_descriptor_children log = logging.getLogger(__name__) @@ -83,10 +90,10 @@ class CourseViewMixin(object): def check_course_permissions(self, user, course): """ Checks if the request user can access the course. - Raises PermissionDenied if the user does not have course access. + Raises 404 if the user does not have course access. """ if not self.user_can_access_course(user, course): - raise PermissionDenied + raise Http404 def perform_authentication(self, request): """ @@ -290,3 +297,387 @@ class CourseGradingPolicy(CourseViewMixin, ListAPIView): @CourseViewMixin.course_check def get(self, request, **kwargs): return Response(api.course_grading_policy(self.course_key)) + + +@view_auth_classes() +class CourseBlocksAndNavigation(ListAPIView): + """ + **Use Case** + + The following endpoints return the content of the course according to the requesting user's access level. + + * Blocks - Get the course's blocks. + + * Navigation - Get the course's navigation information per the navigation depth requested. + + * Blocks+Navigation - Get both the course's blocks and the course's navigation information. + + **Example requests**: + + GET api/course_structure/v0/courses/{course_id}/blocks/ + GET api/course_structure/v0/courses/{course_id}/navigation/ + GET api/course_structure/v0/courses/{course_id}/blocks+navigation/ + &block_count=video + &block_json={"video":{"profiles":["mobile_low"]}} + &fields=graded,format,responsive_ui + + **Parameters**: + + * block_json: (dict) Indicates for which block types to return student_view_json data. The key is the block + type and the value is the "context" that is passed to the block's student_view_json method. + + Example: block_json={"video":{"profiles":["mobile_high","mobile_low"]}} + + * block_count: (list) Indicates for which block types to return the aggregate count of the blocks. + + Example: block_count="video,problem" + + * fields: (list) Indicates which additional fields to return for each block. + Default is "children,graded,format,responsive_ui" + + Example: fields=graded,format,responsive_ui + + * navigation_depth (integer) Indicates how far deep to traverse into the course hierarchy before bundling + all the descendants. + Default is 3 since typical navigational views of the course show a maximum of chapter->sequential->vertical. + + Example: navigation_depth=3 + + **Response Values** + + The following fields are returned with a successful response. + Only either one of blocks, navigation, or blocks+navigation is returned depending on which endpoint is used. + The "root" field is returned for all endpoints. + + * root: The ID of the root node of the course blocks. + + * blocks: A dictionary that maps block usage IDs to a collection of information about each block. + Each block contains the following fields. Returned only if using the "blocks" endpoint. + + * id: (string) The usage ID of the block. + + * type: (string) The type of block. Possible values include course, chapter, sequential, vertical, html, + problem, video, and discussion. The type can also be the name of a custom type of block used for the course. + + * display_name: (string) The display name of the block. + + * children: (list) If the block has child blocks, a list of IDs of the child blocks. + Returned only if the "children" input parameter is True. + + * block_count: (dict) For each block type specified in the block_count parameter to the endpoint, the + aggregate number of blocks of that type for this block and all of its descendants. + Returned only if the "block_count" input parameter contains this block's type. + + * block_json: (dict) The JSON data for this block. + Returned only if the "block_json" input parameter contains this block's type. + + * block_url: (string) The URL to retrieve the HTML rendering of this block. The HTML could include + CSS and Javascript code. This URL can be used as a fallback if the custom block_json for this + block type is not requested and not supported. + + * web_url: (string) The URL to the website location of this block. This URL can be used as a further + fallback if the block_url and the block_json is not supported. + + * graded (boolean) Whether or not the block or any of its descendants is graded. + Returned only if "graded" is included in the "fields" parameter. + + * format: (string) The assignment type of the block. + Possible values can be "Homework", "Lab", "Midterm Exam", and "Final Exam". + Returned only if "format" is included in the "fields" parameter. + + * responsive_ui: (boolean) Whether or not the block's rendering obtained via block_url is responsive. + Returned only if "responsive_ui" is included in the "fields" parameter. + + * navigation: A dictionary that maps block IDs to a collection of navigation information about each block. + Each block contains the following fields. Returned only if using the "navigation" endpoint. + + * descendants: (list) A list of IDs of the children of the block if the block's depth in the + course hierarchy is less than the navigation_depth. Otherwise, a list of IDs of the aggregate descendants + of the block. + + * blocks+navigation: A dictionary that combines both the blocks and navigation data. + Returned only if using the "blocks+navigation" endpoint. + + """ + class RequestInfo(object): + """ + A class for encapsulating the request information, including what optional fields are requested. + """ + DEFAULT_FIELDS = "children,graded,format,responsive_ui" + + def __init__(self, request, course): + self.request = request + self.course = course + self.field_data_cache = None + + # check what fields are requested + try: + # fields + self.fields = set(request.GET.get('fields', self.DEFAULT_FIELDS).split(",")) + + # children + self.children = 'children' in self.fields + self.fields.discard('children') + + # block_count + self.block_count = request.GET.get('block_count', "") + self.block_count = ( + self.block_count.split(",") if self.block_count else [] + ) + + # navigation_depth + # See docstring for why we default to 3. + self.navigation_depth = int(request.GET.get('navigation_depth', '3')) + + # block_json + self.block_json = json.loads(request.GET.get('block_json', "{}")) + if self.block_json and not isinstance(self.block_json, dict): + raise ParseError + except: + raise ParseError + + class ResultData(object): + """ + A class for encapsulating the result information, specifically the blocks and navigation data. + """ + def __init__(self, return_blocks, return_nav): + self.blocks = {} + self.navigation = {} + if return_blocks and return_nav: + self.navigation = self.blocks + + def update_response(self, response, return_blocks, return_nav): + """ + Updates the response object with result information. + """ + if return_blocks and return_nav: + response["blocks+navigation"] = self.blocks + elif return_blocks: + response["blocks"] = self.blocks + elif return_nav: + response["navigation"] = self.navigation + + class BlockInfo(object): + """ + A class for encapsulating a block's information as needed during traversal of a block hierarchy. + """ + def __init__(self, block, request_info, parent_block_info=None): + # the block for which the recursion is being computed + self.block = block + + # the type of the block + self.type = block.category + + # the block's depth in the block hierarchy + self.depth = 0 + + # the block's children + self.children = [] + + # descendants_of_parent: the list of descendants for this block's parent + self.descendants_of_parent = [] + self.descendants_of_self = [] + + # if a parent block was provided, update this block's data based on the parent's data + if parent_block_info: + # increment this block's depth value + self.depth = parent_block_info.depth + 1 + + # set this blocks' descendants_of_parent + self.descendants_of_parent = parent_block_info.descendants_of_self + + # add ourselves to the parent's children, if requested. + if request_info.children: + parent_block_info.value.setdefault("children", []).append(unicode(block.location)) + + # the block's data to include in the response + self.value = { + "id": unicode(block.location), + "type": self.type, + "display_name": block.display_name, + "web_url": reverse( + "jump_to", + kwargs={"course_id": unicode(request_info.course.id), "location": unicode(block.location)}, + request=request_info.request, + ), + "block_url": reverse( + "courseware.views.render_xblock", + kwargs={"usage_key_string": unicode(block.location)}, + request=request_info.request, + ), + } + + @view_course_access(depth=None) + def list(self, request, course, return_blocks=True, return_nav=True, *args, **kwargs): + """ + REST API endpoint for listing all the blocks and/or navigation information in the course, + while regarding user access and roles. + + Arguments: + request - Django request object + course - course module object + return_blocks - If true, returns the blocks information for the course. + return_nav - If true, returns the navigation information for the course. + """ + # set starting point + start_block = course + + # initialize request and result objects + request_info = self.RequestInfo(request, course) + result_data = self.ResultData(return_blocks, return_nav) + + # create and populate a field data cache by pre-fetching for the course (with depth=None) + request_info.field_data_cache = FieldDataCache.cache_for_descriptor_descendents( + course.id, request.user, course, depth=None, + ) + + # start the recursion with the start_block + self.recurse_blocks_nav(request_info, result_data, self.BlockInfo(start_block, request_info)) + + # return response + response = {"root": unicode(start_block.location)} + result_data.update_response(response, return_blocks, return_nav) + return Response(response) + + def recurse_blocks_nav(self, request_info, result_data, block_info): + """ + A depth-first recursive function that supports calculation of both the list of blocks in the course + and the navigation information up to the requested navigation_depth of the course. + + Arguments: + request_info - Object encapsulating the request information. + result_data - Running result data that is updated during the recursion. + block_info - Information about the current block in the recursion. + """ + # bind user data to the block + block_info.block = get_module_for_descriptor( + request_info.request.user, + request_info.request, + block_info.block, + request_info.field_data_cache, + request_info.course.id + ) + + # verify the user has access to this block + if not has_access(request_info.request.user, 'load', block_info.block, course_key=request_info.course.id): + return + + # add the block's value to the result + result_data.blocks[unicode(block_info.block.location)] = block_info.value + + # descendants + self.update_descendants(request_info, result_data, block_info) + + # children: recursively call the function for each of the children, while supporting dynamic children. + if block_info.block.has_children: + block_info.children = get_dynamic_descriptor_children(block_info.block, request_info.request.user.id) + for child in block_info.children: + self.recurse_blocks_nav( + request_info, + result_data, + self.BlockInfo(child, request_info, parent_block_info=block_info) + ) + + # block count + self.update_block_count(request_info, result_data, block_info) + + # block JSON data + self.add_block_json(request_info, block_info) + + # additional fields + self.add_additional_fields(request_info, block_info) + + def update_descendants(self, request_info, result_data, block_info): + """ + Updates the descendants data for the current block. + + The current block is added to its parent's descendants if it is visible in the navigation + (i.e., the 'hide_from_toc' setting is False). + + Additionally, the block's depth is compared with the navigation_depth parameter to determine whether the + descendants of the block should be added to its own descendants (if block.depth <= navigation_depth) + or to the descendants of the block's parents (if block.depth > navigation_depth). + + block_info.descendants_of_self is the list of descendants that is passed to this block's children. + It should be either: + descendants_of_parent - if this block's depth is greater than the requested navigation_depth. + a dangling [] - if this block's hide_from_toc is True. + a referenced [] in navigation[block.location]["descendants"] - if this block's depth is within + the requested navigation depth. + """ + # Blocks with the 'hide_from_toc' setting are accessible, just not navigatable from the table-of-contents. + # If the 'hide_from_toc' setting is set on the block, do not add this block to the parent's descendants + # list and let the block's descendants add themselves to a dangling (unreferenced) descendants list. + if not block_info.block.hide_from_toc: + # add this block to the parent's descendants + block_info.descendants_of_parent.append(unicode(block_info.block.location)) + + # if this block's depth in the hierarchy is greater than the requested navigation depth, + # have the block's descendants add themselves to the parent's descendants. + if block_info.depth > request_info.navigation_depth: + block_info.descendants_of_self = block_info.descendants_of_parent + + # otherwise, have the block's descendants add themselves to this block's descendants by + # referencing/attaching descendants_of_self from this block's navigation value. + else: + result_data.navigation.setdefault( + unicode(block_info.block.location), {} + )["descendants"] = block_info.descendants_of_self + + def update_block_count(self, request_info, result_data, block_info): + """ + For all the block types that are requested to be counted, include the count of that block type as + aggregated from the block's descendants. + + Arguments: + request_info - Object encapsulating the request information. + result_data - Running result data that is updated during the recursion. + block_info - Information about the current block in the recursion. + """ + for b_type in request_info.block_count: + block_info.value.setdefault("block_count", {})[b_type] = ( + sum( + result_data.blocks.get(unicode(child.location), {}).get("block_count", {}).get(b_type, 0) + for child in block_info.children + ) + + (1 if b_type == block_info.type else 0) + ) + + def add_block_json(self, request_info, block_info): + """ + If the JSON data for this block's type is requested, and the block supports the 'student_view_json' + method, add the response from the 'student_view_json" method as the data for the block. + """ + if block_info.type in request_info.block_json: + if getattr(block_info.block, 'student_view_json', None): + block_info.value["block_json"] = block_info.block.student_view_json( + context=request_info.block_json[block_info.type] + ) + + # A mapping of API-exposed field names to xBlock field names and API field defaults. + BlockApiField = namedtuple('BlockApiField', 'block_field_name api_field_default') + FIELD_MAP = { + 'graded': BlockApiField(block_field_name='graded', api_field_default=False), + 'format': BlockApiField(block_field_name='format', api_field_default=None), + 'responsive_ui': BlockApiField(block_field_name='has_responsive_ui', api_field_default=False), + } + + def add_additional_fields(self, request_info, block_info): + """ + Add additional field names and values of the block as requested in the request_info. + """ + for field_name in request_info.fields: + if field_name in self.FIELD_MAP: + block_info.value[field_name] = getattr( + block_info.block, + self.FIELD_MAP[field_name].block_field_name, + self.FIELD_MAP[field_name].api_field_default, + ) + + def perform_authentication(self, request): + """ + Ensures that the user is authenticated (e.g. not an AnonymousUser) + """ + super(CourseBlocksAndNavigation, self).perform_authentication(request) + if request.user.is_anonymous(): + raise AuthenticationFailed diff --git a/lms/djangoapps/courseware/entrance_exams.py b/lms/djangoapps/courseware/entrance_exams.py index e7a0400ec1..24e596f8c2 100644 --- a/lms/djangoapps/courseware/entrance_exams.py +++ b/lms/djangoapps/courseware/entrance_exams.py @@ -9,7 +9,7 @@ from courseware.models import StudentModule from opaque_keys.edx.keys import UsageKey from student.models import EntranceExamConfiguration from util.milestones_helpers import get_required_content -from util.module_utils import yield_dynamic_descriptor_descendents +from util.module_utils import yield_dynamic_descriptor_descendants from xmodule.modulestore.django import modulestore @@ -147,8 +147,9 @@ def get_entrance_exam_score(request, course): course.id ) - exam_module_generators = yield_dynamic_descriptor_descendents( + exam_module_generators = yield_dynamic_descriptor_descendants( exam_descriptor, + request.user.id, inner_get_module ) exam_modules = [module for module in exam_module_generators] diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index aff9005bb7..b0849bcd85 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -15,7 +15,7 @@ import dogstats_wrapper as dog_stats_api from courseware import courses from courseware.model_data import FieldDataCache from student.models import anonymous_id_for_user -from util.module_utils import yield_dynamic_descriptor_descendents +from util.module_utils import yield_dynamic_descriptor_descendants from xmodule import graders from xmodule.graders import Score from xmodule.modulestore.django import modulestore @@ -209,7 +209,9 @@ def _grade(student, request, course, keep_raw_scores): field_data_cache = FieldDataCache([descriptor], course.id, student) return get_module_for_descriptor(student, request, descriptor, field_data_cache, course.id) - for module_descriptor in yield_dynamic_descriptor_descendents(section_descriptor, create_module): + for module_descriptor in yield_dynamic_descriptor_descendants( + section_descriptor, student.id, create_module + ): (correct, total) = get_score( course.id, student, module_descriptor, create_module, scores_cache=submissions_scores @@ -364,7 +366,9 @@ def _progress_summary(student, request, course): module_creator = section_module.xmodule_runtime.get_module - for module_descriptor in yield_dynamic_descriptor_descendents(section_module, module_creator): + for module_descriptor in yield_dynamic_descriptor_descendants( + section_module, student.id, module_creator + ): course_id = course.id (correct, total) = get_score( course_id, student, module_descriptor, module_creator, scores_cache=submissions_scores diff --git a/lms/djangoapps/mobile_api/video_outlines/serializers.py b/lms/djangoapps/mobile_api/video_outlines/serializers.py index 167abf3e23..5eacab4341 100644 --- a/lms/djangoapps/mobile_api/video_outlines/serializers.py +++ b/lms/djangoapps/mobile_api/video_outlines/serializers.py @@ -86,6 +86,7 @@ class BlockOutline(object): if curr_block.has_children: children = get_dynamic_descriptor_children( curr_block, + self.request.user.id, create_module, usage_key_filter=parent_or_requested_block_type ) diff --git a/lms/envs/common.py b/lms/envs/common.py index ada3612a29..45649a8389 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -321,7 +321,10 @@ FEATURES = { # ENABLE_OAUTH2_PROVIDER to True 'ENABLE_MOBILE_REST_API': False, 'ENABLE_MOBILE_SOCIAL_FACEBOOK_FEATURES': False, + + # Enable APIs required for xBlocks on Mobile, and supported in general 'ENABLE_RENDER_XBLOCK_API': False, + 'ENABLE_COURSE_BLOCKS_NAVIGATION_API': False, # Enable the combined login/registration form 'ENABLE_COMBINED_LOGIN_REGISTRATION': False, diff --git a/lms/envs/test.py b/lms/envs/test.py index 323fa9bfe8..8519d860b2 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -268,6 +268,8 @@ FEATURES['ENABLE_OAUTH2_PROVIDER'] = True FEATURES['ENABLE_MOBILE_REST_API'] = True FEATURES['ENABLE_MOBILE_SOCIAL_FACEBOOK_FEATURES'] = True FEATURES['ENABLE_VIDEO_ABSTRACTION_LAYER_API'] = True +FEATURES['ENABLE_COURSE_BLOCKS_NAVIGATION_API'] = True +FEATURES['ENABLE_RENDER_XBLOCK_API'] = True ###################### Payment ##############################3 # Enable fake payment processing page From 89ea8b31c6f248b45e991c4c7ad0ca8a4afb9ad9 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Tue, 26 May 2015 23:20:39 -0400 Subject: [PATCH 4/4] MA-725 responsive_ui indication on responsive xBlocks. --- common/lib/capa/capa/capa_problem.py | 7 +++++++ common/lib/capa/capa/responsetypes.py | 10 ++++++++++ common/lib/xmodule/xmodule/capa_module.py | 7 +++++++ common/lib/xmodule/xmodule/html_module.py | 1 + 4 files changed, 25 insertions(+) diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index 48b07bd876..707232cc79 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -527,6 +527,13 @@ class LoncapaProblem(object): log.warning("Could not find matching input for id: %s", input_id) return {} + @property + def has_responsive_ui(self): + """ + Returns whether this capa problem has support for responsive UI. + """ + return all(responder.has_responsive_ui for responder in self.responders.values()) + # ======= Private Methods Below ======== def _process_includes(self): diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index ec23d4ba9c..54fe52af26 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -138,6 +138,11 @@ class LoncapaResponse(object): allowed_inputfields = [] required_attributes = [] + # Overridable field that specifies whether this capa response type has support for + # responsive UI, for rendering on devices of different sizes and shapes. + # By default, we set this to False, allowing subclasses to override as appropriate. + has_responsive_ui = False + def __init__(self, xml, inputfields, context, system): """ Init is passed the following arguments: @@ -692,6 +697,7 @@ class ChoiceResponse(LoncapaResponse): max_inputfields = 1 allowed_inputfields = ['checkboxgroup', 'radiogroup'] correct_choices = None + has_responsive_ui = True def setup_response(self): @@ -763,6 +769,7 @@ class MultipleChoiceResponse(LoncapaResponse): max_inputfields = 1 allowed_inputfields = ['choicegroup'] correct_choices = None + has_responsive_ui = True def setup_response(self): # call secondary setup for MultipleChoice questions, to set name @@ -1084,6 +1091,7 @@ class OptionResponse(LoncapaResponse): hint_tag = 'optionhint' allowed_inputfields = ['optioninput'] answer_fields = None + has_responsive_ui = True def setup_response(self): self.answer_fields = self.inputfields @@ -1136,6 +1144,7 @@ class NumericalResponse(LoncapaResponse): allowed_inputfields = ['textline', 'formulaequationinput'] required_attributes = ['answer'] max_inputfields = 1 + has_responsive_ui = True def __init__(self, *args, **kwargs): self.correct_answer = '' @@ -1338,6 +1347,7 @@ class StringResponse(LoncapaResponse): required_attributes = ['answer'] max_inputfields = 1 correct_answer = [] + has_responsive_ui = True def setup_response_backward(self): self.correct_answer = [ diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index d647aa8c0d..92459e91cb 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -187,6 +187,13 @@ class CapaDescriptor(CapaFields, RawDescriptor): registered_tags = responsetypes.registry.registered_tags() return set([node.tag for node in tree.iter() if node.tag in registered_tags]) + @property + def has_responsive_ui(self): + """ + Returns whether this module has support for responsive UI. + """ + return self.lcp.has_responsive_ui + def index_dictionary(self): """ Return dictionary prepared with module content and type for indexing. diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index 81b92f5fb6..8dcedfcc52 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -95,6 +95,7 @@ class HtmlDescriptor(HtmlFields, XmlDescriptor, EditingDescriptor): # pylint: d module_class = HtmlModule filename_extension = "xml" template_dir_name = "html" + has_responsive_ui = True js = {'coffee': [resource_string(__name__, 'js/src/html/edit.coffee')]} js_module_name = "HTMLEditingDescriptor"