Backport: fix: handle paste of library content blocks correctly

[bradenmacdonald]

This backports #33926 to Quince - please see that PR's description. This is a bugfix PR.

The commit/code is mostly the same but I removed:

• the breaking changes to the create_library() API - this backport doesn't move the library_type parameter to the end of the parameter list nor make it optional like the master PR did.
• The changes to the content_tagging test_views were excluded as the library part of that code isn't present on Quince.
• I had to skip the test for this change, because the test depends on v2 libraries which don't work properly in quince. However, the fix should apply to both v1 and v2 so is still useful to backport.

Private ref: MNG-4161

[openedx-webhooks]

Thanks for the pull request, @bradenmacdonald! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

• supporting documentation
• Open edX discussion forum threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

[bradenmacdonald]

@kdmccormick I've run into a snag here:

• The goal of this backport is to fix the issue that:

  if you copy and then paste a Library Content Block, it will appear to work fine. But when you later "sync" the block with the latest changes from the library, all of the child blocks will be deleted and re-created with new IDs. This could result in a loss of learner state (answers) for any child blocks of the pasted Library Content block.

• For Redwood, we fixed this by calling new_xblock.sync_from_library(upgrade_to_latest=False) during paste.
• But in Quince, sync_from_library() doesn't exist. We only have refresh_children/update_children, which won't preserve the current version, but rather will update from the library's latest version. This means that when you paste, you could get a newer version of the library content than what you copied.
• Also, the test for this fix was written using v2 libraries, and doesn't work in Quince where the library content block only can import from v1.

I think we have some options:

1. Backport/implement sync_from_library() in Quince. I don't think this would be worth the effort.
2. Simply throw an error if people try to paste a Library Content Block. Won't be nice or fix the issue, but will prevent unexpected loss of learner state.
3. If people try to paste a Library Content Block, during the paste, we can exclude its children from being pasted and set the "version" to 0, to force users to click "update from library" to see the children.
4. Leave it as is.

Thoughts?

[kdmccormick]

@bradenmacdonald The equivalent of sync_from_library(update_children=False) in Quince was to call update_children and pass in the current source library version, e.g.: https://github.com/openedx/edx-platform/blob/open-release/quince.master/xmodule/modulestore/xml_importer.py#L903-L906. I think you could do that.

If that doesn't work, I'm in favor of option (3) -- it makes the limitation immediately obvious to the author rather than hiding it where it'll bite them down the road.

[bradenmacdonald]

@kdmccormick I think this is ready for review, though I haven't been able to manually test it. The new test case is passing though.

[bradenmacdonald]

@kdmccormick Is it still worth getting this merged? If so, do you have time to review or suggest a reviewer?

[kdmccormick]

Yup, I think it's worth it still.

Code looks great. I'd like to test it out. I'm provisioning a PR sandbox for it now (fingers crossed...).

[open-craft-grove]

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

[open-craft-grove]

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

[kdmccormick]

I was able to confirm this on the sandbox (username/password = openedx/openedx):

• We have this library, with one problem, that was originally "v1" and is now "v2"
• In this unit, there is an RCB-Original, which was created when the library was at "v1".
• I copied RCB-Original and pasted it into RCB-Copy-at-v1.
• I updated the library to "v2".
• I copied RCB-Original again and pasted it into RCB-Copy-at-v2, and confirmed that it originally contained "v2" library content.
• I published the unit and answered each problem.
• Finally, I updated RCB-Copy-at-v2 so that it contains v2 library content, and published.
• I confirmed that my student state of RCB-Copy-at-v2 is still intact.

Thanks for seeing this through @bradenmacdonald . Merge away 🚀

[openedx-webhooks]

@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.