[Waste] [Echo] Check event before/after sending.

If we get an internal error from Echo when sending, try and look up the
event directly to see if it has actually been created. Also do the same
before sending as an extra check.

perllib/FixMyStreet/Cobrand/Brent.pm

@@ -678,6 +678,17 @@ sub open311_extra_data_exclude {
     return [];
 }
 
+=head2 open311_pre_send
+
+We check in Echo to see if something has already been sent there first.
+
+=cut
+
+sub open311_pre_send {
+    my ($self, $row, $open311) = @_;
+    return 'SENT' if $self->open311_pre_send_check($row, 'FMS');
+}
+
 =head2 open311_post_send
 
 Restore the original detail field if it was changed by open311_extra_data_include
@@ -699,6 +710,8 @@ sub open311_post_send {
         if ($error =~ /Selected reservations expired|Invalid reservation reference/) {
             $self->bulky_refetch_slots($row2);
             $row->discard_changes;
+        } elsif ($error =~ /Internal error/) {
+            $self->open311_post_send_error_check("FMS", $row, $row2, $sender);
         }
     });
 }

perllib/FixMyStreet/Cobrand/Bromley.pm

@@ -261,6 +261,8 @@ sub open311_pre_send {
         my $text = $row->detail . "\n\nPrivate comments: $private_comments";
         $row->detail($text);
     }
+
+    return 'SENT' if $self->open311_pre_send_check($row, 'FMS');
 }
 
 sub _include_user_title_in_extra {
@@ -321,6 +323,8 @@ sub open311_post_send {
         } elsif ($error =~ /Selected reservations expired|Invalid reservation reference/) {
             $self->bulky_refetch_slots($row2);
             $row->discard_changes;
+        } elsif ($error =~ /Internal error/) {
+            $self->open311_post_send_error_check("FMS", $row, $row2, $sender);
         }
     });
 }

perllib/FixMyStreet/Roles/CobrandEcho.pm

@@ -569,6 +569,43 @@ sub admin_templates_external_status_code_hook {
     return $code;
 }
 
+sub open311_pre_send_check {
+    my ($self, $row, $prefix) = @_;
+
+    my $guid = $self->open311_check_existing_event("$prefix-" . $row->id, "ClientReference");
+    if ($guid) {
+        # Event already exists
+        $row->discard_changes;
+        $row->update({ external_id => $guid });
+        return 1;
+    }
+    return 0;
+}
+
+sub open311_post_send_error_check {
+    my ($self, $prefix, $row, $row2, $sender) = @_;
+    $self->open311_post_send_check("$prefix-" . $row->id, 'ClientReference', $row, $row2, $sender);
+}
+
+sub open311_post_send_check {
+    my ($self, $id, $type, $row, $row2, $sender) = @_;
+    if (my $guid = $self->open311_check_existing_event($id, $type)) {
+        $row2->external_id($guid);
+        $sender->success(1);
+        $row2->update;
+        $row->discard_changes;
+    }
+}
+
+sub open311_check_existing_event {
+    my ($self, $id, $type) = @_;
+
+    my $cfg = $self->feature('echo');
+    my $echo = Integrations::Echo->new(%$cfg);
+    my $event = $echo->GetEvent($id, $type) || {};
+    return $event->{Guid};
+}
+
 =item waste_fetch_events
 
 Loop through all open waste events to see if there have been any updates

perllib/FixMyStreet/Roles/CobrandSLWP.pm

@@ -95,6 +95,17 @@ sub open311_extra_data_include {
     return $open311_only;
 }
 
+=item * We check in Echo to see if something has already been sent there first
+
+=cut
+
+sub open311_pre_send {
+    my ($self, $row, $open311) = @_;
+
+    my $prefix = $self->moniker eq 'sutton' ? 'LBS' : 'RBK';
+    return 'SENT' if $self->open311_pre_send_check($row, $prefix);
+}
+
 =item * If Echo errors, we try and deal with standard issues - a renewal on an expired subscription, or a duplicate event
 
 =cut
@@ -120,13 +131,10 @@ sub open311_post_send {
             $row->discard_changes;
         } elsif ($error =~ /Duplicate Event! Original eventID: (\d+)/) {
             my $id = $1;
-            my $cfg = $self->feature('echo');
-            my $echo = Integrations::Echo->new(%$cfg);
-            my $event = $echo->GetEvent($id, 'Id');
-            $row2->external_id($event->{Guid});
-            $sender->success(1);
-            $row2->update;
-            $row->discard_changes;
+            $self->open311_post_send_check($id, "Id", $row, $row2, $sender);
+        } elsif ($error =~ /Internal error/) {
+            my $prefix = $self->moniker eq 'sutton' ? 'LBS' : 'RBK';
+            $self->open311_post_send_error_check($prefix, $row, $row2, $sender);
         }
     });
 }

perllib/FixMyStreet/SendReport/Open311.pm

@@ -77,7 +77,7 @@ sub send {
     my $open311 = Open311->new( %open311_params );
 
     my $skip = $cobrand->call_hook(open311_pre_send => $row, $open311);
-    $skip = $skip && $skip eq 'SKIP';
+    $skip = $skip && ($skip eq 'SKIP' || $skip eq 'SENT');
 
     my $resp;
     if (!$skip) {
@@ -88,7 +88,7 @@ sub send {
     $row->discard_changes;
 
     if ( $skip || $resp ) {
-        $row->update({ external_id => $resp });
+        $row->update({ external_id => $resp }) if $resp;
         $self->success( 1 );
     } else {
         $self->error( "Failed to send over Open311\n" ) unless $self->error;

t/app/controller/waste_bromley_bulky.t

@@ -162,6 +162,7 @@ FixMyStreet::override_config {
     $echo->mock( 'CancelReservedSlotsForEvent', sub { [] } );
     $echo->mock( 'GetTasks', sub { [] } );
     $echo->mock( 'GetEventsForObject', sub { [] } );
+    $echo->mock( 'GetEvent', sub { {} } );
     $echo->mock( 'FindPoints',sub { [
         {
             Description => '2 Example Street, Bromley, BR1 1AF',

t/app/controller/waste_kingston_r.t

@@ -443,6 +443,7 @@ sub shared_echo_mocks {
     });
     $e->mock('GetServiceUnitsForObject', sub { $bin_data });
     $e->mock('GetEventsForObject', sub { [] });
+    $e->mock('GetEvent', sub { {} });
     $e->mock('GetTasks', sub { [] });
     $e->mock( 'CancelReservedSlotsForEvent', sub {
         my (undef, $guid) = @_;

t/app/controller/waste_sutton_r.t

@@ -436,6 +436,7 @@ sub shared_echo_mocks {
     });
     $e->mock('GetServiceUnitsForObject', sub { $bin_data });
     $e->mock('GetEventsForObject', sub { [] });
+    $e->mock('GetEvent', sub { {} });
     $e->mock('GetTasks', sub { [] });
     $e->mock( 'CancelReservedSlotsForEvent', sub {
         my (undef, $guid) = @_;

t/cobrand/brent.t

@@ -195,6 +195,8 @@ create_contact({ category => 'Additional collection', email => 'general@brent.go
     { code => 'service_id', required => 1, automated => 'hidden_field' },
 );
 
+my $echo = shared_echo_mocks();
+
 subtest "title is labelled 'location of problem' in open311 extended description" => sub {
     my ($problem) = $mech->create_problems_for_body(1, $brent->id, 'title', {
         category => 'Graffiti' ,
@@ -672,6 +674,8 @@ FixMyStreet::override_config {
     $mech->host("brent.fixmystreet.com");
 };
 
+undef $echo; # Otherwise tests below fail
+
 package SOAP::Result;
 sub result { return $_[0]->{result}; }
 sub new { my $c = shift; bless { @_ }, $c; }
@@ -880,6 +884,8 @@ FixMyStreet::override_config {
     }
 };
 
+$echo = shared_echo_mocks();
+
 FixMyStreet::override_config {
     ALLOWED_COBRANDS => [ 'brent', 'tfl' ],
     MAPIT_URL => 'http://mapit.uk/',
@@ -1467,6 +1473,7 @@ sub shared_echo_mocks {
         };
     });
     $e->mock('GetEventsForObject', sub { [] });
+    $e->mock('GetEvent', sub { {} });
     $e->mock('GetTasks', sub { [] });
     return $e;
 }

t/cobrand/bromley.t

@@ -148,6 +148,9 @@ subtest 'Updates from staff with no text but with private comments are sent' =>
     };
 };
 
+my $echo = Test::MockModule->new('Integrations::Echo');
+$echo->mock(GetEvent => sub { {} });
+
 for my $test (
     {
         desc => 'testing special Open311 behaviour',

t/cobrand/bromley_waste.t

@@ -93,6 +93,9 @@ subtest 'check footer is powered by SocietyWorks' => sub {
 };
 
 subtest 'test waste duplicate' => sub {
+    my $echo = Test::MockModule->new('Integrations::Echo');
+    $echo->mock(GetEvent => sub { {} });
+
     my $sender = FixMyStreet::SendReport::Open311->new(
         bodies => [ $body ], body_config => { $body->id => $body },
     );
@@ -110,6 +113,9 @@ subtest 'test waste duplicate' => sub {
 };
 
 subtest 'test DD taking so long it expires' => sub {
+    my $echo = Test::MockModule->new('Integrations::Echo');
+    $echo->mock(GetEvent => sub { {} });
+
     my $title = $report->title;
     $report->update({ title => "Garden Subscription - Renew" });
     my $sender = FixMyStreet::SendReport::Open311->new(

t/cobrand/sutton.t

@@ -72,6 +72,9 @@ FixMyStreet::override_config {
     MAPIT_URL => 'http://mapit.uk/',
     STAGING_FLAGS => { send_reports => 1 },
 }, sub {
+    my $echo = Test::MockModule->new('Integrations::Echo');
+    $echo->mock('GetEvent', sub { {} });
+
     subtest 'test waste duplicate' => sub {
         my $sender = FixMyStreet::SendReport::Open311->new(
             bodies => [ $body ], body_config => { $body->id => $body },
@@ -106,11 +109,10 @@ FixMyStreet::override_config {
         my $sender = FixMyStreet::SendReport::Open311->new(
             bodies => [ $body ], body_config => { $body->id => $body },
         );
-        my $echo = Test::MockModule->new('Integrations::Echo');
-        $echo->mock('GetEvent', sub { {
-            Guid => 'a-guid',
-            Id => 123,
-        } } );
+        $echo->mock('GetEvent', sub {
+            return {} if $_[2] eq 'ClientReference';
+            return { Guid => 'a-guid', Id => 123 }
+        });
         Open311->_inject_response('/requests.xml', '<?xml version="1.0" encoding="utf-8"?><errors><error><code></code><description>Duplicate Event! Original eventID: 123</description></error></errors>', 500);
         $sender->send($report, {
             easting => 1,
@@ -121,6 +123,61 @@ FixMyStreet::override_config {
         is $report->external_id, 'a-guid';
     };
 
+    subtest 'test internal error, no event, at the Echo side' => sub {
+        $report->update({ external_id => undef });
+        my $sender = FixMyStreet::SendReport::Open311->new(
+            bodies => [ $body ], body_config => { $body->id => $body },
+        );
+        $echo->mock('GetEvent', sub { {} });
+        Open311->_inject_response('/requests.xml', '<?xml version="1.0" encoding="utf-8"?><errors><error><code></code><description>Internal error</description></error></errors>', 500);
+        $sender->send($report, {
+            easting => 1,
+            northing => 2,
+            url => 'http://example.org/',
+        });
+        is $sender->success, 0;
+        is $report->external_id, undef;
+    };
+
+    subtest 'test internal error, event accepted, at the Echo side' => sub {
+        my $sender = FixMyStreet::SendReport::Open311->new(
+            bodies => [ $body ], body_config => { $body->id => $body },
+        );
+        $echo->mock('GetEvent', sub {
+            my $fn = (caller(2))[3];
+            if ($fn =~ /open311_post_send_check/) {
+                return { Guid => 'c-guid', Id => 123 }
+            } else {
+                return {};
+            }
+        });
+        Open311->_inject_response('/requests.xml', '<?xml version="1.0" encoding="utf-8"?><errors><error><code></code><description>Internal error</description></error></errors>', 500);
+        $sender->send($report, {
+            easting => 1,
+            northing => 2,
+            url => 'http://example.org/',
+        });
+        is $sender->success, 1;
+        is $report->external_id, 'c-guid';
+    };
+
+    subtest 'test event already existing at the Echo side' => sub {
+        my $sender = FixMyStreet::SendReport::Open311->new(
+            bodies => [ $body ], body_config => { $body->id => $body },
+        );
+        $echo->mock('GetEvent', sub {
+            return { Guid => 'b-guid', Id => 123 };
+        });
+        $sender->send($report, {
+            easting => 1,
+            northing => 2,
+            url => 'http://example.org/',
+        });
+        is $sender->success, 1;
+        is $report->external_id, 'b-guid';
+        $echo->mock('GetEvent', sub { {} });
+    };
+
     subtest 'correct payment data sent across' => sub {
         $report->category('Garden Subscription');
         $report->update_extra_field({ name => 'PaymentCode', value => 'Code4321' });
@@ -159,7 +216,7 @@ FixMyStreet::override_config {
         like $body, qr/Dear Sutton Council,\s+A user of FixMyStreet has submitted/;
         like $body, qr{http://www.example.org/report/$id};
     };
-
+    undef $echo;
 };
 
 package SOAP::Result;
@@ -393,6 +450,7 @@ FixMyStreet::override_config {
             bodies => [ $body ], body_config => { $body->id => $body },
         );
         my $echo = Test::MockModule->new('Integrations::Echo');
+        $echo->mock('GetEvent', sub { {} });
         $echo->mock('CancelReservedSlotsForEvent', sub {
             is $_[1], $report->get_extra_field_value('GUID');
         });

t/sendreport/open311.t

@@ -16,6 +16,9 @@ $ukc->mock('lookup_site_code', sub {
     return "Road ID";
 });
 
+my $echo = Test::MockModule->new('Integrations::Echo');
+$echo->mock(GetEvent => sub { {} });
+
 package main;
 sub test_overrides; # defined below