Skip to content

Commit

Permalink
fix #584 show albums and tracks when filtering for genres via search…
Browse files Browse the repository at this point in the history
… text (#596)

* fix #584 show albums and tracks when filtering for genres via search text

* refactor filter-pipes for tracks and albums
  • Loading branch information
dermeck committed Apr 11, 2024
1 parent 7a32a39 commit eb49a34
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 152 deletions.
1 change: 1 addition & 0 deletions src/app/data/entities/album-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export class AlbumData {
public albumKey: string;
public artworkId: string | undefined;
public year: number | undefined;
public genres: string | undefined;
public dateFileCreated: number | undefined;
public dateAdded: number | undefined;
public dateLastPlayed: number | undefined;
Expand Down
1 change: 1 addition & 0 deletions src/app/data/query-parts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export class QueryParts {
a.ArtworkID as artworkId,
MAX(t.Artists) AS artists,
MAX(t.Year) AS year,
GROUP_CONCAT(distinct t.Genres) AS genres,
MAX(t.DateFileCreated) AS dateFileCreated,
MAX(t.DateAdded) AS dateAdded,
MAX(t.DateLastPlayed) AS dateLastPlayed FROM Track t
Expand Down
4 changes: 4 additions & 0 deletions src/app/services/album/album-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ export class AlbumModel implements ISelectable {
return this.albumData.year ?? 0;
}

public get genres(): string[] {
return DataDelimiter.fromDelimitedString(this.albumData.genres);
}

public get albumKey(): string {
return this.albumData.albumKey ?? '';
}
Expand Down
52 changes: 13 additions & 39 deletions src/app/ui/pipes/albums-filter.pipe.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { IMock, It, Mock } from 'typemoq';
import { IMock, It, Mock, Times } from 'typemoq';
import { AlbumsFilterPipe } from './albums-filter.pipe';
import { SearchServiceBase } from '../../services/search/search.service.base';
import { TranslatorServiceBase } from '../../services/translator/translator.service.base';
Expand All @@ -22,11 +22,13 @@ describe('AlbumsFilterPipe', () => {
albumData1.albumTitle = 'album_title1';
albumData1.albumArtists = ';album_artist1_1;;album_artist1_2;';
albumData1.year = 2001;
albumData1.genres = ';genre1_1;genre1_2';
const album1: AlbumModel = new AlbumModel(albumData1, translatorServiceMock.object, applicationPathsMock.object);
const albumData2: AlbumData = new AlbumData();
albumData2.albumTitle = 'album_title2';
albumData2.albumArtists = ';album_artist2_1;;album_artist2_2;';
albumData2.year = 2002;
albumData2.genres = ';genre2_1;genre2_2';
const album2: AlbumModel = new AlbumModel(albumData2, translatorServiceMock.object, applicationPathsMock.object);
return [album1, album2];
}
Expand Down Expand Up @@ -75,54 +77,26 @@ describe('AlbumsFilterPipe', () => {
expect(filteredAlbums).toEqual(albums);
});

it('should return only albums with a title containing the search text', () => {
it('performs search once for each album, searching "title", "artist", "year" and "genres', () => {
// Arrange
searchServiceMock.setup((x) => x.matchesSearchText('album_title1', It.isAny())).returns(() => true);
searchServiceMock.setup((x) => x.matchesSearchText('album_title2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('album_artist1_1', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('album_artist2_1', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('2001', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('2002', It.isAny())).returns(() => false);

const albums: AlbumModel[] = createAlbumModels();
const pipe: AlbumsFilterPipe = createPipe();

// Act
const filteredAlbums: AlbumModel[] = pipe.transform(albums, 'dummy');
pipe.transform(albums, 'dummy');

// Assert
expect(filteredAlbums.length).toEqual(1);
expect(filteredAlbums[0]).toEqual(albums[0]);
});

it('should return only albums with album artists containing the search text', () => {
// Arrange
searchServiceMock.setup((x) => x.matchesSearchText('album_title1', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('album_title2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('album_artist1_1', It.isAny())).returns(() => true);
searchServiceMock.setup((x) => x.matchesSearchText('album_artist2_1', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('2001', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('2002', It.isAny())).returns(() => false);

const albums: AlbumModel[] = createAlbumModels();
const pipe: AlbumsFilterPipe = createPipe();
const expectedTextToSearchAlbum1 = 'album_title1 album_artist1_1 2001 genre1_1 genre1_2';
const expectedTextToSearchAlbum2 = 'album_title2 album_artist2_1 2002 genre2_1 genre2_2';

// Act
const filteredAlbums: AlbumModel[] = pipe.transform(albums, 'dummy');

// Assert
expect(filteredAlbums.length).toEqual(1);
expect(filteredAlbums[0]).toEqual(filteredAlbums[0]);
searchServiceMock.verify((x) => x.matchesSearchText(expectedTextToSearchAlbum1, 'dummy'), Times.once());
searchServiceMock.verify((x) => x.matchesSearchText(expectedTextToSearchAlbum2, 'dummy'), Times.once());
});

it('should return only albums with a year containing the search text', () => {
it('should return only albums for which search returns true', () => {
// Arrange
searchServiceMock.setup((x) => x.matchesSearchText('album_title1', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('album_title2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('album_artist1_1', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('album_artist2_1', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('2001', It.isAny())).returns(() => true);
searchServiceMock.setup((x) => x.matchesSearchText('2002', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText(It.isAny(), It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText(It.isAny(), It.isAny())).returns(() => true);

const albums: AlbumModel[] = createAlbumModels();
const pipe: AlbumsFilterPipe = createPipe();
Expand All @@ -132,7 +106,7 @@ describe('AlbumsFilterPipe', () => {

// Assert
expect(filteredAlbums.length).toEqual(1);
expect(filteredAlbums[0]).toEqual(filteredAlbums[0]);
expect(filteredAlbums[0]).toEqual(albums[1]);
});
});
});
7 changes: 2 additions & 5 deletions src/app/ui/pipes/albums-filter.pipe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,8 @@ export class AlbumsFilterPipe implements PipeTransform {
const filteredAlbums: AlbumModel[] = [];

for (const album of albums) {
if (this.searchService.matchesSearchText(album.albumTitle, textToContain!)) {
filteredAlbums.push(album);
} else if (this.searchService.matchesSearchText(album.albumArtist, textToContain!)) {
filteredAlbums.push(album);
} else if (this.searchService.matchesSearchText(album.year.toString(), textToContain!)) {
const textToSearch = [album.albumTitle, album.albumArtist, album.year.toString(), album.genres.join(' ')].join(' ');
if (this.searchService.matchesSearchText(textToSearch, textToContain!)) {
filteredAlbums.push(album);
}
}
Expand Down
114 changes: 15 additions & 99 deletions src/app/ui/pipes/tracks-filter.pipe.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { IMock, It, Mock } from 'typemoq';
import { IMock, It, Mock, Times } from 'typemoq';
import { TracksFilterPipe } from './tracks-filter.pipe';
import { TranslatorServiceBase } from '../../services/translator/translator.service.base';
import { SearchServiceBase } from '../../services/search/search.service.base';
Expand All @@ -23,12 +23,14 @@ describe('TracksFilterPipe', () => {
track1.albumArtists = ';album_artist1_1;;album_artist1_2;';
track1.artists = ';artist1_1;;artist1_2;';
track1.year = 2001;
track1.genres = ';genre1_1;;genre1_2';
const track2: Track = new Track('/path2/file2.mp3');
track2.fileName = 'file2.mp3';
track2.trackTitle = 'title2';
track2.albumArtists = ';album_artist2_1;;album_artist2_2;';
track2.artists = ';artist2_1;;artist2_2;';
track2.year = 2002;
track2.genres = ';genre2_1;;genre2_2;';
const trackModel1: TrackModel = new TrackModel(track1, dateTimeMock.object, translatorServiceMock.object);
const trackModel2: TrackModel = new TrackModel(track2, dateTimeMock.object, translatorServiceMock.object);
const trackModels: TrackModels = new TrackModels();
Expand Down Expand Up @@ -81,114 +83,28 @@ describe('TracksFilterPipe', () => {
expect(filteredTrackModels).toEqual(trackModels);
});

it('should return only trackModels with a title containing the search text', () => {
it('performs search once for each track, searching "title", "album artists", "artists", "fileName", "year" and "genres', () => {
// Arrange
searchServiceMock.setup((x) => x.matchesSearchText('title1', It.isAny())).returns(() => true);
searchServiceMock.setup((x) => x.matchesSearchText('title2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('album_artist1_1, album_artist1_2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('album_artist2_1, album_artist2_2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('artist1_1, artist1_2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('artist2_1, artist2_2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('file1.mp3', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('file2.mp3', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('2001', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('2002', It.isAny())).returns(() => false);

const trackModels: TrackModels = createTrackModels();
const pipe: TracksFilterPipe = createPipe();

// Act
const filteredTrackModels: TrackModels = pipe.transform(trackModels, 'dummy');

// Assert
expect(filteredTrackModels.tracks.length).toEqual(1);
expect(filteredTrackModels.tracks[0]).toEqual(trackModels.tracks[0]);
});

it('should return only trackModels with album artists containing the search text', () => {
// Arrange
searchServiceMock.setup((x) => x.matchesSearchText('title1', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('title2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('album_artist1_1, album_artist1_2', It.isAny())).returns(() => true);
searchServiceMock.setup((x) => x.matchesSearchText('album_artist2_1, album_artist2_2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('artist1_1, artist1_2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('artist2_1, artist2_2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('file1.mp3', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('file2.mp3', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('2001', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('2002', It.isAny())).returns(() => false);

const trackModels: TrackModels = createTrackModels();
const pipe: TracksFilterPipe = createPipe();

// Act
const filteredTrackModels: TrackModels = pipe.transform(trackModels, 'dummy');

// Assert
expect(filteredTrackModels.tracks.length).toEqual(1);
expect(filteredTrackModels.tracks[0]).toEqual(trackModels.tracks[0]);
});

it('should return only trackModels with artists containing the search text', () => {
// Arrange
searchServiceMock.setup((x) => x.matchesSearchText('title1', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('title2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('album_artist1_1, album_artist1_2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('album_artist2_1, album_artist2_2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('artist1_1, artist1_2', It.isAny())).returns(() => true);
searchServiceMock.setup((x) => x.matchesSearchText('artist2_1, artist2_2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('file1.mp3', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('file2.mp3', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('2001', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('2002', It.isAny())).returns(() => false);

const trackModels: TrackModels = createTrackModels();
const pipe: TracksFilterPipe = createPipe();

// Act
const filteredTrackModels: TrackModels = pipe.transform(trackModels, 'dummy');
pipe.transform(trackModels, 'dummy');

// Assert
expect(filteredTrackModels.tracks.length).toEqual(1);
expect(filteredTrackModels.tracks[0]).toEqual(trackModels.tracks[0]);
});
const expectedTextToSearchTrack1 =
'title1 album_artist1_1, album_artist1_2 artist1_1, artist1_2 file1.mp3 2001 genre1_1, genre1_2';
const expectedTextToSearchTrack2 =
'title2 album_artist2_1, album_artist2_2 artist2_1, artist2_2 file2.mp3 2002 genre2_1, genre2_2';

it('should return only trackModels with a file name containing the search text', () => {
// Arrange
searchServiceMock.setup((x) => x.matchesSearchText('title1', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('title2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('album_artist1_1, album_artist1_2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('album_artist2_1, album_artist2_2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('artist1_1, artist1_2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('artist2_1, artist2_2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('file1.mp3', It.isAny())).returns(() => true);
searchServiceMock.setup((x) => x.matchesSearchText('file2.mp3', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('2001', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('2002', It.isAny())).returns(() => false);

const trackModels: TrackModels = createTrackModels();
const pipe: TracksFilterPipe = createPipe();

// Act
const filteredTrackModels: TrackModels = pipe.transform(trackModels, 'dummy');

// Assert
expect(filteredTrackModels.tracks.length).toEqual(1);
expect(filteredTrackModels.tracks[0]).toEqual(trackModels.tracks[0]);
searchServiceMock.verify((x) => x.matchesSearchText(expectedTextToSearchTrack1, 'dummy'), Times.once());
searchServiceMock.verify((x) => x.matchesSearchText(expectedTextToSearchTrack2, 'dummy'), Times.once());
});

it('should return only trackModels with a year containing the search text', () => {
it('should return only tracks for which search returns true', () => {
// Arrange
searchServiceMock.setup((x) => x.matchesSearchText('title1', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('title2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('album_artist1_1, album_artist1_2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('album_artist2_1, album_artist2_2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('artist1_1, artist1_2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('artist2_1, artist2_2', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('file1.mp3', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('file2.mp3', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText('2001', It.isAny())).returns(() => true);
searchServiceMock.setup((x) => x.matchesSearchText('2002', It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText(It.isAny(), It.isAny())).returns(() => false);
searchServiceMock.setup((x) => x.matchesSearchText(It.isAny(), It.isAny())).returns(() => true);

const trackModels: TrackModels = createTrackModels();
const pipe: TracksFilterPipe = createPipe();
Expand All @@ -198,7 +114,7 @@ describe('TracksFilterPipe', () => {

// Assert
expect(filteredTrackModels.tracks.length).toEqual(1);
expect(filteredTrackModels.tracks[0]).toEqual(trackModels.tracks[0]);
expect(filteredTrackModels.tracks[0]).toEqual(trackModels.tracks[1]);
});
});
});
14 changes: 5 additions & 9 deletions src/app/ui/pipes/tracks-filter.pipe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,11 @@ export class TracksFilterPipe implements PipeTransform {
const filteredTracks: TrackModels = new TrackModels();

for (const track of tracks.tracks) {
if (this.searchService.matchesSearchText(track.title, textToContain!)) {
filteredTracks.addTrack(track);
} else if (this.searchService.matchesSearchText(track.albumArtists, textToContain!)) {
filteredTracks.addTrack(track);
} else if (this.searchService.matchesSearchText(track.artists, textToContain!)) {
filteredTracks.addTrack(track);
} else if (this.searchService.matchesSearchText(track.fileName, textToContain!)) {
filteredTracks.addTrack(track);
} else if (this.searchService.matchesSearchText(track.year.toString(), textToContain!)) {
const textToSearch = [track.title, track.albumArtists, track.artists, track.fileName, track.year.toString(), track.genres].join(
' ',
);

if (this.searchService.matchesSearchText(textToSearch, textToContain!)) {
filteredTracks.addTrack(track);
}
}
Expand Down

0 comments on commit eb49a34

Please sign in to comment.