Fetch panics if a list shortens with a selection beyond the new length (#258)
Closes #257 Reviewed-on: #258
This commit is contained in:
parent
ffd0d4d82f
commit
54b170a5b5
@ -6,13 +6,11 @@ use musichoard::collection::{
|
||||
musicbrainz::{MbRefOption, Mbid},
|
||||
};
|
||||
|
||||
use crate::tui::{
|
||||
app::{
|
||||
machine::{fetch_state::FetchState, input::Input, App, AppInner, AppMachine},
|
||||
AlbumMatches, AppPublicState, AppState, ArtistMatches, Delta, EntityMatches,
|
||||
IAppInteractMatch, MatchOption, MatchStatePublic, WidgetState,
|
||||
},
|
||||
lib::IMusicHoard,
|
||||
use crate::tui::app::{
|
||||
machine::{fetch_state::FetchState, input::Input, App, AppInner, AppMachine},
|
||||
selection::KeySelection,
|
||||
AlbumMatches, AppPublicState, AppState, ArtistMatches, Delta, EntityMatches, IAppInteractMatch,
|
||||
MatchOption, MatchStatePublic, WidgetState,
|
||||
};
|
||||
|
||||
struct ArtistInfoTuple {
|
||||
@ -205,20 +203,21 @@ impl AppMachine<MatchState> {
|
||||
}
|
||||
|
||||
fn select_artist(
|
||||
mh: &mut Box<dyn IMusicHoard>,
|
||||
inner: &mut AppInner,
|
||||
matches: &ArtistMatches,
|
||||
tuple: ArtistInfoTuple,
|
||||
) -> Result<(), musichoard::Error> {
|
||||
let mh = &mut inner.music_hoard;
|
||||
mh.merge_artist_info(&matches.matching.id, tuple.info)?;
|
||||
mh.set_artist_mb_ref(&matches.matching.id, tuple.mb_ref)
|
||||
}
|
||||
|
||||
fn select_album(
|
||||
mh: &mut Box<dyn IMusicHoard>,
|
||||
inner: &mut AppInner,
|
||||
matches: &AlbumMatches,
|
||||
tuple: AlbumInfoTuple,
|
||||
) -> Result<(), musichoard::Error> {
|
||||
let coll = mh.get_collection();
|
||||
let coll = inner.music_hoard.get_collection();
|
||||
let mut clashing = vec![];
|
||||
if let Some(artist) = coll.iter().find(|artist| artist.meta.id == matches.artist) {
|
||||
// While we expect only one, there is nothing stopping anybody from having multiple
|
||||
@ -231,8 +230,16 @@ impl AppMachine<MatchState> {
|
||||
}
|
||||
|
||||
for album in clashing.into_iter() {
|
||||
mh.remove_album(&matches.artist, &album)?;
|
||||
let coll = inner.music_hoard.get_filtered();
|
||||
let selection = KeySelection::get(coll, &inner.selection);
|
||||
|
||||
inner.music_hoard.remove_album(&matches.artist, &album)?;
|
||||
|
||||
let coll = inner.music_hoard.get_filtered();
|
||||
inner.selection.select_by_id(coll, selection);
|
||||
}
|
||||
|
||||
let mh = &mut inner.music_hoard;
|
||||
mh.merge_album_info(&matches.artist, &matches.matching, tuple.info)?;
|
||||
mh.set_album_mb_ref(&matches.artist, &matches.matching, tuple.mb_ref)
|
||||
}
|
||||
@ -279,14 +286,14 @@ impl IAppInteractMatch for AppMachine<MatchState> {
|
||||
fn select(mut self) -> Self::APP {
|
||||
let index = self.state.state.list.selected().unwrap();
|
||||
|
||||
let mh = &mut self.inner.music_hoard;
|
||||
let inner = &mut self.inner;
|
||||
let result = match self.state.current {
|
||||
EntityMatches::Artist(ref mut matches) => match matches.list.extract_info(index) {
|
||||
InfoOption::Info(tuple) => Self::select_artist(mh, matches, tuple),
|
||||
InfoOption::Info(tuple) => Self::select_artist(inner, matches, tuple),
|
||||
InfoOption::NeedInput => return self.get_input(),
|
||||
},
|
||||
EntityMatches::Album(ref mut matches) => match matches.list.extract_info(index) {
|
||||
InfoOption::Info(tuple) => Self::select_album(mh, matches, tuple),
|
||||
InfoOption::Info(tuple) => Self::select_album(inner, matches, tuple),
|
||||
InfoOption::NeedInput => return self.get_input(),
|
||||
},
|
||||
};
|
||||
@ -323,9 +330,12 @@ mod tests {
|
||||
machine::tests::{inner, inner_with_mb, input_event, music_hoard},
|
||||
IApp, IAppAccess, IAppInput,
|
||||
},
|
||||
lib::interface::musicbrainz::{
|
||||
api::Entity,
|
||||
daemon::{MbParams, MockIMbJobSender},
|
||||
lib::{
|
||||
interface::musicbrainz::{
|
||||
api::Entity,
|
||||
daemon::{MbParams, MockIMbJobSender},
|
||||
},
|
||||
MockIMusicHoard,
|
||||
},
|
||||
};
|
||||
|
||||
@ -653,14 +663,34 @@ mod tests {
|
||||
// (3) An album with a different album_id than the selected one.
|
||||
// (4) This album has an MBID that is identical to that of the selected match.
|
||||
let mb_ref = AlbumMbRef::Some(mbid().into());
|
||||
let removed = Album::new(AlbumId::new("Not the Same").with_mb_ref(mb_ref));
|
||||
let removed = Album::new(AlbumId::new("Album: Not the Same").with_mb_ref(mb_ref.clone()));
|
||||
artist.albums.push(removed.clone());
|
||||
|
||||
// (5) An album after the one that will be removed. Selection must remain on it.
|
||||
let selected = Album::new(AlbumId::new("Album: Z"));
|
||||
artist.albums.push(selected.clone());
|
||||
|
||||
let (_tx, rx) = mpsc::channel();
|
||||
let app_matches = MatchState::new(matches_info.clone(), FetchState::search(rx));
|
||||
|
||||
let collection = vec![artist];
|
||||
let mut music_hoard = music_hoard(collection.clone());
|
||||
let mut reduced = collection.clone();
|
||||
reduced[0].albums.retain(|a| a.meta.id.mb_ref != mb_ref);
|
||||
|
||||
// We need a custom music_hoard to override the get_filtered expectations.
|
||||
let mut music_hoard = MockIMusicHoard::new();
|
||||
let mut seq = Sequence::new();
|
||||
music_hoard
|
||||
.expect_get_filtered()
|
||||
.times(2)
|
||||
.in_sequence(&mut seq)
|
||||
.return_const(collection.clone());
|
||||
music_hoard
|
||||
.expect_get_filtered()
|
||||
.times(1)
|
||||
.in_sequence(&mut seq)
|
||||
.return_const(reduced.clone());
|
||||
|
||||
match matches_info {
|
||||
EntityMatches::Artist(_) => panic!(),
|
||||
EntityMatches::Album(matches) => {
|
||||
@ -671,7 +701,7 @@ mod tests {
|
||||
.expect_get_collection()
|
||||
.times(1)
|
||||
.in_sequence(&mut seq)
|
||||
.return_const(collection);
|
||||
.return_const(collection.clone());
|
||||
music_hoard
|
||||
.expect_remove_album()
|
||||
.times(1)
|
||||
@ -691,8 +721,26 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
let matches = AppMachine::match_state(inner(music_hoard), app_matches);
|
||||
matches.select().unwrap_fetch();
|
||||
let mut matches = AppMachine::match_state(inner(music_hoard), app_matches);
|
||||
|
||||
// Make sure the last album is selected.
|
||||
let selection = &mut matches.inner.selection;
|
||||
selection.increment_category();
|
||||
selection.increment_selection(&collection, Delta::Line);
|
||||
selection.increment_selection(&collection, Delta::Line);
|
||||
|
||||
// Check that the correct album is selected.
|
||||
let state = selection.state_album(&collection).unwrap();
|
||||
let album = &collection[0].albums[state.index];
|
||||
assert_eq!(album.meta, selected.meta);
|
||||
|
||||
let mut fetch = matches.select().unwrap_fetch();
|
||||
|
||||
// Check that the correct album is still selected in the reduced collection.
|
||||
let selection = &mut fetch.inner.selection;
|
||||
let state = selection.state_album(&reduced).unwrap();
|
||||
let album = &reduced[0].albums[state.index];
|
||||
assert_eq!(album.meta, selected.meta);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
Loading…
x
Reference in New Issue
Block a user