Last.fm support #196
Last.fm support #196
Conversation
currently only played songs are added
plugins/last-fm/back.js
Outdated
| const queryData = []; | ||
| params.api_sig = api_sig; | ||
| for (key in params) { | ||
| queryData.push(`${key}=${params[key]}`); |
There was a problem hiding this comment.
To avoid issues:
| queryData.push(`${key}=${params[key]}`); | |
| queryData.push(`${encodeURIComponent(key)}=${encodeURIComponent (params[key])}`); |
plugins/last-fm/back.js
Outdated
| const cleanupArtistName = (config, artist) => { | ||
| // removes the suffixes of the artist name for more recognition by last.fm | ||
| let { suffixesToRemove } = config; | ||
| if (suffixesToRemove === undefined) { |
There was a problem hiding this comment.
nit: it does not seem necessary to override the suffixes if not defined, we can just return early:
| if (suffixesToRemove === undefined) { | |
| if (suffixesToRemove === undefined) { | |
| return artist; | |
| } |
plugins/last-fm/back.js
Outdated
| duration: songInfo.songDuration, | ||
| }; | ||
| data.api_sig = createApiSig(data, config.secret); | ||
| axios.post('https://ws.audioscrobbler.com/2.0/', createFormData(data)) |
There was a problem hiding this comment.
nit: node-fetch is already there and could be used (but axios is well-known and widely used so no big deal and no need to change it)
There was a problem hiding this comment.
Ah, didn't know you could post data with node-fetch, removed axios, because there is no need for an extra dependency :)
plugins/last-fm/back.js
Outdated
| duration: songInfo.songDuration, | ||
| }; | ||
| data.api_sig = createApiSig(data, config.secret); | ||
| axios.post('https://ws.audioscrobbler.com/2.0/', createFormData(data)) |
There was a problem hiding this comment.
Code looks mostly the same as addScrobble so it could probably be refactored with a util function.
|
Thanks for your feedback, I have done some additional refactoring and solved the merge conflicts. |
th-ch
left a comment
th-ch
left a comment
There was a problem hiding this comment.
Code looks good, thanks for the contribution! Merging so that it can ship in the next version, will also update the yarn.lock file!
| resolved "https://registry.yarnpkg.com/aws4/-/aws4-1.11.0.tgz#d61f46d83b2519250e2784daf5b09479a8b41c59" | ||
| integrity sha512-xh1Rl34h6Fi1DC2WWKfxUTVqRsNnr6LsKz2+hfwDxQJWmrx8+c7ylaqBMcHfl1U1r2dsifOvKX3LQuLNZ+XSvA== | ||
|
|
||
| axios@^0.21.1: |
There was a problem hiding this comment.
Not a big deal but it seems the lock file is not up-to-date (axios is not used anymore), yarn will need to be run on the project to update it!
|
any chance to see the lastfm plugin, in the ytm App? |
3 participants
This adds a last.fm plugin with the following features,
When the plugin is enabled for the first time, it should ask the user to authenticate with last.fm and give permissions to YouTube music.