Skip to content

Commit

Permalink
[UK] Remove broken area check.
Browse files Browse the repository at this point in the history
`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.
  • Loading branch information
dracos committed Oct 21, 2024
1 parent dc9169e commit 52c7f81
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 22 deletions.
13 changes: 3 additions & 10 deletions perllib/FixMyStreet/Cobrand/Brent.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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'};
}
}

Expand Down
2 changes: 1 addition & 1 deletion perllib/FixMyStreet/Cobrand/Bromley.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 3 additions & 10 deletions perllib/FixMyStreet/Cobrand/Camden.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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'};
}
}

Expand Down
2 changes: 1 addition & 1 deletion perllib/FixMyStreet/Cobrand/UKCouncils.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 52c7f81

Please sign in to comment.