Skip to content

Commit

Permalink
fix: needs(field-data); XBlock-like; etc
Browse files Browse the repository at this point in the history
  • Loading branch information
kdmccormick committed Feb 12, 2024
1 parent 1e9b8f3 commit e13b78f
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 24 deletions.
34 changes: 18 additions & 16 deletions xblock/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
])

# __all__ controls what classes end up in the docs.
__all__ = ['Blocklike', 'XBlock', 'XBlockAside']
__all__ = ['XBlock', 'XBlockAside']

UNSET = object()

Expand Down Expand Up @@ -98,24 +98,24 @@ class Blocklike(metaclass=_AutoNamedFieldsMetaclass):
@classmethod
def get_resources_dir(cls):
"""
Gets the resource directory for this Blocklike.
Gets the resource directory for this XBlock-like class.
"""
return cls.resources_dir

@classmethod
def get_public_dir(cls):
"""
Gets the public directory for this Blocklike.
Gets the public directory for this XBlock-like class.
"""
return cls.public_dir

@classmethod
def get_i18n_js_namespace(cls):
"""
Gets the JavaScript translations namespace for this Blocklike.
Gets the JavaScript translations namespace for this XBlock-like class.
Returns:
str: The JavaScript namespace for this Blocklike.
str: The JavaScript namespace for this XBlock-like class.
None: If this doesn't have JavaScript translations configured.
"""
return cls.i18n_js_namespace
Expand All @@ -128,13 +128,13 @@ def open_local_resource(cls, uri):
The container calls this method when it receives a request for a
resource on a URL which was generated by Runtime.local_resource_url().
It will pass the URI from the original call to local_resource_url()
back to this method. The Blocklike must parse this URI and return an open
back to this method. The XBlock-like must parse this URI and return an open
file-like object for the resource.
For security reasons, the default implementation will return only a
very restricted set of file types, which must be located in a folder
that defaults to "public". The location used for public resources can
be changed on a per-Blocklike basis. Blocklike authors who want to override
be changed on a per-XBlock-like basis. XBlock-like authors who want to override
this behavior will need to take care to ensure that the method only
serves legitimate public resources. At the least, the URI should be
matched against a whitelist regex to ensure that you do not serve an
Expand Down Expand Up @@ -210,7 +210,7 @@ def handler(cls, func):
@classmethod
def needs(cls, *service_names):
"""
A class decorator to indicate that a Blocklike class needs particular services.
A class decorator to indicate that an XBlock-like class needs particular services.
"""
def _decorator(cls_):
for service_name in service_names:
Expand All @@ -221,7 +221,7 @@ def _decorator(cls_):
@classmethod
def wants(cls, *service_names):
"""
A class decorator to indicate that a Blocklike class wants particular services.
A class decorator to indicate that a XBlock-like class wants particular services.
"""
def _decorator(cls_):
for service_name in service_names:
Expand All @@ -234,7 +234,7 @@ def service_declaration(cls, service_name):
"""
Find and return a service declaration.
Blocklikes declare their service requirements with `@XBlock{Aside}.needs` and
XBlock-like classes declare their service requirements with `@XBlock{Aside}.needs` and
`@XBlock{Aside}.wants` decorators. These store information on the class.
This function finds those declarations for a block.
Expand All @@ -251,7 +251,7 @@ def _services_requested(cls): # pylint: disable=no-self-argument
"""
A per-class dictionary to store the services requested by a particular XBlock.
"""
return {"field-data": "needs"} # All Blocklikes needs field data
return {}

@class_lazy
def _combined_services(cls): # pylint: disable=no-self-argument
Expand Down Expand Up @@ -305,7 +305,7 @@ def _set_field_if_present(cls, block, name, value, attrs):
else:
setattr(block, name, value)
else:
logging.warning("Blocklike %s does not contain field %s", type(block), name)
logging.warning("%s does not contain field %s", type(block), name)

def __init__(self, runtime, scope_ids, field_data=None, **kwargs):
"""
Expand All @@ -317,7 +317,7 @@ def __init__(self, runtime, scope_ids, field_data=None, **kwargs):
scope_ids (:class:`.ScopeIds`): Identifiers needed to resolve
scopes.
field_data (:class:`.FieldData`): Interface used by Blocklike's
field_data (:class:`.FieldData`): Interface used by XBlock-likes'
fields to access their data from wherever it is persisted.
DEPRECATED--supply a field-data Runtime service instead.
"""
Expand Down Expand Up @@ -369,7 +369,7 @@ def __repr__(self):
@property
def usage_key(self):
"""
A key identifying this particular usage of the Blocklike, unique across all learning contexts in the system.
A key identifying this particular usage of the XBlock-like, unique across all learning contexts in the system.
Equivalent to to `.scope_ids.usage_id`.
"""
Expand All @@ -378,7 +378,7 @@ def usage_key(self):
@property
def context_key(self):
"""
A key identifying the learning context (course, library, etc.) that contains this Blocklike usage.
A key identifying the learning context (course, library, etc.) that contains this XBlock-like usage.
Equivalent to `.scope_ids.usage_id.context_key`.
Expand Down Expand Up @@ -607,6 +607,7 @@ def __new__(mcs, name, bases, attrs):
return super().__new__(mcs, name, bases, attrs)


@Blocklike.needs("field-data")
class XBlock(Plugin, Blocklike, metaclass=_HasChildrenMetaclass):
"""
Base class for XBlocks. Derive from this class to create new type of XBlock.
Expand Down Expand Up @@ -732,8 +733,8 @@ def __init__(
self,
runtime,
field_data=None,
*args, # pylint: disable=keyword-arg-before-vararg
scope_ids=UNSET,
*args, # pylint: disable=keyword-arg-before-vararg
**kwargs
):
"""
Expand Down Expand Up @@ -891,6 +892,7 @@ def has_support(self, view, functionality):
return hasattr(view, "_supports") and functionality in view._supports # pylint: disable=protected-access


@Blocklike.needs("field-data")
class XBlockAside(Plugin, Blocklike):
"""
Base class for XBlock-like objects that are rendered alongside :class:`.XBlock` views.
Expand Down
19 changes: 13 additions & 6 deletions xblock/test/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,7 @@ def to_json(self, value):
"""Convert a datetime object to a string"""
return value.strftime("%m/%d/%Y")

@Blocklike.needs("field-data")
class FieldTester(Blocklike):
"""Toy class for field access testing"""

Expand Down Expand Up @@ -434,8 +435,9 @@ class FieldTester(XBlock):

def test_object_identity():
# Check that values that are modified are what is returned
@Blocklike.needs("field-data")
class FieldTester(Blocklike):
"""Toy class for ModelMetaclass and field access testing"""
"""Toy class for field access testing"""
field_a = List(scope=Scope.settings)

def mock_default(block, name):
Expand Down Expand Up @@ -474,8 +476,9 @@ def mock_default(block, name):

def test_caching_is_per_instance():
# Test that values cached for one instance do not appear on another
@Blocklike.needs("field-data")
class FieldTester(Blocklike):
"""Toy class for ModelMetaclass and field access testing"""
"""Toy class for field access testing"""
field_a = List(scope=Scope.settings)

field_data = MagicMock(spec=FieldData)
Expand Down Expand Up @@ -776,10 +779,13 @@ def test_handle_shortcut():


def test_services_decorators():
# A default XBlock has requested no services
xblock = XBlock(None, None, None)
assert not XBlock._services_requested
assert not xblock._services_requested

class NoServicesBlock(XBlock):
"""XBlock requesting no services"""

no_services_block = NoServicesBlock(None, None, None)
assert not NoServicesBlock._services_requested
assert not no_services_block._services_requested

@XBlock.needs("n")
@XBlock.wants("w")
Expand Down Expand Up @@ -811,6 +817,7 @@ class SubServiceUsingBlock(ServiceUsingBlock):
assert sub_service_using_block.service_declaration("w1") == "want"
assert sub_service_using_block.service_declaration("n2") == "need"
assert sub_service_using_block.service_declaration("w2") == "want"
assert sub_service_using_block.service_declaration("field-data") == "need"
assert sub_service_using_block.service_declaration("xx") is None


Expand Down
4 changes: 2 additions & 2 deletions xblock/test/test_parsing.py
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ def test_dict_and_list_export_format(self):
def test_unknown_field_as_attribute_raises_warning(self, parameter_name):
with mock.patch('logging.warning') as patched_warn:
block = self.parse_xml_to_block(f"<leaf {parameter_name}='something irrelevant'></leaf>")
patched_warn.assert_called_once_with("XBlock %s does not contain field %s", type(block), parameter_name)
patched_warn.assert_called_once_with("%s does not contain field %s", type(block), parameter_name)

@XBlock.register_temp_plugin(LeafWithOption)
@ddt.data(
Expand All @@ -361,7 +361,7 @@ def test_unknown_field_as_node_raises_warning(self, parameter_name):
""") % (get_namespace_attrs(), parameter_name, parameter_name)
with mock.patch('logging.warning') as patched_warn:
block = self.parse_xml_to_block(xml)
patched_warn.assert_called_once_with("XBlock %s does not contain field %s", type(block), parameter_name)
patched_warn.assert_called_once_with("%s does not contain field %s", type(block), parameter_name)


class TestRoundTrip(XmlTest, unittest.TestCase):
Expand Down

0 comments on commit e13b78f

Please sign in to comment.