Clarify ‘inspected’ behaviour

A report is now considered 'inspected' if its state was changed by an inspector.
Additionally, an AdminLog entry is created so the time of inspection and
inspector can be recorded.

4 files changed, 20 insertions, 14 deletions

diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm
index 175db4a19..ad26ecf84 100644
--- a/perllib/FixMyStreet/App/Controller/Report.pm
+++ b/perllib/FixMyStreet/App/Controller/Report.pm
@@ -343,12 +343,9 @@ sub inspect : Private {
                 $problem->set_extra_metadata( $_ => $c->get_param($_) );
             }
 
-            if ( $c->get_param('save_inspected') ) {
+            if ( $c->get_param('include_update') ) {
                 $update_text = Utils::cleanup_text( $c->get_param('public_update'), { allow_multiline => 1 } );
-                if ($update_text) {
-                    $problem->set_extra_metadata( inspected => 1 );
-                    $reputation_change = 1;
-                } else {
+                if (!$update_text) {
                     $valid = 0;
                     $c->stash->{errors} ||= [];
                     push @{ $c->stash->{errors} }, _('Please provide a public update for this report.');
@@ -374,6 +371,14 @@ sub inspect : Private {
             }
             if ( $problem->state ne $old_state ) {
                 $c->forward( '/admin/log_edit', [ $problem->id, 'problem', 'state_change' ] );
+
+                # If the state has been changed by an inspector, consider the
+                # report to be inspected.
+                unless ($problem->get_extra_metadata('inspected')) {
+                    $problem->set_extra_metadata( inspected => 1 );
+                    $c->forward( '/admin/log_edit', [ $problem->id, 'problem', 'inspected' ] );
+                    $reputation_change = 1;
+                }
             }
         }
 
diff --git a/t/app/controller/report_inspect.t b/t/app/controller/report_inspect.t
index 9888fc0f4..be2240cd2 100644
--- a/t/app/controller/report_inspect.t
+++ b/t/app/controller/report_inspect.t
@@ -57,7 +57,7 @@ FixMyStreet::override_config {
     };
 
     subtest "test basic inspect submission" => sub {
-        $mech->submit_form_ok({ button => 'save', with_fields => { traffic_information => 'Yes', state => 'Action Scheduled', save_inspected => undef } });
+        $mech->submit_form_ok({ button => 'save', with_fields => { traffic_information => 'Yes', state => 'Action Scheduled', include_update => undef } });
         $report->discard_changes;
         is $report->state, 'action scheduled', 'report state changed';
         is $report->get_extra_metadata('traffic_information'), 'Yes', 'report data changed';
@@ -65,10 +65,11 @@ FixMyStreet::override_config {
 
     subtest "test inspect & instruct submission" => sub {
         $report->unset_extra_metadata('inspected');
+        $report->state('confirmed');
         $report->update;
         my $reputation = $report->user->get_extra_metadata("reputation") || 0;
         $mech->get_ok("/report/$report_id");
-        $mech->submit_form_ok({ button => 'save', with_fields => { public_update => "This is a public update.", save_inspected => "1" } });
+        $mech->submit_form_ok({ button => 'save', with_fields => { public_update => "This is a public update.", include_update => "1", state => 'action scheduled' } });
         $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';
@@ -80,7 +81,7 @@ FixMyStreet::override_config {
         $report->update;
         $report->comments->delete_all;
         $mech->get_ok("/report/$report_id");
-        $mech->submit_form_ok({ button => 'save', with_fields => { public_update => undef, save_inspected => "1" } });
+        $mech->submit_form_ok({ button => 'save', with_fields => { public_update => undef, include_update => "1" } });
         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";
@@ -117,7 +118,7 @@ FixMyStreet::override_config {
         $report->comments->delete_all;
 
         $mech->get_ok("/report/$report_id");
-        $mech->submit_form_ok({ button => 'save', with_fields => { state => 'Duplicate', duplicate_of => $report2->id, public_update => "This is a duplicate.", save_inspected => "1" } });
+        $mech->submit_form_ok({ button => 'save', with_fields => { state => 'Duplicate', duplicate_of => $report2->id, public_update => "This is a duplicate.", include_update => "1" } });
         $report->discard_changes;
 
         is $report->state, 'duplicate', 'report marked as duplicate';
@@ -136,7 +137,7 @@ FixMyStreet::override_config {
             state => 'Duplicate',
             duplicate_of => $report2->id,
             public_update => $update_text,
-            save_inspected => "1",
+            include_update => "1",
         }});
         $report->discard_changes;
 
@@ -190,7 +191,7 @@ FixMyStreet::override_config {
     subtest "Oxfordshire-specific traffic management options are shown" => sub {
         $report->update({ state => 'confirmed' });
         $mech->get_ok("/report/$report_id");
-        $mech->submit_form_ok({ button => 'save', with_fields => { traffic_information => 'Signs and Cones', state => 'Action Scheduled', save_inspected => undef } });
+        $mech->submit_form_ok({ button => 'save', with_fields => { traffic_information => 'Signs and Cones', state => 'Action Scheduled', include_update => undef } });
         $report->discard_changes;
         is $report->state, 'action scheduled', 'report state changed';
         is $report->get_extra_metadata('traffic_information'), 'Signs and Cones', 'report data changed';
@@ -211,7 +212,7 @@ FixMyStreet::override_config {
         # which should cause it to be resent. We clear the host because
         # otherwise testing stays on host() above.
         $mech->clear_host;
-        $mech->submit_form(button => 'save', with_fields => { category => 'Horses', save_inspected => undef, });
+        $mech->submit_form(button => 'save', with_fields => { category => 'Horses', include_update => undef, });
 
         $report->discard_changes;
         is $report->category, "Horses", "Report in correct category";
diff --git a/templates/web/base/report/_inspect.html b/templates/web/base/report/_inspect.html
index 5a7e99ef9..33b548c76 100644
--- a/templates/web/base/report/_inspect.html
+++ b/templates/web/base/report/_inspect.html
@@ -127,7 +127,7 @@
         [% IF permissions.report_inspect %]
           <p>
             <label class="label-containing-checkbox">
-              <input type="checkbox" name="save_inspected" value="1" class="js-toggle-public-update" checked>
+              <input type="checkbox" name="include_update" value="1" class="js-toggle-public-update" checked>
               [% loc('Save with a public update') %]
             </label>
           </p>
diff --git a/web/cobrands/fixmystreet/offline.js b/web/cobrands/fixmystreet/offline.js
index 7b9be4c6c..0fdad1279 100644
--- a/web/cobrands/fixmystreet/offline.js
+++ b/web/cobrands/fixmystreet/offline.js
@@ -322,7 +322,7 @@ fixmystreet.offline = (function() {
         if (savedForm) {
             savedForm.replace(/\+/g, '%20').split('&').forEach(function(kv) {
                 kv = kv.split('=', 2);
-                if (kv[0] != 'save_inspected' && kv[0] != 'public_update' && kv[0] != 'save') {
+                if (kv[0] != 'include_update' && kv[0] != 'public_update' && kv[0] != 'save') {
                     $('[name=' + kv[0] + ']').val(decodeURIComponent(kv[1]));
                 }
             });