From 4896444d10ec650f1c44e4c9a4c01178b61114c7 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Fri, 2 Aug 2013 09:29:55 -0400 Subject: [PATCH 01/35] Clean up item views, use JsonResponse class --- cms/djangoapps/contentstore/views/item.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index efebded9b9..ff347a2878 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -1,15 +1,13 @@ -import json from uuid import uuid4 from django.core.exceptions import PermissionDenied -from django.http import HttpResponse from django.contrib.auth.decorators import login_required from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore from xmodule.modulestore.inheritance import own_metadata -from util.json_request import expect_json +from util.json_request import expect_json, JsonResponse from ..utils import get_modulestore from .access import has_access from .requests import _xmodule_recurse @@ -20,6 +18,7 @@ __all__ = ['save_item', 'create_item', 'delete_item'] # cdodge: these are categories which should not be parented, they are detached from the hierarchy DETACHED_CATEGORIES = ['about', 'static_tab', 'course_info'] + @login_required @expect_json def save_item(request): @@ -80,7 +79,7 @@ def save_item(request): # commit to datastore store.update_metadata(item_location, own_metadata(existing_item)) - return HttpResponse() + return JsonResponse() # [DHM] A hack until we implement a permanent soln. Proposed perm solution is to make namespace fields also top level @@ -139,13 +138,17 @@ def create_item(request): if display_name is not None: metadata['display_name'] = display_name - get_modulestore(category).create_and_save_xmodule(dest_location, definition_data=data, - metadata=metadata, system=parent.system) + get_modulestore(category).create_and_save_xmodule( + dest_location, + definition_data=data, + metadata=metadata, + system=parent.system, + ) if category not in DETACHED_CATEGORIES: get_modulestore(parent.location).update_children(parent_location, parent.children + [dest_location.url()]) - return HttpResponse(json.dumps({'id': dest_location.url()})) + return JsonResponse({'id': dest_location.url()}) @login_required @@ -184,4 +187,4 @@ def delete_item(request): parent.children = children modulestore('direct').update_children(parent.location, parent.children) - return HttpResponse() + return JsonResponse() From be103dfa0d3e758cec8c25eb08181d0570e315a9 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Wed, 31 Jul 2013 12:50:22 -0400 Subject: [PATCH 02/35] improving code style --- common/lib/xmodule/xmodule/x_module.py | 70 +++++++++++--------------- 1 file changed, 29 insertions(+), 41 deletions(-) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 3556f3f0f3..310a871b72 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -711,20 +711,20 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): # =============================== BUILTIN METHODS ========================== def __eq__(self, other): - eq = (self.__class__ == other.__class__ and + return (self.__class__ == other.__class__ and all(getattr(self, attr, None) == getattr(other, attr, None) for attr in self.equality_attributes)) - return eq - def __repr__(self): - return ("{class_}({system!r}, location={location!r}," - " model_data={model_data!r})".format( - class_=self.__class__.__name__, - system=self.system, - location=self.location, - model_data=self._model_data, - )) + return ( + "{class_}({system!r}, location={location!r}," + " model_data={model_data!r})".format( + class_=self.__class__.__name__, + system=self.system, + location=self.location, + model_data=self._model_data, + ) + ) @property def non_editable_metadata_fields(self): @@ -785,15 +785,17 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): editor_type = "Float" elif isinstance(field, List): editor_type = "List" - metadata_fields[field.name] = {'field_name': field.name, - 'type': editor_type, - 'display_name': field.display_name, - 'value': field.to_json(value), - 'options': [] if values is None else values, - 'default_value': field.to_json(default_value), - 'inheritable': inheritable, - 'explicitly_set': explicitly_set, - 'help': field.help} + metadata_fields[field.name] = { + 'field_name': field.name, + 'type': editor_type, + 'display_name': field.display_name, + 'value': field.to_json(value), + 'options': [] if values is None else values, + 'default_value': field.to_json(default_value), + 'inheritable': inheritable, + 'explicitly_set': explicitly_set, + 'help': field.help, + } return metadata_fields @@ -885,28 +887,14 @@ class ModuleSystem(Runtime): Note that these functions can be closures over e.g. a django request and user, or other environment-specific info. ''' - def __init__(self, - ajax_url, - track_function, - get_module, - render_template, - replace_urls, - xblock_model_data, - user=None, - filestore=None, - debug=False, - xqueue=None, - publish=None, - node_path="", - anonymous_student_id='', - course_id=None, - open_ended_grading_interface=None, - s3_interface=None, - cache=None, - can_execute_unsafe_code=None, - replace_course_urls=None, - replace_jump_to_id_urls=None - ): + def __init__( + self, ajax_url, track_function, get_module, render_template, + replace_urls, xblock_model_data, user=None, filestore=None, + debug=False, xqueue=None, publish=None, node_path="", + anonymous_student_id='', course_id=None, + open_ended_grading_interface=None, s3_interface=None, + cache=None, can_execute_unsafe_code=None, replace_course_urls=None, + replace_jump_to_id_urls=None): ''' Create a closure around the system environment. From 9634e222bed244d3310f449faa137a029493977b Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Wed, 31 Jul 2013 12:51:01 -0400 Subject: [PATCH 03/35] Refactored get_module_previews function --- cms/djangoapps/contentstore/views/preview.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index f2a07abe32..a9c9757d1d 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -163,6 +163,11 @@ def load_preview_module(request, preview_id, descriptor): return module +def get_preview_html(request, descriptor, idx): + module = load_preview_module(request, str(idx), descriptor) + return module.get_html() + + def get_module_previews(request, descriptor): """ Returns a list of preview XModule html contents. One preview is returned for each @@ -170,8 +175,5 @@ def get_module_previews(request, descriptor): descriptor: An XModuleDescriptor """ - preview_html = [] - for idx, (_instance_state, _shared_state) in enumerate(descriptor.get_sample_state()): - module = load_preview_module(request, str(idx), descriptor) - preview_html.append(module.get_html()) - return preview_html + return tuple(get_preview_html(request, descriptor, idx) + for idx in range(len(descriptor.get_sample_state()))) From 8a95d7e6f0c72fe9836f01a209e6eb1d4ca4bab4 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Fri, 2 Aug 2013 13:51:46 -0400 Subject: [PATCH 04/35] XBlock integration: replaced `get_html` with `runtime.render()` Currently calls the same machinery, but re-routes the logic in preparation of deeper integration with XBlock --- cms/djangoapps/contentstore/views/preview.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index a9c9757d1d..fa55cb2c24 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -165,7 +165,7 @@ def load_preview_module(request, preview_id, descriptor): def get_preview_html(request, descriptor, idx): module = load_preview_module(request, str(idx), descriptor) - return module.get_html() + return module.runtime.render(module, None, "student_view") def get_module_previews(request, descriptor): From 3fa990ea60dc3c704c82eea3539d1a71a5eafbd3 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Mon, 5 Aug 2013 09:38:51 -0400 Subject: [PATCH 05/35] Made some tests more general, less fragile --- .../contentstore/tests/test_contentstore.py | 30 +++++++++---------- .../contentstore/tests/test_item.py | 2 +- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 838af2cafa..64fa53433e 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1237,7 +1237,7 @@ class ContentStoreTest(ModuleStoreTestCase): 'course': loc.course, 'name': loc.name})) - self.assertEqual(200, resp.status_code) + self.assert2XX(resp.status_code) self.assertContains(resp, 'Chapter 2') # go to various pages @@ -1247,92 +1247,92 @@ class ContentStoreTest(ModuleStoreTestCase): kwargs={'org': loc.org, 'course': loc.course, 'name': loc.name})) - self.assertEqual(200, resp.status_code) + self.assert2XX(resp.status_code) # export page resp = self.client.get(reverse('export_course', kwargs={'org': loc.org, 'course': loc.course, 'name': loc.name})) - self.assertEqual(200, resp.status_code) + self.assert2XX(resp.status_code) # manage users resp = self.client.get(reverse('manage_users', kwargs={'org': loc.org, 'course': loc.course, 'name': loc.name})) - self.assertEqual(200, resp.status_code) + self.assert2XX(resp.status_code) # course info resp = self.client.get(reverse('course_info', kwargs={'org': loc.org, 'course': loc.course, 'name': loc.name})) - self.assertEqual(200, resp.status_code) + self.assert2XX(resp.status_code) # settings_details resp = self.client.get(reverse('settings_details', kwargs={'org': loc.org, 'course': loc.course, 'name': loc.name})) - self.assertEqual(200, resp.status_code) + self.assert2XX(resp.status_code) # settings_details resp = self.client.get(reverse('settings_grading', kwargs={'org': loc.org, 'course': loc.course, 'name': loc.name})) - self.assertEqual(200, resp.status_code) + self.assert2XX(resp.status_code) # static_pages resp = self.client.get(reverse('static_pages', kwargs={'org': loc.org, 'course': loc.course, 'coursename': loc.name})) - self.assertEqual(200, resp.status_code) + self.assert2XX(resp.status_code) # static_pages resp = self.client.get(reverse('asset_index', kwargs={'org': loc.org, 'course': loc.course, 'name': loc.name})) - self.assertEqual(200, resp.status_code) + self.assert2XX(resp.status_code) # go look at a subsection page subsection_location = loc.replace(category='sequential', name='test_sequence') resp = self.client.get(reverse('edit_subsection', kwargs={'location': subsection_location.url()})) - self.assertEqual(200, resp.status_code) + self.assert2XX(resp.status_code) # go look at the Edit page unit_location = loc.replace(category='vertical', name='test_vertical') resp = self.client.get(reverse('edit_unit', kwargs={'location': unit_location.url()})) - self.assertEqual(200, resp.status_code) + self.assert2XX(resp.status_code) # delete a component del_loc = loc.replace(category='html', name='test_html') resp = self.client.post(reverse('delete_item'), json.dumps({'id': del_loc.url()}), "application/json") - self.assertEqual(200, resp.status_code) + self.assert2XX(resp.status_code) # delete a unit del_loc = loc.replace(category='vertical', name='test_vertical') resp = self.client.post(reverse('delete_item'), json.dumps({'id': del_loc.url()}), "application/json") - self.assertEqual(200, resp.status_code) + self.assert2XX(resp.status_code) # delete a unit del_loc = loc.replace(category='sequential', name='test_sequence') resp = self.client.post(reverse('delete_item'), json.dumps({'id': del_loc.url()}), "application/json") - self.assertEqual(200, resp.status_code) + self.assert2XX(resp.status_code) # delete a chapter del_loc = loc.replace(category='chapter', name='chapter_2') resp = self.client.post(reverse('delete_item'), json.dumps({'id': del_loc.url()}), "application/json") - self.assertEqual(200, resp.status_code) + self.assert2XX(resp.status_code) def test_import_metadata_with_attempts_empty_string(self): module_store = modulestore('direct') diff --git a/cms/djangoapps/contentstore/tests/test_item.py b/cms/djangoapps/contentstore/tests/test_item.py index 827dd1b054..260444a8f7 100644 --- a/cms/djangoapps/contentstore/tests/test_item.py +++ b/cms/djangoapps/contentstore/tests/test_item.py @@ -34,7 +34,7 @@ class DeleteItem(CourseTestCase): resp.content, "application/json" ) - self.assertEqual(resp.status_code, 200) + self.assert2XX(resp.status_code) class TestCreateItem(CourseTestCase): From baa9bd5bdca69c358f2f4e81e4632febe2f6019f Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Mon, 5 Aug 2013 11:04:23 -0400 Subject: [PATCH 06/35] Make sure to return the content, not the fragment --- cms/djangoapps/contentstore/views/preview.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index fa55cb2c24..202927bdfb 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -165,7 +165,7 @@ def load_preview_module(request, preview_id, descriptor): def get_preview_html(request, descriptor, idx): module = load_preview_module(request, str(idx), descriptor) - return module.runtime.render(module, None, "student_view") + return module.runtime.render(module, None, "student_view").content def get_module_previews(request, descriptor): From a87a1bfcdab0fa42d169f630e9eab85137e50e29 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Wed, 7 Aug 2013 16:14:07 -0400 Subject: [PATCH 07/35] Docstrings --- cms/djangoapps/contentstore/views/preview.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 202927bdfb..1fef30dd99 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -164,13 +164,16 @@ def load_preview_module(request, preview_id, descriptor): def get_preview_html(request, descriptor, idx): + """ + Returns the HTML for the XModule specified by the descriptor and idx. + """ module = load_preview_module(request, str(idx), descriptor) return module.runtime.render(module, None, "student_view").content def get_module_previews(request, descriptor): """ - Returns a list of preview XModule html contents. One preview is returned for each + Returns a tuple of preview XModule html contents. One preview is returned for each pair of states returned by get_sample_state() for the supplied descriptor. descriptor: An XModuleDescriptor From 7aec95c3100132149e9f84f6d8e58927f78655e7 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Thu, 8 Aug 2013 09:52:39 -0400 Subject: [PATCH 08/35] Removed get_module_previews function According to @cpennington, no modules return anything for `get_sample_state`, so this function is extraneous. --- cms/djangoapps/contentstore/views/preview.py | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 1fef30dd99..a325dd3b34 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -76,7 +76,7 @@ def preview_component(request, location): component = modulestore().get_item(location) return render_to_response('component.html', { - 'preview': get_module_previews(request, component)[0], + 'preview': get_preview_html(request, component, 0), 'editor': wrap_xmodule(component.get_html, component, 'xmodule_edit.html')(), }) @@ -169,14 +169,3 @@ def get_preview_html(request, descriptor, idx): """ module = load_preview_module(request, str(idx), descriptor) return module.runtime.render(module, None, "student_view").content - - -def get_module_previews(request, descriptor): - """ - Returns a tuple of preview XModule html contents. One preview is returned for each - pair of states returned by get_sample_state() for the supplied descriptor. - - descriptor: An XModuleDescriptor - """ - return tuple(get_preview_html(request, descriptor, idx) - for idx in range(len(descriptor.get_sample_state()))) From 32f76988c6bb3128da5b3c804bc305f8bc7281d0 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Thu, 8 Aug 2013 09:53:19 -0400 Subject: [PATCH 09/35] Update docstring --- cms/djangoapps/contentstore/views/preview.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index a325dd3b34..121bf98393 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -165,7 +165,8 @@ def load_preview_module(request, preview_id, descriptor): def get_preview_html(request, descriptor, idx): """ - Returns the HTML for the XModule specified by the descriptor and idx. + Returns the HTML returned by the XModule's student_view, + specified by the descriptor and idx. """ module = load_preview_module(request, str(idx), descriptor) return module.runtime.render(module, None, "student_view").content From 264e3b246b20f38efc558cc50fba7bcd8535f9c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Andr=C3=A9s=20Rocha?= Date: Thu, 8 Aug 2013 17:38:20 -0400 Subject: [PATCH 10/35] Change console logging stream from stdout to stderr --- common/lib/logsettings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/logsettings.py b/common/lib/logsettings.py index 8fc2bb9db1..26ef3c8478 100644 --- a/common/lib/logsettings.py +++ b/common/lib/logsettings.py @@ -72,7 +72,7 @@ def get_logger_config(log_dir, 'level': console_loglevel, 'class': 'logging.StreamHandler', 'formatter': 'standard', - 'stream': sys.stdout, + 'stream': sys.stderr, }, 'syslogger-remote': { 'level': 'INFO', From e6288ad19ce08ea82bf13c1ef467a06c67391db3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Andr=C3=A9s=20Rocha?= Date: Thu, 8 Aug 2013 19:06:41 -0400 Subject: [PATCH 11/35] Fix manage.py to ouput the help of the django command if requested Commands like the following were not working correctly: ``` $ python manage.py lms runserver --lms ``` --- manage.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/manage.py b/manage.py index d6b74025f5..ebaebe8b66 100755 --- a/manage.py +++ b/manage.py @@ -20,7 +20,7 @@ from argparse import ArgumentParser def parse_args(): """Parse edx specific arguments to manage.py""" parser = ArgumentParser() - subparsers = parser.add_subparsers(title='system', description='edx service to run') + subparsers = parser.add_subparsers(title='system', description='edX service to run') lms = subparsers.add_parser( 'lms', @@ -31,8 +31,8 @@ def parse_args(): lms.add_argument('-h', '--help', action='store_true', help='show this help message and exit') lms.add_argument( '--settings', - help="Which django settings module to use from inside of lms.envs. If not provided, the DJANGO_SETTINGS_MODULE " - "environment variable will be used if it is set, otherwise will default to lms.envs.dev") + help="Which django settings module to use under lms.envs. If not provided, the DJANGO_SETTINGS_MODULE " + "environment variable will be used if it is set, otherwise it will default to lms.envs.dev") lms.add_argument( '--service-variant', choices=['lms', 'lms-xml', 'lms-preview'], @@ -52,8 +52,8 @@ def parse_args(): ) cms.add_argument( '--settings', - help="Which django settings module to use from inside cms.envs. If not provided, the DJANGO_SETTINGS_MODULE " - "environment variable will be used if it is set, otherwise will default to cms.envs.dev") + help="Which django settings module to use under cms.envs. If not provided, the DJANGO_SETTINGS_MODULE " + "environment variable will be used if it is set, otherwise it will default to cms.envs.dev") cms.add_argument('-h', '--help', action='store_true', help='show this help message and exit') cms.set_defaults( help_string=cms.format_help(), @@ -62,7 +62,6 @@ def parse_args(): service_variant='cms' ) - edx_args, django_args = parser.parse_known_args() if edx_args.help: @@ -79,11 +78,13 @@ if __name__ == "__main__": os.environ["DJANGO_SETTINGS_MODULE"] = edx_args.settings_base.replace('/', '.') + "." + edx_args.settings else: os.environ.setdefault("DJANGO_SETTINGS_MODULE", edx_args.default_settings) + os.environ.setdefault("SERVICE_VARIANT", edx_args.service_variant) + if edx_args.help: print "Django:" # This will trigger django-admin.py to print out its help - django_args.insert(0, '--help') + django_args.append('--help') from django.core.management import execute_from_command_line From 9769f364e1ff5e95f0ab65090d42aca544f9b187 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Fri, 9 Aug 2013 12:10:53 -0400 Subject: [PATCH 12/35] Updated tests to weaken "number" input field requirement, which isn't supported in Firefox. --- .../coffee/spec/views/metadata_edit_spec.coffee | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/cms/static/coffee/spec/views/metadata_edit_spec.coffee b/cms/static/coffee/spec/views/metadata_edit_spec.coffee index 926e5be315..2327779b8a 100644 --- a/cms/static/coffee/spec/views/metadata_edit_spec.coffee +++ b/cms/static/coffee/spec/views/metadata_edit_spec.coffee @@ -113,6 +113,13 @@ describe "Test Metadata Editor", -> verifyEntry = (index, display_name, type) -> expect(childModels[index].get('display_name')).toBe(display_name) + + # Some browsers (e.g. FireFox) do not support the "number" + # input type. We can accept a "text" input instead + # and still get acceptable behavior in the UI. + if type == 'number' and childViews[index].type != 'number' + type = 'text' + expect(childViews[index].type).toBe(type) verifyEntry(0, 'Display Name', 'text') @@ -164,6 +171,13 @@ describe "Test Metadata Editor", -> assertInputType = (view, expectedType) -> input = view.$el.find('.setting-input') expect(input.length).toEqual(1) + + # Some browsers (e.g. FireFox) do not support the "number" + # input type. We can accept a "text" input instead + # and still get acceptable behavior in the UI. + if expectedType == 'number' and input[0].type != 'number' + expectedType = 'text' + expect(input[0].type).toEqual(expectedType) assertValueInView = (view, expectedValue) -> From 58c6b9bb6145f2ee0be447b47bc430790eb38685 Mon Sep 17 00:00:00 2001 From: Miles Steele Date: Thu, 8 Aug 2013 10:09:03 -0400 Subject: [PATCH 13/35] add privilege copy --- .../instructor_dashboard_2/membership.html | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/lms/templates/instructor/instructor_dashboard_2/membership.html b/lms/templates/instructor/instructor_dashboard_2/membership.html index c226f74215..b22d31e190 100644 --- a/lms/templates/instructor/instructor_dashboard_2/membership.html +++ b/lms/templates/instructor/instructor_dashboard_2/membership.html @@ -59,10 +59,10 @@ data-rolename="staff" data-display-name="Course Staff" data-info-text=" - Course staff can help you manage limited aspects of your course. Staff can - enroll and unenroll students, as well as modify their grades and see all - course data. Course staff are not given access to Studio will not be able to - edit your course." + Course staff can help you manage limited aspects of your course. Staff + can enroll and unenroll students, as well as modify their grades and + see all course data. Course staff are not automatically given access + to Studio and will not be able to edit your course." data-list-endpoint="${ section_data['list_course_role_members_url'] }" data-modify-endpoint="${ section_data['modify_access_url'] }" data-add-button-label="Add Staff" @@ -74,8 +74,7 @@ data-display-name="Instructors" data-info-text=" Instructors are the core administration of your course. Instructors can - add and remove course staff, as well as administer forum access. - " + add and remove course staff, as well as administer forum access." data-list-endpoint="${ section_data['list_course_role_members_url'] }" data-modify-endpoint="${ section_data['modify_access_url'] }" data-add-button-label="Add Instructor" @@ -88,7 +87,7 @@ data-info-text=" Beta testers can see course content before the rest of the students. They can make sure that the content works, but have no additional - privelages." + privileges." data-list-endpoint="${ section_data['list_course_role_members_url'] }" data-modify-endpoint="${ section_data['modify_access_url'] }" data-add-button-label="Add Beta Tester" @@ -99,6 +98,9 @@
Date: Thu, 8 Aug 2013 11:05:59 -0400 Subject: [PATCH 14/35] hide empty management list selector, add explanation text --- .../instructor_dashboard/membership.coffee | 2 + .../instructor_dashboard_2/membership.html | 52 ++++++++++--------- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/lms/static/coffee/src/instructor_dashboard/membership.coffee b/lms/static/coffee/src/instructor_dashboard/membership.coffee index 733480e268..a50cd2c3dd 100644 --- a/lms/static/coffee/src/instructor_dashboard/membership.coffee +++ b/lms/static/coffee/src/instructor_dashboard/membership.coffee @@ -463,6 +463,8 @@ class Membership text: auth_list.$container.data 'display-name' data: auth_list: auth_list + if @auth_lists.length is 0 + @$list_selector.hide() @$list_selector.change => $opt = @$list_selector.children('option:selected') diff --git a/lms/templates/instructor/instructor_dashboard_2/membership.html b/lms/templates/instructor/instructor_dashboard_2/membership.html index b22d31e190..0a96d23a27 100644 --- a/lms/templates/instructor/instructor_dashboard_2/membership.html +++ b/lms/templates/instructor/instructor_dashboard_2/membership.html @@ -54,6 +54,14 @@
+ %if not section_data['access']['instructor']: +

+ Staff cannot modify staff or beta tester lists. To modify these lists, + contact your instructor and ask them to add you as an instructor for staff + and beta lists, or a forum admin for forum management. +

+ %endif + %if section_data['access']['instructor']:
- %if section_data['access']['instructor']: -
- %endif +
- %endif - %if section_data['access']['instructor']: -
+
%endif %if section_data['access']['instructor'] or section_data['access']['forum_admin']: From e9aca1363641481f450683300c4f00f5efc78573 Mon Sep 17 00:00:00 2001 From: Miles Steele Date: Thu, 8 Aug 2013 10:09:47 -0400 Subject: [PATCH 15/35] enable beta dashboard --- lms/envs/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/envs/common.py b/lms/envs/common.py index 0cbcbb774a..de78816a10 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -145,7 +145,7 @@ MITX_FEATURES = { 'ENABLE_INSTRUCTOR_BACKGROUND_TASKS': True, # Enable instructor dash beta version link - 'ENABLE_INSTRUCTOR_BETA_DASHBOARD': False, + 'ENABLE_INSTRUCTOR_BETA_DASHBOARD': True, # Allow use of the hint managment instructor view. 'ENABLE_HINTER_INSTRUCTOR_VIEW': False, From 28b0ba5e10309a878a1a30645706e195b149659f Mon Sep 17 00:00:00 2001 From: Vasyl Nakvasiuk Date: Thu, 8 Aug 2013 14:07:55 +0300 Subject: [PATCH 16/35] Migrate video tests to videoalpha tests, remove video tests. --- .../xmodule/tests/test_video_module.py | 104 --------------- .../xmodule/xmodule/tests/test_video_xml.py | 120 ------------------ .../courseware/tests/test_video_mongo.py | 27 ---- .../courseware/tests/test_videoalpha_xml.py | 75 ++++++++++- 4 files changed, 73 insertions(+), 253 deletions(-) delete mode 100644 common/lib/xmodule/xmodule/tests/test_video_module.py delete mode 100644 common/lib/xmodule/xmodule/tests/test_video_xml.py delete mode 100644 lms/djangoapps/courseware/tests/test_video_mongo.py diff --git a/common/lib/xmodule/xmodule/tests/test_video_module.py b/common/lib/xmodule/xmodule/tests/test_video_module.py deleted file mode 100644 index e11686176a..0000000000 --- a/common/lib/xmodule/xmodule/tests/test_video_module.py +++ /dev/null @@ -1,104 +0,0 @@ -# -*- coding: utf-8 -*- -import unittest - -from xmodule.modulestore import Location -from xmodule.video_module import VideoDescriptor -from .test_import import DummySystem - - -class VideoDescriptorImportTestCase(unittest.TestCase): - """ - Make sure that VideoDescriptor can import an old XML-based video correctly. - """ - - def test_constructor(self): - sample_xml = ''' - - ''' - location = Location(["i4x", "edX", "video", "default", - "SampleProblem1"]) - model_data = {'data': sample_xml, - 'location': location} - system = DummySystem(load_error_modules=True) - descriptor = VideoDescriptor(system, model_data) - self.assertEquals(descriptor.youtube_id_0_75, 'izygArpw-Qo') - self.assertEquals(descriptor.youtube_id_1_0, 'p2Q6BrNhdh8') - self.assertEquals(descriptor.youtube_id_1_25, '1EeWXzPdhSA') - self.assertEquals(descriptor.youtube_id_1_5, 'rABDYkeK0x8') - self.assertEquals(descriptor.show_captions, False) - self.assertEquals(descriptor.start_time, 1.0) - self.assertEquals(descriptor.end_time, 60) - self.assertEquals(descriptor.track, 'http://www.example.com/track') - self.assertEquals(descriptor.source, 'http://www.example.com/source.mp4') - - def test_from_xml(self): - module_system = DummySystem(load_error_modules=True) - xml_data = ''' - - ''' - output = VideoDescriptor.from_xml(xml_data, module_system) - self.assertEquals(output.youtube_id_0_75, 'izygArpw-Qo') - self.assertEquals(output.youtube_id_1_0, 'p2Q6BrNhdh8') - self.assertEquals(output.youtube_id_1_25, '1EeWXzPdhSA') - self.assertEquals(output.youtube_id_1_5, 'rABDYkeK0x8') - self.assertEquals(output.show_captions, False) - self.assertEquals(output.start_time, 1.0) - self.assertEquals(output.end_time, 60) - self.assertEquals(output.track, 'http://www.example.com/track') - self.assertEquals(output.source, 'http://www.example.com/source.mp4') - - def test_from_xml_missing_attributes(self): - """ - Ensure that attributes have the right values if they aren't - explicitly set in XML. - """ - module_system = DummySystem(load_error_modules=True) - xml_data = ''' - - ''' - output = VideoDescriptor.from_xml(xml_data, module_system) - self.assertEquals(output.youtube_id_0_75, '') - self.assertEquals(output.youtube_id_1_0, 'p2Q6BrNhdh8') - self.assertEquals(output.youtube_id_1_25, '1EeWXzPdhSA') - self.assertEquals(output.youtube_id_1_5, '') - self.assertEquals(output.show_captions, True) - self.assertEquals(output.start_time, 0.0) - self.assertEquals(output.end_time, 0.0) - self.assertEquals(output.track, 'http://www.example.com/track') - self.assertEquals(output.source, 'http://www.example.com/source.mp4') - - def test_from_xml_no_attributes(self): - """ - Make sure settings are correct if none are explicitly set in XML. - """ - module_system = DummySystem(load_error_modules=True) - xml_data = '' - output = VideoDescriptor.from_xml(xml_data, module_system) - self.assertEquals(output.youtube_id_0_75, '') - self.assertEquals(output.youtube_id_1_0, 'OEoXaMPEzfM') - self.assertEquals(output.youtube_id_1_25, '') - self.assertEquals(output.youtube_id_1_5, '') - self.assertEquals(output.show_captions, True) - self.assertEquals(output.start_time, 0.0) - self.assertEquals(output.end_time, 0.0) - self.assertEquals(output.track, '') - self.assertEquals(output.source, '') diff --git a/common/lib/xmodule/xmodule/tests/test_video_xml.py b/common/lib/xmodule/xmodule/tests/test_video_xml.py deleted file mode 100644 index 1ccc633ee2..0000000000 --- a/common/lib/xmodule/xmodule/tests/test_video_xml.py +++ /dev/null @@ -1,120 +0,0 @@ -# -*- coding: utf-8 -*- -"""Test for Video Xmodule functional logic. -These tests data readed from xml, not from mongo. - -We have a ModuleStoreTestCase class defined in -common/lib/xmodule/xmodule/modulestore/tests/django_utils.py. -You can search for usages of this in the cms and lms tests for examples. -You use this so that it will do things like point the modulestore -setting to mongo, flush the contentstore before and after, load the -templates, etc. -You can then use the CourseFactory and XModuleItemFactory as defined in -common/lib/xmodule/xmodule/modulestore/tests/factories.py to create the -course, section, subsection, unit, etc. -""" - -from mock import Mock - -from xmodule.video_module import VideoDescriptor, VideoModule, _parse_time, _parse_youtube -from xmodule.modulestore import Location -from xmodule.tests import get_test_system -from xmodule.tests import LogicTest - - -class VideoFactory(object): - """A helper class to create video modules with various parameters - for testing. - """ - - # tag that uses youtube videos - sample_problem_xml_youtube = """ - - """ - - @staticmethod - def create(): - """Method return Video Xmodule instance.""" - location = Location(["i4x", "edX", "video", "default", - "SampleProblem1"]) - model_data = {'data': VideoFactory.sample_problem_xml_youtube, 'location': location} - - descriptor = Mock(weight="1", url_name="SampleProblem1") - - system = get_test_system() - system.render_template = lambda template, context: context - module = VideoModule(system, descriptor, model_data) - - return module - - -class VideoModuleLogicTest(LogicTest): - """Tests for logic of Video Xmodule.""" - - descriptor_class = VideoDescriptor - - raw_model_data = { - 'data': '