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/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/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/static/scripts/edit/MB/Control/ArtistEdit.js b/root/static/scripts/edit/MB/Control/ArtistEdit.js index df98b8b4111..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); @@ -54,6 +65,7 @@ MB.Control.ArtistEdit = function () { ); self.changeAreaText(l('Born in:'), l('Died in:')); self.enableGender(); + self.enableEndSection(); break; case '2': @@ -69,6 +81,7 @@ MB.Control.ArtistEdit = function () { addColonText(lp('Dissolved in', 'group artist')), ); self.disableGender(); + self.enableEndSection(); break; case '4': @@ -82,6 +95,7 @@ MB.Control.ArtistEdit = function () { addColonText(l('End area')), ); self.enableGender(); + self.disableEndSection(); break; case '0': @@ -96,6 +110,7 @@ MB.Control.ArtistEdit = function () { addColonText(l('End area')), ); self.enableGender(); + self.enableEndSection(); break; } }; @@ -114,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/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;