Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions bin/update-schema
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ else {
# (assuming schema change files are never half-applied, which should be the case)
sub get_db_version {
return 'EMPTY' if ! table_exists('problem');
return '0096' if column_exists('response_templates', 'old_external_status_code');
return '0095' if column_exists('response_templates', 'deleted');
return '0094' if index_exists('problem_confirmed_idx');
return '0093' if index_exists('user_planned_reports_report_id_idx');
Expand Down
5 changes: 5 additions & 0 deletions db/downgrade_0096---0095.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
BEGIN;

ALTER TABLE response_templates DROP COLUMN old_external_status_code;

COMMIT;
1 change: 1 addition & 0 deletions db/schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,7 @@ create table response_templates (
state text,
external_status_code text,
deleted boolean not null default 'f',
old_external_status_code text NOT NULL DEFAULT '',
unique(body_id, title)
);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
BEGIN;

ALTER TABLE response_templates ADD old_external_status_code text NOT NULL DEFAULT '';

COMMIT;
23 changes: 21 additions & 2 deletions perllib/FixMyStreet/App/Controller/Admin/Templates.pm
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ sub edit : Path : Args(2) {
$ext_code ||= $c->get_param('external_status_code');
$template->external_status_code($ext_code);

my $old_ext_code = $c->get_param('old_external_status_code') // '';
$template->old_external_status_code($old_ext_code);

$template->deleted ( $c->get_param('deleted') ? 1 : 0 );

# Allow cobrands to validate the external_status_code format
Expand All @@ -125,6 +128,11 @@ sub edit : Path : Args(2) {
$c->stash->{errors}->{external_status_code} = _("State and external status code cannot be used simultaneously.");
}

if ( $template->external_status_code && $template->old_external_status_code ) {
$c->stash->{errors}->{external_status_code} = 'Old status cannot be the same as new status'
if $template->external_status_code =~ /^${\$template->old_external_status_code}:/;
}

$template->auto_response( $c->get_param('auto_response') && ( $template->state || $template->external_status_code ) ? 1 : 0 );
if ($template->auto_response) {
my @check_contact_ids = @new_contact_ids;
Expand All @@ -136,13 +144,24 @@ sub edit : Path : Args(2) {
}

my $state_param = { $template->state ? ('me.state' => $template->state) : () };
my $code_param = { $template->external_status_code ? ('me.external_status_code' => $template->external_status_code) : () };
# Always set a param for old_external_status_code if there is
# an external_status_code - for most templates/cobrands, it
# will be empty string
my $code_param = {
$template->external_status_code
? ( 'me.external_status_code' =>
$template->external_status_code,
'me.old_external_status_code' =>
$template->old_external_status_code // '',
)
: ()
};
my $params;
if ($c->cobrand->admin_templates_state_and_external_status_code) {
# Both can be set, if external code set need to check that alone
$params = $template->external_status_code ? $code_param : $state_param;
} else {
$params = { -or => { %$state_param, %$code_param } };
$params = { -or => { %$state_param, -and => $code_param } };
}

my $query = {
Expand Down
4 changes: 3 additions & 1 deletion perllib/FixMyStreet/Cobrand/Dumfries.pm
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,9 @@ sub response_template_external_status_code_regex_match {
(LENGTH(me.external_status_code) - LENGTH(REPLACE(me.external_status_code, '+', '')))
) ASC,
(LENGTH(me.external_status_code) - LENGTH(REPLACE(me.external_status_code, '*', ''))) ASC,
me.external_status_code DESC NULLS LAST, contact.category
me.external_status_code DESC NULLS LAST,
me.old_external_status_code DESC,
contact.category
};

return {
Expand Down
5 changes: 5 additions & 0 deletions perllib/FixMyStreet/DB/Result/Problem.pm
Original file line number Diff line number Diff line change
Expand Up @@ -951,11 +951,16 @@ sub response_template_for {
)) {
# Use regex matching - the template's pattern is converted to a regex
# and matched against the update's external_status_code

my ($old_ext_code_first_part) = ($old_ext_code // '') =~ /^([^:]+)/;
$old_ext_code_first_part //= '';

push @$state_params, {
'me.state' => '',
'me.external_status_code' => { '!=' => '' }, # Must have a pattern
# Use -bool with a literal SQL ref for the regex match
-bool => \[$regex_match->{sql}, @{$regex_match->{bind}}],
'me.old_external_status_code' => [ $old_ext_code_first_part, '' ],
};
$order = { order_by => \$regex_match->{order} };
} else {
Expand Down
6 changes: 4 additions & 2 deletions perllib/FixMyStreet/DB/Result/ResponseTemplate.pm
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ __PACKAGE__->add_columns(
{ data_type => "text", is_nullable => 1 },
"deleted",
{ data_type => "boolean", default_value => \"false", is_nullable => 0 },
"old_external_status_code",
{ data_type => "text", default_value => "", is_nullable => 0 },
);
__PACKAGE__->set_primary_key("id");
__PACKAGE__->add_unique_constraint("response_templates_body_id_title_key", ["body_id", "title"]);
Expand All @@ -62,8 +64,8 @@ __PACKAGE__->has_many(
);


# Created by DBIx::Class::Schema::Loader v0.07035 @ 2026-05-05 10:26:47
# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:a3Z6rif7L1Gvv1sRFktTVA
# Created by DBIx::Class::Schema::Loader v0.07035 @ 2026-05-11 21:59:47
# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:blYQ0knqCGoDgvD0QxGq+w

__PACKAGE__->many_to_many( contacts => 'contact_response_templates', 'contact' );

Expand Down
221 changes: 219 additions & 2 deletions t/cobrand/dumfries.t
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,15 @@ subtest 'admin template external_status_code validation' => sub {
{ name => 'Wildcard in middle of text returns error', code => 'abc:d*f:ghi', valid => 0, error => 'cannot be mixed' },
{ name => 'Wildcard at end of text returns error', code => 'abc:def:ghi*', valid => 0, error => 'cannot be mixed' },
{ name => 'Double wildcard returns error', code => 'abc:**:ghi', valid => 0, error => 'cannot be mixed' },

{ name => 'Add an old_external_status_code - valid', code => 'abc:def:ghi', old_code => 'def', valid => 1 },
{ name =>
'Add an old_external_status_code - matches first part of external_status_code',
code => 'abc:def:ghi',
old_code => 'abc',
valid => 0,
error => 'Old status cannot be the same as new',
},
);

my $i = 0;
Expand All @@ -701,6 +710,7 @@ subtest 'admin template external_status_code validation' => sub {
text => 'Template text',
auto_response => 'on',
defined $case->{code} ? (external_status_code => $case->{code}) : (),
defined $case->{old_code} ? (old_external_status_code => $case->{old_code}) : (),
);

$mech->get_ok('/admin/templates/' . $body->id . '/new');
Expand All @@ -709,8 +719,15 @@ subtest 'admin template external_status_code validation' => sub {
if ($case->{valid}) {
is $mech->uri->path, '/admin/templates/' . $body->id,
'Redirected after valid submission';
$mech->delete_response_template($_)
for $body->response_templates->search({ title => 'Test template' });

my $template = $body->response_templates->search(
{ title => 'Test template' } )->first;

is $template->old_external_status_code,
$case->{old_code} // '',
'correct old code or empty string set';

$mech->delete_response_template($template);
} else {
is $mech->uri->path, '/admin/templates/' . $body->id . '/new',
'Not redirected on error';
Expand All @@ -719,8 +736,208 @@ subtest 'admin template external_status_code validation' => sub {
};
}

subtest 'Set templates with same external_status_code but different old_external_status_codes' => sub {
my %fields = (
title => 'Test template',
text => 'Template text',
auto_response => 'on',
external_status_code => 'abc:def:ghi',
old_external_status_code => 'xyz',
);

$mech->get_ok('/admin/templates/' . $body->id . '/new');
$mech->submit_form_ok({ with_fields => \%fields });
is $mech->uri->path, '/admin/templates/' . $body->id,
'Redirected after valid submission';

%fields = (
title => 'Test template 2',
text => 'Template text 2',
auto_response => 'on',
external_status_code => 'abc:def:ghi',
old_external_status_code => 'zyx',
);

$mech->get_ok('/admin/templates/' . $body->id . '/new');
$mech->submit_form_ok({ with_fields => \%fields });
is $mech->uri->path, '/admin/templates/' . $body->id,
'Redirected after valid submission';

%fields = (
title => 'Test template 3',
text => 'Template text 3',
auto_response => 'on',
external_status_code => 'abc:def:ghi',
old_external_status_code => '',
);

$mech->get_ok('/admin/templates/' . $body->id . '/new');
$mech->submit_form_ok({ with_fields => \%fields });
is $mech->uri->path, '/admin/templates/' . $body->id,
'Redirected after valid submission';

$mech->delete_response_template($_)
for $body->response_templates->search(
{ title => [
'Test template',
'Test template 2',
'Test template 3',
]
}
);
};

$mech->log_out_ok;
};
};

subtest 'Alloy "planned" set back to "reported" - template message triggers' => sub {
FixMyStreet::override_config {
ALLOWED_COBRANDS => ['dumfries'],
}, sub {
my %templates = (
1 => { body_id => $body->id,
title => 'Alloy anything to reported (empty string)',
text =>
'This defect has been reopened (matches empty string).',
auto_response => 1,
state => '',
external_status_code => 'external_reported:*:*',
old_external_status_code => '',
},
2 => { body_id => $body->id,
title => 'Alloy planned to reported',
text =>
'A planned job was cancelled so this defect has been reopened.',
auto_response => 1,
state => '',
external_status_code => 'external_reported:*:*',
old_external_status_code => 'external_planned',
},
3 => { body_id => $body->id,
title => 'Alloy planned to reported and priority_1',
text =>
'A planned job was cancelled so this defect has been reopened with priority_1.',
auto_response => 1,
state => '',
external_status_code => 'external_reported:*:priority_1',
old_external_status_code => 'external_planned',
},
);

# Use hash to make sure we are not relying on a particular order of
# templates
for ( values %templates ) {
FixMyStreet::DB->resultset('ResponseTemplate')->create($_);
}

my $test_problem = FixMyStreet::DB->resultset('Problem')->create({
postcode => 'DG1 1AA',
bodies_str => $body->id,
areas => ',2656,',
category => 'Potholes',
title => 'Pretending to be a report sent to Alloy',
detail => 'Test detail',
used_map => 1,
name => 'Reporter',
anonymous => 0,
state => 'confirmed',
confirmed => DateTime->now,
lastupdate => DateTime->now,
latitude => 55.0706,
longitude => -3.9568,
user_id => $reporter->id,
cobrand => 'dumfries',

external_id => 12345,
});

my $o = Open311->new( jurisdiction => 'mysociety', endpoint => 'http://example.com');
my $update = Open311::GetServiceRequestUpdates->new(
system_user => $staff_user,
current_open311 => $o,
current_body => $body,
);

note 'No initial external_status_code';

_inject_response( update_id => 'UPDATE_1' );

is $test_problem->state, 'confirmed';
is $test_problem->get_extra_metadata('external_status_code'), undef;
$update->process_body;
$test_problem->discard_changes;
is $test_problem->comments->order_by('-id')->first->text,
'This defect has been reopened (matches empty string).',
'Problem with no initial external_status_code correctly triggers general template';

note 'Planned to reported';

_inject_response( update_id => 'UPDATE_2' );

$test_problem->state('planned');
$test_problem->set_extra_metadata( 'external_status_code',
'external_planned::' );
$test_problem->update;
$update->process_body;
$test_problem->discard_changes;
is $test_problem->comments->order_by('-id')->first->text,
'A planned job was cancelled so this defect has been reopened.',
'Planned->reported problem correctly triggers specific template';

note 'Planned to reported with priority_1';

_inject_response(
update_id => 'UPDATE_3',
external_status_code => 'external_reported::priority_1',
);

$test_problem->state('planned');
$test_problem->set_extra_metadata( 'external_status_code',
'external_planned::' );
$test_problem->update;
$update->process_body;
$test_problem->discard_changes;
is $test_problem->comments->order_by('-id')->first->text,
'A planned job was cancelled so this defect has been reopened with priority_1.',
'Planned->reported-with-priority problem correctly triggers specific template';

note 'Investigating to reported';

_inject_response( update_id => 'UPDATE_4' );

$test_problem->state('investigating');
$test_problem->set_extra_metadata( 'external_status_code',
'external_investigating::' );
$test_problem->update;
$update->process_body;
$test_problem->discard_changes;
is $test_problem->comments->order_by('-id')->first->text,
'This defect has been reopened (matches empty string).',
'Investigating->reported problem correctly triggers general template';
};
};

sub _inject_response {
my %args = @_;

$args{external_status_code} //= 'external_reported::';

my $requests_xml = qq{<?xml version="1.0" encoding="utf-8"?>
<service_requests_updates>
<request_update>
<update_id>$args{update_id}</update_id>
<service_request_id>12345</service_request_id>
<status>open</status>
<external_status_code>$args{external_status_code}</external_status_code>
<updated_datetime>UPDATED_DATETIME</updated_datetime>
</request_update>
</service_requests_updates>
};
my $update_dt = DateTime->now(formatter => DateTime::Format::W3CDTF->new);
$requests_xml =~ s/UPDATED_DATETIME/$update_dt/g;

Open311->_inject_response('/servicerequestupdates.xml', $requests_xml);
}

done_testing();
Loading
Loading