feat: Explicit mapping from container_types to OLX tags - #36580
Conversation
|
|
||
| These tag names are historical. We keep them around for the backwards compatibility of OLX | ||
| and for easier interation with modulestore-powered content (e.g., copy-paste between content | ||
| libraries and courses. |
There was a problem hiding this comment.
| libraries and courses. | |
| libraries and courses). |
| Section = "section" | ||
|
|
||
| @property | ||
| def olx_tag(self) -> str: |
There was a problem hiding this comment.
I'm torn between calling this block_type vs olx_tag. On one hand, we generally don't want to think of Containers as pseudo-XBlocks, so I'm trying to frame this all as an OLX thing. On the other hand, container_sync will need to understand that Unit maps to the block_type vertical.
bradenmacdonald
left a comment
bradenmacdonald
left a comment
There was a problem hiding this comment.
Thanks, just one request.
| raise TypeError(f"unexpected ContainerType: {self!r}") | ||
|
|
||
| @property | ||
| def source_olx_tags(self) -> set[str]: |
There was a problem hiding this comment.
I get the need for this, but I'd prefer that this be a private method; I don't want the public API to involve multiple tags per container as it's confusing. i.e. lets focus on .olx_tag and .from_source_olx_tag() as the public API, and this is an implementation detail.
There was a problem hiding this comment.
Good call. Updated, ready for re-review @bradenmacdonald .
kdmccormick
changed the title
a4eb18f to
19fce2c
Compare
19fce2c to
e48bdee
Compare
bradenmacdonald
left a comment
bradenmacdonald
left a comment
There was a problem hiding this comment.
I didn't test this but the code LGTM.
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
1 similar comment
|
2U Release Notice: This PR has been deployed to the edX production environment. |
This is needed by several aspects of the Teak Libraries Overhaul (https://github.com/orgs/openedx/projects/66) including: * copy-paste of containers between courses and V2 libraries * syncing of containers in courses from V2 libraries * the import_from_modulestore API
This is needed by several aspects of the Teak Libraries Overhaul (https://github.com/orgs/openedx/projects/66) including: * copy-paste of containers between courses and V2 libraries * syncing of containers in courses from V2 libraries * the import_from_modulestore API
This is needed by several aspects of the Teak Libraries Overhaul (https://github.com/orgs/openedx/projects/66) including: * copy-paste of containers between courses and V2 libraries * syncing of containers in courses from V2 libraries * the import_from_modulestore API
4 participants
This is needed by several aspects of the Teak Libraries Overhaul (Libraries Overhaul) including: