✨(core) change field to store configuration name and improve fallback #7
Conversation
Use 'lti_id' field instead of 'description' because it will ease course based configuration fallback if this xblock is not installed
| return conf[0] | ||
| else: | ||
| logger.warning( | ||
| "Configuration '{name}' does not exist".format(name=name)) |
There was a problem hiding this comment.
there should be an "else" clause to the "for" loop and raise an ConfigurationError as well because if the "for" loop completes normally, it means that we did not return and so the "lti_consumer" conf was not found.
| logger.warning( | ||
| "Configuration '{name}' does not exist".format(name=name)) | ||
| else: | ||
| logger.warning("CONFIGURABLE_XBLOCKS_SETTINGS settings are not set") |
There was a problem hiding this comment.
let's break and raise a ConfigurationError instead of just logging a warning because it is the admin sys reponsibility to fix it before starting the server. WDYT?
| if conf: | ||
| return conf[0] | ||
| return None | ||
| if getattr(settings, "CONFIGURABLE_XBLOCKS_SETTINGS"): |
There was a problem hiding this comment.
We could add a defaults.py file to this project to define the default setting (i.e act as a normal lti xblock):
"""Default settings for the ``core`` app of the Marsha project."""
from django.conf import settings
CONFIGURABLE_XBLOCKS_SETTINGS = getattr(
settings, "CONFIGURABLE_XBLOCKS_SETTINGS", {
"components": [{
"module": "lti_consumer",
"base_class": "ConfigurableLtiConsumerXBlock",
"subclasses": [{
"name": "Generic",
"display": "Generic LTI xblock",
"default_values": {
"description": "Generic",
"ask_to_send_username": True,
"ask_to_send_email": True
}
}]
}]
}
)This would allow us to remove the default settings in openfun/openedx-docker
There was a problem hiding this comment.
Good idea, then it implies that this configuration becomes specific to this xblock.
the components list becomes useless
Developers working on this project need an easy way to setup a development environment for this xblock. Thanks to docker and docker-compose, we are now able to easily bootstrap such environment in an openedx context.
Use 'lti_id' field instead of 'description' because it will ease course
based configuration fallback if this xblock is not installed