Check provided report ID for update alert signup.

Remove some unused code.

3 files changed, 19 insertions, 35 deletions

diff --git a/perllib/FixMyStreet/App/Controller/Alert.pm b/perllib/FixMyStreet/App/Controller/Alert.pm
index 755602562..f93231235 100644
--- a/perllib/FixMyStreet/App/Controller/Alert.pm
+++ b/perllib/FixMyStreet/App/Controller/Alert.pm
@@ -58,12 +58,15 @@ sub subscribe : Path('subscribe') : Args(0) {
 
     $c->detach('rss') if $c->get_param('rss');
 
+    my $id = $c->get_param('id');
+    $c->forward('/report/load_problem_or_display_error', [ $id ]) if $id;
+
     # if it exists then it's been submitted so we should
     # go to subscribe email and let it work out the next step
     $c->detach('subscribe_email')
         if $c->get_param('rznvy') || $c->get_param('alert');
 
-    $c->go('updates') if $c->get_param('id');
+    $c->go('updates') if $id;
 
     # shouldn't get to here but if we have then do something sensible
     $c->go('index');
@@ -211,13 +214,10 @@ Set up the options in the stash required to create a problem update alert
 sub set_update_alert_options : Private {
     my ( $self, $c ) = @_;
 
-    my $report_id = $c->get_param('id');
-    return unless $report_id =~ /^[1-9]\d*$/;
-
     my $options = {
         user       => $c->stash->{alert_user},
         alert_type => 'new_updates',
-        parameter  => $report_id,
+        parameter  => $c->stash->{problem}->id,
     };
 
     $c->stash->{alert_options} = $options;
@@ -344,12 +344,8 @@ sub process_user : Private {
         return;
     }
 
-    # Extract all the params to a hash to make them easier to work with
-    my %params = map { $_ => $c->get_param($_) }
-      ( 'rznvy' ); # , 'password_register' );
-
-    # cleanup the email address
-    my $email = $params{rznvy} ? lc $params{rznvy} : '';
+    my $email = $c->get_param('rznvy');
+    $email = $email ? lc $email : '';
     $email =~ s{\s+}{}g;
 
     push @{ $c->stash->{errors} }, _('Please enter a valid email address')
@@ -357,19 +353,6 @@ sub process_user : Private {
 
     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->get_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( $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 ed2bc814c..132090634 100644
--- a/t/app/controller/alert_new.t
+++ b/t/app/controller/alert_new.t
@@ -6,6 +6,9 @@ my $mech = FixMyStreet::TestMech->new;
 my $user = FixMyStreet::App->model('DB::User')
           ->new( { email => 'test@example.com' } );
 
+my $body = $mech->create_body_ok(2651, 'Edinburgh Council');
+my ($report) = $mech->create_problems_for_body(1, $body->id, 'Existing');
+
 foreach my $test (
     {
         email      => $user->email,
@@ -51,8 +54,8 @@ foreach my $test (
         type       => 'new_updates',
         content    => 'Click the link in our confirmation email to activate your alert',
         email_text => "confirms that you'd like to receive an email",
-        uri    => '/alert/subscribe?type=updates&rznvy=' . $user->email . '&id=1',
-        param1 => 1,
+        uri    => '/alert/subscribe?type=updates&rznvy=' . $user->email . '&id=' . $report->id,
+        param1 => $report->id,
     }
   )
 {
@@ -61,7 +64,7 @@ foreach my $test (
 
         my $type = $test->{type};
 
-        $mech->get_ok('/alert/subscribe?id=1');
+        $mech->get_ok('/alert/subscribe?id=' . $report->id);
         my ($csrf) = $mech->content =~ /name="token" value="([^"]*)"/;
 
         $mech->get_ok( $test->{uri} . "&token=$csrf" );
@@ -155,7 +158,7 @@ foreach my $test (
         # clear existing data so we can be sure we're creating it
         ok $alert->delete() if $alert && !$test->{exist};
 
-        $mech->get_ok('/alert/subscribe?id=1');
+        $mech->get_ok('/alert/subscribe?id=' . $report->id);
         my ($csrf) = $mech->content =~ /name="token" value="([^"]*)"/;
 
         $mech->get_ok( '/alert/subscribe?type=local&rznvy=' . $user->email . '&feed=area:1000:A_Location&token=' . $csrf );
@@ -179,8 +182,6 @@ foreach my $test (
     };
 }
 
-my $body = $mech->create_body_ok(2651, 'Edinburgh Council');
-
 foreach my $test (
     {
         desc       => 'logged in user signing up',
@@ -231,8 +232,8 @@ for my $test (
         type       => 'new_updates',
         content    => 'Click the link in our confirmation email to activate your alert',
         email_text => 'confirm the alert',
-        uri    => '/alert/subscribe?type=updates&rznvy=' . $user->email . '&id=1',
-        param1 => 1,
+        uri    => '/alert/subscribe?type=updates&rznvy=' . $user->email . '&id=' . $report->id,
+        param1 => $report->id,
     }
   )
 {
@@ -255,7 +256,7 @@ for my $test (
           FixMyStreet::DB->resultset('Abuse')
           ->find_or_create( { email => $test->{email} } );
 
-        $mech->get_ok('/alert/subscribe?id=1');
+        $mech->get_ok('/alert/subscribe?id=' . $report->id);
         my ($csrf) = $mech->content =~ /name="token" value="([^"]*)"/;
 
         $mech->get_ok( $test->{uri} . "&token=$csrf" );
diff --git a/templates/web/base/alert/updates.html b/templates/web/base/alert/updates.html
index a77f8d0f9..96d1c467d 100644
--- a/templates/web/base/alert/updates.html
+++ b/templates/web/base/alert/updates.html
@@ -5,7 +5,7 @@
 
 [% INCLUDE 'errors.html' %]
 
-<p><a href="[% c.uri_for( '/rss', problem_id ) %]">
+<p><a href="[% c.uri_for( '/rss', problem.id ) %]">
         <img src="/i/feed.png" width="16" height="16" title="[% loc('RSS feed') %]" alt="[% loc('RSS feed of updates to this problem' ) %]" border="0" style="float:right">
     </a>
 </p>
@@ -24,7 +24,7 @@
         </div>
 
         <input type="hidden" name="token" value="[% csrf_token %]">
-        <input type="hidden" name="id" value="[% problem_id | html %]">
+        <input type="hidden" name="id" value="[% problem.id | html %]">
         <input type="hidden" name="type" value="updates">
     </fieldset>
 </form>