From 52c7f811a298646ddcf8e77226fdac1457e2fcad Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Mon, 21 Oct 2024 14:48:07 +0100 Subject: [PATCH] [UK] Remove broken area check. `check_report_is_on_cobrand_asset` used shift @_, so $council_area was always blank. This function is always returning the name of an area if it matches against the asset layer. I think the intention was if you clicked on an area that was outside the boundary, but their responsibility, you'd continue, but outside and someone else's responsibility, you'd get "go to .com". But what actually happened was you'd continue on the cobrand site, which worked fine. --- perllib/FixMyStreet/Cobrand/Brent.pm | 13 +++---------- perllib/FixMyStreet/Cobrand/Bromley.pm | 2 +- perllib/FixMyStreet/Cobrand/Camden.pm | 13 +++---------- perllib/FixMyStreet/Cobrand/UKCouncils.pm | 2 +- 4 files changed, 8 insertions(+), 22 deletions(-) diff --git a/perllib/FixMyStreet/Cobrand/Brent.pm b/perllib/FixMyStreet/Cobrand/Brent.pm index 417ef0b94b2..0e775918fc4 100644 --- a/perllib/FixMyStreet/Cobrand/Brent.pm +++ b/perllib/FixMyStreet/Cobrand/Brent.pm @@ -161,13 +161,12 @@ sub geocoder_munge_results { =head2 check_report_is_on_cobrand_asset If the location is covered by an area of differing responsibility (e.g. Brent -in Camden, or Camden in Brent), return true (either 1 if an area name is -provided, or the name of the area if not). +in Camden, or Camden in Brent), return the name of the area. =cut sub check_report_is_on_cobrand_asset { - my ($self, $council_area) = shift @_; + my $self = shift; my $lat = $self->{c}->stash->{latitude}; my $lon = $self->{c}->stash->{longitude}; @@ -184,13 +183,7 @@ sub check_report_is_on_cobrand_asset { my $features = $self->_fetch_features($cfg, -1, -1, 1); if ($$features[0]) { - if ($council_area) { - if ($$features[0]->{'ms:BrentDiffs'}->{'ms:name'} eq $council_area) { - return 1; - } - } else { - return $$features[0]->{'ms:BrentDiffs'}->{'ms:name'}; - } + return $$features[0]->{'ms:BrentDiffs'}->{'ms:name'}; } } diff --git a/perllib/FixMyStreet/Cobrand/Bromley.pm b/perllib/FixMyStreet/Cobrand/Bromley.pm index d387838aa3e..77170743b88 100644 --- a/perllib/FixMyStreet/Cobrand/Bromley.pm +++ b/perllib/FixMyStreet/Cobrand/Bromley.pm @@ -152,7 +152,7 @@ sub title_list { } sub check_report_is_on_cobrand_asset { - my ($self, $council_area) = shift @_; + my $self = shift; if ($self->{c}->get_param('feature_id') && $self->{c}->get_param('feature_id') =~ /A-48-24|A-48-26|A-48-27/) { return 1; diff --git a/perllib/FixMyStreet/Cobrand/Camden.pm b/perllib/FixMyStreet/Cobrand/Camden.pm index 3c979db6222..284fd50ba7c 100644 --- a/perllib/FixMyStreet/Cobrand/Camden.pm +++ b/perllib/FixMyStreet/Cobrand/Camden.pm @@ -215,13 +215,12 @@ sub user_from_oidc { =head2 check_report_is_on_cobrand_asset If the location is covered by an area of differing responsibility (e.g. Brent -in Camden, or Camden in Brent), return true (either 1 if an area name is -provided, or the name of the area if not). +in Camden, or Camden in Brent), return the name of that area. =cut sub check_report_is_on_cobrand_asset { - my ($self, $council_area) = shift @_; + my $self = shift; my $lat = $self->{c}->stash->{latitude}; my $lon = $self->{c}->stash->{longitude}; @@ -239,13 +238,7 @@ sub check_report_is_on_cobrand_asset { my $features = $self->_fetch_features($cfg, $x, $y, 1); if ($$features[0]) { - if ($council_area) { - if ($$features[0]->{'ms:AgreementBoundaries'}->{'ms:RESPBOROUG'} eq $council_area) { - return 1; - } - } else { - return $$features[0]->{'ms:AgreementBoundaries'}->{'ms:RESPBOROUG'}; - } + return $$features[0]->{'ms:AgreementBoundaries'}->{'ms:RESPBOROUG'}; } } diff --git a/perllib/FixMyStreet/Cobrand/UKCouncils.pm b/perllib/FixMyStreet/Cobrand/UKCouncils.pm index bc2f1583985..8ed4e1d746a 100644 --- a/perllib/FixMyStreet/Cobrand/UKCouncils.pm +++ b/perllib/FixMyStreet/Cobrand/UKCouncils.pm @@ -210,7 +210,7 @@ sub responsible_for_areas { if (grep ($self->council_area_id->[0] == $_, keys %$councils)) { return 1; } else { - return $self->check_report_is_on_cobrand_asset($self->council_area); + return $self->check_report_is_on_cobrand_asset; } } else { # The majority of cobrands only cover a single area, but e.g. Northamptonshire