diff --git a/perllib/FixMyStreet/Cobrand/Brent.pm b/perllib/FixMyStreet/Cobrand/Brent.pm index c300fb56e3..c086014652 100644 --- a/perllib/FixMyStreet/Cobrand/Brent.pm +++ b/perllib/FixMyStreet/Cobrand/Brent.pm @@ -707,9 +707,16 @@ 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 'SKIP' if $row->category eq 'Request new container' && $row->get_extra_field_value('request_referral'); + return 'SENT' if $self->open311_pre_send_check($row, 'FMS'); } =head2 open311_post_send @@ -745,6 +752,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 879ef8d7df..167ee100d6 100644 --- a/perllib/FixMyStreet/Cobrand/Bromley.pm +++ b/perllib/FixMyStreet/Cobrand/Bromley.pm @@ -288,6 +288,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 { @@ -348,6 +350,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/Cobrand/Echo.pm b/perllib/FixMyStreet/Roles/Cobrand/Echo.pm index 0b9f1657c2..9ee8bb397e 100644 --- a/perllib/FixMyStreet/Roles/Cobrand/Echo.pm +++ b/perllib/FixMyStreet/Roles/Cobrand/Echo.pm @@ -673,6 +673,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/Cobrand/SLWP.pm b/perllib/FixMyStreet/Roles/Cobrand/SLWP.pm index 3520dae816..73235fe27d 100644 --- a/perllib/FixMyStreet/Roles/Cobrand/SLWP.pm +++ b/perllib/FixMyStreet/Roles/Cobrand/SLWP.pm @@ -478,6 +478,23 @@ sub open311_extra_data_include { return $open311_only; } +sub echo_reference_prefix { + my $self = shift; + return 'LBS' if $self->moniker eq 'sutton'; + return 'RBK' if $self->moniker eq 'kingston'; + return 'MRT' if $self->moniker eq 'merton'; +} + +=item * 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, $self->echo_reference_prefix); +} + =item * If Echo errors, we try and deal with standard issues - a renewal on an expired subscription, or a duplicate event =cut @@ -503,13 +520,9 @@ 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/) { + $self->open311_post_send_error_check($self->echo_reference_prefix, $row, $row2, $sender); } }); } diff --git a/perllib/FixMyStreet/SendReport/Open311.pm b/perllib/FixMyStreet/SendReport/Open311.pm index f4bdfcea35..83187efe6a 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 0549955a87..b48ec1109f 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 a25f10e623..c820461126 100644 --- a/t/app/controller/waste_kingston_r.t +++ b/t/app/controller/waste_kingston_r.t @@ -485,6 +485,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 c433a7e362..4645a22bd7 100644 --- a/t/app/controller/waste_sutton_r.t +++ b/t/app/controller/waste_sutton_r.t @@ -454,6 +454,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 f3b93eb03d..a18a648c61 100644 --- a/t/cobrand/brent.t +++ b/t/cobrand/brent.t @@ -223,6 +223,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' , @@ -698,6 +700,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; } @@ -906,6 +910,8 @@ FixMyStreet::override_config { } }; +$echo = shared_echo_mocks(); + FixMyStreet::override_config { ALLOWED_COBRANDS => [ 'brent', 'tfl' ], MAPIT_URL => 'http://mapit.uk/', @@ -1587,6 +1593,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 b440506047..fe2b6e69a3 100644 --- a/t/cobrand/bromley.t +++ b/t/cobrand/bromley.t @@ -160,6 +160,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 b3aa0adfa0..50698e6337 100644 --- a/t/cobrand/bromley_waste.t +++ b/t/cobrand/bromley_waste.t @@ -94,6 +94,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 }, ); @@ -111,6 +114,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 c305abf3d0..6efeb5161f 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; @@ -402,6 +459,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 d50da9835b..60878b598f 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