allow bulk removal of staff status from users

Bulk option to remove body, roles and permisions, and disable login in
admin for users.

Fixes mysociety/fixmystreet-commercial#2025

5 files changed, 91 insertions, 9 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 70dd5771d..e53c176fe 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -50,6 +50,7 @@
         - Record whether report made on desktop or mobile.
         - Do not display deleted priorities in inspect form. #3195
         - Include extra fields in submit emails.
+        - can remove staff status from users in bulk
     - Development improvements:
         - `#geolocate_link` is now easier to re-style. #3006
         - Links inside `#front-main` can be customised using `$primary_link_*` Sass variables. #3007
diff --git a/docs/_includes/admin-tasks-content.md b/docs/_includes/admin-tasks-content.md
index 234565587..410caf272 100644
--- a/docs/_includes/admin-tasks-content.md
+++ b/docs/_includes/admin-tasks-content.md
@@ -528,6 +528,20 @@ maintenance) to a staff user by editing the user and checking the relevant categ
 staff user, when logged in, will then only see reports within those categories. This is useful where a
 staff user only deals with reports of a specific type.
 
+#### Removing staff status from accounts
+
+To remove the staff status from an account visit the user page and
+uncheck the ‘staff’ checkbox. To prevent a user from logging in uncheck
+the ‘email verified’ checkbox.
+
+You can do this in bulk on the Users screen by selecting the checkbox
+next to the user's name on the users page, selecting the ‘Remove staff
+permission’ checkbox and clicking submit.
+
+Note that if a user has access to the email address associated with an
+account they can re-enable their account, but will not have staff
+permissions.
+
 #### Removing accounts
 
 To remove an account, the Administrator-level member of staff should make contact with
diff --git a/perllib/FixMyStreet/App/Controller/Admin/Users.pm b/perllib/FixMyStreet/App/Controller/Admin/Users.pm
index a05e737ab..7ebfb9bbd 100644
--- a/perllib/FixMyStreet/App/Controller/Admin/Users.pm
+++ b/perllib/FixMyStreet/App/Controller/Admin/Users.pm
@@ -29,17 +29,29 @@ sub index :Path : Args(0) {
 
     if ($c->req->method eq 'POST') {
         my @uids = $c->get_param_list('uid');
-        my @role_ids = $c->get_param_list('roles');
         my $user_rs = FixMyStreet::DB->resultset("User")->search({ id => \@uids });
-        foreach my $user ($user_rs->all) {
-            $user->admin_user_body_permissions->delete;
-            $user->user_roles->search({
-                role_id => { -not_in => \@role_ids },
-            })->delete;
-            foreach my $role (@role_ids) {
-                $user->user_roles->find_or_create({
-                    role_id => $role,
+        if ( $c->get_param('remove-staff') ) {
+            foreach my $user ($user_rs->all) {
+                $user->update({
+                    from_body => undef,
+                    email_verified => 0,
+                    phone_verified => 0,
                 });
+                $user->user_roles->delete;
+                $user->admin_user_body_permissions->delete;
+            }
+        } else {
+            my @role_ids = $c->get_param_list('roles');
+            foreach my $user ($user_rs->all) {
+                $user->admin_user_body_permissions->delete;
+                $user->user_roles->search({
+                    role_id => { -not_in => \@role_ids },
+                })->delete;
+                foreach my $role (@role_ids) {
+                    $user->user_roles->find_or_create({
+                        role_id => $role,
+                    });
+                }
             }
         }
         $c->stash->{status_message} = _('Updated!');
diff --git a/t/app/controller/admin/users.t b/t/app/controller/admin/users.t
index a3bd4a784..6f3971149 100644
--- a/t/app/controller/admin/users.t
+++ b/t/app/controller/admin/users.t
@@ -6,6 +6,8 @@ my $user = $mech->create_user_ok('test@example.com', name => 'Test User');
 my $original_user_id = $user->id; # For log later
 my $user2 = $mech->create_user_ok('test2@example.com', name => 'Test User 2');
 my $user3 = $mech->create_user_ok('test3@example.com', name => 'Test User 3');
+my $user4 = $mech->create_user_ok('test4@example.com', name => 'Test User 4');
+my $user5 = $mech->create_user_ok('test5@example.com', name => 'Test User 5');
 
 my $superuser = $mech->create_user_ok('superuser@example.com', name => 'Super User', is_superuser => 1);
 
@@ -13,6 +15,21 @@ my $oxfordshire = $mech->create_body_ok(2237, 'Oxfordshire County Council');
 my $haringey = $mech->create_body_ok(2509, 'Haringey Borough Council');
 my $southend = $mech->create_body_ok(2607, 'Southend-on-Sea Borough Council');
 
+$user4->from_body( $oxfordshire->id );
+$user4->update;
+$user4->user_body_permissions->create( {
+    body => $oxfordshire,
+    permission_type => 'user_edit',
+} );
+$user5->from_body( $oxfordshire->id );
+$user5->update;
+my $occ_role = $user5->roles->create({
+    body => $oxfordshire,
+    name => 'Role A',
+    permissions => ['moderate', 'user_edit'],
+});
+$user5->add_to_roles($occ_role);
+
 $mech->log_in_ok( $superuser->email );
 
 subtest 'search abuse' => sub {
@@ -97,6 +114,38 @@ subtest 'user assign role' => sub {
     is $user->roles->count, 1;
 };
 
+subtest 'remove users from staff' => sub {
+    is $user4->from_body->id, $oxfordshire->id, 'user4 has a body';
+    is $user4->email_verified, 1, 'user4 email is verified';
+    is $user4->user_body_permissions->count, 1, 'user4 has permissions';
+    is $user5->from_body->id, $oxfordshire->id, 'user5 has a body';
+    is $user5->email_verified, 1, 'user5 email is verified';
+    is $user5->user_roles->count, 1, 'user5 has a role';
+
+    $mech->get_ok('/admin/users');
+    $mech->content_contains($user4->email);
+    $mech->content_contains($user5->email);
+
+    $mech->submit_form_ok({ with_fields => { uid => $user4->id, 'remove-staff' => 'remove-staff'} });
+    $mech->content_lacks($user4->email);
+    $mech->content_contains($user5->email);
+    $user4->discard_changes;
+    $user5->discard_changes;
+    is $user4->from_body, undef, 'user4 removed from body';
+    is $user4->email_verified, 0, 'user4 email unverified';
+    is $user4->user_body_permissions->count, 0, 'no user4 permissions';
+    is $user5->from_body->id, $oxfordshire->id, 'user5 has a body';
+    is $user5->email_verified, 1, 'user5 email is verified';
+    is $user5->user_roles->count, 1, 'user5 has a role';
+
+    $mech->submit_form_ok({ with_fields => { uid => $user5->id, 'remove-staff' => 'remove-staff'} });
+    $mech->content_lacks($user5->email);
+    $user5->discard_changes;
+    is $user5->from_body, undef, 'user5 has no body';
+    is $user5->email_verified, 0, 'user5 email unverified';
+    is $user5->user_roles->count, 0, 'no user5 roles';
+};
+
 subtest 'search does not show user from another council' => sub {
     FixMyStreet::override_config {
         ALLOWED_COBRANDS => [ 'oxfordshire' ],
diff --git a/templates/web/base/admin/users/index.html b/templates/web/base/admin/users/index.html
index e573c10fe..dbd32f3a1 100644
--- a/templates/web/base/admin/users/index.html
+++ b/templates/web/base/admin/users/index.html
@@ -80,6 +80,12 @@
   [% END %]
 </select>
 [% END %]
+<p>
+    <label for="remove-staff">
+        [% loc("Remove staff permissions") %]
+        <input type="checkbox" id="remove-staff" name="remove-staff" value="remove-staff">
+    </label>
+</p>
 <p><input class="btn" type="submit" value="[% loc('Save changes') %]">
 </form>