3. the check here returns false because the listen was updated already when the modification in 2 happened.
2023-12-04 33838, 2023
lucifer
so we get an outdated/incorrect cover art thumbnail.
2023-12-04 33857, 2023
lucifer
does this make sense?
2023-12-04 33806, 2023
monkey
OK, I'm following so far, looks like the classic react class props updating issues
2023-12-04 33811, 2023
monkey
(my bad)
2023-12-04 33847, 2023
lucifer
right that props issues plus not cloning the object before storing it in state
2023-12-04 33822, 2023
monkey
OK, that seems easy to fix
2023-12-04 33853, 2023
lucifer
the point fix is indeed easy.
2023-12-04 33834, 2023
lucifer
we could deepcopy the playing now listen before passing it in the prop.
2023-12-04 33803, 2023
lucifer
and update the check in componentDidUpdate to compare the existing state to the new prop
2023-12-04 33816, 2023
lucifer
and merge the data together in case of a mismatch.
2023-12-04 33842, 2023
lucifer
or we could start including the caa_id and caa_release_mbid in the react key for the listen cards.
2023-12-04 33805, 2023
lucifer
the fix involving key is probably the recommended one.
2023-12-04 33829, 2023
monkey
Yep, I was wondering what we could use in the key to make it more robust
2023-12-04 33840, 2023
monkey
It's indeed the absolute simlest.
2023-12-04 33843, 2023
monkey
+p
2023-12-04 33845, 2023
lucifer
however, i was wondering as we move LB towards a SPA. there will be more and more such cases of fields that need updating.
2023-12-04 33813, 2023
monkey
However it does require the same mechanism be applied everywhre ListenCards are in use, which will result in some issue eventually
2023-12-04 33821, 2023
lucifer
how many fields do we manually list out in the key? should we instead look into hashing the entire object and using the hashcode as a key
2023-12-04 33833, 2023
monkey
Hm, not a bad idea at all
2023-12-04 33819, 2023
monkey
Our objects are not gigantic so the computational tradeoff is probably OK.
2023-12-04 33830, 2023
lucifer
i couldn't find anything online about using hashcodes as keys which is weird.
2023-12-04 33801, 2023
lucifer
i assumed others would have faced similar issues
2023-12-04 33855, 2023
lucifer
right the computational tradeoff is something i am not sure about. i was also going to suggest to always deepcopy the listen inside the constructor of the ListenCard to avoid the other issue in this bug but again that comes at a computational tradeoff.
2023-12-04 33825, 2023
monkey
Keys are usually easier to generate, and I think the main issue is in componendDidUpdate which is being retired, so probably not a huge use case
2023-12-04 33846, 2023
monkey
The deep cloning shouldn't be an issue actually.
2023-12-04 33818, 2023
monkey
At least for our sizes of objects and number of listencards
2023-12-04 33824, 2023
lucifer
i see makes sense.
2023-12-04 33808, 2023
lucifer
i can see the same issues arising out of multiple useEffects interactions in ListenCard when we do the rewrite of the component but we can take a look into it when it actually happens i guess.
that should improve the playlists, but may end up not generating as many tracks.
2023-12-04 33823, 2023
mayhem
lucifer: thanks!
2023-12-04 33833, 2023
mayhem is starting on the mosaic
2023-12-04 33805, 2023
monkey
I think it would be a good idea to generate this year's stats (complete with new datapoints) for a few users for some proper testing, including all the "you don't have enough data" stuff
2023-12-04 33829, 2023
lucifer
sure will do
2023-12-04 33846, 2023
lucifer
do we want to add any new stats/datapoints?
2023-12-04 33853, 2023
monkey
We also have the SVG images currently pending. I started setting them up
2023-12-04 33808, 2023
lucifer
i can do the SVGs if you want.
2023-12-04 33840, 2023
monkey
Re: new data points: if you compare with the figma design, there are some items currently missing, in particular genre-related data
We already have some of that data, and I guess you'll be working on the tags stats which can then fit in there
2023-12-04 33839, 2023
lucifer
makes sense.
2023-12-04 33851, 2023
lucifer
i'll get the stats done
2023-12-04 33805, 2023
monkey
I started some small refactoring for the YIM art endpoints, by the way, to allow for this new year while reusing some of the existing code
2023-12-04 33813, 2023
monkey
It's all on branch `yim-2023`
2023-12-04 33839, 2023
monkey
Ah, and I believe you took care of the email design as well last year; do you want to do that this year too, or would you like some help with it?
2023-12-04 33847, 2023
yvanzo
lucifer: done, added a comment to the pull request.
2023-12-04 33803, 2023
monkey
On my side, I'm going to continue implementing the stats art SVGs and prepare the "coming soon" page, the "not enough data" page and other small items well need to have ready
2023-12-04 33835, 2023
lucifer
monkey: should be able to do the emails but would need help with testing on apple devices.
And maybe in a week or so we have another chat, see where we're at and add in the new datapoints in the page, maybe deploy to test and have a look at what we're forgetting ? :D
2023-12-04 33807, 2023
lucifer
sounds great.
2023-12-04 33826, 2023
monkey
👍 sweet, we're on track :)
2023-12-04 33830, 2023
lucifer
thanks!
2023-12-04 33812, 2023
monkey
Thank you !
2023-12-04 33829, 2023
lucifer
mayhem: how did you handle the case of multiple artists on one track in troi? do you count it as one recording for each?
2023-12-04 33857, 2023
mayhem
lemme see
2023-12-04 33853, 2023
mayhem
yes.
2023-12-04 33807, 2023
mayhem
I unroll the artist credit and then count each of the artists as one.
2023-12-04 33823, 2023
mayhem
that is perhaps a bit to tight, but lets just keep that for now.