Add ‘Inspection required’ field to categories

Categories can now require reports to be marked as 'inspected' via the frontend
before they're sent by send-reports.

A side-effect here is that send-reports will perform an extra n queries for each
report, where n is the number of bodies that report is being sent to, but
hopefully in practice this won't matter as it's an offline cronjob.

See mysociety/fixmystreetforcouncils#50

9 files changed, 111 insertions, 13 deletions

diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm
index 66b46877f..c853827d0 100644
--- a/perllib/FixMyStreet/App/Controller/Admin.pm
+++ b/perllib/FixMyStreet/App/Controller/Admin.pm
@@ -362,13 +362,19 @@ sub update_contacts : Private {
         $contact->api_key( $c->get_param('api_key') );
         $contact->send_method( $c->get_param('send_method') );
 
-        # Set the photo_required flag in extra to the appropriate value
+        # Set flags in extra to the appropriate values
         if ( $c->get_param('photo_required') ) {
             $contact->set_extra_metadata_if_undefined(  photo_required => 1 );
         }
         else {
             $contact->unset_extra_metadata( 'photo_required' );
         }
+        if ( $c->get_param('inspection_required') ) {
+            $contact->set_extra_metadata( inspection_required => 1 );
+        }
+        else {
+            $contact->unset_extra_metadata( 'inspection_required' );
+        }
 
         if ( %errors ) {
             $c->stash->{updated} = _('Please correct the errors below');
diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm
index 2f6418886..7f1132117 100644
--- a/perllib/FixMyStreet/App/Controller/Report.pm
+++ b/perllib/FixMyStreet/App/Controller/Report.pm
@@ -318,13 +318,17 @@ sub inspect : Private {
         }
     }
 
-    if ( $c->get_param('save') ) {
+    if ( $c->get_param('save') || $c->get_param('save_inspected') ) {
         $c->forward('/auth/check_csrf_token');
 
         foreach (qw/priority 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 );
+        }
+
         # Handle the state changing
         my $old_state = $problem->state;
         $problem->state($c->get_param('state'));
diff --git a/perllib/FixMyStreet/Cobrand/Default.pm b/perllib/FixMyStreet/Cobrand/Default.pm
index a58f50b18..b5487ea5b 100644
--- a/perllib/FixMyStreet/Cobrand/Default.pm
+++ b/perllib/FixMyStreet/Cobrand/Default.pm
@@ -860,25 +860,25 @@ sub get_report_stats { return 0; }
 sub get_body_sender {
     my ( $self, $body, $category ) = @_;
 
+    # look up via category
+    my $contact = $body->contacts->search( { category => $category } )->first;
     if ( $body->can_be_devolved ) {
-        # look up via category
-        my $config = $body->result_source->schema->resultset("Contact")->search( { body_id => $body->id, category => $category } )->first;
-        if ( $config->send_method ) {
-            return { method => $config->send_method, config => $config };
+        if ( $contact->send_method ) {
+            return { method => $contact->send_method, config => $contact, contact => $contact };
         } else {
-            return { method => $body->send_method, config => $body };
+            return { method => $body->send_method, config => $body, contact => $contact };
         }
     } elsif ( $body->send_method ) {
-        return { method => $body->send_method, config => $body };
+        return { method => $body->send_method, config => $body, contact => $contact };
     }
 
-    return $self->_fallback_body_sender( $body, $category );
+    return $self->_fallback_body_sender( $body, $category, $contact );
 }
 
 sub _fallback_body_sender {
-    my ( $self, $body, $category ) = @_;
+    my ( $self, $body, $category, $contact ) = @_;
 
-    return { method => 'Email' };
+    return { method => 'Email', contact => $contact };
 };
 
 sub example_places {
diff --git a/perllib/FixMyStreet/Script/Reports.pm b/perllib/FixMyStreet/Script/Reports.pm
index a51923456..ab0d90ba8 100644
--- a/perllib/FixMyStreet/Script/Reports.pm
+++ b/perllib/FixMyStreet/Script/Reports.pm
@@ -143,6 +143,12 @@ sub send(;$) {
             }
             $reporters{ $sender } ||= $sender->new();
 
+            my $inspection_required = $sender_info->{contact}->get_extra_metadata('inspection_required') if $sender_info->{contact};
+            if ( $inspection_required && !$row->get_extra_metadata('inspected') ) {
+                $skip = 1;
+                debug_print("skipped because not yet inspected", $row->id) if $debug_mode;
+            }
+
             if ( $reporters{ $sender }->should_skip( $row ) ) {
                 $skip = 1;
                 debug_print("skipped by sender " . $sender_info->{method} . " (might be due to previous failed attempts?)", $row->id) if $debug_mode;
diff --git a/t/app/sendreport/inspection_required.t b/t/app/sendreport/inspection_required.t
new file mode 100644
index 000000000..178fa2a1f
--- /dev/null
+++ b/t/app/sendreport/inspection_required.t
@@ -0,0 +1,59 @@
+use strict;
+use warnings;
+
+use Test::More;
+
+use FixMyStreet;
+use FixMyStreet::DB;
+use FixMyStreet::TestMech;
+use FixMyStreet::SendReport::Email;
+use mySociety::Locale;
+
+ok( my $mech = FixMyStreet::TestMech->new, 'Created mech object' );
+
+my $user = $mech->create_user_ok( 'user@example.com' );
+
+my $body = $mech->create_body_ok( 2237, 'Oxfordshire County Council', id => 2237 );
+# $body->update({ send_method => 'Email' });
+
+my $contact = $mech->create_contact_ok(
+    body_id => $body->id,
+    category => 'Pothole',
+    email => 'test@example.org',
+);
+$contact->set_extra_metadata(inspection_required => 1);
+$contact->update;
+
+my @reports = $mech->create_problems_for_body( 1, $body->id, 'Test', {
+    cobrand => 'oxfordshire',
+    category => $contact->category,
+    user => $user,
+});
+my $report = $reports[0];
+
+subtest 'Report isn’t sent if uninspected' => sub {
+    $mech->clear_emails_ok;
+
+    FixMyStreet::DB->resultset('Problem')->send_reports();
+
+    $mech->email_count_is( 0 );
+    is $report->whensent, undef, 'Report hasn’t been sent';
+};
+
+subtest 'Report is sent when inspected' => sub {
+    $mech->clear_emails_ok;
+    $report->set_extra_metadata(inspected => 1);
+    $report->update;
+
+    FixMyStreet::DB->resultset('Problem')->send_reports();
+
+    $report->discard_changes;
+    $mech->email_count_is( 1 );
+    ok $report->whensent, 'Report marked as sent';
+};
+
+done_testing();
+
+END {
+    $mech->delete_body($body);
+}
diff --git a/t/cobrand/get_body_sender.t b/t/cobrand/get_body_sender.t
index 66cfc02b7..fbdfbffa7 100644
--- a/t/cobrand/get_body_sender.t
+++ b/t/cobrand/get_body_sender.t
@@ -25,9 +25,9 @@ FixMyStreet::override_config {
     MAPIT_TYPES => [ 'LBO' ],
     MAPIT_URL => 'http://mapit.mysociety.org/',
 }, sub {
-    is_deeply $c->get_body_sender( $body ), { method => 'Email' }, 'defaults to email';
+    is_deeply $c->get_body_sender( $body ), { method => 'Email', contact => undef }, 'defaults to email';
     $body_area->update({ area_id => 2481 }); # Croydon LBO
-    is_deeply $c->get_body_sender( $body ), { method => 'Email' }, 'still email if London borough';
+    is_deeply $c->get_body_sender( $body ), { method => 'Email', contact => undef }, 'still email if London borough';
 };
 
 $body->send_method( 'TestMethod' );
diff --git a/templates/web/base/admin/body.html b/templates/web/base/admin/body.html
index a00c5ca4c..6d23c1458 100644
--- a/templates/web/base/admin/body.html
+++ b/templates/web/base/admin/body.html
@@ -204,6 +204,16 @@
       <label for="non_public" class="inline">[% loc('Private') %]</label>
     </p>
 
+    <div class="admin-hint">
+      <p>
+        [% loc("Check <strong>inspection required</strong> if reports in this category <strong>must be inspected</strong> before being sent.") %]
+      </p>
+    </div>
+    <p>
+      <input type="checkbox" name="inspection_required" value="1" id="inspection_required" [% 'checked' IF contact.get_extra_metadata('inspection_required') %]>
+      <label for="inspection_required" class="inline">[% loc('Inspection required') %]</label>
+    </p>
+
     <p>
       <input type="hidden" name="posted" value="new" >
       <input type="hidden" name="token" value="[% csrf_token %]" >
diff --git a/templates/web/base/admin/category_edit.html b/templates/web/base/admin/category_edit.html
index 6537fe028..cdf371643 100644
--- a/templates/web/base/admin/category_edit.html
+++ b/templates/web/base/admin/category_edit.html
@@ -43,6 +43,8 @@
       [% IF c.cobrand.moniker != 'zurich' %]
         <input type="checkbox" name="non_public" value="1" id="non_public"[% ' checked' IF contact.non_public %]>
         <label class="inline" for="non_public">[% loc('Private') %]</label>
+        <input type="checkbox" name="inspection_required" value="1" id="inspection_required"[% ' checked' IF contact.get_extra_metadata('inspection_required') %]>
+        <label class="inline" for="inspection_required">[% loc('Inspection required') %]</label>
       [% ELSE %]
         <input type="checkbox" name="photo_required" value="1" id="photo_required"[% ' checked' IF contact.get_extra_metadata('photo_required') %]>
         <label class="inline" for="photo_required">[% loc('Photo required') %]</label>
diff --git a/templates/web/base/report/_inspect.html b/templates/web/base/report/_inspect.html
index 6f81cc04a..188816ec1 100644
--- a/templates/web/base/report/_inspect.html
+++ b/templates/web/base/report/_inspect.html
@@ -76,6 +76,17 @@
           <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') %]
+            <input type="submit" value="[% loc('Save changes + send') %]" name="save_inspected" />
+          [% ELSE %]
+            [% 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>
       </form>
     </div>
   </div>