refactor: Simplify backcompat definitions for url_name, et al#37845
Open
kdmccormick wants to merge 1 commit intomasterfrom
Open
refactor: Simplify backcompat definitions for url_name, et al#37845kdmccormick wants to merge 1 commit intomasterfrom
kdmccormick wants to merge 1 commit intomasterfrom
Conversation
kdmccormick
requested review from
farhan,
irtazaakram and
salman2013
as code owners
kdmccormick
force-pushed
the
kdmccormick/url-name
branch
from
56032b2 to
7c8ccfb
Compare
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
kdmccormick
force-pushed
the
kdmccormick/url-name
branch
from
7c8ccfb to
1c292be
Compare
Member
Author
|
@farhan , could you please review this, and then incorporate it into openedx/xblocks-contrib#128 ? |
Contributor
Sure, I will, added in the queue |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description & Supporting Info
XModuleMixin and CourseOverview both provided some backcompat names for some XBlock key attributes. Due to refactors 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:
These are the ways we would like developers to access these attributes going forward, so it's important that we use them in XModuleMixin and CourseOverview to set a good example.
We also remove the
block_metadata_utils.url_name_for_blockmethod, as it was unnecessarily indirect for such a simple operation. There are no remaining references to it in edx-platform.Note: It is technically correct to go through
.scope_idsfor 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
Testing instructions
None
Deadline
Soon, as this influences openedx/xblocks-contrib#128