feat: add library restore endpoint#37439
Conversation
|
Thanks for the pull request, @wgu-taylor-payne! This repository is currently maintained by 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 approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo 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:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere 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:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
4170bdf to
e196adb
Compare
|
Just wanted to flag that #37499 might introduce some minor conflicts with this PR. |
9272ef8 to
a3aedf5
Compare
rodmgwgu
left a comment
There was a problem hiding this comment.
Looking good, just some small comments
ca4c169 to
d8e073c
Compare
rodmgwgu
left a comment
There was a problem hiding this comment.
Thanks for addressing the comments, code looks good. I haven't been able to test it yet as I'm facing some issues with my local env.
ormsbee
left a comment
There was a problem hiding this comment.
Some minor requests. I haven't reviewed the test code yet, but I wanted to submit this review anyway, in case you were still working today. (I need to make dinner for the family, and can't be back on for a while.)
| if provided_learning_package := learning_package is not None: | ||
| assert isinstance(learning_package, LearningPackage) |
There was a problem hiding this comment.
Is mypy not doing this validation properly? I wouldn't normally think that this would be necessary to have.
| # Make sure the LearningPackage key is in the correct format | ||
| authoring_api.update_learning_package( | ||
| learning_package.id, | ||
| key=f'lib:{org.short_name}:{slug}', |
There was a problem hiding this comment.
This is also str(ref.library_key), right?
| if not provided_learning_package: | ||
| learning_package = authoring_api.create_learning_package( | ||
| key=str(ref.library_key), | ||
| title=title, | ||
| description=description, | ||
| ) | ||
| assert learning_package is not None | ||
| ref.learning_package = learning_package | ||
| ref.save() | ||
|
|
||
| if provided_learning_package: | ||
| # Make sure the LearningPackage key is in the correct format | ||
| authoring_api.update_learning_package( | ||
| learning_package.id, |
There was a problem hiding this comment.
Nit: This whole block seems correct to me, but it seems a little more complex than it has to be? As opposed to something like:
if learning_package:
# A temporary LearningPackage was passed in, so change its key to match our library.
authoring_api.update_learning_package(learning_package.id, key=str(ref.library_key))
else:
# We have to generate a new LearningPackage for this library.
learning_package = authoring_api.create_learning_package(
key=str(ref.library_key),
title=title,
description=description,
)
ref.learning_package = learning_package
ref.save() (This is not a merge-blocker.)
There was a problem hiding this comment.
Just wondering — in the case where learning_package exists, don’t we need to pass the title? I noticed the frontend is requesting that field.
There was a problem hiding this comment.
along with title, should we also update the description?
There was a problem hiding this comment.
I didn’t see any description field in the design, but I think it could be helpful to include it just in case. What do you think, @ormsbee ?
There was a problem hiding this comment.
Yes to title--I had forgotten that we stored a that in the LearningPackage and not the ContentLibrary.
The description is something that used to be passed in and editable in an earlier iteration of the design, but got dropped at some point. I think it's fine to add it to the update_learning_package() call.
| # Make sure the LearningPackage key is in the correct format | ||
| authoring_api.update_learning_package( | ||
| learning_package.id, | ||
| key=f'lib:{org.short_name}:{slug}', |
There was a problem hiding this comment.
I noticed that when provided_learning_package is not provided, it uses str(self.key), which I believe is the correct way to create the key since it uses LibraryLocatorV2.
| if not provided_learning_package: | ||
| learning_package = authoring_api.create_learning_package( | ||
| key=str(ref.library_key), | ||
| title=title, | ||
| description=description, | ||
| ) | ||
| assert learning_package is not None | ||
| ref.learning_package = learning_package | ||
| ref.save() | ||
|
|
||
| if provided_learning_package: | ||
| # Make sure the LearningPackage key is in the correct format | ||
| authoring_api.update_learning_package( | ||
| learning_package.id, |
There was a problem hiding this comment.
Just wondering — in the case where learning_package exists, don’t we need to pass the title? I noticed the frontend is requesting that field.
Adds a library restore endpoint to restore a learning package from a backup zip archive (/api/libraries/v2/restore/). The learning package can then be used to create a content library.
Description
Adds a library restore endpoint to restore a learning package from a backup. The learning package can then be used to create a content library.
POST /api/libraries/v2/restore/
Requires a
filefield, which must be a ZIP file.A library restore task will be queued for execution, and a task id will be returned in the JSON response.
{ "task_id": "29cd29cb-a963-427b-8aaa-675c999832a1" }GET /api/libraries/v2/restore/
Requires a
task_idparameter to be provided. If given a validtask_idit will return the task status and data based on the result of the restore.Success:
{ "state": "Succeeded", "result": { "learning_package_id": 1, "title": "Demo Learning Package", "org": "CL-TEST", "slug": "LIB_C001", "key": "lp-restore:Admin:CL-TEST:LIB_C001:1761076028573", "archive_key": "lib:CL-TEST:LIB_C001", "containers": 0, "components": 0, "collections": 0, "sections": 0, "subsections": 0, "units": 0, "created_on_server": "cms.test", "created_at": "2025-10-05T18:23:45.180Z", "created_by": { "username": "test_author", "email": "[email protected]" } }, "error": null, "error_log": null }Failure:
{ "state": "Failed", "result": null, "error": "Library restore failed. See error log for details.", "error_log": "http://testserver/uploads/user_tasks/2025/10/21/2cff2a1c-7fa9-4097-b8ca-6b1b80126475-error.log" }Pending (All other non-final states, like "In-Progress", will have the same result - only state is provided)
{ "state": "Pending", "result": null, "error": null, "error_log": null }Supporting information
Resolves openedx/openedx-core#388.
Testing instructions
Deadline
Ulmo release