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
This commit is contained in:
vicrodh committed 2026-01-26 18:55:05 -06:00
1 parent 9dd63e106b
commit dddb4d4fd1
3 files changed
+82 -38

No files matched your search

+2 -2
View File
@@ -80,7 +80,7 @@ pub async fn remote_metadata_cache_stats(
state: State<'_, RemoteMetadataSharedState>, state: State<'_, RemoteMetadataSharedState>,
) -> Result<crate::library::remote_metadata::CacheStats, String> { ) -> Result<crate::library::remote_metadata::CacheStats, String> {
let state_guard = state.inner.lock().await; let state_guard = state.inner.lock().await;
Ok(state_guard.cache.stats()) Ok(state_guard.cache.stats().await)
} }
/// Clear the remote metadata cache /// Clear the remote metadata cache
@@ -90,7 +90,7 @@ pub async fn remote_metadata_clear_cache(
) -> Result<(), String> { ) -> Result<(), String> {
log::info!("Command: remote_metadata_clear_cache"); log::info!("Command: remote_metadata_clear_cache");
let state_guard = state.inner.lock().await; let state_guard = state.inner.lock().await;
state_guard.cache.clear_all(); state_guard.cache.clear_all().await;
Ok(()) Ok(())
} }
+23 -34
View File
@@ -287,11 +287,11 @@ pub fn musicbrainz_full_to_metadata(
}); });
// Extract genres from tags (sorted by count, take top 5) // Extract genres from tags (sorted by count, take top 5)
let genres = release.tags.as_ref().map(|tags| { let genres: Vec<String> = release.tags.as_ref().map(|tags| {
let mut sorted: Vec<_> = tags.iter().collect(); let mut sorted: Vec<_> = tags.iter().collect();
sorted.sort_by(|a, b| b.count.cmp(&a.count)); sorted.sort_by(|a, b| b.count.cmp(&a.count));
sorted.iter().take(5).map(|t| t.name.clone()).collect() sorted.iter().take(5).map(|t| t.name.clone()).collect()
}); }).unwrap_or_default();
// Extract label and catalog number // Extract label and catalog number
let (label, catalog_number) = release let (label, catalog_number) = release
@@ -307,17 +307,17 @@ pub fn musicbrainz_full_to_metadata(
.unwrap_or((None, None)); .unwrap_or((None, None));
// Count discs // 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 // Convert tracks
let tracks = release.media.as_ref().map(|media| { let tracks: Vec<RemoteTrackMetadata> = release.media.as_ref().map(|media| {
let mut all_tracks = Vec::new(); let mut all_tracks = Vec::new();
for medium in media { for medium in media {
if let Some(tracks) = &medium.tracks { if let Some(tracks) = &medium.tracks {
for track in tracks { for track in tracks {
all_tracks.push(RemoteTrackMetadata { all_tracks.push(RemoteTrackMetadata {
disc_number: medium.position, disc_number: medium.position.unwrap_or(1),
track_number: track.position, track_number: track.position.unwrap_or(1),
title: track.title.clone().or_else(|| { title: track.title.clone().or_else(|| {
track.recording.as_ref().and_then(|r| r.title.clone()) track.recording.as_ref().and_then(|r| r.title.clone())
}).unwrap_or_default(), }).unwrap_or_default(),
@@ -329,7 +329,7 @@ pub fn musicbrainz_full_to_metadata(
} }
} }
all_tracks all_tracks
}); }).unwrap_or_default();
RemoteAlbumMetadata { RemoteAlbumMetadata {
provider: RemoteProvider::MusicBrainz, provider: RemoteProvider::MusicBrainz,
@@ -372,7 +372,7 @@ pub fn discogs_full_to_metadata(
.unwrap_or_default(); .unwrap_or_default();
// Combine genres and styles // Combine genres and styles
let genres = { let genres: Vec<String> = {
let mut combined = Vec::new(); let mut combined = Vec::new();
if let Some(g) = &release.genres { if let Some(g) = &release.genres {
combined.extend(g.clone()); combined.extend(g.clone());
@@ -380,11 +380,7 @@ pub fn discogs_full_to_metadata(
if let Some(s) = &release.styles { if let Some(s) = &release.styles {
combined.extend(s.clone()); combined.extend(s.clone());
} }
if combined.is_empty() { combined
None
} else {
Some(combined)
}
}; };
// Get first label and catalog number // Get first label and catalog number
@@ -396,7 +392,7 @@ pub fn discogs_full_to_metadata(
.unwrap_or((None, None)); .unwrap_or((None, None));
// Convert tracklist // Convert tracklist
let tracks = release.tracklist.as_ref().map(|tracklist| { let tracks: Vec<RemoteTrackMetadata> = release.tracklist.as_ref().map(|tracklist| {
tracklist tracklist
.iter() .iter()
.filter(|t| { .filter(|t| {
@@ -406,22 +402,17 @@ pub fn discogs_full_to_metadata(
.map(|t| { .map(|t| {
let (disc_number, track_number) = parse_discogs_position(&t.position); let (disc_number, track_number) = parse_discogs_position(&t.position);
RemoteTrackMetadata { RemoteTrackMetadata {
disc_number: Some(disc_number), disc_number,
track_number: Some(track_number), track_number,
title: t.title.clone(), title: t.title.clone(),
duration_ms: t.duration.as_ref().and_then(|d| parse_discogs_duration(d)), duration_ms: t.duration.as_ref().and_then(|d| parse_discogs_duration(d)),
} }
}) })
.collect() .collect()
}); }).unwrap_or_default();
// Count unique discs // Count unique discs
let disc_count = tracks.as_ref().map(|t| { let disc_count = tracks.iter().map(|t| t.disc_number).max().unwrap_or(1);
t.iter()
.filter_map(|track| track.disc_number)
.max()
.unwrap_or(1)
});
RemoteAlbumMetadata { RemoteAlbumMetadata {
provider: RemoteProvider::Discogs, provider: RemoteProvider::Discogs,
@@ -452,9 +443,8 @@ impl RemoteMetadataState {
limit: usize, limit: usize,
) -> Result<Vec<RemoteAlbumSearchResult>, RemoteMetadataError> { ) -> Result<Vec<RemoteAlbumSearchResult>, RemoteMetadataError> {
// Check cache first // Check cache first
let cache_key = format!("{:?}:{}:{:?}:{}", provider, query, artist, limit); if let Some(cached) = self.cache.get_search(provider, query, None, artist).await {
if let Some(cached) = self.cache.get_search(&cache_key) { log::debug!("Cache hit for search: {:?} {}", provider, query);
log::debug!("Cache hit for search: {}", cache_key);
return Ok(cached); return Ok(cached);
} }
@@ -468,7 +458,7 @@ impl RemoteMetadataState {
}; };
// Cache results // Cache results
self.cache.set_search(&cache_key, results.clone()); self.cache.set_search(provider, query, None, artist, results.clone()).await;
Ok(results) Ok(results)
} }
@@ -480,9 +470,8 @@ impl RemoteMetadataState {
provider_id: &str, provider_id: &str,
) -> Result<RemoteAlbumMetadata, RemoteMetadataError> { ) -> Result<RemoteAlbumMetadata, RemoteMetadataError> {
// Check cache first // Check cache first
let cache_key = format!("{:?}:{}", provider, provider_id); if let Some(cached) = self.cache.get_metadata(provider, provider_id).await {
if let Some(cached) = self.cache.get_metadata(&cache_key) { log::debug!("Cache hit for metadata: {:?} {}", provider, provider_id);
log::debug!("Cache hit for metadata: {}", cache_key);
return Ok(cached); return Ok(cached);
} }
@@ -496,7 +485,7 @@ impl RemoteMetadataState {
}; };
// Cache results // Cache results
self.cache.set_metadata(&cache_key, metadata.clone()); self.cache.set_metadata(provider, provider_id, metadata.clone()).await;
Ok(metadata) Ok(metadata)
} }
@@ -509,7 +498,7 @@ impl RemoteMetadataState {
limit: usize, limit: usize,
) -> Result<Vec<RemoteAlbumSearchResult>, RemoteMetadataError> { ) -> Result<Vec<RemoteAlbumSearchResult>, RemoteMetadataError> {
let mb_state = self.musicbrainz.as_ref() 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 let response = mb_state.client
.search_releases_extended(query, artist, None, limit) .search_releases_extended(query, artist, None, limit)
@@ -552,7 +541,7 @@ impl RemoteMetadataState {
release_id: &str, release_id: &str,
) -> Result<RemoteAlbumMetadata, RemoteMetadataError> { ) -> Result<RemoteAlbumMetadata, RemoteMetadataError> {
let mb_state = self.musicbrainz.as_ref() 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 let response = mb_state.client
.get_release_with_tracks(release_id) .get_release_with_tracks(release_id)
@@ -568,7 +557,7 @@ impl RemoteMetadataState {
release_id: &str, release_id: &str,
) -> Result<RemoteAlbumMetadata, RemoteMetadataError> { ) -> Result<RemoteAlbumMetadata, RemoteMetadataError> {
let release_id: u64 = release_id.parse() 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; let client = self.discogs.lock().await;
@@ -2,6 +2,7 @@
import Modal from './Modal.svelte'; import Modal from './Modal.svelte';
import { invoke } from '@tauri-apps/api/core'; import { invoke } from '@tauri-apps/api/core';
import { ask } from '@tauri-apps/plugin-dialog'; import { ask } from '@tauri-apps/plugin-dialog';
import { open } from '@tauri-apps/plugin-opener';
import { showToast } from '$lib/stores/toastStore'; import { showToast } from '$lib/stores/toastStore';
interface LocalTrack { interface LocalTrack {
@@ -195,6 +196,11 @@
if (metadata.catalog_number) catalogNumber = metadata.catalog_number; if (metadata.catalog_number) catalogNumber = metadata.catalog_number;
if (metadata.disc_count) albumTotalDiscs = metadata.disc_count; 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 // Apply track-level metadata if available
if (metadata.tracks && metadata.tracks.length > 0) { if (metadata.tracks && metadata.tracks.length > 0) {
// Match tracks by position // 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; showRemotePanel = false;
} catch (err) { } catch (err) {
console.error('Failed to fetch metadata:', 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 { } finally {
remoteLoading = false; 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 { function parseYear(): number | null {
const trimmed = yearInput.trim(); const trimmed = yearInput.trim();
if (!trimmed) return null; if (!trimmed) return null;
@@ -429,6 +469,20 @@
</div> </div>
<div class="remote-actions"> <div class="remote-actions">
<button
class="btn btn-ghost btn-sm"
onclick={openInBrowser}
disabled={!selectedRemoteId}
type="button"
title="Open in browser"
>
<svg class="icon-inline" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2">
<path d="M18 13v6a2 2 0 0 1-2 2H5a2 2 0 0 1-2-2V8a2 2 0 0 1 2-2h6"/>
<polyline points="15 3 21 3 21 9"/>
<line x1="10" y1="14" x2="21" y2="3"/>
</svg>
Open
</button>
<button <button
class="btn btn-primary btn-sm" class="btn btn-primary btn-sm"
onclick={applyRemoteMetadata} onclick={applyRemoteMetadata}
@@ -915,6 +969,7 @@ input[type="number"] {
.remote-actions { .remote-actions {
display: flex; display: flex;
justify-content: flex-end; justify-content: flex-end;
gap: 8px;
} }
.spinner-inline { .spinner-inline {