Skip to content

fix: restore parent folders before their children from the trash - #4076

Merged
JammingBen merged 1 commit into
mainfrom
fix/restore-parent-folder-in-batch
Oct 30, 2026
Merged

JammingBen merged 1 commit into
mainfrom
fix/restore-parent-folder-in-batch

Conversation

@JammingBen

@JammingBen JammingBen commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

Description

Restoring a trashed folder together with items from inside it (e.g. "select all" in the trash bin) could create an empty folder for the child first. The restore of the actual folder then replaced it, sending the child back to the trash.

  • Items are restored level by level, so a folder is back before its children.
  • A folder that is restored in the same batch is no longer created as a missing parent.
  • Restores no longer overwrite by default. Only items the user chose "Replace" for in the conflict dialog overwrite an existing item. Anything else fails if a same-named item exists, instead of silently replacing it. Such a failure currently only shows the generic "Failed to restore" message.
  • Items with the same path within one batch (e.g. a file deleted twice) no longer show up in the conflict dialog. Each one follows the choice made for the first item at that path: it is restored as a copy (file (1).txt) if that item is restored, and stays in the trash if it is skipped.
  • In a locked vault a copy would get a cleartext name, so such duplicates are not restored and a "Duplicate name in an encrypted folder" error is shown.

How to test

Parent folders:

  • A folder contains keep.md and a.md. Delete a.md, then the folder.
  • In Deleted files select both and Restore.

Duplicates in one batch:

  • Create and delete file.txt twice, then create file.txt once more.
  • In Deleted files select both file.txt and Restore, choose "Skip": nothing is restored.
  • Repeat with "Replace": the existing file is replaced, the other one is restored as file (1).txt.
  • Delete file.txt so none exists, restore both: file.txt and file (1).txt, no dialog.

Limitations

This is a can of worms, more known issues are filed in #4077.

@JammingBen JammingBen self-assigned this Oct 30, 2026
}

// restore level by level so parent folders in the batch exist before their children
const depths = [...new Set(resources.map(({ path }) => getDepth(path)))].sort((a, b) => a - b)

@kulmann kulmann Oct 30, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL: there is Map.groupBy 🙈 with that you can do:

const resourcesByDepth = Map.groupBy(resources, ({ path }) => getDepth(path))
const sortedDepths = [...resourcesByDepth.keys()].sort((a, b) => a - b)
for (const depth of sortedDepths) {
  await Promise.allSettled(
    resourcesByDepth.get(depth).map((r) => queue.add(() => restoreResource(r)))
  )
}

which saves you filtering "again and again" (well... to be fair, that's a bit exaggerated) per depth value.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My agent warned me though that not every Browser we officially support also supports this, since it's a somewhat new ES2024 feature.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoopsi. ok 😓

@JammingBen
JammingBen merged commit 906338a into main Oct 30, 2026
28 checks passed
@JammingBen
JammingBen deleted the fix/restore-parent-folder-in-batch branch October 30, 2026 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants