diff options
author | Matthew Somerville <matthew@mysociety.org> | 2020-04-17 12:43:57 +0100 |
---|---|---|
committer | Matthew Somerville <matthew@mysociety.org> | 2020-05-07 18:45:30 +0100 |
commit | 7d395d5ca8049a904473e90957115ce675e57db7 (patch) | |
tree | a29f34e3201e73ea98bcb03dd57d8cc028b1a596 | |
parent | dc1507989b865999a96b448aad46a65ccb468244 (diff) |
Refactor report page permissions.
Look up user's permissions once at the start, and
use that throughout the report page templates.
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Report.pm | 28 | ||||
-rw-r--r-- | perllib/FixMyStreet/DB/Result/User.pm | 2 | ||||
-rw-r--r-- | templates/web/base/admin/triage/_inspect.html | 1 | ||||
-rw-r--r-- | templates/web/base/report/_inspect.html | 1 | ||||
-rw-r--r-- | templates/web/base/report/display.html | 3 | ||||
-rw-r--r-- | templates/web/base/report/display_tools.html | 2 | ||||
-rw-r--r-- | templates/web/base/report/inspect/information.html | 2 | ||||
-rw-r--r-- | templates/web/base/report/update/form_user_loggedin.html | 6 |
8 files changed, 18 insertions, 27 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm index 058edebd8..82e8b107f 100644 --- a/perllib/FixMyStreet/App/Controller/Report.pm +++ b/perllib/FixMyStreet/App/Controller/Report.pm @@ -85,14 +85,14 @@ sub display :PathPart('') :Chained('id') :Args(0) { $c->forward( 'load_updates' ); $c->forward( 'format_problem_for_display' ); - my $permissions = $c->stash->{_permissions} ||= $c->forward( 'check_has_permission_to', - [ qw/report_inspect report_edit_category report_edit_priority report_mark_private triage/ ] ); - if (any { $_ } values %$permissions) { + my $permissions = $c->stash->{permissions} ||= $c->forward('fetch_permissions'); + + if (grep { $permissions->{$_} } qw/report_inspect report_edit_category report_edit_priority report_mark_private triage/) { $c->stash->{template} = 'report/inspect.html'; $c->forward('inspect'); } - if ($c->user_exists && $c->user->has_permission_to(contribute_as_another_user => $c->stash->{problem}->bodies_str_ids)) { + if ($permissions->{contribute_as_another_user}) { $c->stash->{email} = $c->user->email; } } @@ -160,8 +160,7 @@ sub load_problem_or_display_error : Private { } elsif ( $problem->non_public ) { # Creator, and inspection users can see non_public reports $c->stash->{problem} = $problem; - my $permissions = $c->stash->{_permissions} = $c->forward( 'check_has_permission_to', - [ qw/report_inspect report_edit_category report_edit_priority report_mark_private / ] ); + my $permissions = $c->stash->{permissions} = $c->forward('fetch_permissions'); # If someone has clicked a unique token link in an email to them my $from_email = $c->sessionid && $c->flash->{alert_to_reporter} && $c->flash->{alert_to_reporter} == $problem->id; @@ -386,7 +385,7 @@ sub delete :Chained('id') :Args(0) { sub inspect : Private { my ( $self, $c ) = @_; my $problem = $c->stash->{problem}; - my $permissions = $c->stash->{_permissions}; + my $permissions = $c->stash->{permissions}; $c->forward('/admin/reports/categories_for_point'); $c->stash->{report_meta} = { map { 'x' . $_->{name} => $_ } @{ $c->stash->{problem}->get_extra_fields() } }; @@ -668,22 +667,19 @@ sub _nearby_json :Private { } -=head2 check_has_permission_to +=head2 fetch_permissions -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. +Returns a hash of the user's permissions, applied to the problem +in $c->stash->{problem}. =cut -sub check_has_permission_to : Private { - my ( $self, $c, @permissions ) = @_; +sub fetch_permissions : Private { + my ( $self, $c ) = @_; return {} unless $c->user_exists; - my $bodies = $c->stash->{problem}->bodies_str_ids; - my %permissions = map { $_ => $c->user->has_permission_to($_, $bodies) } @permissions; - return \%permissions; + return $c->user->permissions($c->stash->{problem}); }; - sub stash_category_groups : Private { my ( $self, $c, $contacts, $combine_multiple ) = @_; diff --git a/perllib/FixMyStreet/DB/Result/User.pm b/perllib/FixMyStreet/DB/Result/User.pm index b0a05d0b7..49338f245 100644 --- a/perllib/FixMyStreet/DB/Result/User.pm +++ b/perllib/FixMyStreet/DB/Result/User.pm @@ -444,7 +444,7 @@ sub permissions { my $body_id = $problem->bodies_str; - return unless $self->belongs_to_body($body_id); + return {} unless $self->belongs_to_body($body_id); my @permissions = grep { $_->{body_id} == $self->from_body->id } @{$self->body_permissions}; return { map { $_->{permission} => 1 } @permissions }; diff --git a/templates/web/base/admin/triage/_inspect.html b/templates/web/base/admin/triage/_inspect.html index 926197ceb..bd76a47f7 100644 --- a/templates/web/base/admin/triage/_inspect.html +++ b/templates/web/base/admin/triage/_inspect.html @@ -24,7 +24,6 @@ </select> [% END %] -[% permissions = c.user.permissions(problem) %] [% second_column = BLOCK -%] <div id="side-inspect"> diff --git a/templates/web/base/report/_inspect.html b/templates/web/base/report/_inspect.html index 771942b16..a8be342d0 100644 --- a/templates/web/base/report/_inspect.html +++ b/templates/web/base/report/_inspect.html @@ -1,4 +1,3 @@ -[% permissions = c.user.permissions(problem) %] [% second_column = BLOCK -%] <div id="side-inspect"> diff --git a/templates/web/base/report/display.html b/templates/web/base/report/display.html index f08df931d..af2282f66 100644 --- a/templates/web/base/report/display.html +++ b/templates/web/base/report/display.html @@ -38,9 +38,6 @@ [% SET shown_form = 1 %] [% END %] -[% IF c.user_exists %] - [% DEFAULT permissions = c.user.permissions(problem) %] -[%- END %] [% INCLUDE 'report/_main.html' %] [% IF problem.duplicate_of %] diff --git a/templates/web/base/report/display_tools.html b/templates/web/base/report/display_tools.html index e16ffcb2c..a09abd73b 100644 --- a/templates/web/base/report/display_tools.html +++ b/templates/web/base/report/display_tools.html @@ -46,7 +46,7 @@ [% loc('Receive email when updates are left on this problem.' ) %]</p> <fieldset> [% IF c.user_exists %] - [% IF c.user.has_permission_to("contribute_as_another_user", problem.bodies_str_ids) %] + [% IF permissions.contribute_as_another_user %] <label for="alert_rznvy">[% loc('Email') %]</label> <div class="form-txt-submit-box"> <input type="email" class="form-control" name="rznvy" id="alert_rznvy" value="[% email | html %]" size="30"> diff --git a/templates/web/base/report/inspect/information.html b/templates/web/base/report/inspect/information.html index b81b37543..3abde9a98 100644 --- a/templates/web/base/report/inspect/information.html +++ b/templates/web/base/report/inspect/information.html @@ -6,7 +6,7 @@ <p> <strong>[% loc('Report ID:') %]</strong> <span class="js-report-id">[% problem.id %]</span> - [% IF c.user_exists AND c.cobrand.admin_allow_user(c.user) AND c.user.has_permission_to('report_edit', problem.bodies_str_ids) %] + [% IF c.user_exists AND c.cobrand.admin_allow_user(c.user) AND permissions.report_edit %] (<a href="[% c.uri_for_action( 'admin/reports/edit', [ problem.id ] ) %]">[% loc('admin') %]</a>) [% END %] </p> diff --git a/templates/web/base/report/update/form_user_loggedin.html b/templates/web/base/report/update/form_user_loggedin.html index bec783bb4..35c9beeff 100644 --- a/templates/web/base/report/update/form_user_loggedin.html +++ b/templates/web/base/report/update/form_user_loggedin.html @@ -4,9 +4,9 @@ [% PROCESS 'user/_anonymity.html' anonymous = update.anonymous %] - [% can_contribute_as_another_user = c.user.has_permission_to("contribute_as_another_user", problem.bodies_str_ids) %] - [% can_contribute_as_anonymous_user = c.user.has_permission_to("contribute_as_anonymous_user", problem.bodies_str_ids) %] - [% can_contribute_as_body = c.user.from_body AND c.user.has_permission_to("contribute_as_body", problem.bodies_str_ids) %] + [% can_contribute_as_another_user = permissions.contribute_as_another_user %] + [% can_contribute_as_anonymous_user = permissions.contribute_as_anonymous_user %] + [% can_contribute_as_body = c.user.from_body AND permissions.contribute_as_body %] [% IF can_contribute_as_another_user OR can_contribute_as_body OR can_contribute_as_anonymous_user %] <label for="form_as">[% loc('Provide update as') %]</label> |