Skip to content
This repository was archived by the owner on Mar 18, 2026. It is now read-only.

Fix so real children work in studio and LMS (OC-513) - #5

Closed
bradenmacdonald wants to merge 4 commits into
masterfrom
real-children-studio
Closed

Fix so real children work in studio and LMS (OC-513)#5
bradenmacdonald wants to merge 4 commits into
masterfrom
real-children-studio

Conversation

@bradenmacdonald

Copy link
Copy Markdown
Member

This branch fixes two things:

  1. The "real children" conversion (Real children (OC-88) #1) was not compatible with Studio, due to the way mentoring is designed to be edited as XML, which Studio doesn't really support when it comes to child XBlocks. See the extensive discussion in OC-513 and on the ML.
    • Fix: I added some questionable hacks (contained within a new file, studio_xml_utils.py). Since Studio provides no mechanism for instantiating XBlocks from XML, the workaround is to create our own in-memory XBlock runtime and instantiate temporary XBlocks from XML using that temporary runtime. We can then see all the XBlocks as they are created from the XML and tell Studio to create similar ones and make them children of the mentoring block.
    • Problems with this fix:
      • It's definitely a bit of a hack, but that may be considered acceptable until we get around to implementing a new way of editing the block in Studio that doesn't require XML.
      • The child XBlocks can't expect to persist data to their XBlock fields. This is fine as none of them are currently designed to do this.
      • studio_xml_utils.py needs to access the modulestore directly. This is not ideal but isn't horrible since we already know we're being used in edX Studio. (This only ever occurs from the studio_submit view handler).
      • The XBlock must be edited and saved in Studio before it will work properly. This means that if you update an existing course or even just add a new block, you have to click "Edit > Save" on each XBlock before they will work. This is the only problem that I see being a big deal. I'm still thinking about what to do about this.
  2. edx-platform already has an html XBlock which conflicts with the html block we include in this repository. I fixed this by renaming all of the XBlocks to e.g. mentoring-html instead of html. I added some magic so that inside a <mentoring> element, the new mentoring- prefix can be left out.

Comment thread mentoring/studio_xml_utils.py Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we make these NotSupportedErrors?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kelketek I don't think there is such a thing as NotSupportedError? I can't make them NotImplementedError because the whole reason I added these four methods is to override the versions that raise NotImplementedError - otherwise this class is considered abstract and I get TypeError: Can't instantiate abstract class TransientRuntime.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bradenmacdonald Ah, yes, NotImplementedError is what I was going for. You should still replace these exceptions with that. @abstractmethod requires the method to be overwritten with something but I believe that something can raise a NotImplementedError and still meet the requirement.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh cool, you're right. I'll change them to NotImplementedError.

@Kelketek

Copy link
Copy Markdown
Member

@bradenmacdonald This solution makes sense to me. I'm not certain there's a 'more right' method, given the circumstances. I've left a comment on a line in your code there for a semantic issue I found, but other than that, this looks good to me. Are you looking for a thumbs up right now, or is this PR intended to receive more work first, and exist primarily as a conceptual demo?

@bradenmacdonald

Copy link
Copy Markdown
Member Author

Thanks @Kelketek ! Mostly just wanting feedback on this approach at the moment. Once I've finalized the approach and the code and tested against the existing course data I'll look for a thumbs up.

@bradenmacdonald

Copy link
Copy Markdown
Member Author

Closing in favor of #6.

@bradenmacdonald
bradenmacdonald deleted the real-children-studio branch February 23, 2015 20:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants