Skip to content

Commit

Permalink
Merge pull request openedx#31492 from open-craft/pooja/unify-xblock-n…
Browse files Browse the repository at this point in the history
…aming-descriptor

[BD-13][BB-6927] Unify xblock naming descriptor
  • Loading branch information
jristau1984 committed Apr 26, 2023
2 parents a2f1ad1 + e1fb866 commit 4afb210
Show file tree
Hide file tree
Showing 129 changed files with 1,684 additions and 1,714 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def print_course(course):
print('num type name')
for index, item in enumerate(course.tabs):
print(index + 1, '"' + item.get('type') + '"', '"' + item.get('name', '') + '"')
# If a course is bad we will get an error descriptor here, dump it and die instead of
# If a course is bad we will get an error here, dump it and die instead of
# just sending up the error that .id doesn't exist.
except AttributeError:
print(course)
Expand Down
4 changes: 2 additions & 2 deletions cms/djangoapps/contentstore/signals/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
LibrarySearchIndexer,
)
from common.djangoapps.track.event_transaction_utils import get_event_transaction_id, get_event_transaction_type
from common.djangoapps.util.block_utils import yield_dynamic_descriptor_descendants
from common.djangoapps.util.block_utils import yield_dynamic_block_descendants
from lms.djangoapps.grades.api import task_compute_all_grades_for_course
from openedx.core.djangoapps.content.learning_sequences.api import key_supports_outlines
from openedx.core.djangoapps.discussions.tasks import update_discussions_settings_from_course_task
Expand Down Expand Up @@ -252,7 +252,7 @@ def handle_item_deleted(**kwargs):
usage_key = usage_key.for_branch(None)
course_key = usage_key.course_key
deleted_block = modulestore().get_item(usage_key)
for block in yield_dynamic_descriptor_descendants(deleted_block, kwargs.get('user_id')):
for block in yield_dynamic_block_descendants(deleted_block, kwargs.get('user_id')):
# Remove prerequisite milestone data
gating_api.remove_prerequisite(block.location)
# Remove any 'requires' course content milestone relationships
Expand Down
18 changes: 9 additions & 9 deletions cms/djangoapps/contentstore/tests/test_contentstore.py
Original file line number Diff line number Diff line change
Expand Up @@ -1713,17 +1713,17 @@ def setUp(self):
"""
video_data = VideoBlock.parse_video_xml(video_sample_xml)
video_data.pop('source')
self.video_descriptor = BlockFactory.create(
self.video_block = BlockFactory.create(
parent_location=course.location, category='video',
**video_data
)

def test_metadata_not_persistence(self):
"""
Test that descriptors which set metadata fields in their
Test that blocks which set metadata fields in their
constructor are correctly deleted.
"""
self.assertIn('html5_sources', own_metadata(self.video_descriptor))
self.assertIn('html5_sources', own_metadata(self.video_block))
attrs_to_strip = {
'show_captions',
'youtube_id_1_0',
Expand All @@ -1736,13 +1736,13 @@ def test_metadata_not_persistence(self):
'track'
}

location = self.video_descriptor.location
location = self.video_block.location

for field_name in attrs_to_strip:
delattr(self.video_descriptor, field_name)
delattr(self.video_block, field_name)

self.assertNotIn('html5_sources', own_metadata(self.video_descriptor))
self.store.update_item(self.video_descriptor, self.user.id)
self.assertNotIn('html5_sources', own_metadata(self.video_block))
self.store.update_item(self.video_block, self.user.id)
block = self.store.get_item(location)

self.assertNotIn('html5_sources', own_metadata(block))
Expand Down Expand Up @@ -2047,12 +2047,12 @@ def test_course_license_export(self):
def test_video_license_export(self):
content_store = contentstore()
root_dir = path(mkdtemp_clean())
video_descriptor = BlockFactory.create(
video_block = BlockFactory.create(
parent_location=self.course.location, category='video',
license="all-rights-reserved"
)
export_course_to_xml(self.store, content_store, self.course.id, root_dir, 'test_license')
fname = f"{video_descriptor.scope_ids.usage_id.block_id}.xml"
fname = f"{video_block.scope_ids.usage_id.block_id}.xml"
video_file_path = root_dir / "test_license" / "video" / fname
with video_file_path.open() as f:
video_xml = etree.parse(f)
Expand Down
20 changes: 10 additions & 10 deletions cms/djangoapps/contentstore/tests/test_course_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -885,33 +885,33 @@ def test_delete_grace_period(self):
@mock.patch('cms.djangoapps.contentstore.signals.signals.GRADING_POLICY_CHANGED.send')
def test_update_section_grader_type(self, send_signal, tracker, uuid):
uuid.return_value = 'mockUUID'
# Get the descriptor and the section_grader_type and assert they are the default values
descriptor = modulestore().get_item(self.course.location)
# Get the block and the section_grader_type and assert they are the default values
block = modulestore().get_item(self.course.location)
section_grader_type = CourseGradingModel.get_section_grader_type(self.course.location)

self.assertEqual('notgraded', section_grader_type['graderType'])
self.assertEqual(None, descriptor.format)
self.assertEqual(False, descriptor.graded)
self.assertEqual(None, block.format)
self.assertEqual(False, block.graded)

# Change the default grader type to Homework, which should also mark the section as graded
CourseGradingModel.update_section_grader_type(self.course, 'Homework', self.user)
descriptor = modulestore().get_item(self.course.location)
block = modulestore().get_item(self.course.location)
section_grader_type = CourseGradingModel.get_section_grader_type(self.course.location)
grading_policy_1 = self._grading_policy_hash_for_course()

self.assertEqual('Homework', section_grader_type['graderType'])
self.assertEqual('Homework', descriptor.format)
self.assertEqual(True, descriptor.graded)
self.assertEqual('Homework', block.format)
self.assertEqual(True, block.graded)

# Change the grader type back to notgraded, which should also unmark the section as graded
CourseGradingModel.update_section_grader_type(self.course, 'notgraded', self.user)
descriptor = modulestore().get_item(self.course.location)
block = modulestore().get_item(self.course.location)
section_grader_type = CourseGradingModel.get_section_grader_type(self.course.location)
grading_policy_2 = self._grading_policy_hash_for_course()

self.assertEqual('notgraded', section_grader_type['graderType'])
self.assertEqual(None, descriptor.format)
self.assertEqual(False, descriptor.graded)
self.assertEqual(None, block.format)
self.assertEqual(False, block.graded)

# one for each call to update_section_grader_type()
send_signal.assert_has_calls([
Expand Down
18 changes: 9 additions & 9 deletions cms/djangoapps/contentstore/tests/test_i18n.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,19 +69,19 @@ def setUp(self):
self.request = mock.Mock()
self.course = CourseFactory.create()
self.field_data = mock.Mock()
self.descriptor = BlockFactory(category="pure", parent=self.course)
self.block = BlockFactory(category="pure", parent=self.course)
_prepare_runtime_for_preview(
self.request,
self.descriptor,
self.block,
self.field_data,
)
self.addCleanup(translation.deactivate)

def get_block_i18n_service(self, descriptor):
def get_block_i18n_service(self, block):
"""
return the block i18n service.
"""
i18n_service = self.descriptor.runtime.service(descriptor, 'i18n')
i18n_service = self.block.runtime.service(block, 'i18n')
self.assertIsNotNone(i18n_service)
self.assertIsInstance(i18n_service, XBlockI18nService)
return i18n_service
Expand Down Expand Up @@ -113,7 +113,7 @@ def __exit__(self, _type, _value, _traceback):
self.module.ugettext = self.old_ugettext
self.module.gettext = self.old_ugettext

i18n_service = self.get_block_i18n_service(self.descriptor)
i18n_service = self.get_block_i18n_service(self.block)

# Activate french, so that if the fr files haven't been loaded, they will be loaded now.
with translation.override("fr"):
Expand Down Expand Up @@ -150,28 +150,28 @@ def test_message_catalog_translations(self):
translation.activate("es")
with mock.patch('gettext.translation', return_value=_translator(domain='text', localedir=localedir,
languages=[get_language()])):
i18n_service = self.get_block_i18n_service(self.descriptor)
i18n_service = self.get_block_i18n_service(self.block)
self.assertEqual(i18n_service.ugettext('Hello'), 'es-hello-world')

translation.activate("ar")
with mock.patch('gettext.translation', return_value=_translator(domain='text', localedir=localedir,
languages=[get_language()])):
i18n_service = self.get_block_i18n_service(self.descriptor)
i18n_service = self.get_block_i18n_service(self.block)
self.assertEqual(get_gettext(i18n_service)('Hello'), 'Hello')
self.assertNotEqual(get_gettext(i18n_service)('Hello'), 'fr-hello-world')
self.assertNotEqual(get_gettext(i18n_service)('Hello'), 'es-hello-world')

translation.activate("fr")
with mock.patch('gettext.translation', return_value=_translator(domain='text', localedir=localedir,
languages=[get_language()])):
i18n_service = self.get_block_i18n_service(self.descriptor)
i18n_service = self.get_block_i18n_service(self.block)
self.assertEqual(i18n_service.ugettext('Hello'), 'fr-hello-world')

def test_i18n_service_callable(self):
"""
Test: i18n service should be callable in studio.
"""
self.assertTrue(callable(self.descriptor.runtime._services.get('i18n'))) # pylint: disable=protected-access
self.assertTrue(callable(self.block.runtime._services.get('i18n'))) # pylint: disable=protected-access


class InternationalizationTest(ModuleStoreTestCase):
Expand Down
12 changes: 6 additions & 6 deletions cms/djangoapps/contentstore/tests/test_libraries.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ def _refresh_children(self, lib_content_block, status_code_expected=200):
self.assertEqual(response.status_code, status_code_expected)
return modulestore().get_item(lib_content_block.location)

def _bind_block(self, descriptor, user=None):
def _bind_block(self, block, user=None):
"""
Helper to use the CMS's module system so we can access student-specific fields.
"""
Expand All @@ -123,7 +123,7 @@ def _bind_block(self, descriptor, user=None):
if user not in self.session_data:
self.session_data[user] = {}
request = Mock(user=user, session=self.session_data[user])
_load_preview_block(request, descriptor)
_load_preview_block(request, block)

def _update_block(self, usage_key, metadata):
"""
Expand Down Expand Up @@ -174,13 +174,13 @@ def test_max_items(self, num_to_create, num_to_select, num_expected):
lc_block = self._refresh_children(lc_block)

# Now, we want to make sure that .children has the total # of potential
# children, and that get_child_descriptors() returns the actual children
# children, and that get_child_blocks() returns the actual children
# chosen for a given student.
# In order to be able to call get_child_descriptors(), we must first
# In order to be able to call get_child_blocks(), we must first
# call bind_for_student:
self._bind_block(lc_block)
self.assertEqual(len(lc_block.children), num_to_create)
self.assertEqual(len(lc_block.get_child_descriptors()), num_expected)
self.assertEqual(len(lc_block.get_child_blocks()), num_expected)

def test_consistent_children(self):
"""
Expand All @@ -204,7 +204,7 @@ def get_child_of_lc_block(block):
"""
Fetch the child shown to the current user.
"""
children = block.get_child_descriptors()
children = block.get_child_blocks()
self.assertEqual(len(children), 1)
return children[0]

Expand Down
2 changes: 1 addition & 1 deletion cms/djangoapps/contentstore/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ def test_exclude_partitions_with_no_groups(self):
self.assertEqual(partitions[0]["scheme"], "random")

def _set_partitions(self, partitions):
"""Set the user partitions of the course descriptor. """
"""Set the user partitions of the course block. """
self.course.user_partitions = partitions
self.course = self.store.update_item(self.course, ModuleStoreEnum.UserID.test)

Expand Down
4 changes: 2 additions & 2 deletions cms/djangoapps/contentstore/tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,8 @@ def check_verticals(self, items):
""" Test getting the editing HTML for each vertical. """
# assert is here to make sure that the course being tested actually has verticals (units) to check.
self.assertGreater(len(items), 0, "Course has no verticals (units) to check")
for descriptor in items:
resp = self.client.get_html(get_url('container_handler', descriptor.location))
for block in items:
resp = self.client.get_html(get_url('container_handler', block.location))
self.assertEqual(resp.status_code, 200)

def assertAssetsEqual(self, asset_son, course1_id, course2_id):
Expand Down
12 changes: 6 additions & 6 deletions cms/djangoapps/contentstore/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ def get_split_group_display_name(xblock, course):
Arguments:
xblock (XBlock): The courseware component.
course (XBlock): The course descriptor.
course (XBlock): The course block.
Returns:
group name (String): Group name of the matching group xblock.
Expand All @@ -399,14 +399,14 @@ def get_user_partition_info(xblock, schemes=None, course=None):
schemes (iterable of str): If provided, filter partitions to include only
schemes with the provided names.
course (XBlock): The course descriptor. If provided, uses this to look up the user partitions
course (XBlock): The course block. If provided, uses this to look up the user partitions
instead of loading the course. This is useful if we're calling this function multiple
times for the same course want to minimize queries to the modulestore.
Returns: list
Example Usage:
>>> get_user_partition_info(block, schemes=["cohort", "verification"])
>>> get_user_partition_info(xblock, schemes=["cohort", "verification"])
[
{
"id": 12345,
Expand Down Expand Up @@ -509,7 +509,7 @@ def get_visibility_partition_info(xblock, course=None):
Arguments:
xblock (XBlock): The component being edited.
course (XBlock): The course descriptor. If provided, uses this to look up the user partitions
course (XBlock): The course block. If provided, uses this to look up the user partitions
instead of loading the course. This is useful if we're calling this function multiple
times for the same course want to minimize queries to the modulestore.
Expand Down Expand Up @@ -569,8 +569,8 @@ def get_xblock_aside_instance(usage_key):
:param usage_key: Usage key of aside xblock
"""
try:
descriptor = modulestore().get_item(usage_key.usage_key)
for aside in descriptor.runtime.get_asides(descriptor):
xblock = modulestore().get_item(usage_key.usage_key)
for aside in xblock.runtime.get_asides(xblock):
if aside.scope_ids.block_type == usage_key.aside_type:
return aside
except ItemNotFoundError:
Expand Down
2 changes: 1 addition & 1 deletion cms/djangoapps/contentstore/views/certificates.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ class CertificateValidationError(CertificateException):
class CertificateManager:
"""
The CertificateManager is responsible for storage, retrieval, and manipulation of Certificates
Certificates are not stored in the Django ORM, they are a field/setting on the course descriptor
Certificates are not stored in the Django ORM, they are a field/setting on the course block
"""
@staticmethod
def parse(json_string):
Expand Down
20 changes: 10 additions & 10 deletions cms/djangoapps/contentstore/views/component.py
Original file line number Diff line number Diff line change
Expand Up @@ -552,18 +552,18 @@ def component_handler(request, usage_key_string, handler, suffix=''):

try:
if is_xblock_aside(usage_key):
# Get the descriptor for the block being wrapped by the aside (not the aside itself)
descriptor = modulestore().get_item(usage_key.usage_key)
handler_descriptor = get_aside_from_xblock(descriptor, usage_key.aside_type)
asides = [handler_descriptor]
# Get the block being wrapped by the aside (not the aside itself)
block = modulestore().get_item(usage_key.usage_key)
handler_block = get_aside_from_xblock(block, usage_key.aside_type)
asides = [handler_block]
else:
descriptor = modulestore().get_item(usage_key)
handler_descriptor = descriptor
block = modulestore().get_item(usage_key)
handler_block = block
asides = []
load_services_for_studio(handler_descriptor.runtime, request.user)
resp = handler_descriptor.handle(handler, req, suffix)
load_services_for_studio(handler_block.runtime, request.user)
resp = handler_block.handle(handler, req, suffix)
except NoSuchHandlerError:
log.info("XBlock %s attempted to access missing handler %r", handler_descriptor, handler, exc_info=True)
log.info("XBlock %s attempted to access missing handler %r", handler_block, handler, exc_info=True)
raise Http404 # lint-amnesty, pylint: disable=raise-missing-from

# unintentional update to handle any side effects of handle call
Expand All @@ -572,7 +572,7 @@ def component_handler(request, usage_key_string, handler, suffix=''):
# TNL 101-62 studio write permission is also checked for editing content.

if has_course_author_access(request.user, usage_key.course_key):
modulestore().update_item(descriptor, request.user.id, asides=asides)
modulestore().update_item(block, request.user.id, asides=asides)
else:
#fail quietly if user is not course author.
log.warning(
Expand Down
4 changes: 2 additions & 2 deletions cms/djangoapps/contentstore/views/entrance_exam.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,9 @@ def _get_entrance_exam(request, course_key):
except InvalidKeyError:
return HttpResponse(status=404)
try:
exam_descriptor = modulestore().get_item(exam_key)
exam_block = modulestore().get_item(exam_key)
return HttpResponse( # lint-amnesty, pylint: disable=http-response-with-content-type-json
dump_js_escaped_json({'locator': str(exam_descriptor.location)}),
dump_js_escaped_json({'locator': str(exam_block.location)}),
status=200, content_type='application/json')
except ItemNotFoundError:
return HttpResponse(status=404)
Expand Down
Loading

0 comments on commit 4afb210

Please sign in to comment.