diff --git a/perllib/FixMyStreet/Cobrand/Brent.pm b/perllib/FixMyStreet/Cobrand/Brent.pm
index 93334f0f6d2..021155b0d11 100644
--- a/perllib/FixMyStreet/Cobrand/Brent.pm
+++ b/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);
}
});
}
diff --git a/perllib/FixMyStreet/Cobrand/Bromley.pm b/perllib/FixMyStreet/Cobrand/Bromley.pm
index 03a9ba9c350..cc7febd89fc 100644
--- a/perllib/FixMyStreet/Cobrand/Bromley.pm
+++ b/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);
}
});
}
diff --git a/perllib/FixMyStreet/Roles/CobrandEcho.pm b/perllib/FixMyStreet/Roles/CobrandEcho.pm
index 5ebfd31a834..64541e8e4c7 100644
--- a/perllib/FixMyStreet/Roles/CobrandEcho.pm
+++ b/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
diff --git a/perllib/FixMyStreet/Roles/CobrandSLWP.pm b/perllib/FixMyStreet/Roles/CobrandSLWP.pm
index 2bd20dadb3b..1f2b5d81395 100644
--- a/perllib/FixMyStreet/Roles/CobrandSLWP.pm
+++ b/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);
}
});
}
diff --git a/perllib/FixMyStreet/SendReport/Open311.pm b/perllib/FixMyStreet/SendReport/Open311.pm
index f4bdfcea355..83187efe6af 100644
--- a/perllib/FixMyStreet/SendReport/Open311.pm
+++ b/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;
diff --git a/t/app/controller/waste_bromley_bulky.t b/t/app/controller/waste_bromley_bulky.t
index edb48b98606..e3e889d5384 100644
--- a/t/app/controller/waste_bromley_bulky.t
+++ b/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',
diff --git a/t/app/controller/waste_kingston_r.t b/t/app/controller/waste_kingston_r.t
index 751a70441a7..983f820140e 100644
--- a/t/app/controller/waste_kingston_r.t
+++ b/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) = @_;
diff --git a/t/app/controller/waste_sutton_r.t b/t/app/controller/waste_sutton_r.t
index 56c872c49db..41503f93e06 100644
--- a/t/app/controller/waste_sutton_r.t
+++ b/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) = @_;
diff --git a/t/cobrand/brent.t b/t/cobrand/brent.t
index 04b0659e322..58ba91e7be9 100644
--- a/t/cobrand/brent.t
+++ b/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;
}
diff --git a/t/cobrand/bromley.t b/t/cobrand/bromley.t
index 1ff0237b80b..cdcf9f6a808 100644
--- a/t/cobrand/bromley.t
+++ b/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',
diff --git a/t/cobrand/bromley_waste.t b/t/cobrand/bromley_waste.t
index 49f625d5542..054a66d2e4b 100644
--- a/t/cobrand/bromley_waste.t
+++ b/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(
diff --git a/t/cobrand/sutton.t b/t/cobrand/sutton.t
index 4903e1b9f6a..ba8a64d766c 100644
--- a/t/cobrand/sutton.t
+++ b/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', '
Duplicate Event! Original eventID: 123', 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', '
Internal error', 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', '
Internal error', 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');
});
diff --git a/t/sendreport/open311.t b/t/sendreport/open311.t
index d50da9835bf..60878b598fe 100644
--- a/t/sendreport/open311.t
+++ b/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