From 71c447768db164e2d7f089fb48b4c3405eb2710d Mon Sep 17 00:00:00 2001 From: John Jarvis Date: Tue, 13 May 2014 17:47:52 -0400 Subject: [PATCH 01/62] returning enrollment mode for both active and inactive users --- common/djangoapps/student/models.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 9d8ae8accd..c6e97efe75 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -871,10 +871,14 @@ class CourseEnrollment(models.Model): `user` is a Django User object `course_id` is our usual course_id string (e.g. "edX/Test101/2013_Fall) + + Returns the mode for both inactive and active users. + Returns None if the courseenrollment record does not exist. """ try: record = CourseEnrollment.objects.get(user=user, course_id=course_id) - if record.is_active: + + if hasattr(record, 'mode'): return record.mode else: return None From e911d579112f721d8911a17a513837589e531fb3 Mon Sep 17 00:00:00 2001 From: lduarte1991 Date: Thu, 15 May 2014 15:23:28 -0400 Subject: [PATCH 02/62] Revert "Revert pull request #3466" This reverts commit 59e3cae4c9c6f2c95a816a8280b470ec6add9486. --- cms/envs/common.py | 2 +- .../student/firebase_token_generator.py | 99 -------------- .../student/tests/test_token_generator.py | 43 ------ common/djangoapps/student/tests/tests.py | 25 +--- common/djangoapps/student/views.py | 24 ---- common/lib/xmodule/xmodule/annotator_token.py | 32 +++++ .../xmodule/tests/test_annotator_token.py | 20 +++ .../xmodule/tests/test_textannotation.py | 13 +- .../xmodule/tests/test_videoannotation.py | 98 +------------- .../xmodule/xmodule/textannotation_module.py | 27 ++-- .../xmodule/xmodule/videoannotation_module.py | 82 ++---------- .../ova/annotator-full-firebase-auth.js | 22 +++ lms/djangoapps/notes/views.py | 4 +- lms/envs/common.py | 1 + lms/templates/notes.html | 6 +- lms/templates/textannotation.html | 126 +++++++++--------- lms/templates/videoannotation.html | 14 +- lms/urls.py | 1 - requirements/edx/base.txt | 1 + 19 files changed, 176 insertions(+), 464 deletions(-) delete mode 100644 common/djangoapps/student/firebase_token_generator.py delete mode 100644 common/djangoapps/student/tests/test_token_generator.py create mode 100644 common/lib/xmodule/xmodule/annotator_token.py create mode 100644 common/lib/xmodule/xmodule/tests/test_annotator_token.py create mode 100644 common/static/js/vendor/ova/annotator-full-firebase-auth.js diff --git a/cms/envs/common.py b/cms/envs/common.py index f7195d2c54..76e0f4d50f 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -318,7 +318,7 @@ PIPELINE_CSS = { 'css/vendor/ui-lightness/jquery-ui-1.8.22.custom.css', 'css/vendor/jquery.qtip.min.css', 'js/vendor/markitup/skins/simple/style.css', - 'js/vendor/markitup/sets/wiki/style.css' + 'js/vendor/markitup/sets/wiki/style.css', ], 'output_filename': 'css/cms-style-vendor.css', }, diff --git a/common/djangoapps/student/firebase_token_generator.py b/common/djangoapps/student/firebase_token_generator.py deleted file mode 100644 index f84a85277e..0000000000 --- a/common/djangoapps/student/firebase_token_generator.py +++ /dev/null @@ -1,99 +0,0 @@ -''' - Firebase - library to generate a token - License: https://github.com/firebase/firebase-token-generator-python/blob/master/LICENSE - Tweaked and Edited by @danielcebrianr and @lduarte1991 - - This library will take either objects or strings and use python's built-in encoding - system as specified by RFC 3548. Thanks to the firebase team for their open-source - library. This was made specifically for speaking with the annotation_storage_url and - can be used and expanded, but not modified by anyone else needing such a process. -''' -from base64 import urlsafe_b64encode -import hashlib -import hmac -import sys -try: - import json -except ImportError: - import simplejson as json - -__all__ = ['create_token'] - -TOKEN_SEP = '.' - - -def create_token(secret, data): - ''' - Simply takes in the secret key and the data and - passes it to the local function _encode_token - ''' - return _encode_token(secret, data) - - -if sys.version_info < (2, 7): - def _encode(bytes_data): - ''' - Takes a json object, string, or binary and - uses python's urlsafe_b64encode to encode data - and make it safe pass along in a url. - To make sure it does not conflict with variables - we make sure equal signs are removed. - More info: docs.python.org/2/library/base64.html - ''' - encoded = urlsafe_b64encode(bytes(bytes_data)) - return encoded.decode('utf-8').replace('=', '') -else: - def _encode(bytes_info): - ''' - Same as above function but for Python 2.7 or later - ''' - encoded = urlsafe_b64encode(bytes_info) - return encoded.decode('utf-8').replace('=', '') - - -def _encode_json(obj): - ''' - Before a python dict object can be properly encoded, - it must be transformed into a jason object and then - transformed into bytes to be encoded using the function - defined above. - ''' - return _encode(bytearray(json.dumps(obj), 'utf-8')) - - -def _sign(secret, to_sign): - ''' - This function creates a sign that goes at the end of the - message that is specific to the secret and not the actual - content of the encoded body. - More info on hashing: http://docs.python.org/2/library/hmac.html - The function creates a hashed values of the secret and to_sign - and returns the digested values based the secure hash - algorithm, 256 - ''' - def portable_bytes(string): - ''' - Simply transforms a string into a bytes object, - which is a series of immutable integers 0<=x<=256. - Always try to encode as utf-8, unless it is not - compliant. - ''' - try: - return bytes(string, 'utf-8') - except TypeError: - return bytes(string) - return _encode(hmac.new(portable_bytes(secret), portable_bytes(to_sign), hashlib.sha256).digest()) # pylint: disable=E1101 - - -def _encode_token(secret, claims): - ''' - This is the main function that takes the secret token and - the data to be transmitted. There is a header created for decoding - purposes. Token_SEP means that a period/full stop separates the - header, data object/message, and signatures. - ''' - encoded_header = _encode_json({'typ': 'JWT', 'alg': 'HS256'}) - encoded_claims = _encode_json(claims) - secure_bits = '%s%s%s' % (encoded_header, TOKEN_SEP, encoded_claims) - sig = _sign(secret, secure_bits) - return '%s%s%s' % (secure_bits, TOKEN_SEP, sig) diff --git a/common/djangoapps/student/tests/test_token_generator.py b/common/djangoapps/student/tests/test_token_generator.py deleted file mode 100644 index 1eb09c9173..0000000000 --- a/common/djangoapps/student/tests/test_token_generator.py +++ /dev/null @@ -1,43 +0,0 @@ -""" -This test will run for firebase_token_generator.py. -""" - -from django.test import TestCase - -from student.firebase_token_generator import _encode, _encode_json, _encode_token, create_token - - -class TokenGenerator(TestCase): - """ - Tests for the file firebase_token_generator.py - """ - def test_encode(self): - """ - This tests makes sure that no matter what version of python - you have, the _encode function still returns the appropriate result - for a string. - """ - expected = "dGVzdDE" - result = _encode("test1") - self.assertEqual(expected, result) - - def test_encode_json(self): - """ - Same as above, but this one focuses on a python dict type - transformed into a json object and then encoded. - """ - expected = "eyJ0d28iOiAidGVzdDIiLCAib25lIjogInRlc3QxIn0" - result = _encode_json({'one': 'test1', 'two': 'test2'}) - self.assertEqual(expected, result) - - def test_create_token(self): - """ - Unlike its counterpart in student/views.py, this function - just checks for the encoding of a token. The other function - will test depending on time and user. - """ - expected = "eyJhbGciOiAiSFMyNTYiLCAidHlwIjogIkpXVCJ9.eyJ1c2VySWQiOiAidXNlcm5hbWUiLCAidHRsIjogODY0MDB9.-p1sr7uwCapidTQ0qB7DdU2dbF-hViKpPNN_5vD10t8" - result1 = _encode_token('4c7f4d1c-8ac4-4e9f-84c8-b271c57fcac4', {"userId": "username", "ttl": 86400}) - result2 = create_token('4c7f4d1c-8ac4-4e9f-84c8-b271c57fcac4', {"userId": "username", "ttl": 86400}) - self.assertEqual(expected, result1) - self.assertEqual(expected, result2) diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index c28a54afe8..199a794bc4 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -26,7 +26,7 @@ from mock import Mock, patch from student.models import anonymous_id_for_user, user_by_anonymous_id, CourseEnrollment, unique_id_for_user from student.views import (process_survey_link, _cert_info, - change_enrollment, complete_course_mode_info, token) + change_enrollment, complete_course_mode_info) from student.tests.factories import UserFactory, CourseModeFactory import shoppingcart @@ -498,26 +498,3 @@ class AnonymousLookupTable(TestCase): anonymous_id = anonymous_id_for_user(self.user, self.course.id) real_user = user_by_anonymous_id(anonymous_id) self.assertEqual(self.user, real_user) - - -@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) -class Token(ModuleStoreTestCase): - """ - Test for the token generator. This creates a random course and passes it through the token file which generates the - token that will be passed in to the annotation_storage_url. - """ - request_factory = RequestFactory() - COURSE_SLUG = "100" - COURSE_NAME = "test_course" - COURSE_ORG = "edx" - - def setUp(self): - self.course = CourseFactory.create(org=self.COURSE_ORG, display_name=self.COURSE_NAME, number=self.COURSE_SLUG) - self.user = User.objects.create(username="username", email="username") - self.req = self.request_factory.post('/token?course_id=edx/100/test_course', {'user': self.user}) - self.req.user = self.user - - def test_token(self): - expected = HttpResponse("eyJhbGciOiAiSFMyNTYiLCAidHlwIjogIkpXVCJ9.eyJpc3N1ZWRBdCI6ICIyMDE0LTAxLTIzVDE5OjM1OjE3LjUyMjEwNC01OjAwIiwgImNvbnN1bWVyS2V5IjogInh4eHh4eHh4LXh4eHgteHh4eC14eHh4LXh4eHh4eHh4eHh4eCIsICJ1c2VySWQiOiAidXNlcm5hbWUiLCAidHRsIjogODY0MDB9.OjWz9mzqJnYuzX-f3uCBllqJUa8PVWJjcDy_McfxLvc", mimetype="text/plain") - response = token(self.req) - self.assertEqual(expected.content.split('.')[0], response.content.split('.')[0]) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 5cecaff5df..b199196679 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -44,7 +44,6 @@ from student.models import ( create_comments_service_user, PasswordHistory ) from student.forms import PasswordResetFormNoActive -from student.firebase_token_generator import create_token from verify_student.models import SoftwareSecurePhotoVerification, MidcourseReverificationWindow from certificates.models import CertificateStatuses, certificate_status_for_student @@ -1852,26 +1851,3 @@ def change_email_settings(request): track.views.server_track(request, "change-email-settings", {"receive_emails": "no", "course": course_id}, page='dashboard') return JsonResponse({"success": True}) - - -@login_required -def token(request): - ''' - Return a token for the backend of annotations. - It uses the course id to retrieve a variable that contains the secret - token found in inheritance.py. It also contains information of when - the token was issued. This will be stored with the user along with - the id for identification purposes in the backend. - ''' - course_id = request.GET.get("course_id") - course = course_from_id(course_id) - dtnow = datetime.datetime.now() - dtutcnow = datetime.datetime.utcnow() - delta = dtnow - dtutcnow - newhour, newmin = divmod((delta.days * 24 * 60 * 60 + delta.seconds + 30) // 60, 60) - newtime = "%s%+02d:%02d" % (dtnow.isoformat(), newhour, newmin) - secret = course.annotation_token_secret - custom_data = {"issuedAt": newtime, "consumerKey": secret, "userId": request.user.email, "ttl": 86400} - newtoken = create_token(secret, custom_data) - response = HttpResponse(newtoken, mimetype="text/plain") - return response diff --git a/common/lib/xmodule/xmodule/annotator_token.py b/common/lib/xmodule/xmodule/annotator_token.py new file mode 100644 index 0000000000..6fa5695978 --- /dev/null +++ b/common/lib/xmodule/xmodule/annotator_token.py @@ -0,0 +1,32 @@ +""" +This file contains a function used to retrieve the token for the annotation backend +without having to create a view, but just returning a string instead. + +It can be called from other files by using the following: +from xmodule.annotator_token import retrieve_token +""" +import datetime +from firebase_token_generator import create_token + + +def retrieve_token(userid, secret): + ''' + Return a token for the backend of annotations. + It uses the course id to retrieve a variable that contains the secret + token found in inheritance.py. It also contains information of when + the token was issued. This will be stored with the user along with + the id for identification purposes in the backend. + ''' + + # the following five lines of code allows you to include the default timezone in the iso format + # for more information: http://stackoverflow.com/questions/3401428/how-to-get-an-isoformat-datetime-string-including-the-default-timezone + dtnow = datetime.datetime.now() + dtutcnow = datetime.datetime.utcnow() + delta = dtnow - dtutcnow + newhour, newmin = divmod((delta.days * 24 * 60 * 60 + delta.seconds + 30) // 60, 60) + newtime = "%s%+02d:%02d" % (dtnow.isoformat(), newhour, newmin) + # uses the issued time (UTC plus timezone), the consumer key and the user's email to maintain a + # federated system in the annotation backend server + custom_data = {"issuedAt": newtime, "consumerKey": secret, "userId": userid, "ttl": 86400} + newtoken = create_token(secret, custom_data) + return newtoken diff --git a/common/lib/xmodule/xmodule/tests/test_annotator_token.py b/common/lib/xmodule/xmodule/tests/test_annotator_token.py new file mode 100644 index 0000000000..ae06808bba --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/test_annotator_token.py @@ -0,0 +1,20 @@ +""" +This test will run for annotator_token.py +""" +import unittest + +from xmodule.annotator_token import retrieve_token + + +class TokenRetriever(unittest.TestCase): + """ + Tests to make sure that when passed in a username and secret token, that it will be encoded correctly + """ + def test_token(self): + """ + Test for the token generator. Give an a random username and secret token, it should create the properly encoded string of text. + """ + expected = "eyJhbGciOiAiSFMyNTYiLCAidHlwIjogIkpXVCJ9.eyJpc3N1ZWRBdCI6ICIyMDE0LTAyLTI3VDE3OjAwOjQyLjQwNjQ0MSswOjAwIiwgImNvbnN1bWVyS2V5IjogImZha2Vfc2VjcmV0IiwgInVzZXJJZCI6ICJ1c2VybmFtZSIsICJ0dGwiOiA4NjQwMH0.Dx1PoF-7mqBOOSGDMZ9R_s3oaaLRPnn6CJgGGF2A5CQ" + response = retrieve_token("username", "fake_secret") + self.assertEqual(expected.split('.')[0], response.split('.')[0]) + self.assertNotEqual(expected.split('.')[2], response.split('.')[2]) \ No newline at end of file diff --git a/common/lib/xmodule/xmodule/tests/test_textannotation.py b/common/lib/xmodule/xmodule/tests/test_textannotation.py index 397e3990ef..907eb78780 100644 --- a/common/lib/xmodule/xmodule/tests/test_textannotation.py +++ b/common/lib/xmodule/xmodule/tests/test_textannotation.py @@ -38,17 +38,6 @@ class TextAnnotationModuleTestCase(unittest.TestCase): ScopeIds(None, None, None, None) ) - def test_render_content(self): - """ - Tests to make sure the sample xml is rendered and that it forms a valid xmltree - that does not contain a display_name. - """ - content = self.mod._render_content() # pylint: disable=W0212 - self.assertIsNotNone(content) - element = etree.fromstring(content) - self.assertIsNotNone(element) - self.assertFalse('display_name' in element.attrib, "Display Name should have been deleted from Content") - def test_extract_instructions(self): """ Tests to make sure that the instructions are correctly pulled from the sample xml above. @@ -70,5 +59,5 @@ class TextAnnotationModuleTestCase(unittest.TestCase): Tests the function that passes in all the information in the context that will be used in templates/textannotation.html """ context = self.mod.get_html() - for key in ['display_name', 'tag', 'source', 'instructions_html', 'content_html', 'annotation_storage']: + for key in ['display_name', 'tag', 'source', 'instructions_html', 'content_html', 'annotation_storage', 'token']: self.assertIn(key, context) diff --git a/common/lib/xmodule/xmodule/tests/test_videoannotation.py b/common/lib/xmodule/xmodule/tests/test_videoannotation.py index cb63d05503..4a081803aa 100644 --- a/common/lib/xmodule/xmodule/tests/test_videoannotation.py +++ b/common/lib/xmodule/xmodule/tests/test_videoannotation.py @@ -34,100 +34,6 @@ class VideoAnnotationModuleTestCase(unittest.TestCase): ScopeIds(None, None, None, None) ) - def test_annotation_class_attr_default(self): - """ - Makes sure that it can detect annotation values in text-form if user - decides to add text to the area below video, video functionality is completely - found in javascript. - """ - xml = 'test' - element = etree.fromstring(xml) - - expected_attr = {'class': {'value': 'annotatable-span highlight'}} - actual_attr = self.mod._get_annotation_class_attr(element) # pylint: disable=W0212 - - self.assertIsInstance(actual_attr, dict) - self.assertDictEqual(expected_attr, actual_attr) - - def test_annotation_class_attr_with_valid_highlight(self): - """ - Same as above but more specific to an area that is highlightable in the appropriate - color designated. - """ - xml = 'test' - - for color in self.mod.highlight_colors: - element = etree.fromstring(xml.format(highlight=color)) - value = 'annotatable-span highlight highlight-{highlight}'.format(highlight=color) - - expected_attr = {'class': { - 'value': value, - '_delete': 'highlight'} - } - actual_attr = self.mod._get_annotation_class_attr(element) # pylint: disable=W0212 - - self.assertIsInstance(actual_attr, dict) - self.assertDictEqual(expected_attr, actual_attr) - - def test_annotation_class_attr_with_invalid_highlight(self): - """ - Same as above, but checked with invalid colors. - """ - xml = 'test' - - for invalid_color in ['rainbow', 'blink', 'invisible', '', None]: - element = etree.fromstring(xml.format(highlight=invalid_color)) - expected_attr = {'class': { - 'value': 'annotatable-span highlight', - '_delete': 'highlight'} - } - actual_attr = self.mod._get_annotation_class_attr(element) # pylint: disable=W0212 - - self.assertIsInstance(actual_attr, dict) - self.assertDictEqual(expected_attr, actual_attr) - - def test_annotation_data_attr(self): - """ - Test that each highlight contains the data information from the annotation itself. - """ - element = etree.fromstring('test') - - expected_attr = { - 'data-comment-body': {'value': 'foo', '_delete': 'body'}, - 'data-comment-title': {'value': 'bar', '_delete': 'title'}, - 'data-problem-id': {'value': '0', '_delete': 'problem'} - } - - actual_attr = self.mod._get_annotation_data_attr(element) # pylint: disable=W0212 - - self.assertIsInstance(actual_attr, dict) - self.assertDictEqual(expected_attr, actual_attr) - - def test_render_annotation(self): - """ - Tests to make sure that the spans designating annotations acutally visually render as annotations. - """ - expected_html = 'z' - expected_el = etree.fromstring(expected_html) - - actual_el = etree.fromstring('z') - self.mod._render_annotation(actual_el) # pylint: disable=W0212 - - self.assertEqual(expected_el.tag, actual_el.tag) - self.assertEqual(expected_el.text, actual_el.text) - self.assertDictEqual(dict(expected_el.attrib), dict(actual_el.attrib)) - - def test_render_content(self): - """ - Like above, but using the entire text, it makes sure that display_name is removed and that there is only one - div encompassing the annotatable area. - """ - content = self.mod._render_content() # pylint: disable=W0212 - element = etree.fromstring(content) - self.assertIsNotNone(element) - self.assertEqual('div', element.tag, 'root tag is a div') - self.assertFalse('display_name' in element.attrib, "Display Name should have been deleted from Content") - def test_extract_instructions(self): """ This test ensures that if an instruction exists it is pulled and @@ -160,6 +66,6 @@ class VideoAnnotationModuleTestCase(unittest.TestCase): """ Tests to make sure variables passed in truly exist within the html once it is all rendered. """ - context = self.mod.get_html() - for key in ['display_name', 'content_html', 'instructions_html', 'sourceUrl', 'typeSource', 'poster', 'alert', 'annotation_storage']: + context = self.mod.get_html() # pylint: disable=W0212 + for key in ['display_name', 'instructions_html', 'sourceUrl', 'typeSource', 'poster', 'annotation_storage']: self.assertIn(key, context) diff --git a/common/lib/xmodule/xmodule/textannotation_module.py b/common/lib/xmodule/xmodule/textannotation_module.py index 1d732d8709..4a673eb33e 100644 --- a/common/lib/xmodule/xmodule/textannotation_module.py +++ b/common/lib/xmodule/xmodule/textannotation_module.py @@ -6,6 +6,7 @@ from pkg_resources import resource_string from xmodule.x_module import XModule from xmodule.raw_module import RawDescriptor from xblock.core import Scope, String +from xmodule.annotator_token import retrieve_token import textwrap @@ -30,7 +31,7 @@ class AnnotatableFields(object): scope=Scope.settings, default='Text Annotation', ) - tags = String( + instructor_tags = String( display_name="Tags for Assignments", help="Add tags that automatically highlight in a certain color using the comma-separated form, i.e. imagery:red,parallelism:blue", scope=Scope.settings, @@ -43,6 +44,7 @@ class AnnotatableFields(object): default='None', ) annotation_storage_url = String(help="Location of Annotation backend", scope=Scope.settings, default="http://your_annotation_storage.com", display_name="Url for Annotation Storage") + annotation_token_secret = String(help="Secret string for annotation storage", scope=Scope.settings, default="xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx", display_name="Secret Token String for Annotation") class TextAnnotationModule(AnnotatableFields, XModule): @@ -59,15 +61,9 @@ class TextAnnotationModule(AnnotatableFields, XModule): self.instructions = self._extract_instructions(xmltree) self.content = etree.tostring(xmltree, encoding='unicode') - self.highlight_colors = ['yellow', 'orange', 'purple', 'blue', 'green'] - - def _render_content(self): - """ Renders annotatable content with annotation spans and returns HTML. """ - xmltree = etree.fromstring(self.content) - if 'display_name' in xmltree.attrib: - del xmltree.attrib['display_name'] - - return etree.tostring(xmltree, encoding='unicode') + self.user_email = "" + if self.runtime.get_real_user is not None: + self.user_email = self.runtime.get_real_user(self.runtime.anonymous_student_id).email def _extract_instructions(self, xmltree): """ Removes from the xmltree and returns them as a string, otherwise None. """ @@ -82,13 +78,13 @@ class TextAnnotationModule(AnnotatableFields, XModule): """ Renders parameters to template. """ context = { 'display_name': self.display_name_with_default, - 'tag': self.tags, + 'tag': self.instructor_tags, 'source': self.source, 'instructions_html': self.instructions, - 'content_html': self._render_content(), - 'annotation_storage': self.annotation_storage_url + 'content_html': self.content, + 'annotation_storage': self.annotation_storage_url, + 'token': retrieve_token(self.user_email, self.annotation_token_secret), } - return self.system.render_template('textannotation.html', context) @@ -101,6 +97,7 @@ class TextAnnotationDescriptor(AnnotatableFields, RawDescriptor): def non_editable_metadata_fields(self): non_editable_fields = super(TextAnnotationDescriptor, self).non_editable_metadata_fields non_editable_fields.extend([ - TextAnnotationDescriptor.annotation_storage_url + TextAnnotationDescriptor.annotation_storage_url, + TextAnnotationDescriptor.annotation_token_secret, ]) return non_editable_fields diff --git a/common/lib/xmodule/xmodule/videoannotation_module.py b/common/lib/xmodule/xmodule/videoannotation_module.py index 5f31509d01..68e5b40413 100644 --- a/common/lib/xmodule/xmodule/videoannotation_module.py +++ b/common/lib/xmodule/xmodule/videoannotation_module.py @@ -7,6 +7,7 @@ from pkg_resources import resource_string from xmodule.x_module import XModule from xmodule.raw_module import RawDescriptor from xblock.core import Scope, String +from xmodule.annotator_token import retrieve_token import textwrap @@ -31,7 +32,7 @@ class AnnotatableFields(object): sourceurl = String(help="The external source URL for the video.", display_name="Source URL", scope=Scope.settings, default="http://video-js.zencoder.com/oceans-clip.mp4") poster_url = String(help="Poster Image URL", display_name="Poster URL", scope=Scope.settings, default="") annotation_storage_url = String(help="Location of Annotation backend", scope=Scope.settings, default="http://your_annotation_storage.com", display_name="Url for Annotation Storage") - + annotation_token_secret = String(help="Secret string for annotation storage", scope=Scope.settings, default="xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx", display_name="Secret Token String for Annotation") class VideoAnnotationModule(AnnotatableFields, XModule): '''Video Annotation Module''' @@ -55,73 +56,9 @@ class VideoAnnotationModule(AnnotatableFields, XModule): self.instructions = self._extract_instructions(xmltree) self.content = etree.tostring(xmltree, encoding='unicode') - self.highlight_colors = ['yellow', 'orange', 'purple', 'blue', 'green'] - - def _get_annotation_class_attr(self, element): - """ Returns a dict with the CSS class attribute to set on the annotation - and an XML key to delete from the element. - """ - - attr = {} - cls = ['annotatable-span', 'highlight'] - highlight_key = 'highlight' - color = element.get(highlight_key) - - if color is not None: - if color in self.highlight_colors: - cls.append('highlight-' + color) - attr['_delete'] = highlight_key - attr['value'] = ' '.join(cls) - - return {'class': attr} - - def _get_annotation_data_attr(self, element): - """ Returns a dict in which the keys are the HTML data attributes - to set on the annotation element. Each data attribute has a - corresponding 'value' and (optional) '_delete' key to specify - an XML attribute to delete. - """ - - data_attrs = {} - attrs_map = { - 'body': 'data-comment-body', - 'title': 'data-comment-title', - 'problem': 'data-problem-id' - } - - for xml_key in attrs_map.keys(): - if xml_key in element.attrib: - value = element.get(xml_key, '') - html_key = attrs_map[xml_key] - data_attrs[html_key] = {'value': value, '_delete': xml_key} - - return data_attrs - - def _render_annotation(self, element): - """ Renders an annotation element for HTML output. """ - attr = {} - attr.update(self._get_annotation_class_attr(element)) - attr.update(self._get_annotation_data_attr(element)) - - element.tag = 'span' - - for key in attr.keys(): - element.set(key, attr[key]['value']) - if '_delete' in attr[key] and attr[key]['_delete'] is not None: - delete_key = attr[key]['_delete'] - del element.attrib[delete_key] - - def _render_content(self): - """ Renders annotatable content with annotation spans and returns HTML. """ - xmltree = etree.fromstring(self.content) - xmltree.tag = 'div' - if 'display_name' in xmltree.attrib: - del xmltree.attrib['display_name'] - - for element in xmltree.findall('.//annotation'): - self._render_annotation(element) - - return etree.tostring(xmltree, encoding='unicode') + self.user_email = "" + if self.runtime.get_real_user is not None: + self.user_email = self.runtime.get_real_user(self.runtime.anonymous_student_id).email def _extract_instructions(self, xmltree): """ Removes from the xmltree and returns them as a string, otherwise None. """ @@ -154,9 +91,9 @@ class VideoAnnotationModule(AnnotatableFields, XModule): 'sourceUrl': self.sourceurl, 'typeSource': extension, 'poster': self.poster_url, - 'alert': self, - 'content_html': self._render_content(), - 'annotation_storage': self.annotation_storage_url + 'content_html': self.content, + 'annotation_storage': self.annotation_storage_url, + 'token': retrieve_token(self.user_email, self.annotation_token_secret), } return self.system.render_template('videoannotation.html', context) @@ -171,6 +108,7 @@ class VideoAnnotationDescriptor(AnnotatableFields, RawDescriptor): def non_editable_metadata_fields(self): non_editable_fields = super(VideoAnnotationDescriptor, self).non_editable_metadata_fields non_editable_fields.extend([ - VideoAnnotationDescriptor.annotation_storage_url + VideoAnnotationDescriptor.annotation_storage_url, + VideoAnnotationDescriptor.annotation_token_secret, ]) return non_editable_fields diff --git a/common/static/js/vendor/ova/annotator-full-firebase-auth.js b/common/static/js/vendor/ova/annotator-full-firebase-auth.js new file mode 100644 index 0000000000..defc25fc95 --- /dev/null +++ b/common/static/js/vendor/ova/annotator-full-firebase-auth.js @@ -0,0 +1,22 @@ +Annotator.Plugin.Auth.prototype.haveValidToken = function() { + return ( + this._unsafeToken && + this._unsafeToken.d.issuedAt && + this._unsafeToken.d.ttl && + this._unsafeToken.d.consumerKey && + this.timeToExpiry() > 0 + ); +}; + +Annotator.Plugin.Auth.prototype.timeToExpiry = function() { + var expiry, issue, now, timeToExpiry; + now = new Date().getTime() / 1000; + issue = createDateFromISO8601(this._unsafeToken.d.issuedAt).getTime() / 1000; + expiry = issue + this._unsafeToken.d.ttl; + timeToExpiry = expiry - now; + if (timeToExpiry > 0) { + return timeToExpiry; + } else { + return 0; + } +}; \ No newline at end of file diff --git a/lms/djangoapps/notes/views.py b/lms/djangoapps/notes/views.py index b6670a7e09..1e14fcaa25 100644 --- a/lms/djangoapps/notes/views.py +++ b/lms/djangoapps/notes/views.py @@ -4,6 +4,7 @@ from edxmako.shortcuts import render_to_response from courseware.courses import get_course_with_access from notes.models import Note from notes.utils import notes_enabled_for_course +from xmodule.annotator_token import retrieve_token @login_required @@ -22,7 +23,8 @@ def notes(request, course_id): 'course': course, 'notes': notes, 'student': student, - 'storage': storage + 'storage': storage, + 'token': retrieve_token(student.email, course.annotation_token_secret), } return render_to_response('notes.html', context) diff --git a/lms/envs/common.py b/lms/envs/common.py index 9340aecc54..0c2b37b2d5 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -828,6 +828,7 @@ main_vendor_js = [ 'js/vendor/swfobject/swfobject.js', 'js/vendor/jquery.ba-bbq.min.js', 'js/vendor/ova/annotator-full.js', + 'js/vendor/ova/annotator-full-firebase-auth.js', 'js/vendor/ova/video.dev.js', 'js/vendor/ova/vjs.youtube.js', 'js/vendor/ova/rangeslider.js', diff --git a/lms/templates/notes.html b/lms/templates/notes.html index d896725581..e44a78b08e 100644 --- a/lms/templates/notes.html +++ b/lms/templates/notes.html @@ -68,10 +68,8 @@ //Grab uri of the course var parts = window.location.href.split("/"), - uri = '', - courseid; + uri = ''; for (var index = 0; index <= 6; index += 1) uri += parts[index]+"/"; //Get the unit url - courseid = parts[4] + "/" + parts[5] + "/" + parts[6]; var pagination = 100, is_staff = false, options = { @@ -130,7 +128,7 @@ }, }, auth: { - tokenUrl: location.protocol+'//'+location.host+"/token?course_id="+courseid + token: "${token}" }, store: { // The endpoint of the store on your server. diff --git a/lms/templates/textannotation.html b/lms/templates/textannotation.html index 3532681051..f69cb7b68c 100644 --- a/lms/templates/textannotation.html +++ b/lms/templates/textannotation.html @@ -1,64 +1,63 @@ <%! from django.utils.translation import ugettext as _ %>
-
- % if display_name is not UNDEFINED and display_name is not None: -
${display_name}
- % endif -
- % if instructions_html is not UNDEFINED and instructions_html is not None: -
-
- ${_('Instructions')} - ${_('Collapse Instructions')} -
-
- ${instructions_html} -
-
- % endif -
-
-
${content_html}
-
${_('Source:')} ${source}
-
-
${_('You do not have any notes.')}
-
-
-
+
+ % if display_name is not UNDEFINED and display_name is not None: +
${display_name}
+ % endif +
+ % if instructions_html is not UNDEFINED and instructions_html is not None: +
+
+ ${_('Instructions')} + ${_('Collapse Instructions')} +
+
+ ${instructions_html} +
+
+ % endif +
+
+
${content_html}
+
${_('Source:')} ${source}
+
+
${_('You do not have any notes.')}
+
+
+
+ }); // end of require() + <%block name="content"> diff --git a/cms/templates/js/asset-upload-modal.underscore b/cms/templates/js/asset-upload-modal.underscore new file mode 100644 index 0000000000..6d352ca3cc --- /dev/null +++ b/cms/templates/js/asset-upload-modal.underscore @@ -0,0 +1,19 @@ + From 6d0e2b215c2a69cb32b3c2108bc67926394c745c Mon Sep 17 00:00:00 2001 From: zubair-arbi Date: Wed, 21 May 2014 18:31:19 +0500 Subject: [PATCH 36/62] update _xmodule_recurse method --- cms/djangoapps/contentstore/views/helpers.py | 19 +++++++------------ cms/djangoapps/contentstore/views/item.py | 6 +++--- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/cms/djangoapps/contentstore/views/helpers.py b/cms/djangoapps/contentstore/views/helpers.py index 466ed178f4..06b9258017 100644 --- a/cms/djangoapps/contentstore/views/helpers.py +++ b/cms/djangoapps/contentstore/views/helpers.py @@ -4,7 +4,6 @@ from django.http import HttpResponse from django.shortcuts import redirect from edxmako.shortcuts import render_to_string, render_to_response from xmodule.modulestore.django import loc_mapper, modulestore -from xmodule.modulestore.exceptions import DuplicateItemError, ItemNotFoundError __all__ = ['edge', 'event', 'landing'] @@ -37,24 +36,20 @@ def render_from_lms(template_name, dictionary, context=None, namespace='main'): return render_to_string(template_name, dictionary, context, namespace="lms." + namespace) -def _xmodule_recurse(item, action, ignore_exception=None): +def _xmodule_recurse(item, action, ignore_exception=()): """ Recursively apply provided action on item and its children - ignore_duplicate_exception (str): A optional argument; when set to a certain value then ignores the corresponding - exception raised while xmodule recursion + ignore_exception (Exception Object): A optional argument; when passed ignores the corresponding + exception raised during xmodule recursion, """ for child in item.get_children(): _xmodule_recurse(child, action, ignore_exception) - if ignore_exception in ['ItemNotFoundError', 'DuplicateItemError']: - # In case of valid provided exception; ignore it and continue recursion - try: - return action(item) - except (ItemNotFoundError, DuplicateItemError): - return - - action(item) + try: + return action(item) + except ignore_exception: + return def get_parent_xblock(xblock): diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 4ed5e29a75..60753f33e3 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -21,7 +21,7 @@ from xblock.fragment import Fragment import xmodule from xmodule.modulestore.django import modulestore, loc_mapper -from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError +from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError, DuplicateItemError from xmodule.modulestore.inheritance import own_metadata from xmodule.modulestore.locator import BlockUsageLocator from xmodule.modulestore import Location @@ -313,7 +313,7 @@ def _save_item(request, usage_loc, item_location, data=None, children=None, meta _xmodule_recurse( existing_item, lambda i: modulestore().unpublish(i.location), - ignore_exception='ItemNotFoundError' + ignore_exception=ItemNotFoundError ) elif publish == 'create_draft': # This recursively clones the existing item location to a draft location (the draft is @@ -321,7 +321,7 @@ def _save_item(request, usage_loc, item_location, data=None, children=None, meta _xmodule_recurse( existing_item, lambda i: modulestore().convert_to_draft(i.location), - ignore_exception='DuplicateItemError' + ignore_exception=DuplicateItemError ) if data: From dba9c4fefbe56a1785ddf06e003d4369240f4cc4 Mon Sep 17 00:00:00 2001 From: Alison Hodges Date: Wed, 21 May 2014 10:59:36 -0400 Subject: [PATCH 37/62] Anton's and Sylvia's edits --- .../internal_data_formats/change_log.rst | 2 +- .../internal_data_formats/tracking_logs.rst | 20 +++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/en_us/data/source/internal_data_formats/change_log.rst b/docs/en_us/data/source/internal_data_formats/change_log.rst index d84a7ed15e..fe3efd5672 100644 --- a/docs/en_us/data/source/internal_data_formats/change_log.rst +++ b/docs/en_us/data/source/internal_data_formats/change_log.rst @@ -11,7 +11,7 @@ Change Log * - Date - Change - * - 05/20/14 + * - 05/21/14 - Added descriptions of five video- and problem-related events to the :ref:`Tracking Logs` chapter. * - diff --git a/docs/en_us/data/source/internal_data_formats/tracking_logs.rst b/docs/en_us/data/source/internal_data_formats/tracking_logs.rst index 673c19c60c..aa27d715cc 100644 --- a/docs/en_us/data/source/internal_data_formats/tracking_logs.rst +++ b/docs/en_us/data/source/internal_data_formats/tracking_logs.rst @@ -490,8 +490,8 @@ The browser emits these events when a user works with a video. - EdX ID of the video being watched (for example, i4x-HarvardX-PH207x-video-Simple_Random_Sample). * - ``code`` - string - - The YouTube ID of the video being loaded (for example, OEyXaRPEzfM) or - 'html5' for html5 video. + - For YouTube videos, the ID of the video being loaded (for example, + OEyXaRPEzfM). For non-YouTube videos, 'html5'. * - ``currentTime`` - float - Time the video was played, in seconds. @@ -569,8 +569,8 @@ The browser emits ``load_video`` events when the video is fully rendered and re - Details * - ``code`` - string - - The YouTube ID of the video being loaded (for example, OEyXaRPEzfM) or - 'html5' for html5 video. + - For YouTube videos, the ID of the video being loaded (for example, + OEyXaRPEzfM). For non-YouTube videos, 'html5'. ``hide_transcript`` ------------------- @@ -589,11 +589,11 @@ suppress display of the video transcript. - Details * - ``code`` - string - - The YouTube ID of the video being loaded (for example, OEyXaRPEzfM) or - 'html5' for html5 video. + - For YouTube videos, the ID of the video being loaded (for example, + OEyXaRPEzfM). For non-YouTube videos, 'html5'. * - ``currentTime`` - float - - Time the video transcript was hidden, in seconds. + - The point in the video file at which the transcript was hidden, in seconds. ``show_transcript`` -------------------- @@ -611,11 +611,11 @@ display the video transcript. - Details * - ``code`` - string - - The YouTube ID of the video being loaded (for example, OEyXaRPEzfM) or - 'html5' for html5 video. + - For YouTube videos, the ID of the video being loaded (for example, + OEyXaRPEzfM). For non-YouTube videos, 'html5'. * - ``currentTime`` - float - - Time the video transcript was displayed, in seconds. + - The point in the video file at which the transcript was opened, in seconds. .. _pdf: From db239c69fe39ea757dc21115d9eeaf2a9d7e586a Mon Sep 17 00:00:00 2001 From: jmclaus Date: Mon, 28 Apr 2014 14:38:10 +0200 Subject: [PATCH 38/62] Tolerance expressed in percentage now computes correctly. [BLD-522] --- CHANGELOG.rst | 2 + .../lib/capa/capa/tests/test_responsetypes.py | 17 +++- common/lib/capa/capa/tests/test_util.py | 82 +++++++++++++++++++ common/lib/capa/capa/util.py | 54 ++++++++---- 4 files changed, 134 insertions(+), 21 deletions(-) create mode 100644 common/lib/capa/capa/tests/test_util.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 66744fa8c8..9e65666570 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,8 @@ These are notable changes in edx-platform. This is a rolling list of changes, in roughly chronological order, most recent first. Add your entries at or near the top. Include a label indicating the component affected. +Blades: Tolerance expressed in percentage now computes correctly. BLD-522. + Studio: Add drag-and-drop support to the container page. STUD-1309. Common: Add extensible third-party auth module. diff --git a/common/lib/capa/capa/tests/test_responsetypes.py b/common/lib/capa/capa/tests/test_responsetypes.py index 0057b90c3d..86a6401408 100644 --- a/common/lib/capa/capa/tests/test_responsetypes.py +++ b/common/lib/capa/capa/tests/test_responsetypes.py @@ -20,6 +20,7 @@ from capa.responsetypes import LoncapaProblemError, \ StudentInputError, ResponseError from capa.correctmap import CorrectMap from capa.util import convert_files_to_filenames +from capa.util import compare_with_tolerance from capa.xqueue_interface import dateformat from pytz import UTC @@ -1120,7 +1121,6 @@ class NumericalResponseTest(ResponseTest): # We blend the line between integration (using evaluator) and exclusively # unit testing the NumericalResponse (mocking out the evaluator) # For simple things its not worth the effort. - def test_grade_range_tolerance(self): problem_setup = [ # [given_asnwer, [list of correct responses], [list of incorrect responses]] @@ -1177,9 +1177,20 @@ class NumericalResponseTest(ResponseTest): self.assert_multiple_grade(problem, correct_responses, incorrect_responses) def test_grade_percent_tolerance(self): + # Positive only range problem = self.build_problem(answer=4, tolerance="10%") - correct_responses = ["4.0", "4.3", "3.7", "4.30", "3.70"] - incorrect_responses = ["", "4.5", "3.5", "0"] + correct_responses = ["4.0", "4.00", "4.39", "3.61"] + incorrect_responses = ["", "4.41", "3.59", "0"] + self.assert_multiple_grade(problem, correct_responses, incorrect_responses) + # Negative only range + problem = self.build_problem(answer=-4, tolerance="10%") + correct_responses = ["-4.0", "-4.00", "-4.39", "-3.61"] + incorrect_responses = ["", "-4.41", "-3.59", "0"] + self.assert_multiple_grade(problem, correct_responses, incorrect_responses) + # Mixed negative/positive range + problem = self.build_problem(answer=1, tolerance="200%") + correct_responses = ["1", "1.00", "2.99", "0.99"] + incorrect_responses = ["", "3.01", "-1.01"] self.assert_multiple_grade(problem, correct_responses, incorrect_responses) def test_floats(self): diff --git a/common/lib/capa/capa/tests/test_util.py b/common/lib/capa/capa/tests/test_util.py new file mode 100644 index 0000000000..1b0fb11de7 --- /dev/null +++ b/common/lib/capa/capa/tests/test_util.py @@ -0,0 +1,82 @@ +"""Tests capa util""" + +import unittest +import textwrap +from . import test_capa_system +from capa.util import compare_with_tolerance + + +class UtilTest(unittest.TestCase): + """Tests for util""" + def setUp(self): + super(UtilTest, self).setUp() + self.system = test_capa_system() + + def test_compare_with_tolerance(self): + # Test default tolerance '0.001%' (it is relative) + result = compare_with_tolerance(100.0, 100.0) + self.assertTrue(result) + result = compare_with_tolerance(100.001, 100.0) + self.assertTrue(result) + result = compare_with_tolerance(101.0, 100.0) + self.assertFalse(result) + # Test absolute percentage tolerance + result = compare_with_tolerance(109.9, 100.0, '10%', False) + self.assertTrue(result) + result = compare_with_tolerance(110.1, 100.0, '10%', False) + self.assertFalse(result) + # Test relative percentage tolerance + result = compare_with_tolerance(111.0, 100.0, '10%', True) + self.assertTrue(result) + result = compare_with_tolerance(112.0, 100.0, '10%', True) + self.assertFalse(result) + # Test absolute tolerance (string) + result = compare_with_tolerance(109.9, 100.0, '10.0', False) + self.assertTrue(result) + result = compare_with_tolerance(110.1, 100.0, '10.0', False) + self.assertFalse(result) + # Test relative tolerance (string) + result = compare_with_tolerance(111.0, 100.0, '0.1', True) + self.assertTrue(result) + result = compare_with_tolerance(112.0, 100.0, '0.1', True) + self.assertFalse(result) + # Test absolute tolerance (float) + result = compare_with_tolerance(109.9, 100.0, 10.0, False) + self.assertTrue(result) + result = compare_with_tolerance(110.1, 100.0, 10.0, False) + self.assertFalse(result) + # Test relative tolerance (float) + result = compare_with_tolerance(111.0, 100.0, 0.1, True) + self.assertTrue(result) + result = compare_with_tolerance(112.0, 100.0, 0.1, True) + self.assertFalse(result) + ##### Infinite values ##### + infinity = float('Inf') + # Test relative tolerance (float) + result = compare_with_tolerance(infinity, 100.0, 1.0, True) + self.assertFalse(result) + result = compare_with_tolerance(100.0, infinity, 1.0, True) + self.assertFalse(result) + result = compare_with_tolerance(infinity, infinity, 1.0, True) + self.assertTrue(result) + # Test absolute tolerance (float) + result = compare_with_tolerance(infinity, 100.0, 1.0, False) + self.assertFalse(result) + result = compare_with_tolerance(100.0, infinity, 1.0, False) + self.assertFalse(result) + result = compare_with_tolerance(infinity, infinity, 1.0, False) + self.assertTrue(result) + # Test relative tolerance (string) + result = compare_with_tolerance(infinity, 100.0, '1.0', True) + self.assertFalse(result) + result = compare_with_tolerance(100.0, infinity, '1.0', True) + self.assertFalse(result) + result = compare_with_tolerance(infinity, infinity, '1.0', True) + self.assertTrue(result) + # Test absolute tolerance (string) + result = compare_with_tolerance(infinity, 100.0, '1.0', False) + self.assertFalse(result) + result = compare_with_tolerance(100.0, infinity, '1.0', False) + self.assertFalse(result) + result = compare_with_tolerance(infinity, infinity, '1.0', False) + self.assertTrue(result) diff --git a/common/lib/capa/capa/util.py b/common/lib/capa/capa/util.py index c4f10ccb0e..5b54d7c666 100644 --- a/common/lib/capa/capa/util.py +++ b/common/lib/capa/capa/util.py @@ -7,16 +7,29 @@ from cmath import isinf default_tolerance = '0.001%' -def compare_with_tolerance(complex1, complex2, tolerance=default_tolerance, relative_tolerance=False): +def compare_with_tolerance(student_complex, instructor_complex, tolerance=default_tolerance, relative_tolerance=False): """ - Compare complex1 to complex2 with maximum tolerance tol. + Compare student_complex to instructor_complex with maximum tolerance tolerance. - If tolerance is type string, then it is counted as relative if it ends in %; otherwise, it is absolute. + - student_complex : student result (float complex number) + - instructor_complex : instructor result (float complex number) + - tolerance : float, or string (representing a float or a percentage) + - relative_tolerance: bool, to explicitly use passed tolerance as relative - - complex1 : student result (float complex number) - - complex2 : instructor result (float complex number) - - tolerance : string representing a number or float - - relative_tolerance: bool, used when`tolerance` is float to explicitly use passed tolerance as relative. + Note: when a tolerance is a percentage (i.e. '10%'), it will compute that + percentage of the instructor result and yield a number. + + If relative_tolerance is set to False, it will use that value and the + instructor result to define the bounds of valid student result: + instructor_complex = 10, tolerance = '10%' will give [9.0, 11.0]. + + If relative_tolerance is set to True, it will use that value and both + instructor result and student result to define the bounds of valid student + result: + instructor_complex = 10, student_complex = 20, tolerance = '10%' will give + [8.0, 12.0]. + This is typically used internally to compare float, with a + default_tolerance = '0.001%'. Default tolerance of 1e-3% is added to compare two floats for near-equality (to handle machine representation errors). @@ -29,23 +42,28 @@ def compare_with_tolerance(complex1, complex2, tolerance=default_tolerance, rela In [212]: 1.9e24 - 1.9*10**24 Out[212]: 268435456.0 """ - if relative_tolerance: - tolerance = tolerance * max(abs(complex1), abs(complex2)) - elif tolerance.endswith('%'): - tolerance = evaluator(dict(), dict(), tolerance[:-1]) * 0.01 - tolerance = tolerance * max(abs(complex1), abs(complex2)) - else: - tolerance = evaluator(dict(), dict(), tolerance) + if isinstance(tolerance, str): + if tolerance == default_tolerance: + relative_tolerance = True + if tolerance.endswith('%'): + tolerance = evaluator(dict(), dict(), tolerance[:-1]) * 0.01 + if not relative_tolerance: + tolerance = tolerance * abs(instructor_complex) + else: + tolerance = evaluator(dict(), dict(), tolerance) - if isinf(complex1) or isinf(complex2): - # If an input is infinite, we can end up with `abs(complex1-complex2)` and + if relative_tolerance: + tolerance = tolerance * max(abs(student_complex), abs(instructor_complex)) + + if isinf(student_complex) or isinf(instructor_complex): + # If an input is infinite, we can end up with `abs(student_complex-instructor_complex)` and # `tolerance` both equal to infinity. Then, below we would have # `inf <= inf` which is a fail. Instead, compare directly. - return complex1 == complex2 + return student_complex == instructor_complex else: # v1 and v2 are, in general, complex numbers: # there are some notes about backward compatibility issue: see responsetypes.get_staff_ans()). - return abs(complex1 - complex2) <= tolerance + return abs(student_complex - instructor_complex) <= tolerance def contextualize_text(text, context): # private From 541d20ef83d33e6872405cbded3794ec97cd52a8 Mon Sep 17 00:00:00 2001 From: Andy Armstrong Date: Fri, 2 May 2014 14:53:31 -0400 Subject: [PATCH 39/62] Allow creation of components on container page This commit implements STUD-1490, allowing creation of components on the container page. It also enables the delete and duplicate buttons now that new content can be created that would benefit. Note that it also creates shared functionality for adding components, and refactors the unit page to use it too. --- CHANGELOG.rst | 5 +- .../contentstore/tests/test_contentstore.py | 10 +- .../contentstore/views/component.py | 163 +++--- cms/djangoapps/contentstore/views/helpers.py | 46 +- cms/djangoapps/contentstore/views/item.py | 45 +- cms/djangoapps/contentstore/views/preview.py | 21 +- .../views/tests/test_container.py | 154 ----- .../views/tests/test_container_page.py | 158 +++++ .../contentstore/views/tests/test_tabs.py | 2 +- .../views/tests/test_unit_page.py | 77 +++ .../contentstore/views/tests/utils.py | 81 +++ cms/envs/common.py | 6 - .../coffee/spec/views/module_edit_spec.coffee | 2 +- cms/static/coffee/src/views/unit.coffee | 496 ++++++++-------- .../js/collections/component_template.js | 5 + cms/static/js/models/component_template.js | 31 + cms/static/js/spec/views/baseview_spec.js | 36 +- cms/static/js/spec/views/container_spec.js | 54 +- .../js/spec/views/pages/container_spec.js | 244 +++++--- cms/static/js/spec/views/unit_spec.js | 380 +++++------- .../js/spec/views/xblock_editor_spec.js | 2 +- cms/static/js/spec_helpers/create_sinon.js | 10 +- cms/static/js/spec_helpers/edit_helpers.js | 77 ++- cms/static/js/spec_helpers/modal_helpers.js | 9 +- cms/static/js/spec_helpers/view_helpers.js | 47 +- cms/static/js/utils/module.js | 4 +- cms/static/js/utils/templates.js | 20 + cms/static/js/views/baseview.js | 91 ++- cms/static/js/views/components/add_xblock.js | 74 +++ .../js/views/components/add_xblock_button.js | 13 + .../js/views/components/add_xblock_menu.js | 19 + cms/static/js/views/container.js | 24 +- cms/static/js/views/feedback.js | 219 +++---- cms/static/js/views/pages/container.js | 232 +++++--- cms/static/js/views/xblock.js | 17 +- .../sass/elements/_system-feedback.scss | 12 +- cms/static/sass/elements/_xblocks.scss | 9 +- cms/static/sass/views/_container.scss | 63 +- cms/static/sass/views/_unit.scss | 544 +++++++++--------- cms/templates/component.html | 4 +- cms/templates/container.html | 11 +- cms/templates/container_xblock_component.html | 6 +- cms/templates/edit-tabs.html | 2 +- .../js/add-xblock-component-button.underscore | 8 + ...d-xblock-component-menu-problem.underscore | 47 ++ .../js/add-xblock-component-menu.underscore | 23 + .../js/add-xblock-component.underscore | 5 + .../js/mock/mock-container-page.underscore | 7 +- .../js/mock/mock-container-xblock.underscore | 412 ++++++------- .../mock-empty-container-xblock.underscore | 6 +- .../js/mock/mock-unit-page-xblock.underscore | 27 + .../js/mock/mock-unit-page.underscore | 47 ++ .../js/mock/mock-updated-xblock.underscore | 36 +- cms/templates/js/mock/mock-xblock.underscore | 38 +- cms/templates/overview.html | 6 +- cms/templates/studio_container_wrapper.html | 39 ++ cms/templates/studio_vertical_wrapper.html | 24 - cms/templates/studio_xblock_wrapper.html | 44 +- cms/templates/unit.html | 93 +-- cms/templates/ux/reference/unit.html | 6 +- cms/templates/widgets/units.html | 5 +- common/lib/xmodule/xmodule/studio_editable.py | 30 + .../xmodule/tests/test_studio_editable.py | 25 + .../xmodule/xmodule/tests/test_vertical.py | 65 +++ common/lib/xmodule/xmodule/vertical_module.py | 7 +- .../test/acceptance/pages/studio/container.py | 47 +- common/test/acceptance/pages/studio/utils.py | 35 ++ .../acceptance/tests/test_studio_container.py | 206 +++++-- .../studio_render_children_view.html | 8 + lms/templates/vert_module_studio_view.html | 17 - 70 files changed, 2907 insertions(+), 1931 deletions(-) delete mode 100644 cms/djangoapps/contentstore/views/tests/test_container.py create mode 100644 cms/djangoapps/contentstore/views/tests/test_container_page.py create mode 100644 cms/djangoapps/contentstore/views/tests/test_unit_page.py create mode 100644 cms/djangoapps/contentstore/views/tests/utils.py create mode 100644 cms/static/js/collections/component_template.js create mode 100644 cms/static/js/models/component_template.js create mode 100644 cms/static/js/utils/templates.js create mode 100644 cms/static/js/views/components/add_xblock.js create mode 100644 cms/static/js/views/components/add_xblock_button.js create mode 100644 cms/static/js/views/components/add_xblock_menu.js create mode 100644 cms/templates/js/add-xblock-component-button.underscore create mode 100644 cms/templates/js/add-xblock-component-menu-problem.underscore create mode 100644 cms/templates/js/add-xblock-component-menu.underscore create mode 100644 cms/templates/js/add-xblock-component.underscore create mode 100644 cms/templates/js/mock/mock-unit-page-xblock.underscore create mode 100644 cms/templates/js/mock/mock-unit-page.underscore create mode 100644 cms/templates/studio_container_wrapper.html delete mode 100644 cms/templates/studio_vertical_wrapper.html create mode 100644 common/lib/xmodule/xmodule/studio_editable.py create mode 100644 common/lib/xmodule/xmodule/tests/test_studio_editable.py create mode 100644 common/lib/xmodule/xmodule/tests/test_vertical.py create mode 100644 common/test/acceptance/pages/studio/utils.py create mode 100644 lms/templates/studio_render_children_view.html delete mode 100644 lms/templates/vert_module_studio_view.html diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9e65666570..ba1c51ddae 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -7,6 +7,8 @@ the top. Include a label indicating the component affected. Blades: Tolerance expressed in percentage now computes correctly. BLD-522. +Studio: Support add, delete and duplicate on the container page. STUD-1490. + Studio: Add drag-and-drop support to the container page. STUD-1309. Common: Add extensible third-party auth module. @@ -20,7 +22,8 @@ LMS: Switch default instructor dashboard to the new (formerly "beta") Blades: Handle situation if no response were sent from XQueue to LMS in Matlab problem after Run Code button press. BLD-994. -Blades: Set initial video quality to large instead of default to avoid automatic switch to HD when iframe resizes. BLD-981. +Blades: Set initial video quality to large instead of default to avoid automatic +switch to HD when iframe resizes. BLD-981. Blades: Add an upload button for authors to provide students with an option to download a handout associated with a video (of arbitrary file format). BLD-1000. diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 2539a4efbf..ce14c55e52 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -490,12 +490,10 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): Tests the ajax callback to render an XModule """ resp = self._test_preview(Location('i4x', 'edX', 'toy', 'vertical', 'vertical_test', None), 'container_preview') - # These are the data-ids of the xblocks contained in the vertical. - # Ultimately, these must be converted to new locators. - self.assertContains(resp, 'i4x://edX/toy/video/sample_video') - self.assertContains(resp, 'i4x://edX/toy/video/separate_file_video') - self.assertContains(resp, 'i4x://edX/toy/video/video_with_end_time') - self.assertContains(resp, 'i4x://edX/toy/poll_question/T1_changemind_poll_foo_2') + self.assertContains(resp, '/branch/draft/block/sample_video') + self.assertContains(resp, '/branch/draft/block/separate_file_video') + self.assertContains(resp, '/branch/draft/block/video_with_end_time') + self.assertContains(resp, '/branch/draft/block/T1_changemind_poll_foo_2') def _test_preview(self, location, view_name): """ Preview test case. """ diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 425b330353..6df2e407dd 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -164,70 +164,7 @@ def unit_handler(request, tag=None, package_id=None, branch=None, version_guid=N except ItemNotFoundError: return HttpResponseBadRequest() - component_templates = defaultdict(list) - for category in COMPONENT_TYPES: - component_class = _load_mixed_class(category) - # add the default template - # TODO: Once mixins are defined per-application, rather than per-runtime, - # this should use a cms mixed-in class. (cpennington) - if hasattr(component_class, 'display_name'): - display_name = component_class.display_name.default or 'Blank' - else: - display_name = 'Blank' - component_templates[category].append(( - display_name, - category, - False, # No defaults have markdown (hardcoded current default) - None # no boilerplate for overrides - )) - # add boilerplates - if hasattr(component_class, 'templates'): - for template in component_class.templates(): - filter_templates = getattr(component_class, 'filter_templates', None) - if not filter_templates or filter_templates(template, course): - component_templates[category].append(( - template['metadata'].get('display_name'), - category, - template['metadata'].get('markdown') is not None, - template.get('template_id') - )) - - # Check if there are any advanced modules specified in the course policy. - # These modules should be specified as a list of strings, where the strings - # are the names of the modules in ADVANCED_COMPONENT_TYPES that should be - # enabled for the course. - course_advanced_keys = course.advanced_modules - - # Set component types according to course policy file - if isinstance(course_advanced_keys, list): - for category in course_advanced_keys: - if category in ADVANCED_COMPONENT_TYPES: - # Do I need to allow for boilerplates or just defaults on the - # class? i.e., can an advanced have more than one entry in the - # menu? one for default and others for prefilled boilerplates? - try: - component_class = _load_mixed_class(category) - - component_templates['advanced'].append( - ( - component_class.display_name.default or category, - category, - False, - None # don't override default data - ) - ) - except PluginMissingError: - # dhm: I got this once but it can happen any time the - # course author configures an advanced component which does - # not exist on the server. This code here merely - # prevents any authors from trying to instantiate the - # non-existent component type by not showing it in the menu - pass - else: - log.error( - "Improper format for course advanced keys! %s", - course_advanced_keys - ) + component_templates = _get_component_templates(course) xblocks = item.get_children() locators = [ @@ -274,7 +211,7 @@ def unit_handler(request, tag=None, package_id=None, branch=None, version_guid=N 'unit': item, 'unit_locator': locator, 'locators': locators, - 'component_templates': component_templates, + 'component_templates': json.dumps(component_templates), 'draft_preview_link': preview_lms_link, 'published_preview_link': lms_link, 'subsection': containing_subsection, @@ -312,6 +249,7 @@ def container_handler(request, tag=None, package_id=None, branch=None, version_g except ItemNotFoundError: return HttpResponseBadRequest() + component_templates = _get_component_templates(course) ancestor_xblocks = [] parent = get_parent_xblock(xblock) while parent and parent.category != 'sequential': @@ -329,11 +267,106 @@ def container_handler(request, tag=None, package_id=None, branch=None, version_g 'unit': unit, 'unit_publish_state': unit_publish_state, 'ancestor_xblocks': ancestor_xblocks, + 'component_templates': json.dumps(component_templates), }) else: return HttpResponseBadRequest("Only supports html requests") +def _get_component_templates(course): + """ + Returns the applicable component templates that can be used by the specified course. + """ + def create_template_dict(name, cat, boilerplate_name=None, is_common=False): + """ + Creates a component template dict. + + Parameters + display_name: the user-visible name of the component + category: the type of component (problem, html, etc.) + boilerplate_name: name of boilerplate for filling in default values. May be None. + is_common: True if "common" problem, False if "advanced". May be None, as it is only used for problems. + + """ + return { + "display_name": name, + "category": cat, + "boilerplate_name": boilerplate_name, + "is_common": is_common + } + + component_templates = [] + # The component_templates array is in the order of "advanced" (if present), followed + # by the components in the order listed in COMPONENT_TYPES. + for category in COMPONENT_TYPES: + templates_for_category = [] + component_class = _load_mixed_class(category) + # add the default template + # TODO: Once mixins are defined per-application, rather than per-runtime, + # this should use a cms mixed-in class. (cpennington) + if hasattr(component_class, 'display_name'): + display_name = component_class.display_name.default or 'Blank' + else: + display_name = 'Blank' + templates_for_category.append(create_template_dict(display_name, category)) + + # add boilerplates + if hasattr(component_class, 'templates'): + for template in component_class.templates(): + filter_templates = getattr(component_class, 'filter_templates', None) + if not filter_templates or filter_templates(template, course): + templates_for_category.append( + create_template_dict( + template['metadata'].get('display_name'), + category, + template.get('template_id'), + template['metadata'].get('markdown') is not None + ) + ) + component_templates.append({"type": category, "templates": templates_for_category}) + + # Check if there are any advanced modules specified in the course policy. + # These modules should be specified as a list of strings, where the strings + # are the names of the modules in ADVANCED_COMPONENT_TYPES that should be + # enabled for the course. + course_advanced_keys = course.advanced_modules + advanced_component_templates = {"type": "advanced", "templates": []} + # Set component types according to course policy file + if isinstance(course_advanced_keys, list): + for category in course_advanced_keys: + if category in ADVANCED_COMPONENT_TYPES: + # boilerplates not supported for advanced components + try: + component_class = _load_mixed_class(category) + + advanced_component_templates['templates'].append( + create_template_dict( + component_class.display_name.default or category, + category + ) + ) + except PluginMissingError: + # dhm: I got this once but it can happen any time the + # course author configures an advanced component which does + # not exist on the server. This code here merely + # prevents any authors from trying to instantiate the + # non-existent component type by not showing it in the menu + log.warning( + "Advanced component %s does not exist. It will not be added to the Studio new component menu.", + category + ) + pass + else: + log.error( + "Improper format for course advanced keys! %s", + course_advanced_keys + ) + if len(advanced_component_templates['templates']) > 0: + component_templates.insert(0, advanced_component_templates) + + return component_templates + + @login_required def _get_item_in_course(request, locator): """ diff --git a/cms/djangoapps/contentstore/views/helpers.py b/cms/djangoapps/contentstore/views/helpers.py index 2141ecc3c5..f2baa2377c 100644 --- a/cms/djangoapps/contentstore/views/helpers.py +++ b/cms/djangoapps/contentstore/views/helpers.py @@ -8,7 +8,9 @@ from xmodule.modulestore.django import loc_mapper, modulestore __all__ = ['edge', 'event', 'landing'] EDITING_TEMPLATES = [ - "basic-modal", "modal-button", "edit-xblock-modal", "editor-mode-button", "upload-dialog", "image-modal" + "basic-modal", "modal-button", "edit-xblock-modal", "editor-mode-button", "upload-dialog", "image-modal", + "add-xblock-component", "add-xblock-component-button", "add-xblock-component-menu", + "add-xblock-component-menu-problem" ] # points to the temporary course landing page with log in and sign up @@ -57,40 +59,54 @@ def get_parent_xblock(xblock): return modulestore().get_item(parent_locations[0]) -def _xblock_has_studio_page(xblock): +def is_unit(xblock): + """ + Returns true if the specified xblock is a vertical that is treated as a unit. + A unit is a vertical that is a direct child of a sequential (aka a subsection). + """ + if xblock.category == 'vertical': + parent_xblock = get_parent_xblock(xblock) + parent_category = parent_xblock.category if parent_xblock else None + return parent_category == 'sequential' + return False + + +def xblock_has_own_studio_page(xblock): """ Returns true if the specified xblock has an associated Studio page. Most xblocks do not have their own page but are instead shown on the page of their parent. There are a few exceptions: 1. Courses - 2. Verticals + 2. Verticals that are either: + - themselves treated as units (in which case they are shown on a unit page) + - a direct child of a unit (in which case they are shown on a container page) 3. XBlocks with children, except for: - - subsections (aka sequential blocks) - - chapters + - sequentials (aka subsections) + - chapters (aka sections) """ category = xblock.category - if category in ('course', 'vertical'): + + if is_unit(xblock): return True + elif category == 'vertical': + parent_xblock = get_parent_xblock(xblock) + return is_unit(parent_xblock) if parent_xblock else False elif category in ('sequential', 'chapter'): return False - elif xblock.has_children: - return True - else: - return False + + # All other xblocks with children have their own page + return xblock.has_children def xblock_studio_url(xblock, course=None): """ Returns the Studio editing URL for the specified xblock. """ - if not _xblock_has_studio_page(xblock): + if not xblock_has_own_studio_page(xblock): return None category = xblock.category parent_xblock = get_parent_xblock(xblock) - if parent_xblock: - parent_category = parent_xblock.category - else: - parent_category = None + parent_category = parent_xblock.category if parent_xblock else None if category == 'course': prefix = 'course' elif category == 'vertical' and parent_category == 'sequential': diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 6916120caf..f8fbc01a3e 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -33,7 +33,7 @@ from util.string_utils import str_to_bool from ..utils import get_modulestore from .access import has_course_access -from .helpers import _xmodule_recurse +from .helpers import _xmodule_recurse, xblock_has_own_studio_page from contentstore.utils import compute_publish_state, PublishState from xmodule.modulestore.draft import DIRECT_ONLY_CATEGORIES from contentstore.views.preview import get_preview_fragment @@ -193,46 +193,56 @@ def xblock_view_handler(request, package_id, view_name, tag=None, branch=None, v if 'application/json' in accept_header: store = get_modulestore(old_location) - component = store.get_item(old_location) - is_read_only = _xblock_is_read_only(component) + xblock = store.get_item(old_location) + is_read_only = _is_xblock_read_only(xblock) + container_views = ['container_preview', 'reorderable_container_child_preview'] + unit_views = ['student_view'] # wrap the generated fragment in the xmodule_editor div so that the javascript # can bind to it correctly - component.runtime.wrappers.append(partial(wrap_xblock, 'StudioRuntime')) + xblock.runtime.wrappers.append(partial(wrap_xblock, 'StudioRuntime')) if view_name == 'studio_view': try: - fragment = component.render('studio_view') + fragment = xblock.render('studio_view') # catch exceptions indiscriminately, since after this point they escape the # dungeon and surface as uneditable, unsaveable, and undeletable # component-goblins. except Exception as exc: # pylint: disable=w0703 - log.debug("unable to render studio_view for %r", component, exc_info=True) + log.debug("unable to render studio_view for %r", xblock, exc_info=True) fragment = Fragment(render_to_string('html_error.html', {'message': str(exc)})) # change not authored by requestor but by xblocks. - store.update_item(component, None) + store.update_item(xblock, None) - elif view_name == 'student_view' and component.has_children: + elif view_name == 'student_view' and xblock_has_own_studio_page(xblock): context = { 'runtime_type': 'studio', 'container_view': False, 'read_only': is_read_only, - 'root_xblock': component, + 'root_xblock': xblock, } # For non-leaf xblocks on the unit page, show the special rendering # which links to the new container page. html = render_to_string('container_xblock_component.html', { 'xblock_context': context, - 'xblock': component, + 'xblock': xblock, 'locator': locator, }) return JsonResponse({ 'html': html, 'resources': [], }) - elif view_name in ('student_view', 'container_preview'): - is_container_view = (view_name == 'container_preview') + elif view_name in (unit_views + container_views): + is_container_view = (view_name in container_views) + + # Determine the items to be shown as reorderable. Note that the view + # 'reorderable_container_child_preview' is only rendered for xblocks that + # are being shown in a reorderable container, so the xblock is automatically + # added to the list. + reorderable_items = set() + if view_name == 'reorderable_container_child_preview': + reorderable_items.add(xblock.location) # Only show the new style HTML for the container view, i.e. for non-verticals # Note: this special case logic can be removed once the unit page is replaced @@ -241,10 +251,11 @@ def xblock_view_handler(request, package_id, view_name, tag=None, branch=None, v 'runtime_type': 'studio', 'container_view': is_container_view, 'read_only': is_read_only, - 'root_xblock': component, + 'root_xblock': xblock if (view_name == 'container_preview') else None, + 'reorderable_items': reorderable_items } - fragment = get_preview_fragment(request, component, context) + fragment = get_preview_fragment(request, xblock, context) # For old-style pages (such as unit and static pages), wrap the preview with # the component div. Note that the container view recursively adds headers # into the preview fragment, so we don't want to add another header here. @@ -252,7 +263,7 @@ def xblock_view_handler(request, package_id, view_name, tag=None, branch=None, v fragment.content = render_to_string('component.html', { 'xblock_context': context, 'preview': fragment.content, - 'label': component.display_name or component.scope_ids.block_type, + 'label': xblock.display_name or xblock.scope_ids.block_type, }) else: raise Http404 @@ -270,7 +281,7 @@ def xblock_view_handler(request, package_id, view_name, tag=None, branch=None, v return HttpResponse(status=406) -def _xblock_is_read_only(xblock): +def _is_xblock_read_only(xblock): """ Returns true if the specified xblock is read-only, meaning that it cannot be edited. """ @@ -411,7 +422,7 @@ def _create_item(request): metadata = {} data = None template_id = request.json.get('boilerplate') - if template_id is not None: + if template_id: clz = parent.runtime.load_block_type(category) if clz is not None: template = clz.get_template(template_id) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 239accb4a6..b2847f5e20 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -28,7 +28,7 @@ from util.sandboxing import can_execute_unsafe_code import static_replace from .session_kv_store import SessionKeyValueStore -from .helpers import render_from_lms +from .helpers import render_from_lms, xblock_has_own_studio_page from ..utils import get_course_for_item from contentstore.views.access import get_user_role @@ -166,6 +166,13 @@ def _load_preview_module(request, descriptor): return descriptor +def _is_xblock_reorderable(xblock, context): + """ + Returns true if the specified xblock is in the set of reorderable xblocks. + """ + return xblock.location in context['reorderable_items'] + + # pylint: disable=unused-argument def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): """ @@ -173,17 +180,21 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): """ # Only add the Studio wrapper when on the container page. The unit page will remain as is for now. if context.get('container_view', None) and view == 'student_view': + root_xblock = context.get('root_xblock') + is_root = root_xblock and xblock.location == root_xblock.location locator = loc_mapper().translate_location(xblock.course_id, xblock.location, published=False) + is_reorderable = _is_xblock_reorderable(xblock, context) template_context = { 'xblock_context': context, 'xblock': xblock, 'locator': locator, 'content': frag.content, + 'is_root': is_root, + 'is_reorderable': is_reorderable, } - if xblock.category == 'vertical': - template = 'studio_vertical_wrapper.html' - elif xblock.location != context.get('root_xblock').location and xblock.has_children: - template = 'container_xblock_component.html' + # For child xblocks with their own page, render a link to the page + if xblock_has_own_studio_page(xblock) and not is_root: + template = 'studio_container_wrapper.html' else: template = 'studio_xblock_wrapper.html' html = render_to_string(template, template_context) diff --git a/cms/djangoapps/contentstore/views/tests/test_container.py b/cms/djangoapps/contentstore/views/tests/test_container.py deleted file mode 100644 index 0310a9544b..0000000000 --- a/cms/djangoapps/contentstore/views/tests/test_container.py +++ /dev/null @@ -1,154 +0,0 @@ -""" -Unit tests for the container view. -""" - -import json - -from contentstore.tests.utils import CourseTestCase -from contentstore.utils import compute_publish_state, PublishState -from contentstore.views.helpers import xblock_studio_url -from xmodule.modulestore.django import loc_mapper, modulestore -from xmodule.modulestore.tests.factories import ItemFactory - - -class ContainerViewTestCase(CourseTestCase): - """ - Unit tests for the container view. - """ - - def setUp(self): - super(ContainerViewTestCase, self).setUp() - self.chapter = ItemFactory.create(parent_location=self.course.location, - category='chapter', display_name="Week 1") - self.sequential = ItemFactory.create(parent_location=self.chapter.location, - category='sequential', display_name="Lesson 1") - self.vertical = ItemFactory.create(parent_location=self.sequential.location, - category='vertical', display_name='Unit') - self.child_vertical = ItemFactory.create(parent_location=self.vertical.location, - category='vertical', display_name='Child Vertical') - self.video = ItemFactory.create(parent_location=self.child_vertical.location, - category="video", display_name="My Video") - - def test_container_html(self): - branch_name = "MITx.999.Robot_Super_Course/branch/draft/block" - self._test_html_content( - self.child_vertical, - branch_name=branch_name, - expected_section_tag=( - '