aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--perllib/FixMyStreet/App/Controller/Report/New.pm74
-rw-r--r--perllib/FixMyStreet/DB/Result/Problem.pm2
-rw-r--r--t/app/controller/auth.t2
-rw-r--r--t/app/controller/report_new.t167
-rw-r--r--templates/web/default/alert/updates.html2
-rw-r--r--templates/web/default/auth/general.html2
-rw-r--r--templates/web/default/report/display.html2
-rw-r--r--templates/web/default/report/new/fill_in_details.html116
-rw-r--r--web/js/fixmystreet.js2
9 files changed, 293 insertions, 76 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm
index a6be6c90c..4488ce8cd 100644
--- a/perllib/FixMyStreet/App/Controller/Report/New.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/New.pm
@@ -28,7 +28,10 @@ Create a new report, or complete a partial one .
=head2 flow control
-submit_problem: true if a problem has been submitted
+submit_problem: true if a problem has been submitted, at all.
+submit_sign_in: true if the sign in button has been clicked by logged out user.
+submit_register: true if the register/confirm by email button has been clicked
+by logged out user.
=head2 location (required)
@@ -90,14 +93,13 @@ sub report_new : Path : Args(0) {
$c->forward('generate_map');
# deal with the user and report and check both are happy
- return
- unless $c->forward('check_form_submitted')
- && $c->forward('process_user')
- && $c->forward('process_report')
- && $c->forward('process_photo')
- && $c->forward('check_for_errors')
- && $c->forward('save_user_and_report')
- && $c->forward('redirect_or_confirm_creation');
+ return unless $c->forward('check_form_submitted');
+ $c->forward('process_user');
+ $c->forward('process_report');
+ $c->forward('process_photo');
+ return unless $c->forward('check_for_errors');
+ $c->forward('save_user_and_report');
+ $c->forward('redirect_or_confirm_creation');
}
=head2 report_import
@@ -548,22 +550,48 @@ Load user from the database or prepare a new one.
sub process_user : Private {
my ( $self, $c ) = @_;
+ my $report = $c->stash->{report};
+
+ # The user is already signed in
+ if ( $c->user_exists ) {
+ my $user = $c->user->obj;
+ my %params = map { $_ => scalar $c->req->param($_) } ( 'name', 'phone' );
+ $user->name( Utils::trim_text( $params{name} ) ) if $params{name};
+ $user->phone( Utils::trim_text( $params{phone} ) );
+ $report->user( $user );
+ $report->name( $user->name );
+ return 1;
+ }
+
# Extract all the params to a hash to make them easier to work with
- my %params = #
- map { $_ => scalar $c->req->param($_) } #
- ( 'email', 'name', 'phone', );
+ my %params = map { $_ => scalar $c->req->param($_) }
+ ( 'email', 'name', 'phone', 'password_register' );
# cleanup the email address
my $email = $params{email} ? lc $params{email} : '';
$email =~ s{\s+}{}g;
- my $report = $c->stash->{report};
$report->user( $c->model('DB::User')->find_or_new( { email => $email } ) )
unless $report->user;
- # set the user's name and phone (if given)
+ # The user is trying to sign in. We only care about email from the params.
+ if ( $c->req->param('submit_sign_in') ) {
+ unless ( $c->forward( '/auth/sign_in' ) ) {
+ $c->stash->{field_errors}->{password} = _('There was a problem with your email/password combination. Please try again.');
+ return 1;
+ }
+ my $user = $c->user->obj;
+ $report->user( $user );
+ $report->name( $user->name );
+ $c->stash->{field_errors}->{name} = _('You have successfully signed in; please check and confirm your details are accurate:');
+ return 1;
+ }
+
+ # set the user's name, phone, and password
$report->user->name( Utils::trim_text( $params{name} ) ) if $params{name};
$report->user->phone( Utils::trim_text( $params{phone} ) );
+ $report->user->password( Utils::trim_text( $params{password_register} ) );
+ $report->name( Utils::trim_text( $params{name} ) );
return 1;
}
@@ -584,7 +612,7 @@ sub process_report : Private {
map { $_ => scalar $c->req->param($_) } #
(
'title', 'detail', 'pc', #
- 'name', 'may_show_name', #
+ 'may_show_name', #
'category', #
'partial', #
);
@@ -606,7 +634,6 @@ sub process_report : Private {
Utils::cleanup_text( $params{detail}, { allow_multiline => 1 } ) );
# set these straight from the params
- $report->name( Utils::trim_text( $params{name} ) );
$report->category( _ $params{category} );
my $areas = $c->stash->{all_areas};
@@ -624,10 +651,7 @@ sub process_report : Private {
} elsif ( $first_council->{type} eq 'LBO') {
unless ( Utils::london_categories()->{ $report->category } ) {
- # TODO Perfect world, this wouldn't short-circuit, other errors would
- # be included as well.
- $c->stash->{field_errors} = { category => _('Please choose a category') };
- return;
+ $c->stash->{field_errors}->{category} = _('Please choose a category');
}
$report->council( $first_council->{id} );
@@ -646,8 +670,9 @@ sub process_report : Private {
)->all;
unless ( @contacts ) {
- $c->stash->{field_errors} = { category => _('Please choose a category') };
- return;
+ $c->stash->{field_errors}->{category} = _('Please choose a category');
+ $report->council( -1 );
+ return 1;
}
# construct the council string:
@@ -662,8 +687,7 @@ sub process_report : Private {
} elsif ( @{ $c->stash->{area_ids_to_list} } ) {
# There was an area with categories, but we've not been given one. Bail.
- $c->stash->{field_errors} = { category => _('Please choose a category') };
- return;
+ $c->stash->{field_errors}->{category} = _('Please choose a category');
} else {
@@ -783,7 +807,9 @@ sub check_for_errors : Private {
my ( $self, $c ) = @_;
# let the model check for errors
+ $c->stash->{field_errors} ||= {};
my %field_errors = (
+ %{ $c->stash->{field_errors} },
%{ $c->stash->{report}->user->check_for_errors },
%{ $c->stash->{report}->check_for_errors },
);
diff --git a/perllib/FixMyStreet/DB/Result/Problem.pm b/perllib/FixMyStreet/DB/Result/Problem.pm
index c5851b256..da23802c3 100644
--- a/perllib/FixMyStreet/DB/Result/Problem.pm
+++ b/perllib/FixMyStreet/DB/Result/Problem.pm
@@ -188,7 +188,7 @@ sub check_for_errors {
unless $self->council
&& $self->council =~ m/^(?:-1|[\d,]+(?:\|[\d,]+)?)$/;
- if ( $self->name !~ m/\S/ ) {
+ if ( !$self->name || $self->name !~ m/\S/ ) {
$errors{name} = _('Please enter your name');
}
elsif (length( $self->name ) < 5
diff --git a/t/app/controller/auth.t b/t/app/controller/auth.t
index 8fbc413ec..67dfa1869 100644
--- a/t/app/controller/auth.t
+++ b/t/app/controller/auth.t
@@ -224,7 +224,7 @@ $mech->submit_form_ok(
"sign in with '$test_email' & '$test_password"
);
is $mech->uri->path, '/auth', "redirected to correct page";
-$mech->content_contains( 'Email or password wrong', 'found error message' );
+$mech->content_contains( 'problem with your email/password combination', 'found error message' );
# more test:
# TODO: test that email are always lowercased
diff --git a/t/app/controller/report_new.t b/t/app/controller/report_new.t
index 3e486d22c..537786ebc 100644
--- a/t/app/controller/report_new.t
+++ b/t/app/controller/report_new.t
@@ -65,13 +65,16 @@ foreach my $test (
email => '',
phone => '',
category => 'Street lighting',
+ password_sign_in => '',
+ password_register => '',
+ remember_me => undef,
},
changes => {},
errors => [
'Please enter a subject',
'Please enter some details',
- 'Please enter your name',
'Please enter your email',
+ 'Please enter your name',
],
},
{
@@ -86,13 +89,16 @@ foreach my $test (
email => '',
phone => '',
category => 'Street lighting',
+ password_sign_in => '',
+ password_register => '',
+ remember_me => undef,
},
changes => { may_show_name => '1' },
errors => [
'Please enter a subject',
'Please enter some details',
- 'Please enter your name',
'Please enter your email',
+ 'Please enter your name',
],
},
{
@@ -107,6 +113,9 @@ foreach my $test (
email => '',
phone => '',
category => 'Street lighting',
+ password_sign_in => '',
+ password_register => '',
+ remember_me => undef,
},
changes => {},
errors => [
@@ -127,6 +136,9 @@ foreach my $test (
email => '',
phone => '',
category => 'Street lighting',
+ password_sign_in => '',
+ password_register => '',
+ remember_me => undef,
},
changes => {},
errors => [
@@ -147,6 +159,9 @@ foreach my $test (
email => '',
phone => '',
category => 'Street lighting',
+ password_sign_in => '',
+ password_register => '',
+ remember_me => undef,
},
changes => {
title => 'Dog poo on walls',
@@ -167,11 +182,14 @@ foreach my $test (
email => '',
phone => '',
category => 'Street lighting',
+ password_sign_in => '',
+ password_register => '',
+ remember_me => undef,
},
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',
+'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',
],
},
{
@@ -186,11 +204,14 @@ foreach my $test (
email => '',
phone => '',
category => 'Street lighting',
+ password_sign_in => '',
+ password_register => '',
+ remember_me => undef,
},
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',
+'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',
],
},
{
@@ -205,6 +226,9 @@ foreach my $test (
email => 'not an email',
phone => '',
category => 'Street lighting',
+ password_sign_in => '',
+ password_register => '',
+ remember_me => undef,
},
changes => { email => 'notanemail', },
errors => [ 'Please enter a valid email', ],
@@ -221,12 +245,18 @@ foreach my $test (
email => '',
phone => '',
category => 'Street lighting',
+ password_sign_in => '',
+ password_register => '',
+ remember_me => undef,
},
changes => {
title => 'Test title',
detail => "First line\n\nSecond line",
},
- errors => [ 'Please enter your name', 'Please enter your email', ],
+ errors => [
+ 'Please enter your email',
+ 'Please enter your name',
+ ],
},
{
msg => 'clean up name and email',
@@ -240,6 +270,9 @@ foreach my $test (
email => ' BOB @ExAmplE.COM ',
phone => '',
category => 'Street lighting',
+ password_sign_in => '',
+ password_register => '',
+ remember_me => undef,
},
changes => {
name => 'Bob Jones',
@@ -278,14 +311,30 @@ foreach my $test (
};
}
-subtest "test report creation for a user who does not have an account" => sub {
+foreach my $test (
+ {
+ desc => 'does not have an account',
+ user => 0,
+ },
+ {
+ desc => 'does have an account and is not signed in; does not sign in',
+ user => 1,
+ }
+) {
+ subtest "test report creation for a user who " . $test->{desc} => sub {
$mech->log_out_ok;
$mech->clear_emails_ok;
# check that the user does not exist
my $test_email = 'test-1@example.com';
- ok !FixMyStreet::App->model('DB::User')->find( { email => $test_email } ),
- "test user does not exist";
+ if ($test->{user}) {
+ my $user = FixMyStreet::App->model('DB::User')->find( { email => $test_email } );
+ ok $user, "test user does exist";
+ $user->problems->delete;
+ } else {
+ ok !FixMyStreet::App->model('DB::User')->find( { email => $test_email } ),
+ "test user does not exist";
+ }
# submit initial pc form
$mech->get_ok('/around');
@@ -298,6 +347,7 @@ subtest "test report creation for a user who does not have an account" => sub {
$mech->submit_form_ok(
{
+ button => 'submit_register',
with_fields => {
title => 'Test Report',
detail => 'Test report details.',
@@ -307,6 +357,7 @@ subtest "test report creation for a user who does not have an account" => sub {
email => 'test-1@example.com',
phone => '07903 123 456',
category => 'Street lighting',
+ password_register => 'secret',
}
},
"submit good details"
@@ -319,6 +370,7 @@ subtest "test report creation for a user who does not have an account" => sub {
my $user =
FixMyStreet::App->model('DB::User')->find( { email => $test_email } );
ok $user, "created new user";
+ ok $user->check_password('secret'), 'password set correctly';
# find the report
my $report = $user->problems->first;
@@ -358,15 +410,98 @@ subtest "test report creation for a user who does not have an account" => sub {
$mech->logged_in_ok;
# cleanup
- $mech->delete_user($user);
-};
+ $mech->delete_user($user)
+ if $test->{user};
+ };
+}
+
+subtest "test report creation for a user who is signing in as they report" => sub {
+ $mech->log_out_ok;
+ $mech->clear_emails_ok;
+
+ # check that the user does not exist
+ my $test_email = 'test-2@example.com';
+
+ my $user = FixMyStreet::App->model('DB::User')->find_or_create( { email => $test_email } );
+ ok $user, "test user does exist";
+
+ # setup the user.
+ ok $user->update( {
+ name => 'Joe Bloggs',
+ phone => '01234 567 890',
+ password => 'secret2',
+ } ), "set user details";
+
+ # submit initial pc form
+ $mech->get_ok('/around');
+ $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB', } },
+ "submit location" );
+
+ # click through to the report page
+ $mech->follow_link_ok( { text => 'skip this step', },
+ "follow 'skip this step' link" );
+
+ $mech->submit_form_ok(
+ {
+ button => 'submit_sign_in',
+ with_fields => {
+ title => 'Test Report',
+ detail => 'Test report details.',
+ photo => '',
+ email => 'test-2@example.com',
+ password_sign_in => 'secret2',
+ category => 'Street lighting',
+ }
+ },
+ "submit good details"
+ );
+
+ # check that we got the errors expected
+ is_deeply $mech->form_errors, [
+ 'You have successfully signed in; please check and confirm your details are accurate:',
+ ], "check there were errors";
+
+ # Now submit with a name
+ $mech->submit_form_ok(
+ {
+ with_fields => {
+ name => 'Joe Bloggs',
+ }
+ },
+ "submit good details"
+ );
+
+ # find the report
+ my $report = $user->problems->first;
+ ok $report, "Found the report";
-#### test report creation for a user who has account but is not logged in
-# come to site
-# fill in report
-# receive token
-# confirm token
-# report is confirmed
+ # check that we got redirected to /report/
+ is $mech->uri->path, "/report/" . $report->id, "redirected to report page";
+
+ # Check the report has been assigned appropriately
+ is $report->council, 2651;
+
+ # check that no emails have been sent
+ $mech->email_count_is(0);
+
+ # check report is confirmed and available
+ is $report->state, 'confirmed', "report is now confirmed";
+ $mech->get_ok( '/report/' . $report->id );
+
+ # check that the reporter has an alert
+ my $alert = FixMyStreet::App->model('DB::Alert')->find( {
+ user => $report->user,
+ alert_type => 'new_updates',
+ parameter => $report->id,
+ } );
+ ok $alert, "created new alert";
+
+ # user is created and logged in
+ $mech->logged_in_ok;
+
+ # cleanup
+ $mech->delete_user($user)
+};
#### test report creation for user with account and logged in
foreach my $test (
diff --git a/templates/web/default/alert/updates.html b/templates/web/default/alert/updates.html
index 88b014ff6..76b5ef23e 100644
--- a/templates/web/default/alert/updates.html
+++ b/templates/web/default/alert/updates.html
@@ -11,7 +11,7 @@
<form action="/alert/subscribe" method="post">
<label class="n" for="alert_rznvy">[% loc('Email:') %]</label>
-<input type="text" name="rznvy" id="alert_rznvy" value="[% email | html %]" size="30">
+<input type="email" name="rznvy" id="alert_rznvy" value="[% email | html %]" size="30">
<input type="hidden" name="id" value="[% problem_id | html %]">
<input type="hidden" name="type" value="updates">
<input type="submit" value="[% loc('Subscribe') %]">
diff --git a/templates/web/default/auth/general.html b/templates/web/default/auth/general.html
index d30fefcee..ffc85b3dc 100644
--- a/templates/web/default/auth/general.html
+++ b/templates/web/default/auth/general.html
@@ -20,7 +20,7 @@
[% IF loc_email_error %]
<div class="form-error">[% loc_email_error %]</div>
[% ELSIF sign_in_error %]
- <div class="form-error">[% loc('Email or password wrong - please try again.') %]</div>
+ <div class="form-error">[% loc('There was a problem with your email/password combination. Please try again.') %]</div>
[% END %]
<div class="form-field">
diff --git a/templates/web/default/report/display.html b/templates/web/default/report/display.html
index 40c5f081f..3c9768657 100644
--- a/templates/web/default/report/display.html
+++ b/templates/web/default/report/display.html
@@ -44,7 +44,7 @@
<form action="[% c.uri_for( '/alert/subscribe' ) %]" method="post" id="email_alert_box">
<p>[% loc('Receive email when updates are left on this problem.' ) %]</p>
<label class="n" for="alert_rznvy">[% loc('Email:') %]</label>
- <input type="text" name="rznvy" id="alert_rznvy" value="[% email | html %]" size="30">
+ <input type="email" name="rznvy" id="alert_rznvy" value="[% email | html %]" size="30">
<input type="hidden" name="id" value="[% problem.id %]">
<input type="hidden" name="type" value="updates">
<input type="submit" value="[% loc('Subscribe') %]">
diff --git a/templates/web/default/report/new/fill_in_details.html b/templates/web/default/report/new/fill_in_details.html
index 4a40dd707..68443770d 100644
--- a/templates/web/default/report/new/fill_in_details.html
+++ b/templates/web/default/report/new/fill_in_details.html
@@ -5,7 +5,6 @@
[% IF report.used_map %]
<form action="[% c.uri_for('/report/new') %]" method="post" name="mapForm" id="mapForm"[% IF c.cobrand.allow_photo_upload %] enctype="multipart/form-data"[% END %]>
-<input type="hidden" name="submit_map" value="1">
<input type="hidden" name="map" value="[% c.req.params.map | html %]">
<input type="hidden" name="pc" value="[% pc | html %]">
[% c.cobrand.form_elements('mapForm') %]
@@ -118,58 +117,90 @@
[% END %]
<label for="form_photo">[% loc('Photo:') %]</label>
- <input type="file" name="photo" id="form_photo" >
+ <input type="file" name="photo" id="form_photo" style="width:20em">
</div>
[% END %]
-[% IF field_errors.name %]
- <div class='form-error'>[% field_errors.name %]</div>
-[% END %]
-
-<div class='form-field'>
- <label for="form_name">[% loc('Name:') %]</label>
- <input type="text" value="[% report.name | html %]" name="name" id="form_name" size="25">
-</div>
+[% IF c.user_exists %]
+ [% INCLUDE name_phone %]
-<div class="checkbox">
+ [% INCLUDE 'report/new/notes.html' %]
- [%# 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 %]>
-
- <label for="form_may_show_name">[% loc('Show my name publicly') %]</label>
- <small>[% loc('(we never show your email address or phone number)') %]</small>
-</div>
+ <p id="problem_submit">
+ <input type="submit" value="[% loc('Submit') %]">
+ </p>
-[% IF NOT c.user_exists %]
+[% ELSE %]
[% IF field_errors.email %]
<div class='form-error'>[% field_errors.email %]</div>
[% END %]
<div class="form-field">
- <label for="form_email">[% loc('Email:') %]</label>
- <input type="text" value="[% report.user.email | html %]" name="email" id="form_email" size="25">
+ <label for="form_email">[% loc('Your email:') %]</label>
+ <input type="email" value="[% report.user.email | html %]" name="email" id="form_email" size="25">
</div>
-[% END %]
+[% INCLUDE 'report/new/notes.html' %]
+
+<div id="form_sign_in">
+ <h3>[% loc("Now to submit your report&hellip; do you have a FixMyStreet password?") %]</h3>
+
+ <div id="form_sign_in_yes">
+
+ [% IF field_errors.password %]
+ <div class='form-error'>[% field_errors.password %]</div>
+ [% END %]
+
+ <p>
+ <label class="n" for="password_sign_in">[% loc('<strong>Yes</strong>, I have a password:') %]</label>
+ <input type="password" name="password_sign_in" id="password_sign_in" value="" size="25">
+ </p>
+
+ <p>
+ <input type="checkbox" name="remember_me" value='1'[% ' checked' IF remember_me %]>
+ <label class="n" for="remember_me">
+ [% loc('Keep me signed in on this computer') %]
+ </label>
+ </p>
+
+ <p>
+ <input type="submit" name="submit_sign_in" value="[% loc('Submit') %]">
+ </p>
+
+ </div>
+ <div id="form_sign_in_no">
+
+ <p>[% loc('<strong>No</strong>, let me confirm my report by email:') %]</p>
+
+ <div id="fieldset">
+
+ [% INCLUDE name_phone %]
+
+ <div class="form-field">
+ <label for="password_register">[% loc('Enter a new password:') %]</label>
+ <input type="password" name="password_register" id="password_register" value="" size="25">
+ </div>
+ </div>
+
+ <p style="clear:both"><small>[% loc('Providing a password is optional, but doing so will allow you to more easily report problems, leave updates and manage your reports.') %]</small></p>
+
+ <p>
+ <input type="submit" name="submit_register" value="[% loc('Submit') %]">
+ </p>
+
+ </div>
-<div>
- <label for="form_phone">[% loc('Phone:') %]</label>
- <input type="text" value="[% report.user.phone | html %]" name="phone" id="form_phone" size="15">
- <small>[% loc('(optional)') %]</small>
</div>
-[% INCLUDE 'report/new/notes.html' %]
+[% END %]
[% IF partial_token %]
<input type="hidden" name="partial" value="[% partial_token.token %]">
[% END %]
-<p id="problem_submit">
- <input type="hidden" name="submit_problem" value="1">
- <input type="submit" value="[% loc('Submit') %]">
-</p>
+<input type="hidden" name="submit_problem" value="1">
</div>
</div>
@@ -178,3 +209,28 @@
</form>
[% INCLUDE 'footer.html' %]
+
+[% BLOCK name_phone %]
+ [% IF field_errors.name %]
+ <div class='form-error'>[% field_errors.name %]</div>
+ [% END %]
+
+ <div class="form-field">
+ <label for="form_name">[% loc('Your name:') %]</label>
+ <input type="text" value="[% report.name | html %]" name="name" id="form_name" size="25">
+ </div>
+
+ <div class="checkbox">
+ [%# 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 %]>
+ <label for="form_may_show_name">[% loc('Show my name publicly') %]</label>
+ <br><small>[% loc('(we never show your email address or phone number)') %]</small>
+ </div>
+
+ <div>
+ <label for="form_phone">[% loc('Phone:') %]</label>
+ <input type="text" value="[% report.user.phone | html %]" name="phone" id="form_phone" size="15">
+ <small>[% loc('(optional)') %]</small>
+ </div>
+[% END %]
+
diff --git a/web/js/fixmystreet.js b/web/js/fixmystreet.js
index 7f6014c6c..50ccb2ac3 100644
--- a/web/js/fixmystreet.js
+++ b/web/js/fixmystreet.js
@@ -12,7 +12,7 @@ YAHOO.util.Event.onContentReady('pc', function() {
YAHOO.util.Event.onContentReady('mapForm', function() {
this.onsubmit = function() {
- if (this.submit_problem) {
+ if (this.submit_problem) {
this.onsubmit = function() { return false; };
}
generated by cgit v1.2.3 (git 2.25.1) at 2025-07-22 07:35:18 +0000