Allow fetching a speaker with their talks as JSON #821
Conversation
There was a problem hiding this comment.
I like the idea to improve our json api
I do have a few concern about the performance with the current implementation
also if you can try on a user with lots of talks like Marco or Aaron, remove the cache and ensure they are no n+1 (I think they are now)
you might want to remove some of the present? https://www.speedshop.co/2019/01/10/three-activerecord-mistakes.html
| if speaker.user.present? | ||
| json.bio speaker.user.bio | ||
| json.avatar_url speaker.user.avatar_url | ||
| end |
There was a problem hiding this comment.
I got a 500 here I think those attributes should come directly from the speaker model not the user
| json.transcript do | ||
| json.raw talk.raw_transcript | ||
| json.enhanced talk.enhanced_transcript | ||
| end |
There was a problem hiding this comment.
Those can be pretty large fields. before this PR it was only used for the Talks#show view.
Now the problem is that for a speaker with lots of Talk (hello Marco/Aaron) it generates a huge json.
I think we should remove the transcript when we fetch a collection of talks
|
Oh whoops I hadn't intended to open a PR yet. Thanks for the input! I'll circle back on this and address that. |
d35f39a to
ec30df0
Compare
No description provided.