Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Open Location Codes support to search box. #3047

Merged
merged 2 commits into from
Jun 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
## Releases

* Unreleased
- New features:
- Add Open Location Codes support to search box. #3047
- Bugfixes:
- Fix issue with dashboard report CSV export. #3026
- bin/update-schema PostgreSQL 12 compatibility. #3043
Expand Down
1 change: 1 addition & 0 deletions cpanfile
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ requires 'Error';
requires 'FCGI'; # Required by e.g. Plack::Handler::FCGI
requires 'File::Find';
requires 'File::Path';
requires 'Geo::OLC';
requires 'Geography::NationalGrid',
mirror => 'https://cpan.metacpan.org/';
requires 'Getopt::Long::Descriptive', '0.105';
Expand Down
9 changes: 9 additions & 0 deletions cpanfile.snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -3116,6 +3116,15 @@ DISTRIBUTIONS
perl 5.014000
strict 0
warnings 0
Geo-OLC-1
pathname: J/JG/JGREELY/Geo-OLC-1.tar.gz
provides:
Geo::OLC 1
requirements:
ExtUtils::MakeMaker 0
List::Util 1
Test::More 0
perl 5.010001
Geography-NationalGrid-1.6
pathname: P/PK/PKENT/Geography-NationalGrid-1.6.tar.gz
provides:
Expand Down
25 changes: 25 additions & 0 deletions perllib/FixMyStreet/App/Controller/Location.pm
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ BEGIN {extends 'Catalyst::Controller'; }

use Encode;
use FixMyStreet::Geocode;
use Geo::OLC;
use Try::Tiny;
use Utils;

Expand Down Expand Up @@ -74,6 +75,30 @@ sub determine_location_from_pc : Private {
map { Utils::truncate_coordinate($_) } ($1, $2);
return $c->forward( 'check_location' );
}

if (Geo::OLC::is_full($pc)) {
my $ref = Geo::OLC::decode($pc);
($c->stash->{latitude}, $c->stash->{longitude}) =
map { Utils::truncate_coordinate($_) } @{$ref->{center}};
return $c->forward( 'check_location' );
}

if ($pc =~ /^\s*([2-9CFGHJMPQRVWX]{4,6}\+[2-9CFGHJMPQRVWX]{2,3})\s+(.*)$/i) {
my ($code, $rest) = ($1, $2);
my ($lat, $lon, $error) = FixMyStreet::Geocode::lookup($rest, $c);
if (ref($error) eq 'ARRAY') { # Just take the first result
$lat = $error->[0]{latitude};
$lon = $error->[0]{longitude};
}
if (defined $lat && defined $lon) {
$code = Geo::OLC::recover_nearest($code, $lat, $lon);
my $ref = Geo::OLC::decode($code);
($c->stash->{latitude}, $c->stash->{longitude}) =
map { Utils::truncate_coordinate($_) } @{$ref->{center}};
return $c->forward( 'check_location' );
}
}

if ( $c->cobrand->country eq 'GB' && $pc =~ /^([A-Z])([A-Z])([\d\s]{4,})$/i) {
if (my $convert = gridref_to_latlon( $1, $2, $3 )) {
($c->stash->{latitude}, $c->stash->{longitude}) =
Expand Down
5 changes: 5 additions & 0 deletions t/Mock/Nominatim.pm
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package t::Mock::Nominatim;

use JSON::MaybeXS;
use LWP::Protocol::PSGI;
use Web::Simple;

has json => (
Expand Down Expand Up @@ -35,6 +36,10 @@ sub query {
{"osm_type"=>"way","osm_id"=>"4684282","lat"=>"55.9504009","lon"=>"-3.1858425","display_name"=>"High Street, Old Town, City of Ed\x{ed}nburgh, Scotland, EH1 1SP, United Kingdom","class"=>"highway","type"=>"tertiary","importance"=>0.55892577838734},
{"osm_type"=>"node","osm_id"=>"27424410","lat"=>"55.8596449","lon"=>"-4.240377","display_name"=>"High Street, Collegelands, Merchant City, Glasgow, Glasgow City, Scotland, G, United Kingdom","class"=>"railway","type"=>"station","importance"=>0.53074299592768}
];
} elsif ($q eq 'edinburgh') {
return [
{"osm_type"=>"node","osm_id"=>"17898859","lat"=>"55.9533456","lon"=>"-3.1883749","display_name"=>"Edinburgh","class"=>"place","type"=>"place:city","importance"=>0.676704}
];
}
return [];
}
Expand Down
120 changes: 57 additions & 63 deletions t/app/controller/around.t
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use constant MIN_ZOOM_LEVEL => 88;
package main;

use Test::MockModule;
use t::Mock::Nominatim;

use FixMyStreet::TestMech;
my $mech = FixMyStreet::TestMech->new;
Expand Down Expand Up @@ -53,6 +54,11 @@ foreach my $test (
};
}

FixMyStreet::override_config {
ALLOWED_COBRANDS => 'fixmystreet',
MAPIT_URL => 'http://mapit.uk/',
}, sub {

# check that exact queries result in the correct lat,lng
foreach my $test (
{
Expand All @@ -69,19 +75,45 @@ foreach my $test (
{
subtest "check lat/lng for '$test->{pc}'" => sub {
$mech->get_ok('/');
FixMyStreet::override_config {
ALLOWED_COBRANDS => [ { 'fixmystreet' => '.' } ],
MAPIT_URL => 'http://mapit.uk/',
}, sub {
$mech->submit_form_ok( { with_fields => { pc => $test->{pc} } },
"good location" );
};
$mech->submit_form_ok( { with_fields => { pc => $test->{pc} } },
"good location" );
is_deeply $mech->page_errors, [], "no errors for pc '$test->{pc}'";
is_deeply $mech->extract_location, $test,
"got expected location for pc '$test->{pc}'";
$mech->get_ok('/');
my $pc = "$test->{latitude},$test->{longitude}";
$mech->submit_form_ok( { with_fields => { pc => $pc } },
"good location" );
is_deeply $mech->page_errors, [], "no errors for pc '$pc'";
is_deeply $mech->extract_location, { %$test, pc => $pc },
"got expected location for pc '$pc'";
};
}

subtest "check lat/lng for full plus code" => sub {
$mech->get_ok('/');
$mech->submit_form_ok( { with_fields => { pc => "9C7RXR26+R5" } } );
is_deeply $mech->page_errors, [], "no errors for plus code";
is_deeply $mech->extract_location, {
pc => "9C7RXR26+R5",
latitude => 55.952063,
longitude => -3.189562,
},
"got expected location for full plus code";
};

subtest "check lat/lng for short plus code" => sub {
$mech->get_ok('/');
$mech->submit_form_ok( { with_fields => { pc => "XR26+R5 Edinburgh" } } );
is_deeply $mech->page_errors, [], "no errors for plus code";
is_deeply $mech->extract_location, {
pc => "XR26+R5 Edinburgh",
latitude => 55.952063,
longitude => -3.189562,
},
"got expected location for short plus code";
};

my $body_edin_id = $mech->create_body_ok(2651, 'City of Edinburgh Council')->id;
my $body_west_id = $mech->create_body_ok(2504, 'Westminster City Council')->id;

Expand All @@ -93,10 +125,10 @@ my @edinburgh_problems = $mech->create_problems_for_body( 5, $body_edin_id, 'Aro

subtest 'check lookup by reference' => sub {
$mech->get_ok('/');
$mech->submit_form_ok( { with_fields => { pc => 'ref:12345' } }, 'bad ref');
$mech->submit_form_ok( { with_fields => { pc => '12345' } }, 'bad ref');
$mech->content_contains('Searching found no reports');
my $id = $edinburgh_problems[0]->id;
$mech->submit_form_ok( { with_fields => { pc => "ref:$id" } }, 'good ref');
$mech->submit_form_ok( { with_fields => { pc => $id } }, 'good ref');
is $mech->uri->path, "/report/$id", "redirected to report page";
};

Expand All @@ -106,46 +138,31 @@ subtest 'check lookup by reference does not show non_public reports' => sub {
});
my $id = $edinburgh_problems[0]->id;
$mech->get_ok('/');
$mech->submit_form_ok( { with_fields => { pc => "ref:$id" } }, 'non_public ref');
$mech->submit_form_ok( { with_fields => { pc => $id } }, 'non_public ref');
$mech->content_contains('Searching found no reports');
};

subtest 'check non public reports are not displayed on around page' => sub {
$mech->get_ok('/');
FixMyStreet::override_config {
ALLOWED_COBRANDS => [ { 'fixmystreet' => '.' } ],
MAPIT_URL => 'http://mapit.uk/',
}, sub {
$mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } },
"good location" );
};
$mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } },
"good location" );
$mech->content_contains( "Around page Test 3 for $body_edin_id",
'problem to be marked non public visible' );

my $private = $edinburgh_problems[2];
ok $private->update( { non_public => 1 } ), 'problem marked non public';

$mech->get_ok('/');
FixMyStreet::override_config {
ALLOWED_COBRANDS => [ { 'fixmystreet' => '.' } ],
MAPIT_URL => 'http://mapit.uk/',
}, sub {
$mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } },
"good location" );
};
$mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } },
"good location" );
$mech->content_lacks( "Around page Test 3 for $body_edin_id",
'problem marked non public is not visible' );
};

subtest 'check missing body message not shown when it does not need to be' => sub {
$mech->get_ok('/');
FixMyStreet::override_config {
ALLOWED_COBRANDS => 'fixmystreet',
MAPIT_URL => 'http://mapit.uk/',
}, sub {
$mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } },
"good location" );
};
$mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } },
"good location" );
$mech->content_lacks('yet have details for the other councils that cover this location');
};

Expand All @@ -162,24 +179,14 @@ for my $permission ( qw/ report_inspect report_mark_private/ ) {
});

$mech->get_ok('/');
FixMyStreet::override_config {
ALLOWED_COBRANDS => [ { 'fixmystreet' => '.' } ],
MAPIT_URL => 'http://mapit.uk/',
}, sub {
$mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } },
"good location" );
};
$mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } },
"good location" );
$mech->content_contains( "Around page Test 3 for $body_edin_id",
'problem marked non public is visible' );
$mech->content_contains( "Around page Test 2 for $body_edin_id",
'problem marked public is visible' );

FixMyStreet::override_config {
ALLOWED_COBRANDS => [ { 'fixmystreet' => '.' } ],
MAPIT_URL => 'http://mapit.uk/',
}, sub {
$mech->get_ok('/around?pc=EH1+1BB&status=non_public');
};
$mech->get_ok('/around?pc=EH1+1BB&status=non_public');
$mech->content_contains( "Around page Test 3 for $body_edin_id",
'problem marked non public is visible' );
$mech->content_lacks( "Around page Test 2 for $body_edin_id",
Expand All @@ -193,24 +200,14 @@ for my $permission ( qw/ report_inspect report_mark_private/ ) {
});

$mech->get_ok('/');
FixMyStreet::override_config {
ALLOWED_COBRANDS => [ { 'fixmystreet' => '.' } ],
MAPIT_URL => 'http://mapit.uk/',
}, sub {
$mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } },
"good location" );
};
$mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } },
"good location" );
$mech->content_lacks( "Around page Test 3 for $body_edin_id",
'problem marked non public is not visible' );
$mech->content_contains( "Around page Test 2 for $body_edin_id",
'problem marked public is visible' );

FixMyStreet::override_config {
ALLOWED_COBRANDS => [ { 'fixmystreet' => '.' } ],
MAPIT_URL => 'http://mapit.uk/',
}, sub {
$mech->get_ok('/around?pc=EH1+1BB&status=non_public');
};
$mech->get_ok('/around?pc=EH1+1BB&status=non_public');
$mech->content_lacks( "Around page Test 3 for $body_edin_id",
'problem marked non public is not visible' );
$mech->content_lacks( "Around page Test 2 for $body_edin_id",
Expand All @@ -230,17 +227,14 @@ subtest 'check assigned-only list items do not display shortlist buttons' => sub
$user->update({ from_body => $body });
$user->user_body_permissions->find_or_create({ body => $body, permission_type => 'planned_reports' });

FixMyStreet::override_config {
ALLOWED_COBRANDS => 'fixmystreet',
MAPIT_URL => 'http://mapit.uk/',
}, sub {
$mech->get_ok('/around?pc=EH1+1BB');
};
$mech->get_ok('/around?pc=EH1+1BB');
$mech->content_contains('shortlist-add-' . $edinburgh_problems[4]->id);
$mech->content_lacks('shortlist-add-' . $edinburgh_problems[3]->id);
$mech->content_lacks('shortlist-add-' . $edinburgh_problems[1]->id);
};

}; # End big override_config

my $body = $mech->create_body_ok(2237, "Oxfordshire");

subtest 'check category, status and extra filtering works on /around' => sub {
Expand Down