Skip to content

[Dumfries] Handle Cancelled Job response templates for Open311 updates #6000

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
davea wants to merge 1 commit into
base: master
Choose a base branch
from
+237 −2
Draft

[Dumfries] Handle Cancelled Job response templates for Open311 updates #6000

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions perllib/FixMyStreet/App/Controller/Admin/Templates.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,15 @@ sub edit : Path : Args(2) {
}

$template->auto_response( $c->get_param('auto_response') && ( $template->state || $template->external_status_code ) ? 1 : 0 );

if ($body_cobrand) {
my $errors = $body_cobrand->call_hook(validate_response_template => $template);
if ($errors && ref $errors eq 'HASH') {
$c->stash->{errors} ||= {};
%{ $c->stash->{errors} } = (%{ $c->stash->{errors} }, %$errors);

@nephila-nacrea nephila-nacrea Jun 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this would read better as something like
$c->stash->{errors}{$_} = $errors->{$_} for keys %$errors;

}
}

if ($template->auto_response) {
my @check_contact_ids = @new_contact_ids;
# If the new template has not specific categories (i.e. it
Expand Down
51 changes: 51 additions & 0 deletions perllib/FixMyStreet/Cobrand/Dumfries.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,57 @@ sub state_groups_inspect {
]
}

=head2 response_template_override

When an update moves a report from planned to confirmed, use the first
matching response template whose title starts with "Cancelled Job".

=cut

sub response_template_override {
my ($self, $problem, $state, $old_state) = @_;

return unless $old_state eq 'planned' && $state eq 'confirmed';

return $problem->response_templates->search({
'me.title' => { -like => 'Cancelled Job%' },
}, {
order_by => 'me.title',
})->first;
}


=head2 validate_response_template


Performs some custom validation for templates whose title start with 'Cancelled Job':
can't be auto-response; can't be for an external status code; must be for 'confirmed' state.
=cut

@nephila-nacrea nephila-nacrea Jun 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would look better to move one of the empty lines down to be just above =cut.


sub validate_response_template {
my ($self, $template) = @_;

return unless ($template->title || '') =~ /^Cancelled Job/;

my %errors;

if (($template->state || '') ne 'confirmed') {
$errors{state} =
'Templates whose title starts with "Cancelled Job" are reserved for cancelled job handling and must use the Open state.';

@nephila-nacrea nephila-nacrea Jun 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could put the initial part of the error message into a shared variable, as it is used by all the error messages.

}

if ($template->external_status_code) {
$errors{external_status_code} =
'Templates whose title starts with "Cancelled Job" are reserved for cancelled job handling and cannot use an external status code.';
}

if ($template->auto_response) {
$errors{auto_response} =
'Templates whose title starts with "Cancelled Job" are reserved for the cancelled job feature and cannot be set as auto-response.';
}

return %errors ? \%errors : undef;
}

=head2 validate_response_template_external_status_code

Expand Down
11 changes: 9 additions & 2 deletions perllib/FixMyStreet/DB/Result/Problem.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -928,6 +928,7 @@ sub response_templates {

sub response_template_for {
my ($self, $body, $state, $old_state, $ext_code, $old_ext_code) = @_;
my $cobrand = $body->get_cobrand_handler;

# Response templates are only triggered if the state/external status has changed.
# And treat any fixed state as fixed.
Expand All @@ -943,8 +944,6 @@ sub response_template_for {
push @$state_params, { 'me.state' => $state, 'me.external_status_code' => ["", undef] };
}
if ($ext_code_changed) {
my $cobrand = $body->get_cobrand_handler;

# Allow cobrands to use regex matching for wildcard patterns in templates
if (my $regex_match = $cobrand && $cobrand->call_hook(
response_template_external_status_code_regex_match => $ext_code
Expand Down Expand Up @@ -972,6 +971,14 @@ sub response_template_for {
auto_response => 1,
-or => $state_params,
}, $order )->first;

if ($cobrand) {
$template = $cobrand->call_hook(
response_template_override => $self,
$state,
$old_state,
) || $template;
}
}
return $template;
}
Expand Down
168 changes: 168 additions & 0 deletions t/cobrand/dumfries.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ use Test::More;
use Test::MockModule;
use Test::MockTime qw(:all);
use LWP::Protocol::PSGI;
use Open311;
use Open311::GetUpdates;
use t::Mock::MyGovScotOIDC;

my $mech = FixMyStreet::TestMech->new;
Expand Down Expand Up @@ -759,6 +761,172 @@ subtest 'admin template external_status_code validation' => sub {
};
};

subtest 'planned->confirmed uses Cancelled Job template override' => sub {
FixMyStreet::override_config {
ALLOWED_COBRANDS => ['dumfries'],
}, sub {
my $test_problem = FixMyStreet::DB->resultset('Problem')->create({
postcode => 'DG1 1AA',
bodies_str => $body->id,
areas => ',2656,',
category => 'Potholes',
title => 'Cancelled Job override test problem',
detail => 'Test detail',
used_map => 1,
name => 'Reporter',
anonymous => 0,
state => 'planned',
confirmed => DateTime->now->subtract(days => 1),
lastupdate => DateTime->now->subtract(days => 1),
latitude => 55.0706,
longitude => -3.9568,
user_id => $reporter->id,
cobrand => 'dumfries',
external_id => 'dumfries-open311-cancelled-job-1',
});

my $normal_template = FixMyStreet::DB->resultset('ResponseTemplate')->create({
body_id => $body->id,
title => 'Normal Confirmed Auto Template',
text => 'Normal confirmed template text',
auto_response => 1,
state => 'confirmed',
external_status_code => '',
});

my $cancelled_job_template = FixMyStreet::DB->resultset('ResponseTemplate')->create({
body_id => $body->id,
title => 'Cancelled Job - Planned to Confirmed',
text => 'Cancelled job template text',
auto_response => 0,
state => '',

@nephila-nacrea nephila-nacrea Jun 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the template have either state or external_status_code?

external_status_code => '',
});

my $open311 = Open311->new(endpoint => 'http://example.com', jurisdiction => 'dumfries');
my $updates = Open311::GetUpdates->new(
current_open311 => $open311,
current_body => $body,
system_user => $staff_user,
);

subtest 'override applies for planned->confirmed transition via update processing' => sub {
my $request = {
comment_time => DateTime->now,
status => 'OPEN',
description => '',
update_id => 'cancelled-job-override-1',
external_status_code => '',
};

my $comment = $updates->process_update($request, $test_problem);
ok $comment, 'Update comment created';
is $comment->text, 'Cancelled job template text',
'Cancelled Job template text used for planned->confirmed';
is $comment->problem_state, 'confirmed',
'Comment state is confirmed';

$test_problem->discard_changes;
is $test_problem->state, 'confirmed',
'Problem moved to confirmed';
};

subtest 'override does not apply outside planned->confirmed transition' => sub {
$test_problem->update({
state => 'investigating',
lastupdate => DateTime->now->subtract(hours => 1),
});
$test_problem->discard_changes;

my $request = {
comment_time => DateTime->now,
status => 'OPEN',
description => '',
update_id => 'cancelled-job-override-2',
external_status_code => '',
};

my $comment = $updates->process_update($request, $test_problem);
ok $comment, 'Second update comment created';
is $comment->text, 'Normal confirmed template text',
'Normal template used when transition is not planned->confirmed';
};

$test_problem->comments->delete;
$normal_template->delete;
$cancelled_job_template->delete;
$test_problem->delete;
};
};

subtest 'admin validates reserved Cancelled Job templates' => sub {
FixMyStreet::override_config {
ALLOWED_COBRANDS => ['dumfries'],
}, sub {
$mech->log_in_ok($superuser->email);

subtest 'auto-response set on Cancelled Job title is rejected' => sub {
$mech->get_ok('/admin/templates/' . $body->id . '/new');
$mech->submit_form_ok({ with_fields => {
title => 'Cancelled Job - Reserved Template',
text => 'Template text',
state => 'confirmed',
auto_response => 'on',
} });

is $mech->uri->path, '/admin/templates/' . $body->id . '/new',
'Not redirected when reserved template is set as auto-response';
$mech->content_contains('reserved for the cancelled job feature',

@nephila-nacrea nephila-nacrea Jun 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be better to specifically test for the 'cannot be set as auto-response' bit.

'Reserved-template validation message shown');
};

subtest 'Cancelled Job title must use Open state' => sub {
$mech->get_ok('/admin/templates/' . $body->id . '/new');
$mech->submit_form_ok({ with_fields => {
title => 'Cancelled Job - Wrong State',
text => 'Template text',
state => 'investigating',
} });

is $mech->uri->path, '/admin/templates/' . $body->id . '/new',
'Not redirected when reserved template uses non-open state';
$mech->content_contains('must use the Open state',
'Open-state validation message shown');
};

subtest 'Cancelled Job title cannot use external status code' => sub {
$mech->get_ok('/admin/templates/' . $body->id . '/new');
$mech->submit_form_ok({ with_fields => {
title => 'Cancelled Job - External Code',
text => 'Template text',
state => 'confirmed',
external_status_code => 'status1:outcome1:priority1',
} });

is $mech->uri->path, '/admin/templates/' . $body->id . '/new',
'Not redirected when reserved template uses external status code';
$mech->content_contains('cannot use an external status code',
'External status code validation message shown');
};

subtest 'Cancelled Job title without auto-response is allowed' => sub {
$mech->get_ok('/admin/templates/' . $body->id . '/new');
$mech->submit_form_ok({ with_fields => {
title => 'Cancelled Job - Manual Template',
text => 'Template text',
state => 'confirmed',
} });

is $mech->uri->path, '/admin/templates/' . $body->id,
'Redirected when Cancelled Job template is not auto-response';
$mech->delete_response_template($_)
for $body->response_templates->search({ title => 'Cancelled Job - Manual Template' });
};

$mech->log_out_ok;
};
};

subtest "check not responsible as correct text" => sub {
my $c = FixMyStreet::DB->resultset('Comment')->create({
problem => $problem, user => $problem->user, anonymous => 't', text => 'Update text',
Expand Down
Loading