diff options
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Reports/New.pm | 50 | ||||
| -rw-r--r-- | perllib/FixMyStreet/TestMech.pm | 36 | ||||
| -rw-r--r-- | t/app/controller/reports_new.t | 218 | ||||
| -rw-r--r-- | templates/web/default/reports/new/fill_in_details.html | 4 |
4 files changed, 290 insertions, 18 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Reports/New.pm b/perllib/FixMyStreet/App/Controller/Reports/New.pm index 80bf6abf8..0a29614c6 100644 --- a/perllib/FixMyStreet/App/Controller/Reports/New.pm +++ b/perllib/FixMyStreet/App/Controller/Reports/New.pm @@ -527,12 +527,15 @@ sub process_user : Private { map { $_ => scalar $c->req->param($_) } # ( 'email', 'name', 'phone', ); - my $report_user = - $c->model('DB::User')->find_or_new( { email => $params{email} } ); + # cleanup the email address + my $email = lc $params{email}; + $email =~ s{\s+}{}g; + + my $report_user = $c->model('DB::User')->find_or_new( { email => $email } ); # set the user's name and phone (if given) - $report_user->name( $params{name} ); - $report_user->phone( $params{phone} ) if $params{phone}; + $report_user->name( _trim_text( $params{name} ) ); + $report_user->phone( _trim_text( $params{phone} ) ) if $params{phone}; $c->stash->{report_user} = $report_user; @@ -547,15 +550,14 @@ provided) returns undef. =cut +# args: allow_multiline => bool - strips out "\n\n" linebreaks sub _cleanup_text { my $input = shift || ''; + my $args = shift || {}; # lowercase everything if looks like it might be SHOUTING $input = lc $input if $input !~ /[a-z]/; - # Start with a capital - $input = ucfirst $input; - # clean up language and tradmarks for ($input) { @@ -567,6 +569,27 @@ sub _cleanup_text { s{kabin\]}{cabin\]}ig; } + # Remove unneeded whitespace + my @lines = grep { m/\S/ } split m/\n\n/, $input; + for (@lines) { + $_ = _trim_text($_); + $_ = ucfirst $_; # start with capital + } + + my $join_char = $args->{allow_multiline} ? "\n\n" : " "; + $input = join $join_char, @lines; + + return $input; +} + +sub _trim_text { + my $input = shift; + for ($input) { + last unless $_; + s{\s+}{ }g; # all whitespace to single space + s{^ }{}; # trim leading + s{ $}{}; # trim trailing + } return $input; } @@ -574,7 +597,7 @@ sub process_report : Private { my ( $self, $c ) = @_; # Extract all the params to a hash to make them easier to work with - my %params = # + my %params = # map { $_ => scalar $c->req->param($_) } # ( 'title', 'detail', 'pc', # @@ -597,11 +620,12 @@ sub process_report : Private { # clean up text before setting $report->title( _cleanup_text( $params{title} ) ); - $report->detail( _cleanup_text( $params{detail} ) ); + $report->detail( + _cleanup_text( $params{detail}, { allow_multiline => 1 } ) ); # set these straight from the params - $report->name( $params{name} ); - $report->category( $params{category} ); + $report->name( _trim_text( $params{name} ) ); + $report->category( _ $params{category} ); my $mapit_query = sprintf( "4326/%s,%s", $report->longitude, $report->latitude ); @@ -896,10 +920,10 @@ END_MAP_HTML pins => [ [ $latitude, $longitude, 'purple' ] ], ); } - + # get the closing for the map $c->stash->{map_end} = FixMyStreet::Map::display_map_end(1); - + return 1; } diff --git a/perllib/FixMyStreet/TestMech.pm b/perllib/FixMyStreet/TestMech.pm index 03ece6c47..94e87f50b 100644 --- a/perllib/FixMyStreet/TestMech.pm +++ b/perllib/FixMyStreet/TestMech.pm @@ -8,6 +8,7 @@ use Test::WWW::Mechanize::Catalyst 'FixMyStreet::App'; use Test::More; use Web::Scraper; +use Carp; =head1 NAME @@ -84,4 +85,39 @@ sub extract_location { }; } +=head2 visible_form_values + + $hashref = $mech->visible_form_values( ); + +Return all the visible form values on the page - ie not the hidden ones. + +=cut + +sub visible_form_values { + my $mech = shift; + + my @forms = $mech->forms; + + # insert form filtering here (eg ignore login form) + + croak "Found no forms - can't continue..." + unless @forms; + croak "Found several forms - don't know which to use..." + if @forms > 1; + + my $form = $forms[0]; + + my @visible_fields = + grep { ref($_) ne 'HTML::Form::SubmitInput' } + grep { ref($_) ne 'HTML::Form::ImageInput' } + grep { ref($_) ne 'HTML::Form::TextInput' || $_->type ne 'hidden' } + $form->inputs; + + my @visible_field_names = map { $_->name } @visible_fields; + + my %params = map { $_ => $form->value($_) } @visible_field_names; + + return \%params; +} + 1; diff --git a/t/app/controller/reports_new.t b/t/app/controller/reports_new.t index 506678459..dd353a903 100644 --- a/t/app/controller/reports_new.t +++ b/t/app/controller/reports_new.t @@ -36,9 +36,9 @@ foreach my $test ( pc_alternatives => [], }, { - pc => 'glenthorpe', - errors => [], - pc_alternatives => [ + pc => 'glenthorpe', + errors => [], + pc_alternatives => [ # TODO - should filter out these non-UK addresses 'Glenthorpe Crescent, Leeds LS9 7, UK', 'Glenthorpe Rd, Merton, Greater London SM4 4, UK', 'Glenthorpe Ln, Katy, TX 77494, USA', @@ -89,6 +89,218 @@ foreach my $test ( is_deeply $mech->form_errors, [], "no errors for pc '$test->{pc}'"; is_deeply $mech->extract_location, $test, "got expected location for pc '$test->{pc}'"; +} + +# test that the various bit of form get filled in and errors correctly +# generated. +foreach my $test ( + { + msg => 'all fields empty', + pc => 'SW1A 1AA', + fields => { + title => '', + detail => '', + photo => '', + name => '', + may_show_name => '1', + email => '', + phone => '', + }, + changes => {}, + errors => [ + 'Please enter a subject', + 'Please enter some details', + 'Please enter your name', + 'Please enter your email', + ], + }, + { + msg => 'may_show_name defaults to true', + pc => 'SW1A 1AA', + fields => { + title => '', + detail => '', + photo => '', + name => '', + may_show_name => undef, + email => '', + phone => '', + }, + changes => { may_show_name => '1' }, + errors => [ + 'Please enter a subject', + 'Please enter some details', + 'Please enter your name', + 'Please enter your email', + ], + }, + { + msg => 'may_show_name unchanged if name is present (stays false)', + pc => 'SW1A 1AA', + fields => { + title => '', + detail => '', + photo => '', + name => 'Bob Jones', + may_show_name => undef, + email => '', + phone => '', + }, + changes => {}, + errors => [ + 'Please enter a subject', + 'Please enter some details', + 'Please enter your email', + ], + }, + { + msg => 'may_show_name unchanged if name is present (stays true)', + pc => 'SW1A 1AA', + fields => { + title => '', + detail => '', + photo => '', + name => 'Bob Jones', + may_show_name => '1', + email => '', + phone => '', + }, + changes => {}, + errors => [ + 'Please enter a subject', + 'Please enter some details', + 'Please enter your email', + ], + }, + { + msg => 'title and details tidied up', + pc => 'SW1A 1AA', + fields => { + title => 'DOG SHIT ON WALLS', + detail => 'on this portakabin - more of a portaloo HEH!!', + photo => '', + name => 'Bob Jones', + may_show_name => '1', + email => '', + phone => '', + }, + changes => { + title => 'Dog poo on walls', + detail => + 'On this [portable cabin] - more of a [portable loo] HEH!!', + }, + errors => [ 'Please enter your email', ], + }, + { + msg => 'name too short', + pc => 'SW1A 1AA', + fields => { + title => 'Test title', + detail => 'Test detail', + photo => '', + name => 'DUDE', + may_show_name => '1', + email => '', + phone => '', + }, + changes => {}, + errors => [ +'Please enter your full name, councils need this information - if you do not wish your name to be shown on the site, untick the box', + 'Please enter your email', + ], + }, + { + msg => 'name is anonymous', + pc => 'SW1A 1AA', + fields => { + title => 'Test title', + detail => 'Test detail', + photo => '', + name => 'anonymous', + may_show_name => '1', + email => '', + phone => '', + }, + changes => {}, + errors => [ +'Please enter your full name, councils need this information - if you do not wish your name to be shown on the site, untick the box', + 'Please enter your email', + ], + }, + { + msg => 'email invalid', + pc => 'SW1A 1AA', + fields => { + title => 'Test title', + detail => 'Test detail', + photo => '', + name => 'Joe Smith', + may_show_name => '1', + email => 'not an email', + phone => '', + }, + changes => { email => 'notanemail', }, + errors => [ 'Please enter a valid email', ], + }, + { + msg => 'cleanup title and detail', + pc => 'SW1A 1AA', + fields => { + title => " Test title ", + detail => " first line \n\n second\nline\n\n ", + photo => '', + name => '', + may_show_name => '1', + email => '', + phone => '', + }, + changes => { + title => 'Test title', + detail => "First line\n\nSecond line", + }, + errors => [ 'Please enter your name', 'Please enter your email', ], + }, + { + msg => 'clean up name and email', + pc => 'SW1A 1AA', + fields => { + title => '', + detail => '', + photo => '', + name => ' Bob Jones ', + may_show_name => '1', + email => ' BOB @ExAmplE.COM ', + phone => '', + }, + changes => { + name => 'Bob Jones', + email => 'bob@example.com', + }, + errors => [ 'Please enter a subject', 'Please enter some details', ], + }, + ) +{ + pass "--- $test->{msg} ---"; + $mech->get_ok('/reports/new'); + + # submit initial pc form + $mech->submit_form_ok( { with_fields => { pc => $test->{pc} } }, + "submit location" ); + is_deeply $mech->form_errors, [], "no errors for pc '$test->{pc}'"; + + # submit the main form + $mech->submit_form_ok( { with_fields => $test->{fields} }, "submit form" ); + + # check that we got the errors expected + is_deeply $mech->form_errors, $test->{errors}, "check errors"; + + # check that fields have changed as expected + my $new_values = { + %{ $test->{fields} }, # values added to form + %{ $test->{changes} }, # changes we expect + }; + is_deeply $mech->visible_form_values, $new_values, + "values correctly changed"; } diff --git a/templates/web/default/reports/new/fill_in_details.html b/templates/web/default/reports/new/fill_in_details.html index caa406bde..a6c7bf047 100644 --- a/templates/web/default/reports/new/fill_in_details.html +++ b/templates/web/default/reports/new/fill_in_details.html @@ -108,8 +108,8 @@ <div class="checkbox"> - [%# if there is nothing in the title field then set check box as default on form %] - <input type="checkbox" name="may_show_name" id="form_may_show_name" value="1"[% ' checked' IF !report.anonymous || !report.title %]> + [%# if there is nothing in the name field then set check box as default on form %] + <input type="checkbox" name="may_show_name" id="form_may_show_name" value="1"[% ' checked' IF !report.anonymous || !report.name %]> <!-- FIXME - empythomes should be 'Can we show your name on the site?'--> <label for="form_may_show_name">[% loc('Can we show your name publicly?') %]</label> |