aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--CHANGELOG.md2
-rw-r--r--docs/_includes/admin-tasks-content.md11
-rw-r--r--perllib/FixMyStreet/SendReport.pm17
-rw-r--r--perllib/FixMyStreet/SendReport/Email.pm5
-rw-r--r--perllib/FixMyStreet/SendReport/Email/SingleBodyOnly.pm6
-rw-r--r--perllib/FixMyStreet/SendReport/Open311.pm8
-rw-r--r--perllib/FixMyStreet/SendReport/Zurich.pm5
-rw-r--r--perllib/Open311/GetServiceRequestUpdates.pm17
-rw-r--r--t/app/sendreport/email/highways.t1
-rw-r--r--t/app/sendreport/email/tfl.t1
-rw-r--r--t/open311/getservicerequestupdates.t33
-rw-r--r--t/sendreport/open311.t22
12 files changed, 100 insertions, 28 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7ffa67eb5..4b94191ac 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@
- Don't include private reports when searching by ref from front page.
- Set fixmystreet.bodies sooner client-side, for two-tier locations.
- Fix front-end testing script when run with Vagrant.
+ - Handle missing category when sending open311 reports #2502
- Development improvements:
- Upgrade the underlying framework and a number of other packages.
- Add feature cobrand helper function.
@@ -25,6 +26,7 @@
marked private on the site.
- Add new upload_files flag which sends files/photos as part of the
POST service request.
+ - Allow description in email template with placeholder.
* v2.6 (3rd May 2019)
- New features:
diff --git a/docs/_includes/admin-tasks-content.md b/docs/_includes/admin-tasks-content.md
index 3f18a00dc..b04cc01ef 100644
--- a/docs/_includes/admin-tasks-content.md
+++ b/docs/_includes/admin-tasks-content.md
@@ -342,7 +342,7 @@ underway’ and ‘This issue is now closed’.
From the report page, staff with the appropriate permissions may select from the ‘public update’
dropdown. This will prefill an update with template text for one of a number of common statuses.
-The templates are created by the Administrator; see ‘[Creating response templates](#creating-and-editing-priorities)’.
+The templates are created by the Administrator; see ‘[Creating response templates](#creating-editing-response-templates)’.
The text in template responses is fully editable on the report page, so staff may also choose to add
their own comments or edit the preformatted responses to reflect the precise circumstances of the
@@ -379,7 +379,7 @@ details' must be ticked.</span>
#### Setting a priority
From the panel on the right hand side of a report, staff with the appropriate permissions may
select a priority from a drop-down list. These priorities are created by Administrator-level users;
-see ‘[Setting categories and priorities](#creating-and-editing-priorities) ’.
+see ‘[Setting categories and priorities](#creating-editing-priorities) ’.
</div>
@@ -694,8 +694,11 @@ the ‘Resolved’ status update text is automatically applied. While this funct
time-saver, we advise using it with caution to ensure that the template text is applicable to every
situation in which is will be automatically applied.
-If you have an Open311 connection, you can click ‘auto-response’ so that a template will be
-applied when the state is updated by the automated Open311 process.
+If you have an Open311 connection, you can click ‘auto-response’ so that a
+template will be applied when the state is updated by the automated Open311
+process. In this instance, if your Open311 server returns extra text as part of
+the update, you may put the placeholder `{{description}}` in the template here,
+and that placeholder will be replaced by the text from the Open311 server.
#### Editing or deleting a template
diff --git a/perllib/FixMyStreet/SendReport.pm b/perllib/FixMyStreet/SendReport.pm
index db95850e6..b869299a2 100644
--- a/perllib/FixMyStreet/SendReport.pm
+++ b/perllib/FixMyStreet/SendReport.pm
@@ -60,4 +60,21 @@ sub add_body {
$self->body_config->{ $body->id } = $config;
}
+sub fetch_category {
+ my ($self, $body, $row, $category_override) = @_;
+
+ my $contact = $row->result_source->schema->resultset("Contact")->not_deleted->find( {
+ body_id => $body->id,
+ category => $category_override || $row->category,
+ } );
+
+ unless ($contact) {
+ my $error = "Category " . $row->category . " does not exist for body " . $body->id . " and report " . $row->id . "\n";
+ $self->error( "Failed to send over Open311\n" ) unless $self->error;
+ $self->error( $self->error . "\n" . $error );
+ }
+
+ return $contact;
+}
+
1;
diff --git a/perllib/FixMyStreet/SendReport/Email.pm b/perllib/FixMyStreet/SendReport/Email.pm
index 679530507..6cd9afccd 100644
--- a/perllib/FixMyStreet/SendReport/Email.pm
+++ b/perllib/FixMyStreet/SendReport/Email.pm
@@ -12,10 +12,7 @@ sub build_recipient_list {
my $all_confirmed = 1;
foreach my $body ( @{ $self->bodies } ) {
- my $contact = $row->result_source->schema->resultset("Contact")->not_deleted->find( {
- body_id => $body->id,
- category => $row->category
- } );
+ my $contact = $self->fetch_category($body, $row) or next;
my ($body_email, $state, $note) = ( $contact->email, $contact->state, $contact->note );
diff --git a/perllib/FixMyStreet/SendReport/Email/SingleBodyOnly.pm b/perllib/FixMyStreet/SendReport/Email/SingleBodyOnly.pm
index cf778c549..1ae938317 100644
--- a/perllib/FixMyStreet/SendReport/Email/SingleBodyOnly.pm
+++ b/perllib/FixMyStreet/SendReport/Email/SingleBodyOnly.pm
@@ -15,11 +15,7 @@ sub build_recipient_list {
my $body = $self->bodies->[0];
# We don't care what the category was, look up the relevant contact
- my $contact = $row->result_source->schema->resultset("Contact")->not_deleted->find({
- body_id => $body->id,
- category => $self->contact,
- });
- return unless $contact;
+ my $contact = $self->fetch_category($body, $row, $self->contact) or return;
@{$self->to} = map { [ $_, $body->name ] } split /,/, $contact->email;
return 1;
diff --git a/perllib/FixMyStreet/SendReport/Open311.pm b/perllib/FixMyStreet/SendReport/Open311.pm
index 8a6b992fd..45b056dc2 100644
--- a/perllib/FixMyStreet/SendReport/Open311.pm
+++ b/perllib/FixMyStreet/SendReport/Open311.pm
@@ -36,13 +36,9 @@ sub send {
my $cobrand = $body->get_cobrand_handler || $row->get_cobrand_logged;
$cobrand->call_hook(open311_config => $row, $h, \%open311_params);
- # Try and fill in some ones that we've been asked for, but not asked the user for
-
- my $contact = $row->result_source->schema->resultset("Contact")->not_deleted->find( {
- body_id => $body->id,
- category => $row->category
- } );
+ my $contact = $self->fetch_category($body, $row) or next;
+ # Try and fill in some ones that we've been asked for, but not asked the user for
my $extra = $row->get_extra_fields();
my $id_field = $contact->id_field;
diff --git a/perllib/FixMyStreet/SendReport/Zurich.pm b/perllib/FixMyStreet/SendReport/Zurich.pm
index 59adfd688..7416c64f9 100644
--- a/perllib/FixMyStreet/SendReport/Zurich.pm
+++ b/perllib/FixMyStreet/SendReport/Zurich.pm
@@ -29,10 +29,7 @@ sub build_recipient_list {
my $parent = $body->parent;
if ($parent && !$parent->parent) {
# Division, might have an individual contact email address
- my $contact = $row->result_source->schema->resultset("Contact")->find( {
- body_id => $body->id,
- category => $row->category
- } );
+ my $contact = $self->fetch_category($body, $row);
$body_email = $contact->email if $contact && $contact->email;
}
diff --git a/perllib/Open311/GetServiceRequestUpdates.pm b/perllib/Open311/GetServiceRequestUpdates.pm
index 8193aaa9b..c0da7793f 100644
--- a/perllib/Open311/GetServiceRequestUpdates.pm
+++ b/perllib/Open311/GetServiceRequestUpdates.pm
@@ -235,13 +235,12 @@ sub comment_text_for_request {
my ($self, $request, $problem, $state, $old_state,
$ext_code, $old_ext_code) = @_;
- return $request->{description} if $request->{description};
-
# Response templates are only triggered if the state/external status has changed.
# And treat any fixed state as fixed.
my $state_changed = $state ne $old_state
&& !( $problem->is_fixed && FixMyStreet::DB::Result::Problem->fixed_states()->{$state} );
my $ext_code_changed = $ext_code ne $old_ext_code;
+ my $template;
if ($state_changed || $ext_code_changed) {
my $state_params = {
'me.state' => $state
@@ -250,14 +249,24 @@ sub comment_text_for_request {
$state_params->{'me.external_status_code'} = $ext_code;
};
- if (my $template = $problem->response_templates->search({
+ if (my $t = $problem->response_templates->search({
auto_response => 1,
-or => $state_params,
})->first) {
- return $template->text;
+ $template = $t->text;
}
}
+ my $desc = $request->{description} || '';
+ if ($desc && (!$template || $template !~ /\{\{description}}/)) {
+ return $desc;
+ }
+
+ if ($template) {
+ $template =~ s/\{\{description}}/$desc/;
+ return $template;
+ }
+
return "" if $self->blank_updates_permitted;
print STDERR "Couldn't determine update text for $request->{update_id} (report " . $problem->id . ")\n";
diff --git a/t/app/sendreport/email/highways.t b/t/app/sendreport/email/highways.t
index f53062336..452dda822 100644
--- a/t/app/sendreport/email/highways.t
+++ b/t/app/sendreport/email/highways.t
@@ -11,6 +11,7 @@ $mech->create_contact_ok(email => 'council@example.com', body_id => $bromley->id
$mech->create_contact_ok(email => 'highways@example.com', body_id => $highways->id, category => 'Pothole');
my $row = FixMyStreet::DB->resultset('Problem')->new( {
+ id => 123,
bodies_str => '1000',
category => 'Pothole',
cobrand => '',
diff --git a/t/app/sendreport/email/tfl.t b/t/app/sendreport/email/tfl.t
index 0322de551..8c6eeffa0 100644
--- a/t/app/sendreport/email/tfl.t
+++ b/t/app/sendreport/email/tfl.t
@@ -11,6 +11,7 @@ $mech->create_contact_ok(email => 'council@example.com', body_id => $bromley->id
$mech->create_contact_ok(email => 'tfl@example.com', body_id => $tfl->id, category => 'Traffic lights');
my $row = FixMyStreet::DB->resultset('Problem')->new( {
+ id => 456,
bodies_str => '1000',
category => 'Faulty street light',
cobrand => '',
diff --git a/t/open311/getservicerequestupdates.t b/t/open311/getservicerequestupdates.t
index 130b618c9..3cb2fda69 100644
--- a/t/open311/getservicerequestupdates.t
+++ b/t/open311/getservicerequestupdates.t
@@ -144,7 +144,7 @@ sub create_problem {
detail => '',
used_map => 1,
user_id => 1,
- name => '',
+ name => 'Test User',
state => 'confirmed',
service => '',
cobrand => 'default',
@@ -453,6 +453,37 @@ for my $test (
};
}
+my $response_template_vars = $bodies{2482}->response_templates->create({
+ title => "a placeholder action scheduled template",
+ text => "We are investigating this report: {{description}}",
+ auto_response => 1,
+ state => "action scheduled"
+});
+subtest 'Check template placeholders' => sub {
+ my $local_requests_xml = setup_xml($problem->external_id, $problem->id, 'ACTION_SCHEDULED', 'We will do this in the morning.');
+ my $o = Open311->new( jurisdiction => 'mysociety', endpoint => 'http://example.com', extended_statuses => undef, test_mode => 1, test_get_returns => { 'servicerequestupdates.xml' => $local_requests_xml } );
+
+ $problem->lastupdate( DateTime->now()->subtract( days => 1 ) );
+ $problem->state( 'fixed - council' );
+ $problem->update;
+
+ my $update = Open311::GetServiceRequestUpdates->new;
+ $update->fetch($o);
+
+ is $problem->comments->count, 1, 'comment count';
+ $problem->discard_changes;
+
+ my $c = FixMyStreet::DB->resultset('Comment')->search( { external_id => 638344 } )->first;
+ ok $c, 'comment exists';
+ is $c->text, "We are investigating this report: We will do this in the morning.", 'text correct';
+ is $c->mark_fixed, 0, 'mark_closed correct';
+ is $c->problem_state, 'action scheduled', 'problem_state correct';
+ is $c->mark_open, 0, 'mark_open correct';
+ is $c->state, 'confirmed', 'comment state correct';
+ is $problem->state, 'action scheduled', 'correct problem state';
+ $problem->comments->delete;
+};
+
my $problemB = create_problem($bodies{2237}->id);
for my $test (
diff --git a/t/sendreport/open311.t b/t/sendreport/open311.t
index e68a0aa3c..382df39f0 100644
--- a/t/sendreport/open311.t
+++ b/t/sendreport/open311.t
@@ -11,6 +11,7 @@ package main;
use CGI::Simple;
use Path::Tiny;
+use Test::Warn;
use FixMyStreet::Script::Reports;
use FixMyStreet::TestMech;
my $mech = FixMyStreet::TestMech->new;
@@ -133,4 +134,25 @@ subtest 'test sending multiple photos', sub {
], 'Multiple photos in media_url';
};
+my ($bad_category_report) = $mech->create_problems_for_body( 1, $body->id, 'Test', {
+ cobrand => 'fixmystreet',
+ category => 'Flytipping',
+ user => $user,
+});
+
+subtest 'test handles bad category', sub {
+ $body->update( { send_method => 'Open311', endpoint => 'http://endpoint.example.com', jurisdiction => 'FMS', api_key => 'test' } );
+ my $test_data;
+ FixMyStreet::override_config {
+ STAGING_FLAGS => { send_reports => 1 },
+ ALLOWED_COBRANDS => [ 'fixmystreet' ],
+ MAPIT_URL => 'http://mapit.uk/',
+ }, sub {
+ $test_data = FixMyStreet::Script::Reports::send();
+ };
+ $bad_category_report->discard_changes;
+ ok !$bad_category_report->whensent, 'Report not marked as sent';
+ like $bad_category_report->send_fail_reason, qr/Category Flytipping does not exist for body/, 'failure message set';
+};
+
done_testing();
generated by cgit v1.2.3 (git 2.25.1) at 2025-07-17 14:57:01 +0000