MBS-9246: Load extra data for Artist Credit field #467
Conversation
| $name->{artist}->{gid} = $artists->{$name->{artist}->{id}}->gid if $artists->{$name->{artist}->{id}}; | ||
| my $artist = $artists->{$name->{artist}->{id}}; | ||
| if ($artist) | ||
| { |
There was a problem hiding this comment.
Nitpick: we prefer opening brackets to always be on the same line as the control statement.
There was a problem hiding this comment.
Meta Nitpick: “Curly braces are always on their own line” according to Server Coding Style 😆
There was a problem hiding this comment.
Whooops...at least the heading on that page is accurate: "Status: This page is work in progress and has very questionable content." :)
I'll fix it to be less questionable.
| $name->{artist}->{gid} = $artist->{gid}; | ||
| $name->{artist}->{sortName} = $artist->{sort_name}; | ||
| $name->{artist}->{comment} = $artist->{comment}; | ||
| $name->{artist}->{editsPending} = $artist->{edits_pending}; |
There was a problem hiding this comment.
Looks good, but there's already an $artist->TO_JSON method which should encode things in the same format as here—probably with additional fields that aren't needed, but using it could prevent future issues if the format changes.
As an aside, for blessed Moose objects we always use the accessor methods ($artist->gid vs. $artist->{gid}). Both will work, but the accessors make it easier to see it's a blessed instance, not a hash reference (and are also easier to type).
There was a problem hiding this comment.
Ok. At first glance Artist JSON seemed to output a lot more details than what is useful to display Artists Credit, but I will look into it again. Thanks for the blessed tip!
There was a problem hiding this comment.
It is indeed reasonable to use full JSON export. Done in 3cd70df.
Previously, only artist name, id, gid, and type were loaded for preview in the Artist Credit editor. Consequently, tool-tips did not contain sort name or disambiguation comment (but 'undefined' instead), and artists with pending edits were not highlighted. It affects Artist Credit field editor of editing forms for recording, release-group, artist credit, and artist split. Only Artist Credit field of release editor is unaffected, because it is initialized in a different way from React. This patch loads all JSON-encoded artist fields including sort name, disambiguation comment, and status regarding pending edits to the Artist Credit form field. All of the above mentioned bugs are fixed, ensuring consistency with release editor.
yvanzo
force-pushed
the
mbs-9246-ac-editor-tooltips
branch
from
133be5b
to
3cd70df
Compare
Fix MBS-9246 Tool-tips displaying "undefined" text.
Previously, only artist name, id, gid, and type were loaded for preview in the Artist Credit editor. Consequently, tool-tips did not contain sort name or disambiguation comment (but undefined instead), and artists with pending edits were not highlighted. It affects Artist Credit field editor of editing forms for recording, release-group, artist credit, and artist split. Only Artist Credit field of release editor is unaffected, because it is initialized in a different way using React.
This patch loads all JSON-serialized artist fields including sort name, disambiguation comment, and status regarding pending edits to the Artist Credit form field. All of the above mentioned bugs are fixed, ensuring consistency with release editor.