-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
REST API to export modulestore XBlocks as OLX #23068
Conversation
Thanks for the pull request, @bradenmacdonald! I've created OSPR-4087 to keep track of it in JIRA. JIRA is a place for product owners to prioritize feature reviews by the engineering development teams. Feel free to add as much of the following information to the ticket:
All technical communication about the code itself will still be done via the GitHub pull request interface. As a reminder, our process documentation is here. |
ba7833c
to
85bfb41
Compare
85bfb41
to
1bb99de
Compare
jenkins run a11y |
1bb99de
to
b734573
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple minor comments/requests.
# Replace static urls like '/static/foo.png' | ||
static_paths = [] | ||
# Drag-and-drop-v2 has | ||
# "/static/blah.png" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(sigh)... totally legit comment... but (sigh)
5eb6082
to
50694c6
Compare
Thanks @ormsbee. Comments addressed, rebased, and squashed. |
59edb84
to
20084ae
Compare
@bradenmacdonald: Please update your commit message to include the context from your PR description. Also, please make a note of this new API on the Juniper page: https://openedx.atlassian.net/wiki/spaces/COMM/pages/940048716/Juniper Thank you. |
This was originally a separate plugin called openedx-olx-rest-api. It provides a Studio API that any user with course authoring permission can use to get the OLX of an individual XBlock or a unit. Without this, the only way to get an XBlock's OLX was to download the tarball of the entire course. Examples of usage (be logged in to Studio on devstack): Simple HTML XBlock: http://localhost:18010/api/olx-export/v1/xblock/block-v1:edX+DemoX+Demo_Course+type@html+block@030e35c4756a4ddc8d40b95fbbfff4d4/ Exporting a unit: http://localhost:18010/api/olx-export/v1/xblock/block-v1:edX+DemoX+Demo_Course+type@vertical+block@134df56c516a4a0dbb24dd5facef746e/ Example output for an HTML block: { "root_block_id":"block-v1:edX+DemoX+Demo_Course+type@html+block@030e35c4756a4ddc8d40b95fbbfff4d4", "blocks":{ "block-v1:edX+DemoX+Demo_Course+type@html+block@030e35c4756a4ddc8d40b95fbbfff4d4":{ "olx":"<html display_name=\"Blank HTML Page\"><![CDATA[\n<p><strong>Welcome to the edX Demo Course Introduction.</strong></p>\n]]></html>\n" } } } The code is designed primarily for use when importing content into Blockstore. So it will: * Export HTML blocks as a combined OLX/HTML file, with the HTML in a CDATA section * Convert vertical blocks to unit blocks (unit is like a vertical but has no UI elements) * Detect static files (such as images) used by the XBlock and list the absolute URL of each static file in the "static_files": {...} JSON element for each XBlock that has at least one static file usage. This can handle static files that are in mongo ("contentstore" / "Files & Uploads") as well as files generated on-the-fly during OLX serialization via the export_fs API (mostly this is video transcripts).
20084ae
to
23d649d
Compare
Your PR has finished running tests. There were no failures. |
@ormsbee Done (both) - thanks! |
@bradenmacdonald 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production. |
EdX Release Notice: This PR has been deployed to the production environment. |
olx_node = etree.Element("html") | ||
if block.display_name: | ||
olx_node.attrib["display_name"] = block.display_name | ||
olx_node.text = etree.CDATA("\n" + block.data + "\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bradenmacdonald Something I stumbled into while unit testing upstream_sync: this line causes html_block.data
to be wrapped in newlines, if it's not wrapped already. Does that sound right? It doesn't change the meaning of the HTML, so I think it's fine?
Minimal repro:
# Boilerplate...
from organizations.api import ensure_organization
from organizations.models import Organization
from openedx.core.djangoapps.content_libraries import api as libs
from openedx.core.djangoapps.xblock import api as xblock
from django.contrib.auth.models import User
user = User.objects.get(username="openedx")
ensure_organization("TestX")
library = libs.create_library(
org=Organization.objects.get(short_name="TestX"), slug="TestLib", title="Test Library"
)
block_key = libs.create_library_block(library.key, "html", "test-newlines").usage_key
# Initial block data: No newlines
block = xblock.load_block(block_key, user)
block.data = "<html><body>Hello</body></html>"
print(repr(block.data)) # '<html><body>Hello</body></html>'
block.save()
# First round-trip: data is wrapped in newlines
block = xblock.load_block(block_key, user)
print(repr(block.data)) # '\n<html><body>Hello</body></html>\n'
block.save()
# Second round-trip: data is stilled wrapped in newlines
# Fortunately, it didn't add another pair of newlines.
block = xblock.load_block(block_key, user)
print(repr(block.data)) # '\n<html><body>Hello</body></html>\n'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, yeah I don't think we need the newlines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bradenmacdonald Actually, it's a little worse than I thought. As long as any block fields are edited, then it always adds a pair of newlines to .data
, regardless of whether it's already wrapped in a pair.
print(repr(block.data)) # '\n<html><body>Hello</body></html>\n'
block.display_name = "blah"
block.save()
block = xblock.load_block(block_key, user)
print(repr(block.data)) # '\n\n<html><body>Hello</body></html>\n\n'
This still wouldn't break anything for end users AFAICT, but it'll make unit tests awkward, and it'll probably bother folks who edit OLX directly. I'll open a bug ticket. (EDIT: #35525)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll remove the newlines and see if any unit tests break.
This PR adds
openedx-olx-rest-api
into the core edx-platform. It provides a Studio API that any user with course authoring permission can use to get the OLX of an individual XBlock or a unit. Without this, the only way to get an XBlock's OLX was to download the tarball of the entire course.Examples of usage (be logged in to Studio on devstack):
Example output for an HTML block:
The code is designed primarily for use when importing content into Blockstore. So it will:
CDATA
sectionvertical
blocks tounit
blocks (unit
is like avertical
but has no UI elements)"static_files": {...}
JSON element for each XBlock that has at least one static file usage.export_fs
API (mostly this is video transcripts).