aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMatthew Somerville <matthew@mysociety.org>2020-04-17 12:43:57 +0100
committerMatthew Somerville <matthew@mysociety.org>2020-05-07 18:45:30 +0100
commit7d395d5ca8049a904473e90957115ce675e57db7 (patch)
treea29f34e3201e73ea98bcb03dd57d8cc028b1a596
parentdc1507989b865999a96b448aad46a65ccb468244 (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.pm28
-rw-r--r--perllib/FixMyStreet/DB/Result/User.pm2
-rw-r--r--templates/web/base/admin/triage/_inspect.html1
-rw-r--r--templates/web/base/report/_inspect.html1
-rw-r--r--templates/web/base/report/display.html3
-rw-r--r--templates/web/base/report/display_tools.html2
-rw-r--r--templates/web/base/report/inspect/information.html2
-rw-r--r--templates/web/base/report/update/form_user_loggedin.html6
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>