aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--CHANGELOG.md1
-rw-r--r--perllib/FixMyStreet/App/Controller/Admin/Bodies.pm16
-rw-r--r--perllib/FixMyStreet/DB/Result/Contact.pm21
-rw-r--r--perllib/FixMyStreet/Roles/Extra.pm50
-rw-r--r--t/app/controller/admin/bodies.t25
-rw-r--r--t/app/model/extra.t25
-rw-r--r--templates/web/base/admin/bodies/contact-form.html164
-rw-r--r--web/cobrands/fixmystreet/admin.js17
-rw-r--r--web/cobrands/fixmystreet/fixmystreet.js12
-rw-r--r--web/cobrands/sass/_base.scss41
10 files changed, 257 insertions, 115 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6a171366e..4ddb95d9d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -17,6 +17,7 @@
- Add new roles system, to group permissions and apply to users. #2483
- Contact form emails now include user admin links.
- Allow categories/Open311 questions to disable the reporting form. #2599
+ - Improve category edit form. #2469
- New features:
- Categories can be listed under more than one group #2475
- OpenID Connect login support. #2523
diff --git a/perllib/FixMyStreet/App/Controller/Admin/Bodies.pm b/perllib/FixMyStreet/App/Controller/Admin/Bodies.pm
index 8ca6bbc22..fa5a55213 100644
--- a/perllib/FixMyStreet/App/Controller/Admin/Bodies.pm
+++ b/perllib/FixMyStreet/App/Controller/Admin/Bodies.pm
@@ -288,6 +288,22 @@ sub update_contacts : Private {
$c->forward('/admin/update_extra_fields', [ $contact ]);
$c->forward('contact_cobrand_extra_fields', [ $contact ]);
+ # Special form disabling form
+ if ($c->get_param('disable')) {
+ my $msg = $c->get_param('disable_message');
+ $errors{category} = _('Please enter a message') unless $msg;
+ my $meta = {
+ code => '_fms_disable_',
+ variable => 'false',
+ protected => 'true',
+ disable_form => 'true',
+ description => $msg,
+ };
+ $contact->update_extra_field($meta);
+ } else {
+ $contact->remove_extra_field('_fms_disable_');
+ }
+
if ( %errors ) {
$c->stash->{updated} = _('Please correct the errors below');
$c->stash->{contact} = $contact;
diff --git a/perllib/FixMyStreet/DB/Result/Contact.pm b/perllib/FixMyStreet/DB/Result/Contact.pm
index 3ce0ec66f..bc91c84ee 100644
--- a/perllib/FixMyStreet/DB/Result/Contact.pm
+++ b/perllib/FixMyStreet/DB/Result/Contact.pm
@@ -98,7 +98,7 @@ sub category_display {
$self->translate_column('category');
}
-sub get_metadata_for_editing {
+sub get_all_metadata {
my $self = shift;
my @metadata = @{$self->get_extra_fields};
@@ -111,9 +111,19 @@ sub get_metadata_for_editing {
return \@metadata;
}
+sub get_metadata_for_editing {
+ my $self = shift;
+ my $metadata = $self->get_all_metadata;
+
+ # Ignore the special admin-form-created entry
+ my @metadata = grep { $_->{code} ne '_fms_disable_' } @$metadata;
+
+ return \@metadata;
+}
+
sub get_metadata_for_input {
my $self = shift;
- my $metadata = $self->get_metadata_for_editing;
+ my $metadata = $self->get_all_metadata;
# Also ignore any we have with a 'server_set' automated attribute
my @metadata = grep { !$_->{automated} || $_->{automated} ne 'server_set' } @$metadata;
@@ -136,4 +146,11 @@ sub id_field {
return $self->get_extra_metadata('id_field') || 'fixmystreet_id';
}
+sub disable_form_field {
+ my $self = shift;
+ my $metadata = $self->get_all_metadata;
+ my ($field) = grep { $_->{code} eq '_fms_disable_' } @$metadata;
+ return $field;
+}
+
1;
diff --git a/perllib/FixMyStreet/Roles/Extra.pm b/perllib/FixMyStreet/Roles/Extra.pm
index 445f6d91c..883ac2fd7 100644
--- a/perllib/FixMyStreet/Roles/Extra.pm
+++ b/perllib/FixMyStreet/Roles/Extra.pm
@@ -135,6 +135,56 @@ sub push_extra_fields {
$self->extra({ %$extra, $META_FIELD => [ @$existing, @fields ] });
}
+=head2 update_extra_field
+
+ $problem->update_extra_field( { ... } );
+
+Given an extra field, will replace one with the same code in the
+existing list of fields, or add to the end if not present.
+
+=cut
+
+sub update_extra_field {
+ my ($self, $field) = @_;
+
+ # Can operate on list that uses code (Contact) or name (Problem),
+ # but make sure we have one of them
+ my $attr;
+ $attr = 'code' if $field->{code};
+ $attr = 'name' if $field->{name};
+ die unless $attr;
+
+ my $existing = $self->get_extra_fields;
+ my $found;
+ foreach (@$existing) {
+ if ($_->{$attr} eq $field->{$attr}) {
+ $_ = $field;
+ $found = 1;
+ }
+ }
+ if (!$found) {
+ push @$existing, $field;
+ }
+
+ $self->set_extra_fields(@$existing);
+}
+
+=head2 remove_extra_field
+
+ $problem->remove_extra_field( $code );
+
+Given an extra field code, will remove it from the list of fields.
+
+=cut
+
+sub remove_extra_field {
+ my ($self, $code) = @_;
+
+ my @fields = @{ $self->get_extra_fields() };
+ @fields = grep { ($_->{code} || $_->{name}) ne $code } @fields;
+ $self->set_extra_fields(@fields);
+}
+
=head1 HELPER METHODS
For internal use mostly.
diff --git a/t/app/controller/admin/bodies.t b/t/app/controller/admin/bodies.t
index a07f19494..75f0f606f 100644
--- a/t/app/controller/admin/bodies.t
+++ b/t/app/controller/admin/bodies.t
@@ -217,6 +217,24 @@ subtest 'check text output' => sub {
$mech->content_lacks('<body');
};
+subtest 'disable form message editing' => sub {
+ $mech->get_ok('/admin/body/' . $body->id . '/test%20category');
+ $mech->submit_form_ok( { with_fields => {
+ disable => 1,
+ disable_message => 'Please ring us!',
+ note => 'Adding emergency message',
+ } } );
+ $mech->content_contains('Values updated');
+ my $contact = $body->contacts->find({ category => 'test category' });
+ is_deeply $contact->get_extra_fields, [{
+ description => 'Please ring us!',
+ code => '_fms_disable_',
+ protected => 'true',
+ variable => 'false',
+ disable_form => 'true',
+ }], 'right message added';
+};
+
}; # END of override wrap
@@ -230,7 +248,7 @@ FixMyStreet::override_config {
}, sub {
subtest 'group editing works' => sub {
$mech->get_ok('/admin/body/' . $body->id);
- $mech->content_contains( 'group</strong> is used for the top-level category' );
+ $mech->content_contains('Parent categories');
$mech->submit_form_ok( { with_fields => {
category => 'grouped category',
@@ -247,7 +265,7 @@ FixMyStreet::override_config {
subtest 'group can be unset' => sub {
$mech->get_ok('/admin/body/' . $body->id);
- $mech->content_contains( 'group</strong> is used for the top-level category' );
+ $mech->content_contains('Parent categories');
$mech->submit_form_ok( { with_fields => {
category => 'grouped category',
@@ -270,12 +288,11 @@ FixMyStreet::override_config {
BASE_URL => 'http://www.example.org',
COBRAND_FEATURES => {
category_groups => { default => 1 },
- multiple_category_groups => { default => 1 },
}
}, sub {
subtest 'multi group editing works' => sub {
$mech->get_ok('/admin/body/' . $body->id);
- $mech->content_contains( 'group</strong> is used for the top-level category' );
+ $mech->content_contains('Parent categories');
# have to do this as a post as adding a second group requires
# javascript
diff --git a/t/app/model/extra.t b/t/app/model/extra.t
index 55accb086..970efc465 100644
--- a/t/app/model/extra.t
+++ b/t/app/model/extra.t
@@ -79,6 +79,31 @@ subtest 'Default hash layout' => sub {
is_deeply $contact->extra, { _fields => \@fields }, '(sanity check layout)';
};
+ subtest 'updating extra field' => sub {
+ my $contact = get_test_contact();
+ my @fields = ( { code => 'ABC', description => 'ABC', variable => 'false', }, { code => 'DEF', description => 'DEF', variable => 'true' } );
+ $contact->set_extra_fields(@fields);
+ is_deeply $contact->get_extra_fields, \@fields, 'extra fields set...';
+ my $new_field = { code => 'ABC', description => 'XYZ', variable => 'false' };
+ $contact->update_extra_field($new_field);
+ $fields[0] = $new_field;
+ is_deeply $contact->get_extra_fields, \@fields, 'extra fields changed';
+ $new_field = { code => 'GHI', description => 'GHI', variable => 'false' };
+ $contact->update_extra_field($new_field);
+ push @fields, $new_field;
+ is_deeply $contact->get_extra_fields, \@fields, 'extra fields changed';
+ };
+
+ subtest 'removing extra field' => sub {
+ my $contact = get_test_contact();
+ my @fields = ( { code => 'ABC', description => 'ABC', variable => 'false', }, { code => 'DEF', description => 'DEF', variable => 'true' } );
+ $contact->set_extra_fields(@fields);
+ is_deeply $contact->get_extra_fields, \@fields, 'extra fields set...';
+ $contact->remove_extra_field('DEF');
+ pop @fields;
+ is_deeply $contact->get_extra_fields(), \@fields, 'extra field removed';
+ };
+
subtest 'metadata' => sub {
my $contact = get_test_contact();
is_deeply $contact->get_extra_metadata, {}, 'No extra metadata';
diff --git a/templates/web/base/admin/bodies/contact-form.html b/templates/web/base/admin/bodies/contact-form.html
index d3b100e15..7a58644e5 100644
--- a/templates/web/base/admin/bodies/contact-form.html
+++ b/templates/web/base/admin/bodies/contact-form.html
@@ -22,81 +22,74 @@
[% INCLUDE 'admin/bodies/_translations.html' %]
- <div class="admin-hint">
- <p>
- [% loc("The <strong>email address</strong> is the destination to which reports about this category will be sent.
- Other categories for this body may have the same email address.") %]
+ <label for="destination">[% loc('Destination') %]</label>
+ <p class="form-hint" id="destination-hint">
+ [% IF body.can_be_devolved %]
+ [% loc('An email address or service ID (Open311 or similar).') %]
+ [% ELSIF body.send_method == 'Open311' %]
+ [% loc('A service ID (Open311 or similar).') %]
+ [% ELSIF body.send_method.match('Email') OR NOT body.send_method %]
+ [% loc('An email address.') %]
+ [% ELSE %]
+ [% loc('An email address or service ID (Open311 or similar).') %]
+ [% END %]
</p>
- [% IF (body.send_method AND body.send_method.match('Email')) OR body.can_be_devolved %]
- <p>
- [% loc("If you're using <strong>a send method that is not email</strong>, enter the service ID (Open311) or equivalent identifier here.") %]
- </p>
- [% END %]
- </div>
+ <input type="text" class="form-control" id="destination" aria-describedby="destination-hint" name="email" size="30" value="[% contact.email | html %]" required>
- <p>
- <strong>[% loc('Email address') %] </strong><input type="text" class="form-control" name="email" size="30" value="[% contact.email | html %]" required>
- </p>
+ <fieldset>
+ <legend>[% loc('State') %]</legend>
+ <div class="form-check form-check--inline">
+ <input type="radio" name="state" id="state-unconfirmed" aria-describedby="state-unconfirmed-hint" value="unconfirmed"[% ' checked' IF contact.state == 'unconfirmed' %]>
+ <label for="state-unconfirmed">[% loc('Unconfirmed') %]</label>
+ <p class="form-hint" id="state-unconfirmed-hint">[% loc('You are not sure of the origin or validity of the contact.') %]</p>
+ </div>
+ <div class="form-check form-check--inline">
+ <input type="radio" name="state" id="state-confirmed" aria-describedby="state-confirmed-hint" value="confirmed"[% ' checked' IF contact.state == 'confirmed' || contact.state == "" %]>
+ <label for="state-confirmed">[% loc('Confirmed') %]</label>
+ <p class="form-hint" id="state-confirmed-hint">[% loc('The contact has been confirmed as correct.') %]</p>
+ </div>
+ <div class="form-check form-check--inline">
+ <input type="radio" name="state" id="state-inactive" aria-describedby="state-inactive-hint" value="inactive"[% ' checked' IF contact.state == 'inactive' %]>
+ <label for="state-inactive">[% loc('Inactive') %]</label>
+ <p class="form-hint" id="state-inactive-hint">[% loc('Prevent new reports from using this category, but keep it available in map filters.') %]</p>
+ </div>
+ <div class="form-check form-check--inline">
+ <input type="radio" name="state" id="state-deleted" aria-describedby="state-deleted-hint" value="deleted"[% ' checked' IF contact.state == 'deleted' %]>
+ <label for="state-deleted">[% loc('Deleted') %]</label>
+ <p class="form-hint" id="state-deleted-hint">[% loc('Prevent new reports from using this category, <em>and</em> also remove it from map filters.') %]</p>
+ </div>
+ </fieldset>
- <div class="admin-hint">
- <p>
- [%
-loc("Use <strong>confirmed</strong> to indicate that this contact has been
-confirmed as correct. If you are not sure of the origin or validity of the
-contact, use <strong>unconfirmed</strong>. <strong>inactive</strong> will
-remove the category from use when reporting problems, but keep it available in
-map filters, and <strong>deleted</strong> will remove the category from there
-as well.") %]
+ <p class="form-check">
+ <input type="checkbox" name="non_public" value="1" id="non_public" [% ' checked' IF contact.non_public %]>
+ <label for="non_public">[% loc('Hide reports made in this category') %]</label>
+ <span class='form-hint'>[% loc('Use this for issues that you want to allow users to report, but for which there is no public interest in displaying the report, like requesting an extra rubbish bin at a specific address.') %]</span>
</p>
- </div>
- <p>
- <label for="state">[% loc('State') %]</label>
- <select name="state" id="state">
- <option value="unconfirmed"[% ' selected' IF contact.state == 'unconfirmed' %]>[% loc('Unconfirmed') %]
- <option value="confirmed"[% ' selected' IF contact.state == 'confirmed' || contact.state == "" %]>[% loc('Confirmed') %]
- <option value="inactive"[% ' selected' IF contact.state == 'inactive' %]>[% loc('Inactive') %]
- <option value="deleted"[% ' selected' IF contact.state == 'deleted' %]>[% loc('Deleted') %]
- </select>
- </p>
- <div class="admin-hint">
- <p>
- [% loc("Check <strong>private</strong> if reports in this category should <strong>never be displayed on the website</strong>.
- <br>
- Normally, categories are not private.
- <br>
- This is suitable for issues that you want to allow users to report to the body, but for which there is no public
- interest in displaying the report. In the UK, we've used this for services like requesting an extra rubbish bin
- at a specific address.") %]
+ <p class="form-check">
+ <input type="checkbox" name="disable" value="1" id="disable" data-toggle-visibility="#js-disable-form-message-box" [% ' checked' IF contact.disable_form_field %]>
+ <label for="disable">[% loc('Disable form when this category is selected') %]</label>
</p>
- </div>
- <p>
- <input type="checkbox" name="non_public" value="1" id="non_public" [% ' checked' IF contact.non_public %]>
- <label for="non_public" class="inline">[% loc('Private') %]</label>
- </p>
- <div class="admin-hint">
- <p>
- [% loc("Check <strong>inspection required</strong> if reports in this category <strong>must be inspected</strong> before being sent.") %]
+ <p class="form-group form-group--indented [% 'hidden-js' IF NOT contact.disable_form_field %]" id="js-disable-form-message-box">
+ <label for="disabled-message">[% loc('Message to show when form is disabled (HTML permitted)') %]</label>
+ <textarea id="disabled-message" name="disable_message" class="form-control">[% contact.disable_form_field.description %]</textarea>
</p>
- </div>
- <p>
- <input type="checkbox" name="inspection_required" value="1" id="inspection_required" [% 'checked' IF contact.get_extra_metadata('inspection_required') %]>
- <label for="inspection_required" class="inline">[% loc('Inspection required') %]</label>
- </p>
- <div class="admin-hint [% 'hidden' UNLESS contact.get_extra_metadata('inspection_required') %]">
- <p>
- [% loc("Reports will automatically be sent without needing to be inspected if the user's <strong>reputation</strong> is at or above this value. Set to <strong>0</strong> if all reports must be inspected regardless.") %]
+ <p class="form-check">
+ <input type="checkbox" name="inspection_required" value="1" id="inspection_required" data-toggle-visibility="#js-inspection-reputation-box" [% 'checked' IF contact.get_extra_metadata('inspection_required') %]>
+ <label for="inspection_required">[% loc('Reports in this category must be inspected before being sent') %]</label>
+ </p>
+
+ <p class="form-group form-group--indented [% 'hidden-js' IF NOT contact.get_extra_metadata('inspection_required') %]" id="js-inspection-reputation-box">
+ <label for="reputation_threshold">[% loc('Reputation threshold') %]</label>
+ <span class="form-hint" id="reputation_threshold_hint">
+ [% loc("Reports will automatically be sent without needing to be inspected if the user's <strong>reputation</strong> is at or above this value. Set to <strong>0</strong> if all reports must be inspected regardless.") %]
+ </span>
+ <input type="text" class="form-control" name="reputation_threshold" id="reputation_threshold"
+ value="[% contact.get_extra_metadata('reputation_threshold') | html %]" size="30"
+ aria-describedby="reputation_threshold_hint">
</p>
- </div>
- <p [% 'class=hidden' UNLESS contact.get_extra_metadata('inspection_required') %]>
- <label>
- [% loc('Reputation threshold') %]
- <input type="text" class="form-control" name="reputation_threshold" id="reputation_threshold"
- value="[% contact.get_extra_metadata('reputation_threshold') | html %]" size="30">
- </label>
- </p>
[% IF body.can_be_devolved %]
<div class="admin-hint">
@@ -121,17 +114,9 @@ as well.") %]
[% END %]
[% IF c.cobrand.enable_category_groups %]
- <div class="admin-hint">
- <p>
- [% loc(
- "The <strong>group</strong> is used for the top-level category field when
- subcategory grouping is enabled."
- ) %]
- </p>
- </div>
<p>
<label>
- [% loc('Group') %]
+ [% loc('Parent categories') %]
[% IF contact.extra.group %]
[% FOR group IN contact.extra.group %]
<input class="form-control" type="text" name="group" value="[% group | html %]" size="30">
@@ -141,24 +126,25 @@ as well.") %]
[% END %]
<input class="hidden-js js-group-item-template form-control" type="text" name="group" value="" size="30">
<p class="hidden-nojs">
- <button type="button" class="btn btn--small js-group-item-add">[% loc('Add group') %]</button>
+ <button type="button" class="btn btn--small js-group-item-add">[% loc('Add another parent category') %]</button>
</p>
</label>
</p>
[% END %]
+ <h2>[% loc('Extra data:') %] </h2>
+ <dl>
+ [% FOR pair IN contact.get_extra_metadata %]
+ <dt>[% pair.key %]</dt> <dd>[% pair.value OR '<em>-</em>' %]</dd>
+ [% END %]
+ </dl>
+ [% INCLUDE 'admin/extra-metadata-form.html' metas=(contact.get_metadata_for_editing OR []) %]
- <div class="admin-hint">
- <p>
- [% loc("Use this field to record details that are only displayed in the admin. Input is not shown publicly, and is not sent to the body.") %]
+ <p class="form-group">
+ <label for="note">[% loc('Summarise your changes') %]</label>
+ <span class="form-hint" id="note-hint">[% loc("If you’ve made changes, leave a note explaining what, for other admins to see.") %]</span>
+ <input class="form-control" type="text" id="note" name="note" size="30" aria-describedby="note-hint" required>
</p>
- </div>
- <p>
- <label>
- [% loc('Summarise your changes') %]
- <input class="form-control" type="text" name="note" size="30" required>
- </label>
- </p>
<p>
<input type="hidden" name="posted" value="new" >
@@ -166,11 +152,5 @@ as well.") %]
<input type="submit" class="btn" name="Create category" value="[% contact.in_storage ? loc('Save changes') : loc('Create category') %]" >
</p>
- <h2>[% loc('Extra data:') %] </h2>
- <dl>
- [% FOR pair IN contact.get_extra_metadata %]
- <dt>[% pair.key %]</dt> <dd>[% pair.value %]</dd>
- [% END %]
- </dl>
- [% INCLUDE 'admin/extra-metadata-form.html' metas=(contact.get_metadata_for_editing OR []) %]
+
</form>
diff --git a/web/cobrands/fixmystreet/admin.js b/web/cobrands/fixmystreet/admin.js
index 4ed9b1866..b598f52dd 100644
--- a/web/cobrands/fixmystreet/admin.js
+++ b/web/cobrands/fixmystreet/admin.js
@@ -106,23 +106,6 @@ $(function(){
}
}).change();
- // On category edit page, hide the reputation input if inspection isn't required
- $("form#category_edit #inspection_required").change(function() {
- var $p = $("form#category_edit #reputation_threshold").closest("p");
- var $hint = $p.prevUntil().first();
- if (this.checked) {
- $p.removeClass("hidden");
- if ($hint.length) {
- $hint.removeClass("hidden");
- }
- } else {
- $p.addClass("hidden");
- if ($hint.length) {
- $hint.addClass("hidden");
- }
- }
- });
-
// Bits for the report extra fields form builder:
// Reveal the UI when 'show' link is clicked
diff --git a/web/cobrands/fixmystreet/fixmystreet.js b/web/cobrands/fixmystreet/fixmystreet.js
index cda196864..c7749c729 100644
--- a/web/cobrands/fixmystreet/fixmystreet.js
+++ b/web/cobrands/fixmystreet/fixmystreet.js
@@ -991,6 +991,18 @@ $.extend(fixmystreet.set_up, {
}
},
+ toggle_visibility: function() {
+ $('input[type="checkbox"][data-toggle-visibility]').each(function(){
+ var input = this;
+ var $target = $( $(this).attr('data-toggle-visibility') );
+ var update = function() {
+ $target.toggleClass('hidden-js', ! input.checked );
+ };
+ $(input).on('change', update);
+ update();
+ });
+ },
+
form_section_previews: function() {
$('.js-form-section-preview').each(function(){
var $el = $(this);
diff --git a/web/cobrands/sass/_base.scss b/web/cobrands/sass/_base.scss
index 063136eb8..63ff19524 100644
--- a/web/cobrands/sass/_base.scss
+++ b/web/cobrands/sass/_base.scss
@@ -331,6 +331,45 @@ select.form-control {
max-width: 27em;
}
+// Handy for lining up "child" form groups below show/hide checkboxes.
+.form-group--indented {
+ padding-left: 2em;
+}
+
+.form-check {
+ margin-top: 1.5em; // match margin-top on regular labels
+ padding-left: 2em;
+ position: relative;
+
+ & > input {
+ position: absolute;
+ top: 0.3em;
+ left: 0.5em;
+ }
+
+ label {
+ margin-top: 0.5em;
+ }
+}
+
+.form-check--inline {
+ margin: 0.5em 0;
+
+ label {
+ margin: 0;
+ display: inline;
+ }
+
+ .form-hint {
+ display: inline;
+
+ &:before {
+ content: "\2013";
+ margin: 0 0.3em 0 0.1em;
+ }
+ }
+}
+
.required-text {
position: absolute;
#{$right}: 0;
@@ -2030,7 +2069,9 @@ label .muted {
}
.form-hint {
+ display: block;
color: #666;
+ margin-bottom: 0.5em;
// Reduce space between labels and their form-hints
label + & {
generated by cgit v1.2.3 (git 2.25.1) at 2025-07-22 12:52:01 +0000