From aabed59c3822f6053e131d28e2b66ea363ca215e Mon Sep 17 00:00:00 2001
From: Struan Donald <struan@exo.org.uk>
Date: Tue, 9 Apr 2019 13:00:50 +0100
Subject: [Open311] handle bad FixMyStreet id in updates

If an update has a fixmystreet id in it check that it looks like an
integer and if not issue a warning and skip the update.
---
 perllib/Open311/GetServiceRequestUpdates.pm |  5 +++++
 t/open311/getservicerequestupdates.t        | 35 +++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/perllib/Open311/GetServiceRequestUpdates.pm b/perllib/Open311/GetServiceRequestUpdates.pm
index 06b5ce321..263877e0a 100644
--- a/perllib/Open311/GetServiceRequestUpdates.pm
+++ b/perllib/Open311/GetServiceRequestUpdates.pm
@@ -108,6 +108,11 @@ sub update_comments {
 
         # in some cases we only have the FMS id and not the request id so use that
         if ( $request->{fixmystreet_id} ) {
+            unless ( $request->{fixmystreet_id} =~ /^\d+$/ ) {
+                warn "skipping bad fixmystreet id in updates for " . $body->name . ": [" . $request->{fixmystreet_id} . "], external id is $request_id\n";
+                next;
+            }
+
             $criteria = {
                 id => $request->{fixmystreet_id},
             };
diff --git a/t/open311/getservicerequestupdates.t b/t/open311/getservicerequestupdates.t
index 055c5ea5f..573e23cfc 100644
--- a/t/open311/getservicerequestupdates.t
+++ b/t/open311/getservicerequestupdates.t
@@ -4,6 +4,7 @@ use FixMyStreet::Test;
 use Test::Output;
 use CGI::Simple;
 use LWP::Protocol::PSGI;
+use Test::Warn;
 use t::Mock::Static;
 
 use_ok( 'Open311' );
@@ -1091,6 +1092,40 @@ subtest 'check matching on fixmystreet_id overrides service_request_id' => sub {
     is $comments[0]->external_id, 638344, "correct first comment added";
     is $comments[1]->external_id, 638356, "correct second comment added";
 };
+
+subtest 'check bad fixmystreet_id is handled' => sub {
+    my $requests_xml = qq{<?xml version="1.0" encoding="utf-8"?>
+    <service_requests_updates>
+    <request_update>
+    <update_id>638344</update_id>
+    <service_request_id>8888888888888</service_request_id>
+    <fixmystreet_id>123456 654321</fixmystreet_id>
+    <status>open</status>
+    <description>This is a note</description>
+    <updated_datetime>UPDATED_DATETIME</updated_datetime>
+    </request_update>
+    </service_requests_updates>
+    };
+
+    $problem->comments->delete;
+
+    $requests_xml =~ s/UPDATED_DATETIME/$dt/;
+
+    my $o = Open311->new( jurisdiction => 'mysociety', endpoint => 'http://example.com', test_mode => 1, test_get_returns => { 'servicerequestupdates.xml' => $requests_xml } );
+
+    my $update = Open311::GetServiceRequestUpdates->new(
+        system_user => $user,
+    );
+
+    warning_like {
+        $update->update_comments( $o, $bodies{2482} )
+    }
+    qr/skipping bad fixmystreet id in updates for Bromley: \[123456 654321\], external id is 8888888888888/,
+    "warning emitted for bad fixmystreet id";
+
+    $problem->discard_changes;
+    is $problem->comments->count, 0, 'no comments after fetching updates';
+};
 done_testing();
 
 sub setup_xml {
-- 
cgit v1.2.3