diff options
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Admin.pm | 45 | ||||
| -rw-r--r-- | perllib/FixMyStreet/TestMech.pm | 8 | ||||
| -rw-r--r-- | t/app/controller/admin.t | 112 | ||||
| -rw-r--r-- | templates/web/base/admin/template_edit.html | 8 |
4 files changed, 162 insertions, 11 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm index b485ea2dc..e21b0cc2f 100644 --- a/perllib/FixMyStreet/App/Controller/Admin.pm +++ b/perllib/FixMyStreet/App/Controller/Admin.pm @@ -1102,19 +1102,52 @@ sub template_edit : Path('templates') : Args(2) { } } @live_contacts; $c->stash->{contacts} = \@all_contacts; - if ($c->req->method eq 'POST') { + # bare block to use 'last' if form is invalid. + if ($c->req->method eq 'POST') { { if ($c->get_param('delete_template') && $c->get_param('delete_template') eq _("Delete template")) { $template->contact_response_templates->delete_all; $template->delete; } else { + my @live_contact_ids = map { $_->id } @live_contacts; + my @new_contact_ids = grep { $c->get_param("contacts[$_]") } @live_contact_ids; + my %new_contacts = map { $_ => 1 } @new_contact_ids; + for my $contact (@all_contacts) { + $contact->{active} = $new_contacts{$contact->{id}}; + } + $template->title( $c->get_param('title') ); $template->text( $c->get_param('text') ); $template->state( $c->get_param('state') ); - $template->auto_response( $c->get_param('auto_response') ? 1 : 0 ); - $template->update_or_insert; - my @live_contact_ids = map { $_->id } @live_contacts; - my @new_contact_ids = grep { $c->get_param("contacts[$_]") } @live_contact_ids; + $template->auto_response( $c->get_param('auto_response') && $template->state ? 1 : 0 ); + if ($template->auto_response) { + my @check_contact_ids = @new_contact_ids; + # If the new template has not specific categories (i.e. it + # applies to all categories) then we need to check each of those + # category ids for existing auto-response templates. + if (!scalar @check_contact_ids) { + @check_contact_ids = @live_contact_ids; + } + my $query = { + 'auto_response' => 1, + 'contact.id' => [ @check_contact_ids, undef ], + 'me.state' => $template->state, + }; + if ($template->in_storage) { + $query->{'me.id'} = { '!=', $template->id }; + } + if ($c->stash->{body}->response_templates->search($query, { + join => { 'contact_response_templates' => 'contact' }, + })->count) { + $c->stash->{errors} = { + auto_response => _("There is already an auto-response template for this category/state.") + }; + } + } + + last if $c->stash->{errors}; + + $template->update_or_insert; $template->contact_response_templates->search({ contact_id => { '!=' => \@new_contact_ids }, })->delete; @@ -1126,7 +1159,7 @@ sub template_edit : Path('templates') : Args(2) { } $c->res->redirect( $c->uri_for( 'templates', $c->stash->{body}->id ) ); - } + } } $c->stash->{response_template} = $template; diff --git a/perllib/FixMyStreet/TestMech.pm b/perllib/FixMyStreet/TestMech.pm index dbfc94286..ac2ac023d 100644 --- a/perllib/FixMyStreet/TestMech.pm +++ b/perllib/FixMyStreet/TestMech.pm @@ -630,6 +630,14 @@ sub delete_defect_type { $defect_type->delete; } +sub delete_response_template { + my $mech = shift; + my $response_template = shift; + + $response_template->contact_response_templates->delete_all; + $response_template->delete; +} + sub create_contact_ok { my $self = shift; my %contact_params = ( diff --git a/t/app/controller/admin.t b/t/app/controller/admin.t index 0be54dbc5..179557976 100644 --- a/t/app/controller/admin.t +++ b/t/app/controller/admin.t @@ -1659,6 +1659,108 @@ subtest "response templates are included on page" => sub { }; }; +subtest "auto-response templates that duplicate a single category can't be added" => sub { + $mech->delete_response_template($_) for $oxfordshire->response_templates; + my $template = $oxfordshire->response_templates->create({ + title => "Report fixed - potholes", + text => "Thank you for your report. This problem has been fixed.", + auto_response => 1, + state => 'fixed - council', + }); + $template->contact_response_templates->find_or_create({ + contact_id => $oxfordshirecontact->id, + }); + is $oxfordshire->response_templates->count, 1, "Initial response template was created"; + + + $mech->log_in_ok( $superuser->email ); + $mech->get_ok( "/admin/templates/" . $oxfordshire->id . "/new" ); + + # This response template has the same category & state as an existing one + # so won't be allowed. + my $fields = { + title => "Report marked fixed - potholes", + text => "Thank you for your report. This pothole has been fixed.", + auto_response => 'on', + state => 'fixed - council', + "contacts[".$oxfordshirecontact->id."]" => 1, + }; + $mech->submit_form_ok( { with_fields => $fields } ); + is $mech->uri->path, '/admin/templates/' . $oxfordshire->id . '/new', 'not redirected'; + $mech->content_contains( 'Please correct the errors below' ); + $mech->content_contains( 'There is already an auto-response template for this category/state.' ); + + is $oxfordshire->response_templates->count, 1, "Duplicate response template wasn't added"; +}; + +subtest "auto-response templates that duplicate all categories can't be added" => sub { + $mech->delete_response_template($_) for $oxfordshire->response_templates; + $oxfordshire->response_templates->create({ + title => "Report investigating - all cats", + text => "Thank you for your report. This problem has been fixed.", + auto_response => 1, + state => 'fixed - council', + }); + is $oxfordshire->response_templates->count, 1, "Initial response template was created"; + + + $mech->log_in_ok( $superuser->email ); + $mech->get_ok( "/admin/templates/" . $oxfordshire->id . "/new" ); + + # There's already a response template for all categories and this state, so + # this new template won't be allowed. + my $fields = { + title => "Report investigating - single cat", + text => "Thank you for your report. This problem has been fixed.", + auto_response => 'on', + state => 'fixed - council', + "contacts[".$oxfordshirecontact->id."]" => 1, + }; + $mech->submit_form_ok( { with_fields => $fields } ); + is $mech->uri->path, '/admin/templates/' . $oxfordshire->id . '/new', 'not redirected'; + $mech->content_contains( 'Please correct the errors below' ); + $mech->content_contains( 'There is already an auto-response template for this category/state.' ); + + + is $oxfordshire->response_templates->count, 1, "Duplicate response template wasn't added"; +}; + +subtest "all-category auto-response templates that duplicate a single category can't be added" => sub { + $mech->delete_response_template($_) for $oxfordshire->response_templates; + my $template = $oxfordshire->response_templates->create({ + title => "Report fixed - potholes", + text => "Thank you for your report. This problem has been fixed.", + auto_response => 1, + state => 'fixed - council', + }); + $template->contact_response_templates->find_or_create({ + contact_id => $oxfordshirecontact->id, + }); + is $oxfordshire->response_templates->count, 1, "Initial response template was created"; + + + $mech->log_in_ok( $superuser->email ); + $mech->get_ok( "/admin/templates/" . $oxfordshire->id . "/new" ); + + # This response template is implicitly for all categories, but there's + # already a template for a specific category in this state, so it won't be + # allowed. + my $fields = { + title => "Report marked fixed - all cats", + text => "Thank you for your report. This problem has been fixed.", + auto_response => 'on', + state => 'fixed - council', + }; + $mech->submit_form_ok( { with_fields => $fields } ); + is $mech->uri->path, '/admin/templates/' . $oxfordshire->id . '/new', 'not redirected'; + $mech->content_contains( 'Please correct the errors below' ); + $mech->content_contains( 'There is already an auto-response template for this category/state.' ); + + is $oxfordshire->response_templates->count, 1, "Duplicate response template wasn't added"; +}; + + + $mech->log_in_ok( $superuser->email ); subtest "response priorities can be added" => sub { @@ -1674,8 +1776,8 @@ subtest "response priorities can be added" => sub { }; $mech->submit_form_ok( { with_fields => $fields } ); - is $oxfordshire->response_priorities->count, 1, "Response template was added to body"; - is $oxfordshirecontact->response_priorities->count, 1, "Response template was added to contact"; + is $oxfordshire->response_priorities->count, 1, "Response priority was added to body"; + is $oxfordshirecontact->response_priorities->count, 1, "Response priority was added to contact"; }; subtest "response priorities can set to default" => sub { @@ -1693,7 +1795,7 @@ subtest "response priorities can set to default" => sub { $mech->submit_form_ok( { with_fields => $fields } ); is $oxfordshire->response_priorities->count, 1, "Still one response priority"; - is $oxfordshirecontact->response_priorities->count, 1, "Still one response template"; + is $oxfordshirecontact->response_priorities->count, 1, "Still one response priority"; ok $oxfordshire->response_priorities->first->is_default, "Response priority set to default"; }; @@ -1710,8 +1812,8 @@ subtest "response priorities are limited by body" => sub { name => "Bromley Cat 0", } ); - is $bromley->response_priorities->count, 1, "Response template was added to Bromley"; - is $oxfordshire->response_priorities->count, 1, "Response template wasn't added to Oxfordshire"; + is $bromley->response_priorities->count, 1, "Response priority was added to Bromley"; + is $oxfordshire->response_priorities->count, 1, "Response priority wasn't added to Oxfordshire"; $mech->get_ok( "/admin/responsepriorities/" . $oxfordshire->id ); $mech->content_lacks( $bromleypriority->name ); diff --git a/templates/web/base/admin/template_edit.html b/templates/web/base/admin/template_edit.html index 2014ac2fb..3e436dbf9 100644 --- a/templates/web/base/admin/template_edit.html +++ b/templates/web/base/admin/template_edit.html @@ -9,6 +9,11 @@ accept-charset="utf-8" class="validate"> + [% IF errors %] + <p class="error">[% loc('Please correct the errors below') %]</p> + [% END %] + + <div class="admin-hint"> <p> [% loc('This is a <strong>private</strong> name for this template so you can identify it when updating reports or editing in the admin.') %] @@ -46,6 +51,9 @@ [% INCLUDE 'admin/state_groups_select.html' current_state=rt.state include_empty=1 %] </p> + [% IF errors.auto_response %] + <div class="form-error">[% errors.auto_response %]</div> + [% END %] <div class="admin-hint"> <p> [% loc('If ticked, this template will be used for Open311 updates that put problems in this state.') %] |