From a32397feda27d670b2e33e7e68693a1d5d176c28 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Fri, 15 Dec 2023 12:11:54 +0000 Subject: [PATCH] MDL-80328 user: use modal confirmation for admin deleting users. --- .../local/systemreports/users.php | 28 +++++++++++++++++-- admin/tests/behat/browse_users.feature | 7 +++-- admin/user.php | 4 +-- grade/tests/behat/grade_average.feature | 8 +----- lang/en/admin.php | 1 + user/tests/behat/bulk_editenrolment.feature | 8 +----- user/tests/behat/delete_users.feature | 8 +----- 7 files changed, 36 insertions(+), 28 deletions(-) diff --git a/admin/classes/reportbuilder/local/systemreports/users.php b/admin/classes/reportbuilder/local/systemreports/users.php index df6616079ed..680b1c5f047 100644 --- a/admin/classes/reportbuilder/local/systemreports/users.php +++ b/admin/classes/reportbuilder/local/systemreports/users.php @@ -28,6 +28,7 @@ use core_reportbuilder\local\report\action; use core_reportbuilder\local\report\filter; use core_reportbuilder\system_report; use core_role\reportbuilder\local\entities\role; +use core_user\fields; use lang_string; use moodle_url; use pix_icon; @@ -62,8 +63,9 @@ class users extends system_report { $this->add_entity($entityuser); // Any columns required by actions should be defined here to ensure they're always available. + $fullnamefields = array_map(fn($field) => "{$entityuseralias}.{$field}", fields::get_name_fields()); $this->add_base_fields("{$entityuseralias}.id, {$entityuseralias}.confirmed, {$entityuseralias}.mnethostid, - {$entityuseralias}.suspended, {$entityuseralias}.username"); + {$entityuseralias}.suspended, {$entityuseralias}.username, " . implode(', ', $fullnamefields)); $paramguest = database::generate_param_name(); $this->add_base_condition_sql("{$entityuseralias}.deleted <> 1 AND {$entityuseralias}.id <> :{$paramguest}", @@ -332,10 +334,32 @@ class users extends system_report { $this->add_action((new action( new moodle_url('/admin/user.php', ['delete' => ':id', 'sesskey' => sesskey()]), new pix_icon('t/delete', ''), - ['class' => 'text-danger'], + [ + 'class' => 'text-danger', + 'data-modal' => 'confirmation', + 'data-modal-title-str' => json_encode(['deleteuser', 'admin']), + 'data-modal-content-str' => ':deletestr', + 'data-modal-yes-button-str' => json_encode(['delete', 'core']), + 'data-modal-destination' => ':deleteurl', + + ], false, new lang_string('delete', 'moodle'), ))->add_callback(static function(\stdclass $row) use ($USER, $contextsystem): bool { + + // Populate deletion modal attributes. + $row->deletestr = json_encode([ + 'deletecheckfull', + 'moodle', + fullname($row, true), + ]); + + $row->deleteurl = (new moodle_url('/admin/user.php', [ + 'delete' => $row->id, + 'confirm' => md5($row->id), + 'sesskey' => sesskey(), + ]))->out(false); + return has_capability('moodle/user:delete', $contextsystem) && !is_mnet_remote_user($row) && $row->id != $USER->id && !is_siteadmin($row); })); diff --git a/admin/tests/behat/browse_users.feature b/admin/tests/behat/browse_users.feature index 0bc5806b238..dd2d3fcb750 100644 --- a/admin/tests/behat/browse_users.feature +++ b/admin/tests/behat/browse_users.feature @@ -79,9 +79,10 @@ Feature: An administrator can browse user accounts Scenario: Delete a user account Given I navigate to "Users > Accounts > Browse list of users" in site administration And I press "Delete" action in the "User One" report row - And I should see "Are you absolutely sure you want to completely delete the user 'User One'" - And I press "Delete" - Then I should not see "User One" in the "reportbuilder-table" "table" + And I should see "Are you absolutely sure you want to completely delete the user User One" in the "Delete user" "dialogue" + And I click on "Delete" "button" in the "Delete user" "dialogue" + Then I should see "Deleted user User One" + And I should not see "User One" in the "reportbuilder-table" "table" @javascript Scenario: Resend email and confirm a user account diff --git a/admin/user.php b/admin/user.php index c33020f3343..4e838e69d43 100644 --- a/admin/user.php +++ b/admin/user.php @@ -89,10 +89,10 @@ echo $OUTPUT->confirm(get_string('deletecheckfull', '', "'$fullname'"), $deletebutton, $returnurl); echo $OUTPUT->footer(); die; - } else if (data_submitted()) { + } else { if (delete_user($user)) { \core\session\manager::gc(); // Remove stale sessions. - redirect($returnurl); + redirect($returnurl, get_string('deleteduserx', 'admin', fullname($user, true))); } else { \core\session\manager::gc(); // Remove stale sessions. echo $OUTPUT->header(); diff --git a/grade/tests/behat/grade_average.feature b/grade/tests/behat/grade_average.feature index 18066632364..3fadd95ced4 100644 --- a/grade/tests/behat/grade_average.feature +++ b/grade/tests/behat/grade_average.feature @@ -42,14 +42,8 @@ Feature: Average grades are displayed in the gradebook | student2 | C1 | student | suspended | And I log in as "admin" And I navigate to "Users > Accounts > Browse list of users" in site administration - And I click on "Filters" "button" - And I set the following fields in the "Username" "core_reportbuilder > Filter" to these values: - | Username operator | Is equal to | - | Username value | student5 | - And I click on "Apply" "button" in the "[data-region='report-filters']" "css_element" - And I click on "Filters" "button" And I press "Delete" action in the "Student 5" report row - And I press "Delete" + And I click on "Delete" "button" in the "Delete user" "dialogue" # Enable averages And I am on the "Course 1" "grades > course grade settings" page And I set the following fields to these values: diff --git a/lang/en/admin.php b/lang/en/admin.php index 4e639a7c8f9..9177bc2165e 100644 --- a/lang/en/admin.php +++ b/lang/en/admin.php @@ -516,6 +516,7 @@ $string['defaultuserroleid'] = 'Default role for all users'; $string['deleteincompleteusers'] = 'Delete incomplete users after'; $string['deleteunconfirmed'] = 'Delete not fully setup users after'; $string['deleteuser'] = 'Delete user'; +$string['deleteduserx'] = 'Deleted user {$a}'; $string['density'] = 'Density'; $string['denyemailaddresses'] = 'Denied email domains'; $string['devlibdirpresent'] = 'Directories with development libraries, especially /vendor and /node_modules, should not be present on public sites. See the security overview report for more details.'; diff --git a/user/tests/behat/bulk_editenrolment.feature b/user/tests/behat/bulk_editenrolment.feature index 854b7db83a7..373fa352c56 100644 --- a/user/tests/behat/bulk_editenrolment.feature +++ b/user/tests/behat/bulk_editenrolment.feature @@ -77,14 +77,8 @@ Feature: Bulk enrolments And I set the field "Available" to "Student 2" And I press "Add to selection" And I navigate to "Users > Accounts > Browse list of users" in site administration - And I click on "Filters" "button" - And I set the following fields in the "Username" "core_reportbuilder > Filter" to these values: - | Username operator | Is equal to | - | Username value | student1 | - And I click on "Apply" "button" in the "[data-region='report-filters']" "css_element" - And I click on "Filters" "button" And I press "Delete" action in the "Student 1" report row - And I press "Delete" + And I click on "Delete" "button" in the "Delete user" "dialogue" And I navigate to "Users > Accounts > Bulk user actions" in site administration And I set the field "id_action" to "Add to cohort" And I press "Go" diff --git a/user/tests/behat/delete_users.feature b/user/tests/behat/delete_users.feature index 5e51668e460..abc176d82b8 100644 --- a/user/tests/behat/delete_users.feature +++ b/user/tests/behat/delete_users.feature @@ -100,13 +100,7 @@ Feature: Deleting users And I press "Add to selection" Then I should see "User One" And I navigate to "Users > Accounts > Browse list of users" in site administration - And I click on "Filters" "button" - And I set the following fields in the "Username" "core_reportbuilder > Filter" to these values: - | Username operator | Is equal to | - | Username value | user1 | - And I click on "Apply" "button" in the "[data-region='report-filters']" "css_element" - And I click on "Filters" "button" And I press "Delete" action in the "User One" report row - And I press "Delete" + And I click on "Delete" "button" in the "Delete user" "dialogue" And I navigate to "Users > Accounts > Bulk user actions" in site administration Then I should not see "User One"