Use Chains in /report.

6 files changed, 42 insertions, 64 deletions

diff --git a/perllib/FixMyStreet/App/Controller/My.pm b/perllib/FixMyStreet/App/Controller/My.pm
index 9a2a4f470..ed890ad82 100644
--- a/perllib/FixMyStreet/App/Controller/My.pm
+++ b/perllib/FixMyStreet/App/Controller/My.pm
@@ -210,7 +210,7 @@ sub planned_change : Path('planned/change') {
         $c->res->content_type('application/json; charset=utf-8');
         $c->res->body(encode_json({ outcome => $add ? 'add' : 'remove' }));
     } else {
-        $c->res->redirect( $c->uri_for_action('report/display', $id) );
+        $c->res->redirect( $c->uri_for_action('report/display', [ $id ]) );
     }
 }
 
diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm
index 854dbf3ea..198edb5f7 100644
--- a/perllib/FixMyStreet/App/Controller/Report.pm
+++ b/perllib/FixMyStreet/App/Controller/Report.pm
@@ -20,8 +20,8 @@ Show a report
 
 =head2 index
 
-Redirect to homepage unless C<id> parameter in query, in which case redirect to
-'/report/$id'.
+Redirect to homepage unless we have a homepage template,
+in which case show that.
 
 =cut
 
@@ -35,13 +35,13 @@ sub index : Path('') : Args(0) {
     }
 }
 
-=head2 report_display
+=head2 id
 
-Display a report.
+Load in ID, for use by chained pages.
 
 =cut
 
-sub display : Path('') : Args(1) {
+sub id :PathPart('report') :Chained :CaptureArgs(1) {
     my ( $self, $c, $id ) = @_;
 
     if (
@@ -49,15 +49,17 @@ sub display : Path('') : Args(1) {
         || $id =~ m{ ^(\d+) \D .* $ }x    # trailing garbage
       )
     {
-        return $c->res->redirect( $c->uri_for($1), 301 );
+        $c->res->redirect( $c->uri_for($1), 301 );
+        $c->detach;
     }
 
-    $c->forward( '_display', [ $id ] );
+    $c->forward( 'load_problem_or_display_error', [ $id ] );
 }
 
 =head2 ajax
 
-Return JSON formatted details of a report
+Return JSON formatted details of a report.
+URL used by mobile app so remains /report/ajax/N.
 
 =cut
 
@@ -65,14 +67,20 @@ sub ajax : Path('ajax') : Args(1) {
     my ( $self, $c, $id ) = @_;
 
     $c->stash->{ajax} = 1;
-    $c->forward( '_display', [ $id ] );
+    $c->forward('load_problem_or_display_error', [ $id ]);
+    $c->forward('display');
 }
 
-sub _display : Private {
-    my ( $self, $c, $id ) = @_;
+=head2 display
+
+Display a report.
+
+=cut
+
+sub display :PathPart('') :Chained('id') :Args(0) {
+    my ( $self, $c ) = @_;
 
     $c->forward('/auth/get_csrf_token');
-    $c->forward( 'load_problem_or_display_error', [ $id ] );
     $c->forward( 'load_updates' );
     $c->forward( 'format_problem_for_display' );
 
@@ -84,21 +92,14 @@ sub _display : Private {
     }
 }
 
-sub support : Path('support') : Args(0) {
+sub support :Chained('id') :Args(0) {
     my ( $self, $c ) = @_;
 
-    my $id = $c->get_param('id');
-
-    my $uri =
-        $id
-      ? $c->uri_for( '/report', $id )
-      : $c->uri_for('/');
-
-    if ( $id && $c->cobrand->can_support_problems && $c->user && $c->user->from_body ) {
-        $c->forward( 'load_problem_or_display_error', [ $id ] );
+    if ( $c->cobrand->can_support_problems && $c->user && $c->user->from_body ) {
         $c->stash->{problem}->update( { interest_count => \'interest_count +1' } );
     }
-    $c->res->redirect( $uri );
+
+    $c->res->redirect($c->stash->{problem}->url);
 }
 
 sub load_problem_or_display_error : Private {
@@ -271,22 +272,18 @@ users too about this change, at which point we can delete:
 
 =cut
 
-sub delete :Local :Args(1) {
-    my ( $self, $c, $id ) = @_;
+sub delete :Chained('id') :Args(0) {
+    my ($self, $c) = @_;
 
     $c->forward('/auth/check_csrf_token');
 
-    $c->forward( 'load_problem_or_display_error', [ $id ] );
     my $p = $c->stash->{problem};
 
-    my $uri = $c->uri_for( '/report', $id );
-
-    return $c->res->redirect($uri) unless $c->user_exists;
+    return $c->res->redirect($p->url) unless $c->user_exists;
 
     my $body = $c->user->obj->from_body;
-    return $c->res->redirect($uri) unless $body;
-
-    return $c->res->redirect($uri) unless $p->bodies->{$body->id};
+    return $c->res->redirect($p->url) unless $body;
+    return $c->res->redirect($p->url) unless $p->bodies->{$body->id};
 
     $p->state('hidden');
     $p->lastupdate( \'current_timestamp' );
@@ -299,26 +296,10 @@ sub delete :Local :Args(1) {
         admin_user => $c->user->from_body->name,
         object_type => 'problem',
         action => 'state_change',
-        object_id => $id,
+        object_id => $p->id,
     } );
 
-    return $c->res->redirect($uri);
-}
-
-=head2 action_router
-
-A router for dispatching handlers for sub-actions on a particular report,
-e.g. /report/1/inspect
-
-=cut
-
-sub action_router : Path('') : Args(2) {
-    my ( $self, $c, $id, $action ) = @_;
-
-    $c->go( 'map', [ $id ] ) if $action eq 'map';
-    $c->go( 'nearby_json', [ $id ] ) if $action eq 'nearby.json';
-
-    $c->detach( '/page_error_404_not_found', [] );
+    return $c->res->redirect($p->url);
 }
 
 sub inspect : Private {
@@ -539,10 +520,8 @@ sub inspect : Private {
     }
 };
 
-sub map : Private {
-    my ( $self, $c, $id ) = @_;
-
-    $c->forward( 'load_problem_or_display_error', [ $id ] );
+sub map :Chained('id') :Args(0) {
+    my ($self, $c) = @_;
 
     my $image = $c->stash->{problem}->static_map;
     $c->res->content_type($image->{content_type});
@@ -550,10 +529,9 @@ sub map : Private {
 }
 
 
-sub nearby_json : Private {
-    my ( $self, $c, $id ) = @_;
+sub nearby_json :PathPart('nearby.json') :Chained('id') :Args(0) {
+    my ($self, $c) = @_;
 
-    $c->forward( 'load_problem_or_display_error', [ $id ] );
     my $p = $c->stash->{problem};
     my $dist = 1;
 
diff --git a/perllib/FixMyStreet/App/Controller/Report/Update.pm b/perllib/FixMyStreet/App/Controller/Report/Update.pm
index cb9bbdb67..dc46be61f 100644
--- a/perllib/FixMyStreet/App/Controller/Report/Update.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/Update.pm
@@ -23,14 +23,14 @@ sub report_update : Path : Args(0) {
     $c->forward('initialize_update');
     $c->forward('load_problem');
     $c->forward('check_form_submitted')
-      or $c->go( '/report/display', [ $c->stash->{problem}->id ] );
+      or $c->go( '/report/display', [ $c->stash->{problem}->id ], [] );
 
     $c->forward('/auth/check_csrf_token');
     $c->forward('process_update');
     $c->forward('process_user');
     $c->forward('/photo/process_photo');
     $c->forward('check_for_errors')
-      or $c->go( '/report/display', [ $c->stash->{problem}->id ] );
+      or $c->go( '/report/display', [ $c->stash->{problem}->id ], [] );
 
     $c->forward('save_update');
     $c->forward('redirect_or_confirm_creation');
diff --git a/t/app/controller/report_interest_count.t b/t/app/controller/report_interest_count.t
index 04f567615..330d844d0 100644
--- a/t/app/controller/report_interest_count.t
+++ b/t/app/controller/report_interest_count.t
@@ -109,7 +109,7 @@ FixMyStreet::override_config {
         $mech->content_contains( '1 supporter' );
 
         $mech->log_out_ok( $user->email );
-        $mech->post_ok("/report/support", { id => $report_id } );
+        $mech->post_ok("/report/$report_id/support");
 
         is $mech->uri->path, "/report/$report_id", 'add support redirects to report page';
 
diff --git a/templates/web/base/report/_support.html b/templates/web/base/report/_support.html
index 3e372ba69..420b632aa 100644
--- a/templates/web/base/report/_support.html
+++ b/templates/web/base/report/_support.html
@@ -8,8 +8,8 @@
 %]
 
     [% IF c.user AND c.user.from_body %]
-    <form action="[% c.uri_for( '/report/support' ) %]">
-        <p id="supporter"><small>[% text %] <input type="hidden" name="id" value="[% problem.id %]"><input type="submit" class="green-btn" value="Add support"></small></p>
+    <form action="/report/[% problem.id %]/support">
+        <p id="supporter"><small>[% text %] <input type="submit" class="green-btn" value="Add support"></small></p>
     </form>
     [% ELSE %]
         <p id="supporter"><small>[% text %]</small></p>
diff --git a/templates/web/base/report/display_tools.html b/templates/web/base/report/display_tools.html
index eaf45fdf3..be788a50d 100644
--- a/templates/web/base/report/display_tools.html
+++ b/templates/web/base/report/display_tools.html
@@ -1,7 +1,7 @@
 <div class="shadow-wrap">
     <ul id="key-tools">
         [% IF c.user_exists AND c.cobrand.users_can_hide AND c.user.belongs_to_body( problem.bodies_str ) %]
-        <li><form method="post" action="/report/delete/[% problem.id %]" id="remove-from-site-form">
+        <li><form method="post" action="/report/[% problem.id %]/delete" id="remove-from-site-form">
             <input type="hidden" name="token" value="[% csrf_token %]">
             <button type="submit" id="key-tool-report-abuse" class="abuse" data-confirm="[% loc('Are you sure?') %]" name="remove_from_site">[% loc('Remove from site') %]</button>
         </form></li>