diff --git a/lib/MusicBrainz/Server/Controller/Admin.pm b/lib/MusicBrainz/Server/Controller/Admin.pm index 087e78e0247..0bcbd4ea141 100644 --- a/lib/MusicBrainz/Server/Controller/Admin.pm +++ b/lib/MusicBrainz/Server/Controller/Admin.pm @@ -7,7 +7,13 @@ use Try::Tiny; extends 'MusicBrainz::Server::Controller'; -use MusicBrainz::Server::Constants qw( :privileges ); +use DBDefs; + +use MusicBrainz::Server::Constants qw( + :privileges + $VOTE_ADMIN_APPROVE + $VOTE_ADMIN_REJECT +); use MusicBrainz::Server::ControllerUtils::JSON qw( serialize_pager ); use MusicBrainz::Server::Data::Utils qw( boolean_to_json ); @@ -340,6 +346,63 @@ sub unlock_username : Path('/admin/locked-usernames/unlock') Args(1) RequireAuth ); } +sub accept_edit : Path('/admin/accept-edit') Args(1) RequireAuth(account_admin) +{ + my ($self, $c, $edit_id) = @_; + + my $edit = $c->model('Edit')->get_by_id($edit_id) + or $c->detach('/error_404'); + + _accept_edit($c, $edit) if $edit->is_open; + $c->response->redirect($c->uri_for_action('/edit/show', [ $edit->id ])); +} + +sub reject_edit : Path('/admin/reject-edit') Args(1) RequireAuth(account_admin) +{ + my ($self, $c, $edit_id) = @_; + + my $edit = $c->model('Edit')->get_by_id($edit_id) + or $c->detach('/error_404'); + + _reject_edit($c, $edit) if $edit->is_open; + $c->response->redirect($c->uri_for_action('/edit/show', [ $edit->id ])); +} + +sub _accept_edit +{ + my ($c, $edit) = @_; + + my $sql = $c->model('MB')->context->sql; + + Sql::run_in_transaction( sub { + $c->model('Vote')->enter_votes( + $c->user, + [{ + vote => $VOTE_ADMIN_APPROVE, + edit_id => $edit->id, + }], + ); + $c->model('Edit')->accept($edit); + }, $sql ); +} + +sub _reject_edit +{ + my ($c, $edit) = @_; + + my $sql = $c->model('MB')->context->sql; + Sql::run_in_transaction( sub { + $c->model('Vote')->enter_votes( + $c->user, + [{ + vote => $VOTE_ADMIN_REJECT, + edit_id => $edit->id, + }], + ); + $c->model('Edit')->reject($edit); + }, $sql ); +} + 1; =head1 COPYRIGHT AND LICENSE diff --git a/lib/MusicBrainz/Server/Data/Vote.pm b/lib/MusicBrainz/Server/Data/Vote.pm index b600f81ee90..0018f73bc28 100644 --- a/lib/MusicBrainz/Server/Data/Vote.pm +++ b/lib/MusicBrainz/Server/Data/Vote.pm @@ -73,9 +73,20 @@ sub enter_votes # not sufficient to filter the vote because the actual approval is happening elsewhere confess 'Unauthorized editor ' . $editor->id . ' tried to approve edit #' . $_->{edit_id}; } + if (any { $_->{vote} == $VOTE_ADMIN_APPROVE && !$editor->is_account_admin } @votes) { + # not sufficient to filter the vote because the actual approval is happening elsewhere + confess 'Unauthorized editor ' . $editor->id . ' tried to admin-approve edit #' . $_->{edit_id}; + } + if (any { $_->{vote} == $VOTE_ADMIN_REJECT && !$editor->is_account_admin } @votes) { + # not sufficient to filter the vote because the actual rejection is happening elsewhere + confess 'Unauthorized editor ' . $editor->id . ' tried to admin-reject edit #' . $_->{edit_id}; + } unless ($opts{override_privs}) { @votes = grep { - $_->{vote} == $VOTE_APPROVE || $edits->{ $_->{edit_id} }->editor_may_vote_on_edit($editor) + $_->{vote} == $VOTE_APPROVE || + $_->{vote} == $VOTE_ADMIN_APPROVE || + $_->{vote} == $VOTE_ADMIN_REJECT || + $edits->{ $_->{edit_id} }->editor_may_vote_on_edit($editor) } @votes; } diff --git a/root/edit/EditIndex.js b/root/edit/EditIndex.js index 9d3cc25d787..a911b8655ae 100644 --- a/root/edit/EditIndex.js +++ b/root/edit/EditIndex.js @@ -17,6 +17,8 @@ import EditorLink from '../static/scripts/common/components/EditorLink.js'; import {DB_STAGING_TESTING_FEATURES} from '../static/scripts/common/DBDefs.mjs'; import linkedEntities from '../static/scripts/common/linkedEntities.mjs'; +import {isAccountAdmin} + from '../static/scripts/common/utility/privileges.js'; import FormSubmit from '../static/scripts/edit/components/FormSubmit.js'; import getVoteName from '../static/scripts/edit/utility/getVoteName.js'; import {editorMayAddNote, editorMayVoteOnEdit} @@ -36,10 +38,12 @@ component EditIndex( fullWidth: boolean = false, ) { const $c = React.useContext(CatalystContext); + const isAdmin = isAccountAdmin($c.user); const canAddNote = Boolean($c.user && editorMayAddNote(edit, $c.user)); const isOwnEdit = Boolean($c.user && $c.user.id === edit.editor_id); const canVoteHere = Boolean($c.user && editorMayVoteOnEdit(edit, $c.user)); const detailsElement = getEditDetailsElement(edit); + const showAcceptReject = DB_STAGING_TESTING_FEATURES || isAdmin; return ( @@ -123,26 +127,44 @@ component EditIndex( ) : null} {$c.user ? ( - edit.is_open && DB_STAGING_TESTING_FEATURES ? ( - <> -

{l('Testing features')}

-

- {l(`To aid in testing, the following features - have been made available on testing servers:`)} -

- - + edit.is_open && showAcceptReject ? ( + isAdmin ? ( + <> +

{l_admin('Admin features')}

+ + + ) : ( + <> +

{l('Testing features')}

+

+ {l(`To aid in testing, the following features + have been made available on testing servers:`)} +

+ + + ) ) : null ) : (

diff --git a/root/edit/components/EditSummary.js b/root/edit/components/EditSummary.js index 585659feafa..9e38a2bb65e 100644 --- a/root/edit/components/EditSummary.js +++ b/root/edit/components/EditSummary.js @@ -18,6 +18,8 @@ import { DB_READ_ONLY, DB_STAGING_TESTING_FEATURES, } from '../../static/scripts/common/DBDefs.mjs'; +import {isAccountAdmin} + from '../../static/scripts/common/utility/privileges.js'; import { editorMayAddNote, editorMayApprove, @@ -34,9 +36,11 @@ component EditSummary( ) { const $c = React.useContext(CatalystContext); const user = $c.user; + const isAdmin = isAccountAdmin(user); const mayAddNote = editorMayAddNote(edit, user); const mayApprove = editorMayApprove(edit, user); const mayCancel = editorMayCancel(edit, user); + const showAcceptReject = DB_STAGING_TESTING_FEATURES || isAdmin; return ( <> @@ -77,8 +81,23 @@ component EditSummary( ) : null} - {edit.status === EDIT_STATUS_OPEN && - DB_STAGING_TESTING_FEATURES ? ( + {edit.status === EDIT_STATUS_OPEN && showAcceptReject ? ( + isAdmin ? ( + <> + + {l('Accept edit')} + + + {l('Reject edit')} + + + ) : ( <> - ) : null} + ) + ) : null} ) : null} ); diff --git a/root/user/PrivilegedUsers.js b/root/user/PrivilegedUsers.js index 5ea46a9526d..1abef3b91bd 100644 --- a/root/user/PrivilegedUsers.js +++ b/root/user/PrivilegedUsers.js @@ -110,7 +110,8 @@ component PrivilegedUsers(

{lp('Account administrators', 'header')}

- {l('Account administrators can edit and delete user accounts.')} + {l(`Account administrators can edit and delete user accounts, + and immediately accept and reject edits.`)}

{texp.l( diff --git a/t/lib/t/MusicBrainz/Server/Controller/Admin/AcceptRejectEdits.pm b/t/lib/t/MusicBrainz/Server/Controller/Admin/AcceptRejectEdits.pm new file mode 100644 index 00000000000..50c8dc805aa --- /dev/null +++ b/t/lib/t/MusicBrainz/Server/Controller/Admin/AcceptRejectEdits.pm @@ -0,0 +1,198 @@ +package t::MusicBrainz::Server::Controller::Admin::AcceptRejectEdits; +use strict; +use warnings; + +use HTTP::Status qw( :constants ); +use Test::Routine; +use Test::More; +use MusicBrainz::Server::Constants qw( + :edit_status + :vote + $EDIT_ARTIST_EDIT + $UNTRUSTED_FLAG +); + +with 't::Mechanize', 't::Context'; + +=head2 Test description + +This test checks whether admins can accept and reject any edits, and ensures +non-admins are not able to do so. + +=cut + +test 'Try to accept/reject edit as a regular user' => sub { + my $test = shift; + my $mech = $test->mech; + my $edit = prepare($test); + + $mech->get_ok('/login'); + $mech->submit_form( with_fields => { + username => 'editor1', + password => 'pass', + } ); + + $mech->get('/'); + $mech->content_contains('editor1', 'Non-admin user "editor1" is logged in'); + + $mech->get( + '/admin/accept-edit/' . $edit->id, + q(Try to access the accept edit endpoint as a non-admin), + ); + is($mech->status, HTTP_FORBIDDEN, + 'The attempt to access the endpoint was rejected'); + + $edit = $test->c->model('Edit')->get_by_id($edit->id); + $test->c->model('Vote')->load_for_edits($edit); + is($edit->status, $STATUS_OPEN, 'The edit is still open'); + is(scalar @{ $edit->votes }, 0, 'The vote count is still 0'); + + $mech->get( + '/admin/reject-edit/' . $edit->id, + q(Try to access the reject edit endpoint as a non-admin), + ); + is($mech->status, HTTP_FORBIDDEN, + 'The attempt to access the endpoint was rejected'); + + $edit = $test->c->model('Edit')->get_by_id($edit->id); + $test->c->model('Vote')->load_for_edits($edit); + is($edit->status, $STATUS_OPEN, 'The edit is still open'); + is(scalar @{ $edit->votes }, 0, 'The vote count is still 0'); +}; + +test 'Try to accept an edit as an admin' => sub { + my $test = shift; + my $mech = $test->mech; + my $edit = prepare($test); + + $mech->get_ok('/login'); + $mech->submit_form( with_fields => { + username => 'editor7', + password => 'pass', + } ); + + $mech->get('/'); + $mech->content_contains('editor7', 'Admin user "editor7" is logged in'); + + $mech->get_ok( + '/admin/accept-edit/' . $edit->id, + q(Admin can access the accept edit endpoint for other's edits), + ); + + $edit = $test->c->model('Edit')->get_by_id($edit->id); + is($edit->status, $STATUS_APPLIED, 'applied'); + $test->c->model('Vote')->load_for_edits($edit); + is($edit->votes->[0]->vote, $VOTE_ADMIN_APPROVE, 'Vote is Admin approval'); + is($edit->votes->[0]->editor_id, 7, 'Vote is by editor7'); +}; + +test 'Try to reject an edit as an admin' => sub { + my $test = shift; + my $mech = $test->mech; + my $edit = prepare($test); + + $mech->get_ok('/login'); + $mech->submit_form( with_fields => { + username => 'editor7', + password => 'pass', + } ); + + $mech->get('/'); + $mech->content_contains('editor7', 'Admin user "editor7" is logged in'); + + $mech->get_ok( + '/admin/reject-edit/' . $edit->id, + q(Admin can access the reject edit endpoint for other's edits), + ); + + $edit = $test->c->model('Edit')->get_by_id($edit->id); + is($edit->status, $STATUS_FAILEDVOTE, 'Rejected with Failed vote'); + $test->c->model('Vote')->load_for_edits($edit); + is($edit->votes->[0]->vote, $VOTE_ADMIN_REJECT, 'Vote is Admin rejection'); + is($edit->votes->[0]->editor_id, 7, 'Vote is by editor7'); +}; + +test 'Try to accept own edit as an admin' => sub { + my $test = shift; + my $mech = $test->mech; + my $edit = prepare($test, 7); + + $mech->get_ok('/login'); + $mech->submit_form( with_fields => { + username => 'editor7', + password => 'pass', + } ); + + $mech->get('/'); + $mech->content_contains('editor7', 'Admin user "editor7" is logged in'); + + $mech->get_ok( + '/admin/accept-edit/' . $edit->id, + q(Admin can access the accept edit endpoint for own edits), + ); + + $edit = $test->c->model('Edit')->get_by_id($edit->id); + is($edit->status, $STATUS_APPLIED, 'applied'); + $test->c->model('Vote')->load_for_edits($edit); + is($edit->votes->[0]->vote, $VOTE_ADMIN_APPROVE, 'Vote is Admin approval'); + is($edit->votes->[0]->editor_id, 7, 'Vote is by editor7'); +}; + +test 'Try to reject own edit as an admin' => sub { + my $test = shift; + my $mech = $test->mech; + my $edit = prepare($test, 7); + + $mech->get_ok('/login'); + $mech->submit_form( with_fields => { + username => 'editor7', + password => 'pass', + } ); + + $mech->get('/'); + $mech->content_contains('editor7', 'Admin user "editor7" is logged in'); + + $mech->get_ok( + '/admin/reject-edit/' . $edit->id, + q(Admin can access the reject edit endpoint for own edits), + ); + + $edit = $test->c->model('Edit')->get_by_id($edit->id); + is($edit->status, $STATUS_FAILEDVOTE, 'Rejected with Failed vote'); + $test->c->model('Vote')->load_for_edits($edit); + is($edit->votes->[0]->vote, $VOTE_ADMIN_REJECT, 'Vote is Admin rejection'); + is($edit->votes->[0]->editor_id, 7, 'Vote is by editor7'); +}; + +sub prepare { + my ($test, $editor_id) = @_; + + local *DBDefs::DB_STAGING_TESTING_FEATURES = sub { 0 }; + my $c = $test->c; + + MusicBrainz::Server::Test->prepare_test_database($test->c, '+vote'); + + my $edit = $test->c->model('Edit')->create( + editor_id => $editor_id // 1, + edit_type => $EDIT_ARTIST_EDIT, + to_edit => $c->model('Artist')->get_by_id(1), + comment => 'Changed comment', + ipi_codes => [], + isni_codes => [], + privileges => $UNTRUSTED_FLAG, + ); + + return $edit; +} + +1; + +=head1 COPYRIGHT AND LICENSE + +Copyright (C) 2024 MetaBrainz Foundation + +This file is part of MusicBrainz, the open internet music database, +and is licensed under the GPL version 2, or (at your option) any +later version: http://www.gnu.org/licenses/gpl-2.0.txt + +=cut diff --git a/t/lib/t/MusicBrainz/Server/Controller/Edit/Show.pm b/t/lib/t/MusicBrainz/Server/Controller/Edit/Show.pm index 7fa5d25d949..d34a495fdc0 100644 --- a/t/lib/t/MusicBrainz/Server/Controller/Edit/Show.pm +++ b/t/lib/t/MusicBrainz/Server/Controller/Edit/Show.pm @@ -52,6 +52,14 @@ test 'Check edit page displays basic components' => sub { 'Edit by ', 'The edit page lists the editor who entered the edit', ); + $mech->content_lacks( + '/admin/accept-edit/' . $edit->id, + 'The edit page lacks the button for the admin to accept the edit', + ); + $mech->content_lacks( + '/admin/reject-edit/' . $edit->id, + 'The edit page lacks the button for the admin to reject the edit', + ); }; test 'Check edit page differences for own edit' => sub { @@ -79,6 +87,14 @@ test 'Check edit page differences for own edit' => sub { 'You are not currently able to vote on this edit', 'The message about not being able to vote is not present', ); + $mech->content_lacks( + '/admin/accept-edit/' . $edit->id, + 'The edit page lacks the button for the admin to accept the edit', + ); + $mech->content_lacks( + '/admin/reject-edit/' . $edit->id, + 'The edit page lacks the button for the admin to reject the edit', + ); }; test 'Check edit page differences for editor without confirmed email' => sub { @@ -106,6 +122,14 @@ test 'Check edit page differences for editor without confirmed email' => sub { 'You are not currently able to add notes to this edit', 'The message about not being able to add notes is present', ); + $mech->content_lacks( + '/admin/accept-edit/' . $edit->id, + 'The edit page lacks the button for the admin to accept the edit', + ); + $mech->content_lacks( + '/admin/reject-edit/' . $edit->id, + 'The edit page lacks the button for the admin to reject the edit', + ); }; test 'Check edit page differences for beginner editor' => sub { @@ -133,6 +157,14 @@ test 'Check edit page differences for beginner editor' => sub { 'You are not currently able to vote on this edit', 'The message about not being able to vote is present', ); + $mech->content_lacks( + '/admin/accept-edit/' . $edit->id, + 'The edit page lacks the button for the admin to accept the edit', + ); + $mech->content_lacks( + '/admin/reject-edit/' . $edit->id, + 'The edit page lacks the button for the admin to reject the edit', + ); }; test 'Check edit page differences when logged out' => sub { @@ -166,16 +198,87 @@ test 'Check edit page differences when logged out' => sub { 'Raw edit data for this edit', 'The raw data link is hidden', ); + $mech->content_lacks( + '/admin/accept-edit/' . $edit->id, + 'The edit page lacks the button for the admin to accept the edit', + ); + $mech->content_lacks( + '/admin/reject-edit/' . $edit->id, + 'The edit page lacks the button for the admin to reject the edit', + ); }; -sub prepare { +test 'Check edit page differences for account admin' => sub { my $test = shift; + my $mech = $test->mech; + my $edit = prepare($test); + + $mech->get_ok('/login'); + $mech->submit_form( with_fields => { + username => 'editor7', + password => 'pass', + } ); + + $mech->get_ok( + '/edit/' . $edit->id, + 'Fetched edit page as an account admin other than the edit author', + ); + html_ok($mech->content); + + $mech->content_contains( + 'Submit vote and note', + 'The edit page contains the button to vote and add an edit note', + ); + $mech->content_contains( + '/admin/accept-edit/' . $edit->id, + 'The edit page contains the button for the admin to accept the edit', + ); + $mech->content_contains( + '/admin/reject-edit/' . $edit->id, + 'The edit page contains the button for the admin to reject the edit', + ); +}; + +test 'Check edit page differences for account admin on own edit' => sub { + my $test = shift; + my $mech = $test->mech; + my $edit = prepare($test, 7); + + $mech->get_ok('/login'); + $mech->submit_form( with_fields => { + username => 'editor7', + password => 'pass', + } ); + + $mech->get_ok( + '/edit/' . $edit->id, + 'Fetched edit page as an account admin who is the edit author', + ); + html_ok($mech->content); + + $mech->content_contains( + 'Submit note', + 'The edit page contains the button to add an edit note', + ); + $mech->content_contains( + 'Accept edit', + 'The edit page contains the button for the admin to accept the edit', + ); + $mech->content_contains( + 'Reject edit', + 'The edit page contains the button for the admin to reject the edit', + ); +}; + +sub prepare { + my ($test, $editor_id) = @_; + my $c = $test->c; MusicBrainz::Server::Test->prepare_test_database($test->c, '+vote'); my $edit = $test->c->model('Edit')->create( - editor_id => 1, + editor_id => $editor_id // 1, edit_type => $EDIT_ARTIST_EDIT, to_edit => $c->model('Artist')->get_by_id(1), comment => 'Changed comment', diff --git a/t/lib/t/MusicBrainz/Server/Controller/UnconfirmedEmailAddresses.pm b/t/lib/t/MusicBrainz/Server/Controller/UnconfirmedEmailAddresses.pm index 760a7c8109b..500a2dda7f4 100644 --- a/t/lib/t/MusicBrainz/Server/Controller/UnconfirmedEmailAddresses.pm +++ b/t/lib/t/MusicBrainz/Server/Controller/UnconfirmedEmailAddresses.pm @@ -84,6 +84,7 @@ test 'Paths that allow browsing without a confirmed email address' => sub { 'Controller::Admin::StatisticsEvent::index', 'Controller::Admin::WikiDoc::history', 'Controller::Admin::WikiDoc::index', + 'Controller::Admin::accept_edit', 'Controller::Admin::delete_user', 'Controller::Admin::edit_banner', 'Controller::Admin::edit_user', @@ -91,6 +92,7 @@ test 'Paths that allow browsing without a confirmed email address' => sub { 'Controller::Admin::ip_lookup', 'Controller::Admin::locked_username_search', 'Controller::Admin::privilege_search', + 'Controller::Admin::reject_edit', 'Controller::Admin::unlock_username', 'Controller::Ajax::filter_artist_events_form', 'Controller::Ajax::filter_artist_recordings_form', diff --git a/t/sql/vote.sql b/t/sql/vote.sql index d5ec7f34e2f..029a6aada24 100644 --- a/t/sql/vote.sql +++ b/t/sql/vote.sql @@ -11,4 +11,6 @@ INSERT INTO editor (id, name, password, email, ha1, email_confirm_date, member_s -- Non-verified editor (5, 'editor5', '{CLEARTEXT}pass', null, '01de7bc91330d78a6d0a84033e293f15', null, '2014-12-03', 0), -- Beginner editor - (6, 'editor6', '{CLEARTEXT}pass', 'editor6@example.com', '01de7bc91330d78a6d0a84033e293f11', now(), '2014-12-03', 8192); + (6, 'editor6', '{CLEARTEXT}pass', 'editor6@example.com', '01de7bc91330d78a6d0a84033e293f11', now(), '2014-12-03', 8192), + -- Account admin + (7, 'editor7', '{CLEARTEXT}pass', 'editor7@example.com', '01de7bc91330d78a6d0a84033e293f12', now(), '2014-12-03', 128);