Add more export data #80
Conversation
- Album release date, explicit, and popularity to base tracks data - Optional: Artist data (genres) - Optional: Audio features Thanks to Pavel Komarov for his contribution in https://github.com/pavel-aicradle/exportify/blob/master/exportify.js Co-authored-by: pavel-aicradle <pavel@safex.ai>
| - Added By | ||
| - Added At | ||
|
|
||
| Additionally, by clicking on the cog and selecting "Include artists data", the following fields will be added: |
There was a problem hiding this comment.
The cog is a really neat UI feature.
There was a problem hiding this comment.
Thanks, glad you like it. Yep I think it works well and doesn't get in the way.
| @@ -0,0 +1,54 @@ | |||
| import './ConfigDropdown.scss' | |||
There was a problem hiding this comment.
What is tsx? A quick Google is only returning cars.
There was a problem hiding this comment.
Haha. tsx is ts with JSX support (React). And ts is TypeScript.
The idea is that introducing typing is the front-line of defence against introducing bugs.
|
|
||
| render() { | ||
| return ( | ||
| <Dropdown> |
There was a problem hiding this comment.
So much less verbose than React javascript calls.
|
|
||
| if (this.config.includeArtistsData) { | ||
| const tracksArtistsData = new TracksArtistsData(this.accessToken, tracks) | ||
| await tracksCsvFile.addData(tracksArtistsData) |
There was a problem hiding this comment.
love that async/await
| }) | ||
| }) | ||
|
|
||
| test("including additional artist data", async () => { |
There was a problem hiding this comment.
Can you elaborate for me what this testing framework is about and how to use it?
There was a problem hiding this comment.
I'm actually going to write something (possibly a blog article) on working with the specifics of working with Jest and MSW for the first time. Overally it's been great though with a few caveats.
I'm not sure if that's what you were asking or if you were asking a more general question. Just in case, in a more general sense, the test suite is there to validate the behavior of the app. It's mostly about guarding against regressions so I'm free to refactor without worrying too much about breaking things.
In this particular case, most of my tests take the form of mocking out the API calls as close to the HTTP transport layer as possible (so as to be client-agnostic), and then asserting that the app behaves in the expected way given a particular set of responses.
The tests don't actually call the API, so it wouldn't help in the case of breaking changes to the Spotify API. For that I have to rely on user feedback but I also have a last line of defence: exception monitoring with Bugsnag.
| track.artists.map((a: any) => { | ||
| return artistsById.has(a.id) ? artistsById.get(a.id)!.genres.filter((g: string) => g).join(',') : "" | ||
| }).filter((g: string) => g).join(",") | ||
| ] |
There was a problem hiding this comment.
This method rhymes with mine, but a lot more OOP and breaking up the code going on here. Amazing how much better the tools have gotten in the last few years.
| audioFeatures.liveness, | ||
| audioFeatures.valence, | ||
| audioFeatures.tempo, | ||
| audioFeatures.time_signature |
There was a problem hiding this comment.
I like seeing them all arrayed like this.
| "href" : null, | ||
| "total" : 2541 | ||
| }, | ||
| "genres" : [ "nottingham indie" ], |
|
With so many interacting files, some kind of system map might be helpful for future contributors. |
|
@pavel-aicradle Thanks a lot for the review 🙏 |
See #67.
This PR introduces the additional data requested in #67 as well as that included in #70. I've marked that specific commit as co-authored since I drew inspiration from that code, though wasn't able to merge it as-is. @pavel-aicradle let me know if that's okay for you 🙂 .
This PR also introduces a lightweight configuration interface so that we don't include data requiring many extra requests by default. This should keep any unnecessary API calls to a minimum, while leaving the option there for more advanced users. It looks like this: