From 0ce0ba32f59947cbc38fe177665c2d371ef757b0 Mon Sep 17 00:00:00 2001 From: hjoncour Date: Tue, 5 May 2026 18:55:21 -0400 Subject: [PATCH 1/2] wip --- dl_setting.md | 434 +++++++++++++++++++++++++++++ fix.md | 287 +++++++++++++++++++ src-tauri/src/commands/files.rs | 33 +-- src-tauri/src/commands/import.rs | 8 +- src-tauri/src/commands/library.rs | 27 +- src-tauri/src/database.rs | 48 ++-- src-tauri/src/download/manager.rs | 40 ++- src-tauri/src/download/pipeline.rs | 17 +- src-tauri/src/settings.rs | 4 + src/pages/settings.rs | 36 +++ 10 files changed, 866 insertions(+), 68 deletions(-) create mode 100644 dl_setting.md create mode 100644 fix.md diff --git a/dl_setting.md b/dl_setting.md new file mode 100644 index 0000000..89eb2f8 --- /dev/null +++ b/dl_setting.md @@ -0,0 +1,434 @@ +# Settings & Download Issues — Investigation & Plan + +--- + +## Issue 1: Parallel Downloads resets to 3 + +### What was verified + +- `settings.json` (`~/Library/Application Support/clip-downloader/settings.json`): file is + written correctly by `save_settings` — confirmed manually, value 5 was on disk. +- `settings.rs :: save_settings()` writes all fields including `parallel_downloads`. +- `settings.rs :: load_settings()` reads all fields; `serde_json::from_str` with + `unwrap_or_default()`. If deserialization fails for any reason, it silently falls back + to `Settings::default()` (pd: 3) **and writes that default back to disk**, overwriting + the saved value. No failure observed in practice, but the pattern is fragile. +- `settings_cmd.rs :: save_settings` command: persists to JSON, then sends `RefreshSettings` + to the download manager (which rereads the file and updates `max_parallel`). Backend path: no bug. + +### Root cause: Yew state-timing bug + +`src/pages/settings.rs` — the number input used `onchange` (fires on blur): + +In Yew 0.21, `UseStateHandle` stores an `Rc` populated at render time. +`handle.set(new_value)` queues a re-render but does **not** update the `Rc` held by +existing callbacks. Every closure created in render N reads the render-N value. + +When the user types "2" and clicks Save, the browser fires within one event-loop turn: + +1. `mousedown` on Save → blur on input +2. `change` → `on_parallel_downloads_change` fires → `settings.set(pd: 2)` + — re-render scheduled, not yet executed +3. `click` → `on_save` fires → `(*settings).clone()` reads the render-N handle → **old value** +4. `save_settings(old value)` sent to backend + +The re-render happens after the save. The typed value is never actually saved. +This is why it "always resets to 3" — the initial default (3) gets re-saved on every attempt. + +Spinbox arrows (▲/▼) work because each click fires `change` and Yew re-renders +before the next click, replacing `on_save` with a closure that sees the new state. + +### Fix applied + +Changed `onchange` → `oninput` with `web_sys::InputEvent`. Every keystroke updates +state and triggers a re-render, so by the time the user clicks Save, `on_save` belongs +to the latest render. + +**Status: FIXED** (committed in the previous session). + +--- + +## Issue 2: Instagram p/ posts — "gallery-dl failed (IG fallback)" + +### What the screenshot shows + +Every issue item is an Instagram `p/` URL (photo or carousel post) with error: +``` +gallery-dl failed (IG fallback) tmp=/var/folders/93/.../tmpXXXX +``` + +The tmp path and raw verbose gallery-dl output are being stored verbatim as the +`last_error` for the row. + +### Pipeline trace (`download/pipeline.rs`) + +For any Instagram URL where `is_ig_post_p = true` (contains `/p/`): +1. yt-dlp is tried first with the browser's cookies. +2. If yt-dlp returns `false` or errors → gallery-dl is run as an image fallback. +3. If gallery-dl also fails → the error message is: + ```rust + format!("gallery-dl failed (IG fallback) tmp={}\n{}", tmp_dir.display(), output) + ``` + This includes the full tmp path + entire verbose gallery-dl output. + +### Identified causes + +**A. yt-dlp always fails on Instagram photo posts.** + yt-dlp is a video downloader. For Instagram `p/` carousel/image posts it returns + a non-zero exit code (no video formats available). The fallback to gallery-dl is + intentional — but gallery-dl is also failing. + +**B. gallery-dl cookie argument format mismatch (high probability).** + The `cookie_arg` built in `utils/os.rs` is formatted as `browser:Profile` (e.g., + `brave:Default`, `chrome:Profile 1`). yt-dlp explicitly supports this + `browser:profile` syntax. Gallery-dl's `--cookies-from-browser` flag may not + accept the profile suffix on all versions, resulting in an "unknown browser" + or cookie-read error that is not caught by `friendly_browser_error`. + +**C. `friendly_browser_error` only catches two patterns.** + It checks for `"find-generic-password failed"` and `"cannot decrypt v10 cookies"`. + Any other gallery-dl failure (wrong browser format, login required, rate limit, + extractor error) falls through and the raw verbose output is stored as the error. + +**D. The error message shown in the UI is unusable.** + It includes an internal tmp path (`/var/folders/...`) and a wall of gallery-dl + `--verbose` output. Neither is actionable for the user. + +### Plan + +1. **Strip the tmp path from the error message.** Users never need to see it. + Change the format string to just show a clean summary. + +2. **Fix gallery-dl's cookie_arg.** When invoking gallery-dl, strip any `:profile` + suffix from `cookie_arg` unless the gallery-dl version is known to support it, + OR pass only the browser name (e.g., `brave`, `chrome`) and let gallery-dl use + its default profile. Alternatively, use a `--cookies` file export approach. + +3. **Improve `friendly_browser_error` to catch more gallery-dl patterns** such as + "Login required", "HTTP Error 401", "No results", etc., to give actionable messages. + +4. **Remove `--verbose` from gallery-dl invocation** (or keep verbose only for debug + mode). The verbose output floods the error string with irrelevant internal details. + +5. **Consider passing only the gallery-dl-compatible browser name** (strip profile): + - `brave:Default` → `brave` + - `chrome:Profile 1` → `chrome` + - `firefox:/path/to/profile` → `firefox` (gallery-dl reads the default profile itself) + +--- + +## Issue 3: Auto-detect system libraries and make it the default + +### Current state + +- `use_system_binaries: bool` in `Settings` (default: `false`, meaning use Tauri sidecar). +- Setting IS persistent (written to `settings.json`). +- **The checkbox is hidden** until the user manually clicks "Check for local libraries". + It only appears when `libs` state is `Some` AND all three tools are detected: + ```rust + if let Some(stats) = (*libs).clone() { + if stats.yt_dlp && stats.gallery_dl && stats.ffmpeg { + // show checkbox + } + } + ``` + `libs` starts as `None` and is never auto-populated. +- Result: users who have system libraries installed don't see the option unless they + know to click "Check". The setting is invisible and stays `false`. + +### Screenshot 2 context + +The user checked and found all three libraries (yt-dlp ✓, gallery-dl ✓, ffmpeg ✓), +and has "Use local dependencies instead of sidecar" checked. But on a fresh app launch +they'd need to click "Check" again just to see this option. + +### Plan + +**Goal:** If system libraries are present, use them by default. Make this automatic and visible. + +#### A. Auto-detect on settings page mount + +In `settings_page()`, replace the single `use_effect_with((), ...)` with two effects: +- The existing one: calls `load_settings` and populates `settings` state. +- A new one: calls `check_sidecar_tools` automatically on mount, populates `libs` state. + +This way, the library status is known immediately when the page opens, without the user +needing to click "Check". + +#### B. Always show the `use_system_binaries` checkbox + +Remove the conditional `if let Some(stats) = (*libs).clone()` gate around the checkbox. +Show it always (grayed out or disabled when detection is in progress, enabled once done). +This makes it clear the option exists regardless of whether the user has clicked "Check". + +#### C. Default to `true` when libraries are available (first launch) + +The challenge: how to distinguish "user explicitly set to false" from "never configured". + +**Solution: add an `Option` for `use_system_binaries`** in `Settings`: +- `None` → never configured; backend resolves to auto-detect result +- `Some(true)` → explicitly on +- `Some(false)` → explicitly off + +At app start (`lib.rs :: run()`), if `use_system_binaries` is `None`, run a sync +check (or spawn a quick task) for yt-dlp/gallery-dl/ffmpeg in PATH. If all present, +resolve to `true`; save to file as `Some(true)`. + +Frontend: treat `None` as "auto" — show the effective value but indicate it was +auto-detected. Allow the user to explicitly override it. + +**Alternative (simpler, lower-fidelity):** Keep `use_system_binaries: bool` but change +`Default` to `true` only when compiling for platforms where system tools are common +(macOS/Linux). At `load_settings`, if this is the first launch (file didn't exist), run +a binary probe at startup. If tools are found, write `true`; otherwise `false`. + +**Recommended approach:** Option in Settings — it cleanly separates +"never touched" from "explicitly configured". + +#### D. Persistence (already works, but needs visibility fix) + +The `use_system_binaries` value is already included in `save_settings` and read back +by `load_settings`. No backend change needed for persistence itself. + +The only problem is that once the checkbox was never shown, the user never triggers +`on_use_system_binaries_change`, and the default (`false`) persists. Fixing visibility +(plan B above) resolves this. + +#### Summary of changes + +| Area | File | Change | +|---|---|---| +| Auto-detect on mount | `src/pages/settings.rs` | Add `use_effect_with((), ...)` that calls `check_sidecar_tools` | +| Always-visible checkbox | `src/pages/settings.rs` | Remove `if let Some(stats)` gate; show checkbox always | +| First-launch default | `src-tauri/src/settings.rs` | Change `use_system_binaries` field to `Option`; resolve at startup | +| Backend type | `src-tauri/src/database.rs` | `use_system_binaries: Option` with serde default `None` | +| Effective value | `src-tauri/src/download/video.rs`, `image.rs` | `settings.use_system_binaries.unwrap_or(false)` | +| Frontend type | `src/pages/settings.rs` | `use_system_binaries: Option` or keep `bool` and rely on auto-detect write | + +--- + +## Feature 4: Cooldown setting + +Adds a configurable delay (in seconds) before each download actually starts. Useful for +rate-limiting or respecting per-site request pacing. A value of 0 disables the feature. + +### New Settings field + +```rust +// src-tauri/src/settings.rs — Settings struct +#[serde(default)] +pub cooldown_secs: u32, // 0 = disabled +``` + +`default` resolves to `0u32` via `Default::default()`, so old settings.json files without +this field are read correctly without any migration. + +### Manager changes (`src-tauri/src/download/manager.rs`) + +Add a `cooldown_secs: u32` state variable alongside `max_parallel`: + +```rust +let mut cooldown_secs = initial_settings.cooldown_secs; +``` + +Update it in `RefreshSettings` (which currently only updates `max_parallel`): + +```rust +DownloadCommand::RefreshSettings => { + let s = settings::load_settings(); + max_parallel = s.parallel_downloads.max(1) as usize; + cooldown_secs = s.cooldown_secs; +} +``` + +Pass `cooldown_secs` into `maybe_start_next` as a new parameter. Inside the spawned +async block (before calling `run_download_with_progress`), add: + +```rust +if cooldown_secs > 0 { + tokio::time::sleep(std::time::Duration::from_secs(cooldown_secs as u64)).await; +} +``` + +This keeps the sleep inside the task, so it counts against active slots — the manager +won't overfill the slot limit during the sleep. The status is already set to +`Downloading` before the sleep, which is accurate (the slot is reserved). + +### Frontend changes (`src/pages/settings.rs`) + +Add `cooldown_secs: u32` to the `Settings` struct with `#[serde(default)]`. + +Add a handler using `oninput` (same pattern as `parallel_downloads`): + +```rust +let on_cooldown_change = { + let settings = settings.clone(); + Callback::from(move |e: web_sys::InputEvent| { + let value = e.target_unchecked_into::().value_as_number() as u32; + let mut s = (*settings).clone(); + s.cooldown_secs = value; + settings.set(s); + }) +}; +``` + +UI element (place after the parallel downloads row): + +```html +
+ + +
+``` + +### Summary of changes + +| File | Change | +|---|---| +| `src-tauri/src/settings.rs` | Add `cooldown_secs: u32` field with `#[serde(default)]` | +| `src-tauri/src/download/manager.rs` | Add `cooldown_secs` state var; update in `RefreshSettings`; pass to `maybe_start_next`; sleep before `run_download_with_progress` | +| `src/pages/settings.rs` | Add `cooldown_secs: u32` to `Settings`; add `on_cooldown_change` handler; add number input | + +--- + +## Feature 5: Retry failed downloads at end of queue + +When the queue drains and no tasks are active, automatically re-enqueue all items that +have `status='error'`. Prevents the user from having to manually click retry on each one. + +### New Settings field + +```rust +// src-tauri/src/settings.rs — Settings struct +#[serde(default)] +pub retry_on_queue_empty: bool, // false = disabled +``` + +### New DB function (`src-tauri/src/database.rs`) + +```rust +pub fn list_error_ids_conn(conn: &Connection) -> Result> { + let mut stmt = conn.prepare( + "SELECT id FROM downloads WHERE status='error' ORDER BY id", + )?; + let rows = stmt.query_map([], |row| row.get(0))?; + let mut out = Vec::new(); + for row in rows { + out.push(row?); + } + Ok(out) +} +``` + +Export it from `database.rs` and add it to the `use crate::database::` import list in +`manager.rs`. + +### Manager changes (`src-tauri/src/download/manager.rs`) + +Add two new state variables: + +```rust +let mut retry_on_queue_empty = initial_settings.retry_on_queue_empty; +let mut auto_retried: std::collections::HashSet = HashSet::new(); +``` + +Update in `RefreshSettings`: + +```rust +retry_on_queue_empty = s.retry_on_queue_empty; +``` + +Clear `auto_retried` on fresh `Enqueue` (the user explicitly added new items, signaling +a new pass — retried failures should be eligible again): + +```rust +DownloadCommand::Enqueue { ids } => { + auto_retried.clear(); + enqueue_ids(...).await; +} +``` + +In `TaskFinished`, after `active.remove(&id)`, check whether a retry pass is warranted +**before** calling `maybe_start_next` (which will consume the newly-queued items): + +```rust +DownloadCommand::TaskFinished { id } => { + active.remove(&id); + if retry_on_queue_empty && !paused && queue.is_empty() && active.is_empty() { + let db_clone = db.clone(); + let error_ids = tauri::async_runtime::spawn_blocking(move || { + let conn = db_clone.blocking_lock(); + list_error_ids_conn(&*conn).unwrap_or_default() + }) + .await + .unwrap_or_default(); + + for eid in error_ids { + if !auto_retried.contains(&eid) { + auto_retried.insert(eid); + queue.push_back(eid); + } + } + } +} +``` + +Then `maybe_start_next` runs as usual after the match block and picks up the newly +enqueued retry items. + +The DB status of retried items is still `'error'` when enqueued. `maybe_start_next` +calls `set_status(..., Downloading)` which transitions them correctly; on success/failure +the final status is written as always. + +### Loop prevention + +`auto_retried` accumulates every ID that has been scheduled for auto-retry. On subsequent +`TaskFinished` events, those IDs are filtered out — so a repeatedly-failing item is only +auto-retried once per "pass". A new pass begins when the user explicitly enqueues new +items (`Enqueue` clears `auto_retried`). + +### Frontend changes (`src/pages/settings.rs`) + +Add `retry_on_queue_empty: bool` to the `Settings` struct with `#[serde(default)]`. + +Handler (standard `onchange` checkbox pattern): + +```rust +let on_retry_on_queue_empty_change = { + let settings = settings.clone(); + Callback::from(move |e: Event| { + let checked = e.target_unchecked_into::().checked(); + let mut s = (*settings).clone(); + s.retry_on_queue_empty = checked; + settings.set(s); + }) +}; +``` + +UI element (place after the cooldown row): + +```html +
+ + +
+``` + +### Edge cases + +| Case | Behaviour | +|---|---| +| Item fails again on retry | Gets status=error again; `auto_retried` blocks a second auto-retry in the same pass | +| User manually retries while auto-retry is armed | Fine — item was already in `auto_retried` or will be deduplicated by `queue.contains` check in `enqueue_ids` | +| `retry_on_queue_empty` toggled off mid-queue | `retry_on_queue_empty` is updated on next `RefreshSettings`; already-queued retry items proceed normally | +| cooldown + retry | Cooldown applies to retried items the same way — they sleep inside their spawned task | +| App restart with error items | On startup, `queue` is populated from `status='queued'` only; error items are NOT auto-enqueued until the next `TaskFinished` check triggers. This is intentional — the feature acts on queue-drain events, not on startup | + +### Summary of changes + +| File | Change | +|---|---| +| `src-tauri/src/database.rs` | Add `list_error_ids_conn` function | +| `src-tauri/src/settings.rs` | Add `retry_on_queue_empty: bool` with `#[serde(default)]` | +| `src-tauri/src/download/manager.rs` | Add `retry_on_queue_empty` + `auto_retried` state vars; update in `RefreshSettings`; clear `auto_retried` on `Enqueue`; trigger retry logic in `TaskFinished`; import `list_error_ids_conn` | +| `src/pages/settings.rs` | Add field, handler, and checkbox UI | diff --git a/fix.md b/fix.md new file mode 100644 index 0000000..3a0814b --- /dev/null +++ b/fix.md @@ -0,0 +1,287 @@ +# Downloads Page Investigation And Fix Plan + +## Scope + +This document covers the remaining Downloads page consistency bug where the page can briefly or fully empty while downloads are finishing, then repopulate after navigating away and back. It also proposes a new `Issues` section for failed items that should remain visible for later investigation. + +No application code is changed in this pass. This is a planning document only. + +## Symptoms Observed + +- The Downloads page sometimes shows only the section headers and no rows. +- The missing rows often come back after leaving the page and returning. +- The blanking tends to happen around download completion or other refresh moments. +- Failed downloads currently disappear from the active UI instead of being retained somewhere actionable. + +## Findings + +### 1. The download event listener is mutating state from a stale snapshot + +The main frontend risk is in the long-lived `download_event` listener in [src/app.rs](/Users/hjoncour/Projects/clipscraper/clip-downloader/src/app.rs:333). + +Inside that listener, the code does: + +- clone `downloads` into `map` +- mutate `map` +- write it back with `downloads.set(map)` + +The problem is that this listener is created once with `use_effect_with((), ...)`, so it can keep dereferencing an old `UseStateHandle` value. Yew documents this exact pitfall: + +- [yew-0.21.0/src/functional/hooks/use_state.rs](/Users/hjoncour/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/yew-0.21.0/src/functional/hooks/use_state.rs:61) + +That warning is directly relevant here: + +- a completion event can write back an older map +- an older map may be partially populated or empty +- a later full reload restores the correct rows, which matches the observed "leave page and come back" recovery + +### 2. Reloads replace the entire snapshot instead of merging + +`spawn_reload_downloads(...)` calls `list_downloads` and replaces the whole in-memory map with `build_download_entries(rows)` in [src/app.rs](/Users/hjoncour/Projects/clipscraper/clip-downloader/src/app.rs:119). + +That means every refresh: + +- drops current stage text +- drops current progress bytes +- drops any in-memory detail that is not already persisted in the DB + +Even when the data is valid, this full replacement makes refreshes visually harsher than necessary. + +### 3. `reconcile_downloads` is not actually awaited to completion + +The current frontend reload path does this: + +1. invoke `reconcile_downloads` +2. invoke `list_downloads` + +But `reconcile_downloads` only sends a manager command over a channel in [src-tauri/src/commands/downloader.rs](/Users/hjoncour/Projects/clipscraper/clip-downloader/src-tauri/src/commands/downloader.rs:106). That command is processed later by the manager loop in [src-tauri/src/download/manager.rs](/Users/hjoncour/Projects/clipscraper/clip-downloader/src-tauri/src/download/manager.rs:201). + +So the frontend currently assumes "reconcile then list", but in reality it is "request reconcile, then immediately list". That leaves a race where the UI can fetch a snapshot before reconciliation has finished. + +This is probably not the primary blanking bug, but it is still a consistency bug and should be fixed as part of the same cleanup. + +### 4. Unknown terminal events do not force recovery + +When the UI receives events for IDs that are not present in the in-memory map, it only schedules a refresh for unknown non-terminal status changes in [src/app.rs](/Users/hjoncour/Projects/clipscraper/clip-downloader/src/app.rs:384). + +Current gaps: + +- unknown `Progress` events do nothing +- unknown `Message` events do nothing +- unknown terminal `StatusChanged` events do not schedule a recovery refresh + +That makes it easier for the page to stay inconsistent until manual navigation triggers a fresh snapshot. + +### 5. Failed items are not retained anywhere visible + +The frontend currently drops `Done`, `Error`, and `Canceled` rows from the in-memory Downloads page map in [src/app.rs](/Users/hjoncour/Projects/clipscraper/clip-downloader/src/app.rs:69) and also removes terminal rows on event receipt in [src/app.rs](/Users/hjoncour/Projects/clipscraper/clip-downloader/src/app.rs:365). + +That means failed items vanish instead of remaining visible for follow-up. There is also no persisted `last_error` field in the `downloads` table today ([src-tauri/src/database.rs](/Users/hjoncour/Projects/clipscraper/clip-downloader/src-tauri/src/database.rs:13)), so even if the status remains `error`, the useful reason is not stored durably. + +## Proposed Fix + +### A. Move Downloads UI state to a reducer + +Replace the current `use_state(HashMap)` mutation pattern with a reducer-driven state model. + +Recommended shape: + +- `DownloadUiState` +- `entries: HashMap` +- `ready: bool` +- `refreshing: bool` +- optional `last_snapshot_seq: u64` + +Recommended actions: + +- `ReplaceSnapshot(Vec)` +- `StatusChanged { id, status }` +- `Progress { id, progress, downloaded_bytes, total_bytes }` +- `Message { id, message }` +- `SetInitialReady` +- `BeginRefresh` +- `EndRefresh` + +Why this is the right fix: + +- reducer dispatchers are stable across renders +- event listeners can dispatch actions without reading stale state +- the reducer always runs against the current state +- it removes the fragile "clone map from closure, mutate, set" pattern + +Target file: + +- [src/app.rs](/Users/hjoncour/Projects/clipscraper/clip-downloader/src/app.rs) + +### B. Merge snapshots instead of replacing volatile fields + +When a fresh DB snapshot arrives, merge it into the reducer state instead of rebuilding a brand-new map blindly. + +Rules: + +- keep DB-backed row fields authoritative +- preserve transient progress and stage text for rows that still exist and are still `downloading` +- preserve failure detail if an item is currently in `error` and its persisted error metadata matches +- remove rows only when they genuinely no longer belong on the page + +This will reduce visual churn and prevent refreshes from wiping useful download-stage context. + +### C. Make refreshes non-destructive + +The Downloads page should not clear existing rows while a background refresh is happening. + +Recommended behavior: + +- only show `Loading downloads...` on the first load +- after that, keep existing content on screen while refreshing +- if needed, show a small `Refreshing...` indicator rather than blanking the page + +This is primarily a UX rule, but it also acts as a guardrail against future state regression. + +### D. Make reconciliation and listing atomic + +The current two-step frontend sequence should be replaced with a backend command that returns a consistent post-reconcile snapshot. + +Recommended backend command: + +- `refresh_download_snapshot` + +Behavior: + +1. reconcile queue/active state +2. read the final list from the same backend flow +3. return the rows directly to the frontend + +This can be implemented either: + +- inside the manager with a reply channel, or +- in a direct DB command if reconciliation logic is moved behind a synchronous API + +Goal: + +- remove the fake "await reconcile" assumption +- guarantee that reloads always see a coherent snapshot + +Likely files: + +- [src-tauri/src/commands/downloader.rs](/Users/hjoncour/Projects/clipscraper/clip-downloader/src-tauri/src/commands/downloader.rs) +- [src-tauri/src/download/manager.rs](/Users/hjoncour/Projects/clipscraper/clip-downloader/src-tauri/src/download/manager.rs) +- [src-tauri/src/commands/list.rs](/Users/hjoncour/Projects/clipscraper/clip-downloader/src-tauri/src/commands/list.rs) + +### E. Always recover on unknown IDs + +If the event listener receives any event for an unknown ID, schedule a debounced snapshot refresh. + +That rule should apply to: + +- `StatusChanged` +- `Progress` +- `Message` + +Terminal unknown events are especially important because they currently do not repair the UI. + +### F. Add an `Issues` section for failed items + +Add a third list section to the Downloads page for items with `status = error`. + +Suggested order: + +1. Downloading +2. Queue +3. Backlog +4. Issues + +This section is for items that failed and should remain available for investigation or retry later. + +Minimum behavior: + +- show all `error` rows +- do not auto-remove them from the Downloads page +- allow retry +- allow move back to backlog +- allow delete + +Recommended row content: + +- collection title +- item label +- last failure reason +- actions: `Retry`, `Backlog`, `Delete` + +Target files: + +- [src/pages/downloads.rs](/Users/hjoncour/Projects/clipscraper/clip-downloader/src/pages/downloads.rs) +- [src/app.rs](/Users/hjoncour/Projects/clipscraper/clip-downloader/src/app.rs) + +### G. Persist failure reason in the database + +To make the `Issues` section genuinely useful, add a nullable `last_error` column to `downloads`. + +Recommended behavior: + +- set `last_error` when a download fails +- clear `last_error` when retrying +- clear `last_error` on successful completion +- expose `last_error` in the UI row model + +This lets failed items survive app restarts with enough context to be actionable. + +Likely files: + +- [src-tauri/src/database.rs](/Users/hjoncour/Projects/clipscraper/clip-downloader/src-tauri/src/database.rs) +- [src-tauri/src/download/manager.rs](/Users/hjoncour/Projects/clipscraper/clip-downloader/src-tauri/src/download/manager.rs) +- [src-tauri/src/download/pipeline.rs](/Users/hjoncour/Projects/clipscraper/clip-downloader/src-tauri/src/download/pipeline.rs) +- [src/types.rs](/Users/hjoncour/Projects/clipscraper/clip-downloader/src/types.rs) + +## Implementation Order + +### Phase 1: Stabilize state + +1. Introduce reducer-based download UI state in `src/app.rs`. +2. Convert the `download_event` listener to dispatch reducer actions only. +3. Stop removing terminal rows in the listener until the reducer has explicit rules for where they belong. +4. Ensure any unknown event type for any unknown ID schedules a debounced refresh. + +### Phase 2: Fix reload semantics + +1. Replace `reconcile_downloads + list_downloads` with one atomic refresh/snapshot command. +2. Merge snapshots instead of recreating a blank transient state. +3. Keep existing rows visible during background refreshes. + +### Phase 3: Add Issues section + +1. Extend the UI row type with `last_error`. +2. Persist `last_error` in SQLite with a migration. +3. Render `Issues` in the Downloads page. +4. Add actions for retry, move to backlog, and delete. + +## Validation Plan + +### Core regression checks + +- Open Downloads with backlog, queue, and active rows already present. +- Stay on Downloads while several active items complete. +- Confirm the page never blanks and never requires navigation to repopulate. +- Confirm queue/backlog counts update in place. + +### Event ordering checks + +- Trigger multiple rapid completions. +- Trigger progress and message updates during a background refresh. +- Confirm stale or out-of-order refreshes do not wipe the visible list. + +### Failure handling checks + +- Force a known download failure. +- Confirm the row lands in `Issues` instead of disappearing. +- Confirm the failure reason remains visible after restart. +- Confirm `Retry` clears the old error and requeues the row. + +## Expected Outcome + +After this fix: + +- the Downloads page should no longer blank during finishes or refreshes +- page state should remain stable without requiring navigation +- failed items should remain visible under `Issues` +- the UI should provide enough context to investigate failures later diff --git a/src-tauri/src/commands/files.rs b/src-tauri/src/commands/files.rs index b3fed4b..be896a8 100644 --- a/src-tauri/src/commands/files.rs +++ b/src-tauri/src/commands/files.rs @@ -23,15 +23,12 @@ pub async fn pick_csv_and_read(app: tauri::AppHandle) -> Result FilePath::Path(path_buf) => { let csv_text = std_fs::read_to_string(path_buf).map_err(|e| e.to_string())?; - match super::import::import_csv_text(csv_text.clone()).await { - Ok(n) => { - println!("[BACKEND] [files] imported {n} rows (picker)"); - let _ = app.emit("import_completed", n); - } - Err(e) => { - eprintln!("[BACKEND] [files] import failed: {e}"); - return Err(e); - } + let result = super::import::import_csv_text(csv_text.clone()).await; + let n = result.as_ref().copied().unwrap_or(0); + println!("[BACKEND] [files] imported {n} rows (picker)"); + let _ = app.emit("import_completed", n); + if let Err(e) = result { + eprintln!("[BACKEND] [files] import partially failed: {e}"); } Ok(csv_text) @@ -50,18 +47,12 @@ pub async fn read_csv_from_path(app: tauri::AppHandle, path: String) -> Result { - println!( - "[BACKEND] [files] imported {n} rows (drag-drop) from {}", - path - ); - let _ = app.emit("import_completed", n); - } - Err(e) => { - eprintln!("[BACKEND] [files] import failed for {path}: {e}"); - return Err(e); - } + let result = super::import::import_csv_text(csv_text.clone()).await; + let n = result.as_ref().copied().unwrap_or(0); + println!("[BACKEND] [files] imported {n} rows (drag-drop) from {path}"); + let _ = app.emit("import_completed", n); + if let Err(e) = result { + eprintln!("[BACKEND] [files] import partially failed for {path}: {e}"); } Ok(csv_text) } diff --git a/src-tauri/src/commands/import.rs b/src-tauri/src/commands/import.rs index 15445d2..ae9e992 100644 --- a/src-tauri/src/commands/import.rs +++ b/src-tauri/src/commands/import.rs @@ -37,7 +37,13 @@ pub async fn import_csv_text(csv_text: String) -> Result { // Process each row for rec in rdr.records() { - let rec = rec.map_err(|e| e.to_string())?; + let rec = match rec { + Ok(r) => r, + Err(e) => { + eprintln!("[import] skipping malformed CSV row: {e}"); + continue; + } + }; // Extract CSV columns (Platform,Type,Handle,Media,link) let platform_s = rec.get(0).unwrap_or("").to_string(); diff --git a/src-tauri/src/commands/library.rs b/src-tauri/src/commands/library.rs index b74fd43..c3f2d1e 100644 --- a/src-tauri/src/commands/library.rs +++ b/src-tauri/src/commands/library.rs @@ -64,11 +64,9 @@ fn path_exists_ok(path: &str) -> bool { #[tauri::command] pub async fn open_file_for_link(link: String) -> Result<(), String> { let db = crate::database::Database::new().map_err(|e| e.to_string())?; - let some = db.find_done_row_by_link(&link).map_err(|e| e.to_string())?; - - let (_id, path) = some.ok_or_else(|| "no library item found for link".to_string())?; + let Some((_id, path)) = db.find_done_row_by_link(&link).map_err(|e| e.to_string())? else { return Ok(()); }; if !path_exists_ok(&path) { - return Err(format!("file not found: {path}")); + return Ok(()); } open_with_default_app(&path) } @@ -76,13 +74,16 @@ pub async fn open_file_for_link(link: String) -> Result<(), String> { #[tauri::command] pub async fn open_folder_for_link(link: String) -> Result<(), String> { let db = crate::database::Database::new().map_err(|e| e.to_string())?; - let some = db.find_done_row_by_link(&link).map_err(|e| e.to_string())?; - - let (_id, path) = some.ok_or_else(|| "no library item found for link".to_string())?; - if !PathBuf::from(&path).exists() { - return Err(format!("path not found: {path}")); - } - open_folder(&path) + let Some((_id, path)) = db.find_done_row_by_link(&link).map_err(|e| e.to_string())? else { return Ok(()); }; + let p = PathBuf::from(&path); + let dir = if p.exists() { + path.clone() + } else if let Some(parent) = p.parent().filter(|d| d.exists()) { + parent.to_string_lossy().to_string() + } else { + return Ok(()); + }; + open_folder(&dir) } #[tauri::command] @@ -91,7 +92,7 @@ pub async fn open_platform_folder(platform: String) -> Result<(), String> { let base = std::path::PathBuf::from(s.download_directory); let p = base.join(platform); if !p.exists() { - return Err(format!("path not found: {}", p.display())); + return Ok(()); } open_folder(&p.to_string_lossy()) } @@ -107,7 +108,7 @@ pub async fn open_collection_folder( let label = crate::database::Database::collection_folder_label(&content_type, &handle); let p = base.join(platform).join(label); if !p.exists() { - return Err(format!("path not found: {}", p.display())); + return Ok(()); } open_folder(&p.to_string_lossy()) } diff --git a/src-tauri/src/database.rs b/src-tauri/src/database.rs index e743788..6cdb09f 100644 --- a/src-tauri/src/database.rs +++ b/src-tauri/src/database.rs @@ -170,6 +170,16 @@ pub fn list_downloading_ids_conn(conn: &Connection) -> Result> { Ok(out) } +pub fn list_error_ids_conn(conn: &Connection) -> Result> { + let mut stmt = conn.prepare("SELECT id FROM downloads WHERE status='error' ORDER BY id")?; + let rows = stmt.query_map([], |row| row.get(0))?; + let mut out = Vec::new(); + for row in rows { + out.push(row?); + } + Ok(out) +} + pub fn mark_id_done_conn(conn: &Connection, id: i64, path: &str) -> Result { let path_value = if path.is_empty() { "unknown_path".to_string() @@ -505,6 +515,12 @@ pub struct Settings { /// Prefer system-installed tools (yt-dlp / ffmpeg / gallery-dl) over bundled sidecars #[serde(default)] pub use_system_binaries: bool, + /// Seconds to wait before starting each download (0 = disabled) + #[serde(default)] + pub cooldown_secs: u32, + /// Auto-retry error items when the queue drains and no tasks are active + #[serde(default)] + pub retry_on_queue_empty: bool, } fn default_true() -> bool { @@ -804,26 +820,22 @@ impl Database { mark_id_done_conn(&self.conn, id, path) } - pub fn link_exists_in_collection( - &self, - link: &str, - platform: &str, - user_handle: &str, - origin: &str, - ) -> Result { + pub fn link_exists_in_collection(&self, link: &str, platform: &str, user_handle: &str, origin: &str) -> Result { + let norm = normalize_link(link.to_string()); let mut stmt = self.conn.prepare( - "SELECT 1 FROM downloads - WHERE link = ?1 - AND platform = ?2 COLLATE NOCASE - AND (user_handle = ?3 COLLATE NOCASE OR (?3 = 'Unknown' AND (user_handle = '' OR user_handle IS NULL))) - AND origin = ?4 COLLATE NOCASE - LIMIT 1", + "SELECT link FROM downloads + WHERE platform = ?1 COLLATE NOCASE + AND (user_handle = ?2 COLLATE NOCASE OR (?2 = 'Unknown' AND (user_handle = '' OR user_handle IS NULL))) + AND origin = ?3 COLLATE NOCASE", )?; - let exists = stmt - .query([link, platform, user_handle, origin])? - .next()? - .is_some(); - Ok(exists) + let mut rows = stmt.query([platform, user_handle, origin])?; + while let Some(row) = rows.next()? { + let db_link: String = row.get(0)?; + if normalize_link(db_link) == norm { + return Ok(true); + } + } + Ok(false) } pub fn find_id_by_link(&self, link: &str) -> Result> { diff --git a/src-tauri/src/download/manager.rs b/src-tauri/src/download/manager.rs index 7c69ada..7c08fd6 100644 --- a/src-tauri/src/download/manager.rs +++ b/src-tauri/src/download/manager.rs @@ -1,4 +1,4 @@ -use std::collections::{HashMap, VecDeque}; +use std::collections::{HashMap, HashSet, VecDeque}; use std::fmt; use std::sync::Arc; @@ -7,9 +7,9 @@ use tokio::sync::{mpsc, oneshot}; use tauri::{AppHandle, Emitter}; use crate::database::{ - find_download_by_id_conn, list_all_ui_conn, list_downloading_ids_conn, list_queued_ids_conn, - mark_id_done_conn, reset_stale_downloading_to_queued_conn, set_last_error_by_id_conn, - set_status_by_id_conn, DownloadStatus, UiBacklogRow, + find_download_by_id_conn, list_all_ui_conn, list_downloading_ids_conn, list_error_ids_conn, + list_queued_ids_conn, mark_id_done_conn, reset_stale_downloading_to_queued_conn, + set_last_error_by_id_conn, set_status_by_id_conn, DownloadStatus, UiBacklogRow, }; use crate::download::pipeline; use crate::settings; @@ -106,6 +106,9 @@ pub async fn run_download_manager( let initial_settings = settings::load_settings(); let mut paused = !initial_settings.download_automatically; let mut max_parallel = initial_settings.parallel_downloads.max(1) as usize; + let mut cooldown_secs = initial_settings.cooldown_secs; + let mut retry_on_queue_empty = initial_settings.retry_on_queue_empty; + let mut auto_retried: HashSet = HashSet::new(); // On startup, recover any rows stuck in 'downloading' from a previous run { @@ -139,6 +142,7 @@ pub async fn run_download_manager( &mut overrides, paused, max_parallel, + cooldown_secs, &cmd_tx, false, ) @@ -148,6 +152,7 @@ pub async fn run_download_manager( let mut force_start = false; match cmd { DownloadCommand::Enqueue { ids } => { + auto_retried.clear(); enqueue_ids( &app, db.clone(), @@ -196,8 +201,11 @@ pub async fn run_download_manager( force_start = true; } DownloadCommand::RefreshSettings => { - max_parallel = settings::load_settings().parallel_downloads.max(1) as usize; - tracing::info!("Updated max_parallel to {}", max_parallel); + let s = settings::load_settings(); + max_parallel = s.parallel_downloads.max(1) as usize; + cooldown_secs = s.cooldown_secs; + retry_on_queue_empty = s.retry_on_queue_empty; + tracing::info!("Updated max_parallel={} cooldown={}s retry_on_empty={}", max_parallel, cooldown_secs, retry_on_queue_empty); } DownloadCommand::SetPaused(next) => { paused = next; @@ -211,6 +219,21 @@ pub async fn run_download_manager( } DownloadCommand::TaskFinished { id } => { active.remove(&id); + if retry_on_queue_empty && !paused && queue.is_empty() && active.is_empty() { + let db_clone = db.clone(); + let error_ids = tauri::async_runtime::spawn_blocking(move || { + let conn = db_clone.blocking_lock(); + list_error_ids_conn(&*conn).unwrap_or_default() + }) + .await + .unwrap_or_default(); + for eid in error_ids { + if !auto_retried.contains(&eid) { + auto_retried.insert(eid); + queue.push_back(eid); + } + } + } } } @@ -222,6 +245,7 @@ pub async fn run_download_manager( &mut overrides, paused, max_parallel, + cooldown_secs, &cmd_tx, force_start, ) @@ -404,6 +428,7 @@ async fn maybe_start_next( overrides: &mut HashMap, paused: bool, max_parallel: usize, + cooldown_secs: u32, cmd_tx: &mpsc::Sender, force: bool, ) { @@ -446,6 +471,9 @@ async fn maybe_start_next( let db_clone = db.clone(); let opts = overrides.remove(&id); let handle = tauri::async_runtime::spawn(async move { + if cooldown_secs > 0 { + tokio::time::sleep(std::time::Duration::from_secs(cooldown_secs as u64)).await; + } match run_download_with_progress(&app_clone, db_clone.clone(), id, opts).await { Ok(path) => { let _ = set_status(db_clone.clone(), id, DownloadStatus::Done).await; diff --git a/src-tauri/src/download/pipeline.rs b/src-tauri/src/download/pipeline.rs index 01c1814..5718fc9 100644 --- a/src-tauri/src/download/pipeline.rs +++ b/src-tauri/src/download/pipeline.rs @@ -263,8 +263,7 @@ pub async fn execute_download_job( }); return Ok(finals.get(0).cloned()); } else { - last_error = - Some(format!("No files moved from {}", tmp_dir.display())); + last_error.get_or_insert_with(|| format!("No files moved from {}", tmp_dir.display())); } } Ok((_ok, output, tmp_dir)) => { @@ -280,7 +279,7 @@ pub async fn execute_download_job( specific_cookie_error = friendly_browser_error(browser, &output); } - last_error = Some(msg.clone()); + last_error.get_or_insert(msg.clone()); (emitter)(DownloadEvent::Message { id: row.id, message: msg, @@ -288,7 +287,7 @@ pub async fn execute_download_job( let _ = fs::remove_dir_all(&tmp_dir); } Err(e) => { - last_error = Some(e.to_string()); + last_error.get_or_insert_with(|| e.to_string()); } } } @@ -333,7 +332,7 @@ pub async fn execute_download_job( }); return Ok(finals.get(0).cloned()); } else { - last_error = Some(format!("No files moved from {}", tmp_dir.display())); + last_error.get_or_insert_with(|| format!("No files moved from {}", tmp_dir.display())); } } Ok((_ok, output, tmp_dir)) => { @@ -343,7 +342,7 @@ pub async fn execute_download_job( if specific_cookie_error.is_none() { specific_cookie_error = friendly_browser_error(browser, &output); } - last_error = Some(msg.clone()); + last_error.get_or_insert(msg.clone()); (emitter)(DownloadEvent::Message { id: row.id, message: msg, @@ -351,7 +350,7 @@ pub async fn execute_download_job( let _ = fs::remove_dir_all(&tmp_dir); } Err(e) => { - last_error = Some(e.to_string()); + last_error.get_or_insert_with(|| e.to_string()); } } continue; @@ -394,14 +393,14 @@ pub async fn execute_download_job( if specific_cookie_error.is_none() { specific_cookie_error = friendly_browser_error(browser, &output); } - last_error = Some(msg.clone()); + last_error.get_or_insert(msg.clone()); (emitter)(DownloadEvent::Message { id: row.id, message: msg, }); } Err(e) => { - last_error = Some(e.to_string()); + last_error.get_or_insert_with(|| e.to_string()); } } } diff --git a/src-tauri/src/settings.rs b/src-tauri/src/settings.rs index ce613f3..0979022 100644 --- a/src-tauri/src/settings.rs +++ b/src-tauri/src/settings.rs @@ -30,6 +30,8 @@ impl Default for Settings { keep_downloading_on_other_pages: true, parallel_downloads: 3, use_system_binaries: false, + cooldown_secs: 0, + retry_on_queue_empty: false, } } } @@ -139,6 +141,8 @@ pub fn save_settings(settings: &Settings) -> Result<(), String> { keep_downloading_on_other_pages: settings.keep_downloading_on_other_pages, parallel_downloads: settings.parallel_downloads, use_system_binaries: settings.use_system_binaries, + cooldown_secs: settings.cooldown_secs, + retry_on_queue_empty: settings.retry_on_queue_empty, }; let body = serde_json::to_string_pretty(&to_write) diff --git a/src/pages/settings.rs b/src/pages/settings.rs index 68d88ef..f05fb67 100644 --- a/src/pages/settings.rs +++ b/src/pages/settings.rs @@ -34,6 +34,10 @@ pub struct Settings { pub parallel_downloads: u8, #[serde(default)] pub use_system_binaries: bool, + #[serde(default)] + pub cooldown_secs: u32, + #[serde(default)] + pub retry_on_queue_empty: bool, } fn default_true() -> bool { @@ -190,6 +194,26 @@ pub fn settings_page() -> Html { }) }; + let on_cooldown_change = { + let settings = settings.clone(); + Callback::from(move |e: web_sys::InputEvent| { + let value = e.target_unchecked_into::().value_as_number() as u32; + let mut s = (*settings).clone(); + s.cooldown_secs = value; + settings.set(s); + }) + }; + + let on_retry_on_queue_empty_change = { + let settings = settings.clone(); + Callback::from(move |e: Event| { + let checked = e.target_unchecked_into::().checked(); + let mut s = (*settings).clone(); + s.retry_on_queue_empty = checked; + settings.set(s); + }) + }; + let on_delete_mode_change = { let settings = settings.clone(); Callback::from(move |e: Event| { @@ -326,6 +350,16 @@ pub fn settings_page() -> Html { +
+ + +
+ +
+ + +
+
@@ -391,6 +425,8 @@ impl Default for Settings { keep_downloading_on_other_pages: true, parallel_downloads: 3, use_system_binaries: false, + cooldown_secs: 0, + retry_on_queue_empty: false, } } } From 71e8a8cdf5815c11a2f471f1fca9639bdc94703c Mon Sep 17 00:00:00 2001 From: hjoncour Date: Thu, 7 May 2026 00:29:10 -0400 Subject: [PATCH 2/2] update --- Cargo.toml | 2 + index.html | 28 ++--- src-tauri/src/commands/library.rs | 8 +- src-tauri/src/database.rs | 29 ++++- src-tauri/src/download/manager.rs | 68 +++++++++-- src-tauri/src/download/pipeline.rs | 8 +- src/app.rs | 19 +++ src/components/sidebar.rs | 20 +-- src/dom.rs | 190 +++++++++++++++++++++++++++++ src/main.rs | 2 + src/pages/downloads.rs | 20 ++- src/pages/extension.rs | 46 ++++--- src/pages/home.rs | 38 +++--- src/pages/library.rs | 7 +- src/pages/settings.rs | 129 +++++++++++--------- src/pages/sponsor.rs | 39 +++--- 16 files changed, 497 insertions(+), 156 deletions(-) create mode 100644 src/dom.rs diff --git a/Cargo.toml b/Cargo.toml index 0530615..c9657ab 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,6 +11,8 @@ wasm-bindgen-futures = "0.4" web-sys = { version = "0.3", features = [ "Window", "Document", + "Element", + "HtmlCollection", "HtmlInputElement", "HtmlSelectElement", "Event", diff --git a/index.html b/index.html index 4cd0412..60fa658 100644 --- a/index.html +++ b/index.html @@ -1,20 +1,20 @@ - - - - - - clip-downloader + + + + + + clip-downloader - - - - - + + + + + - + - + - + diff --git a/src-tauri/src/commands/library.rs b/src-tauri/src/commands/library.rs index c3f2d1e..b2f37ce 100644 --- a/src-tauri/src/commands/library.rs +++ b/src-tauri/src/commands/library.rs @@ -64,7 +64,9 @@ fn path_exists_ok(path: &str) -> bool { #[tauri::command] pub async fn open_file_for_link(link: String) -> Result<(), String> { let db = crate::database::Database::new().map_err(|e| e.to_string())?; - let Some((_id, path)) = db.find_done_row_by_link(&link).map_err(|e| e.to_string())? else { return Ok(()); }; + let Some((_id, path)) = db.find_done_row_by_link(&link).map_err(|e| e.to_string())? else { + return Ok(()); + }; if !path_exists_ok(&path) { return Ok(()); } @@ -74,7 +76,9 @@ pub async fn open_file_for_link(link: String) -> Result<(), String> { #[tauri::command] pub async fn open_folder_for_link(link: String) -> Result<(), String> { let db = crate::database::Database::new().map_err(|e| e.to_string())?; - let Some((_id, path)) = db.find_done_row_by_link(&link).map_err(|e| e.to_string())? else { return Ok(()); }; + let Some((_id, path)) = db.find_done_row_by_link(&link).map_err(|e| e.to_string())? else { + return Ok(()); + }; let p = PathBuf::from(&path); let dir = if p.exists() { path.clone() diff --git a/src-tauri/src/database.rs b/src-tauri/src/database.rs index 6cdb09f..4e12b26 100644 --- a/src-tauri/src/database.rs +++ b/src-tauri/src/database.rs @@ -119,6 +119,27 @@ pub fn set_status_by_id_conn(conn: &Connection, id: i64, status: DownloadStatus) Ok(updated) } +pub fn set_status_bulk_conn( + conn: &Connection, + ids: &[i64], + status: DownloadStatus, +) -> Result { + if ids.is_empty() { + return Ok(0); + } + let id_list = ids + .iter() + .map(|id| id.to_string()) + .collect::>() + .join(","); + let token = status.as_str(); + let updated = conn.execute( + &format!("UPDATE downloads SET status=?1, last_error=CASE WHEN ?1='error' THEN last_error ELSE NULL END WHERE id IN ({id_list}) AND status<>?1"), + [token], + )?; + Ok(updated) +} + pub fn set_last_error_by_id_conn( conn: &Connection, id: i64, @@ -820,7 +841,13 @@ impl Database { mark_id_done_conn(&self.conn, id, path) } - pub fn link_exists_in_collection(&self, link: &str, platform: &str, user_handle: &str, origin: &str) -> Result { + pub fn link_exists_in_collection( + &self, + link: &str, + platform: &str, + user_handle: &str, + origin: &str, + ) -> Result { let norm = normalize_link(link.to_string()); let mut stmt = self.conn.prepare( "SELECT link FROM downloads diff --git a/src-tauri/src/download/manager.rs b/src-tauri/src/download/manager.rs index 7c08fd6..979c49f 100644 --- a/src-tauri/src/download/manager.rs +++ b/src-tauri/src/download/manager.rs @@ -9,7 +9,8 @@ use tauri::{AppHandle, Emitter}; use crate::database::{ find_download_by_id_conn, list_all_ui_conn, list_downloading_ids_conn, list_error_ids_conn, list_queued_ids_conn, mark_id_done_conn, reset_stale_downloading_to_queued_conn, - set_last_error_by_id_conn, set_status_by_id_conn, DownloadStatus, UiBacklogRow, + set_last_error_by_id_conn, set_status_bulk_conn, set_status_by_id_conn, DownloadStatus, + UiBacklogRow, }; use crate::download::pipeline; use crate::settings; @@ -22,6 +23,10 @@ pub enum DownloadEvent { id: i64, status: DownloadStatus, }, + BulkStatusChanged { + ids: Vec, + status: DownloadStatus, + }, Progress { id: i64, progress: f32, @@ -205,7 +210,12 @@ pub async fn run_download_manager( max_parallel = s.parallel_downloads.max(1) as usize; cooldown_secs = s.cooldown_secs; retry_on_queue_empty = s.retry_on_queue_empty; - tracing::info!("Updated max_parallel={} cooldown={}s retry_on_empty={}", max_parallel, cooldown_secs, retry_on_queue_empty); + tracing::info!( + "Updated max_parallel={} cooldown={}s retry_on_empty={}", + max_parallel, + cooldown_secs, + retry_on_queue_empty + ); } DownloadCommand::SetPaused(next) => { paused = next; @@ -316,30 +326,62 @@ async fn enqueue_ids( active: &HashMap, status: DownloadStatus, ) { - for id in ids { - if active.contains_key(id) { - continue; - } - if queue.contains(id) { - continue; + if ids.len() == 1 { + let id = ids[0]; + if active.contains_key(&id) || queue.contains(&id) { + return; } - let changed = match set_status(db.clone(), *id, status).await { + let changed = match set_status(db.clone(), id, status).await { Ok(c) => c, Err(err) => { emit_event( app, DownloadEvent::Message { - id: *id, + id, message: format!("Failed to set status: {err}"), }, ); - continue; + return; } }; if changed { - emit_event(app, DownloadEvent::StatusChanged { id: *id, status }); + emit_event(app, DownloadEvent::StatusChanged { id, status }); } - queue.push_back(*id); + queue.push_back(id); + return; + } + + let to_enqueue: Vec = ids + .iter() + .copied() + .filter(|id| !active.contains_key(id) && !queue.contains(id)) + .collect(); + if to_enqueue.is_empty() { + return; + } + + let db_clone = db.clone(); + let ids_clone = to_enqueue.clone(); + let changed = tauri::async_runtime::spawn_blocking(move || { + let conn = db_clone.blocking_lock(); + set_status_bulk_conn(&*conn, &ids_clone, status) + }) + .await + .ok() + .and_then(Result::ok) + .unwrap_or(0); + + if changed > 0 { + emit_event( + app, + DownloadEvent::BulkStatusChanged { + ids: to_enqueue.clone(), + status, + }, + ); + } + for id in to_enqueue { + queue.push_back(id); } } diff --git a/src-tauri/src/download/pipeline.rs b/src-tauri/src/download/pipeline.rs index 5718fc9..edb834a 100644 --- a/src-tauri/src/download/pipeline.rs +++ b/src-tauri/src/download/pipeline.rs @@ -263,7 +263,9 @@ pub async fn execute_download_job( }); return Ok(finals.get(0).cloned()); } else { - last_error.get_or_insert_with(|| format!("No files moved from {}", tmp_dir.display())); + last_error.get_or_insert_with(|| { + format!("No files moved from {}", tmp_dir.display()) + }); } } Ok((_ok, output, tmp_dir)) => { @@ -332,7 +334,9 @@ pub async fn execute_download_job( }); return Ok(finals.get(0).cloned()); } else { - last_error.get_or_insert_with(|| format!("No files moved from {}", tmp_dir.display())); + last_error.get_or_insert_with(|| { + format!("No files moved from {}", tmp_dir.display()) + }); } } Ok((_ok, output, tmp_dir)) => { diff --git a/src/app.rs b/src/app.rs index 72de171..18b9473 100644 --- a/src/app.rs +++ b/src/app.rs @@ -426,6 +426,10 @@ pub fn app() -> Html { id: i64, status: DownloadStatus, }, + BulkStatusChanged { + ids: Vec, + status: DownloadStatus, + }, Progress { id: i64, progress: f32, @@ -449,6 +453,21 @@ pub fn app() -> Html { let mut should_refresh = false; match evt { + DownloadEventPayload::BulkStatusChanged { ids, status } => { + for id in &ids { + if let Some(entry) = map.get_mut(id) { + entry.row.status = status; + entry.row.last_error = None; + entry.progress = 0.0; + entry.downloaded_bytes = 0; + entry.total_bytes = None; + entry.stage_text = default_stage_text(&entry.row); + entry.last_message = None; + } + } + commit = true; + should_refresh = true; + } DownloadEventPayload::StatusChanged { id, status } => match status { DownloadStatus::Done | DownloadStatus::Canceled => { if map.remove(&id).is_none() { diff --git a/src/components/sidebar.rs b/src/components/sidebar.rs index 7889584..aa28ad5 100644 --- a/src/components/sidebar.rs +++ b/src/components/sidebar.rs @@ -1,4 +1,5 @@ use crate::app::Page; +use crate::dom::assign_missing_descriptive_ids; use crate::log; use yew::prelude::*; use yew_icons::{Icon, IconId}; @@ -10,6 +11,11 @@ pub struct SidebarProps { #[function_component(Sidebar)] pub fn sidebar(props: &SidebarProps) -> Html { + use_effect(|| { + assign_missing_descriptive_ids("app-sidebar"); + || () + }); + let set_page = |p: Page, page_handle: UseStateHandle| { let label = match p { Page::Home => "Home", @@ -28,13 +34,13 @@ pub fn sidebar(props: &SidebarProps) -> Html { }; html! { -
diff --git a/src/pages/home.rs b/src/pages/home.rs index eacabb4..c1e7802 100644 --- a/src/pages/home.rs +++ b/src/pages/home.rs @@ -10,6 +10,7 @@ use yew_hooks::prelude::*; use yew_icons::{Icon, IconId}; use crate::app::log_invoke_err; +use crate::dom::assign_missing_descriptive_ids; #[derive(Serialize, Deserialize, Clone, Debug)] struct DownloadResult { @@ -41,6 +42,10 @@ pub struct Props { #[function_component(HomePage)] pub fn home_page(props: &Props) -> Html { println!("[FRONTEND] [pages/home.rs] [home_page component]"); + use_effect(|| { + assign_missing_descriptive_ids("home-page"); + || () + }); let greet_input_ref = use_node_ref(); let name = use_state(|| String::new()); let download_results = use_state(|| Vec::::new()); @@ -311,17 +316,17 @@ pub fn home_page(props: &Props) -> Html { }; html! { -
-

{"Welcome to Clip Downloader"}

-
- +
+

{"Welcome to Clip Downloader"}

+ + { if !*is_downloading { html! { -
- - +
+ {(*download_progress).clone()} +
} } else { html! {} }} -
- { for (*download_results).clone().into_iter().map(|result| { +
+ { for (*download_results).clone().into_iter().enumerate().map(|(index, result)| { html! { -
+
{ result.message }
} })}
-
- +
+
} diff --git a/src/pages/library.rs b/src/pages/library.rs index 058fe1d..de2a689 100644 --- a/src/pages/library.rs +++ b/src/pages/library.rs @@ -1,3 +1,4 @@ +use crate::dom::assign_missing_descriptive_ids; use crate::types::{content_type_str, platform_str, ClipRow, MediaKind}; use wasm_bindgen::prelude::*; use wasm_bindgen_futures::spawn_local; @@ -81,6 +82,10 @@ fn collection_title(row: &ClipRow) -> String { #[function_component(LibraryPage)] pub fn library_page() -> Html { + use_effect(|| { + assign_missing_descriptive_ids("library-page"); + || () + }); let done_rows = use_state(|| Vec::::new()); // load once @@ -139,7 +144,7 @@ pub fn library_page() -> Html { } html! { -
+

{"Library"}

{ diff --git a/src/pages/settings.rs b/src/pages/settings.rs index f05fb67..5e23e36 100644 --- a/src/pages/settings.rs +++ b/src/pages/settings.rs @@ -1,3 +1,4 @@ +use crate::dom::assign_missing_descriptive_ids; use wasm_bindgen::prelude::*; use wasm_bindgen_futures::spawn_local; use yew::prelude::*; @@ -73,6 +74,10 @@ extern "C" { #[function_component(SettingsPage)] pub fn settings_page() -> Html { + use_effect(|| { + assign_missing_descriptive_ids("settings-page"); + || () + }); let settings = use_state(Settings::default); let libs = use_state(|| None::); let settings_clone = settings.clone(); @@ -197,7 +202,9 @@ pub fn settings_page() -> Html { let on_cooldown_change = { let settings = settings.clone(); Callback::from(move |e: web_sys::InputEvent| { - let value = e.target_unchecked_into::().value_as_number() as u32; + let value = e + .target_unchecked_into::() + .value_as_number() as u32; let mut s = (*settings).clone(); s.cooldown_secs = value; settings.set(s); @@ -207,7 +214,9 @@ pub fn settings_page() -> Html { let on_retry_on_queue_empty_change = { let settings = settings.clone(); Callback::from(move |e: Event| { - let checked = e.target_unchecked_into::().checked(); + let checked = e + .target_unchecked_into::() + .checked(); let mut s = (*settings).clone(); s.retry_on_queue_empty = checked; settings.set(s); @@ -281,102 +290,102 @@ pub fn settings_page() -> Html { }; html! { -
-

{"Settings"}

-
-
- -
- - - +
-
- - + + +
-
- - + -
-
- -
-
diff --git a/src/pages/sponsor.rs b/src/pages/sponsor.rs index 8ecf821..376817f 100644 --- a/src/pages/sponsor.rs +++ b/src/pages/sponsor.rs @@ -1,3 +1,4 @@ +use crate::dom::assign_missing_descriptive_ids; use gloo_timers::callback::Timeout; use wasm_bindgen::prelude::*; use yew::prelude::*; @@ -10,6 +11,10 @@ extern "C" { #[function_component(SponsorPage)] pub fn sponsor_page() -> Html { + use_effect(|| { + assign_missing_descriptive_ids("sponsor-page"); + || () + }); let recently_copied = use_state(|| String::new()); let make_copy_callback = @@ -37,29 +42,29 @@ pub fn sponsor_page() -> Html { let eth_addr = "0x66994e0929576881B752a2BB8C249c9C8e74C253"; html! { -
-

{"Support ClipScraper Development"}

-

{"If you find ClipScraper useful, or if there are any features you would like to see, please consider supporting its development."}

+