Skip to content

Commit 7c8ccfb

Browse files
committed
refactor: Simplify backcompat definitions for url_name, et al
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 497ffae commit 7c8ccfb

6 files changed

Lines changed: 18 additions & 26 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/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: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -312,17 +312,17 @@ def system(self):
312312
@property
313313
def course_id(self):
314314
"""Return the course key for this block."""
315-
return self.location.course_key
315+
return self.context_key
316316

317317
@property
318318
def category(self):
319319
"""Return the block type/category."""
320-
return self.scope_ids.block_type
320+
return self.usage_key.block_type
321321

322322
@property
323323
def location(self):
324324
"""Return the usage key identifying this block instance."""
325-
return self.scope_ids.usage_id
325+
return self.usage_key
326326

327327
@location.setter
328328
def location(self, value):
@@ -335,7 +335,7 @@ def location(self, value):
335335
@property
336336
def url_name(self):
337337
"""Return the URL-friendly name for this block."""
338-
return block_metadata_utils.url_name_for_block(self)
338+
return self.usage_key.block_id
339339

340340
@property
341341
def display_name_with_default(self):

0 commit comments

Comments
 (0)