Require inspector to provide update when instructing report

This adds an update field to the bottom of the inspect form, requiring the
inspector to provide an update to be added to the report as it's sent.

See mysociety/fixmystreetforcouncils#64

4 files changed, 63 insertions, 8 deletions

diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm
index 582de092c..1d67afd0e 100644
--- a/perllib/FixMyStreet/App/Controller/Report.pm
+++ b/perllib/FixMyStreet/App/Controller/Report.pm
@@ -4,6 +4,7 @@ use Moose;
 use namespace::autoclean;
 use JSON::MaybeXS;
 use List::MoreUtils qw(any);
+use Utils;
 
 BEGIN { extends 'Catalyst::Controller'; }
 
@@ -312,13 +313,23 @@ sub inspect : Private {
     if ( $c->get_param('save') || $c->get_param('save_inspected') ) {
         $c->forward('/auth/check_csrf_token');
 
+        my $valid = 1;
+        my $update_text;
+
         if ($permissions->{report_inspect}) {
             foreach (qw/detailed_location detailed_information traffic_information/) {
                 $problem->set_extra_metadata( $_ => $c->get_param($_) );
             }
 
             if ( $c->get_param('save_inspected') ) {
-                $problem->set_extra_metadata( inspected => 1 );
+                $update_text = Utils::cleanup_text( $c->get_param('public_update'), { allow_multiline => 1 } );
+                if ($update_text) {
+                    $problem->set_extra_metadata( inspected => 1 );
+                } else {
+                    $valid = 0;
+                    $c->stash->{errors} ||= [];
+                    push @{ $c->stash->{errors} }, _('Please provide a public update for this report.');
+                }
             }
 
             # Handle the state changing
@@ -339,11 +350,11 @@ sub inspect : Private {
             $problem->response_priority( $problem->response_priorities->find({ id => $c->get_param('priority') }) );
         }
 
-        my $valid = 1;
         if ( !$c->forward( '/admin/report_edit_location', [ $problem ] ) ) {
             # New lat/lon isn't valid, show an error
             $valid = 0;
-            $c->stash->{errors} = [ _('Invalid location. New location must be covered by the same council.') ];
+            $c->stash->{errors} ||= [];
+            push @{ $c->stash->{errors} }, _('Invalid location. New location must be covered by the same council.');
         }
 
         if ($permissions->{report_inspect} || $permissions->{report_edit_category}) {
@@ -352,6 +363,18 @@ sub inspect : Private {
 
         if ($valid) {
             $problem->update;
+            if ( defined($update_text) ) {
+                $problem->add_to_comments( {
+                    text => $update_text,
+                    created => \'current_timestamp',
+                    confirmed => \'current_timestamp',
+                    user_id => $c->user->id,
+                    name => $c->user->from_body->name,
+                    state => 'confirmed',
+                    mark_fixed => 0,
+                    anonymous => 0,
+                } );
+            }
             $c->res->redirect( $c->uri_for( '/report', $problem->id, 'inspect' ) );
         }
     }
diff --git a/t/app/controller/report_inspect.t b/t/app/controller/report_inspect.t
index adba77011..6d6ec6559 100644
--- a/t/app/controller/report_inspect.t
+++ b/t/app/controller/report_inspect.t
@@ -52,6 +52,28 @@ FixMyStreet::override_config {
         is $report->get_extra_metadata('traffic_information'), 'Lots', 'report data changed';
     };
 
+    subtest "test inspect & instruct submission" => sub {
+        $report->unset_extra_metadata('inspected');
+        $report->update;
+        $mech->get_ok("/report/$report_id/inspect");
+        $mech->submit_form_ok({ button => 'save_inspected', with_fields => { public_update => "This is a public update." } });
+        $report->discard_changes;
+        is $report->comments->first->text, "This is a public update.", 'Update was created';
+        is $report->get_extra_metadata('inspected'), 1, 'report marked as inspected';
+    };
+
+    subtest "test update is required when instructing" => sub {
+        $report->unset_extra_metadata('inspected');
+        $report->update;
+        $report->comments->delete_all;
+        $mech->get_ok("/report/$report_id/inspect");
+        $mech->submit_form_ok({ button => 'save_inspected', with_fields => { public_update => undef } });
+        is_deeply $mech->page_errors, [ "Please provide a public update for this report." ], 'errors match';
+        $report->discard_changes;
+        is $report->comments->count, 0, "Update wasn't created";
+        is $report->get_extra_metadata('inspected'), undef, 'report not marked as inspected';
+    };
+
     subtest "test location changes" => sub {
         $mech->get_ok("/report/$report_id/inspect");
         $mech->submit_form_ok({ button => 'save', with_fields => { latitude => 55, longitude => -2 } });
diff --git a/templates/web/base/report/_inspect.html b/templates/web/base/report/_inspect.html
index ce5110c54..7bf20a13a 100644
--- a/templates/web/base/report/_inspect.html
+++ b/templates/web/base/report/_inspect.html
@@ -3,6 +3,8 @@
 [% second_column = BLOCK -%]
   <div id="side-report-secondary">
     <div class="problem-inspector-fields clearfix">
+      [% INCLUDE 'errors.html' %]
+
       <form id="report_inspect_form" method="post" action="[% c.uri_for( '/report', problem.id, 'inspect' ) %]">
         <p class="left">
           <label for="problem_id">[% loc('Report ID:') %]</label>
@@ -91,17 +93,24 @@
           <a href="[% c.uri_for( '/report', problem.id ) %]" class="btn">[% loc('Cancel') %]</a>
           <input type="submit" value="[% loc('Save changes') %]" name="save" />
         </p>
-        <p>
-          [% UNLESS problem.get_extra_metadata('inspected') %]
+        [% UNLESS problem.get_extra_metadata('inspected') %]
+          <p>
+            <label for="public_update">[% loc('Public update:') %]</label>
+            [% INCLUDE 'admin/response_templates_select.html' for='public_update' %]
+            <textarea rows="2" name="public_update" id="public_update" required>[% public_update | html %]</textarea>
+          </p>
+          <p>
             <input type="submit" value="[% loc('Save changes + send') %]" name="save_inspected" />
-          [% ELSE %]
+          </p>
+        [% ELSE %]
+          <p>
             [% IF problem.whensent %]
               [% loc("<strong>Note:</strong> This report has been sent onwards for action. Any changes made won't be passed on.") %]
             [% ELSE %]
               [% loc("<strong>Note:</strong> This report hasn't yet been sent onwards for action. Any changes made may not be passed on.") %]
             [% END %]
-          [% END %]
-        </p>
+          </p>
+        [% END %]
       </form>
     </div>
   </div>
diff --git a/templates/web/base/report/inspect.html b/templates/web/base/report/inspect.html
index ad70b2ac5..0a3fdcbb2 100644
--- a/templates/web/base/report/inspect.html
+++ b/templates/web/base/report/inspect.html
@@ -1,5 +1,6 @@
 [%
   SET bodyclass = 'mappage with-actions';
   PROCESS 'report/_inspect.html';
+  SET shown_form = 1 UNLESS problem.get_extra_metadata('inspected');
   INCLUDE 'report/display.html', hide_inspect_button = 1;
 %]