From ffa698f2bfd9c09c1b26afb89351140a82b2f502 Mon Sep 17 00:00:00 2001 From: Mathieu Piton <27002047+mpiton@users.noreply.github.com> Date: Fri, 1 May 2026 10:04:27 +0200 Subject: [PATCH] feat(link): wire willReuseExisting in MediaGrabberDialog (#136) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add read-only IPC query `package_find_by_external_id` so the playlist banner accurately reports "Reusing playlist package {name}" when a playlist URL is re-resolved, instead of always falling back to "Will create …". Backend (CQRS read side): - `PackageReadRepository::find_package_by_external_id` (port) - SQLite impl filters on existing `idx_packages_external_id` UNIQUE index, mirrors `find_package_by_id` SQL pattern - New `FindPackageByExternalIdQuery` handler maps result to a minimal `PackageSummaryDto { packageId, packageName }` (camelCase) - Tauri IPC: `package_find_by_external_id` registered in lib.rs Frontend: - `PackageSummary` type + `usePackageByExternalId(externalId?)` hook with `enabled: !!externalId` short-circuit - `MediaGrabberDialog` calls the hook with `canonicalPlaylistKey(...)` when `metadata.isPlaylist`, derives `willReuseExisting`, and prefers the real `existingPackage.packageName` for the banner title - Loading state defaults to "Will create" so the banner is never blocked by the round-trip (AC #3) Tests: 7 new Rust (DTO + handler + SQLite), 6 new TypeScript (hook + dialog branches). cargo test 1337 passing, vitest 650 passing, clippy + tsc + oxlint clean. Closes #136 --- CHANGELOG.md | 2 + .../driven/sqlite/package_read_repo.rs | 55 +++++++++++ src-tauri/src/adapters/driving/tauri_ipc.rs | 14 +++ .../queries/find_package_by_external_id.rs | 94 +++++++++++++++++++ src-tauri/src/application/queries/mod.rs | 9 ++ src-tauri/src/application/read_models/mod.rs | 3 + .../read_models/package_summary.rs | 74 +++++++++++++++ src-tauri/src/application/test_support.rs | 33 +++++++ .../ports/driven/package_read_repository.rs | 7 ++ src-tauri/src/lib.rs | 15 +-- .../__tests__/usePackageByExternalId.test.tsx | 79 ++++++++++++++++ src/hooks/usePackageByExternalId.ts | 14 +++ src/types/package.ts | 5 + .../MediaGrabberDialog/MediaGrabberDialog.tsx | 12 ++- .../__tests__/MediaGrabberDialog.test.tsx | 51 ++++++++++ 15 files changed, 459 insertions(+), 8 deletions(-) create mode 100644 src-tauri/src/application/queries/find_package_by_external_id.rs create mode 100644 src-tauri/src/application/read_models/package_summary.rs create mode 100644 src/hooks/__tests__/usePackageByExternalId.test.tsx create mode 100644 src/hooks/usePackageByExternalId.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 8bd1e631..ae13b00d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Playlist banner reuse-aware** (issue #136, PR #135 follow-up): the `MediaGrabberDialog` now predicts whether re-resolving a playlist will reuse an existing package or create a new one, so `PlaylistPackageBanner` shows "Reusing playlist package {name}" instead of always falling back to "Will create …". A new read-only IPC query `package_find_by_external_id` is wired through the CQRS read side: `PackageReadRepository` gains `find_package_by_external_id(external_id) -> Option`, the SQLite adapter mirrors the existing `find_package_by_id` SQL path filtering on the existing `idx_packages_external_id` UNIQUE index, and a new `FindPackageByExternalIdQuery` handler maps the result to a minimal `PackageSummaryDto { packageId, packageName }` (camelCase) so the UI never receives the full aggregate. The new typed React hook `usePackageByExternalId(externalId?)` short-circuits via TanStack Query's `enabled: !!externalId` flag — when the dialog is opened on a non-playlist URL, no IPC call is made. The dialog computes `playlistKey = canonicalPlaylistKey(link.originalUrl)` only when `metadata.isPlaylist` is true, calls the hook unconditionally with `playlistKey | undefined` to preserve hook order, derives `willReuseExisting = !!existingPackage`, and prefers `existingPackage.packageName` over the default-derived name when populating the banner. The "loading → false" default ensures the banner is never blocked by the round-trip; the banner momentarily reports "will create" while the IPC is in flight, which is the safe fallback per AC #3. Seven new Rust tests (`PackageSummaryDto` round-trip + camelCase serialization, three handler tests against `InMemoryPackageReadRepo`, two SQLite tests round-tripping `external_id` matches and misses) and six new TypeScript tests (four hook tests covering invoke args / response / skip / null match, two dialog tests covering the will-create vs will-reuse banner branches) cover every acceptance criterion. cargo test 1337 passing, vitest 650 passing, clippy + tsc clean. + - **Keyboard-accessible move-download-between-packages** (issue #134, PR #133 follow-up): the pointer-only HTML5 drag-and-drop introduced for moving downloads between packages now has a fully keyboard-driven and screen-reader-friendly alternative. Each `PackageDownloadRow` exposes a single toggle button (`Move…` ↔ `Cancel move`) that marks the download as pending-move; the button carries `aria-pressed` so screen readers announce the toggle state, replacing the deprecated `aria-grabbed` named in the original issue. Once a download is selected, every other `PackageRow` (the source package is intentionally excluded) surfaces a `Move selected here` action with an explicit `aria-label="Move selected download to {package}"`; the deprecated `aria-dropeffect` is intentionally not used because the descriptive button label plus the live region cover the same intent without the deprecated attribute. Activating the target reuses the same `package_remove_download` + `package_add_download` IPC pair as the drag-and-drop path (now factored into a shared `moveDownloadBetweenPackages` helper so the rollback logic stays in one place), surfaces start / success / cancel / error transitions through a new visually-hidden `role="status" aria-live="polite"` live region (`packages-move-live-region`), and matches the pointer path's visual feedback by raising `toast.success(packages.toast.moveDownloadSuccess)` on success and `toast.error(packages.toast.moveDownloadError)` in the catch (`toast.error(packages.toast.moveDownloadRollbackError)` still fires when both the move and the rollback fail). The `executeMove` finally block guards against an in-flight move clobbering a newer pending selection: `setPendingMove` is cleared only when the current state still references the move that just completed. The drag-and-drop path is unchanged. New i18n keys `packages.move.{start,startAriaLabel,cancel,cancelAriaLabel,target,targetAriaLabel,liveRegionLabel,announce.{selected,cancelled,started,success,error}}` ship in EN + FR. Six new Vitest tests cover the four issue acceptance criteria: keyboard initiation without a pointer (`Move` button → `aria-pressed` flips), live region announcements (selected / success / cancelled / error wording), unchanged pointer drag-and-drop (existing test still green), error-path coverage of the "target add fails, rollback succeeds" branch with explicit call-order assertions, and a race-condition guard that proves a stale `executeMove` does not erase a newer selection when its `package_remove_download` resolves late. - **Auto-grouping playlist → package** (PRD §6.3, PRD-v2 §P1.11, task 30): when the Link Grabber resolves a YouTube or SoundCloud playlist URL, every selected item is now auto-attached to a single `Package` per playlist. New `PlaylistGrouper` application service (`src-tauri/src/application/services/playlist_grouper.rs`) finds-or-creates a `Package` keyed by the new `external_id` natural key (the playlist URL acts as the natural id), so re-resolving the same playlist reuses the existing package instead of creating a duplicate. New `GroupPlaylistsCommand` handler (`group_playlists.rs`) routes the request through the grouper; new `link_group_playlists` Tauri IPC command (`PlaylistGroupInputDto` → `Vec`) lets the frontend orchestrate the group-then-attach flow. Migration `m20260430_000008_add_package_external_id` adds `packages.external_id TEXT NULL` plus the `idx_packages_external_id` index; the column is intentionally nullable (manual packages keep `NULL`) and the application layer enforces uniqueness through `find_by_external_id` rather than a DB UNIQUE constraint that would forbid more than one manual row. Domain `Package` aggregate gains an `external_id: Option` field with `set_external_id` and `external_id()` accessors; `PackageRepository::find_by_external_id` is added to the trait, returning the oldest row by `(created_at, id)` so repeated reuses pick the same package deterministically. The `MediaGrabberDialog` now shows a `PlaylistPackageBanner` ("Will create package: {name} with {N} items" / "Will reuse existing package …") above the playlist item list once `metadata.isPlaylist` is true, with EN + FR plural translations under `mediaGrabber.playlistBanner.*`. `LinkGrabberView.handleMediaGrabberConfirm` now calls `link_group_playlists` first when the user has selected playlist items, then `download_media_start`, then attaches every returned `downloadId` to the package via `package_add_download` (failures on attach are non-fatal — downloads still run, only the grouping is missed). Visibility of `application::commands::tests_support` is bumped to `pub(crate)` so the new `playlist_grouper` and `group_playlists` tests can reuse the existing `InMemoryPackageRepo` / `CapturingEventBus` mocks. 13 new tests cover the four acceptance criteria: 7 unit tests on `PlaylistGrouper` (create / reuse / blank id / fallback name / batch / partial failure / trim), 5 SQLite tests on `find_by_external_id` (round-trip / match / not-found / no NULL match / oldest-on-duplicates / clear-on-upsert), 3 handler tests, 4 frontend tests on `PlaylistPackageBanner`. cargo test 1327 passing, vitest 626 passing, clippy + fmt clean. Unblocks the Link Grabber's playlist UX. diff --git a/src-tauri/src/adapters/driven/sqlite/package_read_repo.rs b/src-tauri/src/adapters/driven/sqlite/package_read_repo.rs index 2e89e504..ca612074 100644 --- a/src-tauri/src/adapters/driven/sqlite/package_read_repo.rs +++ b/src-tauri/src/adapters/driven/sqlite/package_read_repo.rs @@ -276,6 +276,32 @@ impl PackageReadRepository for SqlitePackageReadRepo { }) } + fn find_package_by_external_id( + &self, + external_id: &str, + ) -> Result, DomainError> { + let sql = format!( + "{PACKAGE_AGG_SELECT} WHERE p.external_id = ? GROUP BY p.id \ + ORDER BY p.created_at ASC, p.id ASC" + ); + let ext_id_value = external_id.to_string(); + block_on(async { + let row = self + .db + .query_one(Statement::from_sql_and_values( + sea_orm::DatabaseBackend::Sqlite, + &sql, + [Value::from(ext_id_value)], + )) + .await + .map_err(map_db_err)?; + match row { + None => Ok(None), + Some(r) => Ok(Some(row_to_view(&r)?)), + } + }) + } + fn find_package_downloads(&self, id: &PackageId) -> Result, DomainError> { use sea_orm::ColumnTrait; @@ -1291,4 +1317,33 @@ mod tests { "fallback to updated_at when id has no embedded ms", ); } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_find_package_by_external_id_returns_some_when_match() { + let db = setup_test_db().await.expect("test db"); + let write = SqlitePackageRepo::new(db.clone()); + let read = SqlitePackageReadRepo::new(db); + + let mut pkg = make_package("ext-1", "YouTube Playlist", PackageSourceType::Playlist, 10); + pkg.set_external_id(Some("youtube:playlist:abc".to_string())); + write.save(&pkg).expect("save"); + + let result = read + .find_package_by_external_id("youtube:playlist:abc") + .expect("query"); + let view = result.expect("should find package by external_id"); + assert_eq!(view.id, "ext-1"); + assert_eq!(view.name, "YouTube Playlist"); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_find_package_by_external_id_returns_none_when_missing() { + let db = setup_test_db().await.expect("test db"); + let read = SqlitePackageReadRepo::new(db); + + let result = read + .find_package_by_external_id("youtube:playlist:missing") + .expect("query"); + assert!(result.is_none()); + } } diff --git a/src-tauri/src/adapters/driving/tauri_ipc.rs b/src-tauri/src/adapters/driving/tauri_ipc.rs index fa3ed7b9..027ea3ac 100644 --- a/src-tauri/src/adapters/driving/tauri_ipc.rs +++ b/src-tauri/src/adapters/driving/tauri_ipc.rs @@ -3194,6 +3194,20 @@ pub async fn package_list_downloads( .map_err(|e| e.to_string()) } +#[tauri::command] +pub async fn package_find_by_external_id( + state: State<'_, AppState>, + external_id: String, +) -> Result, String> { + state + .query_bus + .handle_find_package_by_external_id( + crate::application::queries::FindPackageByExternalIdQuery { external_id }, + ) + .await + .map_err(|e| e.to_string()) +} + #[cfg(test)] mod tests { use super::{ diff --git a/src-tauri/src/application/queries/find_package_by_external_id.rs b/src-tauri/src/application/queries/find_package_by_external_id.rs new file mode 100644 index 00000000..97324f93 --- /dev/null +++ b/src-tauri/src/application/queries/find_package_by_external_id.rs @@ -0,0 +1,94 @@ +//! Handler for [`FindPackageByExternalIdQuery`]. +//! +//! Delegates to [`PackageReadRepository`](crate::domain::ports::driven::PackageReadRepository) +//! which performs a direct index look-up on `packages.external_id`. +//! Returns `None` when no package has been registered for the given key. + +use crate::application::error::AppError; +use crate::application::query_bus::QueryBus; +use crate::application::read_models::PackageSummaryDto; + +impl QueryBus { + pub async fn handle_find_package_by_external_id( + &self, + query: super::FindPackageByExternalIdQuery, + ) -> Result, AppError> { + let repo = self + .package_read_repo() + .ok_or_else(|| AppError::Validation("package read repository not configured".into()))?; + let view = repo.find_package_by_external_id(&query.external_id)?; + Ok(view.as_ref().map(PackageSummaryDto::from)) + } +} + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use crate::application::queries::FindPackageByExternalIdQuery; + use crate::application::test_support::{ + InMemoryPackageReadRepo, query_bus_with_packages, sample_package_view, + }; + + #[tokio::test] + async fn test_find_package_by_external_id_returns_summary_when_match() { + let repo = Arc::new(InMemoryPackageReadRepo::new()); + repo.insert_with_external_id( + sample_package_view("yt-1", "YouTube Mix", "playlist", 1), + "youtube:playlist:abc", + ); + let bus = query_bus_with_packages(repo); + + let result = bus + .handle_find_package_by_external_id(FindPackageByExternalIdQuery { + external_id: "youtube:playlist:abc".to_string(), + }) + .await + .unwrap(); + + let summary = result.expect("should return Some when external_id matches"); + assert_eq!(summary.package_id, "yt-1"); + assert_eq!(summary.package_name, "YouTube Mix"); + } + + #[tokio::test] + async fn test_find_package_by_external_id_returns_none_when_no_match() { + let repo = Arc::new(InMemoryPackageReadRepo::new()); + let bus = query_bus_with_packages(repo); + + let result = bus + .handle_find_package_by_external_id(FindPackageByExternalIdQuery { + external_id: "youtube:playlist:missing".to_string(), + }) + .await + .unwrap(); + + assert!(result.is_none()); + } + + #[tokio::test] + async fn test_find_package_by_external_id_uses_exact_match() { + let repo = Arc::new(InMemoryPackageReadRepo::new()); + repo.insert_with_external_id( + sample_package_view("a", "Alpha", "playlist", 1), + "yt:playlist:aaa", + ); + repo.insert_with_external_id( + sample_package_view("b", "Beta", "playlist", 2), + "yt:playlist:bbb", + ); + repo.insert(sample_package_view("c", "Gamma", "manual", 3)); + let bus = query_bus_with_packages(repo); + + let result = bus + .handle_find_package_by_external_id(FindPackageByExternalIdQuery { + external_id: "yt:playlist:bbb".to_string(), + }) + .await + .unwrap(); + + let summary = result.expect("should return Some for exact key bbb"); + assert_eq!(summary.package_id, "b"); + assert_eq!(summary.package_name, "Beta"); + } +} diff --git a/src-tauri/src/application/queries/mod.rs b/src-tauri/src/application/queries/mod.rs index 9fb08543..5696fb00 100644 --- a/src-tauri/src/application/queries/mod.rs +++ b/src-tauri/src/application/queries/mod.rs @@ -4,6 +4,7 @@ //! Handler implementations live in submodules and add methods to `QueryBus`. mod count_by_state; +mod find_package_by_external_id; mod get_account; mod get_account_traffic; mod get_download_detail; @@ -164,3 +165,11 @@ pub struct ListPackageDownloadsQuery { pub id: PackageId, } impl Query for ListPackageDownloadsQuery {} + +/// Look up the package whose `external_id` matches the given key. +/// Returns `None` when no package has been registered for the given key. +#[derive(Debug)] +pub struct FindPackageByExternalIdQuery { + pub external_id: String, +} +impl Query for FindPackageByExternalIdQuery {} diff --git a/src-tauri/src/application/read_models/mod.rs b/src-tauri/src/application/read_models/mod.rs index 43e5a5cb..993c731a 100644 --- a/src-tauri/src/application/read_models/mod.rs +++ b/src-tauri/src/application/read_models/mod.rs @@ -4,8 +4,11 @@ pub mod account_view; pub mod download_detail_view; pub mod download_view; pub mod history_view; +pub mod package_summary; pub mod package_view; pub mod plugin_config_view; pub mod plugin_store_view; pub mod plugin_view; pub mod stats_view; + +pub use package_summary::PackageSummaryDto; diff --git a/src-tauri/src/application/read_models/package_summary.rs b/src-tauri/src/application/read_models/package_summary.rs new file mode 100644 index 00000000..79af87c0 --- /dev/null +++ b/src-tauri/src/application/read_models/package_summary.rs @@ -0,0 +1,74 @@ +//! Minimal package summary DTO for look-up results. +//! +//! Used by [`crate::application::queries::FindPackageByExternalIdQuery`] to +//! return just enough data for the caller to identify the package without +//! pulling the full aggregated view. + +use serde::Serialize; + +use crate::domain::model::views::PackageView; + +#[derive(Debug, Clone, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct PackageSummaryDto { + pub package_id: String, + pub package_name: String, +} + +impl From<&PackageView> for PackageSummaryDto { + fn from(view: &PackageView) -> Self { + Self { + package_id: view.id.clone(), + package_name: view.name.clone(), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::domain::model::views::PackageView; + + fn make_view(id: &str, name: &str) -> PackageView { + PackageView { + id: id.to_string(), + name: name.to_string(), + source_type: "playlist".to_string(), + folder_path: None, + auto_extract: true, + priority: 5, + created_at: 1_000, + downloads_count: 0, + total_bytes: 0, + downloaded_bytes: 0, + progress_percent: 0.0, + all_completed: false, + } + } + + #[test] + fn test_package_summary_dto_from_view_copies_id_and_name() { + let view = make_view("pkg-1", "My Playlist"); + let dto = PackageSummaryDto::from(&view); + assert_eq!(dto.package_id, "pkg-1"); + assert_eq!(dto.package_name, "My Playlist"); + } + + #[test] + fn test_package_summary_dto_serializes_to_camel_case() { + let view = make_view("pkg-42", "Test Package"); + let dto = PackageSummaryDto::from(&view); + let value = serde_json::to_value(&dto).unwrap(); + let obj = value.as_object().expect("object"); + assert!( + obj.contains_key("packageId"), + "camelCase field `packageId` missing" + ); + assert!( + obj.contains_key("packageName"), + "camelCase field `packageName` missing" + ); + assert_eq!(obj["packageId"], "pkg-42"); + assert_eq!(obj["packageName"], "Test Package"); + } +} diff --git a/src-tauri/src/application/test_support.rs b/src-tauri/src/application/test_support.rs index af69e7e0..57dfd183 100644 --- a/src-tauri/src/application/test_support.rs +++ b/src-tauri/src/application/test_support.rs @@ -654,6 +654,8 @@ pub(crate) fn query_bus_with_accounts(repo: Arc) -> Query pub(crate) struct InMemoryPackageReadRepo { packages: Mutex>, downloads_by_package: Mutex>>, + /// Maps external_id → package_id for `find_package_by_external_id`. + external_id_index: Mutex>, } impl InMemoryPackageReadRepo { @@ -661,6 +663,7 @@ impl InMemoryPackageReadRepo { Self { packages: Mutex::new(Vec::new()), downloads_by_package: Mutex::new(HashMap::new()), + external_id_index: Mutex::new(HashMap::new()), } } @@ -668,6 +671,17 @@ impl InMemoryPackageReadRepo { self.packages.lock().unwrap().push(view); } + /// Insert a package view and register an external_id → package_id mapping + /// so `find_package_by_external_id` can resolve it. + pub(crate) fn insert_with_external_id(&self, view: PackageView, external_id: &str) { + let pkg_id = view.id.clone(); + self.packages.lock().unwrap().push(view); + self.external_id_index + .lock() + .unwrap() + .insert(external_id.to_string(), pkg_id); + } + pub(crate) fn attach_downloads(&self, package_id: &str, downloads: Vec) { self.downloads_by_package .lock() @@ -720,6 +734,25 @@ impl PackageReadRepository for InMemoryPackageReadRepo { .cloned() .unwrap_or_default()) } + + fn find_package_by_external_id( + &self, + external_id: &str, + ) -> Result, DomainError> { + let index = self.external_id_index.lock().unwrap(); + let Some(pkg_id) = index.get(external_id) else { + return Ok(None); + }; + let pkg_id = pkg_id.clone(); + drop(index); + Ok(self + .packages + .lock() + .unwrap() + .iter() + .find(|p| p.id == pkg_id) + .cloned()) + } } /// Build a [`QueryBus`] wired with the given package read repository. diff --git a/src-tauri/src/domain/ports/driven/package_read_repository.rs b/src-tauri/src/domain/ports/driven/package_read_repository.rs index 2ddba73b..ebff0772 100644 --- a/src-tauri/src/domain/ports/driven/package_read_repository.rs +++ b/src-tauri/src/domain/ports/driven/package_read_repository.rs @@ -35,4 +35,11 @@ pub trait PackageReadRepository: Send + Sync { /// `id` ascending. Returns an empty vector when the package has no /// members or does not exist. fn find_package_downloads(&self, id: &PackageId) -> Result, DomainError>; + + /// Fetch the aggregated view for the package whose `external_id` + /// matches the given key. Returns `Ok(None)` when no row matches. + fn find_package_by_external_id( + &self, + external_id: &str, + ) -> Result, DomainError>; } diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index e8220acc..0c05c56b 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -80,13 +80,13 @@ pub use adapters::driving::tauri_ipc::{ download_set_priority, download_start, download_verify_checksum, history_clear, history_delete_entry, history_export, history_get_by_id, history_list, history_purge_older_than, history_search, link_group_playlists, link_resolve, - package_add_download, package_create, package_delete, package_get, package_list, - package_list_downloads, package_move_to_folder, package_remove_download, package_set_password, - package_set_priority, package_toggle_auto_extract, package_update, plugin_config_get, - plugin_config_update, plugin_disable, plugin_enable, plugin_install, plugin_list, - plugin_report_broken, plugin_store_install, plugin_store_list, plugin_store_refresh, - plugin_store_update, plugin_uninstall, reveal_in_folder, settings_get, settings_update, - stats_get, stats_top_modules, status_bar_get, + package_add_download, package_create, package_delete, package_find_by_external_id, package_get, + package_list, package_list_downloads, package_move_to_folder, package_remove_download, + package_set_password, package_set_priority, package_toggle_auto_extract, package_update, + plugin_config_get, plugin_config_update, plugin_disable, plugin_enable, plugin_install, + plugin_list, plugin_report_broken, plugin_store_install, plugin_store_list, + plugin_store_refresh, plugin_store_update, plugin_uninstall, reveal_in_folder, settings_get, + settings_update, stats_get, stats_top_modules, status_bar_get, }; #[cfg_attr(mobile, tauri::mobile_entry_point)] @@ -594,6 +594,7 @@ pub fn run() { package_list, package_get, package_list_downloads, + package_find_by_external_id, ]) .run(tauri::generate_context!()) // Tauri's run() has no meaningful recovery path — panic is intentional here diff --git a/src/hooks/__tests__/usePackageByExternalId.test.tsx b/src/hooks/__tests__/usePackageByExternalId.test.tsx new file mode 100644 index 00000000..dac90c10 --- /dev/null +++ b/src/hooks/__tests__/usePackageByExternalId.test.tsx @@ -0,0 +1,79 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { renderHook, waitFor } from '@testing-library/react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { invoke } from '@tauri-apps/api/core'; +import type { ReactNode } from 'react'; +import { usePackageByExternalId } from '../usePackageByExternalId'; + +vi.mock('@tauri-apps/api/core', () => ({ + invoke: vi.fn(), +})); + +const mockInvoke = vi.mocked(invoke); + +function wrapper() { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false, staleTime: 0 } }, + }); + return function Wrapper({ children }: { children: ReactNode }) { + return {children}; + }; +} + +beforeEach(() => { + mockInvoke.mockReset(); +}); + +describe('usePackageByExternalId', () => { + it('calls invoke with externalId arg when defined', async () => { + mockInvoke.mockResolvedValue({ packageId: 'p1', packageName: 'Mix' }); + + const { result } = renderHook( + () => usePackageByExternalId('youtube:playlist:abc'), + { wrapper: wrapper() }, + ); + await waitFor(() => expect(result.current.isLoading).toBe(false)); + + expect(mockInvoke).toHaveBeenCalledWith( + 'package_find_by_external_id', + { externalId: 'youtube:playlist:abc' }, + ); + }); + + it('returns the PackageSummary from the IPC response', async () => { + mockInvoke.mockResolvedValue({ packageId: 'p1', packageName: 'Mix' }); + + const { result } = renderHook( + () => usePackageByExternalId('youtube:playlist:abc'), + { wrapper: wrapper() }, + ); + await waitFor(() => expect(result.current.isLoading).toBe(false)); + + expect(result.current.data).toEqual({ packageId: 'p1', packageName: 'Mix' }); + }); + + it('does not call invoke when externalId is undefined', async () => { + const { result } = renderHook( + () => usePackageByExternalId(undefined), + { wrapper: wrapper() }, + ); + + // Give TanStack Query a tick to potentially fire — it should not. + await new Promise((r) => setTimeout(r, 50)); + + expect(mockInvoke).not.toHaveBeenCalled(); + expect(result.current.data).toBeUndefined(); + }); + + it('returns null when no package matches', async () => { + mockInvoke.mockResolvedValue(null); + + const { result } = renderHook( + () => usePackageByExternalId('youtube:playlist:xyz'), + { wrapper: wrapper() }, + ); + await waitFor(() => expect(result.current.isLoading).toBe(false)); + + expect(result.current.data).toBeNull(); + }); +}); diff --git a/src/hooks/usePackageByExternalId.ts b/src/hooks/usePackageByExternalId.ts new file mode 100644 index 00000000..a15043fb --- /dev/null +++ b/src/hooks/usePackageByExternalId.ts @@ -0,0 +1,14 @@ +import { useTauriQuery } from "@/api/hooks"; +import type { PackageSummary } from "@/types/package"; + +export function usePackageByExternalId(externalId: string | undefined) { + return useTauriQuery( + "package_find_by_external_id", + externalId ? { externalId } : undefined, + { + queryKey: ["package_find_by_external_id", { externalId }], + enabled: !!externalId, + staleTime: 30_000, + }, + ); +} diff --git a/src/types/package.ts b/src/types/package.ts index a5414e89..dee95f09 100644 --- a/src/types/package.ts +++ b/src/types/package.ts @@ -44,3 +44,8 @@ export interface PackageMoveOutcome { reason: string; }>; } + +export interface PackageSummary { + packageId: string; + packageName: string; +} diff --git a/src/views/LinkGrabberView/MediaGrabberDialog/MediaGrabberDialog.tsx b/src/views/LinkGrabberView/MediaGrabberDialog/MediaGrabberDialog.tsx index 6bfe6887..c5162c0c 100644 --- a/src/views/LinkGrabberView/MediaGrabberDialog/MediaGrabberDialog.tsx +++ b/src/views/LinkGrabberView/MediaGrabberDialog/MediaGrabberDialog.tsx @@ -17,6 +17,8 @@ import { PlaylistSection } from "./PlaylistSection"; import { PlaylistPackageBanner } from "./PlaylistPackageBanner"; import { SizeEstimate } from "./SizeEstimate"; import { useMediaMetadata } from "./useMediaMetadata"; +import { usePackageByExternalId } from "@/hooks/usePackageByExternalId"; +import { canonicalPlaylistKey } from "../canonicalPlaylistKey"; import type { ResolvedLink } from "../types"; import type { MediaGrabberOptions } from "@/types/media"; @@ -49,6 +51,13 @@ export function MediaGrabberDialog({ isError, refetch, } = useMediaMetadata(link.originalUrl, open); + + const playlistKey = metadata?.isPlaylist + ? canonicalPlaylistKey(link.originalUrl) + : undefined; + const { data: existingPackage } = usePackageByExternalId(playlistKey); + const willReuseExisting = !!existingPackage; + const requiresAudioOnly = !!metadata && link.mediaType === "audio" && metadata.availableQualities.length === 0 && @@ -114,12 +123,13 @@ export function MediaGrabberDialog({ {metadata.isPlaylist && metadata.playlistItems && metadata.playlistItems.length > 0 && ( <> 0 ? selectedPlaylistItems.length : metadata.playlistItems.length } + willReuseExisting={willReuseExisting} /> { ); }); }); + +describe("MediaGrabberDialog - existing package lookup", () => { + beforeEach(() => { + mockInvoke.mockReset(); + }); + + const mockPlaylistLink: ResolvedLink = { + ...mockMediaLink, + id: "playlist-1", + originalUrl: "https://youtube.com/playlist?list=PLtest123", + resolvedUrl: "https://youtube.com/playlist?list=PLtest123", + }; + + const mockPlaylistMetadataWithKey: MediaMetadata = { + ...mockPlaylistMetadata, + title: "Test Playlist", + }; + + function setupInvoke(packageFindResult: unknown) { + mockInvoke.mockImplementation(async (cmd: string) => { + if (cmd === "command_get_media_metadata") return mockPlaylistMetadataWithKey; + if (cmd === "package_find_by_external_id") return packageFindResult; + return null; + }); + } + + it("shows 'will create' banner when no existing package matches the playlist key", async () => { + setupInvoke(null); + + renderDialog({ link: mockPlaylistLink }); + + await screen.findByText("Test Playlist"); + + const banner = await screen.findByTestId("playlist-package-banner"); + expect(banner.textContent?.toLowerCase()).not.toMatch(/reuse|réutilisation/); + expect(banner).toBeInTheDocument(); + }); + + it("shows 'reusing' banner with the existing package name when one matches", async () => { + setupInvoke({ packageId: "pkg-1", packageName: "Lo-Fi Beats Mix" }); + + renderDialog({ link: mockPlaylistLink }); + + await screen.findByText("Test Playlist"); + + const banner = await screen.findByTestId("playlist-package-banner"); + expect(banner.textContent?.toLowerCase()).toMatch(/reuse|réutilisation/); + expect(banner.textContent).toContain("Lo-Fi Beats Mix"); + }); +});