[Dumfries] Handle Cancelled Job response templates for Open311 updates#6000
[Dumfries] Handle Cancelled Job response templates for Open311 updates#6000davea wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6000 +/- ##
==========================================
- Coverage 83.43% 83.41% -0.02%
==========================================
Files 500 500
Lines 37304 37324 +20
Branches 6162 6171 +9
==========================================
+ Hits 31124 31135 +11
- Misses 4370 4377 +7
- Partials 1810 1812 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| 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 |
There was a problem hiding this comment.
Would look better to move one of the empty lines down to be just above =cut.
| 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); |
There was a problem hiding this comment.
I wonder if this would read better as something like
$c->stash->{errors}{$_} = $errors->{$_} for keys %$errors;
| title => 'Cancelled Job - Planned to Confirmed', | ||
| text => 'Cancelled job template text', | ||
| auto_response => 0, | ||
| state => '', |
There was a problem hiding this comment.
Should the template have either state or external_status_code?
|
|
||
| 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', |
There was a problem hiding this comment.
Might be better to specifically test for the 'cannot be set as auto-response' bit.
|
|
||
| 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.'; |
There was a problem hiding this comment.
You could put the initial part of the error message into a shared variable, as it is used by all the error messages.
For https://github.com/mysociety/societyworks/issues/5475
This adds a cobrand hook which allows a specific response template to be chosen according to the problem's old/new status. For Dumfries that hook looks for a response template whose title begins 'Cancelled Job' and uses that when a problem state goes from 'planned' to 'confirmed'. Also adds validation for these response templates when edited via the admin - can't be marked as auto response, can't have an external status, must be for 'confirmed' state.
(alternative approach to #5968)
[skip changelog]