If logged in, don't show email on alert sign up.

3 files changed, 73 insertions, 79 deletions

diff --git a/perllib/FixMyStreet/App/Controller/Alert.pm b/perllib/FixMyStreet/App/Controller/Alert.pm
index d1759a330..ff92a7d2d 100644
--- a/perllib/FixMyStreet/App/Controller/Alert.pm
+++ b/perllib/FixMyStreet/App/Controller/Alert.pm
@@ -55,17 +55,14 @@ Target for subscribe form
 sub subscribe : Path('subscribe') : Args(0) {
     my ( $self, $c ) = @_;
 
-    if ( $c->req->param('rss') ) {
-        $c->detach('rss');
-    }
+    $c->detach('rss') if $c->req->param('rss');
+
     # if it exists then it's been submitted so we should
     # go to subscribe email and let it work out the next step
-    elsif ( exists $c->req->params->{'rznvy'} ) {
-        $c->detach('subscribe_email');
-    }
-    elsif ( $c->req->params->{'id'} ) {
-        $c->go('updates');
-    }
+    $c->detach('subscribe_email')
+        if exists $c->req->params->{'rznvy'} || $c->req->params->{'alert'};
+
+    $c->go('updates') if $c->req->params->{'id'};
 
     # shouldn't get to here but if we have then do something sensible
     $c->go('index');
@@ -116,25 +113,18 @@ Sign up to email alerts
 sub subscribe_email : Private {
     my ( $self, $c ) = @_;
 
-    my $type = $c->req->param('type');
-    $c->stash->{email_type} = 'alert';
+    $c->stash->{errors} = [];
+    $c->forward('process_user');
 
-    my @errors;
-    push @errors, _('Please enter a valid email address')
-      unless is_valid_email( $c->req->param('rznvy') );
-    push @errors, _('Please select the type of alert you want')
+    my $type = $c->req->param('type');
+    push @{ $c->stash->{errors} }, _('Please select the type of alert you want')
       if $type && $type eq 'local' && !$c->req->param('feed');
-    if (@errors) {
-        $c->stash->{errors} = \@errors;
+    if (@{ $c->stash->{errors} }) {
         $c->go('updates') if $type && $type eq 'updates';
         $c->go('list') if $type && $type eq 'local';
         $c->go('index');
     }
 
-    my $email = $c->req->param('rznvy');
-    $c->stash->{email} = $email;
-    $c->forward('process_user');
-
     if ( $type eq 'updates' ) {
         $c->forward('set_update_alert_options');
     }
@@ -200,14 +190,12 @@ sub create_alert : Private {
         $options->{cobrand_data} = $c->cobrand->extra_update_data();
         $options->{lang}         = $c->stash->{lang_code};
 
-        if ( $c->user && $c->stash->{alert_user}->in_storage && $c->user->id == $c->stash->{alert_user}->id ) {
-            $options->{confirmed} = 1;
-        }
-
         $alert = $c->model('DB::Alert')->new($options);
         $alert->insert();
     }
 
+    $alert->confirm() if $c->user && $c->user->id == $alert->user->id;
+
     $c->stash->{alert} = $alert;
 }
 
@@ -303,6 +291,7 @@ sub send_confirmation_email : Private {
 
     $c->send_email( 'alert-confirm.txt', { to => $c->stash->{alert}->user->email } );
 
+    $c->stash->{email_type} = 'alert';
     $c->stash->{template} = 'email_sent.html';
 }
 
@@ -331,12 +320,46 @@ sub prettify_pc : Private {
     return 1;
 }
 
+=head2 process_user
+
+Fetch/check email address
+
+=cut
+
 sub process_user : Private {
     my ( $self, $c ) = @_;
 
-    my $email = $c->stash->{email};
+    if ( $c->user_exists ) {
+        $c->stash->{alert_user} = $c->user->obj;
+        return;
+    }
+
+    # Extract all the params to a hash to make them easier to work with
+    my %params = map { $_ => scalar $c->req->param($_) }
+      ( 'rznvy' ); # , 'password_register' );
+
+    # cleanup the email address
+    my $email = $params{rznvy} ? lc $params{rznvy} : '';
+    $email =~ s{\s+}{}g;
+
+    push @{ $c->stash->{errors} }, _('Please enter a valid email address')
+      unless is_valid_email( $email );
+
     my $alert_user = $c->model('DB::User')->find_or_new( { email => $email } );
     $c->stash->{alert_user} = $alert_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;
+#        $c->stash->{alert_user} = $user;
+#        return 1;
+#    }
+#
+#    $alert_user->password( Utils::trim_text( $params{password_register} ) );
 }
 
 =head2 setup_coordinate_rss_feeds
diff --git a/t/app/controller/alert_new.t b/t/app/controller/alert_new.t
index d0976b4bb..cb3b4b656 100644
--- a/t/app/controller/alert_new.t
+++ b/t/app/controller/alert_new.t
@@ -193,22 +193,11 @@ foreach my $test (
 foreach my $test (
     {
         desc       => 'logged in user signing up',
-        user       => 'test-sign-in@example.com',
         email      => 'test-sign-in@example.com',
         type       => 'council',
         param1     => 2651,
         param2     => 2651,
         confirmed  => 1,
-    },
-    {
-        desc       => 'logged in user signing up with different email',
-        user       => 'loggedin@example.com',
-        email      => 'test-sign-in@example.com',
-        type       => 'council',
-        param1     => 2651,
-        param2     => 2651,
-        confirmed  => 0,
-        delete     => 1,
     }
   )
 {
@@ -217,42 +206,18 @@ foreach my $test (
 
         my $user =
           FixMyStreet::App->model('DB::User')
-          ->find_or_create( { email => $test->{user} } );
-
-        my $alert_user =
-          FixMyStreet::App->model('DB::User')
-          ->find( { email => $test->{email} } );
-
-        $mech->log_in_ok( $test->{user} );
+          ->find_or_create( { email => $test->{email} } );
 
+        $mech->log_in_ok( $test->{email} );
         $mech->clear_emails_ok;
 
-        my $alert;
-        if ($alert_user) {
-            $alert = FixMyStreet::App->model('DB::Alert')->find(
-                {
-                    user       => $alert_user,
-                    alert_type => $type
-                }
-            );
-
-            # clear existing data so we can be sure we're creating it
-            $alert->delete() if $alert;
-        }
-
         $mech->get_ok('/alert/list?pc=EH991SP');
-
-        my $form_values = $mech->visible_form_values();
-        ok $form_values->{rznvy} eq $test->{user},
-          'auto filled in correct email';
-
-        $mech->set_visible( [ radio => 'council:2651:City_of_Edinburgh' ],
-            [ text => $test->{email} ] );
+        $mech->set_visible( [ radio => 'council:2651:City_of_Edinburgh' ] );
         $mech->click('alert');
 
-        $alert = FixMyStreet::App->model('DB::Alert')->find(
+        my $alert = FixMyStreet::App->model('DB::Alert')->find(
             {
-                user       => $alert_user,
+                user       => $user,
                 alert_type => $type,
                 parameter  => $test->{param1},
                 parameter2 => $test->{param2},
@@ -261,13 +226,8 @@ foreach my $test (
         );
 
         ok $alert, 'New alert created with logged in user';
-
-        $mech->email_count_is( $test->{confirmed} ? 0 : 1 );
-
-        if ( $test->{delete} ) {
-            $mech->delete_user($user);
-            $mech->delete_user($alert_user);
-        }
+        $mech->email_count_is( 0 );
+        $mech->delete_user($user);
     };
 }
 
@@ -391,17 +351,24 @@ subtest "Test normal alert signups and that alerts are sent" => sub {
     $user2->alerts->delete;
 
     for my $alert (
-        { feed => 'local:55.951963:-3.189944', email_confirm => 1 },
-        { feed => 'council:2651:City_of_Edinburgh', },
+        {
+            fields => {
+                feed => 'local:55.951963:-3.189944',
+                rznvy => $user2->email,
+            },
+            email_confirm => 1
+        },
+        {
+            fields => {
+                feed => 'council:2651:City_of_Edinburgh',
+            }
+        },
     ) {
         $mech->get_ok( '/alert' );
         $mech->submit_form_ok( { with_fields => { pc => 'EH11BB' } } );
         $mech->submit_form_ok( {
             button => 'alert',
-            with_fields => {
-                rznvy => $user2->email,
-                feed => $alert->{feed},
-            }
+            with_fields => $alert->{fields},
         } );
         if ( $alert->{email_confirm} ) {
             my $email = $mech->get_email;
diff --git a/templates/web/default/alert/list.html b/templates/web/default/alert/list.html
index 38007ea62..36bfaffd9 100644
--- a/templates/web/default/alert/list.html
+++ b/templates/web/default/alert/list.html
@@ -116,10 +116,14 @@ for the county council.' ) %]
     [% loc('or') %]
   </p>
 
+[% UNLESS c.user_exists %]
+
   <p>
   [% loc('Your email:') %] <input type="text" id="rznvy" name="rznvy" value="[% rznvy | html %]" size="30">
   </p>
 
+[% END %]
+
   <p>
     <input type="submit" name="alert" value="[% loc('Subscribe me to an email alert') %]">
   </p>