Repository navigation
fix: only trash files PodNotes downloaded - #374
Merged
Merged
Conversation
Removing a file the user played with "Play with PodNotes" from the Local Files playlist must keep that file in the vault. Both tests fail on master: the removal trashes the user's own file.
"Remove file" on a Local Files entry added by "Play with PodNotes" moved the user's own recording to the trash, because removal treated every offline entry as a PodNotes download. Removal now deletes the file only for PodNotes downloads. An entry in the "local file" bucket is dropped from Local Files and its file stays in the vault, which also covers local files already stored in existing data.json files. The menu item for those entries reads "Remove from Local Files". Fixes #354
Deploying podnotes with
|
| Latest commit: |
eaf9ba8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://51671071.podnotes.pages.dev |
| Branch Preview URL: | https://fix-never-trash-user-files.podnotes.pages.dev |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| await obsidian.waitFor( | ||
| () => | ||
| obsidian.dev.evalJson<boolean>( | ||
| `(() => { const target = ${findTarget}; return target instanceof HTMLElement && target.getBoundingClientRect().width > 0; })()`, |
| ), | ||
| { ...WAIT_OPTS, message: `${selector} is not visible in the PodNotes view.` }, | ||
| ); | ||
| await evalJsonAsync<boolean>(obsidian, `(() => { ${findTarget}.click(); return true; })()`); |
Releases 2.17.1 to 2.19.2 registered any file already at a download path as a feed download. Removing such an entry must ask before trashing the file, a fresh download must record that PodNotes made it, and reusing an older registration must not claim it. Local Files entries, including legacy ones without a media type, are never trashed.
A finished download now records downloadedByPodNotes on its entry. Removing an entry with that flag trashes the file as before. Removing a feed entry without it, such as a file an older release registered because it already sat at the download path, opens a prompt that names the path. "Keep file" is the default and removes only the entry; "Move to trash" trashes the file. The decision reads the removed stored entry, not the menu's episode object, so old snapshots without a path or media type cannot change it. Reusing an earlier registration keeps its flag as it was, and the codec drops any stored value other than true. Local Files entries are never trashed.
Obsidian 1.13 renders a modal's close control as .modal-header-button, so the Modal mock and the unit test use that class. The e2e test now closes its own prompt when it fails partway, so a later run cannot read it.
The Setting row drew a divider and an empty label column above the two buttons. A modal-button-container matches Obsidian's own confirm dialogs.
An unconfirmed entry whose file the vault index does not resolve must be dropped without a prompt and without touching the disk, and a confirmed removal must trash the indexed file, never unlink a path. Also cover the owned direction of download reuse and name the two owned-removal tests by how their entry came to be.
Removing a download PodNotes cannot confirm it made now resolves the file through the vault index first. A missing or unindexed file drops the entry without a prompt, and a confirmed removal trashes the resolved file through FileManager instead of falling back to a permanent adapter delete. A download blocked by an existing file now says what was checked. The registered owner lookup returns the entry it found, and addEpisode takes download ownership as an option.
deleteFileWrittenByPodNotes falls back to a permanent adapter delete for paths the vault index misses, so only partial downloads and owned downloads go through it. Drop comments that restated names or explained that choice.
Both reuse sites now go through reuseOwnRegistration, which re-registers this episode's own entry with its recorded ownership and reports whether the path was free, reused, or held by another file. The fast path ignores a foreign occupant and the resolved destination treats it as a collision, as before. registeredOwnerOfPath becomes ownRegistrationAt. The unindexed-file removal tests now use a dot-folder, which Obsidian does not index, and the e2e removal test covers that case through the real Local Files menu.
| () => | ||
| evalJsonAsync<boolean>( | ||
| obsidian, | ||
| `(async () => Boolean(document.querySelector(".modal-container")) || !(await ${original}))()`, |
| } | ||
|
|
||
| async function clickModalButton(obsidian: ObsidianClient, label: string): Promise<void> { | ||
| const button = `Array.from(document.querySelectorAll(".modal-container button")).find((button) => button.textContent?.trim() === ${JSON.stringify(label)})`; |
| ...WAIT_OPTS, | ||
| message: `Modal button "${label}" did not appear.`, | ||
| }); | ||
| await evalJsonAsync<boolean>(obsidian, `(() => { ${button}.click(); return true; })()`); |
chhoumann
changed the title
Obsidian uses Delete for FileManager.trashFile, which follows the user's Deleted files setting, so the label stays true when that setting deletes permanently.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PodNotes now trashes a file only when it knows it downloaded that file itself.
downloadedByPodNotes: trueon its entry. "Remove file" trashes it as before.PodNotes never infers ownership from a feed name, a path, or an enclosure URL. Re-registering an existing download keeps whatever ownership it already had, so an old entry never becomes "owned".
Fixes #354
Scope
docs/docs/local_files.mdgains two sentences about the prompt.Verification
.trash/. On this branch:Recordings/Interview.wavasks first. Enter (Keep file) leaves the file. "Delete file" on another entry trashes it.downloadedByPodNotes: true, keeps it when downloaded again, and "Remove file" trashes it with no prompt.npm run test:e2epasses 41 of 41. The new tests fail on the old code: the local-file test because the file lands in.trash, and the historical-entry test because no prompt appears. Unit mutations fail the suite each: protecting only local entries with a media type, dropping the ownership check in either direction, not recording ownership, upgrading or downgrading ownership on reuse, and trashing through the permanent-delete fallback.npm run lint,format:check,typecheck,build,test, anddocs:buildpass.Release impact
data.jsondownload entries gain an optionaldownloadedByPodNotes: true. There is no migration, and an entry without the field stays valid. After upgrading, removing a download made by an earlier version asks before trashing it.Note
Fix removal to only trash files PodNotes downloaded
removeDownloadedEpisodein src/downloadEpisode.ts leaves local-file entries untouched, trashes confirmed PodNotes downloads without prompting, and asks a keep/delete confirmation modal (src/ui/ConfirmTrashUnownedFileModal.ts) for unconfirmed indexed feed files. Missing or unindexed entries are only removed from the store.trueownership marker and warns on other values (src/persistence/collectionCodecs.ts).downloadedEpisodes.removeEpisodenow returns the full stored entry instead of just a path, and foreign files at a resolved download destination now fail with an error naming the conflicting path.Macroscope summarized eaf9ba8.