mirror of
https://github.com/nimbold/Firelink.git
synced 2026-07-26 12:08:27 +00:00
fix(downloads): harden queue controls and resumable replacement
Prevent queue action clicks from triggering row double-clicks, preserve aria2/yt-dlp resumable sidecars during duplicate replacement, and make aria2 require resume instead of silently restarting. Add a persisted, accessible Folders collapse control with reduced-motion animation and regression coverage for replacement sidecar handling. Fixes #11 Fixes #12 Closes #13 Refs #14
This commit is contained in:
+30
-2
@@ -3475,8 +3475,10 @@ async fn remove_download(
|
||||
state: tauri::State<'_, AppState>,
|
||||
id: String,
|
||||
delete_assets: bool,
|
||||
preserve_resumable: Option<bool>,
|
||||
) -> Result<(), String> {
|
||||
log::info!("remove_download called for id: {}", id);
|
||||
let preserve_resumable = preserve_resumable.unwrap_or(false);
|
||||
let primary_path = crate::download_ownership::primary_path_for_id(&app_handle, &id)?;
|
||||
|
||||
let active_kind = state.queue_manager.active_kind(&id).await;
|
||||
@@ -3535,8 +3537,13 @@ async fn remove_download(
|
||||
crate::ipc::DownloadStateEvent::new(id.clone(), crate::ipc::DownloadStatus::Paused),
|
||||
);
|
||||
|
||||
let preserve_assets = preserve_resumable
|
||||
&& primary_path
|
||||
.as_deref()
|
||||
.is_some_and(has_resumable_download_assets);
|
||||
|
||||
let cleanup_result = async {
|
||||
if delete_assets {
|
||||
if delete_assets && !preserve_assets {
|
||||
if let Some(path) = primary_path.as_deref() {
|
||||
remove_download_assets(path, &app_handle).await?;
|
||||
}
|
||||
@@ -3550,6 +3557,14 @@ async fn remove_download(
|
||||
cleanup_result
|
||||
}
|
||||
|
||||
fn has_resumable_download_assets(primary: &std::path::Path) -> bool {
|
||||
[".aria2", ".part", ".ytdl"].iter().any(|suffix| {
|
||||
let mut candidate = primary.as_os_str().to_os_string();
|
||||
candidate.push(suffix);
|
||||
std::path::PathBuf::from(candidate).exists()
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) async fn remove_download_assets<R: tauri::Runtime>(
|
||||
primary: &std::path::Path,
|
||||
app_handle: &tauri::AppHandle<R>,
|
||||
@@ -4802,7 +4817,8 @@ mod tests {
|
||||
normalize_speed_limit_for_aria2,
|
||||
parse_firelink_deep_link, parse_ffmpeg_version, parse_media_progress_line,
|
||||
redact_log_line, redact_log_line_for_output, sanitize_ytdlp_config_value,
|
||||
should_cleanup_media_artifacts_after_failure, FirelinkDeepLink, MediaProgress,
|
||||
has_resumable_download_assets, should_cleanup_media_artifacts_after_failure,
|
||||
FirelinkDeepLink, MediaProgress,
|
||||
MediaSpeedSampler, MEDIA_PROGRESS_PREFIX,
|
||||
};
|
||||
use serde_json::json;
|
||||
@@ -4825,6 +4841,18 @@ mod tests {
|
||||
assert_eq!(template, destination.join("clip.mp4"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn recognizes_resume_sidecars_without_treating_the_primary_file_as_partial() {
|
||||
let directory = tempfile::tempdir().unwrap();
|
||||
let primary = directory.path().join("download.bin");
|
||||
std::fs::write(&primary, b"partial").unwrap();
|
||||
|
||||
assert!(!has_resumable_download_assets(&primary));
|
||||
|
||||
std::fs::write(directory.path().join("download.bin.aria2"), b"control").unwrap();
|
||||
assert!(has_resumable_download_assets(&primary));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ytdlp_progress_args_force_progress_in_quiet_print_mode() {
|
||||
let args = media_progress_args();
|
||||
|
||||
@@ -1248,6 +1248,8 @@ impl SidecarSpawner for ProductionSpawner {
|
||||
options.insert("max-tries".to_string(), serde_json::json!(mt.to_string()));
|
||||
options.insert("retry-wait".to_string(), serde_json::json!("2"));
|
||||
options.insert("continue".to_string(), serde_json::json!("true"));
|
||||
options.insert("always-resume".to_string(), serde_json::json!("true"));
|
||||
options.insert("auto-file-renaming".to_string(), serde_json::json!("false"));
|
||||
if let Some(speed) = payload
|
||||
.speed_limit
|
||||
.as_deref()
|
||||
|
||||
@@ -519,8 +519,7 @@ export const AddDownloadsModal = () => {
|
||||
itemLocation,
|
||||
finalFile,
|
||||
platform.os
|
||||
) &&
|
||||
download.status !== 'failed'
|
||||
)
|
||||
) {
|
||||
fileExistsInStore = true;
|
||||
break;
|
||||
@@ -662,8 +661,7 @@ export const AddDownloadsModal = () => {
|
||||
itemLocation,
|
||||
finalFile,
|
||||
platform.os
|
||||
) &&
|
||||
download.status !== 'failed'
|
||||
)
|
||||
) {
|
||||
existingItem = download;
|
||||
break;
|
||||
@@ -677,7 +675,11 @@ export const AddDownloadsModal = () => {
|
||||
if (!existingItem) {
|
||||
throw new Error(`Cannot replace ${finalFile}: file is not owned by a Firelink download.`);
|
||||
}
|
||||
await store.removeDownload(existingItem.id, true);
|
||||
// Let the backend decide whether resumable sidecars still
|
||||
// exist after stopping the old transfer. This avoids a race
|
||||
// where a paused item finishes while the replacement is
|
||||
// being prepared.
|
||||
await store.removeDownload(existingItem.id, true, existingItem.status !== 'completed');
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -177,6 +177,7 @@ export const DownloadItem = React.memo<DownloadItemProps>(({
|
||||
|
||||
<div
|
||||
className="hidden group-hover:flex items-center justify-end gap-0.5 w-full ml-auto"
|
||||
onClick={(e) => e.stopPropagation()}
|
||||
onDoubleClick={(e) => e.stopPropagation()}
|
||||
>
|
||||
{(download.status === 'queued' || download.status === 'staged') && queueIndex !== -1 && (
|
||||
|
||||
@@ -3,6 +3,7 @@ import {
|
||||
Inbox, Zap, CheckCircle2, CircleDashed,
|
||||
Film, Music, FileText, Box, Image as ImageIcon, Archive, FileQuestion,
|
||||
List, CalendarClock, Gauge, Bug, Settings, Plus, Play, Pause, Edit2, Trash2, PanelLeft,
|
||||
ChevronDown,
|
||||
type LucideIcon
|
||||
} from 'lucide-react';
|
||||
import { useDownloadStore, DownloadCategory, Queue } from '../store/useDownloadStore';
|
||||
@@ -28,6 +29,11 @@ export const Sidebar: React.FC<SidebarProps> = (props) => {
|
||||
const [renamingQueueId, setRenamingQueueId] = useState<string | null>(null);
|
||||
const [editingQueueName, setEditingQueueName] = useState('');
|
||||
const [contextMenu, setContextMenu] = useState<{ x: number; y: number; id: string } | null>(null);
|
||||
const [foldersCollapsed, setFoldersCollapsed] = useState(() =>
|
||||
window.localStorage.getItem('firelink-folders-collapsed') === 'true'
|
||||
);
|
||||
const foldersToggleRef = useRef<HTMLButtonElement>(null);
|
||||
const foldersListRef = useRef<HTMLDivElement>(null);
|
||||
|
||||
const addInputRef = useRef<HTMLInputElement>(null);
|
||||
const renameInputRef = useRef<HTMLInputElement>(null);
|
||||
@@ -53,6 +59,16 @@ export const Sidebar: React.FC<SidebarProps> = (props) => {
|
||||
if (renamingQueueId) renameInputRef.current?.focus();
|
||||
}, [renamingQueueId]);
|
||||
|
||||
useEffect(() => {
|
||||
window.localStorage.setItem('firelink-folders-collapsed', String(foldersCollapsed));
|
||||
}, [foldersCollapsed]);
|
||||
|
||||
useEffect(() => {
|
||||
if (foldersCollapsed && foldersListRef.current?.contains(document.activeElement)) {
|
||||
foldersToggleRef.current?.focus();
|
||||
}
|
||||
}, [foldersCollapsed]);
|
||||
|
||||
useEffect(() => {
|
||||
const handleKeyDown = (e: KeyboardEvent) => {
|
||||
if (document.querySelector('.app-modal-backdrop') || document.querySelector('.app-modal')) return;
|
||||
@@ -214,14 +230,38 @@ export const Sidebar: React.FC<SidebarProps> = (props) => {
|
||||
</section>
|
||||
|
||||
<section className="sidebar-section">
|
||||
<div className="sidebar-section-label">Folders</div>
|
||||
<NavItem icon={Music} label="Musics" filter="Musics" />
|
||||
<NavItem icon={Film} label="Movies" filter="Movies" />
|
||||
<NavItem icon={Archive} label="Compressed" filter="Compressed" />
|
||||
<NavItem icon={FileText} label="Documents" filter="Documents" />
|
||||
<NavItem icon={ImageIcon} label="Pictures" filter="Pictures" />
|
||||
<NavItem icon={Box} label="Applications" filter="Applications" />
|
||||
<NavItem icon={FileQuestion} label="Other" filter="Other" />
|
||||
<button
|
||||
type="button"
|
||||
ref={foldersToggleRef}
|
||||
className="sidebar-section-label sidebar-section-label-toggle"
|
||||
aria-expanded={!foldersCollapsed}
|
||||
aria-controls="sidebar-folders-list"
|
||||
onClick={() => setFoldersCollapsed(collapsed => !collapsed)}
|
||||
>
|
||||
<span>Folders</span>
|
||||
<ChevronDown
|
||||
aria-hidden="true"
|
||||
size={13}
|
||||
className={`sidebar-section-chevron ${foldersCollapsed ? 'is-collapsed' : ''}`}
|
||||
/>
|
||||
</button>
|
||||
<div
|
||||
ref={foldersListRef}
|
||||
id="sidebar-folders-list"
|
||||
className={`sidebar-collapse-grid ${foldersCollapsed ? 'is-collapsed' : ''}`}
|
||||
aria-hidden={foldersCollapsed}
|
||||
inert={foldersCollapsed}
|
||||
>
|
||||
<div className="sidebar-collapse-content">
|
||||
<NavItem icon={Music} label="Musics" filter="Musics" />
|
||||
<NavItem icon={Film} label="Movies" filter="Movies" />
|
||||
<NavItem icon={Archive} label="Compressed" filter="Compressed" />
|
||||
<NavItem icon={FileText} label="Documents" filter="Documents" />
|
||||
<NavItem icon={ImageIcon} label="Pictures" filter="Pictures" />
|
||||
<NavItem icon={Box} label="Applications" filter="Applications" />
|
||||
<NavItem icon={FileQuestion} label="Other" filter="Other" />
|
||||
</div>
|
||||
</div>
|
||||
</section>
|
||||
|
||||
<section className="sidebar-section">
|
||||
|
||||
@@ -1143,6 +1143,75 @@ html[data-list-density="relaxed"] {
|
||||
color: hsl(var(--text-muted));
|
||||
}
|
||||
|
||||
.sidebar-section-label-toggle {
|
||||
width: 100%;
|
||||
display: flex;
|
||||
align-items: center;
|
||||
justify-content: space-between;
|
||||
border: 0;
|
||||
background: transparent;
|
||||
color: hsl(var(--text-muted));
|
||||
cursor: default;
|
||||
transition: color 140ms ease;
|
||||
}
|
||||
|
||||
.sidebar-section-label-toggle:hover {
|
||||
color: hsl(var(--text-secondary));
|
||||
}
|
||||
|
||||
.sidebar-section-chevron {
|
||||
flex-shrink: 0;
|
||||
opacity: 0.8;
|
||||
transform: rotate(0deg);
|
||||
transition: transform 240ms cubic-bezier(0.22, 1, 0.36, 1), opacity 140ms ease;
|
||||
}
|
||||
|
||||
.sidebar-section-chevron.is-collapsed {
|
||||
transform: rotate(-90deg);
|
||||
opacity: 0.58;
|
||||
}
|
||||
|
||||
.sidebar-collapse-grid {
|
||||
display: grid;
|
||||
grid-template-rows: 1fr;
|
||||
transition: grid-template-rows 240ms cubic-bezier(0.22, 1, 0.36, 1);
|
||||
}
|
||||
|
||||
.sidebar-collapse-grid.is-collapsed {
|
||||
grid-template-rows: 0fr;
|
||||
}
|
||||
|
||||
.sidebar-collapse-content {
|
||||
min-height: 0;
|
||||
overflow: hidden;
|
||||
opacity: 1;
|
||||
transform: translateY(0);
|
||||
transform-origin: top;
|
||||
transition:
|
||||
opacity 150ms ease,
|
||||
transform 240ms cubic-bezier(0.22, 1, 0.36, 1),
|
||||
visibility 0s linear 0s;
|
||||
}
|
||||
|
||||
.sidebar-collapse-grid.is-collapsed .sidebar-collapse-content {
|
||||
opacity: 0;
|
||||
transform: translateY(-6px);
|
||||
pointer-events: none;
|
||||
visibility: hidden;
|
||||
transition:
|
||||
opacity 120ms ease,
|
||||
transform 240ms cubic-bezier(0.22, 1, 0.36, 1),
|
||||
visibility 0s linear 120ms;
|
||||
}
|
||||
|
||||
@media (prefers-reduced-motion: reduce) {
|
||||
.sidebar-section-chevron,
|
||||
.sidebar-collapse-grid,
|
||||
.sidebar-collapse-content {
|
||||
transition: none;
|
||||
}
|
||||
}
|
||||
|
||||
.sidebar-section {
|
||||
margin-bottom: 14px;
|
||||
}
|
||||
|
||||
+1
-1
@@ -32,7 +32,7 @@ type CommandMap = {
|
||||
open_downloaded_file: { args: { path: string }; result: void };
|
||||
pause_download: { args: { id: string }; result: void };
|
||||
resume_download: { args: { id: string }; result: boolean };
|
||||
remove_download: { args: { id: string; deleteAssets: boolean }; result: void };
|
||||
remove_download: { args: { id: string; deleteAssets: boolean; preserveResumable?: boolean }; result: void };
|
||||
detach_download_for_reconfigure: { args: { id: string }; result: void };
|
||||
update_dock_badge: { args: { count: number }; result: void };
|
||||
get_platform_info: { args: undefined; result: PlatformInfo };
|
||||
|
||||
@@ -681,6 +681,23 @@ describe('useDownloadStore', () => {
|
||||
.toEqual(['active']);
|
||||
});
|
||||
|
||||
it('asks the backend to preserve resumable assets during replacement removal', async () => {
|
||||
useDownloadStore.setState({
|
||||
downloads: [
|
||||
{ id: 'paused', url: 'https://example.com/file', fileName: 'file', status: 'paused', category: 'Other', dateAdded: '' }
|
||||
] as any[]
|
||||
});
|
||||
vi.mocked(ipc.invokeCommand).mockResolvedValue(undefined as never);
|
||||
|
||||
await useDownloadStore.getState().removeDownload('paused', true, true);
|
||||
|
||||
expect(ipc.invokeCommand).toHaveBeenCalledWith('remove_download', {
|
||||
id: 'paused',
|
||||
deleteAssets: true,
|
||||
preserveResumable: true
|
||||
});
|
||||
});
|
||||
|
||||
it('starts staged queue items in their persisted queue order', async () => {
|
||||
useDownloadStore.setState({
|
||||
downloads: [
|
||||
|
||||
@@ -356,7 +356,7 @@ interface DownloadState {
|
||||
setSelectedPropertiesDownloadId: (id: string | null) => void;
|
||||
addDownload: (item: DownloadDraft, action: AddDownloadAction) => Promise<boolean>;
|
||||
updateDownload: (id: string, updates: Partial<DownloadItem>) => void;
|
||||
removeDownload: (id: string, deleteFile?: boolean) => Promise<void>;
|
||||
removeDownload: (id: string, deleteFile?: boolean, preserveResumable?: boolean) => Promise<void>;
|
||||
pauseDownload: (id: string) => Promise<void>;
|
||||
redownload: (id: string) => Promise<void>;
|
||||
resumeDownload: (id: string) => Promise<boolean>;
|
||||
@@ -670,12 +670,16 @@ export const useDownloadStore = create<DownloadState>((set, get) => ({
|
||||
syncSystemIntegrations();
|
||||
}
|
||||
},
|
||||
removeDownload: async (id, deleteFile = false) => {
|
||||
removeDownload: async (id, deleteFile = false, preserveResumable = false) => {
|
||||
await invalidateDispatch(id);
|
||||
const item = get().downloads.find(d => d.id === id);
|
||||
|
||||
if (item) {
|
||||
await invoke('remove_download', { id, deleteAssets: deleteFile });
|
||||
await invoke('remove_download', {
|
||||
id,
|
||||
deleteAssets: deleteFile,
|
||||
preserveResumable
|
||||
});
|
||||
}
|
||||
|
||||
set((state) => ({
|
||||
|
||||
Reference in New Issue
Block a user