Make fetch also fetch artist MBID if it is missing #201

Merged
wojtek merged 13 commits from 191---make-fetch-also-fetch-artist-mbid-if-it-is-missing into main 2024-08-30 17:58:44 +02:00
7 changed files with 136 additions and 80 deletions
Showing only changes of commit 5cca6de0fe - Show all commits

View File

@ -113,7 +113,7 @@ impl IAppInteractBrowse for AppMachine<AppBrowse> {
} }
} }
} }
None => match self.inner.musicbrainz.search_artist(&artist.id.name) { None => match self.inner.musicbrainz.search_artist(artist) {
Ok(list) => matches.push(AppMatchesInfo::artist(artist.clone(), list)), Ok(list) => matches.push(AppMatchesInfo::artist(artist.clone(), list)),
Err(err) => return AppMachine::error(self.inner, err.to_string()).into(), Err(err) => return AppMachine::error(self.inner, err.to_string()).into(),
}, },
@ -130,7 +130,7 @@ impl IAppInteractBrowse for AppMachine<AppBrowse> {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use mockall::{predicate, Sequence}; use mockall::{predicate, Sequence};
use musichoard::collection::{album::Album, musicbrainz::Mbid}; use musichoard::collection::{album::Album, artist::Artist, musicbrainz::Mbid};
use crate::tui::{ use crate::tui::{
app::{ app::{
@ -305,15 +305,52 @@ mod tests {
} }
#[test] #[test]
fn fetch_musicbrainz_no_mbid() { fn fetch_musicbrainz_no_artist_mbid() {
let browse = AppMachine::browse(inner(music_hoard(COLLECTION.to_owned()))); let mut mb_api = Box::new(MockIMusicBrainz::new());
let artist = COLLECTION[3].clone();
let artist_match_1 = Match::new(100, artist.clone());
let artist_match_2 = Match::new(50, artist.clone());
let matches = vec![artist_match_1.clone(), artist_match_2.clone()];
let result: Result<Vec<Match<Artist>>, musicbrainz::Error> = Ok(matches.clone());
mb_api
.expect_search_artist()
.with(predicate::eq(artist.clone()))
.times(1)
.return_once(|_| result);
let browse = AppMachine::browse(inner_with_mb(music_hoard(COLLECTION.to_owned()), mb_api));
// Use the fourth artist for this test as they have no MBID. // Use the fourth artist for this test as they have no MBID.
let browse = browse.increment_selection(Delta::Line).unwrap_browse(); let browse = browse.increment_selection(Delta::Line).unwrap_browse();
let browse = browse.increment_selection(Delta::Line).unwrap_browse(); let browse = browse.increment_selection(Delta::Line).unwrap_browse();
let browse = browse.increment_selection(Delta::Line).unwrap_browse(); let browse = browse.increment_selection(Delta::Line).unwrap_browse();
let app = browse.fetch_musicbrainz(); let mut app = browse.fetch_musicbrainz();
app.unwrap_error();
let public = app.get();
assert!(matches!(public.state, AppState::Matches(_)));
let public_matches = public.state.unwrap_matches();
assert_eq!(
public_matches
.matches
.as_ref()
.unwrap()
.artist_ref()
.matching,
&artist
);
assert_eq!(
public_matches.matches.as_ref().unwrap().artist_ref().list,
matches.as_slice()
);
let app = app.unwrap_matches().select();
app.unwrap_browse();
} }
#[test] #[test]

View File

@ -82,20 +82,6 @@ impl AppMatchesInfo {
} }
} }
fn artist_ref(&self) -> &AppArtistMatchesInfo {
match self {
Self::Artist(a) => a,
Self::Album(_) => panic!(),
}
}
fn album_ref(&self) -> &AppAlbumMatchesInfo {
match self {
Self::Album(a) => a,
Self::Artist(_) => panic!(),
}
}
pub fn artist(matching: Artist, list: Vec<Match<Artist>>) -> Self { pub fn artist(matching: Artist, list: Vec<Match<Artist>>) -> Self {
AppMatchesInfo::Artist(AppArtistMatchesInfo { matching, list }) AppMatchesInfo::Artist(AppArtistMatchesInfo { matching, list })
} }
@ -227,6 +213,15 @@ mod tests {
use super::*; use super::*;
impl AppMatchesInfo {
fn album_ref(&self) -> &AppAlbumMatchesInfo {
match self {
Self::Album(a) => a,
Self::Artist(_) => panic!(),
}
}
}
fn matches_info_vec() -> Vec<AppMatchesInfo> { fn matches_info_vec() -> Vec<AppMatchesInfo> {
let album_1 = Album::new( let album_1 = Album::new(
AlbumId::new("Album 1"), AlbumId::new("Album 1"),

View File

@ -147,22 +147,6 @@ pub enum AppPublicMatchesKind<'app> {
Album(AppPublicAlbumMatches<'app>), Album(AppPublicAlbumMatches<'app>),
} }
impl<'app> AppPublicMatchesKind<'app> {
pub fn artist_ref(&self) -> &AppPublicArtistMatches<'app> {
match self {
AppPublicMatchesKind::Artist(m) => m,
_ => panic!(),
}
}
pub fn album_ref(&self) -> &AppPublicAlbumMatches<'app> {
match self {
AppPublicMatchesKind::Album(m) => m,
_ => panic!(),
}
}
}
pub struct AppPublicMatches<'app> { pub struct AppPublicMatches<'app> {
pub matches: Option<AppPublicMatchesKind<'app>>, pub matches: Option<AppPublicMatchesKind<'app>>,
pub state: &'app mut WidgetState, pub state: &'app mut WidgetState,
@ -181,6 +165,22 @@ impl<BS, IS, RS, SS, MS, ES, CS> AppState<BS, IS, RS, SS, MS, ES, CS> {
mod tests { mod tests {
use super::*; use super::*;
impl<'app> AppPublicMatchesKind<'app> {
pub fn artist_ref(&self) -> &AppPublicArtistMatches<'app> {
match self {
AppPublicMatchesKind::Artist(m) => m,
_ => panic!(),
}
}
pub fn album_ref(&self) -> &AppPublicAlbumMatches<'app> {
match self {
AppPublicMatchesKind::Album(m) => m,
_ => panic!(),
}
}
}
#[test] #[test]
fn app_is_state() { fn app_is_state() {
let state = AppPublicState::Search("get rekt"); let state = AppPublicState::Search("get rekt");

View File

@ -32,11 +32,8 @@ impl<Http> MusicBrainz<Http> {
} }
impl<Http: IMusicBrainzHttp> IMusicBrainz for MusicBrainz<Http> { impl<Http: IMusicBrainzHttp> IMusicBrainz for MusicBrainz<Http> {
fn search_artist( fn search_artist(&mut self, artist: &Artist) -> Result<Vec<Match<Artist>>, Error> {
&mut self, let query = SearchArtistRequest::new().string(&artist.id.name);
name: &str,
) -> Result<Vec<Match<musichoard::collection::artist::Artist>>, Error> {
let query = SearchArtistRequest::new().string(name);
let mb_response = self.client.search_artist(query)?; let mb_response = self.client.search_artist(query)?;

View File

@ -8,7 +8,7 @@ use musichoard::collection::{album::Album, artist::Artist, musicbrainz::Mbid};
/// Trait for interacting with the MusicBrainz API. /// Trait for interacting with the MusicBrainz API.
#[cfg_attr(test, automock)] #[cfg_attr(test, automock)]
pub trait IMusicBrainz { pub trait IMusicBrainz {
fn search_artist(&mut self, name: &str) -> Result<Vec<Match<Artist>>, Error>; fn search_artist(&mut self, name: &Artist) -> Result<Vec<Match<Artist>>, Error>;
fn search_release_group( fn search_release_group(
&mut self, &mut self,
arid: &Mbid, arid: &Mbid,

View File

@ -1,22 +1,40 @@
use musichoard::collection::{album::Album, artist::Artist}; use musichoard::collection::{album::Album, artist::Artist};
use ratatui::widgets::{List, ListItem}; use ratatui::widgets::{List, ListItem};
use crate::tui::{app::WidgetState, lib::interface::musicbrainz::Match, ui::display::UiDisplay}; use crate::tui::{
app::{AppPublicMatchesKind, WidgetState},
lib::interface::musicbrainz::Match,
ui::display::UiDisplay,
};
pub struct MatchesState<'a, 'b> { pub struct MatchesState<'a, 'b> {
pub matching: String,
pub list: List<'a>, pub list: List<'a>,
pub state: &'b mut WidgetState, pub state: &'b mut WidgetState,
} }
impl<'a, 'b> MatchesState<'a, 'b> { impl<'a, 'b> MatchesState<'a, 'b> {
pub fn empty(state: &'b mut WidgetState) -> Self { pub fn new(matches: Option<AppPublicMatchesKind>, state: &'b mut WidgetState) -> Self {
match matches {
Some(kind) => match kind {
AppPublicMatchesKind::Artist(m) => Self::artists(m.matching, m.list, state),
AppPublicMatchesKind::Album(m) => Self::albums(m.matching, m.list, state),
},
None => Self::empty(state),
}
}
fn empty(state: &'b mut WidgetState) -> Self {
MatchesState { MatchesState {
matching: UiDisplay::display_matching_nothing_info().to_string(),
list: List::default(), list: List::default(),
state, state,
} }
} }
pub fn artists(matches: &[Match<Artist>], state: &'b mut WidgetState) -> Self { fn artists(matching: &Artist, matches: &[Match<Artist>], state: &'b mut WidgetState) -> Self {
let matching = UiDisplay::display_artist_matching_info(matching);
let list = List::new( let list = List::new(
matches matches
.iter() .iter()
@ -25,10 +43,16 @@ impl<'a, 'b> MatchesState<'a, 'b> {
.collect::<Vec<ListItem>>(), .collect::<Vec<ListItem>>(),
); );
MatchesState { list, state } MatchesState {
matching,
list,
state,
}
} }
pub fn albums(matches: &[Match<Album>], state: &'b mut WidgetState) -> Self { fn albums(matching: &Album, matches: &[Match<Album>], state: &'b mut WidgetState) -> Self {
let matching = UiDisplay::display_album_matching_info(matching);
let list = List::new( let list = List::new(
matches matches
.iter() .iter()
@ -37,6 +61,10 @@ impl<'a, 'b> MatchesState<'a, 'b> {
.collect::<Vec<ListItem>>(), .collect::<Vec<ListItem>>(),
); );
MatchesState { list, state } MatchesState {
matching,
list,
state,
}
} }
} }

View File

@ -139,23 +139,8 @@ impl Ui {
frame: &mut Frame, frame: &mut Frame,
) { ) {
let area = OverlayBuilder::default().build(frame.size()); let area = OverlayBuilder::default().build(frame.size());
let (matching, st) = match matches { let st = MatchesState::new(matches, state);
Some(kind) => match kind { UiWidget::render_overlay_list_widget(&st.matching, st.list, st.state, true, area, frame)
AppPublicMatchesKind::Artist(m) => (
UiDisplay::display_artist_matching_info(m.matching),
MatchesState::artists(m.list, state),
),
AppPublicMatchesKind::Album(m) => (
UiDisplay::display_album_matching_info(m.matching),
MatchesState::albums(m.list, state),
),
},
None => (
UiDisplay::display_matching_nothing_info().to_string(),
MatchesState::empty(state),
),
};
UiWidget::render_overlay_list_widget(&matching, st.list, st.state, true, area, frame)
} }
fn render_error_overlay<S: AsRef<str>>(title: S, msg: S, frame: &mut Frame) { fn render_error_overlay<S: AsRef<str>>(title: S, msg: S, frame: &mut Frame) {
@ -210,8 +195,35 @@ mod tests {
use super::*; use super::*;
impl<'app> AppPublicArtistMatches<'app> {
fn get(&self) -> AppPublicArtistMatches<'app> {
AppPublicArtistMatches {
matching: self.matching,
list: self.list,
}
}
}
impl<'app> AppPublicAlbumMatches<'app> {
fn get(&self) -> AppPublicAlbumMatches<'app> {
AppPublicAlbumMatches {
matching: self.matching,
list: self.list,
}
}
}
impl<'app> AppPublicMatchesKind<'app> {
fn get(&self) -> AppPublicMatchesKind<'app> {
match self {
Self::Artist(a) => Self::Artist(a.get()),
Self::Album(a) => Self::Album(a.get()),
}
}
}
// Automock does not support returning types with generic lifetimes. // Automock does not support returning types with generic lifetimes.
impl IAppAccess for AppPublic<'_> { impl<'app> IAppAccess for AppPublic<'app> {
fn get(&mut self) -> AppPublic { fn get(&mut self) -> AppPublic {
AppPublic { AppPublic {
inner: AppPublicInner { inner: AppPublicInner {
@ -224,20 +236,7 @@ mod tests {
AppState::Reload(()) => AppState::Reload(()), AppState::Reload(()) => AppState::Reload(()),
AppState::Search(s) => AppState::Search(s), AppState::Search(s) => AppState::Search(s),
AppState::Matches(ref mut m) => AppState::Matches(AppPublicMatches { AppState::Matches(ref mut m) => AppState::Matches(AppPublicMatches {
matches: m.matches.as_ref().map(|k| match k { matches: m.matches.as_mut().map(|k| k.get()),
AppPublicMatchesKind::Artist(a) => {
AppPublicMatchesKind::Artist(AppPublicArtistMatches {
matching: a.matching,
list: a.list,
})
}
AppPublicMatchesKind::Album(a) => {
AppPublicMatchesKind::Album(AppPublicAlbumMatches {
matching: a.matching,
list: a.list,
})
}
}),
state: m.state, state: m.state,
}), }),
AppState::Error(s) => AppState::Error(s), AppState::Error(s) => AppState::Error(s),