Update old-style open311 script to use bodies.

Add some comments to the top of the open311 bin scripts.

9 files changed, 70 insertions, 60 deletions

diff --git a/bin/fetch-comments b/bin/fetch-comments
index ef099fcc9..a276433e4 100755
--- a/bin/fetch-comments
+++ b/bin/fetch-comments
@@ -1,4 +1,8 @@
 #!/usr/bin/env perl
+#
+# This script utilises the Open311 extension explained at
+# https://github.com/mysociety/FixMyStreet/wiki/Open311-FMS---Proposed-differences-to-Open311
+# to fetch updates on service requests.
 
 use strict;
 use warnings;
diff --git a/bin/fetch-comments-24hs b/bin/fetch-comments-24hs
index b84f09ba7..602383d40 100644
--- a/bin/fetch-comments-24hs
+++ b/bin/fetch-comments-24hs
@@ -1,4 +1,9 @@
 #!/usr/bin/env perl
+#
+# This script utilises the Open311 extension explained at
+# https://github.com/mysociety/FixMyStreet/wiki/Open311-FMS---Proposed-differences-to-Open311
+# to fetch updates on service requests from the past 24 hours, to check none
+# were missed.
 
 use strict;
 use warnings;
diff --git a/bin/open311-update-reports b/bin/open311-update-reports
index e7ff34273..3b77fef89 100644
--- a/bin/open311-update-reports
+++ b/bin/open311-update-reports
@@ -1,4 +1,10 @@
 #!/usr/bin/perl
+#
+# This script utilises the standard Open311 way of getting updates on reports
+# (by fetching all reports for a body and looking for updates). If possible,
+# please use the extension explained at
+# https://github.com/mysociety/FixMyStreet/wiki/Open311-FMS---Proposed-differences-to-Open311
+# and the fetch-comments/send-comments scripts.
 
 use strict;
 use warnings;
@@ -13,9 +19,9 @@ my $system_user = FixMyStreet::App->model('DB::User')->find_or_create(
     }
 );
 
-my $council_list = FixMyStreet::App->model('DB::Body');
+my $body_list = FixMyStreet::App->model('DB::Body');
 
 my $update = Open311::GetUpdates->new(
-    council_list => $council_list,
+    body_list => $body_list,
     system_user  => $system_user
 )->get_updates;
diff --git a/perllib/FixMyStreet/DB/Result/Problem.pm b/perllib/FixMyStreet/DB/Result/Problem.pm
index a06a339bf..7b5460c51 100644
--- a/perllib/FixMyStreet/DB/Result/Problem.pm
+++ b/perllib/FixMyStreet/DB/Result/Problem.pm
@@ -436,7 +436,7 @@ sub bodies_str_ids {
 
 =head2 bodies
 
-Returns an arrayref of bodies to which a report was sent.
+Returns a hashref of bodies to which a report was sent.
 
 =cut
 
@@ -678,13 +678,16 @@ sub local_coords {
 
 =head2 update_from_open311_service_request
 
-    $p->update_from_open311_service_request( $request, $council_details, $system_user );
+    $p->update_from_open311_service_request( $request, $body, $system_user );
 
-Updates the problem based on information in the passed in open311 request. If the request
-has an older update time than the problem's lastupdate time then nothing happens.
+Updates the problem based on information in the passed in open311 request
+(standard, not the extension that uses GetServiceRequestUpdates) . If the
+request has an older update time than the problem's lastupdate time then
+nothing happens.
 
-Otherwise a comment will be created if there is status update text in the open311 request.
-If the open311 request has a state of closed then the problem will be marked as fixed.
+Otherwise a comment will be created if there is status update text in the
+open311 request. If the open311 request has a state of closed then the problem
+will be marked as fixed.
 
 NB: a comment will always be created if the problem is being marked as fixed.
 
@@ -693,7 +696,7 @@ Fixed problems will not be re-opened by this method.
 =cut
 
 sub update_from_open311_service_request {
-    my ( $self, $request, $council_details, $system_user ) = @_;
+    my ( $self, $request, $body, $system_user ) = @_;
 
     my ( $updated, $status_notes );
 
@@ -716,11 +719,10 @@ sub update_from_open311_service_request {
             mark_fixed => 0,
             user => $system_user,
             anonymous => 0,
-            name => $council_details->{name},
+            name => $body->name,
         }
     );
 
-
     my $w3c = DateTime::Format::W3CDTF->new;
     my $req_time = $w3c->parse_datetime( $request->{updated_datetime} );
 
diff --git a/perllib/Open311/GetUpdates.pm b/perllib/Open311/GetUpdates.pm
index 7441f7f6d..5007a1f82 100644
--- a/perllib/Open311/GetUpdates.pm
+++ b/perllib/Open311/GetUpdates.pm
@@ -4,26 +4,22 @@ use Moose;
 use Open311;
 use FixMyStreet::App;
 
-has council_list => ( is => 'ro' );
+has body_list => ( is => 'ro' );
 has system_user => ( is => 'ro' );
 
 sub get_updates {
     my $self = shift;
 
-    while ( my $council = $self->council_list->next ) {
+    while ( my $body = $self->body_list->next ) {
         my $open311 = Open311->new(
-            endpoint     => $council->endpoint,
-            jurisdiction => $council->jurisdiction,
-            api_key      => $council->api_key
+            endpoint     => $body->endpoint,
+            jurisdiction => $body->jurisdiction,
+            api_key      => $body->api_key
         );
 
-        my $area_id = $council->area_id;
-
-        my $council_details = mySociety::MaPit::call( 'area', $area_id );
-
         my $reports = FixMyStreet::App->model('DB::Problem')->search(
             {
-                council => { like => "\%$area_id\%" },
+                bodies_str => { like => "\%" . $body->id . "\%" },
                 state => { 'IN', [qw/confirmed fixed/] },
                 -and => [
                     external_id => { '!=', undef },
@@ -39,12 +35,12 @@ sub get_updates {
 
         next unless @report_ids;
 
-        $self->update_reports( \@report_ids, $open311, $council_details );
+        $self->update_reports( \@report_ids, $open311, $body );
     }
 }
 
 sub update_reports {
-    my ( $self, $report_ids, $open311, $council_details ) = @_;
+    my ( $self, $report_ids, $open311, $body ) = @_;
 
     my $service_requests = $open311->get_service_requests( $report_ids );
 
@@ -74,7 +70,7 @@ sub update_reports {
         if (my $p = $problem->first) {
             my $cobrand = FixMyStreet::Cobrand->get_class_for_moniker($p->cobrand)->new();
             $cobrand->set_lang_and_domain($p->lang, 1, FixMyStreet->path_to('locale')->stringify );
-            $p->update_from_open311_service_request( $request, $council_details, $self->system_user );
+            $p->update_from_open311_service_request( $request, $body, $self->system_user );
         }
     }
 
diff --git a/t/app/controller/report_new.t b/t/app/controller/report_new.t
index 2993eddac..84258f338 100644
--- a/t/app/controller/report_new.t
+++ b/t/app/controller/report_new.t
@@ -1453,6 +1453,10 @@ subtest "test SeeSomething" => sub {
                 is $mech->uri->path, '/report/new', 'stays on report/new page';
                 $mech->content_contains( 'Your report has been sent', 'use report created template' );
             }
+
+            $user->alerts->delete;
+            $user->problems->delete;
+            $user->delete;
         };
     }
 
diff --git a/t/app/model/problem.t b/t/app/model/problem.t
index fa8cf89ae..24ed959af 100644
--- a/t/app/model/problem.t
+++ b/t/app/model/problem.t
@@ -168,6 +168,10 @@ $problem->insert;
 
 my $tz_local = DateTime::TimeZone->new( name => 'local' );
 
+my $body = FixMyStreet::App->model('DB::Body')->new({
+    name => 'Edinburgh City Council'
+});
+
 for my $test (
     {
         desc => 'request older than problem ignored',
@@ -175,9 +179,6 @@ for my $test (
         request => {
             updated_datetime => DateTime::Format::W3CDTF->new()->format_datetime( DateTime->now()->set_time_zone( $tz_local )->subtract( days => 2 ) ),
         },
-        council => {
-            name => 'Edinburgh City Council',
-        },
         created => 0,
     },
     {
@@ -188,9 +189,6 @@ for my $test (
             status => 'open',
             status_notes => 'this is an update from the council',
         },
-        council => {
-            name => 'Edinburgh City Council',
-        },
         created => 1,
         state => 'confirmed',
         mark_fixed => 0,
@@ -204,9 +202,6 @@ for my $test (
             status => 'closed',
             status_notes => 'the council have fixed this',
         },
-        council => {
-            name => 'Edinburgh City Council',
-        },
         created => 1,
         state => 'fixed',
         mark_fixed => 1,
@@ -220,9 +215,6 @@ for my $test (
             status => 'open',
             status_notes => 'the council do not think this is fixed',
         },
-        council => {
-            name => 'Edinburgh City Council',
-        },
         created => 1,
         start_state => 'fixed',
         state => 'fixed',
@@ -239,7 +231,7 @@ for my $test (
         $problem->update;
         my $w3c = DateTime::Format::W3CDTF->new();
 
-        my $ret = $problem->update_from_open311_service_request( $test->{request}, $test->{council}, $user );
+        my $ret = $problem->update_from_open311_service_request( $test->{request}, $body, $user );
         is $ret, $test->{created}, 'return value';
 
         return unless $test->{created};
@@ -399,9 +391,6 @@ for my $body (
 }
 
 # Let's make some contacts to send things to!
-FixMyStreet::App->model('DB::Contact')->search( {
-    email => { 'like', '%example.com' },
-} )->delete;
 my @contacts;
 for my $contact ( {
     body_id => 2651, # Edinburgh
@@ -760,13 +749,14 @@ subtest 'check reports from abuser not sent' => sub {
     ok $abuse->delete(), 'user removed from abuse table';
 };
 
+END {
+    $problem->comments->delete if $problem;
+    $problem->delete if $problem;
+    $mech->delete_user( $user ) if $user;
 
-$problem->comments->delete;
-$problem->delete;
-$mech->delete_user( $user );
+    foreach (@contacts) {
+        $_->delete;
+    }
 
-foreach (@contacts) {
-    $_->delete;
+    done_testing();
 }
-
-done_testing();
diff --git a/t/open311/getupdates.t b/t/open311/getupdates.t
index 4376e2f4b..7dc7ff164 100644
--- a/t/open311/getupdates.t
+++ b/t/open311/getupdates.t
@@ -15,6 +15,9 @@ my $user = FixMyStreet::App->model('DB::User')->find_or_create(
     }
 );
 
+my $body = FixMyStreet::App->model('DB::Body')->new( {
+    name => 'Test Body',
+} );
 
 my $updates = Open311::GetUpdates->new( system_user => $user );
 ok $updates, 'created object';
@@ -99,7 +102,7 @@ for my $test (
 
         my $o = Open311->new( jurisdiction => 'mysociety', endpoint => 'http://example.com', test_mode => 1, test_get_returns => { 'requests.xml' => $local_requests_xml } );
 
-        ok $updates->update_reports( [ 638344 ], $o, { name => 'Test Council' } );
+        ok $updates->update_reports( [ 638344 ], $o, $body );
         is $o->test_uri_used, 'http://example.com/requests.xml?jurisdiction_id=mysociety&service_request_id=638344', 'get url';
 
         is $problem->comments->count, $test->{comment_count}, 'added a comment';
@@ -174,7 +177,7 @@ subtest 'update with two requests' => sub {
 
     my $o = Open311->new( jurisdiction => 'mysociety', endpoint => 'http://example.com', test_mode => 1, test_get_returns => { 'requests.xml' => $local_requests_xml } );
 
-    ok $updates->update_reports( [ 638344,638345 ], $o, { name => 'Test Council' } );
+    ok $updates->update_reports( [ 638344,638345 ], $o, $body );
     is $o->test_uri_used, 'http://example.com/requests.xml?jurisdiction_id=mysociety&service_request_id=638344%2C638345', 'get url';
 
     is $problem->comments->count, 1, 'added a comment to first problem';
@@ -228,7 +231,7 @@ subtest 'test translation of auto-added comment from old-style Open311 update' =
     FixMyStreet::override_config {
         ALLOWED_COBRANDS => [ 'fixamingata' ],
     }, sub {
-        ok $updates->update_reports( [ 638346 ], $o, { name => 'Test Council' } );
+        ok $updates->update_reports( [ 638346 ], $o, $body );
     };
     is $o->test_uri_used, 'http://example.com/requests.xml?jurisdiction_id=mysociety&service_request_id=638346', 'get url';
 
diff --git a/t/open311/populate-service-list.t b/t/open311/populate-service-list.t
index f33fc97b4..9fd18b607 100644
--- a/t/open311/populate-service-list.t
+++ b/t/open311/populate-service-list.t
@@ -14,7 +14,7 @@ use_ok( 'Open311::PopulateServiceList' );
 use_ok( 'Open311' );
 
 
-my $processor = Open311::PopulateServiceList->new( council_list => [] );
+my $processor = Open311::PopulateServiceList->new();
 ok $processor, 'created object';
 
 my $body = FixMyStreet::App->model('DB::Body')->find_or_create( {
@@ -38,7 +38,7 @@ subtest 'check basic functionality' => sub {
 
     my $service_list = get_xml_simple_object( get_standard_xml() );
 
-    my $processor = Open311::PopulateServiceList->new( council_list => [] );
+    my $processor = Open311::PopulateServiceList->new();
     $processor->_current_body( $body );
     $processor->process_services( $service_list );
 
@@ -64,7 +64,7 @@ subtest 'check non open311 contacts marked as deleted' => sub {
 
     my $service_list = get_xml_simple_object( get_standard_xml() );
 
-    my $processor = Open311::PopulateServiceList->new( council_list => [] );
+    my $processor = Open311::PopulateServiceList->new();
     $processor->_current_body( $body );
     $processor->process_services( $service_list );
 
@@ -95,7 +95,7 @@ subtest 'check email changed if matching category' => sub {
 
     my $service_list = get_xml_simple_object( get_standard_xml() );
 
-    my $processor = Open311::PopulateServiceList->new( council_list => [] );
+    my $processor = Open311::PopulateServiceList->new();
     $processor->_current_body( $body );
     $processor->process_services( $service_list );
 
@@ -128,7 +128,7 @@ subtest 'check category name changed if updated' => sub {
 
     my $service_list = get_xml_simple_object( get_standard_xml() );
 
-    my $processor = Open311::PopulateServiceList->new( council_list => [] );
+    my $processor = Open311::PopulateServiceList->new();
     $processor->_current_body( $body );
     $processor->process_services( $service_list );
 
@@ -177,7 +177,7 @@ subtest 'check conflicting contacts not changed' => sub {
 
     my $service_list = get_xml_simple_object( get_standard_xml() );
 
-    my $processor = Open311::PopulateServiceList->new( council_list => [] );
+    my $processor = Open311::PopulateServiceList->new();
     $processor->_current_body( $body );
     $processor->process_services( $service_list );
 
@@ -198,7 +198,7 @@ subtest 'check conflicting contacts not changed' => sub {
 };
 
 subtest 'check meta data population' => sub {
-    my $processor = Open311::PopulateServiceList->new( council_list => [] );
+    my $processor = Open311::PopulateServiceList->new();
 
     my $meta_xml = '<?xml version="1.0" encoding="utf-8"?>
 <service_definition>
@@ -377,7 +377,7 @@ for my $test (
     },
 ) {
     subtest $test->{desc} => sub {
-        my $processor = Open311::PopulateServiceList->new( council_list => [] );
+        my $processor = Open311::PopulateServiceList->new();
 
         my $services_xml = '<?xml version="1.0" encoding="utf-8"?>
     <services>
@@ -434,7 +434,7 @@ for my $test (
 }
 
 subtest 'check attribute ordering' => sub {
-    my $processor = Open311::PopulateServiceList->new( council_list => [] );
+    my $processor = Open311::PopulateServiceList->new();
 
     my $meta_xml = '<?xml version="1.0" encoding="utf-8"?>
 <service_definition>
@@ -536,7 +536,7 @@ subtest 'check attribute ordering' => sub {
 };
 
 subtest 'check bromely skip code' => sub {
-    my $processor = Open311::PopulateServiceList->new( council_list => [] );
+    my $processor = Open311::PopulateServiceList->new();
 
     my $meta_xml = '<?xml version="1.0" encoding="utf-8"?>
 <service_definition>