MBS-13564: Correctly override primary on alias locale change

Attempting to change the locale of an alias marked as primary to one
which already had a primary alias crashed with an ISE.

I noticed the issue is that we were not clearing the previous primary
value first for this case, since we were only looking for a changed
primary_for_locale. In this case
the locale changes, not the primary flag.

This ensures primary is cleared upon either primary or locale changes
if primary is being set or if it was already set and was not overriden
by the row changes.

lib/MusicBrainz/Server/Data/Alias.pm

@@ -285,12 +285,19 @@ sub update
 
     # Clear existing primary for locale flag for the chosen locale
     # if we are overriding them (the user chose this as primary)
-    if ($row{primary_for_locale}) {
-        # We need to load the alias because %row only contains changed values
+    # We need to check if either primary or the locale changed
+    # and then load the alias because %row only contains changed values
+    if ($row{primary_for_locale} || $row{locale}) {
         my $alias = $self->get_by_id($alias_id);
         my $locale = $row{locale} // $alias->{locale};
+        my $primary_for_locale = $row{primary_for_locale} // $alias->{primary_for_locale};
         my $entity_id = $alias->{$type . '_id'};
-        $self->clear_primary_for_locale($locale, $entity_id);
+        # Whether we just set primary or it was already set and we
+        # are keeping it while changing the locale, we need to clear
+        # the existing primary
+        if ($primary_for_locale) {
+            $self->clear_primary_for_locale($locale, $entity_id);
+        }
     }
 
     add_partial_date_to_row(\%row, $alias_hash->{begin_date}, 'begin_date')

t/lib/t/MusicBrainz/Server/Data/Alias.pm

@@ -8,6 +8,7 @@ use Test::More;
 use Test::Deep qw( cmp_deeply );
 
 use DateTime;
+use List::AllUtils qw( first );
 use MusicBrainz::Server::Context;
 use MusicBrainz::Server::Test;
 use Sql;
@@ -131,7 +132,7 @@ test all => sub {
                                  primary_for_locale => 1,
                                  ended => 0,
                                 });
-    my $alias_id = $alias2->{id};
+    my $alias2_id = $alias2->{id};
     $alias_set = $artist_data->alias->find_by_entity_id(1);
     is(scalar @$alias_set, 1, 'Artist #1 has a single newly inserted alias');
     verify_artist_alias(
@@ -140,14 +141,15 @@ test all => sub {
     );
 
     # Test overriding primary for locale on insert
-    $artist_data->alias->insert({
+    my $alias3 = $artist_data->alias->insert({
                                  artist_id => 1,
                                  name => 'Newer alias',
                                  sort_name => 'Newer sort name',
                                  locale => 'en_AU',
                                  primary_for_locale => 1,
                                  ended => 0,
                                 });
+    my $alias3_id = $alias3->{id};
     $alias_set = $artist_data->alias->find_by_entity_id(1);
     is(scalar @$alias_set, 2, 'Artist #1 has a second newly inserted alias');
     verify_artist_alias(
@@ -162,7 +164,7 @@ test all => sub {
     );
 
     # Test overriding primary for locale on update
-    $artist_data->alias->update($alias_id, {primary_for_locale => 1});
+    $artist_data->alias->update($alias2_id, {primary_for_locale => 1});
     $alias_set = $artist_data->alias->find_by_entity_id(1);
     is(scalar @$alias_set, 2, 'Artist #1 still has two aliases');
     is($alias_set->[1]->primary_for_locale,
@@ -172,6 +174,37 @@ test all => sub {
        1,
        'old alias is again primary_for_locale');
 
+    # Test overriding primary for locale on locale change of alias set as primary (MBS-13564)
+    my $alias4 = $artist_data->alias->insert({
+                                 artist_id => 1,
+                                 name => 'New US alias',
+                                 sort_name => 'New US sort name',
+                                 locale => 'en_US',
+                                 primary_for_locale => 1,
+                                 ended => 0,
+                                });
+    my $alias4_id = $alias4->{id};
+    $alias_set = $artist_data->alias->find_by_entity_id(1);
+    is(scalar @$alias_set, 3, 'Artist #1 now has three aliases');
+
+    $artist_data->alias->update($alias4_id, {locale => 'en_AU'});
+    $alias_set = $artist_data->alias->find_by_entity_id(1);
+    is(scalar @$alias_set, 3, 'Artist #1 still has three aliases');
+    my $updated_alias3 = first { $_->id == $alias3_id } @$alias_set;
+    my $updated_alias4 = first { $_->id == $alias4_id } @$alias_set;
+    is($updated_alias3->primary_for_locale,
+       0,
+       'the previous primary alias is no longer primary_for_locale');
+    is($updated_alias3->locale,
+       'en_AU',
+       'the previous primary alias still has the same locale');
+    is($updated_alias4->primary_for_locale,
+       1,
+       'the changed alias is marked as primary_for_locale');
+    is($updated_alias4->locale,
+       'en_AU',
+       'the changed alias has the updated locale');
+
     $test->c->sql->commit;
 
     # Make sure other data types support aliases