Skip to content

Fix: Ensure folder contents are restored on undo (Fixes #249263) #249273

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

BH4HPA
Copy link

@BH4HPA BH4HPA commented May 19, 2025

Hi team,

This PR addresses issue #249263, where undoing a folder deletion operation did not correctly restore the contents of the deleted folder.

Problem:

Previously, when a folder delete operation was undone, only the folder itself might be recreated, but its child files and subfolders, along with their content, were not being restored. This led to data loss and an incomplete undo state.

Solution:

The DeleteOperation in bulkFileEdits.ts has been modified to ensure a comprehensive restoration of folder contents during an undo. The key changes are:

  1. Recursive Child Collection for Undo: When a folder is deleted, the _collectChildrenForUndo method is now properly invoked to recursively gather all child items (both files and subfolders) within the deleted folder.
  2. Content Preservation for Files: For each file collected, its content is read and stored (respecting the maxSize option to avoid performance issues with very large files). This content is then used when creating the CreateEdit objects for the undo operation.
  3. Correct CreateEdit Generation: CreateEdit instances are now generated for each child file (with its content) and each subfolder, ensuring that the entire directory structure and file data are restored when the delete operation is undone.
  4. Type Compatibility: Adjusted type hints in _collectChildrenForUndo (e.g., IFileStatWithMetadata[] to IFileStat[] for children ) to align with actual resolved types from this._fileService.resolve() when resolveMetadata is true, ensuring smoother operation and preventing potential type errors.
  5. Atomic Deletes: The atomic property in IFileDeleteOptions was explicitly set to false as it was a newly added mandatory property. While not directly related to the undo logic, this ensures the delete operation itself conforms to the latest interface requirements.
    These changes ensure that undoing a folder deletion now accurately restores the folder and all its original contents, providing a more reliable and intuitive user experience.

How to Verify:

  1. Create a folder (e.g., testFolder ).
  2. Inside testFolder , create a few files (e.g., file1.txt with some content, file2.txt with some content).
  3. Create a subfolder inside testFolder (e.g., subFolder ).
  4. Inside subFolder , create another file (e.g., file3.txt with some content).
  5. Delete testFolder .
  6. Perform an "Undo" operation (e.g., Ctrl+Z or Cmd+Z).
  7. Expected Result: testFolder should be restored, along with file1.txt , file2.txt , subFolder , and file3.txt inside subFolder . The content of all files should also be restored.
  8. Verify that if a file's size exceeds the maxSize option (if configured for the delete operation), it is restored without its content, and a warning is logged, as per existing behavior.

Thanks!

@BH4HPA
Copy link
Author

BH4HPA commented May 19, 2025

@microsoft-github-policy-service agree

Previously, when deleting a folder via the explorer and then performing an undo operation (Cmd/Ctrl+Z), only the folder name was restored (i.e., an empty folder was created), and all files and subfolders previously inside were lost.
This behavior was misleading and could lead to accidental data loss.

This commit modifies the `DeleteOperation` to ensure that when preparing the undo operation for a folder deletion, it recursively collects detailed information for all children (files and subfolders) of the deleted folder, including their paths and content (within size limits).
When the undo is performed, this information is used to fully reconstruct the folder structure and all its contents.

Fixes microsoft#249263
@lramos15 lramos15 requested a review from bpasero May 28, 2025 19:06
@bpasero bpasero requested a review from jrieken May 28, 2025 19:33
@bpasero bpasero assigned bpasero and unassigned lramos15 May 28, 2025
@bpasero
Copy link
Member

bpasero commented May 28, 2025

I can take this, I think the original bulk edit code is from @jrieken

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants