Allow people to sign in (or not) as they make an update.

5 files changed, 227 insertions, 84 deletions

diff --git a/perllib/FixMyStreet/App/Controller/Report/Update.pm b/perllib/FixMyStreet/App/Controller/Report/Update.pm
index d586035bc..71031bb0d 100644
--- a/perllib/FixMyStreet/App/Controller/Report/Update.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/Update.pm
@@ -20,15 +20,15 @@ Creates an update to a report
 sub report_update : Path : Args(0) {
     my ( $self, $c ) = @_;
 
-         $c->forward( '/report/load_problem_or_display_error', [ $c->req->param('id') ] )
-      && $c->forward('process_update')
-      && $c->forward('process_user')
-      && $c->forward('/report/new/process_photo')
-      && $c->forward('check_for_errors')
+    $c->forward( '/report/load_problem_or_display_error', [ $c->req->param('id') ] );
+    $c->forward('process_update');
+    $c->forward('process_user');
+    $c->forward('/report/new/process_photo');
+    $c->forward('check_for_errors')
       or $c->go( '/report/display', [ $c->req->param('id') ] );
 
-    return $c->forward('save_update')
-      && $c->forward('redirect_or_confirm_creation');
+    $c->forward('save_update');
+    $c->forward('redirect_or_confirm_creation');
 }
 
 sub confirm : Private {
@@ -88,12 +88,17 @@ sub process_user : Private {
 
     my $update = $c->stash->{update};
 
-    $update->user( $c->user->obj ) if $c->user;
+    if ( $c->user_exists ) {
+        my $user = $c->user->obj;
+        my $name = scalar $c->req->param('name');
+        $user->name( Utils::trim_text( $name ) ) if $name;
+        $update->user( $user );
+        return 1;
+    }
 
     # Extract all the params to a hash to make them easier to work with
-    my %params =    #
-      map { $_ => scalar $c->req->param($_) }    #
-      ( 'rznvy', 'name' );
+    my %params = map { $_ => scalar $c->req->param($_) }
+      ( 'rznvy', 'name', 'password_register' );
 
     # cleanup the email address
     my $email = $params{rznvy} ? lc $params{rznvy} : '';
@@ -102,8 +107,22 @@ sub process_user : Private {
     $update->user( $c->model('DB::User')->find_or_new( { email => $email } ) )
         unless $update->user;
 
+    # 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', [ $email ] ) ) {
+            $c->stash->{field_errors}->{password} = _('There was a problem with your email/password combination. Please try again.');
+            return 1;
+        }
+        my $user = $c->user->obj;
+        $update->user( $user );
+        $update->name( $user->name );
+        $c->stash->{field_errors}->{name} = _('You have successfully signed in; please check and confirm your details are accurate:');
+        return 1;
+    }
+
     $update->user->name( Utils::trim_text( $params{name} ) )
         if $params{name};
+    $update->user->password( Utils::trim_text( $params{password_register} ) );
 
     return 1;
 }
@@ -128,10 +147,7 @@ sub process_update : Private {
       Utils::cleanup_text( $params{update}, { allow_multiline => 1 } );
 
     my $name = Utils::trim_text( $params{name} );
-
-    my $anonymous = 1;
-
-    $anonymous = 0 if ( $name && $c->req->param('may_show_name') );
+    my $anonymous = $c->req->param('may_show_name') ? 0 : 1;
 
     my $update = $c->model('DB::Comment')->new(
         {
@@ -165,25 +181,27 @@ 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->{update}->user->check_for_errors },
         %{ $c->stash->{update}->check_for_errors },
     );
 
-    # we don't care if there are errors with this...
-    delete $field_errors{name};
+    if ( my $photo_error  = delete $c->stash->{photo_error} ) {
+        $field_errors{photo} = $photo_error;
+    }
 
     # all good if no errors
     return 1
       unless ( scalar keys %field_errors
-        || ( $c->stash->{errors} && scalar @{ $c->stash->{errors} } )
-        || $c->stash->{photo_error} );
+        || ( $c->stash->{errors} && scalar @{ $c->stash->{errors} } ) );
 
     $c->stash->{field_errors} = \%field_errors;
 
     $c->stash->{errors} ||= [];
-    push @{ $c->stash->{errors} },
-      _('There were problems with your update. Please see below.');
+    #push @{ $c->stash->{errors} },
+    #  _('There were problems with your update. Please see below.');
 
     return;
 }
diff --git a/perllib/FixMyStreet/DB/Result/Comment.pm b/perllib/FixMyStreet/DB/Result/Comment.pm
index 18bcedc1b..ae152eb31 100644
--- a/perllib/FixMyStreet/DB/Result/Comment.pm
+++ b/perllib/FixMyStreet/DB/Result/Comment.pm
@@ -105,6 +105,9 @@ sub check_for_errors {
 
     my %errors = ();
 
+    $errors{name} = _('Please enter your name')
+        if !$self->name || $self->name !~ m/\S/;
+
     $errors{update} = _('Please enter a message')
       unless $self->text =~ m/\S/;
 
diff --git a/perllib/FixMyStreet/DB/Result/User.pm b/perllib/FixMyStreet/DB/Result/User.pm
index 95e9908c3..4ee413a58 100644
--- a/perllib/FixMyStreet/DB/Result/User.pm
+++ b/perllib/FixMyStreet/DB/Result/User.pm
@@ -84,14 +84,6 @@ sub check_for_errors {
     if ( !$self->name || $self->name !~ m/\S/ ) {
         $errors{name} = _('Please enter your name');
     }
-    elsif (length( $self->name ) < 5
-        || $self->name !~ m/\s/
-        || $self->name =~ m/\ba\s*n+on+((y|o)mo?u?s)?(ly)?\b/i )
-    {
-        $errors{name} = _(
-'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'
-        );
-    }
 
     if ( $self->email !~ /\S/ ) {
         $errors{email} = _('Please enter your email');
diff --git a/t/app/controller/report_updates.t b/t/app/controller/report_updates.t
index fb2f7abb7..9a3cb3947 100644
--- a/t/app/controller/report_updates.t
+++ b/t/app/controller/report_updates.t
@@ -201,9 +201,12 @@ for my $test (
             fixed  => undef,
             add_alert => 1,
             may_show_name => undef,
+            remember_me => undef,
+            password_register => '',
+            password_sign_in => '',
         },
         changes => {},
-        field_errors => [ 'Please enter a message', 'Please enter your email' ]
+        field_errors => [ 'Please enter a message', 'Please enter your email', 'Please enter your name' ]
     },
     {
         desc => 'Invalid email, no message',
@@ -215,9 +218,12 @@ for my $test (
             fixed  => undef,
             add_alert => 1,
             may_show_name => undef,
+            remember_me => undef,
+            password_sign_in => '',
+            password_register => '',
         },
         changes => {},
-        field_errors => [ 'Please enter a message', 'Please enter a valid email' ]
+        field_errors => [ 'Please enter a message', 'Please enter a valid email', 'Please enter your name' ]
     },
     {
         desc => 'email with spaces, no message',
@@ -229,11 +235,14 @@ for my $test (
             fixed  => undef,
             add_alert => 1,
             may_show_name => undef,
+            remember_me => undef,
+            password_register => '',
+            password_sign_in => '',
         },
         changes => {
             rznvy => 'test@example.com',
         },
-        field_errors => [ 'Please enter a message' ]
+        field_errors => [ 'Please enter a message', 'Please enter your name' ]
     },
     {
         desc => 'email with uppercase, no message',
@@ -245,11 +254,14 @@ for my $test (
             fixed  => undef,
             add_alert => 1,
             may_show_name => undef,
+            remember_me => undef,
+            password_register => '',
+            password_sign_in => '',
         },
         changes => {
             rznvy => 'test@example.com',
         },
-        field_errors => [ 'Please enter a message' ]
+        field_errors => [ 'Please enter a message', 'Please enter your name' ]
     },
   )
 {
@@ -281,12 +293,17 @@ for my $test (
             photo         => '',
             update        => '',
             fixed         => undef,
+            remember_me => undef,
+            password_register => '',
+            password_sign_in => '',
         },
         form_values => {
             submit_update => 1,
             rznvy         => 'unregistered@example.com',
             update        => 'Update from an unregistered user',
             add_alert     => undef,
+            name          => 'Unreg User',
+            may_show_name => undef,
         },
         changes => {},
     },
@@ -300,12 +317,17 @@ for my $test (
             photo         => '',
             update        => '',
             fixed         => undef,
+            remember_me => undef,
+            password_register => '',
+            password_sign_in => '',
         },
         form_values => {
             submit_update => 1,
             rznvy         => 'unregistered@example.com',
             update        => 'update from an unregistered user',
             add_alert     => 1,
+            name          => 'Unreg User',
+            may_show_name => undef,
         },
         changes => {
             update => 'Update from an unregistered user',
@@ -384,32 +406,82 @@ for my $test (
 
 for my $test (
     {
-        desc => 'submit update for registered user',
-        initial_values => {
-            name => 'Test User',
-            may_show_name => 1,
-            add_alert => 1,
-            photo => '',
-            update => '',
-            fixed => undef,
-        },
-        email  => 'test@example.com',
-        fields => {
+        desc => 'submit an update for a non registered user, signing in with wrong password',
+        form_values => {
             submit_update => 1,
-            update => 'update from a registered user',
-            add_alert => undef,
-            fixed => undef,
-        },
-        changed => {
-            update => 'Update from a registered user'
+            rznvy         => 'registered@example.com',
+            update        => 'Update from a user',
+            add_alert     => undef,
+            password_sign_in => 'secret',
         },
-        initial_banner => '',
-        endstate_banner => '',
-        alert => 0,
-        anonymous => 0,
+        field_errors => [
+            'There was a problem with your email/password combination. Please try again.',
+            'Please enter your name', # FIXME Not really necessary error
+        ],
     },
     {
-        desc => 'submit update for registered user anonymously by unchecking',
+        desc => 'submit an update for a non registered user and sign in',
+        form_values => {
+            submit_update => 1,
+            rznvy         => 'registered@example.com',
+            update        => 'Update from a user',
+            add_alert     => undef,
+            password_sign_in => 'secret2',
+        },
+        field_errors => [
+            'You have successfully signed in; please check and confirm your details are accurate:',
+        ],
+    }
+) {
+    subtest $test->{desc} => sub {
+        # Set things up
+        my $user = $mech->create_user_ok( $test->{form_values}->{rznvy} );
+        my $pw = 'secret2';
+        $user->update( { name => 'Mr Reg', password => $pw } );
+        $report->comments->delete;
+
+        $mech->log_out_ok();
+        $mech->clear_emails_ok();
+        $mech->get_ok("/report/$report_id");
+        $mech->submit_form_ok(
+            {
+                button => 'submit_sign_in',
+                with_fields => $test->{form_values}
+            },
+            'submit update'
+        );
+
+        is_deeply $mech->form_errors, $test->{field_errors}, 'check there were errors';
+
+        SKIP: {
+            skip( "Incorrect password", 5 ) unless $test->{form_values}{password_sign_in} eq $pw;
+
+            # Now submit with a name
+            $mech->submit_form_ok(
+                {
+                    with_fields => {
+                        name => 'Joe Bloggs',
+                    }
+                },
+                "submit good details"
+            );
+
+            is $mech->uri->path, "/report/" . $report_id, "redirected to report page";
+            $mech->email_count_is(0);
+
+            my $update = $report->comments->first;
+            ok $update, 'found update';
+            is $update->text, $test->{form_values}->{update}, 'update text';
+            is $update->user->email, $test->{form_values}->{rznvy}, 'update user';
+            is $update->state, 'confirmed', 'update confirmed';
+            $mech->delete_user( $update->user );
+        }
+    };
+}
+
+for my $test (
+    {
+        desc => 'submit update for registered user',
         initial_values => {
             name => 'Test User',
             may_show_name => 1,
@@ -422,7 +494,6 @@ for my $test (
         fields => {
             submit_update => 1,
             update => 'update from a registered user',
-            may_show_name => undef,
             add_alert => undef,
             fixed => undef,
         },
@@ -432,10 +503,10 @@ for my $test (
         initial_banner => '',
         endstate_banner => '',
         alert => 0,
-        anonymous => 1,
+        anonymous => 0,
     },
     {
-        desc => 'submit update for registered user anonymously by deleting name',
+        desc => 'submit update for registered user anonymously by unchecking',
         initial_values => {
             name => 'Test User',
             may_show_name => 1,
@@ -447,9 +518,8 @@ for my $test (
         email  => 'test@example.com',
         fields => {
             submit_update => 1,
-            name => '',
             update => 'update from a registered user',
-            may_show_name => 1,
+            may_show_name => undef,
             add_alert => undef,
             fixed => undef,
         },
diff --git a/templates/web/default/report/display.html b/templates/web/default/report/display.html
index 3c9768657..8f819a431 100644
--- a/templates/web/default/report/display.html
+++ b/templates/web/default/report/display.html
@@ -93,12 +93,12 @@
         [% END %]
 
         [% IF c.cobrand.allow_photo_upload %]
-        [% IF photo_error %]
-            <div class='form-error'>[% photo_error %]</div>
+        [% IF field_errors.photo %]
+            <div class='form-error'>[% field_errors.photo %]</div>
         [% END %]
         <div id="fileupload_normalUI">
         [% IF upload_fileid %]
-            <p>[% loc('You have already attached a photo to this report, attaching another one will replace it.') %]</p>
+            <p>[% loc('You have already attached a photo to this update, attaching another one will replace it.') %]</p>
             <input type="hidden" name="upload_fileid" value="[% upload_fileid %]">
         [% END %]
         <label for="form_photo">[% loc('Photo:') %]</label>
@@ -106,38 +106,76 @@
         </div>
         [% END %]
 
-        <div>
-            <label for="form_name">[% loc( 'Name:') %]</label>
-            <input type="text" name="name" id="form_name" value="[% update.name || c.user.name | html %]" size="20">
-        </div>
+[% IF c.user_exists %]
+
+    [% INCLUDE name %]
 
         <div class="checkbox">
-                <input type="checkbox" name="may_show_name" id="form_may_show_name" value="1"[% ' checked' UNLESS update.anonymous %]>
-                <label for="form_may_show_name">[% loc('Show my name publicly') %]</label>
-                <small>[% loc('(we never show your email)') %]</small>
+            <input type="submit" id="update_post" value="[% loc('Post') %]">
         </div>
 
-[% 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_rznvy">[% loc('Your email:' ) %]</label>
+        <input type="email" name="rznvy" id="form_rznvy" value="[% update.user.email | html %]" size="30">
+    </div>
+
+<div id="form_sign_in">
+    <h3>[% loc("Now to submit your update&hellip; do you have a FixMyStreet password?") %]</h3>
 
-        [% IF field_errors.email %]
-            <div class='form-error'>[% field_errors.email %]</div>
+    <div id="form_sign_in_yes">
+
+        [% IF field_errors.password %]
+            <div class='form-error'>[% field_errors.password %]</div>
         [% END %]
-                <div class="form-field">
-            <label for="form_rznvy">[% loc('Email' ) %]</label>
-            <input type="text" name="rznvy" id="form_rznvy" value="[% update.user.email || c.user.email | html %]" size="20">
-        </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>
 
-        <div class="checkbox">
-            <input type="checkbox" name="add_alert" id="form_add_alert" value="1"[% ' checked' IF add_alert %]>
-            <label for="form_add_alert">[% loc( 'Alert me to future updates' ) %]</label>
-        </div>
+        <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>
 
-        <div class="checkbox">
-            <input type="submit" id="update_post" value="[% loc('Post') %]">
+        <p>
+        <input type="submit" name="submit_sign_in" value="[% loc('Post') %]">
+        </p>
+
+    </div>
+    <div id="form_sign_in_no">
+
+        <p>[% loc('<strong>No</strong>, let me confirm my update by email:') %]</p>
+
+        <div id="fieldset">
+
+            [% INCLUDE name %]
+
+            <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('Post') %]">
+        </p>
+
+    </div>
+
+</div>
+
+[% END %]
+
         [% cobrand_update_fields %]
 
     </form>
@@ -146,3 +184,25 @@
 </div>
 
 [% INCLUDE 'footer.html' %]
+
+[% BLOCK name %]
+    [% IF field_errors.name %]
+        <div class='form-error'>[% field_errors.name %]</div>
+    [% END %]
+
+    <div>
+        <label for="form_name">[% loc('Your name:') %]</label>
+        <input type="text" name="name" id="form_name" value="[% update.name || c.user.name | html %]" size="25">
+    </div>
+
+    <div class="checkbox">
+        <input type="checkbox" name="may_show_name" id="form_may_show_name" value="1"[% ' checked' UNLESS update.anonymous %]>
+        <label for="form_may_show_name">[% loc('Show my name publicly') %]</label>
+        <small>[% loc('(we never show your email)') %]</small>
+    </div>
+
+    <div class="checkbox">
+        <input type="checkbox" name="add_alert" id="form_add_alert" value="1"[% ' checked' IF add_alert %]>
+        <label for="form_add_alert">[% loc( 'Alert me to future updates' ) %]</label>
+    </div>
+[% END %]