Skip to content

Commit

Permalink
Split up two username fields.
Browse files Browse the repository at this point in the history 
Rename the not-logging-in username field to username_register.
Keep the sign-in field as username because that e.g. overlaps
with auth code in two-factor authentication.
  • Loading branch information
@dracos
dracos committed Sep 24, 2020
1 parent 52aa059 commit 9e49ef9
Show file tree
Hide file tree
Showing 26 changed files with 137 additions and 106 deletions.
15 changes: 10 additions & 5 deletions perllib/FixMyStreet/App/Controller/Report/New.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ BEGIN { extends 'Catalyst::Controller'; }

use utf8;
use Encode;
use List::Util qw(first uniq);
use List::Util qw(uniq);
use HTML::Entities;
use Path::Class;
use Utils;
Expand Down Expand Up @@ -126,8 +126,10 @@ sub report_new_ajax : Path('mobile') : Args(0) {

# Apps are sending email as username
# Prepare for when they upgrade
if (!$c->get_param('username')) {
$c->set_param('username', $c->get_param('email'));
my $username_field = ( $c->get_param('submit_sign_in') || $c->get_param('password_sign_in') )
? 'username': 'username_register';
if (!$c->get_param($username_field)) {
$c->set_param($username_field, $c->get_param('email'));
}

# create the report - loading a partial if available
Expand Down Expand Up @@ -844,8 +846,11 @@ sub process_user : Private {
my %params = map { $_ => $c->get_param($_) }
qw( email name phone password_register fms_extra_title update_method );

# Report form includes two username fields: #form_username_register and #form_username_sign_in
$params{username} = (first { $_ } $c->get_param_list('username')) || '';
if ( $c->get_param('submit_sign_in') || $c->get_param('password_sign_in') ) {
$params{username} = $c->get_param('username');
} else {
$params{username} = $c->get_param('username_register');
}

my $anon_button = $c->cobrand->allow_anonymous_reports eq 'button' && $c->get_param('report_anonymously');
my $anon_fallback = $c->cobrand->allow_anonymous_reports eq '1' && !$c->user_exists && !$params{username};
Expand Down
8 changes: 5 additions & 3 deletions perllib/FixMyStreet/App/Controller/Report/Update.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ BEGIN { extends 'Catalyst::Controller'; }

use utf8;
use Path::Class;
use List::Util 'first';
use Utils;

=head1 NAME
Expand Down Expand Up @@ -103,8 +102,11 @@ sub process_user : Private {
my %params = map { $_ => $c->get_param($_) }
( 'name', 'password_register', 'fms_extra_title' );

# Update form includes two username fields: #form_username_register and #form_username_sign_in
$params{username} = (first { $_ } $c->get_param_list('username')) || '';
if ( $c->get_param('submit_sign_in') || $c->get_param('password_sign_in') ) {
$params{username} = $c->get_param('username');
} else {
$params{username} = $c->get_param('username_register');
}

my $anon_button = $c->cobrand->allow_anonymous_reports eq 'button' && $c->get_param('report_anonymously');
if ($anon_button) {
Expand Down
6 changes: 3 additions & 3 deletions t/app/controller/contact_enquiry.t
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ FixMyStreet::override_config {
$mech->submit_form_ok( {
with_fields => {
name => 'Test User',
username => 'testuser@example.org',
username_register => 'testuser@example.org',
category => 'Other',
detail => 'This is a general enquiry',
}
Expand Down Expand Up @@ -149,7 +149,7 @@ FixMyStreet::override_config {
$mech->submit_form_ok( {
with_fields => {
name => 'Simon Neil',
username => $user->email,
username_register => $user->email,
category => 'General Enquiry',
detail => 'This is a general enquiry',
}
Expand Down Expand Up @@ -212,7 +212,7 @@ FixMyStreet::override_config {
submit_problem => 1,
token => $csrf,
name => 'Test User',
username => 'testuser@example.org',
username_register => 'testuser@example.org',
category => 'Other',
detail => encode_utf8('This is a general enquiry‽'),
photo1 => [ $sample_jpeg, undef, Content_Type => 'image/jpeg' ],
Expand Down
22 changes: 11 additions & 11 deletions t/app/controller/report_as_other.t
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ subtest "Body user, has permission to add report as another user with email" =>
detail => 'Test report details.',
category => 'Potholes',
name => 'Another User',
username => 'another@example.net',
username_register => 'another@example.net',
);
is $report->name, 'Another User', 'report name is given name';
is $report->user->name, 'Another User', 'user name matches';
Expand All @@ -65,7 +65,7 @@ subtest "Body user, has permission to add report as another user with mobile pho
detail => 'Test report details.',
category => 'Potholes',
name => 'Another User',
username => '07906 111111',
username_register => '07906 111111',
);
is $report->name, 'Another User', 'report name is given name';
is $report->user->name, 'Another User', 'user name matches';
Expand All @@ -85,7 +85,7 @@ subtest "Body user, has permission to add report as another user with landline n
detail => 'Test report details.',
category => 'Potholes',
name => 'Another User',
username => '01685 222222',
username_register => '01685 222222',
);
is $report->name, 'Another User', 'report name is given name';
is $report->user->name, 'Another User', 'user name matches';
Expand All @@ -105,7 +105,7 @@ subtest "Body user, has permission to add report as another user with only name"
detail => 'Test report details.',
category => 'Potholes',
name => 'Another User',
username => '',
username_register => '',
may_show_name => undef,
);
is $report->name, 'Another User', 'report name is name given';
Expand All @@ -126,7 +126,7 @@ subtest "Body user, has permission to add report as another (existing) user with
detail => 'Test report details.',
category => 'Potholes',
name => 'Existing Yooser',
username => $existing->email,
username_register => $existing->email,
);
is $report->name, 'Existing Yooser', 'report name is given name';
is $report->user->name, 'Existing User', 'user name remains same';
Expand All @@ -153,7 +153,7 @@ subtest "Body user, has permission to add report as another (existing) user with
detail => 'Test report details.',
category => 'Potholes',
name => 'Existing Yooser',
username => '07906 333333',
username_register => '07906 333333',
);
is $report->name, 'Existing Yooser', 'report name is given name';
is $report->user->name, 'Existing User', 'user name remains same';
Expand Down Expand Up @@ -235,7 +235,7 @@ subtest "Body user, has permission to add update as another user with email" =>
form_as => 'another_user',
update => 'Test Update',
name => 'Another User',
username => 'another2@example.net',
username_register => 'another2@example.net',
);
is $update->name, 'Another User', 'update name is given name';
is $update->user->name, 'Another User', 'user name matches';
Expand All @@ -250,7 +250,7 @@ subtest "Body user, has permission to add update as another user with mobile pho
form_as => 'another_user',
update => 'Test Update',
name => 'Another User',
username => '07906 444444',
username_register => '07906 444444',
);
is $update->name, 'Another User', 'update name is given name';
is $update->user->name, 'Another User', 'user name matches';
Expand All @@ -265,7 +265,7 @@ subtest "Body user, has permission to add update as another user with landline p
form_as => 'another_user',
update => 'Test Update',
name => 'Another User',
username => '01685 555555',
username_register => '01685 555555',
);
is $update->name, 'Another User', 'update name is given name';
is $update->user->name, 'Another User', 'user name matches';
Expand All @@ -281,7 +281,7 @@ subtest "Body user, has permission to add update as another (existing) user with
form_as => 'another_user',
update => 'Test Update',
name => 'Existing Yooser',
username => $existing->email,
username_register => $existing->email,
);
is $update->name, 'Existing Yooser', 'update name is given name';
is $update->user->name, 'Existing User', 'user name remains same';
Expand All @@ -296,7 +296,7 @@ subtest "Body user, has permission to add update as another (existing) user with
form_as => 'another_user',
update => 'Test Update',
name => 'Existing Yooser',
username => '07906 333333',
username_register => '07906 333333',
);
is $update->name, 'Existing Yooser', 'update name is given name';
is $update->user->name, 'Existing User', 'user name remains same';
Expand Down
2 changes: 1 addition & 1 deletion t/app/controller/report_display.t
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ subtest "test a good report" => sub {

my %fields = (
name => '',
username => '',
username_register => '',
update => '',
add_alert => 1, # defaults to true
fixed => undef
Expand Down
2 changes: 1 addition & 1 deletion t/app/controller/report_import.t
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ subtest "Submit a correct entry (with location) to cobrand" => sub {
photo2 => '',
photo3 => '',
phone => '',
username => 'test-ll@example.com',
username_register => 'test-ll@example.com',
},
"check imported fields are shown"
or diag Dumper( $mech->visible_form_values ); use Data::Dumper;
Expand Down
10 changes: 5 additions & 5 deletions t/app/controller/report_new.t
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ foreach my $test (
photo1 => '',
name => 'Joe Bloggs',
may_show_name => '1',
username => 'test-1@example.com',
username_register => 'test-1@example.com',
phone => '07903 123 456',
category => 'Street lighting',
password_register => $test->{password} ? 'secret' : '',
Expand Down Expand Up @@ -680,7 +680,7 @@ subtest "test report creation for a category that is non public" => sub {
title => 'Test Report',
detail => 'Test report details.',
photo1 => '',
username => $user->email,
username_register => $user->email,
name => 'Joe Bloggs',
category => 'Street lighting',
}
Expand Down Expand Up @@ -940,7 +940,7 @@ for my $test (
title => "Test Report",
detail => 'Test report details.',
photo1 => '',
username => 'firstlast@example.com',
username_register => 'firstlast@example.com',
may_show_name => '1',
phone => '07903 123 456',
category => 'Trees',
Expand Down Expand Up @@ -1085,7 +1085,7 @@ subtest "test Hart" => sub {
$mech->submit_form_ok( { with_fields => { pc => 'GU51 4AE' } }, "submit location" );
$mech->follow_link_ok( { text_regex => qr/skip this step/i, }, "follow 'skip this step' link" );
my %optional_fields = $test->{confirm} ? () :
( username => $test_email, phone => '07903 123 456' );
( username_register => $test_email, phone => '07903 123 456' );

# we do this as otherwise test::www::mechanize::catalyst
# goes to the value set in ->host above irregardless and
Expand Down Expand Up @@ -1279,7 +1279,7 @@ subtest "extra google analytics code displayed on email confirmation problem cre
title => "Test Report",
detail => 'Test report details.',
photo1 => '',
username => 'firstlast@example.com',
username_register => 'firstlast@example.com',
name => 'Test User',
may_show_name => '1',
phone => '07903 123 456',
Expand Down

0 comments on commit 9e49ef9

Please sign in to comment.