From dddb4d4fd169ad95df137540f7a0f4f2d7055da9 Mon Sep 17 00:00:00 2001 From: vicrodh <103399283+vicrodh@users.noreply.github.com> Date: Mon, 26 Jan 2026 18:55:05 -0600 Subject: [PATCH] fix: resolve type mismatches and add Phase 6 polish - Fix RemoteTrackMetadata field types (disc_number, track_number as u8) - Fix RemoteAlbumMetadata fields (genres, tracks as Vec, disc_count as u8) - Fix cache method calls (async, proper signatures) - Fix error variant construction (ProviderUnavailable, InvalidProviderId) - Add track count mismatch warning in UI - Add rate limit error message handling - Add 'Open in browser' button for selected result --- src-tauri/src/commands/remote_metadata.rs | 4 +- src-tauri/src/library/remote_metadata/mod.rs | 57 ++++++++---------- .../LocalLibraryTagEditorModal.svelte | 59 ++++++++++++++++++- 3 files changed, 82 insertions(+), 38 deletions(-) diff --git a/src-tauri/src/commands/remote_metadata.rs b/src-tauri/src/commands/remote_metadata.rs index 5a2f76e6c..a982e524a 100644 --- a/src-tauri/src/commands/remote_metadata.rs +++ b/src-tauri/src/commands/remote_metadata.rs @@ -80,7 +80,7 @@ pub async fn remote_metadata_cache_stats( state: State<'_, RemoteMetadataSharedState>, ) -> Result { let state_guard = state.inner.lock().await; - Ok(state_guard.cache.stats()) + Ok(state_guard.cache.stats().await) } /// Clear the remote metadata cache @@ -90,7 +90,7 @@ pub async fn remote_metadata_clear_cache( ) -> Result<(), String> { log::info!("Command: remote_metadata_clear_cache"); let state_guard = state.inner.lock().await; - state_guard.cache.clear_all(); + state_guard.cache.clear_all().await; Ok(()) } diff --git a/src-tauri/src/library/remote_metadata/mod.rs b/src-tauri/src/library/remote_metadata/mod.rs index 2b7538cfe..44c4c8c3e 100644 --- a/src-tauri/src/library/remote_metadata/mod.rs +++ b/src-tauri/src/library/remote_metadata/mod.rs @@ -287,11 +287,11 @@ pub fn musicbrainz_full_to_metadata( }); // Extract genres from tags (sorted by count, take top 5) - let genres = release.tags.as_ref().map(|tags| { + let genres: Vec = release.tags.as_ref().map(|tags| { let mut sorted: Vec<_> = tags.iter().collect(); sorted.sort_by(|a, b| b.count.cmp(&a.count)); sorted.iter().take(5).map(|t| t.name.clone()).collect() - }); + }).unwrap_or_default(); // Extract label and catalog number let (label, catalog_number) = release @@ -307,17 +307,17 @@ pub fn musicbrainz_full_to_metadata( .unwrap_or((None, None)); // Count discs - let disc_count = release.media.as_ref().map(|m| m.len() as u8); + let disc_count = release.media.as_ref().map(|m| m.len() as u8).unwrap_or(1); // Convert tracks - let tracks = release.media.as_ref().map(|media| { + let tracks: Vec = release.media.as_ref().map(|media| { let mut all_tracks = Vec::new(); for medium in media { if let Some(tracks) = &medium.tracks { for track in tracks { all_tracks.push(RemoteTrackMetadata { - disc_number: medium.position, - track_number: track.position, + disc_number: medium.position.unwrap_or(1), + track_number: track.position.unwrap_or(1), title: track.title.clone().or_else(|| { track.recording.as_ref().and_then(|r| r.title.clone()) }).unwrap_or_default(), @@ -329,7 +329,7 @@ pub fn musicbrainz_full_to_metadata( } } all_tracks - }); + }).unwrap_or_default(); RemoteAlbumMetadata { provider: RemoteProvider::MusicBrainz, @@ -372,7 +372,7 @@ pub fn discogs_full_to_metadata( .unwrap_or_default(); // Combine genres and styles - let genres = { + let genres: Vec = { let mut combined = Vec::new(); if let Some(g) = &release.genres { combined.extend(g.clone()); @@ -380,11 +380,7 @@ pub fn discogs_full_to_metadata( if let Some(s) = &release.styles { combined.extend(s.clone()); } - if combined.is_empty() { - None - } else { - Some(combined) - } + combined }; // Get first label and catalog number @@ -396,7 +392,7 @@ pub fn discogs_full_to_metadata( .unwrap_or((None, None)); // Convert tracklist - let tracks = release.tracklist.as_ref().map(|tracklist| { + let tracks: Vec = release.tracklist.as_ref().map(|tracklist| { tracklist .iter() .filter(|t| { @@ -406,22 +402,17 @@ pub fn discogs_full_to_metadata( .map(|t| { let (disc_number, track_number) = parse_discogs_position(&t.position); RemoteTrackMetadata { - disc_number: Some(disc_number), - track_number: Some(track_number), + disc_number, + track_number, title: t.title.clone(), duration_ms: t.duration.as_ref().and_then(|d| parse_discogs_duration(d)), } }) .collect() - }); + }).unwrap_or_default(); // Count unique discs - let disc_count = tracks.as_ref().map(|t| { - t.iter() - .filter_map(|track| track.disc_number) - .max() - .unwrap_or(1) - }); + let disc_count = tracks.iter().map(|t| t.disc_number).max().unwrap_or(1); RemoteAlbumMetadata { provider: RemoteProvider::Discogs, @@ -452,9 +443,8 @@ impl RemoteMetadataState { limit: usize, ) -> Result, RemoteMetadataError> { // Check cache first - let cache_key = format!("{:?}:{}:{:?}:{}", provider, query, artist, limit); - if let Some(cached) = self.cache.get_search(&cache_key) { - log::debug!("Cache hit for search: {}", cache_key); + if let Some(cached) = self.cache.get_search(provider, query, None, artist).await { + log::debug!("Cache hit for search: {:?} {}", provider, query); return Ok(cached); } @@ -468,7 +458,7 @@ impl RemoteMetadataState { }; // Cache results - self.cache.set_search(&cache_key, results.clone()); + self.cache.set_search(provider, query, None, artist, results.clone()).await; Ok(results) } @@ -480,9 +470,8 @@ impl RemoteMetadataState { provider_id: &str, ) -> Result { // Check cache first - let cache_key = format!("{:?}:{}", provider, provider_id); - if let Some(cached) = self.cache.get_metadata(&cache_key) { - log::debug!("Cache hit for metadata: {}", cache_key); + if let Some(cached) = self.cache.get_metadata(provider, provider_id).await { + log::debug!("Cache hit for metadata: {:?} {}", provider, provider_id); return Ok(cached); } @@ -496,7 +485,7 @@ impl RemoteMetadataState { }; // Cache results - self.cache.set_metadata(&cache_key, metadata.clone()); + self.cache.set_metadata(provider, provider_id, metadata.clone()).await; Ok(metadata) } @@ -509,7 +498,7 @@ impl RemoteMetadataState { limit: usize, ) -> Result, RemoteMetadataError> { let mb_state = self.musicbrainz.as_ref() - .ok_or(RemoteMetadataError::ProviderUnavailable)?; + .ok_or(RemoteMetadataError::ProviderUnavailable("MusicBrainz not initialized".to_string()))?; let response = mb_state.client .search_releases_extended(query, artist, None, limit) @@ -552,7 +541,7 @@ impl RemoteMetadataState { release_id: &str, ) -> Result { let mb_state = self.musicbrainz.as_ref() - .ok_or(RemoteMetadataError::ProviderUnavailable)?; + .ok_or(RemoteMetadataError::ProviderUnavailable("MusicBrainz not initialized".to_string()))?; let response = mb_state.client .get_release_with_tracks(release_id) @@ -568,7 +557,7 @@ impl RemoteMetadataState { release_id: &str, ) -> Result { let release_id: u64 = release_id.parse() - .map_err(|_| RemoteMetadataError::InvalidProviderId)?; + .map_err(|_| RemoteMetadataError::InvalidProviderId("Invalid Discogs release ID".to_string()))?; let client = self.discogs.lock().await; diff --git a/src/lib/components/LocalLibraryTagEditorModal.svelte b/src/lib/components/LocalLibraryTagEditorModal.svelte index e47a6ce0e..929d1c52c 100644 --- a/src/lib/components/LocalLibraryTagEditorModal.svelte +++ b/src/lib/components/LocalLibraryTagEditorModal.svelte @@ -2,6 +2,7 @@ import Modal from './Modal.svelte'; import { invoke } from '@tauri-apps/api/core'; import { ask } from '@tauri-apps/plugin-dialog'; + import { open } from '@tauri-apps/plugin-opener'; import { showToast } from '$lib/stores/toastStore'; interface LocalTrack { @@ -195,6 +196,11 @@ if (metadata.catalog_number) catalogNumber = metadata.catalog_number; if (metadata.disc_count) albumTotalDiscs = metadata.disc_count; + // Check for track count mismatch + const remoteTrackCount = metadata.tracks?.length ?? 0; + const localTrackCount = trackEdits.length; + const hasMismatch = remoteTrackCount > 0 && remoteTrackCount !== localTrackCount; + // Apply track-level metadata if available if (metadata.tracks && metadata.tracks.length > 0) { // Match tracks by position @@ -207,16 +213,50 @@ } } - showToast(`Applied metadata from ${remoteProvider === 'musicbrainz' ? 'MusicBrainz' : 'Discogs'}`, 'success'); + const providerName = remoteProvider === 'musicbrainz' ? 'MusicBrainz' : 'Discogs'; + if (hasMismatch) { + showToast( + `Applied from ${providerName}. Track count differs: local=${localTrackCount}, remote=${remoteTrackCount}`, + 'warning' + ); + } else { + showToast(`Applied metadata from ${providerName}`, 'success'); + } showRemotePanel = false; } catch (err) { console.error('Failed to fetch metadata:', err); - showToast(`Failed to fetch metadata: ${err}`, 'error'); + // Check for rate limiting + const errStr = String(err); + if (errStr.includes('429') || errStr.toLowerCase().includes('rate')) { + showToast('Rate limited. Please wait a moment and try again.', 'warning'); + } else { + showToast(`Failed to fetch metadata: ${err}`, 'error'); + } } finally { remoteLoading = false; } } + function getSourceUrl(result: RemoteAlbumSearchResult): string { + if (result.provider === 'musicbrainz') { + return `https://musicbrainz.org/release/${result.provider_id}`; + } else { + return `https://www.discogs.com/release/${result.provider_id}`; + } + } + + async function openInBrowser() { + const selected = remoteResults.find(r => r.provider_id === selectedRemoteId); + if (!selected) return; + const url = getSourceUrl(selected); + try { + await open(url); + } catch (err) { + console.error('Failed to open URL:', err); + showToast('Failed to open browser', 'error'); + } + } + function parseYear(): number | null { const trimmed = yearInput.trim(); if (!trimmed) return null; @@ -429,6 +469,20 @@
+