feat: api to restore soft-deleted component [FC-0076] #35993
Conversation
openedx-webhooks
added
the
open-source-contribution
|
Thanks for the pull request, @navinkarkera! What's next?Please work through the following steps to get your changes 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. 🔘 Let us know that your PR is ready for review:Who will review my changes?This repository is currently maintained by 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:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
navinkarkera
force-pushed
the
navin/restore-component-api
branch
2 times, most recently
from
f1186af to
270c39d
Compare
navinkarkera
changed the title
| # Set draft version back to the latest available component version id. | ||
| authoring_api.set_draft_version(component.pk, component.versioning.latest.pk) |
There was a problem hiding this comment.
We can add a helper method in openedx-learning similar to soft_delete_draft if required.
There was a problem hiding this comment.
Hi @navinkarkera!
I found a bug while testing it.
The undo works, but if the component is on a Collection, it will not show after the undo on the collection view.
It's an index only problem. The "Collections" field doesn't exists on the restored block document.
navinkarkera
force-pushed
the
navin/restore-component-api
branch
3 times, most recently
from
1386b78 to
aa0d4d8
Compare
There was a problem hiding this comment.
LGTM 👍
Thank you for your work, @navinkarkera!
- I tested this using the instructions from feat: undo component delete [FC-0076] frontend-app-authoring#1556
- I read through the code
- I checked for accessibility issues
- Includes documentation
| @@ -268,7 +267,6 @@ def test_html_library_block(self): | |||
| doc = {} | |||
| doc.update(searchable_doc_for_library_block(self.library_block)) | |||
| doc.update(searchable_doc_tags(self.library_block.usage_key)) | |||
| doc.update(searchable_doc_collections(self.library_block.usage_key)) | |||
There was a problem hiding this comment.
@navinkarkera what is the reason for deleting this?
There was a problem hiding this comment.
Not needed any more.
| @@ -398,6 +401,8 @@ def searchable_doc_for_library_block(xblock_metadata: lib_api.LibraryXBlockMetad | |||
|
|
|||
| # Add the breadcrumbs. In v2 libraries, the library itself is not a "parent" of the XBlocks so we add it here: | |||
| doc[Fields.breadcrumbs] = [{"display_name": library_name}] | |||
| # Add collections data to index if this block is part of any collection | |||
| doc.update(_collections_for_content_object(xblock_metadata.usage_key)) | |||
There was a problem hiding this comment.
@navinkarkera I think you need to call the CONTENT_OBJECT_ASSOCIATIONS_CHANGED event to index the block's collections in restore_library_block instead of indexing collections every time the library is indexed.
There was a problem hiding this comment.
@ChrisChV That is indeed better, and this led to discovery of another bug, the tags are deleted even from the database on soft deleting a component. I am currently investigating the reason and possible fix.
Update: It is intentionally deleted here.
Update 2: Removed the handler to delete tags on soft delete, everything seems to work fine now. The deleted block tags are not shown in tags filter in library page, so that is good. Let me know if you can think of any issue if don't delete tags from database.
cc: @rpenido
There was a problem hiding this comment.
Let me know if you can think of any issue if don't delete tags from database.
I think it's okay to not delete the tags for now 👍
navinkarkera
force-pushed
the
navin/restore-component-api
branch
from
aa0d4d8 to
29eed07
Compare
This helps us to restore the tags on restoring the component
navinkarkera
force-pushed
the
navin/restore-component-api
branch
from
7e8218b to
df581f7
Compare
navinkarkera
force-pushed
the
navin/restore-component-api
branch
from
df581f7 to
30d335e
Compare
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
1 similar comment
|
2U Release Notice: This PR has been deployed to the edX production environment. |
Adds API to handle restoring soft-deleted library blocks.
Description
Adds api to handle restoring soft-deleted library blocks.
Useful information to include:
Supporting information
Private-ref: FAL-3986Testing instructions
Please see openedx/frontend-app-authoring#1556 for instructions.
Deadline
"None" if there's no rush, or provide a specific date or event (and reason) if there is one.