Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions cms/djangoapps/contentstore/tests/test_item.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,20 @@

import json
import datetime
import ddt

from mock import Mock, patch
from pytz import UTC
from webob import Response

from django.http import Http404
from django.test import TestCase
from django.test.client import RequestFactory

from contentstore.views.component import component_handler

from contentstore.tests.utils import CourseTestCase
from student.tests.factories import UserFactory
from xmodule.capa_module import CapaDescriptor
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.django import loc_mapper
Expand Down Expand Up @@ -354,3 +365,53 @@ def test_create_draft_with_update(self):
self.assertIsNone(published.due)
draft = self.get_item_from_modulestore(self.problem_locator, True)
self.assertEqual(draft.due, datetime.datetime(2077, 10, 10, 4, 0, tzinfo=UTC))


@ddt.ddt
class TestComponentHandler(TestCase):
def setUp(self):
self.request_factory = RequestFactory()

patcher = patch('contentstore.views.component.modulestore')
self.modulestore = patcher.start()
self.addCleanup(patcher.stop)

self.descriptor = self.modulestore.return_value.get_item.return_value

self.usage_id = 'dummy_usage_id'

self.user = UserFactory()

self.request = self.request_factory.get('/dummy-url')
self.request.user = self.user

def test_invalid_handler(self):
self.descriptor.handle.side_effect = Http404

with self.assertRaises(Http404):
component_handler(self.request, self.usage_id, 'invalid_handler')

@ddt.data('GET', 'POST', 'PUT', 'DELETE')
def test_request_method(self, method):

def check_handler(handler, request, suffix):
self.assertEquals(request.method, method)
return Response()

self.descriptor.handle = check_handler

# Have to use the right method to create the request to get the HTTP method that we want
req_factory_method = getattr(self.request_factory, method.lower())
request = req_factory_method('/dummy-url')
request.user = self.user

component_handler(request, self.usage_id, 'dummy_handler')

@ddt.data(200, 404, 500)
def test_response_code(self, status_code):
def create_response(handler, request, suffix):
return Response(status_code=status_code)

self.descriptor.handle = create_response

self.assertEquals(component_handler(self.request, self.usage_id, 'dummy_handler').status_code, status_code)
52 changes: 44 additions & 8 deletions cms/djangoapps/contentstore/views/component.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@
import logging
from collections import defaultdict

from django.http import HttpResponseBadRequest
from django.http import HttpResponseBadRequest, Http404
from django.contrib.auth.decorators import login_required
from django.views.decorators.http import require_http_methods
from django.core.exceptions import PermissionDenied
from django_future.csrf import ensure_csrf_cookie
from django.conf import settings
from xmodule.modulestore.exceptions import ItemNotFoundError
from edxmako.shortcuts import render_to_response
Expand All @@ -15,23 +14,27 @@
from xmodule.util.date_utils import get_default_time_display
from xmodule.modulestore.django import loc_mapper
from xmodule.modulestore.locator import BlockUsageLocator
from xmodule.x_module import XModuleDescriptor

from xblock.django.request import webob_to_django_response, django_to_webob_request
from xblock.exceptions import NoSuchHandlerError
from xblock.fields import Scope
from util.json_request import expect_json, JsonResponse
from xblock.plugin import PluginMissingError
from xblock.runtime import Mixologist

from contentstore.utils import get_lms_link_for_item, compute_unit_state, UnitState, get_course_for_item
from lms.lib.xblock.runtime import unquote_slashes

from contentstore.utils import get_lms_link_for_item, compute_unit_state, UnitState

from models.settings.course_grading import CourseGradingModel

from .access import has_access
from xmodule.x_module import XModuleDescriptor
from xblock.plugin import PluginMissingError
from xblock.runtime import Mixologist

__all__ = ['OPEN_ENDED_COMPONENT_TYPES',
'ADVANCED_COMPONENT_POLICY_KEY',
'subsection_handler',
'unit_handler'
'unit_handler',
'component_handler'
]

log = logging.getLogger(__name__)
Expand Down Expand Up @@ -311,3 +314,36 @@ def _get_item_in_course(request, locator):
lms_link = get_lms_link_for_item(old_location, course_id=course.location.course_id)

return old_location, course, item, lms_link


@login_required
def component_handler(request, usage_id, handler, suffix=''):
"""
Dispatch an AJAX action to an xblock

Args:
usage_id: The usage-id of the block to dispatch to, passed through `quote_slashes`
handler (str): The handler to execute
suffix (str): The remainder of the url to be passed to the handler

Returns:
:class:`django.http.HttpResponse`: The response from the handler, converted to a
django response
"""

location = unquote_slashes(usage_id)

descriptor = modulestore().get_item(location)
# Let the module handle the AJAX
req = django_to_webob_request(request)

try:
resp = descriptor.handle(handler, req, suffix)

except NoSuchHandlerError:
log.info("XBlock %s attempted to access missing handler %r", descriptor, handler, exc_info=True)
raise Http404

modulestore().save_xmodule(descriptor)

return webob_to_django_response(resp)
2 changes: 1 addition & 1 deletion cms/djangoapps/models/settings/course_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from contentstore.utils import get_modulestore
from xmodule.modulestore.inheritance import own_metadata
from cms.xmodule_namespace import CmsBlockMixin
from cms.lib.xblock.mixin import CmsBlockMixin


class CourseMetadata(object):
Expand Down
2 changes: 1 addition & 1 deletion cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
from path import path

from lms.lib.xblock.mixin import LmsBlockMixin
from cms.xmodule_namespace import CmsBlockMixin
from cms.lib.xblock.mixin import CmsBlockMixin
from xmodule.modulestore.inheritance import InheritanceMixin
from xmodule.x_module import XModuleMixin
from dealer.git import git
Expand Down
Empty file added cms/lib/__init__.py
Empty file.
Empty file added cms/lib/xblock/__init__.py
Empty file.
2 changes: 1 addition & 1 deletion cms/xmodule_namespace.py → cms/lib/xblock/mixin.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
"""
Namespace defining common fields used by Studio for all blocks
Mixin defining common Studio functionality
"""

import datetime
Expand Down
30 changes: 30 additions & 0 deletions cms/lib/xblock/runtime.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
"""
XBlock runtime implementations for edX Studio
"""

from django.core.urlresolvers import reverse

import xmodule.x_module
from lms.lib.xblock.runtime import quote_slashes


def handler_url(block, handler_name, suffix='', query='', thirdparty=False):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thirdparty is unused. Is that OK? Is it because the way these URLs are made they are always ok for third-party, or because we don't need third-party URLs in Studio?

"""
Handler URL function for Studio
"""

if thirdparty:
raise NotImplementedError("edX Studio doesn't support third-party xblock handler urls")

url = reverse('component_handler', kwargs={
'usage_id': quote_slashes(str(block.scope_ids.usage_id)),
'handler': handler_name,
'suffix': suffix,
}).rstrip('/')

if query:
url += '?' + query

return url

xmodule.x_module.descriptor_global_handler_url = handler_url
51 changes: 51 additions & 0 deletions cms/lib/xblock/test/test_runtime.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
"""
Tests of edX Studio runtime functionality
"""
from urlparse import urlparse

from mock import Mock
from unittest import TestCase
from cms.lib.xblock.runtime import handler_url


class TestHandlerUrl(TestCase):
"""Test the LMS handler_url"""

def setUp(self):
self.block = Mock()
self.course_id = "org/course/run"

def test_trailing_charecters(self):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test just tests what is not returned... it seems like there should also be some tests verifying that what is returned is correct, as well.

self.assertFalse(handler_url(self.block, 'handler').endswith('?'))
self.assertFalse(handler_url(self.block, 'handler').endswith('/'))

self.assertFalse(handler_url(self.block, 'handler', 'suffix').endswith('?'))
self.assertFalse(handler_url(self.block, 'handler', 'suffix').endswith('/'))

self.assertFalse(handler_url(self.block, 'handler', 'suffix', 'query').endswith('?'))
self.assertFalse(handler_url(self.block, 'handler', 'suffix', 'query').endswith('/'))

self.assertFalse(handler_url(self.block, 'handler', query='query').endswith('?'))
self.assertFalse(handler_url(self.block, 'handler', query='query').endswith('/'))

def _parsed_query(self, query_string):
"""Return the parsed query string from a handler_url generated with the supplied query_string"""
return urlparse(handler_url(self.block, 'handler', query=query_string)).query

def test_query_string(self):
self.assertIn('foo=bar', self._parsed_query('foo=bar'))
self.assertIn('foo=bar&baz=true', self._parsed_query('foo=bar&baz=true'))
self.assertIn('foo&bar&baz', self._parsed_query('foo&bar&baz'))

def _parsed_path(self, handler_name='handler', suffix=''):
"""Return the parsed path from a handler_url with the supplied handler_name and suffix"""
return urlparse(handler_url(self.block, handler_name, suffix=suffix)).path

def test_suffix(self):
self.assertTrue(self._parsed_path(suffix="foo").endswith('foo'))
self.assertTrue(self._parsed_path(suffix="foo/bar").endswith('foo/bar'))
self.assertTrue(self._parsed_path(suffix="/foo/bar").endswith('/foo/bar'))

def test_handler_name(self):
self.assertIn('handler1', self._parsed_path('handler1'))
self.assertIn('handler_a', self._parsed_path('handler_a'))
5 changes: 4 additions & 1 deletion cms/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@
url(r'^transcripts/rename$', 'contentstore.views.rename_transcripts', name='rename_transcripts'),
url(r'^transcripts/save$', 'contentstore.views.save_transcripts', name='save_transcripts'),

url(r'^preview/xblock/(?P<usage_id>.*?)/handler/(?P<handler>[^/]*)(?:/(?P<suffix>[^/]*))?$',
url(r'^preview/xblock/(?P<usage_id>.*?)/handler/(?P<handler>[^/]*)(?:/(?P<suffix>.*))?$',
'contentstore.views.preview_handler', name='preview_handler'),

url(r'^xblock/(?P<usage_id>.*?)/handler/(?P<handler>[^/]*)(?:/(?P<suffix>.*))?$',
'contentstore.views.component_handler', name='component_handler'),

# temporary landing page for a course
url(r'^edge/(?P<org>[^/]+)/(?P<course>[^/]+)/course/(?P<coursename>[^/]+)$',
'contentstore.views.landing', name='landing'),
Expand Down
10 changes: 8 additions & 2 deletions common/lib/xmodule/xmodule/tests/test_error_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from xblock.runtime import Runtime, UsageStore
from xblock.field_data import FieldData
from xblock.fields import ScopeIds
from xblock.test.tools import unabc


class SetupTestErrorModules():
Expand Down Expand Up @@ -103,6 +104,11 @@ class TestException(Exception):
pass


@unabc("Tests should not call {}")
class TestRuntime(Runtime):
pass


class TestErrorModuleConstruction(unittest.TestCase):
"""
Test that error module construction happens correctly
Expand All @@ -111,11 +117,11 @@ class TestErrorModuleConstruction(unittest.TestCase):
def setUp(self):
field_data = Mock(spec=FieldData)
self.descriptor = BrokenDescriptor(
Runtime(Mock(spec=UsageStore), field_data),
TestRuntime(Mock(spec=UsageStore), field_data),
field_data,
ScopeIds(None, None, None, 'i4x://org/course/broken/name')
)
self.descriptor.xmodule_runtime = Runtime(Mock(spec=UsageStore), field_data)
self.descriptor.xmodule_runtime = TestRuntime(Mock(spec=UsageStore), field_data)
self.descriptor.xmodule_runtime.error_descriptor_class = ErrorDescriptor
self.descriptor.xmodule_runtime.xmodule_instance = None

Expand Down
37 changes: 35 additions & 2 deletions common/lib/xmodule/xmodule/x_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -841,6 +841,13 @@ def wrap_child(self, block, view, frag, context):
return frag


# This function exists to give applications (LMS/CMS) a place to monkey-patch until
# we can refactor modulestore to split out the FieldData half of its interface from
# the Runtime part of its interface. This function matches the Runtime.handler_url interface
def descriptor_global_handler_url(block, handler_name, suffix='', query='', thirdparty=False):
raise NotImplementedError("Applications must monkey-patch this function before using handler-urls for studio_view")


class DescriptorSystem(ConfigurableFragmentWrapper, Runtime): # pylint: disable=abstract-method
"""
Base class for :class:`Runtime`s to be used with :class:`XModuleDescriptor`s
Expand Down Expand Up @@ -891,9 +898,9 @@ def __init__(self, load_item, resources_fs, error_tracker, **kwargs):
self.resources_fs = resources_fs
self.error_tracker = error_tracker

def get_block(self, block_id):
def get_block(self, usage_id):
"""See documentation for `xblock.runtime:Runtime.get_block`"""
return self.load_item(block_id)
return self.load_item(usage_id)

def get_field_provenance(self, xblock, field):
"""
Expand Down Expand Up @@ -933,6 +940,23 @@ def render(self, block, view_name, context=None):
else:
return super(DescriptorSystem, self).render(block, view_name, context)

def handler_url(self, block, handler_name, suffix='', query='', thirdparty=False):
xmodule_runtime = getattr(block, 'xmodule_runtime', None)
if xmodule_runtime is not None:
return xmodule_runtime.handler_url(block, handler_name, suffix, query, thirdparty)
else:
# Currently, Modulestore is responsible for instantiating DescriptorSystems
# This means that LMS/CMS don't have a way to define a subclass of DescriptorSystem
# that implements the correct handler url. So, for now, instead, we will reference a
# global function that the application can override.
return descriptor_global_handler_url(block, handler_name, suffix, query, thirdparty)

def resources_url(self, resource):
raise NotImplementedError("edX Platform doesn't currently implement XBlock resource urls")

def local_resource_url(self, block, uri):
raise NotImplementedError("edX Platform doesn't currently implement XBlock resource urls")


class XMLParsingSystem(DescriptorSystem):
def __init__(self, process_xml, policy, **kwargs):
Expand Down Expand Up @@ -1079,6 +1103,15 @@ def ajax_url(self):
assert self.xmodule_instance is not None
return self.handler_url(self.xmodule_instance, 'xmodule_handler', '', '').rstrip('/?')

def get_block(self, block_id):
raise NotImplementedError("XModules must use get_module to load other modules")

def resources_url(self, resource):
raise NotImplementedError("edX Platform doesn't currently implement XBlock resource urls")

def local_resource_url(self, block, uri):
raise NotImplementedError("edX Platform doesn't currently implement XBlock resource urls")


class DoNothingCache(object):
"""A duck-compatible object to use in ModuleSystem when there's no cache."""
Expand Down
2 changes: 1 addition & 1 deletion rakelib/tests.rake
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def run_tests(system, report_dir, test_id=nil, stop_on_failure=true)

default_test_id = "#{system}/djangoapps common/djangoapps"

if system == :lms
if system == :lms || system == :cms
default_test_id += " #{system}/lib"
end

Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/github.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
-e git+https://github.com/eventbrite/zendesk.git@d53fe0e81b623f084e91776bcf6369f8b7b63879#egg=zendesk

# Our libraries:
-e git+https://github.com/edx/XBlock.git@c54c63cf8294c54512887e6232d4274003afc6e3#egg=XBlock
-e git+https://github.com/edx/XBlock.git@fa88607#egg=XBlock
-e git+https://github.com/edx/codejail.git@0a1b468#egg=codejail
-e git+https://github.com/edx/diff-cover.git@v0.2.6#egg=diff_cover
-e git+https://github.com/edx/js-test-tool.git@v0.1.4#egg=js_test_tool
Expand Down