Merge branch 'master' of ssh://git.mysociety.org/data/git/public/fixmystreet into new_statuses

Conflicts:
	templates/web/default/report/display.html

4 files changed, 61 insertions, 83 deletions

diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm
index f732b94f1..a15ee993f 100644
--- a/perllib/FixMyStreet/App/Controller/Report.pm
+++ b/perllib/FixMyStreet/App/Controller/Report.pm
@@ -60,9 +60,9 @@ sub load_problem_or_display_error : Private {
     my ( $self, $c, $id ) = @_;
 
     # try to load a report if the id is a number
-    my $problem    #
-      = $id =~ m{\D}    # is id non-numeric?
-      ? undef           # ...don't even search
+    my $problem
+      = ( !$id || $id =~ m{\D} ) # is id non-numeric?
+      ? undef                    # ...don't even search
       : $c->cobrand->problems->find( { id => $id } );
 
     # check that the problem is suitable to show.
@@ -110,23 +110,8 @@ sub format_problem_for_display : Private {
 
     $c->stash->{report_name} = $c->req->param('name');
 
-    if ( $c->req->param('submit_update') ) {
-        # we may have munged these previously in /report/update 
-        # so only set if they're not already in the stash
-        $c->stash->{form_name}   ||= $c->req->param('name');
-        $c->stash->{update_text} ||= $c->req->param('update');
-        $c->stash->{email}       ||= $c->req->param('rznvy');
-        $c->stash->{fixed}       ||= $c->req->param('fixed') ? ' checked' : '';
-        $c->stash->{add_alert_checked} ||=
-          ( $c->req->param('add_alert') ? ' checked' : '' );
-    }
-    else {
-        if ( $c->user ) {
-            $c->stash->{form_name}     = $c->user->name;
-            $c->stash->{email}         = $c->user->email;
-            $c->stash->{may_show_name} = ' checked' if $c->user->name;
-        }
-        $c->stash->{add_alert_checked} = ' checked';
+    unless ( $c->req->param('submit_update') ) {
+        $c->stash->{add_alert} = 1;
     }
 
     $c->forward('generate_map_tags');
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm
index dfa45df55..130eee858 100644
--- a/perllib/FixMyStreet/App/Controller/Report/New.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/New.pm
@@ -93,10 +93,10 @@ sub report_new : Path : Args(0) {
 
     # deal with the user and report and check both are happy
     return
-      unless $c->forward('process_user')
+      unless $c->forward('check_form_submitted')
+          && $c->forward('process_user')
           && $c->forward('process_report')
           && $c->forward('process_photo')
-          && $c->forward('check_form_submitted')
           && $c->forward('check_for_errors')
           && $c->forward('save_user_and_report')
           && $c->forward('redirect_or_confirm_creation');
@@ -325,6 +325,9 @@ sub initialize_report : Private {
 
     }
 
+    # Capture whether the map was used
+    $report->used_map( $c->req->param('skipped') ? 0 : 1 );
+
     $c->stash->{report} = $report;
 
     return 1;
@@ -547,8 +550,6 @@ Load user from the database or prepare a new one.
 sub process_user : Private {
     my ( $self, $c ) = @_;
 
-    # FIXME - If user already logged in use them regardless
-
     # Extract all the params to a hash to make them easier to work with
     my %params =    #
       map { $_ => scalar $c->req->param($_) }    #
@@ -559,15 +560,12 @@ sub process_user : Private {
     $email =~ s{\s+}{}g;
 
     my $report = $c->stash->{report};
-    my $report_user                              #
-      = ( $report ? $report->user : undef )
-      || $c->model('DB::User')->find_or_new( { email => $email } );
+    $report->user( $c->model('DB::User')->find_or_new( { email => $email } ) )
+        unless $report->user;
 
     # set the user's name and phone (if given)
-    $report_user->name( Utils::trim_text( $params{name} ) );
-    $report_user->phone( Utils::trim_text( $params{phone} ) ) if $params{phone};
-
-    $c->stash->{report_user} = $report_user;
+    $report->user->name( Utils::trim_text( $params{name} ) );
+    $report->user->phone( Utils::trim_text( $params{phone} ) );
 
     return 1;
 }
@@ -590,7 +588,7 @@ sub process_report : Private {
         'title', 'detail', 'pc',                 #
         'name', 'may_show_name',                 #
         'category',                              #
-        'partial', 'skipped', 'submit_problem'   #
+        'partial',                               #
       );
 
     # load the report
@@ -601,12 +599,6 @@ sub process_report : Private {
     $report->latitude( $c->stash->{latitude} );
     $report->longitude( $c->stash->{longitude} );
 
-    # Capture whether the map was used
-    $report->used_map( $params{skipped} ? 0 : 1 );
-
-    # Short circuit unless the form has been submitted
-    return 1 unless $params{submit_problem};
-
     # set some simple bool values (note they get inverted)
     $report->anonymous( $params{may_show_name} ? 0 : 1 );
 
@@ -794,7 +786,7 @@ sub check_for_errors : Private {
 
     # let the model check for errors
     my %field_errors = (
-        %{ $c->stash->{report_user}->check_for_errors },
+        %{ $c->stash->{report}->user->check_for_errors },
         %{ $c->stash->{report}->check_for_errors },
     );
 
@@ -822,27 +814,23 @@ before or they are currently logged in. Otherwise discard any changes.
 
 sub save_user_and_report : Private {
     my ( $self, $c ) = @_;
-    my $report_user = $c->stash->{report_user};
     my $report      = $c->stash->{report};
 
     # Save or update the user if appropriate
-    if ( !$report_user->in_storage ) {
-        $report_user->insert();
+    if ( !$report->user->in_storage ) {
+        $report->user->insert();
     }
-    elsif ( $c->user && $report_user->id == $c->user->id ) {
-        $report_user->update();
+    elsif ( $c->user && $report->user->id == $c->user->id ) {
+        $report->user->update();
         $report->confirm;
     }
     else {
 
         # user exists and we are not logged in as them. Throw away changes to
         # the name and phone. TODO - propagate changes using tokens.
-        $report_user->discard_changes();
+        $report->user->discard_changes();
     }
 
-    # add the user to the report
-    $report->user($report_user);
-
     # If there was a photo add that too
     if ( my $fileid = $c->stash->{upload_fileid} ) {
         my $file = file( $c->config->{UPLOAD_CACHE}, "$fileid.jpg" );
diff --git a/perllib/FixMyStreet/App/Controller/Report/Update.pm b/perllib/FixMyStreet/App/Controller/Report/Update.pm
index 02d111d9f..354690949 100644
--- a/perllib/FixMyStreet/App/Controller/Report/Update.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/Update.pm
@@ -20,10 +20,6 @@ Creates an update to a report
 sub report_update : Path : Args(0) {
     my ( $self, $c ) = @_;
 
-    # if there's no id then we should just stop now
-    $c->detach( '/page_error_404_not_found', [ _('Unknown problem ID') ] )
-      unless $c->req->param('id');
-
          $c->forward( '/report/load_problem_or_display_error', [ $c->req->param('id') ] )
       && $c->forward('process_user')
       && $c->forward('process_update')
@@ -81,14 +77,6 @@ sub update_problem : Private {
     return 1;
 }
 
-sub display_confirmation : Private {
-    my ( $self, $c ) = @_;
-
-    $c->stash->{template} = 'tokens/confirm_update.html';
-
-    return 1;
-}
-
 =head2 process_user
 
 Load user from the database or prepare a new one.
@@ -98,25 +86,28 @@ Load user from the database or prepare a new one.
 sub process_user : Private {
     my ( $self, $c ) = @_;
 
-    # FIXME - If user already logged in use them regardless
+    my $update_user;
+    if ( $c->user ) {
+
+        $update_user = $c->user->obj;
+
+    } else {
 
-    # Extract all the params to a hash to make them easier to work with
-    my %params =    #
-      map { $_ => scalar $c->req->param($_) }    #
-      ( 'rznvy', 'name' );
+        # Extract all the params to a hash to make them easier to work with
+        my %params =    #
+          map { $_ => scalar $c->req->param($_) }    #
+          ( 'rznvy', 'name' );
 
-    # cleanup the email address
-    my $email = $params{rznvy} ? lc $params{rznvy} : '';
-    $email =~ s{\s+}{}g;
+        # cleanup the email address
+        my $email = $params{rznvy} ? lc $params{rznvy} : '';
+        $email =~ s{\s+}{}g;
 
-    my $update_user = $c->model('DB::User')->find_or_new( { email => $email } );
+        $update_user = $c->model('DB::User')->find_or_new( { email => $email } );
+        $update_user->name( Utils::trim_text( $params{name} ) );
 
-    # set the user's name if they don't have one
-    $update_user->name( Utils::trim_text( $params{name} ) )
-      unless $update_user->name;
+    }
 
     $c->stash->{update_user} = $update_user;
-    $c->stash->{email}       = $update_user->email;
 
     return 1;
 }
@@ -162,9 +153,7 @@ sub process_update : Private {
     );
 
     $c->stash->{update}        = $update;
-    $c->stash->{update_text}   = $update->text;
     $c->stash->{add_alert}     = $c->req->param('add_alert');
-    $c->stash->{may_show_name} = ' checked' if $c->req->param('may_show_name');
 
     return 1;
 }
@@ -191,7 +180,7 @@ sub check_for_errors : Private {
 
     # let the model check for errors
     my %field_errors = (
-        %{ $c->stash->{update_user}->check_for_errors },
+        %{ $c->stash->{update}->user->check_for_errors },
         %{ $c->stash->{update}->check_for_errors },
     );
 
@@ -222,14 +211,14 @@ Save the update and the user as appropriate.
 sub save_update : Private {
     my ( $self, $c ) = @_;
 
-    my $user   = $c->stash->{update_user};
     my $update = $c->stash->{update};
 
-    if ( !$user->in_storage ) {
-        $user->insert;
+    if ( !$update->user->in_storage ) {
+        $update->user->insert;
     }
-    elsif ( $c->user && $c->user->id == $user->id ) {
-        $user->update;
+    elsif ( $c->user && $c->user->id == $update->user->id ) {
+        # Logged in and same user, so can confirm update straight away
+        $update->user->update;
         $update->confirm;
     }
 
@@ -264,8 +253,8 @@ sub redirect_or_confirm_creation : Private {
 
     # If confirmed send the user straight there.
     if ( $update->confirmed ) {
-        $c->forward( 'signup_for_alerts' );
         $c->forward( 'update_problem' );
+        $c->forward( 'signup_for_alerts' );
         my $report_uri = $c->uri_for( '/report', $update->problem_id );
         $c->res->redirect($report_uri);
         $c->detach;
diff --git a/perllib/FixMyStreet/DB/Result/User.pm b/perllib/FixMyStreet/DB/Result/User.pm
index cdac54280..dfcb18c5b 100644
--- a/perllib/FixMyStreet/DB/Result/User.pm
+++ b/perllib/FixMyStreet/DB/Result/User.pm
@@ -74,7 +74,7 @@ sub check_for_errors {
 
     my %errors = ();
 
-    if ( $self->name !~ m/\S/ ) {
+    if ( !$self->name || $self->name !~ m/\S/ ) {
         $errors{name} = _('Please enter your name');
     }
     elsif (length( $self->name ) < 5
@@ -117,4 +117,20 @@ sub answered_ever_reported {
     return $has_answered->count > 0;
 }
 
+=head2 alert_for_problem
+
+Returns whether the user is already subscribed to an
+alert for the problem ID provided.
+
+=cut
+
+sub alert_for_problem {
+    my ( $self, $id ) = @_;
+
+    return $self->alerts->find( {
+        alert_type => 'new_updates',
+        parameter  => $id,
+    } );
+}
+
 1;