diff options
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Report.pm | 76 | ||||
| -rw-r--r-- | t/app/controller/report_display.t | 75 |
2 files changed, 118 insertions, 33 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm index 022e0caff..f0b70b2e6 100644 --- a/perllib/FixMyStreet/App/Controller/Report.pm +++ b/perllib/FixMyStreet/App/Controller/Report.pm @@ -4,7 +4,6 @@ use Moose; use namespace::autoclean; BEGIN { extends 'Catalyst::Controller'; } - =head1 NAME FixMyStreet::App::Controller::Report - display a report @@ -15,6 +14,26 @@ Show a report =head1 ACTIONS +=head2 index + +Redirect to homepage unless C<id> parameter in query, in which case redirect to +'/report/$id'. + +=cut + +sub index : Path('') : Args(0) { + my ( $self, $c ) = @_; + + my $id = $c->req->param('id'); + + my $uri = + $id + ? $c->uri_for( '/report', $id ) + : $c->uri_for('/'); + + $c->res->redirect($uri); +} + =head2 report_display Display a report. @@ -23,7 +42,6 @@ Display a report. sub display : Path('') : Args(1) { my ( $self, $c, $id ) = @_; -} # my ($q, $errors, $field_errors) = @_; # my @errors = @$errors; @@ -31,46 +49,39 @@ sub display : Path('') : Args(1) { # my $cobrand = Page::get_cobrand($q); # push @errors, _('There were problems with your update. Please see below.') if (scalar keys %field_errors); - # my @vars = qw(id name rznvy update fixed add_alert upload_fileid submit_update); # my %input = map { $_ => $q->param($_) || '' } @vars; # my %input_h = map { $_ => $q->param($_) ? ent($q->param($_)) : '' } @vars; # my $base = Cobrand::base_url($cobrand); + # Some council with bad email software + if ( $id =~ m{ ^ 3D (\d+) $ }x ) { + return $c->res->redirect( $c->uri_for($1), 301 ); + } + # try to load a report if the id is a number + my $problem = + $id =~ m{\D} + ? undef + : $c->model('DB::Problem')->find( { id => $id } ); -# # Some council with bad email software -# if ($input{id} =~ /^3D\d+$/) { -# $input{id} =~ s/^3D//; -# print $q->redirect(-location => $base . '/report/' . $input{id}, -status => 301); -# return ''; -# } - - - -# # Redirect old /?id=NNN URLs to /report/NNN -# if (!@errors && !scalar keys %field_errors && $ENV{SCRIPT_URL} eq '/') { -# print $q->redirect(-location => $base . '/report/' . $input{id}, -status => 301); -# return ''; -# } + if ( !$problem ) { # bad id or id not found + $c->detach( '/page_not_found', [ _('Unknown problem ID') ] ); + } + # elsif () { + # + # } -# # Get all information from database -# return display_location($q, _('Unknown problem ID')) if !$input{id} || $input{id} =~ /\D/; - - -# my $problem = Problems::fetch_problem($input{id}); -# return display_location($q, _('Unknown problem ID')) unless $problem; # return front_page($q, _('That report has been removed from FixMyStreet.'), '410 Gone') if $problem->{state} eq 'hidden'; - -# my $extra_data = Cobrand::extra_data($cobrand, $q); -# my $google_link = Cobrand::base_url_for_emails($cobrand, $extra_data) -# . '/report/' . $problem->{id}; -# # truncate the lat,lon for nicer rss urls -# my ( $short_lat, $short_lon ) = -# map { Utils::truncate_coordinate($_) } # -# ( $problem->{latitude}, $problem->{longitude} ); + # my $extra_data = Cobrand::extra_data($cobrand, $q); + # my $google_link = Cobrand::base_url_for_emails($cobrand, $extra_data) + # . '/report/' . $problem->{id}; + # # truncate the lat,lon for nicer rss urls + # my ( $short_lat, $short_lon ) = + # map { Utils::truncate_coordinate($_) } # + # ( $problem->{latitude}, $problem->{longitude} ); # my $map_links = ''; # $map_links = "<p id='sub_map_links'>" @@ -164,9 +175,8 @@ sub display : Path('') : Args(1) { # # my $page = Page::template_include('problem', $q, Page::template_root($q), %vars); # return ($page, %params); -# } - +} __PACKAGE__->meta->make_immutable; diff --git a/t/app/controller/report_display.t b/t/app/controller/report_display.t index e69de29bb..888e5e5b9 100644 --- a/t/app/controller/report_display.t +++ b/t/app/controller/report_display.t @@ -0,0 +1,75 @@ +use strict; +use warnings; +use Test::More; + +use FixMyStreet::TestMech; +use Web::Scraper; +use Path::Class; + +my $mech = FixMyStreet::TestMech->new; + +# create a test user and report +$mech->delete_user('test@example.com'); +my $user = + FixMyStreet::App->model('DB::User') + ->find_or_create( { email => 'test@example.com', name => 'Test User' } ); +ok $user, "created test user"; + +my $report = FixMyStreet::App->model('DB::Problem')->find_or_create( + { + postcode => 'SW1A 1AA', + council => '2504', + areas => ',105255,11806,11828,2247,2504,', + category => 'Other', + title => 'Test 2', + detail => 'Test 2', + used_map => 't', + name => 'Test User', + anonymous => 'f', + state => 'confirmed', + lang => 'en-gb', + service => '', + cobrand => 'default', + cobrand_data => '', + send_questionnaire => 't', + latitude => '51.5016605453401', + longitude => '-0.142497580865087', + user_id => $user->id, + } +); +my $report_id = $report->id; +ok $report, "created test report - $report_id"; + +subtest "check that no id redirects to homepage" => sub { + $mech->get_ok('/report'); + is $mech->uri->path, '/', "at home page"; +}; + +subtest "test id=NNN redirects to /NNN" => sub { + $mech->get_ok("/report?id=$report_id"); + is $mech->uri->path, "/report/$report_id", "at /report/$report_id"; +}; + +subtest "test bad council email clients web links" => sub { + $mech->get_ok("/report/3D$report_id"); + is $mech->uri->path, "/report/$report_id", "at /report/$report_id"; +}; + +subtest "test bad ids get dealt with (404)" => sub { + foreach my $id ( 'XXX', 99999999 ) { + ok $mech->get("/report/$id"), "get '/report/$id'"; + is $mech->res->code, 404, "page not found"; + is $mech->uri->path, "/report/$id", "at /report/$id"; + } +}; + +subtest "test a good report" => sub { + $mech->get_ok("/report/$report_id"); + is $mech->uri->path, "/report/$report_id", "at /report/$report_id"; +}; + +fail "change report to hidden and check for 400 status"; + +# tidy up +$mech->delete_user('test@example.com'); +done_testing(); |