prevent editing of category names with harcoded metadata

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/societyworks#1992
mysociety · Oct 23, 2020 · a15907f · a15907f
1 parent 9150a5f
commit a15907f
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 1 deletion.
diff --git a/perllib/FixMyStreet/App/Controller/Admin/Bodies.pm b/perllib/FixMyStreet/App/Controller/Admin/Bodies.pm
@@ -276,7 +276,7 @@ sub update_contact : Private {
     $contact->send_method( $c->get_param('send_method') );
 
     # Set flags in extra to the appropriate values
-    foreach (qw(photo_required open311_protect updates_disallowed reopening_disallowed assigned_users_only anonymous_allowed)) {
+    foreach (qw(photo_required open311_protect updates_disallowed reopening_disallowed assigned_users_only anonymous_allowed hardcoded)) {
         if ( $c->get_param($_) ) {
             $contact->set_extra_metadata( $_ => 1 );
         } else {

diff --git 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
@@ -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</strong>[^<]*</td>[^<]*<td>unconfirmed}s);
@@ -442,4 +448,49 @@ 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;
+
+        $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"' );
+        $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
@@ -89,6 +89,13 @@
       </p>
     [% END %]
 
+    [% IF c.user.is_superuser %]
+      <p class="form-check">
+          <input type="checkbox" name="hardcoded" value="1" id="hardcoded"[% ' checked' IF contact.get_extra_metadata('hardcoded') %]>
+          <label for="hardcoded">[% loc("Protect this category from being re-named") %]</label>
+      </p>
+    [% END %]
+
     <p class="form-check">
         <input type="checkbox" name="assigned_users_only" value="1" id="assigned_users_only" [% ' checked' IF contact.extra.assigned_users_only %]>
         <label for="assigned_users_only">[% loc('Frontend staff access only to users assigned to this category') %]</label>