diff --git a/admin/sql/CreateConstraints.sql b/admin/sql/CreateConstraints.sql index 5df42837219..5720281fd90 100644 --- a/admin/sql/CreateConstraints.sql +++ b/admin/sql/CreateConstraints.sql @@ -170,6 +170,17 @@ ADD CONSTRAINT group_type_implies_null_gender CHECK ( OR type IS NULL ); +ALTER TABLE artist +ADD CONSTRAINT character_type_implies_no_end CHECK ( + type != 4 OR ( + end_date_day IS NULL AND + end_date_month IS NULL AND + end_date_year IS NULL AND + end_area IS NULL AND + ended = FALSE + ) +); + ALTER TABLE release_label ADD CHECK (catalog_number IS NOT NULL OR label IS NOT NULL); diff --git a/admin/sql/updates/20241104-mbs-9885.sql b/admin/sql/updates/20241104-mbs-9885.sql new file mode 100644 index 00000000000..4edded422ea --- /dev/null +++ b/admin/sql/updates/20241104-mbs-9885.sql @@ -0,0 +1,27 @@ +\set ON_ERROR_STOP 1 + +BEGIN; + +UPDATE artist +SET end_date_day = NULL, + end_date_month = NULL, + end_date_year = NULL, + end_area = NULL, + ended = FALSE +WHERE type = 4; + +ALTER TABLE artist +DROP CONSTRAINT IF EXISTS character_type_implies_no_end; + +ALTER TABLE artist +ADD CONSTRAINT character_type_implies_no_end CHECK ( + type != 4 OR ( + end_date_day IS NULL AND + end_date_month IS NULL AND + end_date_year IS NULL AND + end_area IS NULL AND + ended = FALSE + ) +); + +COMMIT; diff --git a/admin/sql/updates/schema-change/30.master_and_standalone.sql b/admin/sql/updates/schema-change/30.master_and_standalone.sql new file mode 100644 index 00000000000..4214fd3d6c5 --- /dev/null +++ b/admin/sql/updates/schema-change/30.master_and_standalone.sql @@ -0,0 +1,33 @@ +-- Generated by CompileSchemaScripts.pl from: +-- 20241104-mbs-9885.sql +\set ON_ERROR_STOP 1 +BEGIN; +SET search_path = musicbrainz, public; +SET LOCAL statement_timeout = 0; +-------------------------------------------------------------------------------- +SELECT '20241104-mbs-9885.sql'; + + +UPDATE artist +SET end_date_day = NULL, + end_date_month = NULL, + end_date_year = NULL, + end_area = NULL, + ended = FALSE +WHERE type = 4; + +ALTER TABLE artist +DROP CONSTRAINT IF EXISTS character_type_implies_no_end; + +ALTER TABLE artist +ADD CONSTRAINT character_type_implies_no_end CHECK ( + type != 4 OR ( + end_date_day IS NULL AND + end_date_month IS NULL AND + end_date_year IS NULL AND + end_area IS NULL AND + ended = FALSE + ) +); + +COMMIT; diff --git a/lib/MusicBrainz/Server/Constants.pm b/lib/MusicBrainz/Server/Constants.pm index 1c71142e40d..b92b1d22b4d 100644 --- a/lib/MusicBrainz/Server/Constants.pm +++ b/lib/MusicBrainz/Server/Constants.pm @@ -67,7 +67,7 @@ our @EXPORT_OK = ( $DLABEL_ID $DARTIST_ID $VARTIST_ID $VARTIST_GID $NOLABEL_ID $NOLABEL_GID $COVERART_FRONT_TYPE $COVERART_BACK_TYPE $AREA_TYPE_COUNTRY $AREA_TYPE_CITY - $ARTIST_TYPE_PERSON $ARTIST_TYPE_GROUP + $ARTIST_TYPE_PERSON $ARTIST_TYPE_GROUP $ARTIST_TYPE_CHARACTER $INSTRUMENT_ROOT_ID $VOCAL_ROOT_ID $REQUIRED_VOTES $OPEN_EDIT_DURATION $MINIMUM_RESPONSE_PERIOD $MINIMUM_VOTING_PERIOD @@ -395,6 +395,7 @@ Readonly our $AREA_TYPE_CITY => 3; Readonly our $ARTIST_TYPE_PERSON => 1; Readonly our $ARTIST_TYPE_GROUP => 2; +Readonly our $ARTIST_TYPE_CHARACTER => 4; Readonly our $REQUIRED_VOTES => 3; Readonly our $OPEN_EDIT_DURATION => DateTime::Duration->new(days => 7); diff --git a/lib/MusicBrainz/Server/Data/Artist.pm b/lib/MusicBrainz/Server/Data/Artist.pm index 0b632236b3f..4ddd7d953cd 100644 --- a/lib/MusicBrainz/Server/Data/Artist.pm +++ b/lib/MusicBrainz/Server/Data/Artist.pm @@ -4,7 +4,10 @@ use namespace::autoclean; use Carp; use List::AllUtils qw( any ); -use MusicBrainz::Server::Constants qw( $ARTIST_TYPE_GROUP ); +use MusicBrainz::Server::Constants qw( + $ARTIST_TYPE_CHARACTER + $ARTIST_TYPE_GROUP +); use MusicBrainz::Server::Entity::Artist; use MusicBrainz::Server::Entity::PartialDate; use MusicBrainz::Server::Data::ArtistCredit; @@ -19,6 +22,7 @@ use MusicBrainz::Server::Data::Utils qw( load_subobjects merge_table_attributes merge_date_period + merge_partial_date order_by ); use MusicBrainz::Server::Data::Utils::Cleanup qw( used_in_relationship ); @@ -389,12 +393,21 @@ sub merge # that's arguably more annoying for the editor. See MBS-10187. my %dropped_columns; unless (is_special_artist($new_id)) { - my $merge_columns = [ qw( area begin_area end_area ) ]; - my $target_row = $self->sql->select_single_row_hash('SELECT gender, type FROM artist WHERE id = ?', $new_id); + my $merge_columns = [ qw( area begin_area ) ]; + my $target_row = $self->sql->select_single_row_hash( + 'SELECT gender, type, ended, end_area, end_date_year, end_date_month, end_date_day FROM artist WHERE id = ?', + $new_id, + ); my $target_type = $target_row->{type}; my $target_gender = $target_row->{gender}; + my $target_ended = $target_row->{ended}; + my $target_end_date = MusicBrainz::Server::Entity::PartialDate->new_from_row($target_row, 'end_date_'); + my $target_end_area = $target_row->{end_area}; my $merged_type = $target_type; my $merged_gender = $target_gender; + my $merged_ended = $target_ended; + my $merged_end_date = $target_end_date; + my $merged_end_area = $target_end_area; if (!$merged_type) { my ($query, $args) = conditional_merge_column_query( @@ -408,6 +421,38 @@ sub merge $merged_gender = $self->c->sql->select_single_value($query, @$args); } + if (!$merged_ended) { + my ($query, $args) = conditional_merge_column_query( + 'artist', 'ended', $new_id, [$new_id, @$old_ids], '= TRUE'); + $merged_ended = $self->c->sql->select_single_value($query, @$args) // 0; + } + + if ($merged_end_date->is_empty) { + my ($query, $args) = conditional_merge_column_query( + 'artist', 'end_date_year', $new_id, [$new_id, @$old_ids], 'IS NOT NULL'); + my $merged_end_year = $self->c->sql->select_single_value($query, @$args); + + ($query, $args) = conditional_merge_column_query( + 'artist', 'end_date_month', $new_id, [$new_id, @$old_ids], 'IS NOT NULL'); + my $merged_end_month = $self->c->sql->select_single_value($query, @$args); + + ($query, $args) = conditional_merge_column_query( + 'artist', 'end_date_day', $new_id, [$new_id, @$old_ids], 'IS NOT NULL'); + my $merged_end_day = $self->c->sql->select_single_value($query, @$args); + + $merged_end_date = MusicBrainz::Server::Entity::PartialDate->new( + year => $merged_end_year, + month => $merged_end_month, + day => $merged_end_day, + ); + } + + if (!$merged_end_area) { + my ($query, $args) = conditional_merge_column_query( + 'artist', 'end_area', $new_id, [$new_id, @$old_ids], 'IS NOT NULL'); + $merged_end_area = $self->c->sql->select_single_value($query, @$args); + } + my $group_types = $self->sql->select_single_column_array(q{ WITH RECURSIVE atp(id) AS ( VALUES (?::int) @@ -422,23 +467,61 @@ sub merge defined $merged_type && contains_string($group_types, $merged_type); - if ($merged_type_is_group && $merged_gender) { - my $target_type_is_group = - defined $target_type && - contains_string($group_types, $target_type); - - if ($target_type_is_group) { - $dropped_columns{gender} = $merged_gender; - push @$merge_columns, 'type'; - } elsif ($target_gender) { - $dropped_columns{type} = $merged_type; + my $merged_type_is_character = + defined $merged_type && $merged_type == $ARTIST_TYPE_CHARACTER; + + my $merge_ended = 0; + + if ($merged_type_is_group || $merged_type_is_character) { + if ($merged_type_is_group && $merged_gender) { + my $target_type_is_group = + defined $target_type && + contains_string($group_types, $target_type); + + if ($target_type_is_group) { + $dropped_columns{gender} = $merged_gender; + push @$merge_columns, 'type'; + } elsif ($target_gender) { + $dropped_columns{type} = $merged_type; + push @$merge_columns, 'gender'; + } else { + $dropped_columns{gender} = $merged_gender; + $dropped_columns{type} = $merged_type; + } + } else { push @$merge_columns, 'gender'; + } + + if ( + $merged_type_is_character && ( + $merged_ended || + $merged_end_area || + !($merged_end_date->is_empty) + ) + ) { + my $target_type_is_character = + defined $target_type && $target_type == $ARTIST_TYPE_CHARACTER; + + if ($target_type_is_character) { + $dropped_columns{ended} = $merged_ended; + $dropped_columns{end_area} = $merged_end_area; + $dropped_columns{end_date} = $merged_end_date->format; + push @$merge_columns, 'type'; + } elsif ($target_ended || $target_end_area || !($target_end_date->is_empty)) { + $dropped_columns{type} = $merged_type; + push @$merge_columns, 'end_area'; + $merge_ended = 1; + } else { + $dropped_columns{ended} = $merged_ended; + $dropped_columns{type} = $merged_type; + } } else { - $dropped_columns{gender} = $merged_gender; - $dropped_columns{type} = $merged_type; + push @$merge_columns, 'end_area'; + $merge_ended = 1; } } else { - push @$merge_columns, qw( gender type ); + push @$merge_columns, qw( type gender end_area ); + $merge_ended = 1; } merge_table_attributes( @@ -450,13 +533,24 @@ sub merge ), ); - merge_date_period( - $self->sql => ( - table => 'artist', - old_ids => $old_ids, - new_id => $new_id, - ), - ); + if ($merge_ended) { + merge_date_period( + $self->sql => ( + table => 'artist', + old_ids => $old_ids, + new_id => $new_id, + ), + ); + } else { + merge_partial_date( + $self->sql => ( + table => 'artist', + old_ids => $old_ids, + new_id => $new_id, + ), + field => 'begin_date', + ); + } } $self->_delete_and_redirect_gids('artist', $new_id, @$old_ids); diff --git a/lib/MusicBrainz/Server/Edit/Artist/Edit.pm b/lib/MusicBrainz/Server/Edit/Artist/Edit.pm index 7d2d6ffb6de..06d3a6afe50 100644 --- a/lib/MusicBrainz/Server/Edit/Artist/Edit.pm +++ b/lib/MusicBrainz/Server/Edit/Artist/Edit.pm @@ -3,6 +3,7 @@ use 5.10.0; use Moose; use MusicBrainz::Server::Constants qw( + $ARTIST_TYPE_CHARACTER $ARTIST_TYPE_GROUP $EDIT_ARTIST_CREATE $EDIT_ARTIST_EDIT @@ -272,6 +273,12 @@ around merge_changes => sub { my $artist = $self->current_instance; my $gender_id = exists $merged->{gender_id} ? $merged->{gender_id} : $artist->gender_id; + my $end_date = exists $merged->{end_date} ? + PartialDate->new($merged->{end_date}) : $artist->end_date; + my $ended = (exists $merged->{ended} ? + $merged->{ended} : $artist->ended) // 0; + my $end_area_id = exists $merged->{end_area_id} ? + $merged->{end_area_id} : $artist->end_area_id; my $type_id = exists $merged->{type_id} ? $merged->{type_id} : $artist->type_id; @@ -284,6 +291,24 @@ around merge_changes => sub { ); } + if (defined $end_date && !$end_date->is_empty && defined $type_id) { + MusicBrainz::Server::Edit::Exceptions::GeneralError->throw( + 'Characters cannot have an end date.', + ) if ($type_id == $ARTIST_TYPE_CHARACTER); + } + + if (defined $end_area_id && defined $type_id) { + MusicBrainz::Server::Edit::Exceptions::GeneralError->throw( + 'Characters cannot have an end area.', + ) if ($type_id == $ARTIST_TYPE_CHARACTER); + } + + if ($ended && defined $type_id) { + MusicBrainz::Server::Edit::Exceptions::GeneralError->throw( + 'Characters cannot be marked as ended.', + ) if ($type_id == $ARTIST_TYPE_CHARACTER); + } + return $merged; }; diff --git a/lib/MusicBrainz/Server/Edit/Artist/Merge.pm b/lib/MusicBrainz/Server/Edit/Artist/Merge.pm index 4ba2615a6f6..c4ff18df794 100644 --- a/lib/MusicBrainz/Server/Edit/Artist/Merge.pm +++ b/lib/MusicBrainz/Server/Edit/Artist/Merge.pm @@ -65,20 +65,75 @@ sub do_merge my $dropped_type = $self->c->model('ArtistType')->get_by_id($dropped_columns->{type}); + if ($dropped_type->id == 4) { + $self->c->model('EditNote')->add_note( + $self->id => { + editor_id => $EDITOR_MODBOT, + text => localized_note( + N_l('The following data has not been added to the destination ' . + 'artist because it conflicted with other data: ' . + '{dropped_data}. {reason}'), + vars => { + dropped_data => localized_note( + N_l('Type: {artist_type}'), + vars => { + artist_type => localized_note( + $dropped_type->name, + function => 'lp', + domain => 'attributes', + args => ['artist_type'], + ), + }, + ), + reason => localized_note( + N_l('Characters cannot have an end date nor area ' . + 'nor be marked as ended.'), + ), + }, + ), + }, + ); + } else { + $self->c->model('EditNote')->add_note( + $self->id => { + editor_id => $EDITOR_MODBOT, + text => localized_note( + N_l('The “{artist_type}” type has not been added to the ' . + 'destination artist because it conflicted with the ' . + 'gender setting of one of the artists here. Group ' . + 'artists cannot have a gender.'), + vars => { + artist_type => localized_note( + $dropped_type->name, + function => 'lp', + domain => 'attributes', + args => ['artist_type'], + ), + }, + ), + }, + ); + } + } + + if ($dropped_columns->{gender}) { + my $dropped_gender = + $self->c->model('Gender')->get_by_id($dropped_columns->{gender}); + $self->c->model('EditNote')->add_note( $self->id => { editor_id => $EDITOR_MODBOT, text => localized_note( - N_l('The “{artist_type}” type has not been added to the ' . + N_l('The “{gender}” gender has not been added to the ' . 'destination artist because it conflicted with the ' . - 'gender setting of one of the artists here. Group ' . - 'artists cannot have a gender.'), + 'group type of one of the artists here. Group artists ' . + 'cannot have a gender.'), vars => { - artist_type => localized_note( - $dropped_type->name, + gender => localized_note( + $dropped_gender->name, function => 'lp', domain => 'attributes', - args => ['artist_type'], + args => ['gender'], ), }, ), @@ -86,24 +141,53 @@ sub do_merge ); } - if ($dropped_columns->{gender}) { - my $dropped_gender = - $self->c->model('Gender')->get_by_id($dropped_columns->{gender}); + my $dropped_ended = defined $dropped_columns->{ended}; + + if ($dropped_ended || + $dropped_columns->{end_date} || + $dropped_columns->{end_area} + ) { + my $dropped_area = $dropped_columns->{end_area} + ? $self->c->model('Area')->get_by_id($dropped_columns->{end_area}) + : undef; + + my $dropped_data = []; + + if ($dropped_ended) { + push @$dropped_data, localized_note( N_l('Ended: Yes') ); + } + + if ($dropped_columns->{end_date}) { + push @$dropped_data, localized_note( + N_l('End date: {end_date}'), + vars => { end_date => $dropped_columns->{end_date} }, + ); + } + + if ($dropped_area) { + push @$dropped_data, localized_note( + N_l('End area: {end_area}'), + vars => { end_area => $dropped_area->name }, + ); + } $self->c->model('EditNote')->add_note( $self->id => { editor_id => $EDITOR_MODBOT, text => localized_note( - N_l('The “{gender}” gender has not been added to the ' . - 'destination artist because it conflicted with the ' . - 'group type of one of the artists here. Group artists ' . - 'cannot have a gender.'), + N_l('The following data has not been added to the destination ' . + 'artist because it conflicted with other data: ' . + '{dropped_data}. {reason}'), vars => { - gender => localized_note( - $dropped_gender->name, - function => 'lp', - domain => 'attributes', - args => ['gender'], + dropped_data => localized_note( + undef, + function => 'comma_list', + domain => 'mb_server', + args => $dropped_data, + ), + reason => localized_note( + N_l('Characters cannot have an end date nor area ' . + 'nor be marked as ended.'), ), }, ), diff --git a/lib/MusicBrainz/Server/Entity/EditNote.pm b/lib/MusicBrainz/Server/Entity/EditNote.pm index 7adde8cd0db..46d18b476a7 100644 --- a/lib/MusicBrainz/Server/Entity/EditNote.pm +++ b/lib/MusicBrainz/Server/Entity/EditNote.pm @@ -7,7 +7,10 @@ use Moose; use namespace::autoclean; use MusicBrainz::Server::Constants qw( $EDITOR_MODBOT ); -use MusicBrainz::Server::Data::Utils qw( datetime_to_iso8601 ); +use MusicBrainz::Server::Data::Utils qw( + contains_string + datetime_to_iso8601 +); use MusicBrainz::Server::Entity::Types; use MusicBrainz::Server::Entity::Util::JSON qw( to_json_object ); use MusicBrainz::Server::Filters qw( format_editnote ); @@ -75,17 +78,22 @@ sub _localize_text { my $version = $source->{version} // 0; my $fn_package = 'MusicBrainz::Server::Translation'; my $fn_name = 'l'; - my @args = ($source->{message} // ''); + my @args; my $source_vars; if ($version == 0) { + @args = ($source->{message} // ''); # old versions of `localized_note` passed substitution # variables as `args`. $source_vars = $source->{args}; } elsif ($version == 1) { - push @args, @{ $source->{args} // [] }; + my $no_message_functions = [ qw ( comma_list comma_only_list ) ]; $fn_name = $source->{function} // 'l'; + @args = ($source->{message} // '') + unless contains_string($no_message_functions, $fn_name); + push @args, map { _localize_text($_, $depth + 1) } @{ $source->{args} // [] }; + $source_vars = $source->{vars}; my $domain = $source->{domain}; diff --git a/lib/MusicBrainz/Server/Form/Artist.pm b/lib/MusicBrainz/Server/Form/Artist.pm index e552ee9e6f1..f0b05ba6c35 100644 --- a/lib/MusicBrainz/Server/Form/Artist.pm +++ b/lib/MusicBrainz/Server/Form/Artist.pm @@ -89,6 +89,21 @@ sub validate { $self->field('gender_id')->add_error(l('Choirs cannot have a gender.')); } } + + if ($self->field('type_id')->value && + $self->field('type_id')->value == 4) { + if ($self->field('period.end_date.year')->value || + $self->field('period.end_date.month')->value || + $self->field('period.end_date.day')->value) { + $self->field('period.end_date')->add_error(l('Characters cannot have an end date.')); + } + if ($self->field('period.ended')->value) { + $self->field('period.ended')->add_error(l('Characters cannot be marked as ended.')); + } + if ($self->field('end_area_id')->value) { + $self->field('end_area.name')->add_error(l('Characters cannot have an end area.')); + } + } } 1; diff --git a/root/artist/edit_form.tt b/root/artist/edit_form.tt index dabe29f0c35..9f3d0f126b7 100644 --- a/root/artist/edit_form.tt +++ b/root/artist/edit_form.tt @@ -60,20 +60,22 @@ [% field_errors(r.form, 'begin_area.name') %] [% END %] - [% form_row_date(r, 'period.end_date', add_colon(l('End date'))) %] - [% too_short_year_error('too_short_end_year') %] - [% form_row_checkbox(r, 'period.ended', l('This artist has ended.')) %] - [% WRAPPER form_row %] - [% end_area_field = form.field('end_area.name') %] - - - [% React.embed(c, 'static/scripts/common/components/SearchIcon') %] - [% r.hidden(form.field('end_area').field('gid'), { class => 'gid' }) %] - [% r.hidden('end_area_id', class => 'id') %] - [% r.text(end_area_field, class => 'name') %] - - [% field_errors(r.form, 'end_area.name') %] - [% END %] +
+ [% form_row_date(r, 'period.end_date', add_colon(l('End date'))) %] + [% too_short_year_error('too_short_end_year') %] + [% form_row_checkbox(r, 'period.ended', l('This artist has ended.')) %] + [% WRAPPER form_row %] + [% end_area_field = form.field('end_area.name') %] + + + [% React.embed(c, 'static/scripts/common/components/SearchIcon') %] + [% r.hidden(form.field('end_area').field('gid'), { class => 'gid' }) %] + [% r.hidden('end_area_id', class => 'id') %] + [% r.text(end_area_field, class => 'name') %] + + [% field_errors(r.form, 'end_area.name') %] + [% END %] +
[% PROCESS 'forms/relationship-editor.tt' %] diff --git a/root/artist/utils.js b/root/artist/utils.js index a6ece52bace..abb987c1113 100644 --- a/root/artist/utils.js +++ b/root/artist/utils.js @@ -15,6 +15,8 @@ export function artistBeginAreaLabel(typeId: ?number): string { case 5: case 6: return addColonText(lp('Founded in', 'group artist')); + case 4: + return addColonText(lp('Created in', 'character artist')); default: return addColonText(l('Begin area')); } diff --git a/root/static/scripts/edit/MB/Control/ArtistEdit.js b/root/static/scripts/edit/MB/Control/ArtistEdit.js index 288097094bf..bd3fe95eafa 100644 --- a/root/static/scripts/edit/MB/Control/ArtistEdit.js +++ b/root/static/scripts/edit/MB/Control/ArtistEdit.js @@ -16,12 +16,23 @@ MB.Control.ArtistEdit = function () { self.$name = $('#id-edit-artist\\.name'); self.$begin = $('#label-id-edit-artist\\.period\\.begin_date'); self.$ended = $('#label-id-edit-artist\\.period\\.ended'); + self.$ended_checkbox = $('#id-edit-artist\\.period\\.ended'); self.$end = $('#label-id-edit-artist\\.period\\.end_date'); + self.$end_section = $('#artist\\.end_date_section'); + self.$end_year = $('#id-edit-artist\\.period\\.end_date\\.year'); + self.$end_month = $('#id-edit-artist\\.period\\.end_date\\.month'); + self.$end_day = $('#id-edit-artist\\.period\\.end_date\\.day'); self.$beginarea = $('#label-id-edit-artist\\.begin_area\\.name'); self.$endarea = $('#label-id-edit-artist\\.end_area\\.name'); self.$type = $('#id-edit-artist\\.type_id'); self.$gender = $('#id-edit-artist\\.gender_id'); + self.$end_area_id = $('#id-edit-artist\\.end_area_id'); self.old_gender = self.$gender.val(); + self.old_ended_checkbox = self.$ended_checkbox.prop('checked'); + self.old_end_year = self.$end_year.val(); + self.old_end_month = self.$end_month.val(); + self.old_end_day = self.$end_day.val(); + self.old_end_area = self.$end_area_id.val(); self.changeDateText = function (begin, end, ended) { self.$begin.text(begin); @@ -40,6 +51,7 @@ MB.Control.ArtistEdit = function () { * Unknown: 0 * Person: 1 * Group: 2 + * Character: 4 * Orchestra: 5 * Choir: 6 */ @@ -53,6 +65,7 @@ MB.Control.ArtistEdit = function () { ); self.changeAreaText(l('Born in:'), l('Died in:')); self.enableGender(); + self.enableEndSection(); break; case '2': @@ -68,6 +81,21 @@ MB.Control.ArtistEdit = function () { addColonText(lp('Dissolved in', 'group artist')), ); self.disableGender(); + self.enableEndSection(); + break; + + case '4': + self.changeDateText( + addColonText(lp('Created', 'character artist')), + addColonText(lp('Ended', 'artist end date')), + l('This artist has ended.'), + ); + self.changeAreaText( + addColonText(lp('Created in', 'character artist')), + addColonText(l('End area')), + ); + self.enableGender(); + self.disableEndSection(); break; case '0': @@ -82,6 +110,7 @@ MB.Control.ArtistEdit = function () { addColonText(l('End area')), ); self.enableGender(); + self.enableEndSection(); break; } }; @@ -100,6 +129,36 @@ MB.Control.ArtistEdit = function () { self.$gender.val(''); }; + self.enableEndSection = function () { + if (self.$end_section.is(':hidden')) { + self.$end_section.show(); + self.$ended_checkbox.prop('checked', self.old_ended_checkbox); + self.$end_area_id.val(self.old_end_area); + self.$end_year.val(self.old_end_year); + self.$end_month.val(self.old_end_month); + self.$end_day.val(self.old_end_day); + } + }; + + self.disableEndSection = function () { + self.old_ended_checkbox = self.$ended_checkbox.prop('checked'); + self.$ended_checkbox.prop('checked', false); + + self.old_end_area = self.$end_area_id.val(); + self.$end_area_id.val(''); + + self.old_end_year = self.$end_year.val(); + self.$end_year.val(''); + + self.old_end_month = self.$end_month.val(); + self.$end_month.val(''); + + self.old_end_day = self.$end_day.val(); + self.$end_day.val(''); + + self.$end_section.hide(); + }; + self.typeChanged(); self.$type.bind('change.mb', self.typeChanged); diff --git a/t/lib/t/MusicBrainz/Server/Edit/Artist/Edit.pm b/t/lib/t/MusicBrainz/Server/Edit/Artist/Edit.pm index cefe7ce6974..95b526ece89 100644 --- a/t/lib/t/MusicBrainz/Server/Edit/Artist/Edit.pm +++ b/t/lib/t/MusicBrainz/Server/Edit/Artist/Edit.pm @@ -424,6 +424,116 @@ test 'Type can be set to group when gender is removed (MBS-8801)' => sub { is($c->model('Artist')->get_by_id(2)->type_id, 2); }; +test 'Fails edits trying to set an end date to a character artist' => sub { + my $test = shift; + my $c = $test->c; + + $c->sql->do(<<~'SQL'); + INSERT INTO artist (id, gid, name, sort_name) + VALUES (2, 'cdf5588d-cca8-4e0c-bae1-d53bc73b012a', 'person', 'person'); + SQL + + my $edit = $c->model('Edit')->create( + edit_type => $EDIT_ARTIST_EDIT, + editor_id => 1, + to_edit => $c->model('Artist')->get_by_id(2), + end_date => { year => 2000, month => 3, day => 20 }, + ipi_codes => [], + isni_codes => [], + privileges => $UNTRUSTED_FLAG, + ); + + ok($edit->is_open); + $c->sql->do('UPDATE artist SET type = 4 WHERE id = 2'); + + my $exception = exception { $edit->accept }; + isa_ok $exception, 'MusicBrainz::Server::Edit::Exceptions::GeneralError'; + is $exception->message, 'Characters cannot have an end date.'; +}; + +test 'Fails edits trying to set an artist with an end date as a character' => sub { + my $test = shift; + my $c = $test->c; + + $c->sql->do(<<~'SQL'); + INSERT INTO artist (id, gid, name, sort_name) + VALUES (2, 'cdf5588d-cca8-4e0c-bae1-d53bc73b012a', 'person', 'person'); + SQL + + my $edit = $c->model('Edit')->create( + edit_type => $EDIT_ARTIST_EDIT, + editor_id => 1, + to_edit => $c->model('Artist')->get_by_id(2), + type_id => 4, + ipi_codes => [], + isni_codes => [], + privileges => $UNTRUSTED_FLAG, + ); + + ok($edit->is_open); + $c->sql->do('UPDATE artist SET end_date_year = 1991 WHERE id = 2'); + + my $exception = exception { $edit->accept }; + isa_ok $exception, 'MusicBrainz::Server::Edit::Exceptions::GeneralError'; + is $exception->message, 'Characters cannot have an end date.'; +}; + +test 'Fails edits trying to set a character artist as ended' => sub { + my $test = shift; + my $c = $test->c; + + $c->sql->do(<<~'SQL'); + INSERT INTO artist (id, gid, name, sort_name) + VALUES (2, 'cdf5588d-cca8-4e0c-bae1-d53bc73b012a', 'person', 'person'); + SQL + + my $edit = $c->model('Edit')->create( + edit_type => $EDIT_ARTIST_EDIT, + editor_id => 1, + to_edit => $c->model('Artist')->get_by_id(2), + ended => 1, + ipi_codes => [], + isni_codes => [], + privileges => $UNTRUSTED_FLAG, + ); + + ok($edit->is_open); + $c->sql->do('UPDATE artist SET type = 4 WHERE id = 2'); + + my $exception = exception { $edit->accept }; + isa_ok $exception, 'MusicBrainz::Server::Edit::Exceptions::GeneralError'; + is $exception->message, 'Characters cannot be marked as ended.'; +}; + +test 'Fails edits trying to set an end area to a character artist' => sub { + my $test = shift; + my $c = $test->c; + + $c->sql->do(<<~'SQL'); + INSERT INTO artist (id, gid, name, sort_name) + VALUES (2, 'cdf5588d-cca8-4e0c-bae1-d53bc73b012a', 'person', 'person'); + INSERT INTO area (id, gid, name, type) + VALUES (221, '8a754a16-0027-3a29-b6d7-2b40ea0481ed', 'United Kingdom', 1); + SQL + + my $edit = $c->model('Edit')->create( + edit_type => $EDIT_ARTIST_EDIT, + editor_id => 1, + to_edit => $c->model('Artist')->get_by_id(2), + end_area_id => 221, + ipi_codes => [], + isni_codes => [], + privileges => $UNTRUSTED_FLAG, + ); + + ok($edit->is_open); + $c->sql->do('UPDATE artist SET type = 4 WHERE id = 2'); + + my $exception = exception { $edit->accept }; + isa_ok $exception, 'MusicBrainz::Server::Edit::Exceptions::GeneralError'; + is $exception->message, 'Characters cannot have an end area.'; +}; + sub _create_full_edit { my ($c, $artist) = @_; return $c->model('Edit')->create( diff --git a/t/lib/t/MusicBrainz/Server/Edit/Artist/Merge.pm b/t/lib/t/MusicBrainz/Server/Edit/Artist/Merge.pm index fddaa727c14..62045a246f7 100644 --- a/t/lib/t/MusicBrainz/Server/Edit/Artist/Merge.pm +++ b/t/lib/t/MusicBrainz/Server/Edit/Artist/Merge.pm @@ -207,6 +207,198 @@ test 'Merging a group, and an artist with no type and a gender, into an artist w ); }; +test 'Merging an artist with no type and an end date into a character' => sub { + my $test = shift; + my $c = $test->c; + + MusicBrainz::Server::Test->prepare_test_database($c, '+edit_artist_merge'); + $c->sql->do(<<~'SQL'); + UPDATE artist SET type = 4, end_date_year = NULL WHERE id = 4; + UPDATE artist SET type = NULL, end_date_year = 1999 WHERE id = 3; + SQL + + # merge 3 -> 4 + my $edit = create_edit($c); + + ok(!exception { accept_edit($c, $edit) }, + 'Edit merging an artist with no type and an end date into a character does not cause an exception'); + + my $row = $c->sql->select_single_row_hash('SELECT * FROM artist WHERE id = 4'); + is($row->{type}, 4, 'The resulting type is a character'); + is($row->{end_date_year}, undef, 'The resulting end date year is null'); + + $c->model('EditNote')->load_for_edits($edit); + is(scalar $edit->all_edit_notes, 1); + + my $note = scalar($edit->all_edit_notes) ? $edit->edit_notes->[0] : undef; + is( + defined $note && $note->localize, + 'The following data has not been added to the destination artist ' . + 'because it conflicted with other data: Ended: Yes and End date: 1999. ' . + 'Characters cannot have an end date nor area nor be marked as ended.', + ); +}; + +test 'Merging an artist with no type and an end area into a character' => sub { + my $test = shift; + my $c = $test->c; + + MusicBrainz::Server::Test->prepare_test_database($c, '+edit_artist_merge'); + $c->sql->do(<<~'SQL'); + UPDATE artist SET type = 4, end_area = NULL WHERE id = 4; + UPDATE artist SET type = NULL, end_area = 221 WHERE id = 3; + SQL + + # merge 3 -> 4 + my $edit = create_edit($c); + + ok(!exception { accept_edit($c, $edit) }, + 'Edit merging an artist with no type and an end date into a character does not cause an exception'); + + my $row = $c->sql->select_single_row_hash('SELECT * FROM artist WHERE id = 4'); + is($row->{type}, 4, 'The resulting type is a character'); + is($row->{end_area}, undef, 'The resulting end area is null'); + + $c->model('EditNote')->load_for_edits($edit); + is(scalar $edit->all_edit_notes, 1); + + my $note = scalar($edit->all_edit_notes) ? $edit->edit_notes->[0] : undef; + is( + defined $note && $note->localize, + 'The following data has not been added to the destination artist because ' . + 'it conflicted with other data: Ended: Yes and End area: United Kingdom. ' . + 'Characters cannot have an end date nor area nor be marked as ended.', + ); +}; + +test 'Merging an artist with no type and marked as ended into a character' => sub { + my $test = shift; + my $c = $test->c; + + MusicBrainz::Server::Test->prepare_test_database($c, '+edit_artist_merge'); + $c->sql->do(<<~'SQL'); + UPDATE artist SET type = 4, ended = FALSE WHERE id = 4; + UPDATE artist SET type = NULL, ended = TRUE WHERE id = 3; + SQL + + # merge 3 -> 4 + my $edit = create_edit($c); + + ok(!exception { accept_edit($c, $edit) }, + 'Edit merging an artist with no type and an end date into a character does not cause an exception'); + + my $row = $c->sql->select_single_row_hash('SELECT * FROM artist WHERE id = 4'); + is($row->{type}, 4, 'The resulting type is a character'); + is($row->{ended}, 0, 'The resulting ended value is false'); + + $c->model('EditNote')->load_for_edits($edit); + is(scalar $edit->all_edit_notes, 1); + + my $note = scalar($edit->all_edit_notes) ? $edit->edit_notes->[0] : undef; + is( + defined $note && $note->localize, + 'The following data has not been added to the destination artist ' . + 'because it conflicted with other data: Ended: Yes. ' . + 'Characters cannot have an end date nor area nor be marked as ended.', + ); +}; + +test 'Merging a character into an artist with no type and an end date' => sub { + my $test = shift; + my $c = $test->c; + + MusicBrainz::Server::Test->prepare_test_database($c, '+edit_artist_merge'); + $c->sql->do(<<~'SQL'); + UPDATE artist SET type = NULL, end_date_year = 1999 WHERE id = 4; + UPDATE artist SET type = 4, end_date_year = NULL WHERE id = 3; + SQL + + # merge 3 -> 4 + my $edit = create_edit($c); + + ok(!exception { accept_edit($c, $edit) }, + 'Edit merging a character into an artist with no type and an end date does not cause an exception'); + + my $row = $c->sql->select_single_row_hash('SELECT * FROM artist WHERE id = 4'); + is($row->{type}, undef, 'The resulting type is null'); + is($row->{end_date_year}, 1999, 'The resulting end date year is 1999'); + + $c->model('EditNote')->load_for_edits($edit); + is(scalar $edit->all_edit_notes, 1); + + my $note = scalar($edit->all_edit_notes) ? $edit->edit_notes->[0] : undef; + is( + defined $note && $note->localize, + 'The following data has not been added to the destination artist ' . + 'because it conflicted with other data: Type: Character. ' . + 'Characters cannot have an end date nor area nor be marked as ended.', + ); +}; + +test 'Merging a character into an artist with no type and an end area' => sub { + my $test = shift; + my $c = $test->c; + + MusicBrainz::Server::Test->prepare_test_database($c, '+edit_artist_merge'); + $c->sql->do(<<~'SQL'); + UPDATE artist SET type = NULL, end_area = 221 WHERE id = 4; + UPDATE artist SET type = 4, end_area = NULL WHERE id = 3; + SQL + + # merge 3 -> 4 + my $edit = create_edit($c); + + ok(!exception { accept_edit($c, $edit) }, + 'Edit merging a character into an artist with no type and an end date does not cause an exception'); + + my $row = $c->sql->select_single_row_hash('SELECT * FROM artist WHERE id = 4'); + is($row->{type}, undef, 'The resulting type is null'); + is($row->{end_area}, 221, 'The resulting end area is the United Kingdom'); + + $c->model('EditNote')->load_for_edits($edit); + is(scalar $edit->all_edit_notes, 1); + + my $note = scalar($edit->all_edit_notes) ? $edit->edit_notes->[0] : undef; + is( + defined $note && $note->localize, + 'The following data has not been added to the destination artist ' . + 'because it conflicted with other data: Type: Character. ' . + 'Characters cannot have an end date nor area nor be marked as ended.', + ); +}; + +test 'Merging a character into an artist with no type marked as ended' => sub { + my $test = shift; + my $c = $test->c; + + MusicBrainz::Server::Test->prepare_test_database($c, '+edit_artist_merge'); + $c->sql->do(<<~'SQL'); + UPDATE artist SET type = NULL, ended = TRUE WHERE id = 4; + UPDATE artist SET type = 4, ended = FALSE WHERE id = 3; + SQL + + # merge 3 -> 4 + my $edit = create_edit($c); + + ok(!exception { accept_edit($c, $edit) }, + 'Edit merging a character into an artist with no type and an end date does not cause an exception'); + + my $row = $c->sql->select_single_row_hash('SELECT * FROM artist WHERE id = 4'); + is($row->{type}, undef, 'The resulting type is null'); + is($row->{ended}, 1, 'The resulting ended value is true'); + + $c->model('EditNote')->load_for_edits($edit); + is(scalar $edit->all_edit_notes, 1); + + my $note = scalar($edit->all_edit_notes) ? $edit->edit_notes->[0] : undef; + is( + defined $note && $note->localize, + 'The following data has not been added to the destination artist ' . + 'because it conflicted with other data: Type: Character. ' . + 'Characters cannot have an end date nor area nor be marked as ended.', + ); +}; + # The name of this test may be confusing, since the code should do the opposite # of what is understood to happen in the UI. By "renaming" the credits, the code # should do nothing and leave them empty, so that they take on the new artist diff --git a/t/sql/edit_artist_merge.sql b/t/sql/edit_artist_merge.sql index eb323243d27..8ed6e6b1b61 100644 --- a/t/sql/edit_artist_merge.sql +++ b/t/sql/edit_artist_merge.sql @@ -25,3 +25,6 @@ INSERT INTO recording (id, gid, name, artist_credit, length) INSERT INTO link (id, link_type, attribute_count) VALUES (1, 156, 0); INSERT INTO l_artist_recording (id, link, entity0, entity1) VALUES (1, 1, 3, 1); + +INSERT INTO area (id, gid, name, type) VALUES + (221, '8a754a16-0027-3a29-b6d7-2b40ea0481ed', 'United Kingdom', 1); diff --git a/t/tests.t b/t/tests.t index 94b28f8039d..431a958ccb6 100644 --- a/t/tests.t +++ b/t/tests.t @@ -26,7 +26,7 @@ MusicBrainz::Server::Test->prepare_test_server; @classes = commandline_override('t::MusicBrainz::Server::', @classes); # Image editing temporarily disabled -@classes = grep { $_ !~ /Art/ } @classes; +# @classes = grep { $_ !~ /Art/ } @classes; plan tests => scalar(@classes); run_tests($_ => $_) for @classes; diff --git a/upgrade.json b/upgrade.json index 80853eb68d6..085414f3b7a 100644 --- a/upgrade.json +++ b/upgrade.json @@ -230,5 +230,10 @@ "20240223-mbs-13421-fks.sql", "20240319-mbs-13514.sql" ] + }, + "30": { + "master_and_standalone": [ + "20241104-mbs-9885.sql" + ] } }