shuffle problem update code about to make it easier to test and write some tests

4 files changed, 296 insertions, 62 deletions

diff --git a/bin/open311-update-reports b/bin/open311-update-reports
index 3dfb89feb..41c9c4546 100644
--- a/bin/open311-update-reports
+++ b/bin/open311-update-reports
@@ -2,7 +2,7 @@
 
 use strict;
 use warnings;
-use Open311;
+use Open311::GetUpdates;
 use FixMyStreet::App;
 
 # FIXME - make this configurable and/or better
@@ -15,63 +15,7 @@ my $system_user = FixMyStreet::App->model('DB::User')->find_or_create(
 
 my $council_list = FixMyStreet::App->model('DB::Open311conf');
 
-while ( my $council = $council_list->next ) {
-
-    my $open311 = Open311->new(
-        endpoint     => $council->endpoint,
-        jurisdiction => $council->jurisdiction,
-        api_key      => $council->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\%" },
-            state => { 'IN', [qw/confirmed fixed/] },
-            -and => [
-                external_id => { '!=', undef },
-                external_id => { '!=', '' },
-            ],
-        }
-    );
-
-    my @report_ids = ();
-    while ( my $report = $reports->next ) {
-        push @report_ids, $report->external_id;
-    }
-
-    next unless @report_ids;
-
-    my $service_requests = $open311->get_service_requests( \@report_ids );
-
-    my $requests;
-
-    # XML::Simple is a bit inconsistent in how it structures
-    # things depending on the number of children an element has :(
-    if ( ref $service_requests->{request} eq 'ARRAY' ) {
-        $requests = $service_requests->{request};
-    }
-    else {
-        $requests = [ $service_requests->{request} ];
-    }
-
-    for my $request (@$requests) {
-        # if it's a ref that means it's an empty element
-        # however, if there's no updated date then we can't
-        # tell if it's newer that what we have so we should skip it
-        next if ref $request->{updated_datetime} || ! exists $request->{updated_datetime};
-
-        my $request_id = $request->{service_request_id};
-
-        my $problem =
-          FixMyStreet::App->model('DB::Problem')
-          ->search( { external_id => $request_id, } );
-
-        if (my $p = $problem->first) {
-            $p->update_from_open311_service_request( $request, $council_details );
-        }
-    }
-}
+my $update = Open311::GetUpdates->new(
+    council_list => $council_list,
+    system_user  => $system_user
+)->get_updates;
diff --git a/perllib/Open311.pm b/perllib/Open311.pm
index 9a9ac5eb4..8c26a5cfc 100644
--- a/perllib/Open311.pm
+++ b/perllib/Open311.pm
@@ -10,6 +10,9 @@ use HTTP::Request::Common qw(POST);
 has jurisdiction => ( is => 'ro', isa => 'Str' );;
 has api_key => ( is => 'ro', isa => 'Str' );
 has endpoint => ( is => 'ro', isa => 'Str' );
+has test_mode => ( is => 'ro', isa => 'Bool' );
+has test_uri_used => ( is => 'rw', 'isa' => 'Str' );
+has test_get_returns => ( is => 'rw' );
 
 sub get_service_list {
     my $self = shift;
@@ -125,7 +128,13 @@ sub _get {
     $uri->path( $uri->path . $path );
     $uri->query_form( $params );
 
-    my $content = get( $uri->as_string );
+    my $content;
+    if ( $self->test_mode ) {
+        $content = $self->test_get_returns->{ $path };
+        $self->test_uri_used( $uri->as_string );
+    } else {
+        $content = get( $uri->as_string );
+    }
 
     return $content;
 }
diff --git a/perllib/Open311/GetUpdates.pm b/perllib/Open311/GetUpdates.pm
new file mode 100644
index 000000000..5fecaf89c
--- /dev/null
+++ b/perllib/Open311/GetUpdates.pm
@@ -0,0 +1,83 @@
+package Open311::GetUpdates;
+
+use Moose;
+use Open311;
+use FixMyStreet::App;
+
+has council_list => ( is => 'ro' );
+has system_user => ( is => 'ro' );
+
+sub get_updates {
+    my $self = shift;
+
+    while ( my $council = $self->council_list->next ) {
+        my $open311 = Open311->new(
+            endpoint     => $council->endpoint,
+            jurisdiction => $council->jurisdiction,
+            api_key      => $council->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\%" },
+                state => { 'IN', [qw/confirmed fixed/] },
+                -and => [
+                    external_id => { '!=', undef },
+                    external_id => { '!=', '' },
+                ],
+            }
+        );
+
+        my @report_ids = ();
+        while ( my $report = $reports->next ) {
+            push @report_ids, $report->external_id;
+        }
+
+        next unless @report_ids;
+
+        $self->update_reports( \@report_ids, $open311, $council_details );
+    }
+}
+
+sub update_reports {
+    my ( $self, $report_ids, $open311, $council_details ) = @_;
+
+    my $service_requests = $open311->get_service_requests( $report_ids );
+
+    my $requests;
+
+    # XML::Simple is a bit inconsistent in how it structures
+    # things depending on the number of children an element has :(
+    if ( ref $service_requests->{request} eq 'ARRAY' ) {
+        $requests = $service_requests->{request};
+    }
+    else {
+        $requests = [ $service_requests->{request} ];
+    }
+
+    for my $request (@$requests) {
+        # if it's a ref that means it's an empty element
+        # however, if there's no updated date then we can't
+        # tell if it's newer that what we have so we should skip it
+        next if ref $request->{updated_datetime} || ! exists $request->{updated_datetime};
+
+        my $request_id = $request->{service_request_id};
+
+        my $problem =
+          FixMyStreet::App->model('DB::Problem')
+          ->search( { external_id => $request_id, } );
+
+        if (my $p = $problem->first) {
+            warn 'updating problem ' . $p->id;
+            $p->update_from_open311_service_request( $request, $council_details, $self->system_user );
+        }
+    }
+
+    return 1;
+}
+
+1;
diff --git a/t/open311/getupdates.t b/t/open311/getupdates.t
new file mode 100644
index 000000000..03af0b72b
--- /dev/null
+++ b/t/open311/getupdates.t
@@ -0,0 +1,198 @@
+#!/usr/bin/env perl
+
+use strict;
+use warnings;
+use Test::More;
+
+use FindBin;
+use lib "$FindBin::Bin/../perllib";
+use lib "$FindBin::Bin/../commonlib/perllib";
+
+use_ok( 'Open311::GetUpdates' );
+use_ok( 'Open311' );
+
+my $user = FixMyStreet::App->model('DB::User')->find_or_create(
+    {
+        email => 'system_user@example.com'
+    }
+);
+
+
+my $updates = Open311::GetUpdates->new( system_user => $user );
+ok $updates, 'created object';
+
+my $requests_xml = qq{<?xml version="1.0" encoding="utf-8"?>
+<service_requests>
+<request>
+<service_request_id>638344</service_request_id>
+<status>open</status>
+<status_notes>This is a note.</status_notes>
+<service_name>Sidewalk and Curb Issues</service_name>
+<service_code>006</service_code>
+<description></description>
+<agency_responsible></agency_responsible>
+<service_notice></service_notice>
+<requested_datetime>2010-04-14T06:37:38-08:00</requested_datetime>
+UPDATED_DATETIME
+<expected_datetime>2010-04-15T06:37:38-08:00</expected_datetime>
+<lat>37.762221815</lat>
+<long>-122.4651145</long>
+</request>
+</service_requests>
+};
+
+my $problem_rs = FixMyStreet::App->model('DB::Problem');
+my $problem = $problem_rs->new(
+    {
+        postcode     => 'EH99 1SP',
+        latitude     => 1,
+        longitude    => 1,
+        areas        => 1,
+        title        => '',
+        detail       => '',
+        used_map     => 1,
+        user_id      => 1,
+        name         => '',
+        state        => 'confirmed',
+        service      => '',
+        cobrand      => 'default',
+        cobrand_data => '',
+        user         => $user,
+        created      => DateTime->now()->subtract( days => 1 ),
+        lastupdate   => DateTime->now()->subtract( days => 1 ),
+        anonymous    => 1,
+        external_id  => 638344,
+    }
+);
+
+$problem->insert;
+
+for my $test (
+    {
+        desc => 'element missing',
+        updated_datetime => '',
+        comment_count => 0,
+    },
+    {
+        desc => 'empty element',
+        updated_datetime => '<updated_datetime />',
+        comment_count => 0,
+    },
+    {
+        desc => 'element with no content',
+        updated_datetime => '<updated_datetime></updated_datetime>',
+        comment_count => 0,
+    },
+    {
+        desc => 'element with old content',
+        updated_datetime => sprintf( '<updated_datetime>%s</updated_datetime>', DateTime->now->subtract( days => 3 ) ),
+        comment_count => 0,
+    },
+    {
+        desc => 'element with new content',
+        updated_datetime => sprintf( '<updated_datetime>%s</updated_datetime>', DateTime->now ),
+        comment_count => 1,
+    },
+) {
+    subtest $test->{desc} => sub {
+        $problem->comments->delete;
+        $problem->lastupdate(DateTime->now()->subtract( days => 1 ) ),
+        $problem->update;
+
+        my $local_requests_xml = $requests_xml;
+        $local_requests_xml =~ s/UPDATED_DATETIME/$test->{updated_datetime}/;
+
+        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' } );
+        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';
+    };
+}
+
+$requests_xml = qq{<?xml version="1.0" encoding="utf-8"?>
+<service_requests>
+<request>
+<service_request_id>638344</service_request_id>
+<status>open</status>
+<status_notes>This is a note.</status_notes>
+<service_name>Sidewalk and Curb Issues</service_name>
+<service_code>006</service_code>
+<description></description>
+<agency_responsible></agency_responsible>
+<service_notice></service_notice>
+<requested_datetime>2010-04-14T06:37:38-08:00</requested_datetime>
+<updated_datetime>UPDATED_DATETIME</updated_datetime>
+<expected_datetime>2010-04-15T06:37:38-08:00</expected_datetime>
+<lat>37.762221815</lat>
+<long>-122.4651145</long>
+</request>
+<request>
+<service_request_id>638345</service_request_id>
+<status>open</status>
+<status_notes>This is a for a different issue.</status_notes>
+<service_name>Sidewalk and Curb Issues</service_name>
+<service_code>006</service_code>
+<description></description>
+<agency_responsible></agency_responsible>
+<service_notice></service_notice>
+<requested_datetime>2010-04-14T06:37:38-08:00</requested_datetime>
+<updated_datetime>UPDATED_DATETIME2</updated_datetime>
+<expected_datetime>2010-04-15T06:37:38-08:00</expected_datetime>
+<lat>37.762221815</lat>
+<long>-122.4651145</long>
+</request>
+</service_requests>
+};
+
+my $problem2 = $problem_rs->new(
+    {
+        postcode     => 'EH99 1SP',
+        latitude     => 1,
+        longitude    => 1,
+        areas        => 1,
+        title        => '',
+        detail       => '',
+        used_map     => 1,
+        user_id      => 1,
+        name         => '',
+        state        => 'confirmed',
+        service      => '',
+        cobrand      => 'default',
+        cobrand_data => '',
+        user         => $user,
+        created      => DateTime->now()->subtract( days => 1 ),
+        lastupdate   => DateTime->now()->subtract( days => 1 ),
+        anonymous    => 1,
+        external_id  => 638345,
+    }
+);
+
+$problem->comments->delete;
+subtest 'update with two requests' => sub {
+    $problem->comments->delete;
+    $problem->lastupdate(DateTime->now()->subtract( days => 1 ) ),
+
+    my $date1 = DateTime::Format::W3CDTF->new->formate_datetime( DateTime->now() );
+    my $date2 = DateTime::Format::W3CDTF->new->formate_datetime( DateTime->now->subtract( hour => 1) );
+    my $local_requests_xml = $requests_xml;
+    $local_requests_xml =~ s/UPDATED_DATETIME2/$date2/;
+    $local_requests_xml =~ s/UPDATED_DATETIME/$date1/;
+
+    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' } );
+    is $o->test_uri_used, 'http://example.com/requests.xml?jurisdiction_id=mysociety&service_request_id=638344,638345', 'get url';
+
+    is $problem->comments->count, 1, 'added a comment to first problem';
+    is $problem2->comments->count, 1, 'added a comment to second problem';
+};
+
+$problem->comments->delete;
+$problem->delete;
+$user->comments->delete;
+$user->problems->delete;
+$user->delete;
+
+done_testing();