Support pasting units from course into libraries by bradenmacdonald · Pull Request #36516 · openedx/openedx-platform

bradenmacdonald · 2025-04-11T23:16:46Z

Description

This PR provides the backend API updates requires so that users can paste a unit from a course into a library.

It also fixes the pasting code so that the block_id (used in the URL) is now generated from the title of the copied thing, which creates much nicer usage keys / URLs while still avoiding conflicts.

It also cleans up the serializers and makes them use more common code.

It also fixes the following bugs 🐛:

Sometimes when copying a block with static assets, the CMS console would show an error traceback: fix: IntegrityError: "Column 'md5_hash' cannot be null"
Unsupported XBlocks like Vertical, Problem Builder, Split Test, etc. (i.e. blocks that have children) could be pasted into libraries if you requested it with the API, then all kinds of errors would show when trying to view them or index them.
Remove deprecated def_key from the REST API response

Supporting information

Relates to openedx/frontend-app-authoring#1647 though this is technically the opposite of that. Pasting into a course proved too complicated to implement for now because it requires converting a container to an XBlock.

Testing instructions

Test using openedx/frontend-app-authoring#1812

Deadline

ASAP

Other information

Private ref: FAL-4067

openedx-webhooks · 2025-04-11T23:16:51Z

Thanks for the pull request, @bradenmacdonald!

This repository is currently maintained by @openedx/wg-maintenance-edx-platform.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

rpenido

LGTM 👍
Thank you for your work, @bradenmacdonald!

I tested this using the instructions from openedx/frontend-app-authoring#1812
I read through the code
I checked for accessibility issues
Includes documentation

ormsbee

Looks good! Just some minor questions/requests. Thank you!

ormsbee · 2025-04-15T16:14:54Z

-    XBlock.load_class(block_type)  # Will raise an exception if invalid
+    block_class = XBlock.load_class(block_type)  # Will raise an exception if invalid
+    if block_class.has_children:
+        raise IncompatibleTypesError("XBlocks with children are not supported in content libraries")


Please add the block type and block_id in the error being raised, so that it's easier to pinpoint the problematic content.

ormsbee · 2025-04-15T16:27:16Z

+        olx_node = etree.fromstring(olx_str)
+        title = olx_node.attrib.get("display_name")
+        # Slugify the title and append some random numbers to make a unique slug
+        block_id = slugify(title, allow_unicode=True) + '-' + uuid4().hex[-6:]


This is fine, but in the longer term, do you think it's worthwhile to add this concept to the Learning Core layer (find me some key that starts with this and doesn't collide)? At that layer, we could use things like the primary key to disambiguate them more succinctly.

This is fine, but in the longer term, do you think it's worthwhile to add this concept to the Learning Core layer (find me some key that starts with this and doesn't collide)?

Yes, that would be a nice improvement!

ormsbee · 2025-04-15T16:30:13Z

+    # Generate a block_id:
+    try:
+        olx_node = etree.fromstring(olx_str)
+        title = olx_node.attrib.get("display_name")


If this is pulling directly from the OLX, is it going to end up having a lot of blank values for the display name? Do we need to special case those properly so they say things like "problem_{number}"? Or does the OLX already have that encoded into it at this point?

Good point, I think we should be more defensive here and not assume what the OLX may or may not have for display_name. I've updated it to fall back on the localized block name if there's no display name, which I was already doing for units anyways.

ormsbee · 2025-04-15T16:39:34Z

+    """
+    from openedx.core.djangoapps.content_staging import api as content_staging_api
+    if not content_staging_api:
+        raise RuntimeError("The required content_staging app is not installed")


Is it even possible to not install the content_staging app at this point? Is the concern that this could be called from the LMS instead of Studio?

Technically it's an app plugin, but in practice it's always installed in the CMS unless the install is corrupted. I'll remove these warnings. We do have to continue importing it within these functions though, unless/until we resolve the fact that the content_libraries API is sometimes imported into the LMS.

ormsbee · 2025-04-15T16:41:09Z



-class LibraryXBlockMetadataSerializer(serializers.Serializer):
+class PublishableItemSerializer(serializers.Serializer):


ChrisChV

Looks good! Could you fix the conflicts to merge this?

bradenmacdonald · 2025-04-15T21:20:44Z

Rebased.

edx-pipeline-bot · 2025-04-16T15:07:26Z

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

edx-pipeline-bot · 2025-04-16T15:26:52Z

2U Release Notice: This PR has been deployed to the edX production environment.

edx-pipeline-bot · 2025-04-16T15:26:52Z

2U Release Notice: This PR has been deployed to the edX production environment.

* fix: don't allow pasting xblocks with children into libraries * fix: IntegrityError: "Column 'md5_hash' cannot be null" * feat: allow pasting a unit from a course into a library * feat: auto-generate a nice block_id when pasting into a library * test: add test for pasting unit from course into library * fix: better handle potentially missing display_names during paste * chore: clarifications and import cleanups

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Apr 11, 2025

openedx-webhooks added this to Contributions Apr 11, 2025

github-project-automation Bot moved this to Needs Triage in Contributions Apr 11, 2025

bradenmacdonald mentioned this pull request Apr 11, 2025

feat: allow pasting units from a course into a library openedx/frontend-app-authoring#1812

Merged

bradenmacdonald force-pushed the braden/paste-into-libraries branch 8 times, most recently from 8403d38 to b922d04 Compare April 12, 2025 02:05

rpenido approved these changes Apr 14, 2025

View reviewed changes

mphilbrick211 moved this from Needs Triage to In Eng Review in Contributions Apr 14, 2025

ormsbee requested changes Apr 15, 2025

View reviewed changes

ChrisChV approved these changes Apr 15, 2025

View reviewed changes

bradenmacdonald added 6 commits April 15, 2025 13:52

fix: don't allow pasting xblocks with children into libraries

3f1e9ee

fix: IntegrityError: "Column 'md5_hash' cannot be null"

dec9705

feat: allow pasting a unit from a course into a library

c509881

feat: auto-generate a nice block_id when pasting into a library

7475963

test: add test for pasting unit from course into library

1957816

fix: better handle potentially missing display_names during paste

b46b55a

bradenmacdonald force-pushed the braden/paste-into-libraries branch from c45ed3e to 7ddc572 Compare April 15, 2025 21:06

chore: clarifications and import cleanups

844a4b5

bradenmacdonald force-pushed the braden/paste-into-libraries branch from 7ddc572 to 844a4b5 Compare April 15, 2025 21:15

ormsbee approved these changes Apr 15, 2025

View reviewed changes

bradenmacdonald merged commit 336fb01 into openedx:master Apr 15, 2025

bradenmacdonald deleted the braden/paste-into-libraries branch April 15, 2025 22:25

github-project-automation Bot moved this from In Eng Review to Done in Contributions Apr 15, 2025

ormsbee mentioned this pull request Apr 23, 2025

Remove py2neo dependency #36586

Merged

kingoftech-v01 mentioned this pull request Apr 10, 2026

fix: withdrawn by author #38323

Closed



		class LibraryXBlockMetadataSerializer(serializers.Serializer):
		class PublishableItemSerializer(serializers.Serializer):

Uh oh!

Conversation

bradenmacdonald commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Supporting information

Testing instructions

Deadline

Other information

Uh oh!

openedx-webhooks commented Apr 11, 2025

Uh oh!

rpenido left a comment

Choose a reason for hiding this comment

Uh oh!

ormsbee left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChrisChV left a comment

Choose a reason for hiding this comment

Uh oh!

bradenmacdonald commented Apr 15, 2025

Uh oh!

edx-pipeline-bot commented Apr 16, 2025

Uh oh!

edx-pipeline-bot commented Apr 16, 2025

Uh oh!

edx-pipeline-bot commented Apr 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

bradenmacdonald commented Apr 11, 2025 •

edited

Loading