Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<PackageView>`, 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<PlaylistGroupResultDto>`) 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<String>` 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.
Expand Down
55 changes: 55 additions & 0 deletions src-tauri/src/adapters/driven/sqlite/package_read_repo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,32 @@ impl PackageReadRepository for SqlitePackageReadRepo {
})
}

fn find_package_by_external_id(
&self,
external_id: &str,
) -> Result<Option<PackageView>, 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<Vec<DownloadView>, DomainError> {
use sea_orm::ColumnTrait;

Expand Down Expand Up @@ -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());
}
}
14 changes: 14 additions & 0 deletions src-tauri/src/adapters/driving/tauri_ipc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Option<crate::application::read_models::PackageSummaryDto>, 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::{
Expand Down
94 changes: 94 additions & 0 deletions src-tauri/src/application/queries/find_package_by_external_id.rs
Original file line number Diff line number Diff line change
@@ -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<Option<PackageSummaryDto>, 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");
}
}
9 changes: 9 additions & 0 deletions src-tauri/src/application/queries/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {}
3 changes: 3 additions & 0 deletions src-tauri/src/application/read_models/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
74 changes: 74 additions & 0 deletions src-tauri/src/application/read_models/package_summary.rs
Original file line number Diff line number Diff line change
@@ -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");
}
}
33 changes: 33 additions & 0 deletions src-tauri/src/application/test_support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -654,20 +654,34 @@ pub(crate) fn query_bus_with_accounts(repo: Arc<dyn AccountRepository>) -> Query
pub(crate) struct InMemoryPackageReadRepo {
packages: Mutex<Vec<PackageView>>,
downloads_by_package: Mutex<HashMap<String, Vec<DownloadView>>>,
/// Maps external_id → package_id for `find_package_by_external_id`.
external_id_index: Mutex<HashMap<String, String>>,
}

impl InMemoryPackageReadRepo {
pub(crate) fn new() -> Self {
Self {
packages: Mutex::new(Vec::new()),
downloads_by_package: Mutex::new(HashMap::new()),
external_id_index: Mutex::new(HashMap::new()),
}
}

pub(crate) fn insert(&self, view: PackageView) {
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<DownloadView>) {
self.downloads_by_package
.lock()
Expand Down Expand Up @@ -720,6 +734,25 @@ impl PackageReadRepository for InMemoryPackageReadRepo {
.cloned()
.unwrap_or_default())
}

fn find_package_by_external_id(
&self,
external_id: &str,
) -> Result<Option<PackageView>, 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.
Expand Down
7 changes: 7 additions & 0 deletions src-tauri/src/domain/ports/driven/package_read_repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Vec<DownloadView>, 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<Option<PackageView>, DomainError>;
}
Loading
Loading