From ccbff2dafc29d5ca8185e8d79cab99eea900e8a1 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 2 Dec 2014 13:54:51 -0500 Subject: [PATCH 1/6] Make a generic django field for storing OpaqueKey classes --- common/djangoapps/xmodule_django/models.py | 101 +++++++++------------ 1 file changed, 43 insertions(+), 58 deletions(-) diff --git a/common/djangoapps/xmodule_django/models.py b/common/djangoapps/xmodule_django/models.py index bcb287dc0b..b6449884e2 100644 --- a/common/djangoapps/xmodule_django/models.py +++ b/common/djangoapps/xmodule_django/models.py @@ -1,3 +1,5 @@ +import warnings + from django.db import models from django.core.exceptions import ValidationError from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location @@ -5,8 +7,6 @@ from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locator import Locator from south.modelsinspector import add_introspection_rules -add_introspection_rules([], ["^xmodule_django\.models\.CourseKeyField"]) -add_introspection_rules([], ["^xmodule_django\.models\.LocationKeyField"]) class NoneToEmptyManager(models.Manager): @@ -67,32 +67,50 @@ def _strip_value(value, lookup='exact'): return stripped_value -class CourseKeyField(models.CharField): - description = "A CourseKey object, saved to the DB in the form of a string" +class OpaqueKeyField(models.CharField): + """ + A django field for storing OpaqueKeys. + + The baseclass will return the value from the database as a string, rather than an instance + of an OpaqueKey, leaving the application to determine which key subtype to parse the string + as. + + Subclasses must specify a KEY_CLASS attribute, in which case the field will use :meth:`from_string` + to parse the key string, and will return an instance of KEY_CLASS. + """ + description = "An OpaqueKey object, saved to the DB in the form of a string." __metaclass__ = models.SubfieldBase Empty = object() + KEY_CLASS = None + + def __init__(self, *args, **kwargs): + if self.KEY_CLASS is None: + raise ValueError('Must specify KEY_CLASS in OpaqueKeyField subclasses') + + super(OpaqueKeyField, self).__init__(*args, **kwargs) + def to_python(self, value): if value is self.Empty or value is None: return None - assert isinstance(value, (basestring, CourseKey)) + assert isinstance(value, (basestring, self.KEY_CLASS)) if value == '': # handle empty string for models being created w/o fields populated return None if isinstance(value, basestring): - return CourseKey.from_string(value) + return self.KEY_CLASS.from_string(value) else: return value def get_prep_lookup(self, lookup, value): if lookup == 'isnull': - raise TypeError('Use CourseKeyField.Empty rather than None to query for a missing CourseKeyField') + raise TypeError('Use {0}.Empty rather than None to query for a missing {0}'.format(self.__class__.__name__)) - return super(CourseKeyField, self).get_prep_lookup( + return super(OpaqueKeyField, self).get_prep_lookup( lookup, # strip key before comparing _strip_value(value, lookup) @@ -102,7 +120,7 @@ class CourseKeyField(models.CharField): if value is self.Empty or value is None: return '' # CharFields should use '' as their empty value, rather than None - assert isinstance(value, CourseKey) + assert isinstance(value, self.KEY_CLASS) return unicode(_strip_value(value)) def validate(self, value, model_instance): @@ -111,66 +129,33 @@ class CourseKeyField(models.CharField): if not self.blank and value is self.Empty: raise ValidationError(self.error_messages['blank']) else: - return super(CourseKeyField, self).validate(value, model_instance) + return super(OpaqueKeyField, self).validate(value, model_instance) def run_validators(self, value): """Validate Empty values, otherwise defer to the parent""" if value is self.Empty: return - return super(CourseKeyField, self).run_validators(value) + return super(OpaqueKeyField, self).run_validators(value) -class LocationKeyField(models.CharField): +class CourseKeyField(OpaqueKeyField): + description = "A CourseKey object, saved to the DB in the form of a string" + KEY_CLASS = CourseKey + + +class UsageKeyField(OpaqueKeyField): description = "A Location object, saved to the DB in the form of a string" + KEY_CLASS = UsageKey - __metaclass__ = models.SubfieldBase - Empty = object() +class LocationKeyField(UsageKeyField): + def __init__(self, *args, **kwargs): + warnings.warn("LocationKeyField is deprecated. Please use UsageKeyField instead.", stacklevel=2) + super(LocationKeyField, self).__init__(*args, **kwargs) - def to_python(self, value): - if value is self.Empty or value is None: - return value - assert isinstance(value, (basestring, UsageKey)) - if value == '': - return None - - if isinstance(value, basestring): - return Location.from_deprecated_string(value) - else: - return value - - def get_prep_lookup(self, lookup, value): - if lookup == 'isnull': - raise TypeError('Use LocationKeyField.Empty rather than None to query for a missing LocationKeyField') - - # remove version and branch info before comparing keys - return super(LocationKeyField, self).get_prep_lookup( - lookup, - # strip key before comparing - _strip_value(value, lookup) - ) - - def get_prep_value(self, value): - if value is self.Empty: - return '' - - assert isinstance(value, UsageKey) - return unicode(_strip_value(value)) - - def validate(self, value, model_instance): - """Validate Empty values, otherwise defer to the parent""" - # raise validation error if the use of this field says it can't be blank but it is - if not self.blank and value is self.Empty: - raise ValidationError(self.error_messages['blank']) - else: - return super(LocationKeyField, self).validate(value, model_instance) - - def run_validators(self, value): - """Validate Empty values, otherwise defer to the parent""" - if value is self.Empty: - return - - return super(LocationKeyField, self).run_validators(value) +add_introspection_rules([], ["^xmodule_django\.models\.CourseKeyField"]) +add_introspection_rules([], ["^xmodule_django\.models\.LocationKeyField"]) +add_introspection_rules([], ["^xmodule_django\.models\.UsageKeyField"]) From 42792e978c977407412372c64b2e535a27d48afd Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 2 Dec 2014 15:20:18 -0500 Subject: [PATCH 2/6] Extract common base functionality for storing xblock fields in the relation db --- lms/djangoapps/courseware/models.py | 91 ++++++++++------------------- 1 file changed, 30 insertions(+), 61 deletions(-) diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index c6f5565251..b246e6f8c4 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -130,7 +130,34 @@ class StudentModuleHistory(models.Model): history_entry.save() -class XModuleUserStateSummaryField(models.Model): +class XBlockFieldBase(models.Model): + """ + Base class for all XBlock field storage. + """ + class Meta: + abstract = True + + # The name of the field + field_name = models.CharField(max_length=64, db_index=True) + + # The value of the field. Defaults to None dumped as json + value = models.TextField(default='null') + + created = models.DateTimeField(auto_now_add=True, db_index=True) + modified = models.DateTimeField(auto_now=True, db_index=True) + + def __unicode__(self): + return u'{}<{!r}'.format( + self.__class__.__name__, + { + key:getattr(self, key) + for key in self._meta.get_all_field_names() + if key not in ('created', 'modified') + } + ) + + +class XModuleUserStateSummaryField(XBlockFieldBase): """ Stores data set in the Scope.user_state_summary scope by an xmodule field """ @@ -138,30 +165,11 @@ class XModuleUserStateSummaryField(models.Model): class Meta: unique_together = (('usage_id', 'field_name'),) - # The name of the field - field_name = models.CharField(max_length=64, db_index=True) - # The definition id for the module usage_id = LocationKeyField(max_length=255, db_index=True) - # The value of the field. Defaults to None dumped as json - value = models.TextField(default='null') - created = models.DateTimeField(auto_now_add=True, db_index=True) - modified = models.DateTimeField(auto_now=True, db_index=True) - - def __repr__(self): - return 'XModuleUserStateSummaryField<%r>' % ({ - 'field_name': self.field_name, - 'usage_id': self.usage_id, - 'value': self.value, - },) - - def __unicode__(self): - return unicode(repr(self)) - - -class XModuleStudentPrefsField(models.Model): +class XModuleStudentPrefsField(XBlockFieldBase): """ Stores data set in the Scope.preferences scope by an xmodule field """ @@ -169,33 +177,13 @@ class XModuleStudentPrefsField(models.Model): class Meta: unique_together = (('student', 'module_type', 'field_name'),) - # The name of the field - field_name = models.CharField(max_length=64, db_index=True) - # The type of the module for these preferences module_type = models.CharField(max_length=64, db_index=True) - # The value of the field. Defaults to None dumped as json - value = models.TextField(default='null') - student = models.ForeignKey(User, db_index=True) - created = models.DateTimeField(auto_now_add=True, db_index=True) - modified = models.DateTimeField(auto_now=True, db_index=True) - def __repr__(self): - return 'XModuleStudentPrefsField<%r>' % ({ - 'field_name': self.field_name, - 'module_type': self.module_type, - 'student': self.student.username, - 'value': self.value, - },) - - def __unicode__(self): - return unicode(repr(self)) - - -class XModuleStudentInfoField(models.Model): +class XModuleStudentInfoField(XBlockFieldBase): """ Stores data set in the Scope.preferences scope by an xmodule field """ @@ -203,27 +191,8 @@ class XModuleStudentInfoField(models.Model): class Meta: unique_together = (('student', 'field_name'),) - # The name of the field - field_name = models.CharField(max_length=64, db_index=True) - - # The value of the field. Defaults to None dumped as json - value = models.TextField(default='null') - student = models.ForeignKey(User, db_index=True) - created = models.DateTimeField(auto_now_add=True, db_index=True) - modified = models.DateTimeField(auto_now=True, db_index=True) - - def __repr__(self): - return 'XModuleStudentInfoField<%r>' % ({ - 'field_name': self.field_name, - 'student': self.student.username, - 'value': self.value, - },) - - def __unicode__(self): - return unicode(repr(self)) - class OfflineComputedGrade(models.Model): """ From 980f30c17fd5af45364982703ef02e625373f4d6 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 5 Dec 2014 13:05:46 -0500 Subject: [PATCH 3/6] Move lms.lib.xblock into lms.djangoapps.lms_xblock in preparation add database backed configuration to it --- {lms/lib/xblock => cms/djangoapps}/__init__.py | 0 cms/djangoapps/contentstore/views/preview.py | 2 +- cms/envs/common.py | 2 +- lms/djangoapps/courseware/module_render.py | 4 ++-- lms/djangoapps/courseware/tests/__init__.py | 4 ++-- .../courseware/tests/test_lti_integration.py | 2 +- lms/djangoapps/courseware/tests/test_masquerade.py | 2 +- lms/djangoapps/courseware/tests/test_module_render.py | 3 ++- .../courseware/tests/test_submitting_problems.py | 2 +- lms/djangoapps/courseware/tests/tests.py | 2 +- .../instructor/views/instructor_dashboard.py | 2 +- .../instructor_task/tests/test_integration.py | 2 +- .../xblock/test => djangoapps/lms_xblock}/__init__.py | 0 .../xblock => djangoapps/lms_xblock}/field_data.py | 0 lms/{lib/xblock => djangoapps/lms_xblock}/mixin.py | 0 lms/{lib/xblock => djangoapps/lms_xblock}/runtime.py | 0 lms/djangoapps/lms_xblock/test/__init__.py | 0 .../lms_xblock}/test/test_runtime.py | 2 +- lms/djangoapps/notes/tests.py | 11 ++++++----- .../open_ended_grading/open_ended_notifications.py | 2 +- .../open_ended_grading/staff_grading_service.py | 2 +- lms/djangoapps/open_ended_grading/utils.py | 2 +- lms/envs/common.py | 2 +- pavelib/quality.py | 6 +++--- 24 files changed, 28 insertions(+), 26 deletions(-) rename {lms/lib/xblock => cms/djangoapps}/__init__.py (100%) rename lms/{lib/xblock/test => djangoapps/lms_xblock}/__init__.py (100%) rename lms/{lib/xblock => djangoapps/lms_xblock}/field_data.py (100%) rename lms/{lib/xblock => djangoapps/lms_xblock}/mixin.py (100%) rename lms/{lib/xblock => djangoapps/lms_xblock}/runtime.py (100%) create mode 100644 lms/djangoapps/lms_xblock/test/__init__.py rename lms/{lib/xblock => djangoapps/lms_xblock}/test/test_runtime.py (98%) diff --git a/lms/lib/xblock/__init__.py b/cms/djangoapps/__init__.py similarity index 100% rename from lms/lib/xblock/__init__.py rename to cms/djangoapps/__init__.py diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 772fc4c264..0d1dfec64f 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -22,7 +22,7 @@ from xblock.django.request import webob_to_django_response, django_to_webob_requ from xblock.exceptions import NoSuchHandlerError from xblock.fragment import Fragment -from lms.lib.xblock.field_data import LmsFieldData +from lms.djangoapps.lms_xblock.field_data import LmsFieldData from cms.lib.xblock.field_data import CmsFieldData from cms.lib.xblock.runtime import local_resource_url diff --git a/cms/envs/common.py b/cms/envs/common.py index 6951a08a43..d21242c8b5 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -36,7 +36,7 @@ from lms.envs.common import ( from path import path from warnings import simplefilter -from lms.lib.xblock.mixin import LmsBlockMixin +from lms.djangoapps.lms_xblock.mixin import LmsBlockMixin from dealer.git import git from xmodule.modulestore.edit_info import EditInfoMixin diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 5b79b9dd78..6294f46f00 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -21,8 +21,8 @@ from capa.xqueue_interface import XQueueInterface from courseware.access import has_access, get_user_role from courseware.masquerade import setup_masquerade from courseware.model_data import FieldDataCache, DjangoKeyValueStore -from lms.lib.xblock.field_data import LmsFieldData -from lms.lib.xblock.runtime import LmsModuleSystem, unquote_slashes, quote_slashes +from lms.djangoapps.lms_xblock.field_data import LmsFieldData +from lms.djangoapps.lms_xblock.runtime import LmsModuleSystem, unquote_slashes, quote_slashes from edxmako.shortcuts import render_to_string from eventtracking import tracker from psychometrics.psychoanalyze import make_psychometrics_data_update_handler diff --git a/lms/djangoapps/courseware/tests/__init__.py b/lms/djangoapps/courseware/tests/__init__.py index c0cf78fdb3..ede67b9778 100644 --- a/lms/djangoapps/courseware/tests/__init__.py +++ b/lms/djangoapps/courseware/tests/__init__.py @@ -20,8 +20,8 @@ from opaque_keys.edx.locations import Location from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from lms.lib.xblock.field_data import LmsFieldData -from lms.lib.xblock.runtime import quote_slashes +from lms.djangoapps.lms_xblock.field_data import LmsFieldData +from lms.djangoapps.lms_xblock.runtime import quote_slashes @override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) diff --git a/lms/djangoapps/courseware/tests/test_lti_integration.py b/lms/djangoapps/courseware/tests/test_lti_integration.py index 0811cf09a7..2c7da15823 100644 --- a/lms/djangoapps/courseware/tests/test_lti_integration.py +++ b/lms/djangoapps/courseware/tests/test_lti_integration.py @@ -13,7 +13,7 @@ from django.test.utils import override_settings from courseware.tests import BaseTestXmodule from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from courseware.views import get_course_lti_endpoints -from lms.lib.xblock.runtime import quote_slashes +from lms.djangoapps.lms_xblock.runtime import quote_slashes from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.x_module import STUDENT_VIEW diff --git a/lms/djangoapps/courseware/tests/test_masquerade.py b/lms/djangoapps/courseware/tests/test_masquerade.py index 8262dc6c6b..6a567cb466 100644 --- a/lms/djangoapps/courseware/tests/test_masquerade.py +++ b/lms/djangoapps/courseware/tests/test_masquerade.py @@ -15,7 +15,7 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey from courseware.tests.factories import StaffFactory from courseware.tests.helpers import LoginEnrollmentTestCase -from lms.lib.xblock.runtime import quote_slashes +from lms.djangoapps.lms_xblock.runtime import quote_slashes from xmodule.modulestore.django import modulestore, clear_existing_modulestores from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_GRADED_MODULESTORE diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 28c595fe8a..0c2bb313ee 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -30,9 +30,10 @@ from xmodule.modulestore.tests.django_utils import ( TEST_DATA_XML_MODULESTORE ) from courseware.tests.test_submitting_problems import TestSubmittingProblems -from lms.lib.xblock.runtime import quote_slashes +from lms.djangoapps.lms_xblock.runtime import quote_slashes from student.models import anonymous_id_for_user from xmodule.lti_module import LTIDescriptor + from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase diff --git a/lms/djangoapps/courseware/tests/test_submitting_problems.py b/lms/djangoapps/courseware/tests/test_submitting_problems.py index ae6a4b0dab..3a7ddd8939 100644 --- a/lms/djangoapps/courseware/tests/test_submitting_problems.py +++ b/lms/djangoapps/courseware/tests/test_submitting_problems.py @@ -21,7 +21,7 @@ from courseware import grades from courseware.models import StudentModule from courseware.tests.helpers import LoginEnrollmentTestCase from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE -from lms.lib.xblock.runtime import quote_slashes +from lms.djangoapps.lms_xblock.runtime import quote_slashes from student.tests.factories import UserFactory from student.models import anonymous_id_for_user from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index a748f93666..def1dffbe2 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -12,7 +12,7 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey from courseware.tests.helpers import LoginEnrollmentTestCase from xmodule.modulestore.tests.django_utils import TEST_DATA_XML_MODULESTORE as XML_MODULESTORE from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_TOY_MODULESTORE as TOY_MODULESTORE -from lms.lib.xblock.field_data import LmsFieldData +from lms.djangoapps.lms_xblock.field_data import LmsFieldData from xmodule.error_module import ErrorDescriptor from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index 0ba515b7d4..7a2ed19fad 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -19,7 +19,7 @@ from django.http import Http404 from django.conf import settings from util.json_request import JsonResponse -from lms.lib.xblock.runtime import quote_slashes +from lms.djangoapps.lms_xblock.runtime import quote_slashes from xmodule_modifiers import wrap_xblock from xmodule.html_module import HtmlDescriptor from xmodule.modulestore.django import modulestore diff --git a/lms/djangoapps/instructor_task/tests/test_integration.py b/lms/djangoapps/instructor_task/tests/test_integration.py index 68c1a44cbc..72580de00b 100644 --- a/lms/djangoapps/instructor_task/tests/test_integration.py +++ b/lms/djangoapps/instructor_task/tests/test_integration.py @@ -32,7 +32,7 @@ from instructor_task.tasks_helper import upload_grades_csv from instructor_task.tests.test_base import (InstructorTaskModuleTestCase, TestReportMixin, TEST_COURSE_ORG, TEST_COURSE_NUMBER, OPTION_1, OPTION_2) from capa.responsetypes import StudentInputError -from lms.lib.xblock.runtime import quote_slashes +from lms.djangoapps.lms_xblock.runtime import quote_slashes log = logging.getLogger(__name__) diff --git a/lms/lib/xblock/test/__init__.py b/lms/djangoapps/lms_xblock/__init__.py similarity index 100% rename from lms/lib/xblock/test/__init__.py rename to lms/djangoapps/lms_xblock/__init__.py diff --git a/lms/lib/xblock/field_data.py b/lms/djangoapps/lms_xblock/field_data.py similarity index 100% rename from lms/lib/xblock/field_data.py rename to lms/djangoapps/lms_xblock/field_data.py diff --git a/lms/lib/xblock/mixin.py b/lms/djangoapps/lms_xblock/mixin.py similarity index 100% rename from lms/lib/xblock/mixin.py rename to lms/djangoapps/lms_xblock/mixin.py diff --git a/lms/lib/xblock/runtime.py b/lms/djangoapps/lms_xblock/runtime.py similarity index 100% rename from lms/lib/xblock/runtime.py rename to lms/djangoapps/lms_xblock/runtime.py diff --git a/lms/djangoapps/lms_xblock/test/__init__.py b/lms/djangoapps/lms_xblock/test/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/lib/xblock/test/test_runtime.py b/lms/djangoapps/lms_xblock/test/test_runtime.py similarity index 98% rename from lms/lib/xblock/test/test_runtime.py rename to lms/djangoapps/lms_xblock/test/test_runtime.py index dd6f9200fd..3345fbcc57 100644 --- a/lms/lib/xblock/test/test_runtime.py +++ b/lms/djangoapps/lms_xblock/test/test_runtime.py @@ -9,7 +9,7 @@ from mock import Mock from unittest import TestCase from urlparse import urlparse from opaque_keys.edx.locations import SlashSeparatedCourseKey -from lms.lib.xblock.runtime import quote_slashes, unquote_slashes, LmsModuleSystem +from lms.djangoapps.lms_xblock.runtime import quote_slashes, unquote_slashes, LmsModuleSystem TEST_STRINGS = [ '', diff --git a/lms/djangoapps/notes/tests.py b/lms/djangoapps/notes/tests.py index 3484fd66eb..4da4e7147d 100644 --- a/lms/djangoapps/notes/tests.py +++ b/lms/djangoapps/notes/tests.py @@ -2,6 +2,8 @@ Unit tests for the notes app. """ +from mock import patch, Mock + from opaque_keys.edx.locations import SlashSeparatedCourseKey from django.test import TestCase from django.test.client import Client @@ -12,7 +14,7 @@ from django.core.exceptions import ValidationError import collections import json -from . import utils, api, models +from notes import utils, api, models class UtilsTest(TestCase): @@ -49,7 +51,9 @@ class ApiTest(TestCase): self.client = Client() # Mocks - api.api_enabled = self.mock_api_enabled(True) + patcher = patch.object(api, 'api_enabled', Mock(return_value=True)) + patcher.start() + self.addCleanup(patcher.stop) # Create two accounts self.password = 'abc' @@ -73,9 +77,6 @@ class ApiTest(TestCase): # Make sure no note with this ID ever exists for testing purposes self.NOTE_ID_DOES_NOT_EXIST = 99999 - def mock_api_enabled(self, is_enabled): - return (lambda request, course_id: is_enabled) - def login(self, as_student=None): username = None password = self.password diff --git a/lms/djangoapps/open_ended_grading/open_ended_notifications.py b/lms/djangoapps/open_ended_grading/open_ended_notifications.py index 01267d8b9c..cf1185e1c7 100644 --- a/lms/djangoapps/open_ended_grading/open_ended_notifications.py +++ b/lms/djangoapps/open_ended_grading/open_ended_notifications.py @@ -10,7 +10,7 @@ from xmodule.open_ended_grading_classes.controller_query_service import Controll from xmodule.modulestore.django import ModuleI18nService from courseware.access import has_access -from lms.lib.xblock.runtime import LmsModuleSystem +from lms.djangoapps.lms_xblock.runtime import LmsModuleSystem from edxmako.shortcuts import render_to_string from student.models import unique_id_for_user from util.cache import cache diff --git a/lms/djangoapps/open_ended_grading/staff_grading_service.py b/lms/djangoapps/open_ended_grading/staff_grading_service.py index 0aeed52da2..7d7200fba8 100644 --- a/lms/djangoapps/open_ended_grading/staff_grading_service.py +++ b/lms/djangoapps/open_ended_grading/staff_grading_service.py @@ -14,7 +14,7 @@ from xmodule.open_ended_grading_classes.grading_service_module import GradingSer from xmodule.modulestore.django import ModuleI18nService from courseware.access import has_access -from lms.lib.xblock.runtime import LmsModuleSystem +from lms.djangoapps.lms_xblock.runtime import LmsModuleSystem from edxmako.shortcuts import render_to_string from student.models import unique_id_for_user diff --git a/lms/djangoapps/open_ended_grading/utils.py b/lms/djangoapps/open_ended_grading/utils.py index 6c7532985a..4ab0c4d7ac 100644 --- a/lms/djangoapps/open_ended_grading/utils.py +++ b/lms/djangoapps/open_ended_grading/utils.py @@ -9,7 +9,7 @@ from xmodule.open_ended_grading_classes.grading_service_module import GradingSer from django.utils.translation import ugettext as _ from django.conf import settings -from lms.lib.xblock.runtime import LmsModuleSystem +from lms.djangoapps.lms_xblock.runtime import LmsModuleSystem from edxmako.shortcuts import render_to_string diff --git a/lms/envs/common.py b/lms/envs/common.py index 7bfdd83070..a8b1c25012 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -34,7 +34,7 @@ from django.utils.translation import ugettext_lazy as _ from .discussionsettings import * from xmodule.modulestore.modulestore_settings import update_module_store_settings -from lms.lib.xblock.mixin import LmsBlockMixin +from lms.djangoapps.lms_xblock.mixin import LmsBlockMixin ################################### FEATURES ################################### # The display name of the platform to be used in templates/emails/etc. diff --git a/pavelib/quality.py b/pavelib/quality.py index 18fd9611e0..924ed00e5d 100644 --- a/pavelib/quality.py +++ b/pavelib/quality.py @@ -34,8 +34,8 @@ def find_fixme(options): apps_list = ' '.join(apps) pythonpath_prefix = ( - "PYTHONPATH={system}:{system}/djangoapps:{system}/" - "lib:common/djangoapps:common/lib".format( + "PYTHONPATH={system}:{system}/lib" + "common/djangoapps:common/lib".format( system=system ) ) @@ -83,7 +83,7 @@ def run_pylint(options): apps = [system] - for directory in ['djangoapps', 'lib']: + for directory in ['lib']: dirs = os.listdir(os.path.join(system, directory)) apps.extend([d for d in dirs if os.path.isdir(os.path.join(system, directory, d))]) From e20fe2b879dc1d8349d7c238439aff1a62dc8915 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 5 Dec 2014 13:32:00 -0500 Subject: [PATCH 4/6] Add a configuration model for managing XBlockAsides --- lms/djangoapps/__init__.py | 0 lms/djangoapps/lms_xblock/admin.py | 5 ++ .../lms_xblock/migrations/0001_initial.py | 74 +++++++++++++++++++ .../lms_xblock/migrations/__init__.py | 0 lms/djangoapps/lms_xblock/models.py | 13 ++++ 5 files changed, 92 insertions(+) create mode 100644 lms/djangoapps/__init__.py create mode 100644 lms/djangoapps/lms_xblock/admin.py create mode 100644 lms/djangoapps/lms_xblock/migrations/0001_initial.py create mode 100644 lms/djangoapps/lms_xblock/migrations/__init__.py create mode 100644 lms/djangoapps/lms_xblock/models.py diff --git a/lms/djangoapps/__init__.py b/lms/djangoapps/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/lms_xblock/admin.py b/lms/djangoapps/lms_xblock/admin.py new file mode 100644 index 0000000000..451903245c --- /dev/null +++ b/lms/djangoapps/lms_xblock/admin.py @@ -0,0 +1,5 @@ +from django.contrib import admin +from config_models.admin import ConfigurationModelAdmin +from lms.djangoapps.lms_xblock.models import XBlockAsidesConfig + +admin.site.register(XBlockAsidesConfig, ConfigurationModelAdmin) diff --git a/lms/djangoapps/lms_xblock/migrations/0001_initial.py b/lms/djangoapps/lms_xblock/migrations/0001_initial.py new file mode 100644 index 0000000000..f4cdb0c97d --- /dev/null +++ b/lms/djangoapps/lms_xblock/migrations/0001_initial.py @@ -0,0 +1,74 @@ +# -*- coding: utf-8 -*- +import datetime +from south.db import db +from south.v2 import SchemaMigration +from django.db import models + + +class Migration(SchemaMigration): + + def forwards(self, orm): + # Adding model 'XBlockAsidesConfig' + db.create_table('lms_xblock_xblockasidesconfig', ( + ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)), + ('change_date', self.gf('django.db.models.fields.DateTimeField')(auto_now_add=True, blank=True)), + ('changed_by', self.gf('django.db.models.fields.related.ForeignKey')(to=orm['auth.User'], null=True, on_delete=models.PROTECT)), + ('enabled', self.gf('django.db.models.fields.BooleanField')(default=False)), + ('disabled_blocks', self.gf('django.db.models.fields.TextField')(default='about course_info static_tab')), + )) + db.send_create_signal('lms_xblock', ['XBlockAsidesConfig']) + + + def backwards(self, orm): + # Deleting model 'XBlockAsidesConfig' + db.delete_table('lms_xblock_xblockasidesconfig') + + + models = { + 'auth.group': { + 'Meta': {'object_name': 'Group'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}), + 'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}) + }, + 'auth.permission': { + 'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'}, + 'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + }, + 'auth.user': { + 'Meta': {'object_name': 'User'}, + 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}), + 'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}), + 'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'}) + }, + 'contenttypes.contenttype': { + 'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"}, + 'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'}) + }, + 'lms_xblock.xblockasidesconfig': { + 'Meta': {'object_name': 'XBlockAsidesConfig'}, + 'change_date': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}), + 'changed_by': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'null': 'True', 'on_delete': 'models.PROTECT'}), + 'disabled_blocks': ('django.db.models.fields.TextField', [], {'default': "'about course_info static_tab'"}), + 'enabled': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}) + } + } + + complete_apps = ['lms_xblock'] \ No newline at end of file diff --git a/lms/djangoapps/lms_xblock/migrations/__init__.py b/lms/djangoapps/lms_xblock/migrations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/lms_xblock/models.py b/lms/djangoapps/lms_xblock/models.py new file mode 100644 index 0000000000..8d04dbae95 --- /dev/null +++ b/lms/djangoapps/lms_xblock/models.py @@ -0,0 +1,13 @@ +from django.db.models import TextField + +from config_models.models import ConfigurationModel + +class XBlockAsidesConfig(ConfigurationModel): + """ + Configuration for XBlockAsides. + """ + + disabled_blocks = TextField( + default="about course_info static_tab", + help_text="Space-separated list of XBlocks on which XBlockAsides should never render." + ) From d919d2ae46b379ec6c601dd1256b623c206bc966 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 5 Dec 2014 13:39:53 -0500 Subject: [PATCH 5/6] Teach LMS how to render XBlockAsides [PLAT-217] --- cms/djangoapps/contentstore/views/assets.py | 9 +- cms/djangoapps/contentstore/views/item.py | 3 +- cms/djangoapps/contentstore/views/preview.py | 9 +- cms/lib/xblock/runtime.py | 11 ++ cms/static/coffee/spec/main.coffee | 1 + cms/static/coffee/spec/main_squire.coffee | 1 + cms/static/js_test.yml | 1 + cms/static/js_test_squire.yml | 1 + cms/static/require-config.js | 1 + common/djangoapps/xmodule_django/models.py | 29 +++- common/djangoapps/xmodule_modifiers.py | 13 +- common/lib/xmodule/xmodule/fields.py | 6 +- .../xmodule/xmodule/modulestore/mongo/base.py | 4 + .../split_mongo/caching_descriptor_system.py | 13 +- .../modulestore/split_mongo/id_manager.py | 32 ++++ .../modulestore/tests/test_libraries.py | 3 +- .../xmodule/modulestore/tests/test_mongo.py | 2 +- common/lib/xmodule/xmodule/modulestore/xml.py | 27 +++- common/lib/xmodule/xmodule/tests/__init__.py | 28 ++-- .../xmodule/xmodule/tests/test_conditional.py | 4 +- .../xmodule/tests/test_error_module.py | 8 +- .../lib/xmodule/xmodule/tests/xml/__init__.py | 4 +- common/lib/xmodule/xmodule/x_module.py | 132 +++++++++++++++- .../coffee/spec/xblock/core_spec.coffee | 10 +- common/static/coffee/src/xblock/core.coffee | 57 ------- common/static/js/xblock/core.js | 142 ++++++++++++++++++ common/static/js_test.yml | 1 + .../test/acceptance/pages/studio/container.py | 2 +- .../tests/lms/test_lms_acid_xblock.py | 66 ++++++-- lms/djangoapps/courseware/model_data.py | 71 ++++++--- lms/djangoapps/courseware/models.py | 13 +- lms/djangoapps/courseware/module_render.py | 14 +- .../courseware/tests/test_model_data.py | 5 +- lms/djangoapps/courseware/views.py | 5 +- .../instructor/views/instructor_dashboard.py | 26 +++- lms/djangoapps/lms_xblock/admin.py | 4 + lms/djangoapps/lms_xblock/field_data.py | 2 +- lms/djangoapps/lms_xblock/models.py | 18 +++ lms/djangoapps/lms_xblock/runtime.py | 52 ++++++- .../lms_xblock/test/test_runtime.py | 4 +- lms/djangoapps/open_ended_grading/tests.py | 2 +- lms/envs/common.py | 4 +- lms/static/js/spec/main.js | 2 +- lms/static/js_test.yml | 1 + lms/static/js_test_coffee.yml | 1 + pylintrc | 2 + requirements/edx/github.txt | 6 +- 47 files changed, 669 insertions(+), 183 deletions(-) create mode 100644 common/lib/xmodule/xmodule/modulestore/split_mongo/id_manager.py delete mode 100644 common/static/coffee/src/xblock/core.coffee create mode 100644 common/static/js/xblock/core.js diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py index 4067e06e48..a6b3ef2cd2 100644 --- a/cms/djangoapps/contentstore/views/assets.py +++ b/cms/djangoapps/contentstore/views/assets.py @@ -201,10 +201,11 @@ def _upload_asset(request, course_key): 'File {filename} exceeds maximum size of ' '{size_mb} MB. Please follow the instructions here ' 'to upload a file elsewhere and link to it instead: ' - '{faq_url}').format( - filename=filename, - size_mb=settings.MAX_ASSET_UPLOAD_FILE_SIZE_IN_MB, - faq_url=settings.MAX_ASSET_UPLOAD_FILE_SIZE_URL, + '{faq_url}' + ).format( + filename=filename, + size_mb=settings.MAX_ASSET_UPLOAD_FILE_SIZE_IN_MB, + faq_url=settings.MAX_ASSET_UPLOAD_FILE_SIZE_URL, ) }, status=413) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 28c9a5e480..ffbcd68d18 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -45,7 +45,7 @@ from contentstore.views.helpers import is_unit, xblock_studio_url, xblock_primar from contentstore.views.preview import get_preview_fragment from edxmako.shortcuts import render_to_string from models.settings.course_grading import CourseGradingModel -from cms.lib.xblock.runtime import handler_url, local_resource_url +from cms.lib.xblock.runtime import handler_url, local_resource_url, get_asides from opaque_keys.edx.keys import UsageKey, CourseKey __all__ = ['orphan_handler', 'xblock_handler', 'xblock_view_handler', 'xblock_outline_handler'] @@ -64,6 +64,7 @@ ALWAYS = lambda x: True # TODO: Remove this code when Runtimes are no longer created by modulestores xmodule.x_module.descriptor_global_handler_url = handler_url xmodule.x_module.descriptor_global_local_resource_url = local_resource_url +xmodule.x_module.descriptor_global_get_asides = get_asides def hash_resource(resource): diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 0d1dfec64f..a67b6019b6 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -95,6 +95,10 @@ class PreviewModuleSystem(ModuleSystem): # pylint: disable=abstract-method def local_resource_url(self, block, uri): return local_resource_url(block, uri) + def get_asides(self, block): + # TODO: Implement this to enable XBlockAsides on previews in Studio + return [] + class StudioUserService(object): """ @@ -110,7 +114,7 @@ class StudioUserService(object): return self._request.user.id -def _preview_module_system(request, descriptor): +def _preview_module_system(request, descriptor, field_data): """ Returns a ModuleSystem for the specified descriptor that is specialized for rendering module previews. @@ -163,6 +167,7 @@ def _preview_module_system(request, descriptor): descriptor_runtime=descriptor.runtime, services={ "i18n": ModuleI18nService(), + "field-data": field_data, }, ) @@ -181,7 +186,7 @@ def _load_preview_module(request, descriptor): else: field_data = LmsFieldData(descriptor._field_data, student_data) # pylint: disable=protected-access descriptor.bind_for_student( - _preview_module_system(request, descriptor), + _preview_module_system(request, descriptor, field_data), field_data ) return descriptor diff --git a/cms/lib/xblock/runtime.py b/cms/lib/xblock/runtime.py index b460f175c2..bd2d533ad1 100644 --- a/cms/lib/xblock/runtime.py +++ b/cms/lib/xblock/runtime.py @@ -33,3 +33,14 @@ def local_resource_url(block, uri): 'block_type': block.scope_ids.block_type, 'uri': uri, }) + + +def get_asides(block): # pylint: disable=unused-argument + """ + Return all of the asides which might be decorating this `block`. + + Arguments: + block (:class:`.XBlock`): The block to render retrieve asides for. + """ + # TODO: Implement this method to make XBlockAsides for editing views in Studio + return [] diff --git a/cms/static/coffee/spec/main.coffee b/cms/static/coffee/spec/main.coffee index 7ab87a4461..b9b3546dbf 100644 --- a/cms/static/coffee/spec/main.coffee +++ b/cms/static/coffee/spec/main.coffee @@ -32,6 +32,7 @@ requirejs.config({ "jquery.tinymce": "xmodule_js/common_static/js/vendor/tinymce/js/tinymce/jquery.tinymce", "xmodule": "xmodule_js/src/xmodule", "xblock/cms.runtime.v1": "coffee/src/xblock/cms.runtime.v1", + "xblock/core": "xmodule_js/common_static/js/xblock/core", "xblock": "xmodule_js/common_static/coffee/src/xblock", "utility": "xmodule_js/common_static/js/src/utility", "accessibility": "xmodule_js/common_static/js/src/accessibility_tools", diff --git a/cms/static/coffee/spec/main_squire.coffee b/cms/static/coffee/spec/main_squire.coffee index 66669b374f..24d855a75d 100644 --- a/cms/static/coffee/spec/main_squire.coffee +++ b/cms/static/coffee/spec/main_squire.coffee @@ -30,6 +30,7 @@ requirejs.config({ "jquery.tinymce": "xmodule_js/common_static/js/vendor/tinymce/js/tinymce/jquery.tinymce", "xmodule": "xmodule_js/src/xmodule", "xblock/cms.runtime.v1": "coffee/src/xblock/cms.runtime.v1", + "xblock/core": "xmodule_js/common_static/js/xblock/core", "xblock": "xmodule_js/common_static/coffee/src/xblock", "utility": "xmodule_js/common_static/js/src/utility", "sinon": "xmodule_js/common_static/js/vendor/sinon-1.7.1", diff --git a/cms/static/js_test.yml b/cms/static/js_test.yml index 2f039d5c02..52176290dd 100644 --- a/cms/static/js_test.yml +++ b/cms/static/js_test.yml @@ -60,6 +60,7 @@ lib_paths: - xmodule_js/common_static/js/vendor/URI.min.js - xmodule_js/common_static/js/vendor/jquery.smooth-scroll.min.js - xmodule_js/common_static/coffee/src/jquery.immediateDescendents.js + - xmodule_js/common_static/js/xblock/ - xmodule_js/common_static/coffee/src/xblock/ - xmodule_js/common_static/js/vendor/URI.min.js - xmodule_js/common_static/js/vendor/jQuery-File-Upload/js/jquery.iframe-transport.js diff --git a/cms/static/js_test_squire.yml b/cms/static/js_test_squire.yml index dbafe0859c..2dcb1e75ef 100644 --- a/cms/static/js_test_squire.yml +++ b/cms/static/js_test_squire.yml @@ -55,6 +55,7 @@ lib_paths: - xmodule_js/src/xmodule.js - xmodule_js/common_static/coffee/src/jquery.immediateDescendents.js - xmodule_js/common_static/js/test/i18n.js + - xmodule_js/common_static/js/xblock/ - xmodule_js/common_static/coffee/src/xblock/ - xmodule_js/common_static/js/vendor/URI.min.js - xmodule_js/common_static/js/vendor/jQuery-File-Upload/js/jquery.iframe-transport.js diff --git a/cms/static/require-config.js b/cms/static/require-config.js index 790312ed25..0c5646d037 100644 --- a/cms/static/require-config.js +++ b/cms/static/require-config.js @@ -35,6 +35,7 @@ require.config({ "tinymce": "js/vendor/tinymce/js/tinymce/tinymce.full.min", "jquery.tinymce": "js/vendor/tinymce/js/tinymce/jquery.tinymce.min", "xmodule": "/xmodule/xmodule", + "xblock/core": "js/xblock/core", "xblock": "coffee/src/xblock", "utility": "js/src/utility", "accessibility": "js/src/accessibility_tools", diff --git a/common/djangoapps/xmodule_django/models.py b/common/djangoapps/xmodule_django/models.py index b6449884e2..92257bc26d 100644 --- a/common/djangoapps/xmodule_django/models.py +++ b/common/djangoapps/xmodule_django/models.py @@ -1,9 +1,13 @@ +""" +Useful django models for implementing XBlock infrastructure in django. +""" + import warnings from django.db import models from django.core.exceptions import ValidationError from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location -from opaque_keys.edx.keys import CourseKey, UsageKey +from opaque_keys.edx.keys import CourseKey, UsageKey, BlockTypeKey from opaque_keys.edx.locator import Locator from south.modelsinspector import add_introspection_rules @@ -91,7 +95,6 @@ class OpaqueKeyField(models.CharField): super(OpaqueKeyField, self).__init__(*args, **kwargs) - def to_python(self, value): if value is self.Empty or value is None: return None @@ -140,22 +143,38 @@ class OpaqueKeyField(models.CharField): class CourseKeyField(OpaqueKeyField): + """ + A django Field that stores a CourseKey object as a string. + """ description = "A CourseKey object, saved to the DB in the form of a string" KEY_CLASS = CourseKey class UsageKeyField(OpaqueKeyField): + """ + A django Field that stores a UsageKey object as a string. + """ description = "A Location object, saved to the DB in the form of a string" KEY_CLASS = UsageKey class LocationKeyField(UsageKeyField): + """ + A django Field that stores a UsageKey object as a string. + """ def __init__(self, *args, **kwargs): warnings.warn("LocationKeyField is deprecated. Please use UsageKeyField instead.", stacklevel=2) super(LocationKeyField, self).__init__(*args, **kwargs) +class BlockTypeKeyField(OpaqueKeyField): + """ + A django Field that stores a BlockTypeKey object as a string. + """ + description = "A BlockTypeKey object, saved to the DB in the form of a string." + KEY_CLASS = BlockTypeKey -add_introspection_rules([], ["^xmodule_django\.models\.CourseKeyField"]) -add_introspection_rules([], ["^xmodule_django\.models\.LocationKeyField"]) -add_introspection_rules([], ["^xmodule_django\.models\.UsageKeyField"]) + +add_introspection_rules([], [r"^xmodule_django\.models\.CourseKeyField"]) +add_introspection_rules([], [r"^xmodule_django\.models\.LocationKeyField"]) +add_introspection_rules([], [r"^xmodule_django\.models\.UsageKeyField"]) diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 2349f1e3be..4d3bb974cc 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -72,7 +72,11 @@ def wrap_xblock(runtime_class, block, view, frag, context, usage_id_serializer, data = {} data.update(extra_data) - css_classes = ['xblock', 'xblock-{}'.format(markupsafe.escape(view))] + + css_classes = [ + 'xblock', + 'xblock-{}'.format(markupsafe.escape(view)) + ] if isinstance(block, (XModule, XModuleDescriptor)): if view in PREVIEW_VIEWS: @@ -90,9 +94,10 @@ def wrap_xblock(runtime_class, block, view, frag, context, usage_id_serializer, data['init'] = frag.js_init_fn data['runtime-class'] = runtime_class data['runtime-version'] = frag.js_init_version - data['block-type'] = block.scope_ids.block_type - data['usage-id'] = usage_id_serializer(block.scope_ids.usage_id) - data['request-token'] = request_token + + data['block-type'] = block.scope_ids.block_type + data['usage-id'] = usage_id_serializer(block.scope_ids.usage_id) + data['request-token'] = request_token if block.name: data['name'] = block.name diff --git a/common/lib/xmodule/xmodule/fields.py b/common/lib/xmodule/xmodule/fields.py index 93618be63c..a3c6aa19d7 100644 --- a/common/lib/xmodule/xmodule/fields.py +++ b/common/lib/xmodule/xmodule/fields.py @@ -33,7 +33,7 @@ class Date(JSONField): result = dateutil.parser.parse(field, default=self.PREVENT_DEFAULT_DAY_MON_SEED1) result_other = dateutil.parser.parse(field, default=self.PREVENT_DEFAULT_DAY_MON_SEED2) if result != result_other: - log.warning("Field {0} is missing month or day".format(self._name, field)) + log.warning("Field {0} is missing month or day".format(self.name)) return None if result.tzinfo is None: result = result.replace(tzinfo=UTC) @@ -59,7 +59,7 @@ class Date(JSONField): return field else: msg = "Field {0} has bad value '{1}'".format( - self._name, field) + self.name, field) raise TypeError(msg) def to_json(self, value): @@ -199,7 +199,7 @@ class RelativeTime(JSONField): if isinstance(value, basestring): return self.isotime_to_timedelta(value) - msg = "RelativeTime Field {0} has bad value '{1!r}'".format(self._name, value) + msg = "RelativeTime Field {0} has bad value '{1!r}'".format(self.name, value) raise TypeError(msg) def to_json(self, value): diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 9f8899e399..5d432525c1 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -50,6 +50,7 @@ from xmodule.modulestore.draft_and_published import ModuleStoreDraftAndPublished from xmodule.modulestore.edit_info import EditInfoRuntimeMixin from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseError, ReferentialIntegrityError from xmodule.modulestore.inheritance import InheritanceMixin, inherit_metadata, InheritanceKeyValueStore +from xmodule.modulestore.xml import CourseLocationManager log = logging.getLogger(__name__) @@ -173,6 +174,9 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): render_template: a function for rendering templates, as per MakoDescriptorSystem """ + id_manager = CourseLocationManager(course_key) + kwargs.setdefault('id_reader', id_manager) + kwargs.setdefault('id_generator', id_manager) super(CachingDescriptorSystem, self).__init__( field_data=None, load_item=self.load_item, diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py index 7881be1c2f..205bf1eaf5 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py @@ -1,6 +1,7 @@ import sys import logging from contracts import contract, new_contract +from fs.osfs import OSFS from lazy import lazy from xblock.runtime import KvsFieldData from xblock.fields import ScopeIds @@ -8,13 +9,13 @@ from opaque_keys.edx.locator import BlockUsageLocator, LocalId, CourseLocator, L from xmodule.mako_module import MakoDescriptorSystem from xmodule.error_module import ErrorDescriptor from xmodule.errortracker import exc_info_to_str -from ..exceptions import ItemNotFoundError -from .split_mongo_kvs import SplitMongoKVS -from fs.osfs import OSFS -from .definition_lazy_loader import DefinitionLazyLoader from xmodule.modulestore.edit_info import EditInfoRuntimeMixin +from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.inheritance import inheriting_field_data, InheritanceMixin from xmodule.modulestore.split_mongo import BlockKey, CourseEnvelope +from xmodule.modulestore.split_mongo.id_manager import SplitMongoIdManager +from xmodule.modulestore.split_mongo.definition_lazy_loader import DefinitionLazyLoader +from xmodule.modulestore.split_mongo.split_mongo_kvs import SplitMongoKVS log = logging.getLogger(__name__) @@ -54,6 +55,10 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): root = modulestore.fs_root / course_entry.structure['_id'] root.makedirs_p() # create directory if it doesn't exist + id_manager = SplitMongoIdManager(self) + kwargs.setdefault('id_reader', id_manager) + kwargs.setdefault('id_generator', id_manager) + super(CachingDescriptorSystem, self).__init__( field_data=None, load_item=self._load_item, diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/id_manager.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/id_manager.py new file mode 100644 index 0000000000..4ff486372d --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/id_manager.py @@ -0,0 +1,32 @@ +""" +An implementation of IdReader and IdGenerator that manages ids for the SplitMongo storage +mechanism. +""" + +from opaque_keys.edx.locator import LocalId, DefinitionLocator +from xmodule.x_module import OpaqueKeyReader, AsideKeyGenerator +from xmodule.modulestore.split_mongo import BlockKey + + +# TODO: Migrate split_mongo to use this class for all key mapping/creation. +class SplitMongoIdManager(OpaqueKeyReader, AsideKeyGenerator): # pylint: disable=abstract-method + """ + An IdManager that knows how to retrieve the DefinitionLocator, given + a usage_id and a :class:`.CachingDescriptorSystem`. + """ + def __init__(self, caching_descriptor_system): + self._cds = caching_descriptor_system + + def get_definition_id(self, usage_id): + if isinstance(usage_id.block_id, LocalId): + # a LocalId indicates that this block hasn't been persisted yet, and is instead stored + # in-memory in the local_modules dictionary. + return self._cds.local_modules[usage_id].scope_ids.def_id + else: + block_key = BlockKey.from_usage_key(usage_id) + module_data = self._cds.get_module_data(block_key, usage_id.course_key) + + if 'definition' in module_data: + return DefinitionLocator(usage_id.block_type, module_data['definition']) + else: + raise ValueError("All non-local blocks should have a definition specified") diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py b/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py index c380da1e8b..1447b02384 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py @@ -203,5 +203,6 @@ class TestLibraries(MixedSplitTestCase): message = u"Hello world" hello_render = lambda _, context: Fragment(message) with patch('xmodule.html_module.HtmlDescriptor.author_view', hello_render, create=True): - result = library.render(AUTHOR_VIEW, context) + with patch('xmodule.x_module.descriptor_global_get_asides', lambda block: []): + result = library.render(AUTHOR_VIEW, context) self.assertIn(message, result.content) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index a6f1dd5aa7..aa1f80443b 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -420,7 +420,7 @@ class TestMongoModuleStore(TestMongoModuleStoreBase): assert_equals(len(course_locations), 1) assert_in(SlashSeparatedCourseKey('edX', 'simple', '2012_Fall'), course_locations) - @Plugin.register_temp_plugin(ReferenceTestXBlock, 'ref_test') + @XBlock.register_temp_plugin(ReferenceTestXBlock, 'ref_test') def test_reference_converters(self): """ Test that references types get deserialized correctly diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index e6e08a416b..e6a0ca15c3 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -18,7 +18,7 @@ from contextlib import contextmanager from xmodule.error_module import ErrorDescriptor from xmodule.errortracker import make_error_tracker, exc_info_to_str from xmodule.mako_module import MakoDescriptorSystem -from xmodule.x_module import XMLParsingSystem, policy_key +from xmodule.x_module import XMLParsingSystem, policy_key, OpaqueKeyReader, AsideKeyGenerator from xmodule.modulestore.xml_exporter import DEFAULT_CONTENT_FIELDS from xmodule.modulestore import ModuleStoreEnum, ModuleStoreReadBase from xmodule.tabs import CourseTabList @@ -27,7 +27,7 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location from opaque_keys.edx.locator import CourseLocator from xblock.field_data import DictFieldData -from xblock.runtime import DictKeyValueStore, IdGenerator +from xblock.runtime import DictKeyValueStore from .exceptions import ItemNotFoundError @@ -64,7 +64,6 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): """ self.unnamed = defaultdict(int) # category -> num of new url_names for that category self.used_names = defaultdict(set) # category -> set of used url_names - id_generator = CourseLocationGenerator(course_id) # cdodge: adding the course_id as passed in for later reference rather than having to recomine the org/course/url_name self.course_id = course_id @@ -175,7 +174,7 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): descriptor = create_block_from_xml( etree.tostring(xml_data, encoding='unicode'), self, - id_generator, + id_manager, ) except Exception as err: # pylint: disable=broad-except if not self.load_error_modules: @@ -201,7 +200,7 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): descriptor = ErrorDescriptor.from_xml( xml, self, - id_generator, + id_manager, err_msg ) @@ -229,12 +228,16 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): resources_fs = OSFS(xmlstore.data_dir / course_dir) + id_manager = CourseLocationManager(course_id) + super(ImportSystem, self).__init__( load_item=load_item, resources_fs=resources_fs, render_template=render_template, error_tracker=error_tracker, process_xml=process_xml, + id_generator=id_manager, + id_reader=id_manager, **kwargs ) @@ -245,12 +248,13 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): block.children.append(child_block.scope_ids.usage_id) -class CourseLocationGenerator(IdGenerator): +class CourseLocationManager(OpaqueKeyReader, AsideKeyGenerator): """ IdGenerator for Location-based definition ids and usage ids based within a course """ def __init__(self, course_id): + super(CourseLocationManager, self).__init__() self.course_id = course_id self.autogen_ids = itertools.count(0) @@ -263,6 +267,17 @@ class CourseLocationGenerator(IdGenerator): slug = 'autogen_{}_{}'.format(block_type, self.autogen_ids.next()) return self.course_id.make_usage_key(block_type, slug) + def get_definition_id(self, usage_id): + """Retrieve the definition that a usage is derived from. + + Args: + usage_id: The id of the usage to query + + Returns: + The `definition_id` the usage is derived from + """ + return usage_id + def _make_usage_key(course_key, value): """ diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 44d9fc0123..eac771de45 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -28,6 +28,7 @@ from xmodule.mako_module import MakoDescriptorSystem from xmodule.error_module import ErrorDescriptor from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.mongo.draft import DraftModuleStore +from xmodule.modulestore.xml import CourseLocationManager from xmodule.modulestore.draft_and_published import DIRECT_ONLY_CATEGORIES, ModuleStoreDraftAndPublished @@ -51,9 +52,16 @@ class TestModuleSystem(ModuleSystem): # pylint: disable=abstract-method """ ModuleSystem for testing """ + def __init__(self, **kwargs): + id_manager = CourseLocationManager(kwargs['course_id']) + kwargs.setdefault('id_reader', id_manager) + kwargs.setdefault('id_generator', id_manager) + kwargs.setdefault('services', {}).setdefault('field-data', DictFieldData({})) + super(TestModuleSystem, self).__init__(**kwargs) + def handler_url(self, block, handler, suffix='', query='', thirdparty=False): return '{usage_id}/{handler}{suffix}?{query}'.format( - usage_id=block.scope_ids.usage_id.to_deprecated_string(), + usage_id=unicode(block.scope_ids.usage_id), handler=handler, suffix=suffix, query=query, @@ -61,10 +69,14 @@ class TestModuleSystem(ModuleSystem): # pylint: disable=abstract-method def local_resource_url(self, block, uri): return 'resource/{usage_id}/{uri}'.format( - usage_id=block.scope_ids.usage_id.to_deprecated_string(), + usage_id=unicode(block.scope_ids.usage_id), uri=uri, ) + # Disable XBlockAsides in most tests + def get_asides(self, block): + return [] + def get_test_system(course_id=SlashSeparatedCourseKey('org', 'course', 'run')): """ @@ -113,13 +125,16 @@ def get_test_descriptor_system(): """ Construct a test DescriptorSystem instance. """ + field_data = DictFieldData({}) + return MakoDescriptorSystem( load_item=Mock(), resources_fs=Mock(), error_tracker=Mock(), render_template=mock_render_template, mixins=(InheritanceMixin, XModuleMixin), - field_data=DictFieldData({}), + field_data=field_data, + services={'field-data': field_data}, ) @@ -149,13 +164,8 @@ class LogicTest(unittest.TestCase): raw_field_data = {} def setUp(self): - class EmptyClass: - """Empty object.""" - url_name = '' - category = 'test' - self.system = get_test_system() - self.descriptor = EmptyClass() + self.descriptor = Mock(name="descriptor", url_name='', category='test') self.xmodule_class = self.descriptor_class.module_class usage_key = self.system.course_id.make_usage_key(self.descriptor.category, 'test_loc') diff --git a/common/lib/xmodule/xmodule/tests/test_conditional.py b/common/lib/xmodule/xmodule/tests/test_conditional.py index bc20075704..373388a7ac 100644 --- a/common/lib/xmodule/xmodule/tests/test_conditional.py +++ b/common/lib/xmodule/xmodule/tests/test_conditional.py @@ -8,7 +8,7 @@ from xblock.field_data import DictFieldData from xblock.fields import ScopeIds from xmodule.error_module import NonStaffErrorDescriptor from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location -from xmodule.modulestore.xml import ImportSystem, XMLModuleStore, CourseLocationGenerator +from xmodule.modulestore.xml import ImportSystem, XMLModuleStore, CourseLocationManager from xmodule.conditional_module import ConditionalDescriptor from xmodule.tests import DATA_DIR, get_test_system, get_test_descriptor_system from xmodule.x_module import STUDENT_VIEW @@ -60,7 +60,7 @@ class ConditionalFactory(object): source_descriptor = NonStaffErrorDescriptor.from_xml( 'some random xml data', system, - id_generator=CourseLocationGenerator(SlashSeparatedCourseKey('edX', 'conditional_test', 'test_run')), + id_generator=CourseLocationManager(source_location.course_key), error_msg='random error message' ) else: diff --git a/common/lib/xmodule/xmodule/tests/test_error_module.py b/common/lib/xmodule/xmodule/tests/test_error_module.py index 85c28de7ca..ddb9207576 100644 --- a/common/lib/xmodule/xmodule/tests/test_error_module.py +++ b/common/lib/xmodule/xmodule/tests/test_error_module.py @@ -4,7 +4,7 @@ Tests for ErrorModule and NonStaffErrorModule import unittest from xmodule.tests import get_test_system from xmodule.error_module import ErrorDescriptor, ErrorModule, NonStaffErrorDescriptor -from xmodule.modulestore.xml import CourseLocationGenerator +from xmodule.modulestore.xml import CourseLocationManager from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location from xmodule.x_module import XModuleDescriptor, XModule, STUDENT_VIEW from mock import MagicMock, Mock, patch @@ -34,7 +34,7 @@ class TestErrorModule(unittest.TestCase, SetupTestErrorModules): descriptor = ErrorDescriptor.from_xml( self.valid_xml, self.system, - CourseLocationGenerator(self.course_id), + CourseLocationManager(self.course_id), self.error_msg ) self.assertIsInstance(descriptor, ErrorDescriptor) @@ -69,7 +69,7 @@ class TestNonStaffErrorModule(unittest.TestCase, SetupTestErrorModules): descriptor = NonStaffErrorDescriptor.from_xml( self.valid_xml, self.system, - CourseLocationGenerator(self.course_id) + CourseLocationManager(self.course_id) ) self.assertIsInstance(descriptor, NonStaffErrorDescriptor) @@ -77,7 +77,7 @@ class TestNonStaffErrorModule(unittest.TestCase, SetupTestErrorModules): descriptor = NonStaffErrorDescriptor.from_xml( self.valid_xml, self.system, - CourseLocationGenerator(self.course_id) + CourseLocationManager(self.course_id) ) descriptor.xmodule_runtime = self.system context_repr = self.system.render(descriptor, STUDENT_VIEW).content diff --git a/common/lib/xmodule/xmodule/tests/xml/__init__.py b/common/lib/xmodule/xmodule/tests/xml/__init__.py index 599fa49840..534d3d3d4c 100644 --- a/common/lib/xmodule/xmodule/tests/xml/__init__.py +++ b/common/lib/xmodule/xmodule/tests/xml/__init__.py @@ -7,7 +7,7 @@ from unittest import TestCase from xmodule.x_module import XMLParsingSystem, policy_key from xmodule.mako_module import MakoDescriptorSystem -from xmodule.modulestore.xml import create_block_from_xml, CourseLocationGenerator +from xmodule.modulestore.xml import create_block_from_xml, CourseLocationManager from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location from xblock.runtime import KvsFieldData, DictKeyValueStore @@ -43,7 +43,7 @@ class InMemorySystem(XMLParsingSystem, MakoDescriptorSystem): # pylint: disable descriptor = create_block_from_xml( xml, self, - CourseLocationGenerator(self.course_id), + CourseLocationManager(self.course_id), ) self._descriptors[descriptor.location.to_deprecated_string()] = descriptor return descriptor diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 5332296e15..5b96721153 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -18,12 +18,13 @@ from webob.multidict import MultiDict from xblock.core import XBlock from xblock.fields import Scope, Integer, Float, List, XBlockMixin, String, Dict from xblock.fragment import Fragment -from xblock.runtime import Runtime, IdReader +from xblock.runtime import Runtime, IdReader, IdGenerator from xmodule.fields import RelativeTime from xmodule.errortracker import exc_info_to_str from xmodule.modulestore.exceptions import ItemNotFoundError from opaque_keys.edx.keys import UsageKey +from opaque_keys.edx.asides import AsideUsageKeyV1, AsideDefinitionKeyV1 from xmodule.exceptions import UndefinedContext import dogstats_wrapper as dog_stats_api @@ -65,7 +66,7 @@ class OpaqueKeyReader(IdReader): Returns: The `definition_id` the usage is derived from """ - return usage_id.definition_key + raise NotImplementedError("Specific Modulestores must implement get_definition_id") def get_block_type(self, def_id): """Retrieve the block_type of a particular definition @@ -78,6 +79,91 @@ class OpaqueKeyReader(IdReader): """ return def_id.block_type + def get_usage_id_from_aside(self, aside_id): + """ + Retrieve the XBlock `usage_id` associated with this aside usage id. + + Args: + aside_id: The usage id of the XBlockAside. + + Returns: + The `usage_id` of the usage the aside is commenting on. + """ + return aside_id.usage_key + + def get_definition_id_from_aside(self, aside_id): + """ + Retrieve the XBlock `definition_id` associated with this aside definition id. + + Args: + aside_id: The usage id of the XBlockAside. + + Returns: + The `definition_id` of the usage the aside is commenting on. + """ + return aside_id.definition_key + + def get_aside_type_from_usage(self, aside_id): + """ + Retrieve the XBlockAside `aside_type` associated with this aside + usage id. + + Args: + aside_id: The usage id of the XBlockAside. + + Returns: + The `aside_type` of the aside. + """ + return aside_id.aside_type + + def get_aside_type_from_definition(self, aside_id): + """ + Retrieve the XBlockAside `aside_type` associated with this aside + definition id. + + Args: + aside_id: The definition id of the XBlockAside. + + Returns: + The `aside_type` of the aside. + """ + return aside_id.aside_type + + +class AsideKeyGenerator(IdGenerator): # pylint: disable=abstract-method + """ + An :class:`.IdGenerator` that only provides facilities for constructing new XBlockAsides. + """ + def create_aside(self, definition_id, usage_id, aside_type): + """ + Make a new aside definition and usage ids, indicating an :class:`.XBlockAside` of type `aside_type` + commenting on an :class:`.XBlock` usage `usage_id` + + Returns: + (aside_definition_id, aside_usage_id) + """ + def_key = AsideDefinitionKeyV1(definition_id, aside_type) + usage_key = AsideUsageKeyV1(usage_id, aside_type) + return (def_key, usage_key) + + def create_usage(self, def_id): + """Make a usage, storing its definition id. + + Returns the newly-created usage id. + """ + raise NotImplementedError("Specific Modulestores must provide implementations of create_usage") + + def create_definition(self, block_type, slug=None): + """Make a definition, storing its block type. + + If `slug` is provided, it is a suggestion that the definition id + incorporate the slug somehow. + + Returns the newly-created definition id. + + """ + raise NotImplementedError("Specific Modulestores must provide implementations of create_definition") + def dummy_track(_event_type, _event): pass @@ -160,6 +246,8 @@ class XModuleMixin(XBlockMixin): Adding this Mixin to an :class:`XBlock` allows it to cooperate with old-style :class:`XModules` """ + entry_point = "xmodule.v1" + # Attributes for inspection of the descriptor # This indicates whether the xmodule is a problem-type. @@ -526,6 +614,7 @@ class XModule(XModuleMixin, HTMLSnippet, XBlock): # pylint: disable=abstract-me field_data: A dictionary-like object that maps field names to values for those fields. """ + # Set the descriptor first so that we can proxy to it self.descriptor = descriptor super(XModule, self).__init__(*args, **kwargs) @@ -715,7 +804,6 @@ class XModuleDescriptor(XModuleMixin, HTMLSnippet, ResourceTemplates, XBlock): create a problem, and can generate XModules (which do know about student state). """ - entry_point = "xmodule.v1" module_class = XModule # VS[compat]. Backwards compatibility code that can go away after @@ -997,7 +1085,7 @@ class XModuleDescriptor(XModuleMixin, HTMLSnippet, ResourceTemplates, XBlock): class ConfigurableFragmentWrapper(object): # pylint: disable=abstract-method """ - Runtime mixin that allows for composition of many `wrap_child` wrappers + Runtime mixin that allows for composition of many `wrap_xblock` wrappers """ def __init__(self, wrappers=None, **kwargs): """ @@ -1013,7 +1101,7 @@ class ConfigurableFragmentWrapper(object): # pylint: disable=abstract-method else: self.wrappers = [] - def wrap_child(self, block, view, frag, context): + def wrap_xblock(self, block, view, frag, context): """ See :func:`Runtime.wrap_child` """ @@ -1043,6 +1131,16 @@ def descriptor_global_local_resource_url(block, uri): # pylint: disable=invalid raise NotImplementedError("Applications must monkey-patch this function before using local_resource_url for studio_view") +# This function exists to give applications (LMS/CMS) a place to monkey-patch until +# we can refactor modulestore to split out the FieldData half of its interface from +# the Runtime part of its interface. This function matches the Runtime.get_asides interface +def descriptor_global_get_asides(block): # pylint: disable=unused-argument + """ + See :meth:`xblock.runtime.Runtime.get_asides`. + """ + raise NotImplementedError("Applications must monkey-patch this function before using get_asides from a DescriptorSystem.") + + class MetricsMixin(object): """ Mixin for adding metric logging for render and handle methods in the DescriptorSystem and ModuleSystem. @@ -1137,7 +1235,9 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): # p local_resource_url: an implementation of :meth:`xblock.runtime.Runtime.local_resource_url` """ - super(DescriptorSystem, self).__init__(id_reader=OpaqueKeyReader(), **kwargs) + kwargs.setdefault('id_reader', OpaqueKeyReader()) + kwargs.setdefault('id_generator', AsideKeyGenerator()) + super(DescriptorSystem, self).__init__(**kwargs) # This is used by XModules to write out separate files during xml export self.export_fs = None @@ -1215,6 +1315,19 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): # p # global function that the application can override. return descriptor_global_local_resource_url(block, uri) + def get_asides(self, block): + """ + See :meth:`xblock.runtime.Runtime:get_asides` for documentation. + """ + if getattr(block, 'xmodule_runtime', None) is not None: + return block.xmodule_runtime.get_asides(block) + else: + # Currently, Modulestore is responsible for instantiating DescriptorSystems + # This means that LMS/CMS don't have a way to define a subclass of DescriptorSystem + # that implements the correct get_asides. So, for now, instead, we will reference a + # global function that the application can override. + return descriptor_global_get_asides(block) + def resource_url(self, resource): """ See :meth:`xblock.runtime.Runtime:resource_url` for documentation. @@ -1335,7 +1448,9 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): # pylin # Usage_store is unused, and field_data is often supplanted with an # explicit field_data during construct_xblock. - super(ModuleSystem, self).__init__(id_reader=OpaqueKeyReader(), field_data=field_data, **kwargs) + kwargs.setdefault('id_reader', getattr(descriptor_runtime, 'id_reader', OpaqueKeyReader())) + kwargs.setdefault('id_generator', getattr(descriptor_runtime, 'id_generator', AsideKeyGenerator())) + super(ModuleSystem, self).__init__(field_data=field_data, **kwargs) self.STATIC_URL = static_url self.xqueue = xqueue @@ -1373,6 +1488,9 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): # pylin self.descriptor_runtime = descriptor_runtime self.rebind_noauth_module_to_user = rebind_noauth_module_to_user + if user: + self.user_id = user.id + def get(self, attr): """ provide uniform access to attributes (like etree).""" return self.__dict__.get(attr) diff --git a/common/static/coffee/spec/xblock/core_spec.coffee b/common/static/coffee/spec/xblock/core_spec.coffee index 7df6fbf263..7e7c1e081a 100644 --- a/common/static/coffee/spec/xblock/core_spec.coffee +++ b/common/static/coffee/spec/xblock/core_spec.coffee @@ -35,7 +35,7 @@ describe "XBlock", -> window.initFnZ = jasmine.createSpy() @fakeChildren = ['list', 'of', 'children'] - spyOn(XBlock, 'initializeBlocks').andReturn(@fakeChildren) + spyOn(XBlock, 'initializeXBlocks').andReturn(@fakeChildren) @vANode = $('#vA')[0] @vZNode = $('#vZ')[0] @@ -50,8 +50,8 @@ describe "XBlock", -> expect(TestRuntime.vZ).toHaveBeenCalledWith() it "loads the right init function", -> - expect(window.initFnA).toHaveBeenCalledWith(@runtimeA, @vANode) - expect(window.initFnZ).toHaveBeenCalledWith(@runtimeZ, @vZNode) + expect(window.initFnA).toHaveBeenCalledWith(@runtimeA, @vANode, {}) + expect(window.initFnZ).toHaveBeenCalledWith(@runtimeZ, @vZNode, {}) it "loads when missing versions", -> expect(@missingVersionBlock.element).toBe($('#missing-version')) @@ -74,8 +74,8 @@ describe "XBlock", -> expect(@missingInitBlock.element).toBe($('#missing-init')[0]) it "passes through the request token", -> - expect(XBlock.initializeBlocks).toHaveBeenCalledWith($(@vANode), 'req-token-a') - expect(XBlock.initializeBlocks).toHaveBeenCalledWith($(@vZNode), 'req-token-z') + expect(XBlock.initializeXBlocks).toHaveBeenCalledWith($(@vANode), 'req-token-a') + expect(XBlock.initializeXBlocks).toHaveBeenCalledWith($(@vZNode), 'req-token-z') describe "initializeBlocks", -> diff --git a/common/static/coffee/src/xblock/core.coffee b/common/static/coffee/src/xblock/core.coffee deleted file mode 100644 index bafcff97c9..0000000000 --- a/common/static/coffee/src/xblock/core.coffee +++ /dev/null @@ -1,57 +0,0 @@ -@XBlock = - Runtime: {} - - ### - Initialize the javascript for a single xblock element, and for all of it's - xblock children that match requestToken. If requestToken is omitted, use the - data-request-token attribute from element, or use the request-tokens specified on - the children themselves. - ### - initializeBlock: (element, requestToken) -> - $element = $(element) - requestToken = requestToken or $element.data('request-token') - children = @initializeBlocks($element, requestToken) - runtime = $element.data("runtime-class") - version = $element.data("runtime-version") - initFnName = $element.data("init") - $element.prop('xblock_children', children) - if runtime? and version? and initFnName? - runtime = new window[runtime]["v#{version}"] - initFn = window[initFnName] - if initFn.length > 2 - initargs = $(".xblock_json_init_args", element) - if initargs.length == 0 - console.log("Warning: XBlock expects data parameters") - data = JSON.parse(initargs.text()) - block = initFn(runtime, element, data) ? {} - else - block = initFn(runtime, element) ? {} - block.runtime = runtime - else - elementTag = $('
').append($element.clone()).html(); - console.log("Block #{elementTag} is missing data-runtime, data-runtime-version or data-init, and can't be initialized") - block = {} - - block.element = element - block.name = $element.data("name") - block.type = $element.data("block-type") - - $element.trigger("xblock-initialized") - $element.data("initialized", true) - $element.addClass("xblock-initialized") - block - - ### - Initialize all XBlocks inside element that were rendered with requestToken. - If requestToken is omitted, and element has a 'data-request-token' attribute, use that. - If neither is available, then use the request tokens of the immediateDescendent xblocks. - ### - initializeBlocks: (element, requestToken) -> - requestToken = requestToken or $(element).data('request-token') - if requestToken - selector = ".xblock[data-request-token='#{requestToken}']" - else - selector = ".xblock" - $(element).immediateDescendents(selector).map((idx, elem) => - @initializeBlock(elem, requestToken) - ).toArray() diff --git a/common/static/js/xblock/core.js b/common/static/js/xblock/core.js new file mode 100644 index 0000000000..ffef2b2762 --- /dev/null +++ b/common/static/js/xblock/core.js @@ -0,0 +1,142 @@ +(function($, JSON) { + + 'use strict'; + + function initializeBlockLikes(block_class, initializer, element, requestToken) { + var requestToken = requestToken || $(element).data('request-token'); + if (requestToken) { + var selector = '.' + block_class + '[data-request-token="' + requestToken + '"]'; + } else { + var selector = '.' + block_class; + } + return $(element).immediateDescendents(selector).map(function(idx, elem) { + return initializer(elem, requestToken); + }).toArray(); + } + + function elementRuntime(element) { + var $element = $(element); + var runtime = $element.data('runtime-class'); + var version = $element.data('runtime-version'); + var initFnName = $element.data('init'); + + if (runtime && version && initFnName) { + return new window[runtime]['v' + version]; + } else { + if (!runtime || !version || !initFnName) { + var elementTag = $('
').append($element.clone()).html(); + console.log('Block ' + elementTag + ' is missing data-runtime, data-runtime-version or data-init, and can\'t be initialized'); + } + return null; + } + } + + function initArgs(element) { + var initargs = $('.xblock_json_init_args', element).text(); + return initargs ? JSON.parse(initargs) : {}; + } + + /** + * Construct an XBlock family object from an element. The constructor + * function is loaded from the 'data-init' attribute of the element. + * The constructor is called with the arguments 'runtime', 'element', + * and then all of 'block_args'. + */ + function constructBlock(element, block_args) { + var block; + var $element = $(element); + var runtime = elementRuntime(element); + + block_args.unshift(element); + block_args.unshift(runtime); + + + if (runtime) { + + block = (function() { + var initFn = window[$element.data('init')]; + + // This create a new constructor that can then apply() the block_args + // to the initFn. + function Block() { + return initFn.apply(this, block_args); + } + Block.prototype = initFn.prototype; + + return new Block(); + })(); + block.runtime = runtime; + } else { + block = {}; + } + block.element = element; + block.name = $element.data('name'); + block.type = $element.data('block-type'); + $element.trigger('xblock-initialized'); + $element.data('initialized', true); + $element.addClass('xblock-initialized'); + return block; + } + + var XBlock = { + Runtime: {}, + + /** + * Initialize the javascript for a single xblock element, and for all of it's + * xblock children that match requestToken. If requestToken is omitted, use the + * data-request-token attribute from element, or use the request-tokens specified on + * the children themselves. + */ + initializeBlock: function(element, requestToken) { + var $element = $(element); + + var requestToken = requestToken || $element.data('request-token'); + var children = XBlock.initializeXBlocks($element, requestToken); + $element.prop('xblock_children', children); + + return constructBlock(element, [initArgs(element)]); + }, + + /** + * Initialize the javascript for a single xblock aside element that matches requestToken. + * If requestToken is omitted, use the data-request-token attribute from element, or use + * the request-tokens specified on the children themselves. + */ + initializeAside: function(element, requestToken) { + var blockUsageId = $(element).data('block-id'); + var blockElement = $(element).siblings('[data-usage-id="' + blockUsageId + '"]')[0]; + return constructBlock(element, [blockElement, initArgs(element)]); + }, + + /** + * Initialize all XBlocks inside element that were rendered with requestToken. + * If requestToken is omitted, and element has a 'data-request-token' attribute, use that. + * If neither is available, then use the request tokens of the immediateDescendent xblocks. + */ + initializeXBlocks: function(element, requestToken) { + return initializeBlockLikes('xblock', XBlock.initializeBlock, element, requestToken); + }, + + /** + * Initialize all XBlockAsides inside element that were rendered with requestToken. + * If requestToken is omitted, and element has a 'data-request-token' attribute, use that. + * If neither is available, then use the request tokens of the immediateDescendent xblocks. + */ + initializeXBlockAsides: function(element, requestToken) { + return initializeBlockLikes('xblock_asides-v1', XBlock.initializeAside, element, requestToken); + }, + + /** + * Initialize all XBlock-family blocks inside element that were rendered with requestToken. + * If requestToken is omitted, and element has a 'data-request-token' attribute, use that. + * If neither is available, then use the request tokens of the immediateDescendent xblocks. + */ + initializeBlocks: function(element, requestToken) { + XBlock.initializeXBlockAsides(element, requestToken); + return XBlock.initializeXBlocks(element, requestToken); + } + }; + + this.XBlock = XBlock; + +}).call(this, $, JSON); diff --git a/common/static/js_test.yml b/common/static/js_test.yml index b83c3f6bd1..fc4f9dfac3 100644 --- a/common/static/js_test.yml +++ b/common/static/js_test.yml @@ -45,6 +45,7 @@ lib_paths: # Paths to source JavaScript files src_paths: + - js/xblock - coffee/src - js/src - js/utils diff --git a/common/test/acceptance/pages/studio/container.py b/common/test/acceptance/pages/studio/container.py index 81f7c7dab7..e65d55146f 100644 --- a/common/test/acceptance/pages/studio/container.py +++ b/common/test/acceptance/pages/studio/container.py @@ -51,7 +51,7 @@ class ContainerPage(PageObject): num_wrappers = len(self.q(css='{} [data-request-token="{}"]'.format(XBlockWrapper.BODY_SELECTOR, request_token)).results) # Wait until all components have been loaded and marked as either initialized or failed. # See: - # - common/static/coffee/src/xblock/core.coffee which adds the class "xblock-initialized" + # - common/static/js/xblock/core.js which adds the class "xblock-initialized" # at the end of initializeBlock. # - common/static/js/views/xblock.js which adds the class "xblock-initialization-failed" # if the xblock threw an error while initializing. diff --git a/common/test/acceptance/tests/lms/test_lms_acid_xblock.py b/common/test/acceptance/tests/lms/test_lms_acid_xblock.py index 2aaacdb486..14fd06b496 100644 --- a/common/test/acceptance/tests/lms/test_lms_acid_xblock.py +++ b/common/test/acceptance/tests/lms/test_lms_acid_xblock.py @@ -3,7 +3,7 @@ End-to-end tests for the LMS. """ -from unittest import skip +from unittest import expectedFailure from ..helpers import UniqueCourseTest from ...pages.lms.auto_auth import AutoAuthPage @@ -44,17 +44,6 @@ class XBlockAcidBase(UniqueCourseTest): self.assertTrue(acid_block.scope_passed('preferences')) self.assertTrue(acid_block.scope_passed('user_info')) - def test_acid_block(self): - """ - Verify that all expected acid block tests pass in the lms. - """ - - self.course_info_page.visit() - self.tab_nav.go_to_tab('Courseware') - - acid_block = AcidView(self.browser, '.xblock-student_view[data-block-type=acid]') - self.validate_acid_block_view(acid_block) - class XBlockAcidNoChildTest(XBlockAcidBase): """ @@ -81,7 +70,15 @@ class XBlockAcidNoChildTest(XBlockAcidBase): ).install() def test_acid_block(self): - super(XBlockAcidNoChildTest, self).test_acid_block() + """ + Verify that all expected acid block tests pass in the lms. + """ + + self.course_info_page.visit() + self.tab_nav.go_to_tab('Courseware') + + acid_block = AcidView(self.browser, '.xblock-student_view[data-block-type=acid]') + self.validate_acid_block_view(acid_block) class XBlockAcidChildTest(XBlockAcidBase): @@ -129,3 +126,46 @@ class XBlockAcidChildTest(XBlockAcidBase): acid_block = AcidView(self.browser, '.xblock-student_view[data-block-type=acid]') self.validate_acid_block_view(acid_block) + + +class XBlockAcidAsideTest(XBlockAcidBase): + """ + Tests of an AcidBlock with children + """ + __test__ = True + + def setup_fixtures(self): + course_fix = CourseFixture( + self.course_info['org'], + self.course_info['number'], + self.course_info['run'], + self.course_info['display_name'] + ) + + course_fix.add_children( + XBlockFixtureDesc('chapter', 'Test Section').add_children( + XBlockFixtureDesc('sequential', 'Test Subsection').add_children( + XBlockFixtureDesc('vertical', 'Test Unit').add_children( + XBlockFixtureDesc('acid', 'Acid Block') + ) + ) + ) + ).install() + + @expectedFailure + def test_acid_block(self): + """ + Verify that all expected acid block tests pass in the lms. + """ + + self.course_info_page.visit() + self.tab_nav.go_to_tab('Courseware') + + acid_aside = AcidView(self.browser, '.xblock_asides-v1-student_view[data-block-type=acid_aside]') + self.validate_acid_aside_view(acid_aside) + + acid_block = AcidView(self.browser, '.xblock-student_view[data-block-type=acid]') + self.validate_acid_block_view(acid_block) + + def validate_acid_aside_view(self, acid_aside): + self.validate_acid_block_view(acid_aside) diff --git a/lms/djangoapps/courseware/model_data.py b/lms/djangoapps/courseware/model_data.py index 5173b427cb..eb0993ee4e 100644 --- a/lms/djangoapps/courseware/model_data.py +++ b/lms/djangoapps/courseware/model_data.py @@ -12,16 +12,17 @@ from .models import ( XModuleStudentInfoField ) import logging -from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location -from opaque_keys.edx.keys import CourseKey, UsageKey +from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.block_types import BlockTypeKeyV1 +from opaque_keys.edx.asides import AsideUsageKeyV1 from django.db import DatabaseError -from django.contrib.auth.models import User from xblock.runtime import KeyValueStore from xblock.exceptions import KeyValueMultiSaveError, InvalidScopeError from xblock.fields import Scope, UserScope from xmodule.modulestore.django import modulestore +from xblock.core import XBlockAside log = logging.getLogger(__name__) @@ -46,7 +47,7 @@ class FieldDataCache(object): A cache of django model objects needed to supply the data for a module and its decendants """ - def __init__(self, descriptors, course_id, user, select_for_update=False): + def __init__(self, descriptors, course_id, user, select_for_update=False, asides=None): ''' Find any courseware.models objects that are needed by any descriptor in descriptors. Attempts to minimize the number of queries to the database. @@ -58,11 +59,17 @@ class FieldDataCache(object): course_id: The id of the current course user: The user for which to cache data select_for_update: True if rows should be locked until end of transaction + asides: The list of aside types to load, or None to prefetch no asides. ''' self.cache = {} self.descriptors = descriptors self.select_for_update = select_for_update + if asides is None: + self.asides = [] + else: + self.asides = asides + assert isinstance(course_id, CourseKey) self.course_id = course_id self.user = user @@ -75,7 +82,7 @@ class FieldDataCache(object): @classmethod def cache_for_descriptor_descendents(cls, course_id, user, descriptor, depth=None, descriptor_filter=lambda descriptor: True, - select_for_update=False): + select_for_update=False, asides=None): """ course_id: the course in the context of which we want StudentModules. user: the django user for whom to load modules. @@ -113,7 +120,7 @@ class FieldDataCache(object): with modulestore().bulk_operations(descriptor.location.course_key): descriptors = get_child_descriptors(descriptor, depth, descriptor_filter) - return FieldDataCache(descriptors, course_id, user, select_for_update) + return FieldDataCache(descriptors, course_id, user, select_for_update, asides=asides) def _query(self, model_class, **kwargs): """ @@ -140,6 +147,35 @@ class FieldDataCache(object): ) return res + @property + def _all_usage_ids(self): + """ + Return a set of all usage_ids for the descriptors that this FieldDataCache is caching + against, and well as all asides for those descriptors. + """ + usage_ids = set() + for descriptor in self.descriptors: + usage_ids.add(descriptor.scope_ids.usage_id) + + for aside_type in self.asides: + usage_ids.add(AsideUsageKeyV1(descriptor.scope_ids.usage_id, aside_type)) + + return usage_ids + + @property + def _all_block_types(self): + """ + Return a set of all block_types that are cached by this FieldDataCache. + """ + block_types = set() + for descriptor in self.descriptors: + block_types.add(BlockTypeKeyV1(descriptor.entry_point, descriptor.scope_ids.block_type)) + + for aside_type in self.asides: + block_types.add(BlockTypeKeyV1(XBlockAside.entry_point, aside_type)) + + return block_types + def _retrieve_fields(self, scope, fields): """ Queries the database for all of the fields in the specified scope @@ -148,7 +184,7 @@ class FieldDataCache(object): return self._chunked_query( StudentModule, 'module_state_key__in', - (descriptor.scope_ids.usage_id for descriptor in self.descriptors), + self._all_usage_ids, course_id=self.course_id, student=self.user.pk, ) @@ -156,14 +192,14 @@ class FieldDataCache(object): return self._chunked_query( XModuleUserStateSummaryField, 'usage_id__in', - (descriptor.scope_ids.usage_id for descriptor in self.descriptors), + self._all_usage_ids, field_name__in=set(field.name for field in fields), ) elif scope == Scope.preferences: return self._chunked_query( XModuleStudentPrefsField, 'module_type__in', - set(descriptor.scope_ids.block_type for descriptor in self.descriptors), + self._all_block_types, student=self.user.pk, field_name__in=set(field.name for field in fields), ) @@ -195,7 +231,7 @@ class FieldDataCache(object): elif key.scope == Scope.user_state_summary: return (key.scope, key.block_scope_id, key.field_name) elif key.scope == Scope.preferences: - return (key.scope, key.block_scope_id, key.field_name) + return (key.scope, BlockTypeKeyV1(key.block_family, key.block_scope_id), key.field_name) elif key.scope == Scope.user_info: return (key.scope, key.field_name) @@ -239,31 +275,28 @@ class FieldDataCache(object): return field_object if key.scope == Scope.user_state: - # When we start allowing block_scope_ids to be either Locations or Locators, - # this assertion will fail. Fix the code here when that happens! - assert(isinstance(key.block_scope_id, UsageKey)) - field_object, _ = StudentModule.objects.get_or_create( + field_object, __ = StudentModule.objects.get_or_create( course_id=self.course_id, student_id=key.user_id, module_state_key=key.block_scope_id, defaults={ 'state': json.dumps({}), - 'module_type': key.block_scope_id.category, + 'module_type': key.block_scope_id.block_type, }, ) elif key.scope == Scope.user_state_summary: - field_object, _ = XModuleUserStateSummaryField.objects.get_or_create( + field_object, __ = XModuleUserStateSummaryField.objects.get_or_create( field_name=key.field_name, usage_id=key.block_scope_id ) elif key.scope == Scope.preferences: - field_object, _ = XModuleStudentPrefsField.objects.get_or_create( + field_object, __ = XModuleStudentPrefsField.objects.get_or_create( field_name=key.field_name, - module_type=key.block_scope_id, + module_type=BlockTypeKeyV1(key.block_family, key.block_scope_id), student_id=key.user_id, ) elif key.scope == Scope.user_info: - field_object, _ = XModuleStudentInfoField.objects.get_or_create( + field_object, __ = XModuleStudentInfoField.objects.get_or_create( field_name=key.field_name, student_id=key.user_id, ) diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index b246e6f8c4..56818d4e2e 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -18,7 +18,7 @@ from django.db import models from django.db.models.signals import post_save from django.dispatch import receiver -from xmodule_django.models import CourseKeyField, LocationKeyField +from xmodule_django.models import CourseKeyField, LocationKeyField, BlockTypeKeyField class StudentModule(models.Model): @@ -36,10 +36,7 @@ class StudentModule(models.Model): ## These three are the key for the object module_type = models.CharField(max_length=32, choices=MODULE_TYPES, default='problem', db_index=True) - # Key used to share state. By default, this is the module_id, - # but for abtests and the like, this can be set to a shared value - # for many instances of the module. - # Filename for homeworks, etc. + # Key used to share state. This is the XBlock usage_id module_state_key = LocationKeyField(max_length=255, db_index=True, db_column='module_id') student = models.ForeignKey(User, db_index=True) @@ -150,7 +147,7 @@ class XBlockFieldBase(models.Model): return u'{}<{!r}'.format( self.__class__.__name__, { - key:getattr(self, key) + key: getattr(self, key) for key in self._meta.get_all_field_names() if key not in ('created', 'modified') } @@ -174,11 +171,11 @@ class XModuleStudentPrefsField(XBlockFieldBase): Stores data set in the Scope.preferences scope by an xmodule field """ - class Meta: + class Meta: # pylint: disable=missing-docstring unique_together = (('student', 'module_type', 'field_name'),) # The type of the module for these preferences - module_type = models.CharField(max_length=64, db_index=True) + module_type = BlockTypeKeyField(max_length=64, db_index=True) student = models.ForeignKey(User, db_index=True) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 6294f46f00..07c4d15d38 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -23,6 +23,7 @@ from courseware.masquerade import setup_masquerade from courseware.model_data import FieldDataCache, DjangoKeyValueStore from lms.djangoapps.lms_xblock.field_data import LmsFieldData from lms.djangoapps.lms_xblock.runtime import LmsModuleSystem, unquote_slashes, quote_slashes +from lms.djangoapps.lms_xblock.models import XBlockAsidesConfig from edxmako.shortcuts import render_to_string from eventtracking import tracker from psychometrics.psychoanalyze import make_psychometrics_data_update_handler @@ -405,7 +406,8 @@ def get_module_system_for_user(user, field_data_cache, field_data_cache_real_user = FieldDataCache.cache_for_descriptor_descendents( course_id, real_user, - module.descriptor + module.descriptor, + asides=XBlockAsidesConfig.possible_asides(), ) (inner_system, inner_student_data) = get_module_system_for_user( @@ -496,6 +498,8 @@ def get_module_system_for_user(user, field_data_cache, else: anonymous_student_id = anonymous_id_for_user(user, None) + field_data = LmsFieldData(descriptor._field_data, student_data) # pylint: disable=protected-access + system = LmsModuleSystem( track_function=track_function, render_template=render_to_string, @@ -541,11 +545,13 @@ def get_module_system_for_user(user, field_data_cache, services={ 'i18n': ModuleI18nService(), 'fs': xblock.reference.plugins.FSService(), + 'field-data': field_data, }, get_user_role=lambda: get_user_role(user, course_id), descriptor_runtime=descriptor.runtime, rebind_noauth_module_to_user=rebind_noauth_module_to_user, user_location=user_location, + request_token=request_token, ) # pass position specified in URL to module through ModuleSystem @@ -572,7 +578,7 @@ def get_module_system_for_user(user, field_data_cache, else: system.error_descriptor_class = NonStaffErrorDescriptor - return system, student_data + return system, field_data def get_module_for_descriptor_internal(user, descriptor, field_data_cache, course_id, # pylint: disable=invalid-name @@ -594,7 +600,7 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours if not has_access(user, 'load', descriptor, course_id): return None - (system, student_data) = get_module_system_for_user( + (system, field_data) = get_module_system_for_user( user=user, field_data_cache=field_data_cache, # These have implicit user bindings, the rest of args are considered not to descriptor=descriptor, @@ -609,7 +615,7 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours request_token=request_token ) - descriptor.bind_for_student(system, LmsFieldData(descriptor._field_data, student_data)) # pylint: disable=protected-access + descriptor.bind_for_student(system, field_data) # pylint: disable=protected-access descriptor.scope_ids = descriptor.scope_ids._replace(user_id=user.id) # pylint: disable=protected-access return descriptor diff --git a/lms/djangoapps/courseware/tests/test_model_data.py b/lms/djangoapps/courseware/tests/test_model_data.py index f1a680da96..f7e74b9568 100644 --- a/lms/djangoapps/courseware/tests/test_model_data.py +++ b/lms/djangoapps/courseware/tests/test_model_data.py @@ -16,9 +16,10 @@ from courseware.tests.factories import UserStateSummaryFactory from courseware.tests.factories import StudentPrefsFactory, StudentInfoFactory from xblock.fields import Scope, BlockScope, ScopeIds +from xblock.exceptions import KeyValueMultiSaveError +from xblock.core import XBlock from django.test import TestCase from django.db import DatabaseError -from xblock.exceptions import KeyValueMultiSaveError def mock_field(scope, name): @@ -29,7 +30,7 @@ def mock_field(scope, name): def mock_descriptor(fields=[]): - descriptor = Mock() + descriptor = Mock(entry_point=XBlock.entry_point) descriptor.scope_ids = ScopeIds('user1', 'mock_problem', location('def_id'), location('usage_id')) descriptor.module_class.fields.values.return_value = fields descriptor.fields.values.return_value = fields diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index d452d7a3a7..802c3ef917 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -39,6 +39,8 @@ from .module_render import toc_for_course, get_module_for_descriptor, get_module from courseware.models import StudentModule, StudentModuleHistory from course_modes.models import CourseMode +from lms.djangoapps.lms_xblock.models import XBlockAsidesConfig + from open_ended_grading import open_ended_notifications from student.models import UserTestGroup, CourseEnrollment from student.views import single_course_reverification_info, is_course_blocked @@ -444,7 +446,8 @@ def _index_bulk_op(request, course_key, chapter, section, position): # Load all descendants of the section, because we're going to display its # html, which in general will need all of its children section_field_data_cache = FieldDataCache.cache_for_descriptor_descendents( - course_key, user, section_descriptor, depth=None) + course_key, user, section_descriptor, depth=None, asides=XBlockAsidesConfig.possible_asides() + ) # Verify that position a string is in fact an int if position is not None: diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index 7a2ed19fad..492558d274 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -18,6 +18,7 @@ from django.utils.html import escape from django.http import Http404 from django.conf import settings from util.json_request import JsonResponse +from mock import patch from lms.djangoapps.lms_xblock.runtime import quote_slashes from xmodule_modifiers import wrap_xblock @@ -323,17 +324,28 @@ def _section_data_download(course, access): return section_data +def null_get_asides(block): # pylint: disable=unused-argument + """ + get_aside method for monkey-patching into descriptor_global_get_asides + while rendering an HtmlDescriptor for email text editing. This returns + an empty list. + """ + return [] + + def _section_send_email(course, access): """ Provide data for the corresponding bulk email section """ course_key = course.id - # This HtmlDescriptor is only being used to generate a nice text editor. - html_module = HtmlDescriptor( - course.system, - DictFieldData({'data': ''}), - ScopeIds(None, None, None, course_key.make_usage_key('html', 'fake')) - ) - fragment = course.system.render(html_module, 'studio_view') + # Monkey-patch descriptor_global_get_asides to return no asides for the duration of this render + with patch('xmodule.x_module.descriptor_global_get_asides', null_get_asides): + # This HtmlDescriptor is only being used to generate a nice text editor. + html_module = HtmlDescriptor( + course.system, + DictFieldData({'data': ''}), + ScopeIds(None, None, None, course_key.make_usage_key('html', 'fake')) + ) + fragment = course.system.render(html_module, 'studio_view') fragment = wrap_xblock( 'LmsRuntime', html_module, 'studio_view', fragment, None, extra_data={"course-id": unicode(course_key)}, diff --git a/lms/djangoapps/lms_xblock/admin.py b/lms/djangoapps/lms_xblock/admin.py index 451903245c..81ba9d67fc 100644 --- a/lms/djangoapps/lms_xblock/admin.py +++ b/lms/djangoapps/lms_xblock/admin.py @@ -1,3 +1,7 @@ +""" +Django admin dashboard configuration for LMS XBlock infrastructure. +""" + from django.contrib import admin from config_models.admin import ConfigurationModelAdmin from lms.djangoapps.lms_xblock.models import XBlockAsidesConfig diff --git a/lms/djangoapps/lms_xblock/field_data.py b/lms/djangoapps/lms_xblock/field_data.py index e26897b884..c8d4e0ea90 100644 --- a/lms/djangoapps/lms_xblock/field_data.py +++ b/lms/djangoapps/lms_xblock/field_data.py @@ -16,7 +16,7 @@ class LmsFieldData(SplitFieldData): def __init__(self, authored_data, student_data): # Make sure that we don't repeatedly nest LmsFieldData instances if isinstance(authored_data, LmsFieldData): - authored_data = authored_data._authored_data # pylint: disable=protected-member + authored_data = authored_data._authored_data # pylint: disable=protected-access else: authored_data = ReadOnlyFieldData(authored_data) diff --git a/lms/djangoapps/lms_xblock/models.py b/lms/djangoapps/lms_xblock/models.py index 8d04dbae95..bffe8ebf27 100644 --- a/lms/djangoapps/lms_xblock/models.py +++ b/lms/djangoapps/lms_xblock/models.py @@ -1,7 +1,18 @@ +""" +Models used by LMS XBlock infrastructure. + +Includes: + XBlockAsidesConfig: A ConfigurationModel for managing how XBlockAsides are + rendered in the LMS. +""" + from django.db.models import TextField from config_models.models import ConfigurationModel +from xblock.core import XBlockAside + + class XBlockAsidesConfig(ConfigurationModel): """ Configuration for XBlockAsides. @@ -11,3 +22,10 @@ class XBlockAsidesConfig(ConfigurationModel): default="about course_info static_tab", help_text="Space-separated list of XBlocks on which XBlockAsides should never render." ) + + @classmethod + def possible_asides(cls): + """ + Return a list of all asides that are enabled across all XBlocks. + """ + return [aside_type for aside_type, __ in XBlockAside.load_classes()] diff --git a/lms/djangoapps/lms_xblock/runtime.py b/lms/djangoapps/lms_xblock/runtime.py index f96a157acf..1cad66d8d5 100644 --- a/lms/djangoapps/lms_xblock/runtime.py +++ b/lms/djangoapps/lms_xblock/runtime.py @@ -7,7 +7,9 @@ import xblock.reference.plugins from django.core.urlresolvers import reverse from django.conf import settings +from lms.djangoapps.lms_xblock.models import XBlockAsidesConfig from openedx.core.djangoapps.user_api.api import course_tag as user_course_tag_api +from xblock.core import XBlockAside from xmodule.modulestore.django import modulestore from xmodule.x_module import ModuleSystem from xmodule.partitions.partitions_service import PartitionService @@ -87,8 +89,8 @@ class LmsHandlerUrls(object): view_name = 'xblock_handler_noauth' url = reverse(view_name, kwargs={ - 'course_id': self.course_id.to_deprecated_string(), - 'usage_id': quote_slashes(block.scope_ids.usage_id.to_deprecated_string().encode('utf-8')), + 'course_id': unicode(self.course_id), + 'usage_id': quote_slashes(unicode(block.scope_ids.usage_id).encode('utf-8')), 'handler': handler_name, 'suffix': suffix, }) @@ -198,4 +200,50 @@ class LmsModuleSystem(LmsHandlerUrls, ModuleSystem): # pylint: disable=abstract track_function=kwargs.get('track_function', None), ) services['fs'] = xblock.reference.plugins.FSService() + self.request_token = kwargs.pop('request_token', None) super(LmsModuleSystem, self).__init__(**kwargs) + + def wrap_aside(self, block, aside, view, frag, context): + """ + Creates a div which identifies the aside, points to the original block, + and writes out the json_init_args into a script tag. + + The default implementation creates a frag to wraps frag w/ a div identifying the xblock. If you have + javascript, you'll need to override this impl + """ + extra_data = { + 'block-id': quote_slashes(unicode(block.scope_ids.usage_id)), + 'url-selector': 'asideBaseUrl', + 'runtime-class': 'LmsRuntime', + } + if self.request_token: + extra_data['request-token'] = self.request_token + + return self._wrap_ele( + aside, + view, + frag, + extra_data, + ) + + def get_asides(self, block): + """ + Return all of the asides which might be decorating this `block`. + + Arguments: + block (:class:`.XBlock`): The block to render retrieve asides for. + """ + + config = XBlockAsidesConfig.current() + + if not config.enabled: + return [] + + if block.scope_ids.block_type in config.disabled_blocks.split(): + return [] + + return [ + self.get_aside_of_type(block, aside_type) + for aside_type, __ + in XBlockAside.load_classes() + ] diff --git a/lms/djangoapps/lms_xblock/test/test_runtime.py b/lms/djangoapps/lms_xblock/test/test_runtime.py index 3345fbcc57..e4218e0c58 100644 --- a/lms/djangoapps/lms_xblock/test/test_runtime.py +++ b/lms/djangoapps/lms_xblock/test/test_runtime.py @@ -10,6 +10,7 @@ from unittest import TestCase from urlparse import urlparse from opaque_keys.edx.locations import SlashSeparatedCourseKey from lms.djangoapps.lms_xblock.runtime import quote_slashes, unquote_slashes, LmsModuleSystem +from xblock.fields import ScopeIds TEST_STRINGS = [ '', @@ -42,8 +43,7 @@ class TestHandlerUrl(TestCase): """Test the LMS handler_url""" def setUp(self): - self.block = Mock() - self.block.scope_ids.usage_id.to_deprecated_string.return_value.encode.return_value = 'dummy' + self.block = Mock(name='block', scope_ids=ScopeIds(None, None, None, 'dummy')) self.course_key = SlashSeparatedCourseKey("org", "course", "run") self.runtime = LmsModuleSystem( static_url='/static', diff --git a/lms/djangoapps/open_ended_grading/tests.py b/lms/djangoapps/open_ended_grading/tests.py index 85ae8e4258..5f97ec2afa 100644 --- a/lms/djangoapps/open_ended_grading/tests.py +++ b/lms/djangoapps/open_ended_grading/tests.py @@ -21,7 +21,7 @@ from xblock.fields import ScopeIds from courseware.tests import factories from courseware.tests.helpers import LoginEnrollmentTestCase -from lms.lib.xblock.runtime import LmsModuleSystem +from lms.djangoapps.lms_xblock.runtime import LmsModuleSystem from student.roles import CourseStaffRole from student.models import unique_id_for_user from xmodule import peer_grading_module diff --git a/lms/envs/common.py b/lms/envs/common.py index a8b1c25012..9ed7ecd771 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1165,7 +1165,7 @@ PIPELINE_JS = { 'application': { # Application will contain all paths not in courseware_only_js - 'source_filenames': sorted(common_js) + sorted(project_js) + [ + 'source_filenames': ['js/xblock/core.js'] + sorted(common_js) + sorted(project_js) + [ 'js/form.ext.js', 'js/my_courses_dropdown.js', 'js/toggle_login_modal.js', @@ -1514,6 +1514,8 @@ INSTALLED_APPS = ( # Surveys 'survey', + + 'lms.djangoapps.lms_xblock', ) ######################### MARKETING SITE ############################### diff --git a/lms/static/js/spec/main.js b/lms/static/js/spec/main.js index 3242093e3d..10532bc80b 100644 --- a/lms/static/js/spec/main.js +++ b/lms/static/js/spec/main.js @@ -49,7 +49,7 @@ 'tender': '//edxedge.tenderapp.com/tender_widget', 'coffee/src/ajax_prefix': 'xmodule_js/common_static/coffee/src/ajax_prefix', 'xmodule_js/common_static/js/test/add_ajax_prefix': 'xmodule_js/common_static/js/test/add_ajax_prefix', - 'xblock/core': 'xmodule_js/common_static/coffee/src/xblock/core', + 'xblock/core': 'xmodule_js/common_static/js/xblock/core', 'xblock/runtime.v1': 'xmodule_js/common_static/coffee/src/xblock/runtime.v1', 'xblock/lms.runtime.v1': 'coffee/src/xblock/lms.runtime.v1', 'capa/display': 'xmodule_js/src/capa/display', diff --git a/lms/static/js_test.yml b/lms/static/js_test.yml index 410c92032c..e5fbda003a 100644 --- a/lms/static/js_test.yml +++ b/lms/static/js_test.yml @@ -45,6 +45,7 @@ lib_paths: - xmodule_js/common_static/js/vendor/jQuery-File-Upload/js/jquery.iframe-transport.js - xmodule_js/common_static/js/vendor/url.min.js - xmodule_js/common_static/coffee/src/jquery.immediateDescendents.js + - xmodule_js/common_static/js/xblock - xmodule_js/common_static/coffee/src/xblock - xmodule_js/common_static/js/vendor/sinon-1.7.1.js - xmodule_js/src/capa/ diff --git a/lms/static/js_test_coffee.yml b/lms/static/js_test_coffee.yml index 9c5b118d7a..4e257f9e66 100644 --- a/lms/static/js_test_coffee.yml +++ b/lms/static/js_test_coffee.yml @@ -42,6 +42,7 @@ lib_paths: - xmodule_js/common_static/js/vendor/CodeMirror/codemirror.js - xmodule_js/common_static/js/vendor/URI.min.js - xmodule_js/common_static/coffee/src/jquery.immediateDescendents.js + - xmodule_js/common_static/js/xblock - xmodule_js/common_static/coffee/src/xblock - xmodule_js/src/capa/ - xmodule_js/src/video/ diff --git a/pylintrc b/pylintrc index aa77522d5b..f92ec1806a 100644 --- a/pylintrc +++ b/pylintrc @@ -137,6 +137,8 @@ generated-members= category, name, revision, +# For django models + _meta, [BASIC] diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 50e1477e54..830db725d7 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -22,16 +22,16 @@ git+https://github.com/mitocw/django-cas.git@60a5b8e5a62e63e0d5d224a87f0b489201a0c695#egg=django-cas # Our libraries: --e git+https://github.com/edx/XBlock.git@2029af2a4b524310847decfb34ef39da8a30dc4e#egg=XBlock +-e git+https://github.com/edx/XBlock.git@9c634481dfc85a17dcb3351ca232d7098a38e10e#egg=XBlock -e git+https://github.com/edx/codejail.git@75307b25032d8b0040b1408c01fd6cc9a1989bd5#egg=codejail -e git+https://github.com/edx/diff-cover.git@v0.7.2#egg=diff_cover -e git+https://github.com/edx/js-test-tool.git@v0.1.6#egg=js_test_tool -e git+https://github.com/edx/event-tracking.git@0.1.0#egg=event-tracking -e git+https://github.com/edx/bok-choy.git@4a259e3548a19e41cc39433caf68ea58d10a27ba#egg=bok_choy -e git+https://github.com/edx-solutions/django-splash.git@7579d052afcf474ece1239153cffe1c89935bc4f#egg=django-splash --e git+https://github.com/edx/acid-block.git@df1a7f0cae46567c251d507b8c72168aed8ec042#egg=acid-xblock +-e git+https://github.com/edx/acid-block.git@e46f9cda8a03e121a00c7e347084d142d22ebfb7#egg=acid-xblock -e git+https://github.com/edx/edx-ora2.git@release-2014-10-27T19.33#egg=edx-ora2 --e git+https://github.com/edx/opaque-keys.git@b12401384921c075e5a4ed7aedc3bea57f56ec32#egg=opaque-keys +-e git+https://github.com/edx/opaque-keys.git@1254ed4d615a428591850656f39f26509b86d30a#egg=opaque-keys -e git+https://github.com/edx/ease.git@97de68448e5495385ba043d3091f570a699d5b5f#egg=ease -e git+https://github.com/edx/i18n-tools.git@56f048af9b6868613c14aeae760548834c495011#egg=i18n-tools -e git+https://github.com/edx/edx-oauth2-provider.git@0.4.0#egg=oauth2-provider From 822eaa20c66764058d9e600ac435ba33b750bc64 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 8 Dec 2014 14:14:35 -0500 Subject: [PATCH 6/6] Make ErrorDescriptor more resistent to errors --- common/lib/xmodule/xmodule/error_module.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py index 1391dc4ac2..066a1ad38b 100644 --- a/common/lib/xmodule/xmodule/error_module.py +++ b/common/lib/xmodule/xmodule/error_module.py @@ -108,7 +108,7 @@ class ErrorDescriptor(ErrorFields, XModuleDescriptor): cls, # The error module doesn't use scoped data, and thus doesn't need # real scope keys - ScopeIds('error', None, location, location), + ScopeIds(None, 'error', location, location), field_data, ) @@ -120,9 +120,14 @@ class ErrorDescriptor(ErrorFields, XModuleDescriptor): @classmethod def from_json(cls, json_data, system, location, error_msg='Error not available'): + try: + json_string = json.dumps(json_data, skipkeys=False, indent=4, cls=EdxJSONEncoder) + except: # pylint: disable=bare-except + json_string = repr(json_data) + return cls._construct( system, - json.dumps(json_data, skipkeys=False, indent=4, cls=EdxJSONEncoder), + json_string, error_msg, location=location )