From 07ed5ea201da14f07da48ea940ef839a4f166da8 Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Wed, 22 Jun 2011 16:59:59 +0100 Subject: Store user in report from the start, so name of logged in person isn't blanked out, and don't process unnecessarily. --- perllib/FixMyStreet/App/Controller/Report/New.pm | 44 +++++++++--------------- 1 file changed, 16 insertions(+), 28 deletions(-) (limited to 'perllib/FixMyStreet/App/Controller') 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" ); -- cgit v1.2.3 From 81c542caf4819268bb6b3cb5c1a445f8e5a10b04 Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Thu, 23 Jun 2011 12:23:46 +0100 Subject: Use update user centrally. --- perllib/FixMyStreet/App/Controller/Report.pm | 6 +++--- perllib/FixMyStreet/App/Controller/Report/Update.pm | 15 +++++---------- 2 files changed, 8 insertions(+), 13 deletions(-) (limited to 'perllib/FixMyStreet/App/Controller') diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm index f732b94f1..6c14f33fb 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. diff --git a/perllib/FixMyStreet/App/Controller/Report/Update.pm b/perllib/FixMyStreet/App/Controller/Report/Update.pm index 2f1d88d08..4e0d6d6c5 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') @@ -182,7 +178,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 }, ); @@ -213,14 +209,13 @@ 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 ) { + $update->user->update; $update->confirm; } -- cgit v1.2.3 From 98e23f576f1114d42f4b847e8bb57d090b19d633 Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Thu, 23 Jun 2011 12:26:18 +0100 Subject: Default to show name. --- perllib/FixMyStreet/App/Controller/Report.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'perllib/FixMyStreet/App/Controller') diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm index 6c14f33fb..b50325052 100644 --- a/perllib/FixMyStreet/App/Controller/Report.pm +++ b/perllib/FixMyStreet/App/Controller/Report.pm @@ -124,8 +124,8 @@ sub format_problem_for_display : Private { 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->{may_show_name} = ' checked'; $c->stash->{add_alert_checked} = ' checked'; } -- cgit v1.2.3 From ead78ff31d49cfeff2503f9faf793e0e53110cf6 Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Thu, 23 Jun 2011 13:33:02 +0100 Subject: Remove unneeded defaults checking and simplify. --- perllib/FixMyStreet/App/Controller/Report.pm | 19 ++----------------- perllib/FixMyStreet/App/Controller/Report/Update.pm | 18 +++--------------- 2 files changed, 5 insertions(+), 32 deletions(-) (limited to 'perllib/FixMyStreet/App/Controller') diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm index b50325052..a15ee993f 100644 --- a/perllib/FixMyStreet/App/Controller/Report.pm +++ b/perllib/FixMyStreet/App/Controller/Report.pm @@ -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'; - $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/Update.pm b/perllib/FixMyStreet/App/Controller/Report/Update.pm index 4e0d6d6c5..ccdfa8a54 100644 --- a/perllib/FixMyStreet/App/Controller/Report/Update.pm +++ b/perllib/FixMyStreet/App/Controller/Report/Update.pm @@ -77,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. @@ -106,13 +98,10 @@ sub process_user : Private { $email =~ s{\s+}{}g; my $update_user = $c->model('DB::User')->find_or_new( { email => $email } ); - - # set the user's name if they don't have one $update_user->name( Utils::trim_text( $params{name} ) ) - unless $update_user->name; + if $params{name}; $c->stash->{update_user} = $update_user; - $c->stash->{email} = $update_user->email; return 1; } @@ -158,9 +147,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; } @@ -215,6 +202,7 @@ sub save_update : Private { $update->user->insert; } 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; } @@ -250,8 +238,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; -- cgit v1.2.3 From 09ce7aeb489e5a43054c3b2b7bfa47fd6aba7c43 Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Thu, 23 Jun 2011 14:13:04 +0100 Subject: Use user if logged in, and don't show email field on update. --- .../FixMyStreet/App/Controller/Report/Update.pm | 28 +++++++++++++--------- 1 file changed, 17 insertions(+), 11 deletions(-) (limited to 'perllib/FixMyStreet/App/Controller') diff --git a/perllib/FixMyStreet/App/Controller/Report/Update.pm b/perllib/FixMyStreet/App/Controller/Report/Update.pm index ccdfa8a54..86c84f14c 100644 --- a/perllib/FixMyStreet/App/Controller/Report/Update.pm +++ b/perllib/FixMyStreet/App/Controller/Report/Update.pm @@ -86,20 +86,26 @@ 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 ) { - # Extract all the params to a hash to make them easier to work with - my %params = # - map { $_ => scalar $c->req->param($_) } # - ( 'rznvy', 'name' ); + $update_user = $c->user->obj; - # cleanup the email address - my $email = $params{rznvy} ? lc $params{rznvy} : ''; - $email =~ s{\s+}{}g; + } else { - my $update_user = $c->model('DB::User')->find_or_new( { email => $email } ); - $update_user->name( Utils::trim_text( $params{name} ) ) - if $params{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; + + $update_user = $c->model('DB::User')->find_or_new( { email => $email } ); + $update_user->name( Utils::trim_text( $params{name} ) ); + + } $c->stash->{update_user} = $update_user; -- cgit v1.2.3