aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--CHANGELOG.md2
-rw-r--r--perllib/FixMyStreet/Cobrand/Bristol.pm9
-rw-r--r--perllib/FixMyStreet/Cobrand/Bromley.pm5
-rw-r--r--perllib/FixMyStreet/Cobrand/Default.pm1
-rw-r--r--perllib/FixMyStreet/Cobrand/Greenwich.pm9
-rw-r--r--perllib/FixMyStreet/Cobrand/Oxfordshire.pm6
-rw-r--r--perllib/FixMyStreet/Cobrand/UK.pm3
-rw-r--r--perllib/Open311/GetServiceRequestUpdates.pm14
-rwxr-xr-xperllib/Open311/PostServiceRequestUpdates.pm4
-rw-r--r--t/app/controller/report_new_open311.t3
-rw-r--r--t/cobrand/bristol.t66
-rw-r--r--t/cobrand/greenwich.t114
-rw-r--r--t/open311.t16
-rw-r--r--t/open311/getservicerequestupdates.t54
-rw-r--r--t/open311/populate-service-list.t27
-rw-r--r--t/open311/post-service-request-updates.t4
-rw-r--r--templates/web/base/report/new/category_extras_fields.html2
17 files changed, 323 insertions, 16 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index eeea670eb..f3b2e82a5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -30,6 +30,8 @@
- Improve inline checkbox spacing. #2411
- Prevent duplicate contact history creation with Unicode data.
- Show all Open311 extra fields in edit admin.
+ - Proper bodies check for sending updates.
+ - Check better if extra question has values.
- Development improvements:
- Make front page cache time configurable.
- Better working of /fakemapit/ under https.
diff --git a/perllib/FixMyStreet/Cobrand/Bristol.pm b/perllib/FixMyStreet/Cobrand/Bristol.pm
index 0660acc79..fa2d3fabb 100644
--- a/perllib/FixMyStreet/Cobrand/Bristol.pm
+++ b/perllib/FixMyStreet/Cobrand/Bristol.pm
@@ -75,4 +75,13 @@ sub open311_config {
$params->{always_send_email} = 1;
}
+sub open311_contact_meta_override {
+ my ($self, $service, $contact, $meta) = @_;
+
+ my %server_set = (easting => 1, northing => 1);
+ foreach (@$meta) {
+ $_->{automated} = 'server_set' if $server_set{$_->{code}};
+ }
+}
+
1;
diff --git a/perllib/FixMyStreet/Cobrand/Bromley.pm b/perllib/FixMyStreet/Cobrand/Bromley.pm
index 2ae498b38..341fb6a30 100644
--- a/perllib/FixMyStreet/Cobrand/Bromley.pm
+++ b/perllib/FixMyStreet/Cobrand/Bromley.pm
@@ -252,6 +252,11 @@ sub open311_contact_meta_override {
$contact->set_extra_metadata( id_field => 'service_request_id_ext');
+ my %server_set = (easting => 1, northing => 1, service_request_id_ext => 1);
+ foreach (@$meta) {
+ $_->{automated} = 'server_set' if $server_set{$_->{code}};
+ }
+
# Lights we want to store feature ID, PROW on all categories.
push @$meta, {
code => 'prow_reference',
diff --git a/perllib/FixMyStreet/Cobrand/Default.pm b/perllib/FixMyStreet/Cobrand/Default.pm
index 53d25cec2..785177b5e 100644
--- a/perllib/FixMyStreet/Cobrand/Default.pm
+++ b/perllib/FixMyStreet/Cobrand/Default.pm
@@ -1182,6 +1182,7 @@ Return true if an Open311 service attribute should be a hidden field.
sub category_extra_hidden {
my ($self, $meta) = @_;
+ return 1 if ($meta->{automated} || '') eq 'hidden_field';
return 0;
}
diff --git a/perllib/FixMyStreet/Cobrand/Greenwich.pm b/perllib/FixMyStreet/Cobrand/Greenwich.pm
index 6ff30e83d..2aaa5d776 100644
--- a/perllib/FixMyStreet/Cobrand/Greenwich.pm
+++ b/perllib/FixMyStreet/Cobrand/Greenwich.pm
@@ -66,4 +66,13 @@ sub open311_config {
$row->set_extra_fields( @$extra );
}
+sub open311_contact_meta_override {
+ my ($self, $service, $contact, $meta) = @_;
+
+ my %server_set = (easting => 1, northing => 1, closest_address => 1);
+ foreach (@$meta) {
+ $_->{automated} = 'server_set' if $server_set{$_->{code}};
+ }
+}
+
1;
diff --git a/perllib/FixMyStreet/Cobrand/Oxfordshire.pm b/perllib/FixMyStreet/Cobrand/Oxfordshire.pm
index 5def2bb61..08482a0b3 100644
--- a/perllib/FixMyStreet/Cobrand/Oxfordshire.pm
+++ b/perllib/FixMyStreet/Cobrand/Oxfordshire.pm
@@ -178,14 +178,12 @@ sub open311_config {
my $extra = $row->get_extra_fields;
push @$extra, { name => 'external_id', value => $row->id };
+ push @$extra, { name => 'northing', value => $h->{northing} };
+ push @$extra, { name => 'easting', value => $h->{easting} };
if ($h->{closest_address}) {
push @$extra, { name => 'closest_address', value => "$h->{closest_address}" }
}
- if ( $row->used_map || ( !$row->used_map && !$row->postcode ) ) {
- push @$extra, { name => 'northing', value => $h->{northing} };
- push @$extra, { name => 'easting', value => $h->{easting} };
- }
$row->set_extra_fields( @$extra );
$params->{extended_description} = 'oxfordshire';
diff --git a/perllib/FixMyStreet/Cobrand/UK.pm b/perllib/FixMyStreet/Cobrand/UK.pm
index 1202d48a4..0eb350311 100644
--- a/perllib/FixMyStreet/Cobrand/UK.pm
+++ b/perllib/FixMyStreet/Cobrand/UK.pm
@@ -394,8 +394,7 @@ sub lookup_by_ref_regex {
sub category_extra_hidden {
my ($self, $meta) = @_;
return 1 if $meta->{code} eq 'usrn' || $meta->{code} eq 'asset_id';
- return 1 if $meta->{automated} eq 'hidden_field';
- return 0;
+ return $self->SUPER::category_extra_hidden($meta);
}
1;
diff --git a/perllib/Open311/GetServiceRequestUpdates.pm b/perllib/Open311/GetServiceRequestUpdates.pm
index 1d07e7897..06b5ce321 100644
--- a/perllib/Open311/GetServiceRequestUpdates.pm
+++ b/perllib/Open311/GetServiceRequestUpdates.pm
@@ -91,7 +91,7 @@ sub update_comments {
# If there's no request id then we can't work out
# what problem it belongs to so just skip
- next unless $request_id;
+ next unless $request_id || $request->{fixmystreet_id};
my $comment_time = eval {
DateTime::Format::W3CDTF->parse_datetime( $request->{updated_datetime} || "" );
@@ -101,9 +101,19 @@ sub update_comments {
next if @args && ($updated lt $args[0] || $updated gt $args[1]);
my $problem;
+ my $match_field = 'external_id';
my $criteria = {
external_id => $request_id,
};
+
+ # in some cases we only have the FMS id and not the request id so use that
+ if ( $request->{fixmystreet_id} ) {
+ $criteria = {
+ id => $request->{fixmystreet_id},
+ };
+ $match_field = 'fixmystreet id';
+ }
+
$problem = $self->schema->resultset('Problem')->to_body($body)->search( $criteria );
if (my $p = $problem->first) {
@@ -198,7 +208,7 @@ sub update_comments {
# we get lots of comments that are not related to FMS issues from Lewisham so ignore those otherwise
# way too many warnings.
} elsif (FixMyStreet->config('STAGING_SITE') and $body->name !~ /Lewisham/) {
- warn "Failed to match comment to problem with external_id $request_id for " . $body->name . "\n";
+ warn "Failed to match comment to problem with $match_field $request_id for " . $body->name . "\n";
}
}
diff --git a/perllib/Open311/PostServiceRequestUpdates.pm b/perllib/Open311/PostServiceRequestUpdates.pm
index 252f08aff..1f080b168 100755
--- a/perllib/Open311/PostServiceRequestUpdates.pm
+++ b/perllib/Open311/PostServiceRequestUpdates.pm
@@ -53,18 +53,16 @@ sub process_body {
my $o = Open311->new( $self->open311_params($body) );
- my $comments = FixMyStreet::DB->resultset('Comment')->search( {
+ my $comments = FixMyStreet::DB->resultset('Comment')->to_body($body)->search( {
'me.whensent' => undef,
'me.external_id' => undef,
'me.state' => 'confirmed',
'me.confirmed' => { '!=' => undef },
'problem.whensent' => { '!=' => undef },
'problem.external_id' => { '!=' => undef },
- 'problem.bodies_str' => { -like => '%' . $body->id . '%' },
'problem.send_method_used' => { -like => '%Open311%' },
},
{
- join => 'problem',
order_by => [ 'confirmed', 'id' ],
}
);
diff --git a/t/app/controller/report_new_open311.t b/t/app/controller/report_new_open311.t
index 22b4409b5..dc9a26791 100644
--- a/t/app/controller/report_new_open311.t
+++ b/t/app/controller/report_new_open311.t
@@ -267,7 +267,7 @@ subtest "Category extras includes description label for user" => sub {
ALLOWED_COBRANDS => [ { fixmystreet => '.' } ],
MAPIT_URL => 'http://mapit.uk/',
}, sub {
- $contact4->push_extra_fields({ description => 'Size?', code => 'size', required => 'true', automated => '', variable => 'true', order => '3' });
+ $contact4->push_extra_fields({ description => 'Size?', code => 'size', required => 'true', automated => '', variable => 'true', order => '3', values => undef });
$contact4->update;
for (
{ url => '/report/new/ajax?' },
@@ -280,6 +280,7 @@ subtest "Category extras includes description label for user" => sub {
lacks_string($category_extra, "USRN", "Lacks 'USRN' label");
lacks_string($category_extra, "Asset ID", "Lacks 'Asset ID' label");
contains_string($category_extra, "Size?");
+ lacks_string($category_extra, '<option value=""');
contains_string($category_extra, "resolve your problem quicker, by providing some extra detail", "Contains description text");
}
};
diff --git a/t/cobrand/bristol.t b/t/cobrand/bristol.t
index b4b6ed4ac..b2b8cff13 100644
--- a/t/cobrand/bristol.t
+++ b/t/cobrand/bristol.t
@@ -1,6 +1,8 @@
use FixMyStreet::TestMech;
my $mech = FixMyStreet::TestMech->new;
+use Open311::PopulateServiceList;
+
# Create test data
my $body = $mech->create_body_ok( 2561, 'Bristol County Council', {
send_method => 'Open311',
@@ -41,4 +43,68 @@ subtest 'All categories are shown on FMS cobrand', sub {
};
};
+subtest 'check services override' => sub {
+ my $processor = Open311::PopulateServiceList->new();
+
+ my $meta_xml = '<?xml version="1.0" encoding="utf-8"?>
+<service_definition>
+ <service_code>LIGHT</service_code>
+ <attributes>
+ <attribute>
+ <variable>true</variable>
+ <code>easting</code>
+ <datatype>string</datatype>
+ <required>true</required>
+ <order>1</order>
+ <description>Easting</description>
+ </attribute>
+ <attribute>
+ <variable>true</variable>
+ <code>size</code>
+ <datatype>string</datatype>
+ <required>true</required>
+ <order>2</order>
+ <description>How big is the pothole</description>
+ </attribute>
+ </attributes>
+</service_definition>
+ ';
+
+ my $o = Open311->new(
+ jurisdiction => 'mysociety',
+ endpoint => 'http://example.com',
+ test_mode => 1,
+ test_get_returns => { 'services/LIGHT.xml' => $meta_xml }
+ );
+
+ $processor->_current_open311( $o );
+ FixMyStreet::override_config {
+ ALLOWED_COBRANDS => [ 'bristol' ],
+ }, sub {
+ $processor->_current_body( $body );
+ };
+ $processor->_current_service( { service_code => 'LIGHT' } );
+ $processor->_add_meta_to_contact( $open311_contact );
+
+ my $extra = [ {
+ automated => 'server_set',
+ variable => 'true',
+ code => 'easting',
+ datatype => 'string',
+ required => 'true',
+ order => 1,
+ description => 'Easting',
+ }, {
+ variable => 'true',
+ code => 'size',
+ datatype => 'string',
+ required => 'true',
+ order => 2,
+ description => 'How big is the pothole',
+ } ];
+
+ $open311_contact->discard_changes;
+ is_deeply $open311_contact->get_extra_fields, $extra, 'Easting has automated set';
+};
+
done_testing();
diff --git a/t/cobrand/greenwich.t b/t/cobrand/greenwich.t
new file mode 100644
index 000000000..e6aaca973
--- /dev/null
+++ b/t/cobrand/greenwich.t
@@ -0,0 +1,114 @@
+use CGI::Simple;
+use FixMyStreet::TestMech;
+use FixMyStreet::Script::Reports;
+use Open311::PopulateServiceList;
+
+my $mech = FixMyStreet::TestMech->new;
+
+my $body = $mech->create_body_ok( 2493, 'Greenwich Council', {
+ send_method => 'Open311',
+ endpoint => 'endpoint',
+ api_key => 'key',
+ jurisdiction => 'greenwich',
+});
+
+my $contact = $mech->create_contact_ok(
+ body_id => $body->id,
+ category => 'Pothole',
+ email => 'HOLE',
+);
+
+my $user = $mech->create_user_ok( 'greenwich@example.com' );
+my @reports = $mech->create_problems_for_body( 1, $body->id, 'Test', {
+ category => 'Pothole',
+ cobrand => 'greenwich',
+ user => $user,
+});
+my $report = $reports[0];
+
+subtest 'check services override' => sub {
+ my $processor = Open311::PopulateServiceList->new();
+
+ my $meta_xml = '<?xml version="1.0" encoding="utf-8"?>
+<service_definition>
+ <service_code>HOLE</service_code>
+ <attributes>
+ <attribute>
+ <variable>true</variable>
+ <code>easting</code>
+ <datatype>string</datatype>
+ <required>true</required>
+ <order>1</order>
+ <description>Easting</description>
+ </attribute>
+ <attribute>
+ <variable>true</variable>
+ <code>size</code>
+ <datatype>string</datatype>
+ <required>true</required>
+ <order>2</order>
+ <description>How big is the pothole</description>
+ </attribute>
+ </attributes>
+</service_definition>
+ ';
+
+ my $o = Open311->new(
+ jurisdiction => 'mysociety',
+ endpoint => 'http://example.com',
+ test_mode => 1,
+ test_get_returns => { 'services/HOLE.xml' => $meta_xml }
+ );
+
+ $processor->_current_open311( $o );
+ FixMyStreet::override_config {
+ ALLOWED_COBRANDS => [ 'greenwich' ],
+ }, sub {
+ $processor->_current_body( $body );
+ };
+ $processor->_current_service( { service_code => 'HOLE' } );
+ $processor->_add_meta_to_contact( $contact );
+
+ my $extra = [ {
+ automated => 'server_set',
+ variable => 'true',
+ code => 'easting',
+ datatype => 'string',
+ required => 'true',
+ order => 1,
+ description => 'Easting',
+ }, {
+ variable => 'true',
+ code => 'size',
+ datatype => 'string',
+ required => 'true',
+ order => 2,
+ description => 'How big is the pothole',
+ } ];
+
+ $contact->discard_changes;
+ is_deeply $contact->get_extra_fields, $extra, 'Easting has automated set';
+};
+
+subtest 'testing special Open311 behaviour', sub {
+ my $test_data;
+ FixMyStreet::override_config {
+ STAGING_FLAGS => { send_reports => 1 },
+ ALLOWED_COBRANDS => [ 'greenwich' ],
+ MAPIT_URL => 'http://mapit.uk/',
+ }, sub {
+ $test_data = FixMyStreet::Script::Reports::send();
+ };
+ $report->discard_changes;
+ ok $report->whensent, 'Report marked as sent';
+ is $report->send_method_used, 'Open311', 'Report sent via Open311';
+ is $report->external_id, 248, 'Report has right external ID';
+
+ my $req = $test_data->{test_req_used};
+ my $c = CGI::Simple->new($req->content);
+ is $c->param('attribute[external_id]'), $report->id, 'Request had correct ID';
+ is $c->param('attribute[easting]'), 529025, 'Request had correct easting';
+};
+
+done_testing();
+
diff --git a/t/open311.t b/t/open311.t
index 3bb85f595..85176ff0d 100644
--- a/t/open311.t
+++ b/t/open311.t
@@ -263,6 +263,22 @@ for my $test (
};
}
+subtest 'test always_send_email' => sub {
+ my $email = $user->email;
+ $user->email(undef);
+ my $extra = { url => 'http://example.com/report/1', };
+
+ my $results = make_service_req(
+ $problem, $extra, $problem->category,
+ '<?xml version="1.0" encoding="utf-8"?><service_requests><request><service_request_id>248</service_request_id></request></service_requests>',
+ { always_send_email => 1 }
+ );
+ my $c = CGI::Simple->new( $results->{req}->content );
+
+ is $c->param('email'), 'do-not-reply@example.org', 'correct email';
+ $user->email($email);
+};
+
sub make_comment {
my $cobrand = shift;
FixMyStreet::DB->resultset('Comment')->new( {
diff --git a/t/open311/getservicerequestupdates.t b/t/open311/getservicerequestupdates.t
index 54a7f192d..055c5ea5f 100644
--- a/t/open311/getservicerequestupdates.t
+++ b/t/open311/getservicerequestupdates.t
@@ -1037,6 +1037,60 @@ foreach my $test ( {
}
}
+subtest 'check matching on fixmystreet_id overrides service_request_id' => 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>@{[ $problem->id ]}</fixmystreet_id>
+ <status>open</status>
+ <description>This is a note</description>
+ <updated_datetime>UPDATED_DATETIME</updated_datetime>
+ </request_update>
+ <request_update>
+ <update_id>638354</update_id>
+ <service_request_id>@{[ $problem->external_id ]}</service_request_id>
+ <fixmystreet_id>999999999</fixmystreet_id>
+ <status>open</status>
+ <description>This is a different note</description>
+ <updated_datetime>UPDATED_DATETIME2</updated_datetime>
+ </request_update>
+ <request_update>
+ <update_id>638356</update_id>
+ <service_request_id></service_request_id>
+ <fixmystreet_id>@{[ $problem->id ]}</fixmystreet_id>
+ <status>investigating</status>
+ <description>This is a last note</description>
+ <updated_datetime>UPDATED_DATETIME3</updated_datetime>
+ </request_update>
+ </service_requests_updates>
+ };
+
+ $problem->comments->delete;
+
+ my $dt2 = $dt->clone->subtract( minutes => 30 );
+ my $dt3 = $dt2->clone->subtract( minutes => 30 );
+ $requests_xml =~ s/UPDATED_DATETIME3/$dt/;
+ $requests_xml =~ s/UPDATED_DATETIME2/$dt2/;
+ $requests_xml =~ s/UPDATED_DATETIME/$dt3/;
+
+ 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,
+ );
+
+ $update->update_comments( $o, $bodies{2482} );
+
+ $problem->discard_changes;
+ is $problem->comments->count, 2, 'two comments after fetching updates';
+
+ my @comments = $problem->comments->search(undef, { order_by => [ 'created' ] } )->all;
+
+ is $comments[0]->external_id, 638344, "correct first comment added";
+ is $comments[1]->external_id, 638356, "correct second comment added";
+};
done_testing();
sub setup_xml {
diff --git a/t/open311/populate-service-list.t b/t/open311/populate-service-list.t
index 4d70dfebc..ff4c4cf9d 100644
--- a/t/open311/populate-service-list.t
+++ b/t/open311/populate-service-list.t
@@ -584,6 +584,15 @@ subtest 'check Bromley skip code' => sub {
<order>1</order>
<description>Type of bin</description>
</attribute>
+ <attribute>
+ <variable>true</variable>
+ <code>easting</code>
+ <datatype>string</datatype>
+ <required>true</required>
+ <datatype_description>String</datatype_description>
+ <order>1</order>
+ <description>Easting</description>
+ </attribute>
</attributes>
</service_definition>
';
@@ -626,6 +635,15 @@ subtest 'check Bromley skip code' => sub {
order => 1,
description => 'Type of bin'
}, {
+ automated => 'server_set',
+ variable => 'true',
+ code => 'easting',
+ datatype => 'string',
+ required => 'true',
+ datatype_description => 'String',
+ order => 1,
+ description => 'Easting',
+ }, {
automated => 'hidden_field',
variable => 'true',
code => 'prow_reference',
@@ -671,7 +689,14 @@ subtest 'check Bromley skip code' => sub {
datatype_description => 'Type of bin',
order => 1,
description => 'Type of bin'
-
+ }, {
+ variable => 'true',
+ code => 'easting',
+ datatype => 'string',
+ required => 'true',
+ datatype_description => 'String',
+ order => 1,
+ description => 'Easting',
},
];
diff --git a/t/open311/post-service-request-updates.t b/t/open311/post-service-request-updates.t
index 000bf3a2b..57b8f9a2a 100644
--- a/t/open311/post-service-request-updates.t
+++ b/t/open311/post-service-request-updates.t
@@ -15,8 +15,8 @@ my $params = {
endpoint => 'endpoint',
jurisdiction => 'home',
};
-my $bromley = $mech->create_body_ok(2482, 'Bromley', { %$params, send_extended_statuses => 1 });
-my $oxon = $mech->create_body_ok(2237, 'Oxfordshire', $params);
+my $bromley = $mech->create_body_ok(2482, 'Bromley', { %$params, send_extended_statuses => 1, id => 5 });
+my $oxon = $mech->create_body_ok(2237, 'Oxfordshire', { %$params, id => 55 });
my $bucks = $mech->create_body_ok(2217, 'Buckinghamshire', $params);
my $lewisham = $mech->create_body_ok(2492, 'Lewisham', $params);
diff --git a/templates/web/base/report/new/category_extras_fields.html b/templates/web/base/report/new/category_extras_fields.html
index df6129672..dd5c3911d 100644
--- a/templates/web/base/report/new/category_extras_fields.html
+++ b/templates/web/base/report/new/category_extras_fields.html
@@ -14,7 +14,7 @@
<p class='form-error'>[% field_errors.$x_meta_name %]</p>
[% END -%]
[% IF meta.variable != 'false' %]
- [% IF meta.exists('values') %]
+ [% IF meta.item('values').size %]
<select class="form-control" name="[% cat_prefix %][% meta_name %]" id="[% cat_prefix %]form_[% meta_name %]"[% meta.required == 'true' ? ' required' : '' %]>
<option value="">[% loc('-- Pick an option --') %]</option>
[% FOR option IN meta.values %]
generated by cgit v1.2.3 (git 2.25.1) at 2025-04-26 22:05:34 +0000