Fix artist search when lastfm information isn't available

[sferra]

Addresses #1427

[sferra]

I just added a few checks and extracted the some of the existing logic to a separate function to allow testing it later.

[codecov]

Codecov Report

Attention: Patch coverage is 86.36364% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 71.82%. Comparing base (d01e199) to head (26392a8).

Files	Patch %	Lines
packages/core/src/plugins/meta/discogs.ts	86.36%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1554      +/-   ##
==========================================
+ Coverage   71.60%   71.82%   +0.21%     
==========================================
  Files         364      364              
  Lines        6713     6729      +16     
  Branches      479      483       +4     
==========================================
+ Hits         4807     4833      +26     
+ Misses       1512     1500      -12     
- Partials      394      396       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[nukeop]

Should I start reviewing? Or do you want to mark it as ready later one when you finish?

[sferra]

I did my best to add tests for the my changes but I had to refactor.
Any feedback is appreciated, since I have no prior experience with TypeScript.

[nukeop]

It should be possible to use const artistInfoFromLastFm: SimilarArtist = {, or at least add satisfies SimilarArtist at the end. If the version of TS we're using is too old to have the satisfies keyword, you might be able to use as SimilarArtist.

The same method applies to the as any uses below. In general we want all objects to have appropriate types.

If the existing SimilarArtist type doesn't have the fields you need, you should add them there so that it reflects the type of data you want to return from getSimilarArtists. Or you can create a new type for this.

[sferra]

Thanks for pointing the issues out. The actual type is LastFmArtistInfo which has multiple fields which are not relevant to the call and I didn't want to declare. satisfies doesn't seem to be appropriate here, or I don't know how to apply it. I addressed all as any casts with explicit types. Is that allright?

[nukeop]

I don't think it's needed to unit test small functions like this. Integration tests are more comprehensive, and this function is already included in the output of fetchArtistDetails.

[sferra]

I tested only the code which I added or had to touch. The expression wrapped in the function was one of the things which caused the issue because of the missing lastFmInfo. So it's most certainly not yet covered by any existing test.

Should I remove the function, the tests and inline the expression? Then my change won't be tested. : )

[nukeop]

Ok, I finished my review. In general, I appreciate that you tested everything very comprehensively.

[sferra]

Side note:

I just noticed that when using the Bandcamp metadata provider, the UI doesn't display proper images for similar artists. This is because there's no logic for fetching artist images from Spotify, like the Discogs metadata provider does. If this PR gets approved, afterwards it would be possible to address this issue by extracting and reusing the current implementation for fetching artist images for the Bandcamp metadata provider.