MBS-7646: Show localized artist aliases in more places #3191
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -146,9 +146,9 @@ after 'load' => sub | |
| } | ||
| } | ||
|
|
||
| my $lang = $c->stash->{current_language} // 'en'; | ||
| $c->model('Artist')->load_related_info([$artist], $lang); | ||
| $c->model('ArtistType')->load(map { $_->target } @{ $artist->relationships_by_type('artist') }); | ||
| $c->model('Area')->load_containment($artist->area, $artist->begin_area, $artist->end_area); | ||
| }; | ||
|
|
||
|
|
@@ -286,26 +286,22 @@ sub show : PathPart('') Chained('load') | |
| if (defined $base_name) { | ||
| $c->model('Relationship')->load_subset(['artist'], $base_name); | ||
| $c->stash( legal_name => $base_name ); | ||
|
There was a problem hiding this comment. Is it safe to assume that It seems to work to the extent that I still see legal names when I view artist pages, at least. (Those weren't being shown with this change until I added the There was a problem hiding this comment.
It looks safe to me since There was a problem hiding this comment. Maybe this isn't the case. https://beta.musicbrainz.org/artist/869cf6cc-07bb-470a-aa30-7e7e6000ffcf currently fails to load: There was a problem hiding this comment. It works locally running the beta code, which suggests to me that the product of a prod load without aliases is being cached and causing the beta crash. If that's the case, we could either amend prod to save the info to the artist even if it doesn't use it, or amend beta temporarily to just show nothing rather than ISE if the data is missing. My preference would be the first, if we think it's safe (we're supposed to hotfix prod soon anyway for unrelated reasons) There was a problem hiding this comment. @mwiencek and @reosarevok, just to make sure I understand this properly, will the caching also cause issues with the There was a problem hiding this comment. That shouldn't be an issue as entities are cached immediately before any additional data is loaded onto them. So no |
||
| my @aliases = uniq map { $_->name } | ||
| sort_by { $coll->getSortKey($_->name) } | ||
| # An alias equal to the artist name already shown isn't useful | ||
| grep { ($_->name) ne $base_name->name } | ||
| # A legal name alias marked ended isn't a current legal name | ||
| grep { !($_->ended) } | ||
| grep { ($_->type_name // '') eq 'Legal name' } @{ $base_name->aliases }; | ||
| $c->stash( legal_name_artist_aliases => \@aliases ); | ||
| $base_name_legal_name_aliases = \@aliases; | ||
| push(@identities, $base_name); | ||
| } else { | ||
| my @aliases = uniq map { $_->name } | ||
| sort_by { $coll->getSortKey($_->name) } | ||
| # A legal name alias marked ended isn't a current legal name | ||
| grep { !($_->ended) } | ||
| grep { ($_->type_name // '') eq 'Legal name' } @{ $artist->aliases }; | ||
| $c->stash( legal_name_aliases => \@aliases ); | ||
| $legal_name_aliases = \@aliases; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,6 +44,7 @@ our @EXPORT_OK = qw( | |
| copy_escape | ||
| coordinates_to_hash | ||
| datetime_to_iso8601 | ||
| find_best_primary_alias | ||
| generate_gid | ||
| generate_token | ||
| get_area_containment_join | ||
|
|
@@ -770,6 +771,35 @@ sub contains_string { | |
| return any { $_ eq $string } @$array_ref; | ||
| } | ||
|
|
||
| # Given an array of MusicBrainz::Server::Entity::Alias objects and the UI | ||
| # language, try to find a localized primary alias to display to the user. | ||
| sub find_best_primary_alias { | ||
| my ($aliases_ref, $lang) = @_; | ||
|
|
||
| my $short_lang = substr($lang, 0, 2); | ||
| my ($best, $fallback); | ||
| foreach my $alias (@$aliases_ref) { | ||
| next if !defined $alias->locale || !$alias->primary_for_locale; | ||
|
|
||
| # If we find an exact match for the user's language, use it. | ||
| return $alias if $alias->locale eq $lang; | ||
|
|
||
| # Otherwise, favor more-generic aliases (e.g. "en" over "en_US"). | ||
| if (substr($alias->locale, 0, 2) eq $short_lang && | ||
| (!defined $best || length($alias->locale) == 2)) { | ||
| $best = $alias; | ||
| } | ||
| # If we find an English alias, use it as a fallback. This is likely | ||
|
There was a problem hiding this comment. I figured it'd be worthwhile falling back to English here since it looks like all of the UI languages that are presently listed use the Latin script. There was a problem hiding this comment. I think I mentioned it earlier, but a (future?) downside of this is that if e.g. a user selects Japanese as their UI language (I don't think they can at present), then they'll always see English aliases for Japanese artists, since I don't think there's any way to know that the artist entity's name is already in Japanese. There was a problem hiding this comment. You could in theory check if there's a Japanese artist alias that exactly matches the artist name (but you're right, if that data doesn't exist). There was a problem hiding this comment. Yeah, in practice I don't think that many editors add primary aliases that are identical to entity names. https://musicbrainz.org/artist/9ddd7abc-9e1b-471d-8031-583bc6bc8be9/aliases has a zillion aliases, but of the two matching the "Пётр Ильич Чайковский" entity name, one is a non-primary legal name for Russian and the other is a search hint. Oddly, there's a primary artist name Russian alias of "Пётр Чайковский". |
||
| # better than nothing in many cases, since English aliases often apply | ||
| # to all Latin-script languages. | ||
| if (substr($alias->locale, 0, 2) eq 'en' && | ||
| (!defined $fallback || length($alias->locale) == 2)) { | ||
| $fallback = $alias; | ||
| } | ||
| } | ||
| return $best || $fallback; | ||
| } | ||
|
|
||
| 1; | ||
|
|
||
| =head1 COPYRIGHT AND LICENSE | ||
|
|
||
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.
I'm pretty clueless about how this
c(context?) stash stuff works. I'm passing the language around sincecurrent_languagedidn't seem to be available inload_related_infoand the indexed search code, but please let me know if there's an easier way to get at it there.