Merge branch 'master' into feature1499

.github/workflows/review.yml

@@ -1,6 +1,6 @@
 name: Code Review
 on:
-  pull_request:
+  pull_request_target:
       types: [opened, review_requested]
 
 jobs:

packages/app/app/components/Settings/index.tsx

@@ -1,5 +1,5 @@
 import React, { useEffect } from 'react';
-import { remote } from 'electron';
+import { ipcRenderer, remote } from 'electron';
 import { Button, Input, Radio, Segment, Icon } from 'semantic-ui-react';
 import cx from 'classnames';
 import _ from 'lodash';
@@ -113,7 +113,16 @@ const Settings: React.FC<SettingsProps> = ({
       placeholder={t(placeholder)}
       value={i18n.language}
       options={options}
-      onChange={(e, { value }) => i18n.changeLanguage(value as string)}
+      onChange={(e, { value }) => {
+        i18n.changeLanguage(value as string);
+        ipcRenderer.send('tray-menu-translations-update', {
+          'play': i18n.t('command-palette:actions.play'),
+          'pause': i18n.t('command-palette:actions.pause'),
+          'next': i18n.t('command-palette:actions.next'),
+          'previous': i18n.t('command-palette:actions.previous'),
+          'quit': i18n.t('command-palette:actions.quit')
+        });
+      }}
     />
   );
 

packages/core/src/plugins/meta/discogs.test.ts

@@ -0,0 +1,166 @@
+import DiscogsMetaProvider from './discogs';
+import { SimilarArtist } from '../plugins.types';
+import * as SpotifyApi from '../../rest/Spotify';
+import { SpotifyArtist } from '../../rest/Spotify';
+import { LastFmArtistInfo, LastfmArtistShort } from '../../rest/Lastfm.types';
+
+describe('Tests for DiscogsMetaProvider', () => {
+  const provider = new DiscogsMetaProvider();
+
+  describe('Verify if an artist is on tour', () => {
+
+    test('Should return false if the artist info is missing', () => {
+      expect(provider.isArtistOnTour(undefined)).toBeFalsy();
+    });
+
+    test('Should return false if the \'ontour\' flag is not \'1\'', () => {
+      const artistInfo: any = {
+        ontour: 'something else than 1'
+      };
+      expect(provider.isArtistOnTour(artistInfo)).toBeFalsy();
+    });
+
+    test('Should return false if the \'ontour\' flag is \'1\'', () => {
+      const artistInfo: any = {
+        ontour: '1'
+      };
+      expect(provider.isArtistOnTour(artistInfo)).toBeTruthy();
+    });
+  });
+
+  describe('Get similar artists', () => {
+    const spotifyToken = 'spotify token';
+    const getToken = jest.spyOn(SpotifyApi, 'getToken');
+    const fetchTopSimilarArtistsFromSpotify = jest.spyOn(provider, 'fetchTopSimilarArtistsFromSpotify');
+
+    afterEach(() => {
+      getToken.mockReset();
+      fetchTopSimilarArtistsFromSpotify.mockReset();
+    });
+
+    afterAll(() => {
+      getToken.mockRestore();
+      fetchTopSimilarArtistsFromSpotify.mockRestore();
+    });
+
+    test('Should return an empty result set if the artist info is missing', async () => {
+      expect(await provider.getSimilarArtists({
+        similar: {
+          artist: null
+        }
+      } as LastFmArtistInfo)).toEqual([]);
+    });
+
+    test('Should fetch similar artist', async () => {
+      const similarArtistsOnLastFm = [{
+        name: 'Similar Artist on LastFm'
+      }];
+      const artistInfoFromLastFm = {
+        name: 'Artist Name',
+        similar: {
+          artist: similarArtistsOnLastFm
+        }
+      } as LastFmArtistInfo;
+
+      getToken.mockResolvedValueOnce(spotifyToken);
+
+      const similarArtist: SimilarArtist = {
+        name: 'Similar Artist Name',
+        thumbnail: 'the thumbnail'
+      };
+      fetchTopSimilarArtistsFromSpotify.mockImplementationOnce((artists, token) => {
+        expect(token).toEqual(spotifyToken);
+        expect(artists).toEqual(similarArtistsOnLastFm);
+        return Promise.resolve([similarArtist]);
+      });
+      expect(await provider.getSimilarArtists(artistInfoFromLastFm)).toEqual([similarArtist]);
+    });
+
+    test('Should return empty result set if fetching the Spotify token fails', async () => {
+      getToken.mockRejectedValueOnce('Failed to fetch Spotify token');
+      expect(await provider.getSimilarArtists({
+        similar: {
+          artist: []
+        }
+      } as LastFmArtistInfo)).toEqual([]);
+    });
+
+    test('Should return empty result set if fetching the artists from Spotify fails', async () => {
+      getToken.mockResolvedValueOnce(spotifyToken);
+      fetchTopSimilarArtistsFromSpotify.mockRejectedValueOnce('Failed to fetch similar artists');
+      expect(await provider.getSimilarArtists({
+        similar: {
+          artist: []
+        }
+      } as LastFmArtistInfo)).toEqual([]);
+    });
+  });
+
+  describe('Fetch top similar artists from Spotify', () => {
+
+    test('Should fetch similar artists from Spotify', async () => {
+      const fetchSimilarArtistFromSpotify = jest.spyOn(provider, 'fetchSimilarArtistFromSpotify')
+        .mockImplementation((artistName, token) => {
+          return Promise.resolve({
+            name: `Spotify: ${artistName}`
+          } as SimilarArtist);
+        });
+
+      const spotifyToken = 'Spotify token';
+      const artistsFromLastfm = [
+        { name: null },
+        { name: 'Artist 1' },
+        { name: 'Artist 2' },
+        { name: 'Artist 3' },
+        { name: null },
+        { name: 'Artist 4' },
+        { name: 'Artist 5' },
+        { name: 'Artist 6' }
+      ] as LastfmArtistShort[];
+
+      expect(await provider.fetchTopSimilarArtistsFromSpotify(artistsFromLastfm, spotifyToken))
+        .toEqual([
+          { 'name': 'Spotify: Artist 1' },
+          { 'name': 'Spotify: Artist 2' },
+          { 'name': 'Spotify: Artist 3' },
+          { 'name': 'Spotify: Artist 4' },
+          { 'name': 'Spotify: Artist 5' }
+        ]);
+      expect(fetchSimilarArtistFromSpotify).toHaveBeenCalledTimes(5);
+      fetchSimilarArtistFromSpotify.mockRestore();
+    });
+  });
+
+  describe('Fetch similar artist from Spotify', () => {
+    const searchArtists = jest.spyOn(SpotifyApi, 'searchArtists');
+
+    afterEach(() => {
+      searchArtists.mockReset();
+    });
+
+    afterAll(() => {
+      searchArtists.mockRestore();
+    });
+
+    test('Should fetch similar artist', async () => {
+      searchArtists.mockResolvedValueOnce({
+        images: [{ url: 'the thumbnail URL' }]
+      } as SpotifyArtist);
+      expect(await provider.fetchSimilarArtistFromSpotify('Artist Name', 'Spotify token'))
+        .toEqual({
+          name: 'Artist Name',
+          thumbnail: 'the thumbnail URL'
+        });
+    });
+
+    test('Should return shallow similar artist if fetching fails', async () => {
+      searchArtists.mockRejectedValueOnce('Failed to fetch artist');
+      expect(await provider.fetchSimilarArtistFromSpotify('Artist Name', 'Spotify token'))
+        .toEqual({
+          name: 'Artist Name',
+          thumbnail: null
+        });
+    });
+  });
+
+});

packages/core/src/plugins/meta/discogs.ts

@@ -1,4 +1,4 @@
-import _, { take } from 'lodash';
+import _ from 'lodash';
 
 import MetaProvider from '../metaProvider';
 import * as Discogs from '../../rest/Discogs';
@@ -11,22 +11,23 @@ import {
   SearchResultsSource,
   ArtistDetails,
   AlbumDetails,
-  AlbumType
+  AlbumType,
+  SimilarArtist
 } from '../plugins.types';
 import {
   DiscogsReleaseSearchResponse,
   DiscogsArtistSearchResponse,
-  DiscogsArtistReleasesSearchResponse,
   DiscogsArtistInfo,
   DiscogsArtistSearchResult,
   DiscogsReleaseSearchResult,
   DiscogsArtistReleaseSearchResult,
   DiscogsReleaseInfo,
   DiscogsTrack
 } from '../../rest/Discogs.types';
-import { LastFmArtistInfo, LastfmTopTracks, LastfmTrack } from '../../rest/Lastfm.types';
+import { LastFmArtistInfo, LastfmTopTracks, LastfmTrack, LastfmArtistShort } from '../../rest/Lastfm.types';
 import { cleanName } from '../../structs/Artist';
 import { getToken, searchArtists } from '../../rest/Spotify';
+import logger from 'electron-timber';
 
 class DiscogsMetaProvider extends MetaProvider {
   lastfm: LastFmApi;
@@ -175,29 +176,19 @@ class DiscogsMetaProvider extends MetaProvider {
 
     const lastFmInfo: LastFmArtistInfo = (await (await this.lastfm.getArtistInfo(discogsInfo.name)).json()).artist;
     const lastFmTopTracks: LastfmTopTracks = (await (await this.lastfm.getArtistTopTracks(discogsInfo.name)).json()).toptracks;
-
     const coverImage = this.getCoverImage(discogsInfo);
-
-    const spotifyToken = await getToken();
-    const similarArtists = await Promise.all(take(lastFmInfo.similar.artist, 5).map(async artist => {
-      const similarArtist = await searchArtists(spotifyToken, artist.name);
-
-      return {
-        name: artist.name,
-        thumbnail: similarArtist?.images[0]?.url
-      };
-    }));
+    const similarArtists = await this.getSimilarArtists(lastFmInfo);
 
     return {
       id: discogsInfo.id,
       name: discogsInfo.name,
       description: _.get(lastFmInfo, 'bio.summary'),
       tags: _.map(_.get(lastFmInfo, 'tags.tag'), 'name'),
-      onTour: lastFmInfo.ontour === '1',
+      onTour: this.isArtistOnTour(lastFmInfo),
       coverImage,
       thumb: coverImage,
       images: _.map(discogsInfo.images, 'resource_url'),
-      topTracks: _.map(lastFmTopTracks.track, (track: LastfmTrack) => ({
+      topTracks: _.map(lastFmTopTracks?.track, (track: LastfmTrack) => ({
         name: track.name,
         title: track.name,
         thumb: coverImage,
@@ -210,6 +201,52 @@ class DiscogsMetaProvider extends MetaProvider {
     };
   }
 
+  async getSimilarArtists(artist: LastFmArtistInfo | undefined): Promise<SimilarArtist[]> {
+    if (!artist?.similar?.artist) {
+      return [];
+    }
+    const similarArtists = artist.similar.artist;
+    try {
+      const spotifyToken = await getToken();
+      return await this.fetchTopSimilarArtistsFromSpotify(similarArtists, spotifyToken);
+    } catch (error) {
+      logger.error(`Failed to fetch similar artists for '${artist.name}'`);
+      logger.error(error);
+    }
+    return [];
+  }
+
+  async fetchTopSimilarArtistsFromSpotify(artists: LastfmArtistShort[], spotifyToken: string) {
+    return Promise.all(
+      artists
+        .filter(artist => artist?.name)
+        .slice(0, 5)
+        .map(artist => this.fetchSimilarArtistFromSpotify(artist.name, spotifyToken))
+    );
+  }
+
+  async fetchSimilarArtistFromSpotify(artistName: string, spotifyToken: string): Promise<SimilarArtist> {
+    return searchArtists(spotifyToken, artistName)
+      .then(spotifyArtist => {
+        return {
+          name: artistName,
+          thumbnail: spotifyArtist?.images[0]?.url
+        };
+      })
+      .catch(error => {
+        logger.error(`Failed to fetch artist from Spotify: '${artistName}'`);
+        logger.error(error);
+        return {
+          name: artistName,
+          thumbnail: null
+        };
+      });
+  }
+
+  isArtistOnTour(artistInfo: LastFmArtistInfo | undefined): boolean {
+    return artistInfo?.ontour === '1';
+  }
+
   async fetchArtistDetailsByName(artistName: string): Promise<ArtistDetails> {
     const artistSearch = await (await Discogs.search(artistName, 'artist')).json();
     const artist: DiscogsArtistSearchResult = _.head(artistSearch.results);