From 7fbc166200eeeaefdaecdf060acd3d8d395e21b0 Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Wed, 6 Dec 2023 10:31:32 +0000 Subject: [PATCH 1/3] [UK] Bypass ownership check on reports pages. On reports pages (and heatmap), we know all reports involved are owned by the body, so do not need to perform this check. Copy of e4addf23 for other cobrands. --- perllib/FixMyStreet/Cobrand/Bromley.pm | 2 +- perllib/FixMyStreet/Cobrand/CheshireEast.pm | 2 +- perllib/FixMyStreet/Cobrand/Lincolnshire.pm | 3 +-- perllib/FixMyStreet/Cobrand/Oxfordshire.pm | 2 +- perllib/FixMyStreet/Cobrand/UKCouncils.pm | 2 +- 5 files changed, 5 insertions(+), 6 deletions(-) diff --git a/perllib/FixMyStreet/Cobrand/Bromley.pm b/perllib/FixMyStreet/Cobrand/Bromley.pm index 8a40cdf1169..483567662c1 100644 --- a/perllib/FixMyStreet/Cobrand/Bromley.pm +++ b/perllib/FixMyStreet/Cobrand/Bromley.pm @@ -102,7 +102,7 @@ sub geocode_postcode { # Bromley pins always yellow sub pin_colour { my ( $self, $p, $context ) = @_; - return 'grey' if !$self->owns_problem( $p ); + return 'grey' if ($context ne 'reports' && !$self->owns_problem($p)); return 'yellow'; } diff --git a/perllib/FixMyStreet/Cobrand/CheshireEast.pm b/perllib/FixMyStreet/Cobrand/CheshireEast.pm index e02793b6a9c..9c91faec340 100644 --- a/perllib/FixMyStreet/Cobrand/CheshireEast.pm +++ b/perllib/FixMyStreet/Cobrand/CheshireEast.pm @@ -100,7 +100,7 @@ sub send_questionnaires { 0 } sub pin_colour { my ( $self, $p, $context ) = @_; - return 'grey' if $p->state eq 'not responsible' || !$self->owns_problem( $p ); + return 'grey' if $p->state eq 'not responsible' || ($context ne 'reports' && !$self->owns_problem($p)); return 'green' if $p->is_fixed || $p->is_closed; return 'yellow' if $p->is_in_progress; return 'red'; diff --git a/perllib/FixMyStreet/Cobrand/Lincolnshire.pm b/perllib/FixMyStreet/Cobrand/Lincolnshire.pm index 25a9fc41b2e..e465631eadb 100644 --- a/perllib/FixMyStreet/Cobrand/Lincolnshire.pm +++ b/perllib/FixMyStreet/Cobrand/Lincolnshire.pm @@ -176,10 +176,9 @@ Lincolnshire uses the following pin colours: sub pin_colour { my ( $self, $p, $context ) = @_; - my $ext_status = $p->get_extra_metadata('external_status_code'); return 'grey' - if $p->state eq 'not responsible' || !$self->owns_problem($p); + if $p->state eq 'not responsible' || ($context ne 'reports' && !$self->owns_problem($p)); return 'orange' if $p->state eq 'investigating' || $p->state eq 'for triage'; return 'yellow' diff --git a/perllib/FixMyStreet/Cobrand/Oxfordshire.pm b/perllib/FixMyStreet/Cobrand/Oxfordshire.pm index 8ca5eeef672..edc286a5e39 100644 --- a/perllib/FixMyStreet/Cobrand/Oxfordshire.pm +++ b/perllib/FixMyStreet/Cobrand/Oxfordshire.pm @@ -85,7 +85,7 @@ sub reports_ordering { sub pin_colour { my ( $self, $p, $context ) = @_; - return 'grey' unless $self->owns_problem( $p ); + return 'grey' if $context ne 'reports' && !$self->owns_problem($p); return 'grey' if $p->is_closed; return 'green' if $p->is_fixed; return 'yellow' if $p->state eq 'confirmed'; diff --git a/perllib/FixMyStreet/Cobrand/UKCouncils.pm b/perllib/FixMyStreet/Cobrand/UKCouncils.pm index eb5297ec662..1af1f4b256d 100644 --- a/perllib/FixMyStreet/Cobrand/UKCouncils.pm +++ b/perllib/FixMyStreet/Cobrand/UKCouncils.pm @@ -287,7 +287,7 @@ sub owns_problem { # then show pins for the other council as grey sub pin_colour { my ( $self, $p, $context ) = @_; - return 'grey' if !$self->owns_problem( $p ); + return 'grey' if $context ne 'reports' && !$self->owns_problem($p); return $self->next::method($p, $context); } From bf80e88e6c419d46d3c485df9ec1035e99c47d2c Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Wed, 6 Dec 2023 10:32:49 +0000 Subject: [PATCH 2/3] Remove prefetch for heatmap pin fetching. The prefetch used in the sidebar queries is not needed when fetching the pin data. --- perllib/FixMyStreet/App/Controller/Dashboard.pm | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/perllib/FixMyStreet/App/Controller/Dashboard.pm b/perllib/FixMyStreet/App/Controller/Dashboard.pm index c3cb07f6074..7405f28c10c 100644 --- a/perllib/FixMyStreet/App/Controller/Dashboard.pm +++ b/perllib/FixMyStreet/App/Controller/Dashboard.pm @@ -409,7 +409,9 @@ sub heatmap : Local : Args(0) { my $parameters = $c->forward( '/reports/load_problems_parameters'); my $where = $parameters->{where}; + # Filter includes order_by, rows, and a prefetch entry my $filter = $parameters->{filter}; + # We don't need the rows, as we always want all reports delete $filter->{rows}; $c->forward('heatmap_filters', [ $where ]); @@ -423,6 +425,9 @@ sub heatmap : Local : Args(0) { if ($c->get_param('ajax')) { my @pins; + # We don't need any of the prefetched stuff now + delete $filter->{prefetch}; + my $problems = $c->cobrand->problems->to_body($body)->search($where, $filter); while ( my $problem = $problems->next ) { push @pins, $problem->pin_data('reports'); } From 77a2ea8db02cc68a5b57d09f4a58851cb653871f Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Wed, 6 Dec 2023 10:34:46 +0000 Subject: [PATCH 3/3] Store states in schema cache. Even though these are saved in memcache, if we need to look it up a lot of times in one request, is quicker to cache it locally as well. --- perllib/FixMyStreet/DB/ResultSet/State.pm | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/perllib/FixMyStreet/DB/ResultSet/State.pm b/perllib/FixMyStreet/DB/ResultSet/State.pm index 4f98efbf24a..208acccdd01 100644 --- a/perllib/FixMyStreet/DB/ResultSet/State.pm +++ b/perllib/FixMyStreet/DB/ResultSet/State.pm @@ -18,12 +18,18 @@ sub _hardcoded_states { # we cache these in the package on first use, and clear on update. sub clear { + my $rs = shift; Memcached::set('states', ''); + my $cache = $rs->result_source->schema->cache; + delete $cache->{states}; } sub states { my $rs = shift; + my $cache = $rs->result_source->schema->cache; + return $cache->{states} if $cache->{states}; + my $states = Memcached::get('states'); # If tests are run in parallel, the cached state in Memcached could be # corrupted by multiple tests changing it at the same time @@ -31,6 +37,7 @@ sub states { if ($states && !FixMyStreet->test_mode) { # Need to reattach schema $states->[0]->result_source->schema( $rs->result_source->schema ) if $states->[0]; + $cache->{states} = $states; return $states; } @@ -45,6 +52,7 @@ sub states { my @states = ($rs->_hardcoded_states, $rs->search(undef, { order_by => 'label' })->all); $_->translated->{name} = $trans{$_->id} || {} foreach @states; $states = \@states; + $cache->{states} = $states; Memcached::set('states', $states); return $states; }