Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor or deprecate parent-child relationships between XBlocks #729

Open
13 tasks
kdmccormick opened this issue Mar 19, 2024 · 1 comment
Open
13 tasks

Comments

@kdmccormick
Copy link
Member

kdmccormick commented Mar 19, 2024

This would be a long-term initiative. It probably belongs on the platform-roadmap.

Context

We have decided that parent-child relationships should only exist at/below the VerticalBlock: https://github.com/openedx/edx-platform/blob/master/docs/decisions/0006-role-of-xblock.rst

We have further discussed that parent-child relationships in their entirety should be removed; I'm not sure if we came to a consensus on that.

TODO -- Add more context ( @ormsbee , @kdmccormick , and @bradenmacdonald ).

Tasks

Off the top of my head...

  • File a DEPR encompassing our plan to:
    • Remove parent-child relationships on the base XBlock class. Delete the _HasChildrenMetaclass. Move all parent-child logic to a new subclass, XBlockWithChildren. Make it XBlock.has_children a read-only class property. Some details.
    • Raise an exception if get_parent is called at or above the VerticalBlock level.
    • For all XBlockWithChildren subclasses above the VerticalBlock (so, CourseBlock, SectionBlock, and SequenceBlock): Move their implementations to either Learning Core or edx-platform djangoapps.
  • Decide: Should parent-child relationships still exist at/below the Vertical block?
    • Either way -> ADR it.
    • Yes -> We're done.
    • No -> File another DEPR encompassing our plan to:
      • Raise deprecation warnings on the XBlockWithChildren class.
      • For all remaining XBlockWithChildren subclasses in edx-platform (eg, LibraryContentBlock, SplitTestBlock):
        Move their implementations to either Learning Core or edx-platform djangoapps.
      • Write guidance on how custom XBlockWithChildren subclasses can be similarly migrated. Ensure that, for example, OpenCraft's ProblemBuilder block can be migrated using this guide.
      • Delete the XBlockWithChildren class, and delete all parent-child logic from the XBlock repository.
      • Delete all parent-child logic from modulestore and edx-platform's XBlock runtimes.
@bradenmacdonald
Copy link
Contributor

Should parent-child relationships still exist at/below the Vertical block?

Other than the "structural" XBlocks (course, sequential, vertical/unit, etc.), the only blocks I know of with has_children are:

  • ConditionalBlock (built-in)
  • SplitTestBlock (built-in, for A/B testing)
  • LibraryContentBlock (built-in, may be renamed to Problem Bank or Item Pool Block)
  • Problem Builder (which we may deprecate)
    • Problem Builder makes extensive use of child blocks. If we deprecated has_children, we'd need to figure out a path forward for courses that use Problem Builder, but that's been known for a while so needn't be a blocker here, just something to solve at the same time.

For the other three (conditional, split, LCB), I think they're going to be a blocker for now until we have some kind of Outline concept that can unify and replace them. But what I'd love to do is put in a restriction so that only those three blocks can have children, and attempting to do it for other blocks will raise an error. That way we can prevent people from introducing new dependencies on has_children.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants