-
-
Notifications
You must be signed in to change notification settings - Fork 204
[REWORK] Implement Recycle Bin #4223
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
base: master
Are you sure you want to change the base?
Conversation
…tions Control Panel
Co-authored-by: Steve Piercy <[email protected]>
…n RecycleBin class
…ng exceptions to propagate
…e for improved performance
Co-authored-by: David Glick <[email protected]>
… option from registry
…rovide meaningful error messages
…n RecycleBinItemView
…ition from RecycleBinView and RecycleBinItemView
…mView and removed list for retrieval of child elements
…leBinView (_check_parent_exists)
… bin views and templates
I have added fixes for the suggested changes! You may continue reviewing! |
…nd related templates
| return False | ||
|
|
||
| try: | ||
| # Purge any nested children first if this is a folder |
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.
Is this the same thing that _cleanup_child_references does?
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.
It is more or less the same thing, this is used when deleting an item permanently... I could've refactored it into a single func...
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.
And similarly, I don't think we need or want to do this.
…r better module context
…rage and remove unneccesary checks
…age by removing unnecessary checks
…undant checks for collections
…y for better localization support and optimize addition of workflow entries
…y raising KeyError for missing items
…isting_object method by removing _v_restoring_from_recyclebin check
… storage and recycle bin
|
@davisagli, I have made the next set of changes with some clarifications reqd from u... check #4223 (comment) |
| child_path = child_data.get("path") | ||
| child_orig_id = child_data.get("id") | ||
|
|
||
| for storage_id, storage_data in list(self.storage.get_items()): |
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.
@rohnsha0 Okay, I took another look. Actually I misunderstood. The children are only stored under their parent in ["children"], and not separately stored at the top level of the recycle bin. I think that is good.
I also think it means we don't need to do this process to clean up items with the same path as a child. If I delete an item /a that has a child /a/b, and then I create a new item /a/b and delete it, it should exist as a separate entry in the recycle bin. Restoring or purging the first item that was deleted should not affect the second one. It should remain in the recycle bin, and could be restored to a different path.
| return False | ||
|
|
||
| try: | ||
| # Purge any nested children first if this is a folder |
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.
And similarly, I don't think we need or want to do this.
| # Get items sorted by date (oldest first) and calculate total size | ||
| for item_id, data in self.storage.get_items_sorted_by_date(reverse=False): | ||
| size = data.get("size", 0) | ||
| total_size += size |
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.
We should also include the size of any children, right?
ref #2966
to be merged together:-
replaces #4168