New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MBS-9237: Fix complex artist credit detection #468
Conversation
@@ -37,7 +37,7 @@ const reduceArtistCredit = ac => ac.names.reduce(reduceName, ''); | |||
const isComplexArtistCredit = function (ac) { | |||
const firstName = ac.names.get(0); | |||
if (firstName && hasArtist(firstName)) { | |||
return firstName.artist.name !== reduceArtistCredit(ac); | |||
return firstName.artist.name !== reduceArtistCredit(ac) || !nonEmpty(firstName.name); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think this makes sense. My only recommendation would be to take advantage of short-circuit evaluation and put the !nonEmpty(firstName.name)
check before the first one, since reduceArtistCredit
is more expensive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 37dd594
Test failures look unrelated, despite being for ACs. Those are the same ones that happened in one of my PRs and then disappeared. |
Previously, empty Artist Credits were mistakenly considered as simple, so triggering the Artist field Autocomplete to fill back user emptied Artist Credit in the Artist Credit editor bubble. This is because empty artist credit is reduced to artist name. This patch fixes the isComplexArtistCredit to consider empty Artist Credit as complex. This prevents the automatic fill-back of Artist Credit field in the Artist Credit editor to mistakenly happen before the field looses focus. See `onNameBlur` for proper fill-back.
473fa3b
to
37dd594
Compare
One more request (sorry :)). We have tests for |
Thanks for spotting this. Done in 6606e35. |
8080cba
to
7ed9c28
Compare
Note: `artistCreditFromArray` can not be used since it replaces empty `name` by `artist.name`.
7ed9c28
to
6606e35
Compare
Fix MBS-9237: Don't immediately fill back the default AC while editor is typing.
Previously, empty Artist Credits were mistakenly considered as simple, so triggering the Artist field Autocomplete to fill back user emptied Artist Credit in the Artist Credit editor bubble. This is because empty artist credit is reduced to artist name.
This patch fixes the
isComplexArtistCredit
to consider empty Artist Credit as complex. This prevents the automatic fill-back of Artist Credit field in the Artist Credit editor to mistakenly happen before the field looses focus. SeeonNameBlur
for proper fill-back.