From 683b188b288fe43526e1649c784fa44435559655 Mon Sep 17 00:00:00 2001
From: Matthew Somerville
Date: Thu, 30 Apr 2020 13:51:42 +0100
Subject: Move per-row Contact lookup to the database.
On admin report lists, and in front-end lists when an inspector, each
row was querying the database for `category_display`. We create a new
relationship for this query, and join/prefetch it wherever we request
this data.
Include staff joins on /around page, copying what happens on /reports
to prevent more lookups there too. Also add some joins for user email
in admin report list.
---
perllib/FixMyStreet/App/Controller/Admin.pm | 14 ++++++--
.../FixMyStreet/App/Controller/Admin/Reports.pm | 9 ++++-
perllib/FixMyStreet/App/Controller/Dashboard.pm | 10 +++++-
perllib/FixMyStreet/App/Controller/My.pm | 14 +++++---
perllib/FixMyStreet/App/Controller/Report.pm | 4 ++-
.../FixMyStreet/App/Controller/Report/Update.pm | 16 ++++-----
perllib/FixMyStreet/App/Controller/Reports.pm | 11 +++---
perllib/FixMyStreet/Cobrand/Bexley.pm | 2 +-
perllib/FixMyStreet/Cobrand/Bromley.pm | 12 +++----
perllib/FixMyStreet/Cobrand/EastSussex.pm | 1 -
perllib/FixMyStreet/Cobrand/FixMyStreet.pm | 4 +--
perllib/FixMyStreet/Cobrand/IsleOfWight.pm | 4 +--
perllib/FixMyStreet/Cobrand/Peterborough.pm | 2 +-
perllib/FixMyStreet/DB/Result/Problem.pm | 42 +++++++++++-----------
perllib/FixMyStreet/DB/ResultSet/Nearby.pm | 21 ++++++++---
perllib/FixMyStreet/DB/ResultSet/Problem.pm | 30 +++++++++++-----
t/roles/translatable.t | 5 +--
.../web/isleofwight/report/_item_heading.html | 2 +-
18 files changed, 131 insertions(+), 72 deletions(-)
diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm
index 038cba9e5..8d6a41b9f 100644
--- a/perllib/FixMyStreet/App/Controller/Admin.pm
+++ b/perllib/FixMyStreet/App/Controller/Admin.pm
@@ -71,13 +71,16 @@ sub index : Path : Args(0) {
}
my @unsent = $c->cobrand->problems->search( {
- state => [ FixMyStreet::DB::Result::Problem::open_states() ],
+ 'me.state' => [ FixMyStreet::DB::Result::Problem::open_states() ],
whensent => undef,
bodies_str => { '!=', undef },
# Ignore very recent ones that probably just haven't been sent yet
confirmed => { '<', \"current_timestamp - '5 minutes'::interval" },
},
{
+ '+columns' => ['user.email'],
+ prefetch => 'contact',
+ join => 'user',
order_by => 'confirmed',
} )->all;
$c->stash->{unsent_reports} = \@unsent;
@@ -301,7 +304,14 @@ sub add_flags : Private {
sub flagged : Path('flagged') : Args(0) {
my ( $self, $c ) = @_;
- my $problems = $c->cobrand->problems->search( { flagged => 1 } );
+ my $problems = $c->cobrand->problems->search(
+ { 'me.flagged' => 1 },
+ {
+ '+columns' => ['user.email'],
+ join => 'user',
+ prefetch => 'contact',
+ }
+ );
# pass in as array ref as using same template as search_reports
# which has to use an array ref for sql quoting reasons
diff --git a/perllib/FixMyStreet/App/Controller/Admin/Reports.pm b/perllib/FixMyStreet/App/Controller/Admin/Reports.pm
index 7300fe676..88c4380fc 100644
--- a/perllib/FixMyStreet/App/Controller/Admin/Reports.pm
+++ b/perllib/FixMyStreet/App/Controller/Admin/Reports.pm
@@ -110,6 +110,7 @@ sub index : Path {
{
join => 'user',
'+columns' => 'user.email',
+ prefetch => 'contact',
rows => 50,
order_by => $order,
}
@@ -166,7 +167,13 @@ sub index : Path {
my $problems = $c->cobrand->problems->search(
$query,
- { order_by => $order, rows => 50 }
+ {
+ '+columns' => ['user.email'],
+ join => 'user',
+ prefetch => 'contact',
+ order_by => $order,
+ rows => 50
+ }
)->page( $p_page );
$c->stash->{problems} = [ $problems->all ];
$c->stash->{problems_pager} = $problems->pager;
diff --git a/perllib/FixMyStreet/App/Controller/Dashboard.pm b/perllib/FixMyStreet/App/Controller/Dashboard.pm
index ad6c9ba98..f3b607ba8 100644
--- a/perllib/FixMyStreet/App/Controller/Dashboard.pm
+++ b/perllib/FixMyStreet/App/Controller/Dashboard.pm
@@ -591,7 +591,15 @@ sub heatmap_sidebar :Private {
order_by => 'lastupdate',
})->all ];
- my $params = { map { my $n = $_; s/me\./problem\./; $_ => $where->{$n} } keys %$where };
+ my $params = { map {
+ my $v = $where->{$_};
+ if (ref $v eq 'HASH') {
+ $v = { map { my $vv = $v->{$_}; s/me\./problem\./; $_ => $vv } keys %$v };
+ } else {
+ s/me\./problem\./;
+ }
+ $_ => $v;
+ } keys %$where };
my $body = $c->stash->{body};
my @user;
diff --git a/perllib/FixMyStreet/App/Controller/My.pm b/perllib/FixMyStreet/App/Controller/My.pm
index 3328caac0..316fe08b8 100644
--- a/perllib/FixMyStreet/App/Controller/My.pm
+++ b/perllib/FixMyStreet/App/Controller/My.pm
@@ -135,13 +135,14 @@ sub get_problems : Private {
my $problems = [];
my $states = $c->stash->{filter_problem_states};
+ my $table = $c->action eq 'my/planned' ? 'report' : 'me';
my $params = {
- state => [ keys %$states ],
+ "$table.state" => [ keys %$states ],
};
my $categories = [ $c->get_param_list('filter_category', 1) ];
if ( @$categories ) {
- $params->{category} = $categories;
+ $params->{"$table.category"} = $categories;
$c->stash->{filter_category} = { map { $_ => 1 } @$categories };
}
@@ -149,6 +150,7 @@ sub get_problems : Private {
$rows = 5000 if $c->stash->{sort_key} eq 'shortlist'; # Want all reports
my $rs = $c->stash->{problems_rs}->search( $params, {
+ prefetch => 'contact',
order_by => $c->stash->{sort_order},
rows => $rows,
} )->include_comment_counts->page( $p_page );
@@ -186,12 +188,14 @@ sub get_updates : Private {
sub setup_page_data : Private {
my ($self, $c) = @_;
+ my $table = $c->action eq 'my/planned' ? 'report' : 'me';
my @categories = $c->stash->{problems_rs}->search({
- state => [ FixMyStreet::DB::Result::Problem->visible_states() ],
+ "$table.state" => [ FixMyStreet::DB::Result::Problem->visible_states() ],
}, {
- columns => [ 'category', 'bodies_str', 'extra' ],
+ join => 'contact',
+ columns => [ "$table.category", 'contact.extra', 'contact.category' ],
distinct => 1,
- order_by => [ 'category' ],
+ order_by => [ "$table.category" ],
} )->all;
$c->stash->{filter_categories} = \@categories;
$c->forward('/report/stash_category_groups', [ \@categories ]) if $c->cobrand->enable_category_groups;
diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm
index 72f96013a..058edebd8 100644
--- a/perllib/FixMyStreet/App/Controller/Report.pm
+++ b/perllib/FixMyStreet/App/Controller/Report.pm
@@ -133,11 +133,13 @@ sub support :Chained('id') :Args(0) {
sub load_problem_or_display_error : Private {
my ( $self, $c, $id ) = @_;
+ my $attrs = { prefetch => 'contact' };
+
# try to load a report if the id is a number
my $problem
= ( !$id || $id =~ m{\D} ) # is id non-numeric?
? undef # ...don't even search
- : $c->cobrand->problems->find( { id => $id } )
+ : $c->cobrand->problems->find( { id => $id }, $attrs )
or $c->detach( '/page_error_404_not_found', [ _('Unknown problem ID') ] );
# check that the problem is suitable to show.
diff --git a/perllib/FixMyStreet/App/Controller/Report/Update.pm b/perllib/FixMyStreet/App/Controller/Report/Update.pm
index 41c42b8a1..c5d20a5da 100644
--- a/perllib/FixMyStreet/App/Controller/Report/Update.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/Update.pm
@@ -121,7 +121,7 @@ sub process_user : Private {
if ( $c->user_exists ) { {
my $user = $c->user->obj;
- if ($c->stash->{contributing_as_another_user} = $user->contributing_as('another_user', $c, $update->problem->bodies_str_ids)) {
+ if ($c->stash->{contributing_as_another_user} = $user->contributing_as('another_user', $c, $c->stash->{problem}->bodies_str_ids)) {
# Act as if not logged in (and it will be auto-confirmed later on)
last;
}
@@ -248,7 +248,7 @@ sub load_problem : Private {
# Problem ID could come from existing update in token, or from query parameter
my $problem_id = $update->problem_id || $c->get_param('id');
$c->forward( '/report/load_problem_or_display_error', [ $problem_id ] );
- $update->problem($c->stash->{problem});
+ $update->problem_id($c->stash->{problem}->id);
}
=head2 check_form_submitted
@@ -282,7 +282,8 @@ sub process_update : Private {
my $name = Utils::trim_text( $params{name} );
- $params{reopen} = 0 unless $c->user && $c->user->id == $c->stash->{problem}->user->id;
+ my $problem = $c->stash->{problem};
+ $params{reopen} = 0 unless $c->user && $c->user->id == $problem->user->id;
my $update = $c->stash->{update};
$update->text($params{update});
@@ -290,11 +291,11 @@ sub process_update : Private {
$update->mark_fixed($params{fixed} ? 1 : 0);
$update->mark_open($params{reopen} ? 1 : 0);
- $c->stash->{contributing_as_body} = $c->user_exists && $c->user->contributing_as('body', $c, $update->problem->bodies_str_ids);
- $c->stash->{contributing_as_anonymous_user} = $c->user_exists && $c->user->contributing_as('anonymous_user', $c, $update->problem->bodies_str_ids);
+ $c->stash->{contributing_as_body} = $c->user_exists && $c->user->contributing_as('body', $c, $problem->bodies_str_ids);
+ $c->stash->{contributing_as_anonymous_user} = $c->user_exists && $c->user->contributing_as('anonymous_user', $c, $problem->bodies_str_ids);
# This is also done in process_user, but is needed here for anonymous() just below
- my $anon_button = $c->cobrand->allow_anonymous_reports($update->problem->category) eq 'button' && $c->get_param('report_anonymously');
+ my $anon_button = $c->cobrand->allow_anonymous_reports($problem->category) eq 'button' && $c->get_param('report_anonymously');
if ($anon_button) {
$c->stash->{contributing_as_anonymous_user} = 1;
$c->stash->{contributing_as_body} = undef;
@@ -323,7 +324,6 @@ sub process_update : Private {
# then we are not changing the state of the problem so can use the current
# problem state
} else {
- my $problem = $c->stash->{problem} || $update->problem;
$update->problem_state( $problem->state );
}
}
@@ -332,7 +332,7 @@ sub process_update : Private {
my @extra; # Next function fills this, but we don't need it here.
# This is just so that the error checking for these extra fields runs.
# TODO Use extra here as it is used on reports.
- my $body = (values %{$update->problem->bodies})[0];
+ my $body = (values %{$problem->bodies})[0];
$c->cobrand->process_open311_extras( $c, $body, \@extra );
if ( $c->get_param('fms_extra_title') ) {
diff --git a/perllib/FixMyStreet/App/Controller/Reports.pm b/perllib/FixMyStreet/App/Controller/Reports.pm
index 97976ebe3..53f27eb62 100644
--- a/perllib/FixMyStreet/App/Controller/Reports.pm
+++ b/perllib/FixMyStreet/App/Controller/Reports.pm
@@ -620,6 +620,9 @@ sub load_problems_parameters : Private {
};
if ($c->user_exists && $body) {
my $prefetch = [];
+ if ($c->user->from_body || $c->user->is_superuser) {
+ push @$prefetch, 'contact';
+ }
if ($c->user->has_permission_to('planned_reports', $body->id)) {
push @$prefetch, 'user_planned_reports';
}
@@ -646,7 +649,7 @@ sub load_problems_parameters : Private {
}
if (@$category) {
- $where->{category} = $category;
+ $where->{'me.category'} = $category;
}
if ($c->stash->{wards}) {
@@ -687,12 +690,12 @@ sub check_non_public_reports_permission : Private {
}
if ( $user_has_permission ) {
- $where->{non_public} = 1 if $c->stash->{only_non_public};
+ $where->{'me.non_public'} = 1 if $c->stash->{only_non_public};
} else {
- $where->{non_public} = 0;
+ $where->{'me.non_public'} = 0;
}
} else {
- $where->{non_public} = 0;
+ $where->{'me.non_public'} = 0;
}
}
diff --git a/perllib/FixMyStreet/Cobrand/Bexley.pm b/perllib/FixMyStreet/Cobrand/Bexley.pm
index 481926e72..f37394794 100644
--- a/perllib/FixMyStreet/Cobrand/Bexley.pm
+++ b/perllib/FixMyStreet/Cobrand/Bexley.pm
@@ -54,7 +54,7 @@ sub open311_munge_update_params {
$params->{service_request_id_ext} = $comment->problem->id;
- my $contact = $comment->problem->category_row;
+ my $contact = $comment->problem->contact;
$params->{service_code} = $contact->email;
}
diff --git a/perllib/FixMyStreet/Cobrand/Bromley.pm b/perllib/FixMyStreet/Cobrand/Bromley.pm
index 8f82817a8..bd229f360 100644
--- a/perllib/FixMyStreet/Cobrand/Bromley.pm
+++ b/perllib/FixMyStreet/Cobrand/Bromley.pm
@@ -344,23 +344,23 @@ sub munge_load_and_group_problems {
return unless $c->action eq 'dashboard/heatmap';
# Bromley subcategory stuff
- if (!$where->{category}) {
+ if (!$where->{'me.category'}) {
my $cats = $c->user->categories;
my $subcats = $c->user->get_extra_metadata('subcategories') || [];
- $where->{category} = [ @$cats, @$subcats ] if @$cats || @$subcats;
+ $where->{'me.category'} = [ @$cats, @$subcats ] if @$cats || @$subcats;
}
my %subcats = $self->subcategories;
my $subcat;
- my %chosen = map { $_ => 1 } @{$where->{category} || []};
+ my %chosen = map { $_ => 1 } @{$where->{'me.category'} || []};
my @subcat = grep { $chosen{$_} } map { $_->{key} } map { @$_ } values %subcats;
if (@subcat) {
my %chosen = map { $_ => 1 } @subcat;
$where->{'-or'} = {
- category => [ grep { !$chosen{$_} } @{$where->{category}} ],
- subcategory => \@subcat,
+ 'me.category' => [ grep { !$chosen{$_} } @{$where->{'me.category'}} ],
+ 'me.subcategory' => \@subcat,
};
- delete $where->{category};
+ delete $where->{'me.category'};
}
}
diff --git a/perllib/FixMyStreet/Cobrand/EastSussex.pm b/perllib/FixMyStreet/Cobrand/EastSussex.pm
index e6c2da6c5..8d95980fe 100644
--- a/perllib/FixMyStreet/Cobrand/EastSussex.pm
+++ b/perllib/FixMyStreet/Cobrand/EastSussex.pm
@@ -11,7 +11,6 @@ sub open311_extra_data {
$h->{es_original_detail} = $row->detail;
- $contact = $row->category_row;
my $fields = $contact->get_extra_fields;
my $text = '';
for my $field ( @$fields ) {
diff --git a/perllib/FixMyStreet/Cobrand/FixMyStreet.pm b/perllib/FixMyStreet/Cobrand/FixMyStreet.pm
index dfb511f39..f39558f45 100644
--- a/perllib/FixMyStreet/Cobrand/FixMyStreet.pm
+++ b/perllib/FixMyStreet/Cobrand/FixMyStreet.pm
@@ -139,10 +139,10 @@ sub munge_report_new_contacts {
sub munge_load_and_group_problems {
my ($self, $where, $filter) = @_;
- return unless $where->{category} && $self->{c}->stash->{body}->name eq 'Isle of Wight Council';
+ return unless $where->{'me.category'} && $self->{c}->stash->{body}->name eq 'Isle of Wight Council';
my $iow = FixMyStreet::Cobrand->get_class_for_moniker( 'isleofwight' )->new({ c => $self->{c} });
- $where->{category} = $iow->expand_triage_cat_list($where->{category}, $self->{c}->stash->{body});
+ $where->{'me.category'} = $iow->expand_triage_cat_list($where->{'me.category'}, $self->{c}->stash->{body});
}
sub title_list {
diff --git a/perllib/FixMyStreet/Cobrand/IsleOfWight.pm b/perllib/FixMyStreet/Cobrand/IsleOfWight.pm
index db0a20b9c..3bcea4d3d 100644
--- a/perllib/FixMyStreet/Cobrand/IsleOfWight.pm
+++ b/perllib/FixMyStreet/Cobrand/IsleOfWight.pm
@@ -140,9 +140,9 @@ sub munge_around_category_where {
sub munge_load_and_group_problems {
my ($self, $where, $filter) = @_;
- return unless $where->{category};
+ return unless $where->{'me.category'};
- $where->{category} = $self->_expand_triage_cat_list($where->{category});
+ $where->{'me.category'} = $self->_expand_triage_cat_list($where->{'me.category'});
}
sub munge_around_filter_category_list {
diff --git a/perllib/FixMyStreet/Cobrand/Peterborough.pm b/perllib/FixMyStreet/Cobrand/Peterborough.pm
index 0ddaeacb6..133767fa7 100644
--- a/perllib/FixMyStreet/Cobrand/Peterborough.pm
+++ b/perllib/FixMyStreet/Cobrand/Peterborough.pm
@@ -85,7 +85,7 @@ sub open311_munge_update_params {
# Send the FMS problem ID with the update.
$params->{service_request_id_ext} = $comment->problem->id;
- my $contact = $comment->problem->category_row;
+ my $contact = $comment->problem->contact;
$params->{service_code} = $contact->email;
}
diff --git a/perllib/FixMyStreet/DB/Result/Problem.pm b/perllib/FixMyStreet/DB/Result/Problem.pm
index 37563d327..42983c9ad 100644
--- a/perllib/FixMyStreet/DB/Result/Problem.pm
+++ b/perllib/FixMyStreet/DB/Result/Problem.pm
@@ -193,6 +193,27 @@ __PACKAGE__->might_have(
cascade_copy => 0, cascade_delete => 1 },
);
+# Add a possible join for the Contact object associated with
+# this report (based on bodies_str and category). If the report
+# was sent to multiple bodies, only returns the first.
+__PACKAGE__->belongs_to(
+ contact => "FixMyStreet::DB::Result::Contact",
+ sub {
+ my $args = shift;
+ return {
+ "$args->{foreign_alias}.category" => { -ident => "$args->{self_alias}.category" },
+ -and => [
+ \[ "CAST($args->{foreign_alias}.body_id AS text) = (regexp_split_to_array($args->{self_alias}.bodies_str, ','))[1]" ],
+ ]
+ };
+ },
+ {
+ join_type => "LEFT",
+ on_delete => "NO ACTION",
+ on_update => "NO ACTION",
+ },
+);
+
__PACKAGE__->load_components("+FixMyStreet::DB::RABXColumn");
__PACKAGE__->rabx_column('extra');
__PACKAGE__->rabx_column('geocode');
@@ -407,30 +428,11 @@ sub confirm {
sub category_display {
my $self = shift;
- my $contact = $self->category_row;
+ my $contact = $self->contact;
return $self->category unless $contact; # Fallback; shouldn't happen, but some tests
return $contact->category_display;
}
-=head2 category_row
-
-Returns the corresponding Contact object for this problem's category and body.
-If the report was sent to multiple bodies, only returns the first.
-
-=cut
-
-sub category_row {
- my $self = shift;
- my $schema = $self->result_source->schema;
- my $body_id = $self->bodies_str_ids->[0];
- return unless $body_id && $body_id =~ /^[0-9]+$/;
- my $contact = $schema->resultset("Contact")->find({
- body_id => $body_id,
- category => $self->category,
- });
- return $contact;
-}
-
sub bodies_str_ids {
my $self = shift;
return [] unless $self->bodies_str;
diff --git a/perllib/FixMyStreet/DB/ResultSet/Nearby.pm b/perllib/FixMyStreet/DB/ResultSet/Nearby.pm
index 2ebe309e3..af1142c3a 100644
--- a/perllib/FixMyStreet/DB/ResultSet/Nearby.pm
+++ b/perllib/FixMyStreet/DB/ResultSet/Nearby.pm
@@ -17,16 +17,16 @@ sub nearby {
}
my $params = {
- state => [ keys %{$args{states}} ],
+ 'problem.state' => [ keys %{$args{states}} ],
};
- $params->{id} = { -not_in => $args{ids} }
+ $params->{problem_id} = { -not_in => $args{ids} }
if $args{ids};
- $params->{category} = $args{categories} if $args{categories} && @{$args{categories}};
+ $params->{'problem.category'} = $args{categories} if $args{categories} && @{$args{categories}};
$params->{$c->stash->{report_age_field}} = { '>=', \"current_timestamp-'$args{report_age}'::interval" }
if $args{report_age};
- FixMyStreet::DB::ResultSet::Problem->non_public_if_possible($params, $c);
+ FixMyStreet::DB::ResultSet::Problem->non_public_if_possible($params, $c, 'problem');
$rs = $c->cobrand->problems_restriction($rs);
@@ -34,11 +34,22 @@ sub nearby {
$params = { %$params, %{$args{extra}} } if $args{extra};
my $attrs = {
- prefetch => 'problem',
+ prefetch => { problem => [] },
bind => [ $args{latitude}, $args{longitude}, $args{distance} ],
order_by => [ 'distance', { -desc => 'created' } ],
rows => $args{limit},
};
+ if ($c->user_exists) {
+ if ($c->user->from_body || $c->user->is_superuser) {
+ push @{$attrs->{prefetch}{problem}}, 'contact';
+ }
+ if ($c->user->has_body_permission_to('planned_reports')) {
+ push @{$attrs->{prefetch}{problem}}, 'user_planned_reports';
+ }
+ if ($c->user->has_body_permission_to('report_edit_priority') || $c->user->has_body_permission_to('report_inspect')) {
+ push @{$attrs->{prefetch}{problem}}, 'response_priority';
+ }
+ }
my @problems = mySociety::Locale::in_gb_locale { $rs->search( $params, $attrs )->all };
return \@problems;
diff --git a/perllib/FixMyStreet/DB/ResultSet/Problem.pm b/perllib/FixMyStreet/DB/ResultSet/Problem.pm
index e23cf78e1..2ce4c04be 100644
--- a/perllib/FixMyStreet/DB/ResultSet/Problem.pm
+++ b/perllib/FixMyStreet/DB/ResultSet/Problem.pm
@@ -26,30 +26,31 @@ sub body_query {
# Edits PARAMS in place to either hide non_public reports, or show them
# if user is superuser (all) or inspector (correct body)
sub non_public_if_possible {
- my ($rs, $params, $c) = @_;
+ my ($rs, $params, $c, $table) = @_;
+ $table ||= 'me';
if ($c->user_exists) {
my $only_non_public = $c->stash->{only_non_public} ? 1 : 0;
if ($c->user->is_superuser) {
# See all reports, no restriction
- $params->{non_public} = 1 if $only_non_public;
+ $params->{"$table.non_public"} = 1 if $only_non_public;
} elsif ($c->user->has_body_permission_to('report_inspect') ||
$c->user->has_body_permission_to('report_mark_private')) {
if ($only_non_public) {
$params->{'-and'} = [
- non_public => 1,
+ "$table.non_public" => 1,
$rs->body_query($c->user->from_body->id),
];
} else {
$params->{'-or'} = [
- non_public => 0,
+ "$table.non_public" => 0,
$rs->body_query($c->user->from_body->id),
];
}
} else {
- $params->{non_public} = 0;
+ $params->{"$table.non_public"} = 0;
}
} else {
- $params->{non_public} = 0;
+ $params->{"$table.non_public"} = 0;
}
}
@@ -175,22 +176,33 @@ sub around_map {
my ( $rs, $c, %p) = @_;
my $attr = {
order_by => $p{order},
+ rows => $c->cobrand->reports_per_page,
};
- $attr->{rows} = $c->cobrand->reports_per_page;
+ if ($c->user_exists) {
+ if ($c->user->from_body || $c->user->is_superuser) {
+ push @{$attr->{prefetch}}, 'contact';
+ }
+ if ($c->user->has_body_permission_to('planned_reports')) {
+ push @{$attr->{prefetch}}, 'user_planned_reports';
+ }
+ if ($c->user->has_body_permission_to('report_edit_priority') || $c->user->has_body_permission_to('report_inspect')) {
+ push @{$attr->{prefetch}}, 'response_priority';
+ }
+ }
unless ( $p{states} ) {
$p{states} = FixMyStreet::DB::Result::Problem->visible_states();
}
my $q = {
- state => [ keys %{$p{states}} ],
+ 'me.state' => [ keys %{$p{states}} ],
latitude => { '>=', $p{min_lat}, '<', $p{max_lat} },
longitude => { '>=', $p{min_lon}, '<', $p{max_lon} },
};
$q->{$c->stash->{report_age_field}} = { '>=', \"current_timestamp-'$p{report_age}'::interval" } if
$p{report_age};
- $q->{category} = $p{categories} if $p{categories} && @{$p{categories}};
+ $q->{'me.category'} = $p{categories} if $p{categories} && @{$p{categories}};
$rs->non_public_if_possible($q, $c);
diff --git a/t/roles/translatable.t b/t/roles/translatable.t
index 9f8c67394..beddb7182 100644
--- a/t/roles/translatable.t
+++ b/t/roles/translatable.t
@@ -77,8 +77,9 @@ FixMyStreet::override_config {
subtest 'Check display_name override' => sub {
$contact->set_extra_metadata( display_name => 'Override name' );
$contact->update;
- is $contact->category_display, "Override name";
- is $problem->category_display, "Override name";
+ is $contact->category_display, "Override name", 'Contact uses display_name';
+ $problem->discard_changes;
+ is $problem->category_display, "Override name", 'Problem uses display_name';
};
done_testing;
diff --git a/templates/web/isleofwight/report/_item_heading.html b/templates/web/isleofwight/report/_item_heading.html
index 90010a548..40f9c8ad2 100644
--- a/templates/web/isleofwight/report/_item_heading.html
+++ b/templates/web/isleofwight/report/_item_heading.html
@@ -1,7 +1,7 @@
[%
SET title = problem.title;
IF c.req.uri.path == '/';
- title = problem.category_display;
+ title = problem.category; # Not category_display to prevent unneeded DM lookup
END;
%]
[% title %]
--
cgit v1.2.3
From 730d25ae7218d731590b322d4f419a7df6d4e4fb Mon Sep 17 00:00:00 2001
From: Matthew Somerville
Date: Thu, 30 Apr 2020 13:56:31 +0100
Subject: Add ability to disallow updates in a category.
Add a tickbox to the category admin, and do not allow updates on
reports made in those selected categories.
---
CHANGELOG.md | 1 +
docs/_includes/admin-tasks-content.md | 6 ++--
perllib/FixMyStreet/App/Controller/Admin/Bodies.pm | 15 ++++------
perllib/FixMyStreet/Cobrand/Default.pm | 1 +
t/app/controller/admin/bodies.t | 11 ++++++++
t/app/controller/report_updates.t | 33 ++++++++++++++--------
templates/web/base/admin/bodies/contact-form.html | 5 ++++
7 files changed, 48 insertions(+), 24 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ad53ca547..e5fbb6ea8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,7 @@
- Move stats from main admin index to stats index.
- Speed up dashboard export and report search.
- Allow a template to be an initial update on reports. #2973
+ - Interface for disabling updates for certain categories. #2991
- Bugfixes
- Application user in Docker container can't install packages. #2914
- Look at all categories when sending reports.
diff --git a/docs/_includes/admin-tasks-content.md b/docs/_includes/admin-tasks-content.md
index 7e3d47efe..a846b73cc 100644
--- a/docs/_includes/admin-tasks-content.md
+++ b/docs/_includes/admin-tasks-content.md
@@ -560,14 +560,14 @@ and staff users — can filter reports when viewing them on the site.
From the Admin menu, click on ‘Categories’. You’ll see a table of existing categories, and below
that, a form by which you can create new ones.
-
Input a title for the category, and the email address to which reports in that category should be
forwarded. When creating a category, these are the only fields required.
You can also choose a variety of options – whether to automatically hide any
reports made in this category, whether to prevent form submission when this
-category is selected, or what parent category or categories a particular
-category is in. See below for information on creating/editing extra notices and
questions for a category.
diff --git a/perllib/FixMyStreet/App/Controller/Admin/Bodies.pm b/perllib/FixMyStreet/App/Controller/Admin/Bodies.pm
index 6ae068cd9..61b486f02 100644
--- a/perllib/FixMyStreet/App/Controller/Admin/Bodies.pm
+++ b/perllib/FixMyStreet/App/Controller/Admin/Bodies.pm
@@ -267,15 +267,12 @@ sub update_contact : Private {
$contact->send_method( $c->get_param('send_method') );
# Set flags in extra to the appropriate values
- if ( $c->get_param('photo_required') ) {
- $contact->set_extra_metadata_if_undefined( photo_required => 1 );
- } else {
- $contact->unset_extra_metadata( 'photo_required' );
- }
- if ( $c->get_param('open311_protect') ) {
- $contact->set_extra_metadata( open311_protect => 1 );
- } else {
- $contact->unset_extra_metadata( 'open311_protect' );
+ foreach (qw(photo_required open311_protect updates_disallowed)) {
+ if ( $c->get_param($_) ) {
+ $contact->set_extra_metadata( $_ => 1 );
+ } else {
+ $contact->unset_extra_metadata($_);
+ }
}
if ( my @group = $c->get_param_list('group') ) {
@group = grep { $_ } @group;
diff --git a/perllib/FixMyStreet/Cobrand/Default.pm b/perllib/FixMyStreet/Cobrand/Default.pm
index 695487268..07e781479 100644
--- a/perllib/FixMyStreet/Cobrand/Default.pm
+++ b/perllib/FixMyStreet/Cobrand/Default.pm
@@ -530,6 +530,7 @@ or not. Default behaviour is disallowed if "closed_updates" metadata is set.
sub updates_disallowed {
my ($self, $problem) = @_;
return 1 if $problem->get_extra_metadata('closed_updates');
+ return 1 if $problem->contact && $problem->contact->get_extra_metadata('updates_disallowed');
return 0;
}
diff --git a/t/app/controller/admin/bodies.t b/t/app/controller/admin/bodies.t
index c73a90da1..7ec7aed75 100644
--- a/t/app/controller/admin/bodies.t
+++ b/t/app/controller/admin/bodies.t
@@ -261,6 +261,17 @@ subtest 'open311 protection editing' => sub {
is $contact->get_extra_metadata('open311_protect'), 1, 'Open311 protect flag set';
};
+subtest 'updates disabling' => sub {
+ $mech->get_ok('/admin/body/' . $body->id . '/test%20category');
+ $mech->submit_form_ok( { with_fields => {
+ updates_disallowed => 1,
+ note => 'Disabling updates',
+ } } );
+ $mech->content_contains('Values updated');
+ my $contact = $body->contacts->find({ category => 'test category' });
+ is $contact->get_extra_metadata('updates_disallowed'), 1, 'Updates disallowed flag set';
+};
+
}; # END of override wrap
diff --git a/t/app/controller/report_updates.t b/t/app/controller/report_updates.t
index 07ee48587..e8ab1cc85 100644
--- a/t/app/controller/report_updates.t
+++ b/t/app/controller/report_updates.t
@@ -22,6 +22,8 @@ my $user2 = $mech->create_user_ok('commenter@example.com', name => 'Commenter');
my $body = $mech->create_body_ok(2504, 'Westminster City Council');
+my $contact = $mech->create_contact_ok( body_id => $body->id, category => 'Other', email => 'other' );
+
my $dt = DateTime->new(
year => 2011,
month => 04,
@@ -1893,6 +1895,18 @@ for my $test (
};
}
+$mech->log_in_ok( $report->user->email );
+
+my %standard_fields = (
+ name => $report->user->name,
+ update => 'update text',
+ photo1 => '',
+ photo2 => '',
+ photo3 => '',
+ may_show_name => 1,
+ add_alert => 1,
+);
+
for my $test (
{
desc => 'update confirmed without marking as fixed leaves state unchanged',
@@ -2094,18 +2108,6 @@ for my $test (
},
) {
subtest $test->{desc} => sub {
- $mech->log_in_ok( $report->user->email );
-
- my %standard_fields = (
- name => $report->user->name,
- update => 'update text',
- photo1 => '',
- photo2 => '',
- photo3 => '',
- may_show_name => 1,
- add_alert => 1,
- );
-
my %expected_fields = (
%standard_fields,
%{ $test->{expected_form_fields} },
@@ -2178,4 +2180,11 @@ FixMyStreet::override_config {
};
};
+subtest 'check disabling of updates per category' => sub {
+ $contact->set_extra_metadata( updates_disallowed => 1 );
+ $contact->update;
+ $mech->get_ok("/report/$report_id");
+ $mech->content_lacks('Provide an update');
+};
+
done_testing();
diff --git a/templates/web/base/admin/bodies/contact-form.html b/templates/web/base/admin/bodies/contact-form.html
index 35fab4541..ddee35020 100644
--- a/templates/web/base/admin/bodies/contact-form.html
+++ b/templates/web/base/admin/bodies/contact-form.html
@@ -63,6 +63,11 @@