Skip to content

Commit a8d6dda

Browse files
kdmccormickbrianjbuck-wgu
authored andcommitted
refactor: Simplify backcompat definitions for url_name, et al (openedx#37845)
XModuleMixin and CourseOverview both provided some backcompat names for some XBlock key attributes. Due to refactors that have happened in edx-platform and improvements to the XBlock API that have happened over the years, we can now simplify these backcompat mappings. The new mappings (old -> new) are: * .course_id -> .context_key * .location -> .usage_key * .url_name -> .usage_key.block_id * .category -> .usage_key.block_type These are the ways we would like developers to access these attributes going forward, so it's important that we set the example in XModuleMixin and CourseOverview. Note: It is technically correct to go through `.scope_ids` for these fields, but it's harder to read. Under the hood, the XBlock API indeed uses `.scope_ids`: https://github.com/openedx/XBlock/blob/v5.3.0/xblock/core.py#L422-L446 Part of: openedx/xblocks-contrib#125
1 parent 1ec8de9 commit a8d6dda

7 files changed

Lines changed: 43 additions & 29 deletions

File tree

lms/djangoapps/grades/course_grade.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ def _get_chapter_grade_info(self, chapter, course_structure):
242242
chapter_subsection_grades = self._get_subsection_grades(course_structure, chapter.location)
243243
return {
244244
'display_name': block_metadata_utils.display_name_with_default(chapter),
245-
'url_name': block_metadata_utils.url_name_for_block(chapter),
245+
'url_name': chapter.usage_key.block_id,
246246
'sections': chapter_subsection_grades,
247247
}
248248

lms/djangoapps/grades/subsection_grade.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class SubsectionGradeBase(metaclass=ABCMeta):
2626
def __init__(self, subsection):
2727
self.location = subsection.location
2828
self.display_name = block_metadata_utils.display_name_with_default(subsection)
29-
self.url_name = block_metadata_utils.url_name_for_block(subsection)
29+
self.url_name = subsection.usage_key.block_id
3030

3131
self.due = block_metadata_utils.get_datetime_field(subsection, 'due', None)
3232
self.end = getattr(subsection, 'end', None)

openedx/core/djangoapps/content/block_structure/block_structure.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,10 @@ def __init__(self, usage_key):
393393
# Map of transformer name to its block-specific data.
394394
self.transformer_data = TransformerDataMap()
395395

396+
@property
397+
def usage_key(self):
398+
return self.location
399+
396400

397401
class BlockStructureBlockData(BlockStructure):
398402
"""

openedx/core/djangoapps/content/course_overviews/models.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -478,9 +478,9 @@ def clean_id(self, padding_char='='):
478478
return course_metadata_utils.clean_course_key(self.location.course_key, padding_char)
479479

480480
@property
481-
def location(self):
481+
def usage_key(self):
482482
"""
483-
Returns the UsageKey of this course.
483+
Returns the UsageKey of the root block of this course.
484484
485485
UsageKeyField has a strange behavior where it fails to parse the "run"
486486
of a course out of the serialized form of a Mongo Draft UsageKey. This
@@ -491,6 +491,13 @@ def location(self):
491491
self._location = self._location.map_into_course(self.id)
492492
return self._location
493493

494+
@property
495+
def location(self):
496+
"""
497+
The old name for `usage_key`. This method is analogous to `XModuleMixin.location`.
498+
"""
499+
return self.usage_key
500+
494501
@property
495502
def number(self):
496503
"""
@@ -506,9 +513,9 @@ def number(self):
506513
@property
507514
def url_name(self):
508515
"""
509-
Returns this course's URL name.
516+
The old name for `block_id`. This method is analogous to `XModuleMixin.url_name`.
510517
"""
511-
return block_metadata_utils.url_name_for_block(self)
518+
return self.usage_key.block_id
512519

513520
@property
514521
def display_name_with_default(self):

xmodule/block_metadata_utils.py

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,6 @@
1515
logger = getLogger(__name__) # pylint: disable=invalid-name
1616

1717

18-
def url_name_for_block(block):
19-
"""
20-
Given a block, returns the block's URL name.
21-
22-
Arguments:
23-
block (XModuleMixin|CourseOverview|BlockStructureBlockData):
24-
Block that is being accessed
25-
"""
26-
return block.location.block_id
27-
28-
2918
def display_name_with_default(block):
3019
"""
3120
Calculates the display name for a block.
@@ -50,7 +39,7 @@ def display_name_with_default(block):
5039
"""
5140
return (
5241
block.display_name if block.display_name is not None
53-
else url_name_for_block(block).replace('_', ' ')
42+
else block.usage_key.block_id.replace('_', ' ')
5443
)
5544

5645

xmodule/tests/test_course_metadata_utils.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
from xmodule.block_metadata_utils import (
1313
display_name_with_default,
1414
display_name_with_default_escaped,
15-
url_name_for_block
1615
)
1716
from xmodule.course_metadata_utils import (
1817
DEFAULT_START_DATE,
@@ -125,9 +124,6 @@ def noop_gettext(text): # lint-amnesty, pylint: disable=unused-variable
125124
"course_MNXXK4TTMUWXMMJ2KVXGS5TFOJZWS5DZLAVUGUZNGIYDGK2ZGIYDSNQ~"
126125
),
127126
]),
128-
FunctionTest(url_name_for_block, [
129-
TestScenario((self.html_course,), self.html_course.location.block_id),
130-
]),
131127
FunctionTest(display_name_with_default_escaped, [
132128
# Test course with a display name that contains characters that need escaping.
133129
TestScenario((self.html_course,), "Intro to html"),

xmodule/x_module.py

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -311,18 +311,32 @@ def system(self):
311311

312312
@property
313313
def course_id(self):
314-
"""Return the course key for this block."""
315-
return self.location.course_key
314+
"""
315+
Return the key of the block to which this Course belongs.
316+
317+
New code should always used `context_key`, which is the key of the Learning Context to which
318+
this block belongs. "Learning Context" is a generalized notion of Courses which is inclusive
319+
of Content Libraries, et al.
320+
"""
321+
return self.context_key
316322

317323
@property
318324
def category(self):
319-
"""Return the block type/category."""
325+
"""
326+
Return the block type, formerly known as "category".
327+
328+
Preferred forms for new code: `self.usage_key.block_type` or `self.scope_ids.blocks_type`
329+
"""
320330
return self.scope_ids.block_type
321331

322332
@property
323333
def location(self):
324-
"""Return the usage key identifying this block instance."""
325-
return self.scope_ids.usage_id
334+
"""
335+
Return the usage key identifying this block instance, formerly called the "location".
336+
337+
`self.usage_key` is always preferred in new code.
338+
"""
339+
return self.usage_key
326340

327341
@location.setter
328342
def location(self, value):
@@ -334,8 +348,12 @@ def location(self, value):
334348

335349
@property
336350
def url_name(self):
337-
"""Return the URL-friendly name for this block."""
338-
return block_metadata_utils.url_name_for_block(self)
351+
"""
352+
Return the URL-friendly name for this block.
353+
354+
Preferred forms for new code: `self.usage_key.block_id`
355+
"""
356+
return self.usage_key.block_id
339357

340358
@property
341359
def display_name_with_default(self):

0 commit comments

Comments
 (0)