aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--perllib/FixMyStreet/App/Controller/Admin.pm17
-rw-r--r--perllib/FixMyStreet/App/Controller/Around.pm2
-rw-r--r--perllib/FixMyStreet/App/Controller/Council.pm6
-rw-r--r--perllib/FixMyStreet/App/Controller/Report.pm58
-rw-r--r--perllib/FixMyStreet/App/Controller/Report/New.pm2
-rw-r--r--perllib/FixMyStreet/DB/Result/User.pm20
-rw-r--r--t/app/controller/report_inspect.t97
-rw-r--r--templates/web/base/report/_inspect.html17
-rw-r--r--templates/web/base/report/_main.html29
9 files changed, 196 insertions, 52 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm
index c853827d0..1713c4ff9 100644
--- a/perllib/FixMyStreet/App/Controller/Admin.pm
+++ b/perllib/FixMyStreet/App/Controller/Admin.pm
@@ -859,14 +859,11 @@ $problem->bodies_str.
sub report_edit_location : Private {
my ($self, $c, $problem) = @_;
- return 1 unless $c->forward('/report/new/determine_location');
-
- if ( $c->stash->{latitude} != $problem->latitude || $c->stash->{longitude} != $problem->longitude ) {
- delete $c->stash->{prefetched_all_areas};
- delete $c->stash->{all_areas};
- delete $c->stash->{fetch_all_areas};
- delete $c->stash->{all_areas_mapit};
- $c->forward('/council/load_and_check_areas');
+ return 1 unless $c->forward('/location/determine_location_from_coords');
+
+ my ($lat, $lon) = map { Utils::truncate_coordinate($_) } $problem->latitude, $problem->longitude;
+ if ( $c->stash->{latitude} != $lat || $c->stash->{longitude} != $lon ) {
+ $c->forward('/council/load_and_check_areas', []);
$c->forward('/report/new/setup_categories_and_bodies');
my %allowed_bodies = map { $_ => 1 } @{$problem->bodies_str_ids};
my @new_bodies = @{$c->stash->{bodies_to_list}};
@@ -885,8 +882,8 @@ sub categories_for_point : Private {
# We have a report, stash its location
$c->forward('/report/new/determine_location_from_report');
# Look up the areas for this location
- $c->stash->{prefetched_all_areas} = [ grep { $_ } split ',', $c->stash->{report}->areas ];
- $c->forward('/around/check_location_is_acceptable');
+ my $prefetched_all_areas = [ grep { $_ } split ',', $c->stash->{report}->areas ];
+ $c->forward('/around/check_location_is_acceptable', [ $prefetched_all_areas ]);
# As with a new report, fetch the bodies/categories
$c->forward('/report/new/setup_categories_and_bodies');
diff --git a/perllib/FixMyStreet/App/Controller/Around.pm b/perllib/FixMyStreet/App/Controller/Around.pm
index c4feca8b6..ec84ca09a 100644
--- a/perllib/FixMyStreet/App/Controller/Around.pm
+++ b/perllib/FixMyStreet/App/Controller/Around.pm
@@ -52,7 +52,7 @@ sub index : Path : Args(0) {
}
# Check to see if the spot is covered by a area - if not show an error.
- return unless $c->forward('check_location_is_acceptable');
+ return unless $c->forward('check_location_is_acceptable', []);
# If we have a partial - redirect to /report/new so that it can be
# completed.
diff --git a/perllib/FixMyStreet/App/Controller/Council.pm b/perllib/FixMyStreet/App/Controller/Council.pm
index 06a23aec9..0e7553dc4 100644
--- a/perllib/FixMyStreet/App/Controller/Council.pm
+++ b/perllib/FixMyStreet/App/Controller/Council.pm
@@ -36,7 +36,7 @@ there are no areas then return false.
=cut
sub load_and_check_areas : Private {
- my ( $self, $c ) = @_;
+ my ( $self, $c, $prefetched_all_areas ) = @_;
my $latitude = $c->stash->{latitude};
my $longitude = $c->stash->{longitude};
@@ -55,10 +55,10 @@ sub load_and_check_areas : Private {
$params{generation} = $c->config->{MAPIT_GENERATION}
if $c->config->{MAPIT_GENERATION};
- if ($c->stash->{prefetched_all_areas}) {
+ if ($prefetched_all_areas) {
$all_areas = {
map { $_ => { id => $_ } }
- @{$c->stash->{prefetched_all_areas}}
+ @$prefetched_all_areas
};
} elsif ( $c->stash->{fetch_all_areas} ) {
my %area_types = map { $_ => 1 } @$area_types;
diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm
index 6a7a14b5c..582de092c 100644
--- a/perllib/FixMyStreet/App/Controller/Report.pm
+++ b/perllib/FixMyStreet/App/Controller/Report.pm
@@ -3,6 +3,7 @@ package FixMyStreet::App::Controller::Report;
use Moose;
use namespace::autoclean;
use JSON::MaybeXS;
+use List::MoreUtils qw(any);
BEGIN { extends 'Catalyst::Controller'; }
@@ -299,7 +300,8 @@ sub inspect : Private {
$c->forward('/auth/get_csrf_token');
$c->forward( 'load_problem_or_display_error', [ $id ] );
- $c->forward( 'check_has_permission_to', [ 'report_inspect' ] );
+ my $permissions = $c->forward( 'check_has_permission_to',
+ [ qw/report_inspect report_edit_category report_edit_priority/ ] );
$c->forward( 'load_updates' );
$c->forward( 'format_problem_for_display' );
@@ -310,28 +312,32 @@ sub inspect : Private {
if ( $c->get_param('save') || $c->get_param('save_inspected') ) {
$c->forward('/auth/check_csrf_token');
- foreach (qw/detailed_location detailed_information traffic_information/) {
- $problem->set_extra_metadata( $_ => $c->get_param($_) );
- }
+ if ($permissions->{report_inspect}) {
+ foreach (qw/detailed_location detailed_information traffic_information/) {
+ $problem->set_extra_metadata( $_ => $c->get_param($_) );
+ }
- if ( $c->get_param('save_inspected') ) {
- $problem->set_extra_metadata( inspected => 1 );
- }
+ if ( $c->get_param('save_inspected') ) {
+ $problem->set_extra_metadata( inspected => 1 );
+ }
- # Handle the state changing
- my $old_state = $problem->state;
- $problem->state($c->get_param('state'));
- if ( $problem->is_visible() and $old_state eq 'unconfirmed' ) {
- $problem->confirmed( \'current_timestamp' );
- }
- if ( $problem->state eq 'hidden' ) {
- $problem->get_photoset->delete_cached;
- }
- if ( $problem->state ne $old_state ) {
- $c->forward( '/admin/log_edit', [ $id, 'problem', 'state_change' ] );
+ # Handle the state changing
+ my $old_state = $problem->state;
+ $problem->state($c->get_param('state'));
+ if ( $problem->is_visible() and $old_state eq 'unconfirmed' ) {
+ $problem->confirmed( \'current_timestamp' );
+ }
+ if ( $problem->state eq 'hidden' ) {
+ $problem->get_photoset->delete_cached;
+ }
+ if ( $problem->state ne $old_state ) {
+ $c->forward( '/admin/log_edit', [ $id, 'problem', 'state_change' ] );
+ }
}
- $problem->response_priority( $problem->response_priorities->find({ id => $c->get_param('priority') }) );
+ if ($c->get_param('priority') && ($permissions->{report_inspect} || $permissions->{report_edit_priority})) {
+ $problem->response_priority( $problem->response_priorities->find({ id => $c->get_param('priority') }) );
+ }
my $valid = 1;
if ( !$c->forward( '/admin/report_edit_location', [ $problem ] ) ) {
@@ -340,7 +346,9 @@ sub inspect : Private {
$c->stash->{errors} = [ _('Invalid location. New location must be covered by the same council.') ];
}
- $c->forward( '/admin/report_edit_category', [ $problem ] );
+ if ($permissions->{report_inspect} || $permissions->{report_edit_category}) {
+ $c->forward( '/admin/report_edit_category', [ $problem ] );
+ }
if ($valid) {
$problem->update;
@@ -362,18 +370,22 @@ sub map : Private {
=head2 check_has_permission_to
-Ensure the currently logged-in user has a particular permission that applies
+Ensure the currently logged-in user has any of the provided permissions applied
to the current Problem in $c->stash->{problem}. Shows the 403 page if not.
=cut
sub check_has_permission_to : Private {
- my ( $self, $c, $permission ) = @_;
+ my ( $self, $c, @permissions ) = @_;
my $bodies = $c->stash->{problem}->bodies_str;
+ my %permissions = map { $_ => $c->user->has_permission_to($_, $bodies) } @permissions
+ if $c->user_exists;
$c->detach('/page_error_403_access_denied', [ _("Sorry, you don't have permission to do that.") ] )
- unless $c->user_exists && $c->user->has_permission_to($permission, $bodies);
+ unless $c->user_exists && any { $_ } values %permissions;
+
+ return \%permissions;
};
__PACKAGE__->meta->make_immutable;
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm
index cfb572d83..7960f5e4c 100644
--- a/perllib/FixMyStreet/App/Controller/Report/New.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/New.pm
@@ -481,7 +481,7 @@ sub determine_location : Private {
|| $c->forward('/location/determine_location_from_coords')
|| $c->forward('determine_location_from_report')
) #
- && $c->forward('/around/check_location_is_acceptable');
+ && $c->forward('/around/check_location_is_acceptable', []);
return;
}
diff --git a/perllib/FixMyStreet/DB/Result/User.pm b/perllib/FixMyStreet/DB/Result/User.pm
index 6444cfe6a..0ba7e252c 100644
--- a/perllib/FixMyStreet/DB/Result/User.pm
+++ b/perllib/FixMyStreet/DB/Result/User.pm
@@ -240,18 +240,34 @@ sub split_name {
return { first => $first || '', last => $last || '' };
}
+sub permissions {
+ my ($self, $c, $body_id) = @_;
+
+ if ($self->is_superuser) {
+ my $perms = $c->cobrand->available_permissions;
+ return { map { %$_ } values %$perms };
+ }
+
+ return unless $self->belongs_to_body($body_id);
+
+ my @permissions = $self->user_body_permissions->search({
+ body_id => $self->from_body->id,
+ })->all;
+ return { map { $_->permission_type => 1 } @permissions };
+}
+
sub has_permission_to {
my ($self, $permission_type, $body_id) = @_;
return 1 if $self->is_superuser;
- return unless $self->belongs_to_body($body_id);
+ return 0 unless $self->belongs_to_body($body_id);
my $permission = $self->user_body_permissions->find({
permission_type => $permission_type,
body_id => $self->from_body->id,
});
- return $permission ? 1 : undef;
+ return $permission ? 1 : 0;
}
=head2 has_body_permission_to
diff --git a/t/app/controller/report_inspect.t b/t/app/controller/report_inspect.t
new file mode 100644
index 000000000..adba77011
--- /dev/null
+++ b/t/app/controller/report_inspect.t
@@ -0,0 +1,97 @@
+use strict;
+use warnings;
+use Test::More;
+
+use FixMyStreet::TestMech;
+
+my $mech = FixMyStreet::TestMech->new;
+
+my $brum = $mech->create_body_ok(2514, 'Birmingham City Council');
+my $oxon = $mech->create_body_ok(2237, 'Oxfordshire County Council');
+my $contact = $mech->create_contact_ok( body_id => $oxon->id, category => 'Cows', email => 'cows@example.net' );
+my $rp = FixMyStreet::DB->resultset("ResponsePriority")->create({
+ body => $oxon,
+ name => 'High Priority',
+});
+FixMyStreet::DB->resultset("ContactResponsePriority")->create({
+ contact => $contact,
+ response_priority => $rp,
+});
+
+my ($report) = $mech->create_problems_for_body(1, $oxon->id, 'Test', {
+ category => 'Cows', cobrand => 'fixmystreet', areas => ',2237,' });
+my $report_id = $report->id;
+
+my $user = $mech->log_in_ok('test@example.com');
+$user->update( { from_body => $oxon } );
+
+FixMyStreet::override_config {
+ MAPIT_URL => 'http://mapit.mysociety.org/',
+ ALLOWED_COBRANDS => 'fixmystreet',
+}, sub {
+ subtest "test inspect page" => sub {
+ $mech->get_ok("/report/$report_id");
+ $mech->content_lacks('Inspect');
+ $mech->content_lacks('Manage');
+
+ $user->user_body_permissions->create({ body => $oxon, permission_type => 'report_edit_priority' });
+ $mech->get_ok("/report/$report_id");
+ $mech->content_contains('Manage');
+ $mech->follow_link_ok({ text => 'Manage' });
+
+ $user->user_body_permissions->create({ body => $oxon, permission_type => 'report_inspect' });
+ $mech->get_ok("/report/$report_id");
+ $mech->content_contains('Inspect');
+ $mech->follow_link_ok({ text => 'Inspect' });
+ };
+
+ subtest "test basic inspect submission" => sub {
+ $mech->submit_form_ok({ button => 'save', with_fields => { traffic_information => 'Lots', state => 'Planned' } });
+ $report->discard_changes;
+ is $report->state, 'planned', 'report state changed';
+ is $report->get_extra_metadata('traffic_information'), 'Lots', 'report data changed';
+ };
+
+ subtest "test location changes" => sub {
+ $mech->get_ok("/report/$report_id/inspect");
+ $mech->submit_form_ok({ button => 'save', with_fields => { latitude => 55, longitude => -2 } });
+ $mech->content_contains('Invalid location');
+ $mech->submit_form_ok({ button => 'save', with_fields => { latitude => 51.754926, longitude => -1.256179 } });
+ $mech->content_lacks('Invalid location');
+ };
+
+ foreach my $test (
+ { type => 'report_edit_priority', priority => 1 },
+ { type => 'report_edit_category', category => 1 },
+ { type => 'report_inspect', priority => 1, category => 1, detailed => 1 },
+ ) {
+ subtest "test $test->{type} permission" => sub {
+ $user->user_body_permissions->delete;
+ $user->user_body_permissions->create({ body => $oxon, permission_type => $test->{type} });
+ $mech->get_ok("/report/$report_id/inspect");
+ has_or_lacks($test->{priority}, 'Priority');
+ has_or_lacks($test->{priority}, 'High');
+ has_or_lacks($test->{category}, 'Category');
+ has_or_lacks($test->{detailed}, 'Detailed problem information');
+ $mech->submit_form_ok({
+ button => 'save',
+ with_fields => {
+ $test->{priority} ? (priority => 1) : (),
+ $test->{category} ? (category => 'Cows') : (),
+ $test->{detailed} ? (detailed_information => 'Highland ones') : (),
+ }
+ });
+ };
+ }
+};
+
+END {
+ $mech->delete_body($oxon);
+ $mech->delete_body($brum);
+ done_testing();
+}
+
+sub has_or_lacks {
+ my ($flag, $text) = @_;
+ $flag ? $mech->content_contains($text) : $mech->content_lacks($text);
+}
diff --git a/templates/web/base/report/_inspect.html b/templates/web/base/report/_inspect.html
index 891d94c43..ce5110c54 100644
--- a/templates/web/base/report/_inspect.html
+++ b/templates/web/base/report/_inspect.html
@@ -1,4 +1,5 @@
[% PROCESS 'admin/report_blocks.html'; # For the report state dropdown %]
+[% permissions = c.user.permissions(c, problem.bodies_str) %]
[% second_column = BLOCK -%]
<div id="side-report-secondary">
<div class="problem-inspector-fields clearfix">
@@ -7,6 +8,8 @@
<label for="problem_id">[% loc('Report ID:') %]</label>
<input type="text" readonly id="problem_id" value="[% problem.id %]">
</p>
+
+[% IF permissions.report_edit_priority OR permissions.report_inspect %]
<p class="right">
<label for="problem_priority">[% loc('Priority:') %]</label>
<select name="priority" id="problem_priority">
@@ -16,6 +19,9 @@
[% END %]
</select>
</p>
+[% END %]
+
+[% IF permissions.report_inspect %]
<p class="left">
<label for="state">[% loc('State:') %]</label>
[%# XXX this is duplicated from admin/report_edit.html, should be refactored %]
@@ -29,6 +35,9 @@
[% END %]
</select>
</p>
+[% END %]
+
+[% IF permissions.report_edit_category OR permissions.report_inspect %]
<p class="right">
<label for="category">[% loc('Category:') %]</label>
[%# XXX this is duplicated from admin/report_edit.html, should be refactored %]
@@ -47,6 +56,8 @@
[% END %]
</select>
</p>
+[% END %]
+
<p>
[% SET local_coords = problem.local_coords; %]
<strong>[% loc('Easting/Northing:') %]</strong>
@@ -59,6 +70,8 @@
<a href="#" id="geolocate_link">[% loc('Use my current location') %]</a>,
[% loc('or drag the pin on the map') %] &raquo;
</p>
+
+[% IF permissions.report_inspect %]
<p>
<label for="detailed_information">[% loc('Detailed problem location:') %]</label>
<textarea rows="2" name="detailed_location">[% problem.get_extra_metadata('detailed_location') | html %]</textarea>
@@ -71,6 +84,8 @@
<label for="traffic_information">[% loc('Traffic management information:') %]</label>
<textarea rows="2" name="traffic_information">[% problem.get_extra_metadata('traffic_information') | html %]</textarea>
</p>
+[% END %]
+
<p>
<input type="hidden" name="token" value="[% csrf_token %]">
<a href="[% c.uri_for( '/report', problem.id ) %]" class="btn">[% loc('Cancel') %]</a>
@@ -90,4 +105,4 @@
</form>
</div>
</div>
-[%- END %] \ No newline at end of file
+[%- END %]
diff --git a/templates/web/base/report/_main.html b/templates/web/base/report/_main.html
index 1b1a6e29e..e02d4b2b0 100644
--- a/templates/web/base/report/_main.html
+++ b/templates/web/base/report/_main.html
@@ -1,5 +1,6 @@
-[% can_moderate = c.user && c.user.has_permission_to('moderate', problem.bodies_str) %]
-[% can_inspect = c.user && c.user.has_permission_to('report_inspect', problem.bodies_str) && !hide_inspect_button %]
+[% IF c.user_exists %]
+ [% DEFAULT permissions = c.user.permissions(c, problem.bodies_str) %]
+[%- END %]
<a href="[% c.uri_for( '/around', { lat => latitude, lon => longitude } ) %]"
class="problem-back js-back-to-report-list">[% loc('Back to all reports') %]</a>
@@ -23,7 +24,7 @@
</form>
[% END %]
- [% IF can_moderate %]
+ [% IF permissions.moderate %]
[% original = problem_original %]
<form method="post" action="/moderate/report/[% problem.id %]">
<input type="hidden" name="token" value="[% csrf_token %]">
@@ -31,7 +32,7 @@
<h1 class="moderate-display">[% problem.title | html %]</h1>
- [% IF can_moderate %]
+ [% IF permissions.moderate %]
<div class="moderate-edit">
[% IF problem.title != original.title %]
<label>
@@ -60,7 +61,7 @@
[% INCLUDE 'report/_support.html' %]
- [% IF can_moderate %]
+ [% IF permissions.moderate %]
[% IF problem.photo or original.photo %]
<p class="moderate-edit">
<label>
@@ -76,7 +77,7 @@
[% problem.detail | add_links | html_para %]
</div>
- [% IF can_moderate %]
+ [% IF permissions.moderate %]
<p class="moderate-edit">
[% IF problem.detail != original.detail %]
<label>
@@ -105,18 +106,24 @@
</div>
[% END %]
- [% IF can_moderate OR can_inspect %]
+ [% IF permissions.keys.grep('moderate|report_inspect|report_edit_category|report_edit_priority').size %]
<p class="moderate-display">
- [% IF can_moderate %]
+ [% IF permissions.moderate %]
<input type="button" class="btn moderate" value="Moderate this report">
[% END %]
- [% IF can_inspect %]
- <a href="/report/[% problem.id %]/inspect" class="btn inspect">Inspect</a>
+ [% IF !hide_inspect_button AND permissions.keys.grep('report_inspect|report_edit_category|report_edit_priority').size %]
+ <a href="/report/[% problem.id %]/inspect" class="btn inspect">
+ [%- IF permissions.report_inspect %]
+ [%- loc('Inspect') %]
+ [%- ELSE %]
+ [%- loc('Manage') %]
+ [%- END ~%]
+ </a>
[% END %]
</p>
[% END %]
- [% IF can_moderate %]
+ [% IF permissions.moderate %]
</form>
[% END %]
generated by cgit v1.2.3 (git 2.25.1) at 2025-04-26 13:57:50 +0000