aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--perllib/FixMyStreet/App/Controller/Admin.pm333
-rw-r--r--perllib/FixMyStreet/DB/Result/User.pm13
-rw-r--r--t/app/controller/admin.t178
-rw-r--r--templates/web/base/admin/report_blocks.html2
-rw-r--r--templates/web/base/admin/report_edit.html11
-rw-r--r--templates/web/base/admin/update_edit.html6
-rw-r--r--templates/web/base/admin/user-form.html12
-rw-r--r--templates/web/base/admin/users.html2
-rw-r--r--templates/web/zurich/admin/report_edit-sdm.html2
-rw-r--r--templates/web/zurich/admin/report_edit.html2
-rw-r--r--templates/web/zurich/admin/update_edit.html2
11 files changed, 380 insertions, 183 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm
index 71416622a..0caa25710 100644
--- a/perllib/FixMyStreet/App/Controller/Admin.pm
+++ b/perllib/FixMyStreet/App/Controller/Admin.pm
@@ -14,6 +14,7 @@ use List::MoreUtils 'uniq';
use mySociety::ArrayUtils;
use FixMyStreet::SendReport;
+use FixMyStreet::SMS;
=head1 NAME
@@ -678,6 +679,10 @@ sub reports : Path('reports') {
my $like_search = "%$search%";
+ my $parsed = FixMyStreet::SMS->parse_username($search);
+ my $valid_phone = $parsed->{phone};
+ my $valid_email = $parsed->{email};
+
# when DBIC creates the join it does 'JOIN users user' in the
# SQL which makes PostgreSQL unhappy as user is a reserved
# word. So look up user ID for email separately.
@@ -686,10 +691,19 @@ sub reports : Path('reports') {
}, { columns => [ 'id' ] } )->all;
@user_ids = map { $_->id } @user_ids;
- if (is_valid_email($search)) {
+ my @user_ids_phone = $c->model('DB::User')->search({
+ phone => { ilike => $like_search },
+ }, { columns => [ 'id' ] } )->all;
+ @user_ids_phone = map { $_->id } @user_ids_phone;
+
+ if ($valid_email) {
$query->{'-or'} = [
'me.user_id' => { -in => \@user_ids },
];
+ } elsif ($valid_phone) {
+ $query->{'-or'} = [
+ 'me.user_id' => { -in => \@user_ids_phone },
+ ];
} elsif ($search =~ /^id:(\d+)$/) {
$query->{'-or'} = [
'me.id' => int($1),
@@ -705,7 +719,7 @@ sub reports : Path('reports') {
} else {
$query->{'-or'} = [
'me.id' => $search_n,
- 'me.user_id' => { -in => \@user_ids },
+ 'me.user_id' => { -in => [ @user_ids, @user_ids_phone ] },
'me.external_id' => { ilike => $like_search },
'me.name' => { ilike => $like_search },
'me.title' => { ilike => $like_search },
@@ -726,10 +740,14 @@ sub reports : Path('reports') {
$c->stash->{problems} = [ $problems->all ];
$c->stash->{problems_pager} = $problems->pager;
- if (is_valid_email($search)) {
+ if ($valid_email) {
$query = [
'me.user_id' => { -in => \@user_ids },
];
+ } elsif ($valid_phone) {
+ $query = [
+ 'me.user_id' => { -in => \@user_ids_phone },
+ ];
} elsif ($search =~ /^id:(\d+)$/) {
$query = [
'me.id' => int($1),
@@ -741,7 +759,7 @@ sub reports : Path('reports') {
$query = [
'me.id' => $search_n,
'problem.id' => $search_n,
- 'me.user_id' => { -in => \@user_ids },
+ 'me.user_id' => { -in => [ @user_ids, @user_ids_phone ] },
'me.name' => { ilike => $like_search },
text => { ilike => $like_search },
'me.cobrand_data' => { ilike => $like_search },
@@ -834,7 +852,7 @@ sub report_edit : Path('report_edit') : Args(1) {
return if $done;
}
- $c->forward('check_email_for_abuse', [ $problem->user->email ] );
+ $c->forward('check_username_for_abuse', [ $problem->user ] );
$c->stash->{updates} =
[ $c->model('DB::Comment')
@@ -884,11 +902,12 @@ sub report_edit : Path('report_edit') : Args(1) {
$c->forward( '/admin/report_edit_category', [ $problem ] );
- my $email = lc $c->get_param('email');
- if ( $email ne $problem->user->email ) {
- my $user = $c->model('DB::User')->find_or_create({ email => $email });
- $user->insert unless $user->in_storage;
- $problem->user( $user );
+ my $parsed = FixMyStreet::SMS->parse_username($c->get_param('username'));
+ if ($parsed->{email} || ($parsed->{phone} && $parsed->{phone}->is_mobile)) {
+ my $user = $c->model('DB::User')->find_or_create({ $parsed->{type} => $parsed->{username} });
+ if ($user->id && $user->id != $problem->user->id) {
+ $problem->user( $user );
+ }
}
# Deal with photos
@@ -1145,28 +1164,15 @@ sub users: Path('users') : Args(0) {
{
-or => [
email => { ilike => $isearch },
+ phone => { ilike => $isearch },
name => { ilike => $isearch },
from_body => $search_n,
]
}
);
my @users = $users->all;
- my %email2user = map { $_->email => $_ } @users;
$c->stash->{users} = [ @users ];
-
- if ( $c->user->is_superuser ) {
- my $emails = $c->model('DB::Abuse')->search(
- { email => { ilike => $isearch } }
- );
- foreach my $email ($emails->all) {
- # Slight abuse of the boolean flagged value
- if ($email2user{$email->email}) {
- $email2user{$email->email}->flagged( 2 );
- } else {
- push @{$c->stash->{users}}, { email => $email->email, flagged => 2 };
- }
- }
- }
+ $c->forward('add_flags', [ { email => { ilike => $isearch } } ]);
} else {
$c->forward('/auth/get_csrf_token');
@@ -1178,9 +1184,7 @@ sub users: Path('users') : Args(0) {
{ order_by => 'name' }
);
my @users = $users->all;
- my %email2user = map { $_->email => $_ } @users;
$c->stash->{users} = \@users;
-
}
return 1;
@@ -1203,7 +1207,7 @@ sub update_edit : Path('update_edit') : Args(1) {
return 1;
}
- $c->forward('check_email_for_abuse', [ $update->user->email ] );
+ $c->forward('check_username_for_abuse', [ $update->user ] );
if ( $c->get_param('banuser') ) {
$c->forward('ban_user');
@@ -1227,9 +1231,7 @@ sub update_edit : Path('update_edit') : Args(1) {
# $update->name can be null which makes ne unhappy
my $name = $update->name || '';
- my $email = lc $c->get_param('email');
if ( $c->get_param('name') ne $name
- || $email ne $update->user->email
|| $c->get_param('anonymous') ne $update->anonymous
|| $c->get_param('text') ne $update->text ) {
$edited = 1;
@@ -1249,10 +1251,13 @@ sub update_edit : Path('update_edit') : Args(1) {
$update->anonymous( $c->get_param('anonymous') );
$update->state( $new_state );
- if ( $email ne $update->user->email ) {
- my $user = $c->model('DB::User')->find_or_create({ email => $email });
- $user->insert unless $user->in_storage;
- $update->user($user);
+ my $parsed = FixMyStreet::SMS->parse_username($c->get_param('username'));
+ if ($parsed->{email} || ($parsed->{phone} && $parsed->{phone}->is_mobile)) {
+ my $user = $c->model('DB::User')->find_or_create({ $parsed->{type} => $parsed->{username} });
+ if ($user->id && $user->id != $update->user->id) {
+ $edited = 1;
+ $update->user( $user );
+ }
}
if ( $new_state eq 'confirmed' and $old_state eq 'unconfirmed' ) {
@@ -1305,25 +1310,53 @@ sub user_add : Path('user_edit') : Args(0) {
$c->forward('/auth/check_csrf_token');
$c->stash->{field_errors} = {};
- unless ($c->get_param('email')) {
+ my $email = lc $c->get_param('email');
+ my $phone = $c->get_param('phone');
+ my $email_v = $c->get_param('email_verified');
+ my $phone_v = $c->get_param('phone_verified');
+
+ unless ($email || $phone) {
+ $c->stash->{field_errors}->{username} = _('Please enter a valid email or phone number');
+ }
+ if (!$email_v && !$phone_v) {
+ $c->stash->{field_errors}->{username} = _('Please verify at least one of email/phone');
+ }
+ if ($email && !is_valid_email($email)) {
$c->stash->{field_errors}->{email} = _('Please enter a valid email');
}
unless ($c->get_param('name')) {
$c->stash->{field_errors}->{name} = _('Please enter a name');
}
+
+ if ($phone_v) {
+ my $parsed = FixMyStreet::SMS->parse_username($phone);
+ if ($parsed->{phone} && $parsed->{phone}->is_mobile) {
+ $phone = $parsed->{username};
+ } elsif ($parsed->{phone}) {
+ $c->stash->{field_errors}->{phone} = _('Please enter a mobile number');
+ } else {
+ $c->stash->{field_errors}->{phone} = _('Please check your phone number is correct');
+ }
+ }
+
+ my $existing_email = $email_v && $c->model('DB::User')->find( { email => $email } );
+ my $existing_phone = $phone_v && $c->model('DB::User')->find( { phone => $phone } );
+ if ($existing_email || $existing_phone) {
+ $c->stash->{field_errors}->{username} = _('User already exists');
+ }
+
return if %{$c->stash->{field_errors}};
- my $user = $c->model('DB::User')->find_or_create( {
+ my $user = $c->model('DB::User')->create( {
name => $c->get_param('name'),
- email => lc $c->get_param('email'),
- email_verified => 1,
- phone => $c->get_param('phone') || undef,
+ email => $email ? $email : undef,
+ email_verified => $email && $email_v ? 1 : 0,
+ phone => $phone || undef,
+ phone_verified => $phone && $phone_v ? 1 : 0,
from_body => $c->get_param('body') || undef,
flagged => $c->get_param('flagged') || 0,
# Only superusers can create superusers
is_superuser => ( $c->user->is_superuser && $c->get_param('is_superuser') ) || 0,
- }, {
- key => 'users_email_verified_key'
} );
$c->stash->{user} = $user;
$c->forward('user_cobrand_extra_fields');
@@ -1367,19 +1400,72 @@ sub user_edit : Path('user_edit') : Args(1) {
my $edited = 0;
my $email = lc $c->get_param('email');
- if ( $user->email ne $email ||
+ my $phone = $c->get_param('phone');
+ my $email_v = $c->get_param('email_verified') || 0;
+ my $phone_v = $c->get_param('phone_verified') || 0;
+
+ $c->stash->{field_errors} = {};
+
+ unless ($email || $phone) {
+ $c->stash->{field_errors}->{username} = _('Please enter a valid email or phone number');
+ }
+ if (!$email_v && !$phone_v) {
+ $c->stash->{field_errors}->{username} = _('Please verify at least one of email/phone');
+ }
+ if ($email && !is_valid_email($email)) {
+ $c->stash->{field_errors}->{email} = _('Please enter a valid email');
+ }
+
+ if ($phone_v) {
+ my $parsed = FixMyStreet::SMS->parse_username($phone);
+ if ($parsed->{phone} && $parsed->{phone}->is_mobile) {
+ $phone = $parsed->{username};
+ } elsif ($parsed->{phone}) {
+ $c->stash->{field_errors}->{phone} = _('Please enter a mobile number');
+ } else {
+ $c->stash->{field_errors}->{phone} = _('Please check your phone number is correct');
+ }
+ }
+
+ unless ($user->name) {
+ $c->stash->{field_errors}->{name} = _('Please enter a name');
+ }
+
+ my $email_params = { email => $email, email_verified => 1, id => { '!=', $user->id } };
+ my $phone_params = { phone => $phone, phone_verified => 1, id => { '!=', $user->id } };
+ my $existing_email = $email_v && $c->model('DB::User')->search($email_params)->first;
+ my $existing_phone = $phone_v && $c->model('DB::User')->search($phone_params)->first;
+ my $existing_user = $existing_email || $existing_phone;
+ my $existing_email_cobrand = $email_v && $c->cobrand->users->search($email_params)->first;
+ my $existing_phone_cobrand = $phone_v && $c->cobrand->users->search($phone_params)->first;
+ my $existing_user_cobrand = $existing_email_cobrand || $existing_phone_cobrand;
+ if ($existing_phone_cobrand && $existing_email_cobrand && $existing_email_cobrand->id != $existing_phone_cobrand->id) {
+ $c->stash->{field_errors}->{username} = _('User already exists');
+ }
+
+ return if %{$c->stash->{field_errors}};
+
+ if ( ($user->email || "") ne $email ||
$user->name ne $c->get_param('name') ||
- ($user->phone || "") ne $c->get_param('phone') ||
+ ($user->phone || "") ne $phone ||
($user->from_body && $c->get_param('body') && $user->from_body->id ne $c->get_param('body')) ||
(!$user->from_body && $c->get_param('body'))
) {
$edited = 1;
}
+ if ($existing_user_cobrand) {
+ $existing_user->adopt($user);
+ $c->forward( 'log_edit', [ $id, 'user', 'merge' ] );
+ return $c->res->redirect( $c->uri_for( 'user_edit', $existing_user->id ) );
+ }
+
+ $user->email($email) if !$existing_email;
+ $user->phone($phone) if !$existing_phone;
+ $user->email_verified( $email_v );
+ $user->phone_verified( $phone_v );
$user->name( $c->get_param('name') );
- my $original_email = $user->email;
- $user->email( $email );
- $user->phone( $c->get_param('phone') ) if $c->get_param('phone');
+
$user->flagged( $c->get_param('flagged') || 0 );
# Only superusers can grant superuser status
$user->is_superuser( ( $c->user->is_superuser && $c->get_param('is_superuser') ) || 0 );
@@ -1458,8 +1544,6 @@ sub user_edit : Path('user_edit') : Args(1) {
}
}
- $c->stash->{field_errors} = {};
-
# Update the categories this user operates in
if ( $user->from_body ) {
$c->stash->{body} = $user->from_body;
@@ -1470,33 +1554,12 @@ sub user_edit : Path('user_edit') : Args(1) {
$user->set_extra_metadata('categories', \@new_contact_ids);
}
- unless ($user->email) {
- $c->stash->{field_errors}->{email} = _('Please enter a valid email');
- }
- unless ($user->name) {
- $c->stash->{field_errors}->{name} = _('Please enter a name');
- }
- return if %{$c->stash->{field_errors}};
-
- my $existing_user = $c->model('DB::User')->search({ email => $user->email, id => { '!=', $user->id } })->first;
- my $existing_user_cobrand = $c->cobrand->users->search({ email => $user->email, id => { '!=', $user->id } })->first;
- if ($existing_user_cobrand) {
- $existing_user->adopt($user);
- $c->forward( 'log_edit', [ $id, 'user', 'merge' ] );
- $c->res->redirect( $c->uri_for( 'user_edit', $existing_user->id ) );
- } else {
- if ($existing_user) {
- # Tried to change email to an existing one lacking permission
- # so make sure it's switched back
- $user->email($original_email);
- }
- $user->update;
- if ($edited) {
- $c->forward( 'log_edit', [ $id, 'user', 'edit' ] );
- }
- $c->flash->{status_message} = _("Updated!");
- $c->res->redirect( $c->uri_for( 'user_edit', $user->id ) );
+ $user->update;
+ if ($edited) {
+ $c->forward( 'log_edit', [ $id, 'user', 'edit' ] );
}
+ $c->flash->{status_message} = _("Updated!");
+ return $c->res->redirect( $c->uri_for( 'user_edit', $user->id ) );
}
if ( $user->from_body ) {
@@ -1527,6 +1590,27 @@ sub user_cobrand_extra_fields : Private {
}
}
+sub add_flags : Private {
+ my ( $self, $c, $search ) = @_;
+
+ return unless $c->user->is_superuser;
+
+ my $users = $c->stash->{users};
+ my %email2user = map { $_->email => $_ } grep { $_->email } @$users;
+ my %phone2user = map { $_->phone => $_ } grep { $_->phone } @$users;
+ my %username2user = (%email2user, %phone2user);
+ my $usernames = $c->model('DB::Abuse')->search($search);
+
+ foreach my $username (map { $_->email } $usernames->all) {
+ # Slight abuse of the boolean flagged value
+ if ($username2user{$username}) {
+ $username2user{$username}->flagged( 2 );
+ } else {
+ push @{$c->stash->{users}}, { email => $username, flagged => 2 };
+ }
+ }
+}
+
sub flagged : Path('flagged') : Args(0) {
my ( $self, $c ) = @_;
@@ -1536,23 +1620,10 @@ sub flagged : Path('flagged') : Args(0) {
# which has to use an array ref for sql quoting reasons
$c->stash->{problems} = [ $problems->all ];
- my $users = $c->cobrand->users->search( { flagged => 1 } );
- my @users = $users->all;
- my %email2user = map { $_->email => $_ } @users;
+ my @users = $c->cobrand->users->search( { flagged => 1 } )->all;
$c->stash->{users} = [ @users ];
- my @abuser_emails = $c->model('DB::Abuse')->all()
- if $c->user->is_superuser;
-
- foreach my $email (@abuser_emails) {
- # Slight abuse of the boolean flagged value
- if ($email2user{$email->email}) {
- $email2user{$email->email}->flagged( 2 );
- } else {
- push @{$c->stash->{users}}, { email => $email->email, flagged => 2 };
- }
- }
-
+ $c->forward('add_flags', [ {} ]);
return 1;
}
@@ -1735,47 +1806,60 @@ sub log_edit : Private {
=head2 ban_user
-Add the email address in the email param of the request object to
-the abuse table if they are not already in there and sets status_message
-accordingly
+Add the user's email address/phone number to the abuse table if they are not
+already in there and sets status_message accordingly.
=cut
sub ban_user : Private {
my ( $self, $c ) = @_;
- my $email = lc $c->get_param('email');
-
- return unless $email;
-
- my $abuse = $c->model('DB::Abuse')->find_or_new({ email => $email });
-
- if ( $abuse->in_storage ) {
- $c->stash->{status_message} = _('Email already in abuse list');
- } else {
- $abuse->insert;
- $c->stash->{status_message} = _('Email added to abuse list');
+ my $user;
+ if ($c->stash->{problem}) {
+ $user = $c->stash->{problem}->user;
+ } elsif ($c->stash->{update}) {
+ $user = $c->stash->{update}->user;
}
+ return unless $user;
- $c->stash->{email_in_abuse} = 1;
-
+ if ($user->email_verified && $user->email) {
+ my $abuse = $c->model('DB::Abuse')->find_or_new({ email => $user->email });
+ if ( $abuse->in_storage ) {
+ $c->stash->{status_message} = _('User already in abuse list');
+ } else {
+ $abuse->insert;
+ $c->stash->{status_message} = _('User added to abuse list');
+ }
+ $c->stash->{username_in_abuse} = 1;
+ }
+ if ($user->phone_verified && $user->phone) {
+ my $abuse = $c->model('DB::Abuse')->find_or_new({ email => $user->phone });
+ if ( $abuse->in_storage ) {
+ $c->stash->{status_message} = _('User already in abuse list');
+ } else {
+ $abuse->insert;
+ $c->stash->{status_message} = _('User added to abuse list');
+ }
+ $c->stash->{username_in_abuse} = 1;
+ }
return 1;
}
=head2 flag_user
-Sets the flag on a user with the given email
+Sets the flag on a user
=cut
sub flag_user : Private {
my ( $self, $c ) = @_;
- my $email = lc $c->get_param('email');
-
- return unless $email;
-
- my $user = $c->cobrand->users->find({ email => $email });
+ my $user;
+ if ($c->stash->{problem}) {
+ $user = $c->stash->{problem}->user;
+ } elsif ($c->stash->{update}) {
+ $user = $c->stash->{update}->user;
+ }
if ( !$user ) {
$c->stash->{status_message} = _('Could not find user');
@@ -1792,18 +1876,19 @@ sub flag_user : Private {
=head2 remove_user_flag
-Remove the flag on a user with the given email
+Remove the flag on a user
=cut
sub remove_user_flag : Private {
my ( $self, $c ) = @_;
- my $email = lc $c->get_param('email');
-
- return unless $email;
-
- my $user = $c->cobrand->users->find({ email => $email });
+ my $user;
+ if ($c->stash->{problem}) {
+ $user = $c->stash->{problem}->user;
+ } elsif ($c->stash->{update}) {
+ $user = $c->stash->{update}->user;
+ }
if ( !$user ) {
$c->stash->{status_message} = _('Could not find user');
@@ -1817,20 +1902,20 @@ sub remove_user_flag : Private {
}
-=head2 check_email_for_abuse
+=head2 check_username_for_abuse
- $c->forward('check_email_for_abuse', [ $email ] );
+ $c->forward('check_username_for_abuse', [ $user ] );
-Checks if $email is in the abuse table and sets email_in_abuse accordingly
+Checks if $user is in the abuse table and sets username_in_abuse accordingly.
=cut
-sub check_email_for_abuse : Private {
- my ( $self, $c, $email ) =@_;
+sub check_username_for_abuse : Private {
+ my ( $self, $c, $user ) = @_;
- my $is_abuse = $c->model('DB::Abuse')->find({ email => $email });
+ my $is_abuse = $c->model('DB::Abuse')->find({ email => [ $user->phone, $user->email ] });
- $c->stash->{email_in_abuse} = 1 if $is_abuse;
+ $c->stash->{username_in_abuse} = 1 if $is_abuse;
return 1;
}
diff --git a/perllib/FixMyStreet/DB/Result/User.pm b/perllib/FixMyStreet/DB/Result/User.pm
index b733b47bd..0b6c6bc57 100644
--- a/perllib/FixMyStreet/DB/Result/User.pm
+++ b/perllib/FixMyStreet/DB/Result/User.pm
@@ -133,6 +133,19 @@ __PACKAGE__->add_columns(
},
);
+=head2 username
+
+Returns a verified email or phone for this user, preferring email,
+or undef if neither verified (shouldn't happen).
+
+=cut
+
+sub username {
+ my $self = shift;
+ return $self->email if $self->email_verified;
+ return $self->phone_display if $self->phone_verified;
+}
+
sub latest_anonymity {
my $self = shift;
my $p = $self->problems->search(undef, { order_by => { -desc => 'id' } } )->first;
diff --git a/t/app/controller/admin.t b/t/app/controller/admin.t
index bd0f9e408..9f1b0cfb9 100644
--- a/t/app/controller/admin.t
+++ b/t/app/controller/admin.t
@@ -316,7 +316,7 @@ foreach my $test (
detail => 'Detail for Report to Edit',
state => 'confirmed',
name => 'Test User',
- email => $user->email,
+ username => $user->email,
anonymous => 0,
flagged => undef,
non_public => undef,
@@ -332,7 +332,7 @@ foreach my $test (
detail => 'Detail for Report to Edit',
state => 'confirmed',
name => 'Test User',
- email => $user->email,
+ username => $user->email,
anonymous => 0,
flagged => undef,
non_public => undef,
@@ -348,7 +348,7 @@ foreach my $test (
detail => 'Edited Detail',
state => 'confirmed',
name => 'Test User',
- email => $user->email,
+ username => $user->email,
anonymous => 0,
flagged => undef,
non_public => undef,
@@ -365,7 +365,7 @@ foreach my $test (
detail => 'Edited Detail',
state => 'confirmed',
name => 'Edited User',
- email => $user->email,
+ username => $user->email,
anonymous => 0,
flagged => undef,
non_public => undef,
@@ -384,12 +384,12 @@ foreach my $test (
detail => 'Edited Detail',
state => 'confirmed',
name => 'Edited User',
- email => $user->email,
+ username => $user->email,
anonymous => 0,
flagged => 'on',
non_public => undef,
},
- changes => { email => $user2->email, },
+ changes => { username => $user2->email, },
log_entries => [qw/edit edit edit edit edit/],
resend => 0,
user => $user2,
@@ -401,7 +401,7 @@ foreach my $test (
detail => 'Edited Detail',
state => 'confirmed',
name => 'Edited User',
- email => $user2->email,
+ username => $user2->email,
anonymous => 0,
flagged => 'on',
non_public => undef,
@@ -417,7 +417,7 @@ foreach my $test (
detail => 'Edited Detail',
state => 'unconfirmed',
name => 'Edited User',
- email => $user2->email,
+ username => $user2->email,
anonymous => 0,
flagged => 'on',
non_public => undef,
@@ -433,7 +433,7 @@ foreach my $test (
detail => 'Edited Detail',
state => 'confirmed',
name => 'Edited User',
- email => $user2->email,
+ username => $user2->email,
anonymous => 0,
flagged => 'on',
non_public => undef,
@@ -450,7 +450,7 @@ foreach my $test (
detail => 'Edited Detail',
state => 'fixed',
name => 'Edited User',
- email => $user2->email,
+ username => $user2->email,
anonymous => 0,
flagged => 'on',
non_public => undef,
@@ -468,7 +468,7 @@ foreach my $test (
detail => 'Edited Detail',
state => 'hidden',
name => 'Edited User',
- email => $user2->email,
+ username => $user2->email,
anonymous => 0,
flagged => 'on',
non_public => undef,
@@ -489,7 +489,7 @@ foreach my $test (
detail => 'Edited Detail',
state => 'confirmed',
name => 'Edited User',
- email => $user2->email,
+ username => $user2->email,
anonymous => 1,
flagged => 'on',
non_public => undef,
@@ -507,7 +507,7 @@ foreach my $test (
detail => 'Edited Detail',
state => 'confirmed',
name => 'Edited User',
- email => $user2->email,
+ username => $user2->email,
anonymous => 1,
flagged => 'on',
non_public => undef,
@@ -555,7 +555,7 @@ foreach my $test (
$test->{changes}->{flagged} = 1 if $test->{changes}->{flagged};
$test->{changes}->{non_public} = 1 if $test->{changes}->{non_public};
- is $report->$_, $test->{changes}->{$_}, "$_ updated" for grep { $_ ne 'email' } keys %{ $test->{changes} };
+ is $report->$_, $test->{changes}->{$_}, "$_ updated" for grep { $_ ne 'username' } keys %{ $test->{changes} };
if ( $test->{user} ) {
is $report->user->id, $test->{user}->id, 'user changed';
@@ -603,7 +603,7 @@ subtest 'change email to new user' => sub {
detail => $report->detail,
state => $report->state,
name => $report->name,
- email => $report->user->email,
+ username => $report->user->email,
category => 'Other',
anonymous => 1,
flagged => 'on',
@@ -616,12 +616,10 @@ subtest 'change email to new user' => sub {
is_deeply( $mech->visible_form_values(), $fields, 'initial form values' );
my $changes = {
- email => 'test3@example.com'
+ username => 'test3@example.com'
};
- $user3 =
- FixMyStreet::App->model('DB::User')
- ->find( { email => 'test3@example.com', name => 'Test User 2' } );
+ $user3 = FixMyStreet::App->model('DB::User')->find( { email => 'test3@example.com' } );
ok !$user3, 'user not in database';
@@ -640,9 +638,7 @@ subtest 'change email to new user' => sub {
is $log_entries->first->action, 'edit', 'log action';
is_deeply( $mech->visible_form_values(), $new_fields, 'changed form values' );
- $user3 =
- FixMyStreet::App->model('DB::User')
- ->find( { email => 'test3@example.com', name => 'Test User 2' } );
+ $user3 = FixMyStreet::App->model('DB::User')->find( { email => 'test3@example.com' } );
$report->discard_changes;
@@ -657,18 +653,18 @@ subtest 'adding email to abuse list from report page' => sub {
$abuse->delete if $abuse;
$mech->get_ok( '/admin/report_edit/' . $report->id );
- $mech->content_contains('Ban email address');
+ $mech->content_contains('Ban user');
$mech->click_ok('banuser');
- $mech->content_contains('Email added to abuse list');
- $mech->content_contains('<small>(Email in abuse table)</small>');
+ $mech->content_contains('User added to abuse list');
+ $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>(Email in abuse table)</small>');
+ $mech->content_contains('<small>(User in abuse table)</small>');
};
subtest 'flagging user from report page' => sub {
@@ -742,7 +738,7 @@ for my $test (
state => 'confirmed',
name => '',
anonymous => 1,
- email => 'test@example.com',
+ username => 'test@example.com',
},
changes => {
text => 'this is a changed update',
@@ -757,7 +753,7 @@ for my $test (
state => 'confirmed',
name => '',
anonymous => 1,
- email => 'test@example.com',
+ username => 'test@example.com',
},
changes => {
name => 'A User',
@@ -772,7 +768,7 @@ for my $test (
state => 'confirmed',
name => 'A User',
anonymous => 1,
- email => 'test@example.com',
+ username => 'test@example.com',
},
changes => {
anonymous => 0,
@@ -787,11 +783,10 @@ for my $test (
state => 'confirmed',
name => 'A User',
anonymous => 0,
- email => $update->user->email,
- email => 'test@example.com',
+ username => 'test@example.com',
},
changes => {
- email => 'test2@example.com',
+ username => 'test2@example.com',
},
log_count => 4,
log_entries => [qw/edit edit edit edit/],
@@ -804,7 +799,7 @@ for my $test (
state => 'confirmed',
name => 'A User',
anonymous => 0,
- email => 'test2@example.com',
+ username => 'test2@example.com',
},
changes => {
state => 'unconfirmed',
@@ -819,7 +814,7 @@ for my $test (
state => 'unconfirmed',
name => 'A User',
anonymous => 0,
- email => 'test2@example.com',
+ username => 'test2@example.com',
},
changes => {
text => 'this is a twice changed update',
@@ -849,7 +844,7 @@ for my $test (
$update->discard_changes;
- is $update->$_, $test->{changes}->{$_} for grep { $_ ne 'email' } keys %{ $test->{changes} };
+ is $update->$_, $test->{changes}->{$_} for grep { $_ ne 'username' } keys %{ $test->{changes} };
if ( $test->{changes}{state} && $test->{changes}{state} eq 'confirmed' ) {
isnt $update->confirmed, undef;
}
@@ -935,9 +930,7 @@ for my $test (
}
subtest 'editing update email creates new user if required' => sub {
- my $user = FixMyStreet::App->model('DB::User')->find(
- { email => 'test4@example.com' }
- );
+ my $user = FixMyStreet::App->model('DB::User')->find( { email => 'test4@example.com' } );
$user->delete if $user;
@@ -946,14 +939,12 @@ subtest 'editing update email creates new user if required' => sub {
state => 'hidden',
name => 'A User',
anonymous => 0,
- email => 'test4@example.com',
+ username => 'test4@example.com',
};
$mech->submit_form_ok( { with_fields => $fields } );
- $user = FixMyStreet::App->model('DB::User')->find(
- { email => 'test4@example.com' }
- );
+ $user = FixMyStreet::App->model('DB::User')->find( { email => 'test4@example.com' } );
is_deeply $mech->visible_form_values, $fields, 'submitted form values';
@@ -970,18 +961,18 @@ subtest 'adding email to abuse list from update page' => sub {
$abuse->delete if $abuse;
$mech->get_ok( '/admin/update_edit/' . $update->id );
- $mech->content_contains('Ban email address');
+ $mech->content_contains('Ban user');
$mech->click_ok('banuser');
- $mech->content_contains('Email added to abuse list');
- $mech->content_contains('<small>(Email in abuse table)</small>');
+ $mech->content_contains('User added to abuse list');
+ $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>(Email in abuse table)</small>');
+ $mech->content_contains('<small>(User in abuse table)</small>');
};
subtest 'flagging user from update page' => sub {
@@ -1029,13 +1020,12 @@ subtest 'hiding comment marked as fixed reopens report' => sub {
$report->state('fixed - user');
$report->update;
-
my $fields = {
text => 'this is a changed update',
state => 'hidden',
name => 'A User',
anonymous => 0,
- email => 'test2@example.com',
+ username => 'test2@example.com',
};
$mech->submit_form_ok( { with_fields => $fields } );
@@ -1092,7 +1082,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>\(Email 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 {
@@ -1168,6 +1158,69 @@ $user->update;
my $southend = $mech->create_body_ok(2607, 'Southend-on-Sea Borough Council');
+for my $test (
+ {
+ desc => 'add user - blank form',
+ fields => {
+ email => '', email_verified => 0,
+ phone => '', phone_verified => 0,
+ },
+ error => ['Please verify at least one of email/phone', 'Please enter a name'],
+ },
+ {
+ desc => 'add user - blank, verify phone',
+ fields => {
+ email => '', email_verified => 0,
+ phone => '', phone_verified => 1,
+ },
+ error => ['Please enter a valid email or phone number', 'Please enter a name'],
+ },
+ {
+ desc => 'add user - bad email',
+ fields => {
+ name => 'Norman',
+ email => 'bademail', email_verified => 0,
+ phone => '', phone_verified => 0,
+ },
+ error => ['Please enter a valid email'],
+ },
+ {
+ desc => 'add user - bad phone',
+ fields => {
+ name => 'Norman',
+ phone => '01214960000000', phone_verified => 1,
+ },
+ error => ['Please check your phone number is correct'],
+ },
+ {
+ desc => 'add user - landline',
+ fields => {
+ name => 'Norman Name',
+ phone => '+441214960000',
+ phone_verified => 1,
+ },
+ error => ['Please enter a mobile number'],
+ },
+ {
+ desc => 'add user - good details',
+ fields => {
+ name => 'Norman Name',
+ phone => '+61491570156',
+ phone_verified => 1,
+ },
+ },
+) {
+ subtest $test->{desc} => sub {
+ $mech->get_ok('/admin/users');
+ $mech->submit_form_ok( { with_fields => $test->{fields} } );
+ if ($test->{error}) {
+ $mech->content_contains($_) for @{$test->{error}};
+ } else {
+ $mech->content_contains('Updated');
+ }
+ };
+}
+
my %default_perms = (
"permissions[moderate]" => undef,
"permissions[planned_reports]" => undef,
@@ -1358,6 +1411,32 @@ FixMyStreet::override_config {
}
};
+FixMyStreet::override_config {
+ MAPIT_URL => 'http://mapit.uk/',
+ SMS_AUTHENTICATION => 1,
+}, sub {
+ subtest "Test edit user add verified phone" => sub {
+ $mech->get_ok( '/admin/user_edit/' . $user->id );
+ $mech->submit_form_ok( { with_fields => {
+ phone => '+61491570157',
+ phone_verified => 1,
+ } } );
+ $mech->content_contains( 'Updated!' );
+ };
+
+ subtest "Test changing user to an existing one" => sub {
+ my $existing_user = $mech->create_user_ok('existing@example.com', name => 'Existing User');
+ $mech->create_problems_for_body(2, 2514, 'Title', { user => $existing_user });
+ my $count = FixMyStreet::DB->resultset('Problem')->search({ user_id => $user->id })->count;
+ $mech->get_ok( '/admin/user_edit/' . $user->id );
+ $mech->submit_form_ok( { with_fields => { email => 'existing@example.com' } }, 'submit email change' );
+ is $mech->uri->path, '/admin/user_edit/' . $existing_user->id, 'redirected';
+ my $p = FixMyStreet::DB->resultset('Problem')->search({ user_id => $existing_user->id })->count;
+ is $p, $count + 2, 'reports merged';
+ };
+
+};
+
subtest "Test setting a report from unconfirmed to something else doesn't cause a front end error" => sub {
$report->update( { confirmed => undef, state => 'unconfirmed', non_public => 0 } );
$mech->get_ok("/admin/report_edit/$report_id");
@@ -1380,6 +1459,7 @@ subtest "Check admin_base_url" => sub {
$mech->log_out_ok;
subtest "Users without from_body can't access admin" => sub {
+ $user = FixMyStreet::App->model('DB::User')->find( { email => 'existing@example.com' } );
$user->from_body( undef );
$user->update;
diff --git a/templates/web/base/admin/report_blocks.html b/templates/web/base/admin/report_blocks.html
index f5896b88f..8e8b56393 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 email_in_abuse %]<small>[% loc('(Email in abuse table)') %]</small>[% ELSE %]<input type="submit" class="btn" name="banuser" value="[% loc('Ban email address') %]" />[% 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/report_edit.html b/templates/web/base/admin/report_edit.html
index 3c8134b80..c58b2d605 100644
--- a/templates/web/base/admin/report_edit.html
+++ b/templates/web/base/admin/report_edit.html
@@ -126,12 +126,17 @@ class="admin-offsite-link">[% problem.latitude %], [% problem.longitude %]</a>
</select></li>
<li><label for="name">[% loc('Name:') %]</label>
<input type='text' class="form-control" name='name' id='name' value='[% problem.name | html %]'></li>
-<li><label for="email">[% loc('Email:') %]</label>
- <input type='text' class="form-control" id='email' name='email' value='[% problem.user.email | html %]'>
+<li><label for="username">[% loc('User:') %]</label>
+ <input type='text' class="form-control" id='username' name='username' value='[% problem.user.username | html %]'>
[% PROCESS abuse_button %]
[% PROCESS flag_button user=problem.user %]
</li>
-<li>[% loc('Phone:') %] [% problem.user.phone | html %]</li>
+[% IF problem.user.phone_display != problem.user.username %]
+<li>[% loc('Phone:') %] [% problem.user.phone_display | html %]</li>
+[% END %]
+[% IF problem.user.email != problem.user.username %]
+<li>[% loc('Email:') %] [% problem.user.email | html %]</li>
+[% END %]
<li><label class="inline-text" for="flagged">[% loc('Flagged:') %]</label>
<input type="checkbox" id="flagged" name="flagged"[% ' checked' IF problem.flagged %]></li>
<li><label class="inline-text" for="non_public">[% loc('Private') %]:</label>
diff --git a/templates/web/base/admin/update_edit.html b/templates/web/base/admin/update_edit.html
index 2b20c50b3..34e64310f 100644
--- a/templates/web/base/admin/update_edit.html
+++ b/templates/web/base/admin/update_edit.html
@@ -31,8 +31,10 @@
<option [% 'selected ' IF state.0 == update.state %] value="[% state.0 %]">[% state.1 %]</option>
[% END %]
</select></li>
-<li>[% loc('Name:') %] <input type='text' class="form-control" name='name' id='name' value='[% update.name | html %]'></li>
-<li>[% loc('Email:') %] <input type='text' class="form-control" id='email' name='email' value='[% update.user.email | html %]'>
+<li><label for="name">[% loc('Name:') %]</label>
+ <input type='text' class="form-control" name='name' id='name' value='[% update.name | html %]'></li>
+<li><label for="username">[% loc('User:') %]</label>
+ <input type='text' class="form-control" id='username' name='username' value='[% update.user.username | html %]'>
[%- IF update.user.from_body && update.user.from_body.id == update.problem.bodies_str %]
[% ' (' _ tprintf(loc('user is from same council as problem - %d'), update.user.from_body.id ) _')' %]
[% END -%]
diff --git a/templates/web/base/admin/user-form.html b/templates/web/base/admin/user-form.html
index dbd554b1e..5637252e2 100644
--- a/templates/web/base/admin/user-form.html
+++ b/templates/web/base/admin/user-form.html
@@ -18,8 +18,20 @@
</li>
<li><label for="email">[% loc('Email:') %]</label>
<input type='text' class="form-control" id='email' name='email' value='[% user.email | html %]'></li>
+ [% IF c.config.SMS_AUTHENTICATION %]
+ <li><label class="inline" for="email_verified">[% loc('Email verified:') %]</label>
+ <input type="checkbox" id="email_verified" name="email_verified" value="1" [% user.email_verified ? ' checked' : '' %]>
+ [% ELSE %]
+ <input type="hidden" name="email_verified" value="1">
+ [% END %]
<li><label for="phone">[% loc('Phone:') %]</label>
<input type='text' class="form-control" id='phone' name='phone' value='[% user.phone | html %]'></li>
+ [% IF c.config.SMS_AUTHENTICATION %]
+ <li><label class="inline" for="phone_verified">[% loc('Phone verified:') %]</label>
+ <input type="checkbox" id="phone_verified" name="phone_verified" value="1" [% user.phone_verified ? ' checked' : '' %]>
+ [% ELSE %]
+ <input type="hidden" name="phone_verified" value="0">
+ [% END %]
[% IF c.user.is_superuser || c.cobrand.moniker == 'zurich' %]
<li>
diff --git a/templates/web/base/admin/users.html b/templates/web/base/admin/users.html
index 8e35e1c31..d367c18d8 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('(Email 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>
diff --git a/templates/web/zurich/admin/report_edit-sdm.html b/templates/web/zurich/admin/report_edit-sdm.html
index 07f0332d5..9f5433d8b 100644
--- a/templates/web/zurich/admin/report_edit-sdm.html
+++ b/templates/web/zurich/admin/report_edit-sdm.html
@@ -64,7 +64,7 @@
<br>
[% problem.user.email | html %]
[% IF NOT problem.extra.email_confirmed %]<span class="error">[% loc('Unconfirmed') %]</span>[% END %]
- <input type='hidden' id='email' name='email' value='[% problem.user.email | html %]'>
+ <input type='hidden' id='username' name='username' value='[% problem.user.username | html %]'>
<br>
[% IF problem.user.phone %][% problem.user.phone | html %][% ELSE %]<em>[% loc('(No phone number)') %]</em>[% END %]
</dd>
diff --git a/templates/web/zurich/admin/report_edit.html b/templates/web/zurich/admin/report_edit.html
index 35075a9f0..aa5029eb7 100644
--- a/templates/web/zurich/admin/report_edit.html
+++ b/templates/web/zurich/admin/report_edit.html
@@ -92,7 +92,7 @@
<br>
[% problem.user.email | html %]
[% IF NOT problem.extra.email_confirmed %]<span class="error">[% loc('Unconfirmed') %]</span>[% END %]
- <input type='hidden' id='email' name='email' value='[% problem.user.email | html %]'>
+ <input type='hidden' id='username' name='username' value='[% problem.user.username | html %]'>
<br>
[% IF problem.user.phone %][% problem.user.phone | html %][% ELSE %]<em>[% loc('(No phone number)') %]</em>[% END %]
</dd>
diff --git a/templates/web/zurich/admin/update_edit.html b/templates/web/zurich/admin/update_edit.html
index b2cde0b92..bcf849732 100644
--- a/templates/web/zurich/admin/update_edit.html
+++ b/templates/web/zurich/admin/update_edit.html
@@ -20,7 +20,7 @@
[% END %]
</select></li>
<input type='hidden' name='name' id='name' value='[% update.name | html %]'>
-<input type='hidden' id='email' name='email' value='[% update.user.email | html %]'>
+<input type='hidden' id='username' name='username' value='[% update.user.username | html %]'>
[% IF update.problem_state %]
<li>[% tprintf(loc('Update changed problem state to %s'), update.problem_state) %]</li>
[% END %]
generated by cgit v1.2.3 (git 2.25.1) at 2025-07-15 22:49:22 +0000