allow admin to unban a user

Add an unban button to the user edit page when a user is in the abuse
table.

Fixes #1881

6 files changed, 73 insertions, 10 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 88aa8f927..fe7efbd7d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -33,6 +33,7 @@
       - Marking an item as a duplicate enforces providing duplicate id or
         a public update #1873
       - Report field pre-filling for inspectors configurable #1854
+      - Admins can now unban users #1881
     - UK:
         - Use SVG logo, inlined on front page. #1887
 
diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm
index 05953974e..27aeb9e5b 100644
--- a/perllib/FixMyStreet/App/Controller/Admin.pm
+++ b/perllib/FixMyStreet/App/Controller/Admin.pm
@@ -1415,6 +1415,7 @@ sub user_edit : Path('user_edit') : Args(1) {
     }
 
     $c->stash->{user} = $user;
+    $c->forward( 'check_username_for_abuse', [ $user ] );
 
     if ( $user->from_body && $c->user->has_permission_to('user_manage_permissions', $user->from_body->id) ) {
         $c->stash->{available_permissions} = $c->cobrand->available_permissions;
@@ -1428,7 +1429,10 @@ sub user_edit : Path('user_edit') : Args(1) {
             '<p><em>' . $c->flash->{status_message} . '</em></p>';
     }
 
-    if ( $c->get_param('submit') ) {
+    if ( $c->get_param('submit') and $c->get_param('unban') ) {
+        $c->forward('/auth/check_csrf_token');
+        $c->forward('unban_user', [ $user ]);
+    } elsif ( $c->get_param('submit') ) {
         $c->forward('/auth/check_csrf_token');
 
         my $edited = 0;
@@ -1874,6 +1878,28 @@ sub ban_user : Private {
     return 1;
 }
 
+sub unban_user : Private {
+    my ( $self, $c, $user ) = @_;
+
+    my @username;
+    if ($user->email_verified && $user->email) {
+        push @username, $user->email;
+    }
+    if ($user->phone_verified && $user->phone) {
+        push @username, $user->phone;
+    }
+    if (@username) {
+        my $abuse = $c->model('DB::Abuse')->search({ email => \@username });
+        if ( $abuse ) {
+            $abuse->delete;
+            $c->stash->{status_message} = _('user removed from abuse list');
+        } else {
+            $c->stash->{status_message} = _('user not in abuse list');
+        }
+        $c->stash->{username_in_abuse} = 0;
+    }
+}
+
 =head2 flag_user
 
 Sets the flag on a user
@@ -1945,8 +1971,6 @@ sub check_username_for_abuse : Private {
     my $is_abuse = $c->model('DB::Abuse')->find({ email => [ $user->phone, $user->email ] });
 
     $c->stash->{username_in_abuse} = 1 if $is_abuse;
-
-    return 1;
 }
 
 =head2 rotate_photo
diff --git a/t/app/controller/admin.t b/t/app/controller/admin.t
index b63229444..3f69829f7 100644
--- a/t/app/controller/admin.t
+++ b/t/app/controller/admin.t
@@ -741,13 +741,45 @@ subtest 'adding email to abuse list from report page' => sub {
     $mech->click_ok('banuser');
 
     $mech->content_contains('User added to abuse list');
-    $mech->content_contains('<small>(User in abuse table)</small>');
+    $mech->content_contains('<small>User in abuse table</small>');
 
     $abuse = FixMyStreet::App->model('DB::Abuse')->find( { email => $email } );
     ok $abuse, 'entry created in abuse table';
 
     $mech->get_ok( '/admin/report_edit/' . $report->id );
-    $mech->content_contains('<small>(User in abuse table)</small>');
+    $mech->content_contains('<small>User in abuse table</small>');
+};
+
+subtest 'remove user from abuse list from edit user page' => sub {
+    my $abuse = FixMyStreet::App->model('DB::Abuse')->find_or_create( { email => $user->email } );
+    $mech->get_ok( '/admin/user_edit/' . $user->id );
+    $mech->content_contains('User in abuse table');
+
+    $mech->click_ok('unban');
+
+    $abuse = FixMyStreet::App->model('DB::Abuse')->find( { email => $user->email } );
+    ok !$abuse, 'record removed from abuse table';
+};
+
+subtest 'remove user with phone account from abuse list from edit user page' => sub {
+    my $abuse_user = $mech->create_user_ok('01234 456789');
+    my $abuse = FixMyStreet::App->model('DB::Abuse')->find_or_create( { email => $abuse_user->phone } );
+    $mech->get_ok( '/admin/user_edit/' . $abuse_user->id );
+    $mech->content_contains('User in abuse table');
+    my $abuse_found = FixMyStreet::App->model('DB::Abuse')->find( { email => $abuse_user->phone } );
+    ok $abuse_found, 'user in abuse table';
+
+    $mech->click_ok('unban');
+
+    $abuse = FixMyStreet::App->model('DB::Abuse')->find( { email => $user->phone } );
+    ok !$abuse, 'record removed from abuse table';
+};
+
+subtest 'no option to remove user already in abuse list' => sub {
+    my $abuse = FixMyStreet::App->model('DB::Abuse')->find( { email => $user->email } );
+    $abuse->delete if $abuse;
+    $mech->get_ok( '/admin/user_edit/' . $user->id );
+    $mech->content_lacks('User in abuse table');
 };
 
 subtest 'flagging user from report page' => sub {
@@ -1049,13 +1081,13 @@ subtest 'adding email to abuse list from update page' => sub {
     $mech->click_ok('banuser');
 
     $mech->content_contains('User added to abuse list');
-    $mech->content_contains('<small>(User in abuse table)</small>');
+    $mech->content_contains('<small>User in abuse table</small>');
 
     $abuse = FixMyStreet::App->model('DB::Abuse')->find( { email => $email } );
     ok $abuse, 'entry created in abuse table';
 
     $mech->get_ok( '/admin/update_edit/' . $update->id );
-    $mech->content_contains('<small>(User in abuse table)</small>');
+    $mech->content_contains('<small>User in abuse table</small>');
 };
 
 subtest 'flagging user from update page' => sub {
@@ -1165,7 +1197,7 @@ subtest 'report search' => sub {
 
 subtest 'search abuse' => sub {
     $mech->get_ok( '/admin/users?search=example' );
-    $mech->content_like(qr{test4\@example.com.*</td>\s*<td>.*?</td>\s*<td>\(User in abuse table}s);
+    $mech->content_like(qr{test4\@example.com.*</td>\s*<td>.*?</td>\s*<td>User in abuse table}s);
 };
 
 subtest 'show flagged entries' => sub {
diff --git a/templates/web/base/admin/report_blocks.html b/templates/web/base/admin/report_blocks.html
index 8e8b56393..4c52b14bb 100644
--- a/templates/web/base/admin/report_blocks.html
+++ b/templates/web/base/admin/report_blocks.html
@@ -15,7 +15,7 @@ SET state_groups = c.cobrand.state_groups_admin;
 
 [% BLOCK abuse_button -%]
 [% IF allowed_pages.abuse_edit -%]
-[% IF username_in_abuse %]<small>[% loc('(User in abuse table)') %]</small>[% ELSE %]<input type="submit" class="btn" name="banuser" value="[% loc('Ban user') %]" />[% END %]
+[% IF username_in_abuse %]<small>[% loc('User in abuse table') %]</small>[% ELSE %]<input type="submit" class="btn" name="banuser" value="[% loc('Ban user') %]" />[% END %]
 [%- END %]
 [%- END %]
 
diff --git a/templates/web/base/admin/user-form.html b/templates/web/base/admin/user-form.html
index 5637252e2..7b27f7497 100644
--- a/templates/web/base/admin/user-form.html
+++ b/templates/web/base/admin/user-form.html
@@ -33,6 +33,12 @@
         <input type="hidden" name="phone_verified" value="0">
       [% END %]
 
+      [% IF username_in_abuse %]
+      <li>
+            <p class="error">[% loc('User in abuse table') %] <input name="unban" type="submit" value="[% loc('Unban') %]"></p>
+      </li>
+      [% END %]
+
       [% IF c.user.is_superuser || c.cobrand.moniker == 'zurich' %]
         <li>
           <div class="admin-hint">
diff --git a/templates/web/base/admin/users.html b/templates/web/base/admin/users.html
index d367c18d8..6dfcf4204 100644
--- a/templates/web/base/admin/users.html
+++ b/templates/web/base/admin/users.html
@@ -29,7 +29,7 @@
             [% IF user.is_superuser %] * [% END %]
         </td>
       [% IF c.cobrand.moniker != 'zurich' %]
-        <td>[% user.flagged == 2 ? loc('(User in abuse table)') : user.flagged ? loc('Yes') : '&nbsp;' %]</td>
+        <td>[% user.flagged == 2 ? loc('User in abuse table') : user.flagged ? loc('Yes') : '&nbsp;' %]</td>
       [% END %]
         <td>[% IF user.id %]<a href="[% c.uri_for( 'user_edit', user.id ) %]">[% loc('Edit') %]</a>[% END %]</td>
     </tr>