Allow custom folder for downloaded attachments - #68
Open
birdbox219 wants to merge 2 commits into
Open
birdbox219 wants to merge 2 commits into
birdbox219 wants to merge 2 commits into
Conversation
Add an option in Settings under Files to select a custom directory for downloaded attachments. - Add custom_media_dir setting serialized to settings.json. - Support custom attachment path resolution in AppDirs::media_dir, falling back to the default cache directory. - Update backend worker to direct downloads and outgoing media to the configured media directory. - Add Change, Reset, and Open folder buttons in Settings UI. - Relocate matching media files in archive.db when the folder changes. - Preserve custom user folders on logout by only clearing media_cache_dir.
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved findings affect attachment relocation, cache cleanup, UI blocking, and in-memory path consistency.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds persistent custom attachment storage with folder selection, reset, and live path updates.
Changes:
- Persists and resolves a configurable media directory.
- Routes downloads and outgoing media through it.
- Adds Settings controls and backend path updates.
File summaries
| File | Description |
|---|---|
src/ui/settings.rs |
Adds attachment folder controls. |
src/settings.rs |
Persists the custom directory. |
src/paths.rs |
Resolves the effective media directory. |
src/model.rs |
Adds media directory actions. |
src/main.rs |
Initializes the configured directory. |
src/backend/worker.rs |
Handles relocation, cleanup, and media routing. |
src/backend.rs |
Adds media directory commands and events. |
src/app.rs |
Applies changes and synchronizes loaded media paths. |
Review details
Suppressed comments (4)
src/app.rs:1226
- This handler performs up to two blocking metadata checks for every loaded message on the UI thread. A custom directory may be slow, and this duplicates the worker's
relocate_mediascan. Have the worker return the repointed message paths and let the UI apply those results instead of probing the filesystem here.
for conversation in self.conversations.values_mut() {
for message in &mut conversation.messages {
let Some(media) = message.content.media_mut() else {
continue;
};
src/app.rs:1237
relocate_mediaclears this row from the archive when neither the old path nor a file with the same name in the new directory exists, but this branch leaves the loaded message'smedia.pathunchanged. An open conversation can therefore keep rendering the attachment as downloaded and try to open a nonexistent file until the chat is reloaded. Clear the in-memory path when no candidate is found so it stays consistent with the archive.
if !path.exists() {
let candidate = dir.join(name);
if candidate.exists() {
media.path = Some(candidate);
}
src/app.rs:2428
- The advertised
--demo --demo-page settingsverification cannot exercise these actions: demo startup usesApp::headless, whose detached backend drops commands, and no demo responder handlesPickMediaDirorResetMediaDir. The Change and Reset flows therefore do nothing in that mode; verify them with a real backend or add a demo response path.
Action::PickMediaDir => self.backend.send(Command::PickMediaDir),
Action::ResetMediaDir => self.backend.send(Command::ResetMediaDir),
src/backend/worker.rs:2589
- Changing the setting does not actually relocate existing attachments.
relocate_mediaskips every row whose recorded old path still exists (src/backend/worker.rs:526-527), so selecting a new folder leaves the file and archive row in the old directory; Reset has the symmetric problem. Move existing files into the new directory and update their archive paths as part of the change so the configured folder applies to existing downloads.
self.relocate_media();
- Files reviewed: 8/8 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Reject custom attachment paths inside the cache directory and treat selecting the default cache folder as a reset. - Make logout media cache cleanup skip configured custom subtrees. - Remove blocking directory creation from UI click handler. - Add test fixture asserting in-memory media path repair when the directory changes. - Update README to document custom media directories and preserved folders on logout.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Media relocation, nested-directory cleanup, and opening an uncreated media directory remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (2)
Resolved since last review (4)
This accepts a folder inside the cache media tree, but logout still recursively removes…mediacan now be an arbitrary user-selected path, but this click handler still calls synchronous… This introduces a user-visible storage exception, but README.md still says attachments always live… The added test only checksdirsandsettings; it never creates a loaded media message or…
| /// repairs moved attachment paths or clears missing files for redownload. | ||
| fn relocate_media(&mut self) { | ||
| let dir = self.dirs.media_cache_dir(); | ||
| let dir = self.dirs.media_dir(); |
Comment on lines
200
to
201
| if theme::soft_button(ui, &palette, Some(Icon::ExternalLink), "Open folder", false).clicked() { | ||
| let _ = std::fs::create_dir_all(&media); | ||
| app.actions.push(Action::OpenFile(media.clone())); |
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
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.



Add an option in Settings under Files to select a custom directory for downloaded attachments.
Testing:
Unit tests for settings defaults and round trip (
settings::tests::custom_media_dir_defaults_to_none,settings::tests::round_trips_through_disk).Unit test for path fallback and custom override (
paths::tests::media_dir_uses_custom_path_when_configured).Unit test for live directory update and in-memory message path repair (
app::tests::media_dir_changed_event_updates_settings_and_dirs).Manual verification with
--demo --demo-page settingstesting folder selection, path display, open folder,and reset.Checks run on Windows: fmt, clippy (default and all features), tests (default and all features), and cargo doc, all clean. Other platforms compile through the same code paths with no new platform-specific code.