diff options
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Admin/ResponsePriorities.pm | 140 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Admin/Roles.pm | 11 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Form/ResponsePriority.pm | 50 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Form/Role.pm | 1 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Form/Widget/Field/CheckboxGroup.pm | 6 | ||||
-rw-r--r-- | t/app/controller/admin/priorities.t | 43 | ||||
-rw-r--r-- | templates/web/base/admin/responsepriorities/edit.html | 57 | ||||
-rw-r--r-- | templates/web/base/admin/responsepriorities/index.html | 58 | ||||
-rw-r--r-- | templates/web/base/admin/responsepriorities/list.html | 36 | ||||
-rw-r--r-- | templates/web/base/admin/roles/index.html | 4 |
10 files changed, 221 insertions, 185 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Admin/ResponsePriorities.pm b/perllib/FixMyStreet/App/Controller/Admin/ResponsePriorities.pm index 5077fe78f..5e2908290 100644 --- a/perllib/FixMyStreet/App/Controller/Admin/ResponsePriorities.pm +++ b/perllib/FixMyStreet/App/Controller/Admin/ResponsePriorities.pm @@ -4,98 +4,94 @@ use namespace::autoclean; BEGIN { extends 'Catalyst::Controller'; } +use FixMyStreet::App::Form::ResponsePriority; -sub index : Path : Args(0) { - my ( $self, $c ) = @_; +sub auto :Private { + my ($self, $c) = @_; my $user = $c->user; - if ($user->is_superuser) { - $c->forward('/admin/fetch_all_bodies'); - } elsif ( $user->from_body ) { - $c->forward('load_user_body', [ $user->from_body->id ]); - $c->res->redirect( $c->uri_for( '', $c->stash->{body}->id ) ); - } else { - $c->detach( '/page_error_404_not_found' ); + $c->stash(rs => $c->model('DB::ResponsePriority')->search_rs(undef, { + prefetch => 'body', + order_by => ['body.name', 'me.name'] + })); + } elsif ($user->from_body) { + $c->stash(rs => $user->from_body->response_priorities->search_rs(undef, { + order_by => 'name' + })); } } -sub list : Path : Args(1) { - my ($self, $c, $body_id) = @_; - - $c->forward('load_user_body', [ $body_id ]); +sub index : Path : Args(0) { + my ( $self, $c ) = @_; - my @priorities = $c->stash->{body}->response_priorities->search( - undef, - { - order_by => 'name' - } + if (my $body_id = $c->get_param('body_id')) { + $c->res->redirect($c->uri_for($self->action_for('create'), [ $body_id ])); + $c->detach; + } + if ($c->user->is_superuser) { + $c->forward('/admin/fetch_all_bodies'); + } + $c->stash( + response_priorities => [ $c->stash->{rs}->all ], ); - - $c->stash->{response_priorities} = \@priorities; } -sub edit : Path : Args(2) { - my ( $self, $c, $body_id, $priority_id ) = @_; - - $c->forward('load_user_body', [ $body_id ]); +sub body :PathPart('admin/responsepriorities') :Chained :CaptureArgs(1) { + my ($self, $c, $body_id) = @_; - my $priority; - if ($priority_id eq 'new') { - $priority = $c->stash->{body}->response_priorities->new({}); - } - else { - $priority = $c->stash->{body}->response_priorities->find( $priority_id ) - or $c->detach( '/page_error_404_not_found' ); + my $user = $c->user; + if ($user->is_superuser) { + $c->stash->{body} = $c->model('DB::Body')->find($body_id); + } elsif ($user->from_body && $user->from_body->id == $body_id) { + $c->stash->{body} = $user->from_body; } - $c->forward('/admin/fetch_contacts'); - my @contacts = $priority->contacts->all; - my @live_contacts = $c->stash->{live_contacts}->all; - my %active_contacts = map { $_->id => 1 } @contacts; - my @all_contacts = map { { - id => $_->id, - category => $_->category, - active => $active_contacts{$_->id}, - } } @live_contacts; - $c->stash->{contacts} = \@all_contacts; - - if ($c->req->method eq 'POST') { - $priority->deleted( $c->get_param('deleted') ? 1 : 0 ); - $priority->name( $c->get_param('name') ); - $priority->description( $c->get_param('description') ); - $priority->external_id( $c->get_param('external_id') ); - $priority->is_default( $c->get_param('is_default') ? 1 : 0 ); - $priority->update_or_insert; - - my @live_contact_ids = map { $_->id } @live_contacts; - my @new_contact_ids = grep { $c->get_param("contacts[$_]") } @live_contact_ids; - $priority->contact_response_priorities->search({ - contact_id => { -not_in => \@new_contact_ids }, - })->delete; - foreach my $contact_id (@new_contact_ids) { - $priority->contact_response_priorities->find_or_create({ - contact_id => $contact_id, - }); - } - - $c->res->redirect( $c->uri_for( '', $c->stash->{body}->id ) ); - } + $c->detach( '/page_error_404_not_found' ) unless $c->stash->{body}; +} - $c->stash->{response_priority} = $priority; +sub create :Chained('body') :Args(0) { + my ($self, $c) = @_; + + my $priority = $c->stash->{rs}->new_result({ body => $c->stash->{body} }); + return $self->form($c, $priority); } -sub load_user_body : Private { - my ($self, $c, $body_id) = @_; +sub item :PathPart('') :Chained('body') :CaptureArgs(1) { + my ($self, $c, $id) = @_; - my $has_permission = $c->user->has_body_permission_to('responsepriority_edit', $body_id); + my $obj = $c->stash->{rs}->find($id) + or $c->detach('/page_error_404_not_found', []); + $c->stash(obj => $obj); +} - unless ( $has_permission ) { - $c->detach( '/page_error_404_not_found' ); - } +sub edit :PathPart('') :Chained('item') :Args(0) { + my ($self, $c) = @_; + return $self->form($c, $c->stash->{obj}); +} + +sub form { + my ($self, $c, $priority) = @_; - $c->stash->{body} = $c->model('DB::Body')->find($body_id) - or $c->detach( '/page_error_404_not_found' ); + # Otherwise, the form includes contacts for *all* bodies + $c->forward('/admin/fetch_contacts'); + my @all_contacts = map { + { value => $_->id, label => $_->category } + } $c->stash->{live_contacts}->all; + + my $opts = { + field_list => [ + '+contacts' => { options => \@all_contacts }, + ], + body_id => $c->stash->{body}->id, + }; + + my $form = FixMyStreet::App::Form::ResponsePriority->new(%$opts); + $c->stash(template => 'admin/responsepriorities/edit.html', form => $form); + $form->process(item => $priority, params => $c->req->params); + return unless $form->validated; + + $c->response->redirect($c->uri_for($self->action_for('index'))); } __PACKAGE__->meta->make_immutable; diff --git a/perllib/FixMyStreet/App/Controller/Admin/Roles.pm b/perllib/FixMyStreet/App/Controller/Admin/Roles.pm index 15c96a4ed..902ed6255 100644 --- a/perllib/FixMyStreet/App/Controller/Admin/Roles.pm +++ b/perllib/FixMyStreet/App/Controller/Admin/Roles.pm @@ -11,7 +11,10 @@ sub auto :Private { my $user = $c->user; if ($user->is_superuser) { - $c->stash(rs => $c->model('DB::Role')->search_rs({}, { join => 'body', order_by => ['body.name', 'me.name'] })); + $c->stash(rs => $c->model('DB::Role')->search_rs({}, { + prefetch => 'body', + order_by => ['body.name', 'me.name'] + })); } elsif ($user->from_body) { $c->stash(rs => $user->from_body->roles->search_rs({}, { order_by => 'name' })); } @@ -36,7 +39,7 @@ sub index :Path :Args(0) { } sub create :Local :Args(0) { - my ($self, $c, $id) = @_; + my ($self, $c) = @_; my $role = $c->stash->{rs}->new_result({}); return $self->form($c, $role); @@ -60,7 +63,7 @@ sub form { if ($c->get_param('delete_role')) { $role->delete; - $c->response->redirect($c->uri_for($self->action_for('list'))); + $c->response->redirect($c->uri_for($self->action_for('index'))); $c->detach; } @@ -90,7 +93,7 @@ sub form { $form->process(item => $role, params => $c->req->params); return unless $form->validated; - $c->response->redirect($c->uri_for($self->action_for('list'))); + $c->response->redirect($c->uri_for($self->action_for('index'))); } 1; diff --git a/perllib/FixMyStreet/App/Form/ResponsePriority.pm b/perllib/FixMyStreet/App/Form/ResponsePriority.pm new file mode 100644 index 000000000..9182bd7a1 --- /dev/null +++ b/perllib/FixMyStreet/App/Form/ResponsePriority.pm @@ -0,0 +1,50 @@ +package FixMyStreet::App::Form::ResponsePriority; + +use HTML::FormHandler::Moose; +use FixMyStreet::App::Form::I18N; +extends 'HTML::FormHandler::Model::DBIC'; +use namespace::autoclean; + +has 'body_id' => ( isa => 'Int', is => 'ro' ); + +has '+widget_name_space' => ( default => sub { ['FixMyStreet::App::Form::Widget'] } ); +has '+widget_tags' => ( default => sub { { wrapper_tag => 'p' } } ); +has '+item_class' => ( default => 'ResponsePriority' ); +has_field 'name' => ( required => 1 ); +has_field 'description'; +has_field 'external_id' => ( label => 'External ID' ); +has_field 'is_default' => ( + type => 'Checkbox', + option_label => 'Default priority', + do_label => 0, +); +has_field 'deleted' => ( + type => 'Checkbox', + option_label => 'Flag as deleted', + do_label => 0, +); +has_field 'contacts' => ( + type => 'Multiple', + widget => 'CheckboxGroup', + ul_class => 'no-bullets no-margin', + do_label => 0, + do_wrapper => 0, + tags => { inline => 1 }, +); + +before 'update_model' => sub { + my $self = shift; + $self->item->body_id($self->body_id); +}; + +sub _build_language_handle { FixMyStreet::App::Form::I18N->new } + +has '+unique_messages' => ( + default => sub { + { response_priorities_body_id_name_key => "Names must be unique" }; + } +); + +__PACKAGE__->meta->make_immutable; + +1; diff --git a/perllib/FixMyStreet/App/Form/Role.pm b/perllib/FixMyStreet/App/Form/Role.pm index f0711af15..0b0d20703 100644 --- a/perllib/FixMyStreet/App/Form/Role.pm +++ b/perllib/FixMyStreet/App/Form/Role.pm @@ -15,6 +15,7 @@ has_field 'body' => ( type => 'Select', empty_select => 'Select a body', require has_field 'permissions' => ( type => 'Multiple', widget => 'CheckboxGroup', + ul_class => 'permissions-checkboxes', tags => { inline => 1, wrapper_tag => 'fieldset', }, ); diff --git a/perllib/FixMyStreet/App/Form/Widget/Field/CheckboxGroup.pm b/perllib/FixMyStreet/App/Form/Widget/Field/CheckboxGroup.pm index 1dc55e49b..e755f1c11 100644 --- a/perllib/FixMyStreet/App/Form/Widget/Field/CheckboxGroup.pm +++ b/perllib/FixMyStreet/App/Form/Widget/Field/CheckboxGroup.pm @@ -4,11 +4,13 @@ use Moose::Role; with 'HTML::FormHandler::Widget::Field::CheckboxGroup'; use namespace::autoclean; +has ul_class => ( is => 'ro' ); + sub render_element { my ( $self, $result ) = @_; $result ||= $self->result; - my $output = '<ul class="permissions-checkboxes">'; + my $output = '<ul class="' . ($self->ul_class || '') . '">'; foreach my $option ( @{ $self->{options} } ) { if ( my $label = $option->{group} ) { $label = $self->_localize( $label ) if $self->localize_labels; @@ -23,7 +25,7 @@ sub render_element { $output .= qq{</ul>\n</li>}; } else { - $output .= $self->render_option( $option, $result ); + $output .= '<li>' . $self->render_option( $option, $result ) . '</li>'; } } $output .= '</ul>'; diff --git a/t/app/controller/admin/priorities.t b/t/app/controller/admin/priorities.t index 4eff20be7..8341e212d 100644 --- a/t/app/controller/admin/priorities.t +++ b/t/app/controller/admin/priorities.t @@ -16,14 +16,14 @@ $mech->log_in_ok( $superuser->email ); subtest "response priorities can be added" => sub { is $oxfordshire->response_priorities->count, 0, "No response priorities yet"; - $mech->get_ok( "/admin/responsepriorities/" . $oxfordshire->id . "/new" ); + $mech->get_ok( "/admin/responsepriorities/" . $oxfordshire->id . "/create" ); my $fields = { name => "Cat 1A", description => "Fixed within 24 hours", deleted => undef, is_default => undef, - "contacts[".$oxfordshirecontact->id."]" => 1, + contacts => $oxfordshirecontact->id, }; $mech->submit_form_ok( { with_fields => $fields } ); @@ -41,7 +41,7 @@ subtest "response priorities can set to default" => sub { description => "Fixed within 24 hours", deleted => undef, is_default => 1, - "contacts[".$oxfordshirecontact->id."]" => 1, + contacts => $oxfordshirecontact->id, }; $mech->submit_form_ok( { with_fields => $fields } ); @@ -51,49 +51,36 @@ subtest "response priorities can set to default" => sub { }; subtest "response priorities can be listed" => sub { - $mech->get_ok( "/admin/responsepriorities/" . $oxfordshire->id ); + $mech->get_ok( "/admin/responsepriorities" ); $mech->content_contains( $oxfordshire->response_priorities->first->name ); $mech->content_contains( $oxfordshire->response_priorities->first->description ); }; -subtest "response priorities are limited by body" => sub { - my $bromleypriority = $bromley->response_priorities->create( { - deleted => 0, - name => "Bromley Cat 0", - } ); - - 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 ); - - $mech->get_ok( "/admin/responsepriorities/" . $bromley->id ); - $mech->content_contains( $bromleypriority->name ); -}; - $mech->log_out_ok; subtest "response priorities can't be viewed across councils" => sub { FixMyStreet::override_config { ALLOWED_COBRANDS => [ 'oxfordshire' ], }, sub { + my $bromley_priority = $bromley->response_priorities->create( { + deleted => 0, + name => "Bromley Cat 0", + } ); + + 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"; + $oxfordshireuser->user_body_permissions->create({ body => $oxfordshire, permission_type => 'responsepriority_edit', }); $mech->log_in_ok( $oxfordshireuser->email ); - $mech->get_ok( "/admin/responsepriorities/" . $oxfordshire->id ); + $mech->get_ok( "/admin/responsepriorities" ); $mech->content_contains( $oxfordshire->response_priorities->first->name ); + $mech->content_lacks( $bromley_priority->name ); - - $mech->get( "/admin/responsepriorities/" . $bromley->id ); - ok !$mech->res->is_success(), "want a bad response"; - is $mech->res->code, 404, "got 404"; - - my $bromley_priority_id = $bromley->response_priorities->first->id; - $mech->get( "/admin/responsepriorities/" . $bromley->id . "/" . $bromley_priority_id ); + $mech->get( "/admin/responsepriorities/" . $bromley->id . "/" . $bromley_priority->id ); ok !$mech->res->is_success(), "want a bad response"; is $mech->res->code, 404, "got 404"; }; diff --git a/templates/web/base/admin/responsepriorities/edit.html b/templates/web/base/admin/responsepriorities/edit.html index 02661fd14..90317b375 100644 --- a/templates/web/base/admin/responsepriorities/edit.html +++ b/templates/web/base/admin/responsepriorities/edit.html @@ -1,57 +1,46 @@ [% INCLUDE 'admin/header.html' title=tprintf(loc('Response Priority for %s'), body.name) -%] -[% rp = response_priority %] -[% UNLESS rp.id %]<h3>[% loc('New priority') %]</h3>[% END %] +[% UNLESS obj %]<h3>[% loc('New priority') %]</h3>[% END %] -<form method="post" - action="[% c.uri_for('', body.id, rp.id || 'new' ) %]" - enctype="application/x-www-form-urlencoded" - accept-charset="utf-8" - class="validate"> +<form method="post" accept-charset="utf-8" class="validate"> - <p> - <strong>[% loc('Name:') %] </strong> - <input type="text" name="name" class="required form-control" size="30" value="[% rp.name | html %]"> - </p> - <p> - <strong>[% loc('Description:') %] </strong> - <input type="text" name="description" class="form-control" size="30" value="[% rp.description | html %]"> - </p> + [% form.field('name').render %] + [% form.field('description').render %] <div class="admin-hint"> <p> [% loc('If this priority is passed to an external service (e.g. Exor/Confirm) enter the priority code to use with that service here.') %] </p> </div> - <p> - <strong>[% loc('External ID') %]:</strong> - <input type="text" name="external_id" class="form-control" size="30" value="[% rp.external_id | html %]"> - </p> + [% form.field('external_id').render %] <div class="admin-hint"> <p> [% loc('Select if this is the default priority') %] </p> </div> - <p> - <label> - <input type="checkbox" name="is_default" is="is_deleted" value="1"[% ' checked' IF rp.is_default %]> - [% loc('Default priority') %] - </label> - </p> - - [% INCLUDE 'admin/category-checkboxes.html' hint=loc('If you only want this priority to be an option for specific categories, pick them here. By default they will show for all categories.') %] + [% form.field('is_default').render %] + + <fieldset> + <legend> + <div class="admin-hint"> + <p> + [% loc('If you only want this priority to be an option for specific categories, pick them here. By default they will show for all categories.') %] + </p> + </div> + [% loc('Categories:') %] + </legend> + [%# TODO Select all/none %] + [% form.field('contacts').render %] + </fieldset> + + [% form.field('deleted').render %] <p> - <label> - <input type="checkbox" name="deleted" id="deleted" value="1"[% ' checked' IF rp.deleted %]> - [% loc('Flag as deleted') %] - </label> - </p> - <p> <input type="hidden" name="token" value="[% csrf_token %]" > - <input type="submit" class="btn" name="Edit priorities" value="[% rp.id ? loc('Save changes') : loc('Create priority') %]" > + <input type="submit" class="btn" name="Edit priorities" value="[% obj ? loc('Save changes') : loc('Create priority') %]" > </p> + </form> [% INCLUDE 'admin/footer.html' %] diff --git a/templates/web/base/admin/responsepriorities/index.html b/templates/web/base/admin/responsepriorities/index.html index 882a3c538..bb6324dbe 100644 --- a/templates/web/base/admin/responsepriorities/index.html +++ b/templates/web/base/admin/responsepriorities/index.html @@ -1,11 +1,55 @@ [% INCLUDE 'admin/header.html' title=loc('Response Priorities') -%] -<ul> - [% FOR body IN bodies %] - <li> - <a href="[% c.uri_for('', body.id) %]">[% body.name %]</a> - </li> - [% END %] -</ul> +<table> + <thead> + <tr> + <th> [% loc('Name') %] </th> + <th> [% loc('Description') %] </th> + <th> [% loc('Categories') %] </th> + <th> [% loc('Default') %] </th> + <th> </th> + </tr> + </thead> + <tbody> + [% FOR p IN response_priorities %] + [% IF c.user.is_superuser AND last_name != p.body.name %] + <tr> + <td colspan="5"><strong>[% p.body.name %]</strong></td> + </tr> + [% SET last_name = p.body.name %] + [% END %] + <tr [% 'class="is-deleted"' IF p.deleted %]> + <td> [% p.name | html %] </td> + <td> [% p.description | html %] </td> + <td> + [% UNLESS p.contacts.size %] + <em>[% loc('All categories') %]</em> + [% ELSE %] + [% FOR contact IN p.contacts %] + [% contact.category_display %][% ',' UNLESS loop.last %] + [% END %] + [% END %] + </td> + <td> [% IF p.is_default %]X[% END %]</td> + <td> <a href="[% c.uri_for(c.controller.action_for('edit'), [p.body_id, p.id]) %]" class="btn">[% loc('Edit') %]</a> </td> + </tr> + [% END %] + </tbody> +</table> + +[% IF c.user.is_superuser %] +<h3>[% loc('Create priority') %]</h3> +<form> +<select name="body_id"> + [% FOR body IN bodies %] + <option value="[% body.id %]">[% body.name | html %]</option> + [% END %] +</select> +<input type="submit" value="[% loc('Go') %]"> +</form> + +[% ELSE %] +<a href="[% c.uri_for(c.controller.action_for('create'), [c.user.from_body.id]) %]" class="btn">[% loc('New priority') %]</a> +[% END %] [% INCLUDE 'admin/footer.html' %] diff --git a/templates/web/base/admin/responsepriorities/list.html b/templates/web/base/admin/responsepriorities/list.html deleted file mode 100644 index eedaccfdb..000000000 --- a/templates/web/base/admin/responsepriorities/list.html +++ /dev/null @@ -1,36 +0,0 @@ -[% INCLUDE 'admin/header.html' title=tprintf(loc('Response Priorities for %s'), body.name) -%] - -<table> - <thead> - <tr> - <th> [% loc('Name') %] </th> - <th> [% loc('Description') %] </th> - <th> [% loc('Categories') %] </th> - <th> [% loc('Default') %] </th> - <th> </th> - </tr> - </thead> - <tbody> - [% FOR p IN response_priorities %] - <tr [% 'class="is-deleted"' IF p.deleted %]> - <td> [% p.name | html %] </td> - <td> [% p.description | html %] </td> - <td> - [% UNLESS p.contacts.size %] - <em>[% loc('All categories') %]</em> - [% ELSE %] - [% FOR contact IN p.contacts %] - [% contact.category_display %][% ',' UNLESS loop.last %] - [% END %] - [% END %] - </td> - <td> [% IF p.is_default %]X[% END %]</td> - <td> <a href="[% c.uri_for('', body.id, p.id) %]" class="btn">[% loc('Edit') %]</a> </td> - </tr> - [% END %] - </tbody> -</table> - -<a href="[% c.uri_for('', body.id, 'new') %]" class="btn">[% loc('New priority') %]</a> - -[% INCLUDE 'admin/footer.html' %] diff --git a/templates/web/base/admin/roles/index.html b/templates/web/base/admin/roles/index.html index ba3fd434b..6c561199c 100644 --- a/templates/web/base/admin/roles/index.html +++ b/templates/web/base/admin/roles/index.html @@ -14,7 +14,7 @@ [% SET last_name = role.body.name %] [% END %] <tr> - <td>[% role.name %]</td> + <td>[% role.name | html %]</td> <td><ul class="no-margin no-bullets"> [% FOR perm IN role.permissions.sort %] <li>[% labels.$perm %] @@ -29,7 +29,7 @@ </table> <p> - <a href="[% c.uri_for(c.controller.action_for('create')) %]">Create</a> + <a href="[% c.uri_for(c.controller.action_for('create')) %]">[% loc('Create') %]</a> </p> [% INCLUDE 'admin/footer.html' %] |