From 5f579853c13959d314b2155703fd03642be03443 Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Tue, 13 Dec 2016 16:41:44 +0000 Subject: Fail in has_permission_to if given an empty arrayref. --- perllib/FixMyStreet/DB/Result/User.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/perllib/FixMyStreet/DB/Result/User.pm b/perllib/FixMyStreet/DB/Result/User.pm index 028394795..f4e5144f8 100644 --- a/perllib/FixMyStreet/DB/Result/User.pm +++ b/perllib/FixMyStreet/DB/Result/User.pm @@ -268,7 +268,7 @@ sub has_permission_to { my ($self, $permission_type, $body_ids) = @_; return 1 if $self->is_superuser; - return 0 unless $body_ids; + return 0 if !$body_ids || (ref $body_ids && !@$body_ids); my $permission = $self->user_body_permissions->find({ permission_type => $permission_type, -- cgit v1.2.3 From 6375eb5d31aa250f5d990d6d6420dd04cf25e3bf Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Tue, 13 Dec 2016 16:30:47 +0000 Subject: Update priorities in inspect form on cat. change. Different categories may have a different list of priorities, so store them all and update as the category changes. --- perllib/FixMyStreet/App/Controller/Report.pm | 31 +++++++++++++++++++--- perllib/FixMyStreet/DB/Result/Problem.pm | 11 +------- .../FixMyStreet/DB/ResultSet/ResponsePriority.pm | 22 +++++++++++++++ t/app/controller/report_inspect.t | 4 +-- templates/web/base/report/_inspect.html | 8 +++--- web/cobrands/fixmystreet/fixmystreet.js | 13 +++++++++ 6 files changed, 69 insertions(+), 20 deletions(-) create mode 100644 perllib/FixMyStreet/DB/ResultSet/ResponsePriority.pm diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm index 13967601b..f01ba00cd 100644 --- a/perllib/FixMyStreet/App/Controller/Report.pm +++ b/perllib/FixMyStreet/App/Controller/Report.pm @@ -312,6 +312,28 @@ sub inspect : Private { $c->stash->{categories} = $c->forward('/admin/categories_for_point'); $c->stash->{report_meta} = { map { $_->{name} => $_ } @{ $c->stash->{problem}->get_extra_fields() } }; + my %category_body = map { $_->category => $_->body_id } map { $_->contacts->all } values %{$problem->bodies}; + + my @priorities = $c->model('DB::ResponsePriority')->for_bodies($problem->bodies_str_ids)->all; + my $priorities_by_category = {}; + foreach my $pri (@priorities) { + my $any = 0; + foreach ($pri->contacts->all) { + $any = 1; + push @{$priorities_by_category->{$_->category}}, $pri->id . '=' . URI::Escape::uri_escape_utf8($pri->name); + } + if (!$any) { + foreach (grep { $category_body{$_} == $pri->body_id } @{$c->stash->{categories}}) { + push @{$priorities_by_category->{$_}}, $pri->id . '=' . URI::Escape::uri_escape_utf8($pri->name); + } + } + } + foreach (keys %{$priorities_by_category}) { + $priorities_by_category->{$_} = join('&', @{$priorities_by_category->{$_}}); + } + + $c->stash->{priorities_by_category} = $priorities_by_category; + if ( $c->get_param('save') ) { $c->forward('/auth/check_csrf_token'); @@ -350,10 +372,6 @@ sub inspect : Private { } } - if ($c->get_param('priority') && ($permissions->{report_inspect} || $permissions->{report_edit_priority})) { - $problem->response_priority( $problem->response_priorities->find({ id => $c->get_param('priority') }) ); - } - if ( !$c->forward( '/admin/report_edit_location', [ $problem ] ) ) { # New lat/lon isn't valid, show an error $valid = 0; @@ -373,6 +391,11 @@ sub inspect : Private { $c->forward('/report/new/set_report_extras', [ \@contacts, $param_prefix ]); } + # Updating priority must come after category, in case category has changed (and so might have priorities) + if ($c->get_param('priority') && ($permissions->{report_inspect} || $permissions->{report_edit_priority})) { + $problem->response_priority( $problem->response_priorities->find({ id => $c->get_param('priority') }) ); + } + if ($valid) { if ( $reputation_change != 0 ) { $problem->user->update_reputation($reputation_change); diff --git a/perllib/FixMyStreet/DB/Result/Problem.pm b/perllib/FixMyStreet/DB/Result/Problem.pm index ab6f20050..91b8d449e 100644 --- a/perllib/FixMyStreet/DB/Result/Problem.pm +++ b/perllib/FixMyStreet/DB/Result/Problem.pm @@ -680,16 +680,7 @@ alphabetical order of name. sub response_priorities { my $self = shift; - return $self->result_source->schema->resultset('ResponsePriority')->search( - { - 'me.body_id' => $self->bodies_str_ids, - 'contact.category' => [ $self->category, undef ], - }, - { - order_by => 'name', - join => { 'contact_response_priorities' => 'contact' }, - } - ); + return $self->result_source->schema->resultset('ResponsePriority')->for_bodies($self->bodies_str_ids, $self->category); } # returns true if the external id is the council's ref, i.e., useful to publish it diff --git a/perllib/FixMyStreet/DB/ResultSet/ResponsePriority.pm b/perllib/FixMyStreet/DB/ResultSet/ResponsePriority.pm new file mode 100644 index 000000000..aa9c426f4 --- /dev/null +++ b/perllib/FixMyStreet/DB/ResultSet/ResponsePriority.pm @@ -0,0 +1,22 @@ +package FixMyStreet::DB::ResultSet::ResponsePriority; +use base 'DBIx::Class::ResultSet'; + +use strict; +use warnings; + +sub for_bodies { + my ($rs, $bodies, $category) = @_; + my $attrs = { + 'me.body_id' => $bodies, + }; + if ($category) { + $attrs->{'contact.category'} = [ $category, undef ]; + } + $rs->search($attrs, { + order_by => 'name', + join => { 'contact_response_priorities' => 'contact' }, + distinct => 1, + }); +} + +1; diff --git a/t/app/controller/report_inspect.t b/t/app/controller/report_inspect.t index efb32d116..094af3dfc 100644 --- a/t/app/controller/report_inspect.t +++ b/t/app/controller/report_inspect.t @@ -102,8 +102,8 @@ FixMyStreet::override_config { $user->user_body_permissions->delete; $user->user_body_permissions->create({ body => $oxon, permission_type => $test->{type} }); $mech->get_ok("/report/$report_id"); - $mech->contains_or_lacks($test->{priority}, 'Priority'); - $mech->contains_or_lacks($test->{priority}, 'High'); + $mech->contains_or_lacks($test->{priority}, 'Priority'); + $mech->contains_or_lacks($test->{priority}, '>High'); $mech->contains_or_lacks($test->{category}, 'Category'); $mech->contains_or_lacks($test->{detailed}, 'Extra details'); $mech->submit_form_ok({ diff --git a/templates/web/base/report/_inspect.html b/templates/web/base/report/_inspect.html index 012411b7e..b54c05d74 100644 --- a/templates/web/base/report/_inspect.html +++ b/templates/web/base/report/_inspect.html @@ -52,11 +52,11 @@ [% cat_prefix = category | lower | replace('[^a-z]', '') %] [% cat_prefix = "category_" _ cat_prefix _ "_" %] [% IF category == problem.category %] -

+

[% INCLUDE 'report/new/category_extras_fields.html' %]

- [% ELSIF category_extras.$category.size %] - [% END %] @@ -88,7 +88,7 @@

diff --git a/web/cobrands/fixmystreet/fixmystreet.js b/web/cobrands/fixmystreet/fixmystreet.js index 1701c5cd0..a6a47d5ba 100644 --- a/web/cobrands/fixmystreet/fixmystreet.js +++ b/web/cobrands/fixmystreet/fixmystreet.js @@ -573,6 +573,19 @@ $.extend(fixmystreet.set_up, { selector = "[data-category='" + category + "']"; $("form#report_inspect_form [data-category]:not(" + selector + ")").addClass("hidden"); $("form#report_inspect_form " + selector).removeClass("hidden"); + // And update the associated priority list + var priorities = $("form#report_inspect_form " + selector).data('priorities'); + var $select = $('#problem_priority'), + curr_pri = $select.val(); + $select.find('option:gt(0)').remove(); + $.each(priorities.split('&'), function(i, kv) { + if (!kv) { + return; + } + kv = kv.split('=', 2); + $select.append($('