Add CritiqueBrainz functions to APIService #1584
Conversation
Adds submitReviewToCB(), lookupMBRelease(), lookupMBReleaseFromTrack() for use by the Critiquebrainz review modal
Updates the type and the APIService submitReviewToCB() function
| const data = await response.json(); | ||
| return data; |
There was a problem hiding this comment.
This is equivalent to return await response.json(); which is warned against by an eslint rule. Here's a nice writeup on it, https://jakearchibald.com/2017/await-vs-return-vs-return-await/. TL,DR; is that return await is only useful inside a try/catch.
So in this case, it'll probably be fine to do return response.json(). But I am not sure if we would want to add a try/catch here or in the react component etc. @MonkeyDo knows better about react error handling so this is fine by me for now.
There was a problem hiding this comment.
I agree that over here we will be fine with just return response.json(). The relevant change can be made.
| const data = await response.json(); | ||
| return data; |
There was a problem hiding this comment.
I agree that over here we will be fine with just return response.json(). The relevant change can be made.
| lookupMBRelease = async (releaseMBID: string): Promise<any> => { | ||
| const url = `${this.MBBaseURI}/release/${releaseMBID}?fmt=json&inc=release-groups`; | ||
| const response = await fetch(encodeURI(url)); | ||
| this.checkStatus(response); |
There was a problem hiding this comment.
We are missing an await over here.
| lookupMBReleaseFromTrack = async (trackMBID: string): Promise<any> => { | ||
| const url = `${this.MBBaseURI}/release?track=${trackMBID}&fmt=json`; | ||
| const response = await fetch(encodeURI(url)); | ||
| this.checkStatus(response); |
| entity_id: string; | ||
| entity_type: ReviewableEntityType; | ||
| text: string; | ||
| languageCode: string; |
There was a problem hiding this comment.
This could just be language since the api uses this standard. This is not necessary though.
|
Good catch regarding the missing await! Since, this one is already merged I'll open a new PR fixing this. Also, I see there are many other methods in APIService class which do not have the await. Will fix those too. |
(this PR does not depend on any others and can be merged)
This PR adds some types and helper functions to APIService that will be used by the Critiquebrainz review modal.
lookupMBRelease()&lookupMBReleaseFromTrack()send lookup requests to MusicBrainz, these will be used to search for entities of Listens in the review modal.