From cfda101b3006f12280a41adc4b28ca555b867556 Mon Sep 17 00:00:00 2001 From: Struan Donald Date: Fri, 23 Oct 2020 14:28:38 +0100 Subject: prevent editing of category names with hardcoded flag If a category has hardcoded set to 1 in it's extra metadata then prevent the name being edited in the admin. This is to avoid issues where the name of the category is used in e.g. layers or other configuration and changing it breaks things. Also includes admin interface for setting this that is restricted to super users only. Fixes mysociety/fixmystreet-commercial#1992 --- CHANGELOG.md | 1 + perllib/FixMyStreet/App/Controller/Admin/Bodies.pm | 7 +++ perllib/FixMyStreet/DB/Result/Contact.pm | 4 ++ t/app/controller/admin/bodies.t | 65 ++++++++++++++++++++++ templates/web/base/admin/bodies/contact-form.html | 7 +++ 5 files changed, 84 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e53c176fe..852d5bb17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ - Do not display deleted priorities in inspect form. #3195 - Include extra fields in submit emails. - can remove staff status from users in bulk + - super users can mark category names as hard coded - Development improvements: - `#geolocate_link` is now easier to re-style. #3006 - Links inside `#front-main` can be customised using `$primary_link_*` Sass variables. #3007 diff --git a/perllib/FixMyStreet/App/Controller/Admin/Bodies.pm b/perllib/FixMyStreet/App/Controller/Admin/Bodies.pm index f1a6f938b..31a717a9c 100644 --- a/perllib/FixMyStreet/App/Controller/Admin/Bodies.pm +++ b/perllib/FixMyStreet/App/Controller/Admin/Bodies.pm @@ -283,6 +283,13 @@ sub update_contact : Private { $contact->unset_extra_metadata($_); } } + if ( $c->user->is_superuser ) { + if ( $c->get_param('hardcoded') ) { + $contact->set_extra_metadata( hardcoded => 1 ); + } else { + $contact->unset_extra_metadata('hardcoded'); + } + } if ( my @group = $c->get_param_list('group') ) { @group = grep { $_ } @group; if (scalar @group == 0) { diff --git a/perllib/FixMyStreet/DB/Result/Contact.pm b/perllib/FixMyStreet/DB/Result/Contact.pm index 5cf3302dd..69f8886eb 100644 --- a/perllib/FixMyStreet/DB/Result/Contact.pm +++ b/perllib/FixMyStreet/DB/Result/Contact.pm @@ -191,12 +191,16 @@ sub sent_by_open311 { # We do not want to allow editing of a category's name # if it's Open311, unless it's marked as protected +# Also prevent editing of hardcoded categories sub category_uneditable { my $self = shift; return 1 if $self->in_storage && !$self->get_extra_metadata('open311_protect') && $self->sent_by_open311; + return 1 if + $self->in_storage + && $self->get_extra_metadata('hardcoded'); return 0; } diff --git a/t/app/controller/admin/bodies.t b/t/app/controller/admin/bodies.t index 542c3f4c0..811ac4362 100644 --- a/t/app/controller/admin/bodies.t +++ b/t/app/controller/admin/bodies.t @@ -15,6 +15,10 @@ my $mech = FixMyStreet::TestMech->new; my $superuser = $mech->create_user_ok('superuser@example.com', name => 'Super User', is_superuser => 1); $mech->log_in_ok( $superuser->email ); my $body = $mech->create_body_ok(2650, 'Aberdeen City Council'); +my $body2 = $mech->create_body_ok(2237, 'Oxfordshire County Council'); + +my $user = $mech->create_user_ok('user@example.com', name => 'OCC User', from_body => $body2); +$user->user_body_permissions->create({ body => $body2, permission_type => 'category_edit' }); # This override is wrapped around ALL the /admin/body tests FixMyStreet::override_config { @@ -117,6 +121,8 @@ subtest 'check contact renaming' => sub { $mech->submit_form_ok( { with_fields => { category => 'test category' } } ); }; + + subtest 'check contact updating' => sub { $mech->get_ok('/admin/body/' . $body->id . '/test%20category'); $mech->content_like(qr{test2\@example.com[^<]*[^<]*unconfirmed}s); @@ -442,4 +448,63 @@ subtest 'check update disallowed message' => sub { }; }; +subtest 'check hardcoded contact renaming' => sub { + FixMyStreet::override_config { + MAPIT_URL => 'http://mapit.uk/', + 'ALLOWED_COBRANDS' => [ 'oxfordshire' ], + }, sub { + my $contact = FixMyStreet::DB->resultset('Contact')->create( + { + body_id => $body2->id, + category => 'protected category', + state => 'confirmed', + editor => $0, + whenedited => \'current_timestamp', + note => 'protected contact', + email => 'protected@example.org', + } + ); + $contact->set_extra_metadata( 'hardcoded', 1 ); + $contact->update; + $mech->get_ok('/admin/body/' . $body2->id .'/protected%20category'); + $mech->content_contains( 'name="hardcoded"' ); + $mech->content_like( qr'value="protected category"[^>]*readonly's ); + $mech->submit_form_ok( { with_fields => { category => 'non protected category', note => 'rename category' } } ); + $mech->content_contains( 'protected category' ); + $mech->content_lacks( 'non protected category' ); + $mech->get('/admin/body/' . $body2->id . '/non%20protected%20category'); + is $mech->res->code, 404; + + $mech->get_ok('/admin/body/' . $body2->id .'/protected%20category'); + $mech->submit_form_ok( { with_fields => { hardcoded => 0, note => 'remove hardcoding' } } ); + $mech->get_ok('/admin/body/' . $body2->id .'/protected%20category'); + $mech->content_unlike( qr'value="protected category"[^>]*readonly's ); + $mech->submit_form_ok( { with_fields => { category => 'non protected category', note => 'rename category' } } ); + $mech->content_contains( 'non protected category' ); + $mech->get_ok('/admin/body/' . $body2->id . '/non%20protected%20category'); + $mech->get('/admin/body/' . $body2->id . '/protected%20category'); + is $mech->res->code, 404; + + $contact->discard_changes; + $contact->set_extra_metadata( 'hardcoded', 1 ); + $contact->update; + + $mech->log_out_ok( $superuser->email ); + $mech->log_in_ok( $user->email ); + $mech->get_ok('/admin/body/' . $body2->id . '/non%20protected%20category'); + $mech->content_lacks( 'name="hardcoded"' ); + $user->update( { is_superuser => 1 } ); + $mech->get_ok('/admin/body/' . $body2->id . '/non%20protected%20category'); + $mech->content_contains('name="hardcoded"' ); + $user->update( { is_superuser => 0 } ); + $mech->submit_form_ok( { with_fields => { hardcoded => 0, note => 'remove hardcoding' } } ); + $mech->content_lacks( 'name="hardcoded"' ); + + $contact->discard_changes; + is $contact->get_extra_metadata('hardcoded'), 1, "non superuser can't remove hardcoding"; + + $mech->log_out_ok( $user->email ); + }; +}; + done_testing(); diff --git a/templates/web/base/admin/bodies/contact-form.html b/templates/web/base/admin/bodies/contact-form.html index e1fd2054a..fab485a2f 100644 --- a/templates/web/base/admin/bodies/contact-form.html +++ b/templates/web/base/admin/bodies/contact-form.html @@ -89,6 +89,13 @@

[% END %] + [% IF c.user.is_superuser %] +

+ + +

+ [% END %] +

-- cgit v1.2.3