diff options
| -rw-r--r-- | perllib/FixMyStreet/Geocode.pm | 36 | ||||
| -rw-r--r-- | perllib/FixMyStreet/Map/Tilma/Original.pm | 46 | ||||
| -rw-r--r-- | perllib/Page.pm | 5 | ||||
| -rwxr-xr-x | web/index.cgi | 159 | ||||
| -rwxr-xr-x | web/questionnaire.cgi | 4 |
5 files changed, 166 insertions, 84 deletions
diff --git a/perllib/FixMyStreet/Geocode.pm b/perllib/FixMyStreet/Geocode.pm index 4854411cf..0379169b8 100644 --- a/perllib/FixMyStreet/Geocode.pm +++ b/perllib/FixMyStreet/Geocode.pm @@ -37,40 +37,32 @@ BEGIN { # of the site to diambiguate locations. sub lookup { my ($s, $q) = @_; - my ($easting, $northing, $error); + my ($latitude, $longitude, $error); if ($s =~ /^\d+$/) { $error = 'FixMyStreet is a UK-based website that currently works in England, Scotland, and Wales. Please enter either a postcode, or a Great British street name and area.'; } elsif (mySociety::PostcodeUtil::is_valid_postcode($s)) { my $location = mySociety::MaPit::call('postcode', $s); unless ($error = Page::mapit_check_error($location)) { - $easting = $location->{easting}; - $northing = $location->{northing}; + $latitude = $location->{wgs84_lat}; + $longitude = $location->{wgs84_lon}; } } else { - ($easting, $northing, $error) = FixMyStreet::Geocode::string($s, $q); + ($latitude, $longitude, $error) = FixMyStreet::Geocode::string($s, $q); } - return ($easting, $northing, $error); + return ($latitude, $longitude, $error); } sub geocoded_string_coordinates { my ($js, $q) = @_; - my ($easting, $northing, $error); + my ($latitude, $longitude, $error); my ($accuracy) = $js =~ /"Accuracy" *: *(\d)/; if ($accuracy < 4) { $error = _('Sorry, that location appears to be too general; please be more specific.'); - } else { - - $js =~ /"coordinates" *: *\[ *(.*?), *(.*?),/; - my $lon = $1; my $lat = $2; - try { - ($easting, $northing) = mySociety::GeoUtil::wgs84_to_national_grid($lat, $lon, 'G'); - } catch Error::Simple with { - $error = shift; - $error = _('That location does not appear to be in Britain; please try again.') - if $error =~ /out of the area covered/; - } - } - return ($easting, $northing, $error); + } elsif ( $js =~ /"coordinates" *: *\[ *(.*?), *(.*?),/ ) { + $longitude = $1; + $latitude = $2; + } + return ($latitude, $longitude, $error); } # string STRING QUERY @@ -90,7 +82,7 @@ sub string { my $url = 'http://maps.google.com/maps/geo?' . $s; my $cache_dir = mySociety::Config::get('GEO_CACHE'); my $cache_file = $cache_dir . md5_hex($url); - my ($js, $error, $easting, $northing); + my ($js, $error, $latitude, $longitude); if (-s $cache_file) { $js = File::Slurp::read_file($cache_file); } else { @@ -123,9 +115,9 @@ sub string { # Northern Ireland, hopefully $error = _("We do not cover Northern Ireland, I'm afraid, as our licence doesn't include any maps for the region."); } else { - ($easting, $northing, $error) = geocoded_string_coordinates($js, $q); + ($latitude, $longitude, $error) = geocoded_string_coordinates($js, $q); } - return ($easting, $northing, $error); + return ($latitude, $longitude, $error); } # list_choices diff --git a/perllib/FixMyStreet/Map/Tilma/Original.pm b/perllib/FixMyStreet/Map/Tilma/Original.pm index 2e460092b..a8c44ccac 100644 --- a/perllib/FixMyStreet/Map/Tilma/Original.pm +++ b/perllib/FixMyStreet/Map/Tilma/Original.pm @@ -13,6 +13,7 @@ use LWP::Simple; use Cobrand; use mySociety::Web qw(ent NewURL); +use mySociety::GeoUtil qw(national_grid_to_wgs84); sub header_js { return ' @@ -22,7 +23,7 @@ sub header_js { # display_map Q PARAMS # PARAMS include: -# EASTING, NORTHING for the centre point of the map +# latitude, longitude for the centre point of the map # TYPE is 1 if the map is clickable, 2 if clickable and has a form upload, # 0 if not clickable # PINS is array of pins to show, location and colour @@ -36,6 +37,21 @@ sub _display_map { $mid_point = 189; } + # convert map center point to easting, northing + ( $params{easting}, $params{northing} ) = + mySociety::GeoUtil::wgs84_to_national_grid( # + $params{latitude}, $params{longitude}, 'G' + ); + + # FIXME - convert all pins to lat, lng + # all the pins are currently [lat, lng, colour] - convert them + foreach my $pin ( @{ $params{pins} ||= [] } ) { + my ( $e, $n ) = ( $pin->[0], $pin->[1] ); + my ( $lat, $lon ) = + mySociety::GeoUtil::wgs84_to_national_grid( $e, $n, 'G' ); + ( $pin->[0], $pin->[1] ) = ( $lat, $lon ); + } + # X/Y tile co-ords may be overridden in the query string my @vars = qw(x y); my %input = map { $_ => $q->param($_) || '' } @vars; @@ -196,10 +212,30 @@ sub tile_to_px { sub os_to_tile { return $_[0] / SCALE_FACTOR; } + sub tile_to_os { return int($_[0] * SCALE_FACTOR + 0.5); } +=head2 tile_xy_to_wgs84 + + ($lat, $lon) = tile_xy_to_wgs84( $x, $y ); + +Takes the tile x,y and converts to lat, lon. + +=cut + +sub tile_xy_to_wgs84 { + my ( $x, $y ) = @_; + + my $easting = tile_to_os($x); + my $northing = tile_to_os($y); + + my ( $lat, $lon ) = national_grid_to_wgs84( $easting, $northing, 'G' ); + return ( $lat, $lon ); +} + + sub click_to_tile { my ($pin_tile, $pin, $invert) = @_; $pin -= TILE_WIDTH while $pin > TILE_WIDTH; @@ -219,6 +255,14 @@ sub click_to_os { return ($easting, $northing); } +# Given some click co-ords (the tile they were on, and where in the +# tile they were), convert to WGS84 and return. +sub click_to_wgs84 { + my ( $easting, $northing ) = FixMyStreet::Map::click_to_os(@_); + my ( $lat, $lon ) = national_grid_to_wgs84( $easting, $northing, 'G' ); + return ( $lat, $lon ); +} + # Given (E,N) and potential override (X,Y), return the X/Y tile for the centre # of the map (either to get the point near the middle, or the override X,Y), # and the pixel co-ords of the point, relative to that map. diff --git a/perllib/Page.pm b/perllib/Page.pm index b3f320f5d..fc7127a78 100644 --- a/perllib/Page.pm +++ b/perllib/Page.pm @@ -80,6 +80,9 @@ sub report_error { my $trylater = sprintf(_('Please try again later, or <a href="mailto:%s">email us</a> to let us know.'), $contact_email); my $somethingwrong = _("Sorry! Something's gone wrong."); my $errortext = _("The text of the error was:"); + + my $msg_br = join '<br><br>', split m{\n}, $msg; + print "Status: 500\nContent-Type: text/html; charset=iso-8859-1\n\n", qq(<html><head><title>$somethingwrong</title></head></html>), q(<body>), @@ -87,7 +90,7 @@ sub report_error { qq(<p>$trylater</p>), q(<hr>), qq(<p>$errortext</p>), - qq(<blockquote class="errortext">$msg</blockquote>), + qq(<blockquote class="errortext">$msg_br</blockquote>), q(</body></html>); } diff --git a/web/index.cgi b/web/index.cgi index 5228b7e3f..18a83e302 100755 --- a/web/index.cgi +++ b/web/index.cgi @@ -16,6 +16,8 @@ use RABX; use CGI::Carp; use URI::Escape; +use Carp::Always; + use CrossSell; use FixMyStreet::Geocode; use mySociety::AuthToken; @@ -30,6 +32,11 @@ use mySociety::Random; use mySociety::VotingArea; use mySociety::Web qw(ent NewURL); +sub debug (@) { + my ( $format, @args ) = @_; + warn sprintf $format, map { defined $_ ? $_ : 'undef' } @args; +} + BEGIN { if (!dbh()->selectrow_array('select secret from secret for update of secret')) { local dbh()->{HandleError}; @@ -432,22 +439,29 @@ sub display_form { push @errors, _('There were problems with your report. Please see below.') if (scalar keys %field_errors); my ($pin_x, $pin_y, $pin_tile_x, $pin_tile_y) = (0,0,0,0); - my @vars = qw(title detail name email phone pc easting northing x y skipped council anonymous partial upload_fileid lat lon); - my %input = map { $_ => $q->param($_) || '' } @vars; - my %input_h = map { $_ => $q->param($_) ? ent($q->param($_)) : '' } @vars; + my @vars = qw(title detail name email phone pc latitude longitude x y skipped council anonymous partial upload_fileid); - # Convert lat/lon to easting/northing if given - if ($input{lat}) { - try { - ($input{easting}, $input{northing}) = mySociety::GeoUtil::wgs84_to_national_grid($input{lat}, $input{lon}, 'G'); - $input_h{easting} = $input{easting}; - $input_h{northing} = $input{northing}; - } catch Error::Simple with { - my $e = shift; - push @errors, "We had a problem with the supplied co-ordinates - outside the UK?"; - }; + my %input = (); + my %input_h = (); + + foreach my $key (@vars) { + my $val = $q->param($key); + $input{$key} = defined($val) ? $val : ''; # '0' is valid for longitude + $input_h{$key} = ent( $input{$key} ); } + # Convert lat/lon to easting/northing if given + # if ($input{lat}) { + # try { + # ($input{easting}, $input{northing}) = mySociety::GeoUtil::wgs84_to_national_grid($input{lat}, $input{lon}, 'G'); + # $input_h{easting} = $input{easting}; + # $input_h{northing} = $input{northing}; + # } catch Error::Simple with { + # my $e = shift; + # push @errors, "We had a problem with the supplied co-ordinates - outside the UK?"; + # }; + # } + # Get tile co-ordinates if map clicked ($input{x}) = $input{x} =~ /^(\d+)/; $input{x} ||= 0; ($input{y}) = $input{y} =~ /^(\d+)/; $input{y} ||= 0; @@ -460,28 +474,30 @@ sub display_form { # We need either a map click, an E/N, to be skipping the map, or be filling in a partial form return display_location($q, @errors) unless ($pin_x && $pin_y) - || ($input{easting} && $input{northing}) + || ($input{latitude} && $input{longitude}) || ($input{skipped} && $input{pc}) || ($input{partial} && $input{pc}); # Work out some co-ordinates from whatever we've got - my ($easting, $northing); + my ($latitude, $longitude); if ($input{skipped}) { # Map is being skipped - if ($input{easting} && $input{northing}) { - $easting = $input{easting}; - $northing = $input{northing}; + if ( length $input{latitude} && length $input{longitude} ) { + $latitude = $input{latitude}; + $longitude = $input{longitude}; } else { - my ($e, $n, $error) = FixMyStreet::Geocode::lookup($input{pc}, $q); - $easting = $e; $northing = $n; + my ( $lat, $lon, $error ) = + FixMyStreet::Geocode::lookup( $input{pc}, $q ); + $latitude = $lat; + $longitude = $lon; } } elsif ($pin_x && $pin_y) { # tilma map was clicked on - ($easting, $northing) = FixMyStreet::Map::click_to_os($pin_tile_x, $pin_x, $pin_tile_y, $pin_y); - } elsif ($input{partial} && $input{pc} && !$input{easting} && !$input{northing}) { + ($latitude, $longitude) = FixMyStreet::Map::click_to_wgs84($pin_tile_x, $pin_x, $pin_tile_y, $pin_y); + } elsif ( $input{partial} && $input{pc} && !length $input{latitude} && !length $input{longitude} ) { my $error; try { - ($easting, $northing, $error) = FixMyStreet::Geocode::lookup($input{pc}, $q); + ($latitude, $longitude, $error) = FixMyStreet::Geocode::lookup($input{pc}, $q); } catch Error::Simple with { $error = shift; }; @@ -489,8 +505,8 @@ sub display_form { return front_page($q, $error) if $error; } else { # Normal form submission - $easting = $input_h{easting}; - $northing = $input_h{northing}; + $latitude = $input_h{latitude}; + $longitude = $input_h{longitude}; } # Look up councils and do checks for the point we've got @@ -498,7 +514,7 @@ sub display_form { $parent_types = [qw(DIS LBO MTD UTA LGD COI)] # No CTY if $q->{site} eq 'emptyhomes'; # XXX: I think we want in_gb_locale around the next line, needs testing - my $all_councils = mySociety::MaPit::call('point', "27700/$easting,$northing", type => $parent_types); + my $all_councils = mySociety::MaPit::call('point', "4326/$longitude,$latitude", type => $parent_types); # Let cobrand do a check my ($success, $error_msg) = Cobrand::council_check($cobrand, { all_councils => $all_councils }, $q, 'submit_problem'); @@ -506,10 +522,10 @@ sub display_form { return front_page($q, $error_msg); } - # Ipswich & St Edmundsbury are responsible for everything in their areas, no Suffolk + # Ipswich & St Edmundsbury are responsible for everything in their areas, not Suffolk delete $all_councils->{2241} if $all_councils->{2446} || $all_councils->{2443}; - # Norwich is responsible for everything in its areas, no Norfolk + # Norwich is responsible for everything in its areas, not Norfolk delete $all_councils->{2233} if $all_councils->{2391}; return display_location($q, _('That spot does not appear to be covered by a council. @@ -589,9 +605,9 @@ EOF $type = 1; } $vars{form_start} = FixMyStreet::Map::display_map($q, - easting => $easting, northing => $northing, + latitude => $latitude, longitude => $longitude, type => $type, - pins => [ [ $easting, $northing, 'purple' ] ], + pins => [ [ $latitude, $longitude, 'purple' ] ], ); my $partial_id; if (my $token = $input{partial}) { @@ -685,8 +701,8 @@ photo of the problem if you have one), etc.')); } $vars{text_help} .= ' -<input type="hidden" name="easting" value="' . $easting . '"> -<input type="hidden" name="northing" value="' . $northing . '">'; +<input type="hidden" name="latitude" value="' . $latitude . '"> +<input type="hidden" name="longitude" value="' . $longitude . '">'; if (@errors) { $vars{errors} = '<ul class="error"><li>' . join('</li><li>', @errors) . '</li></ul>'; @@ -768,37 +784,64 @@ EOF sub display_location { my ($q, @errors) = @_; my $cobrand = Page::get_cobrand($q); - my @vars = qw(pc x y e n all_pins no_pins); - my %input = map { $_ => $q->param($_) || '' } @vars; - my %input_h = map { $_ => $q->param($_) ? ent($q->param($_)) : '' } @vars; + my @vars = qw(pc x y lat lon all_pins no_pins); - (my $easting) = $input{e} =~ /^(\d+)/; $easting ||= 0; - (my $northing) = $input{n} =~ /^(\d+)/; $northing ||= 0; + my %input = (); + my %input_h = (); + + foreach my $key (@vars) { + my $val = $q->param($key); + $input{$key} = defined($val) ? $val : ''; # '0' is valid for longitude + $input_h{$key} = ent( $input{$key} ); + } + + use Data::Dumper; + local $Data::Dumper::Sortkeys = 1; + warn Dumper( \%input ); + + my $latitude = $input{lat}; + my $longitude = $input{lon}; # X/Y referring to tiles old-school (my $x) = $input{x} =~ /^(\d+)/; $x ||= 0; (my $y) = $input{y} =~ /^(\d+)/; $y ||= 0; - return front_page($q, @errors) unless $x || $y || $input{pc} || $easting || $northing; - if ($x && $y) { + return front_page( $q, @errors ) + unless ( $x && $y ) + || $input{pc} + || ( defined $latitude && defined $longitude ); + + if ( $x && $y ) { + # Convert the tile co-ordinates to real ones. - $easting = FixMyStreet::Map::tile_to_os($x); - $northing = FixMyStreet::Map::tile_to_os($y); - } elsif ($easting && $northing) { + ( $latitude, $longitude ) = + FixMyStreet::Map::tile_xy_to_wgs84( $x, $y ); + } + elsif ( $latitude && $longitude ) { + # Don't need to do anything - } else { + } + else { my $error; try { - ($easting, $northing, $error) = FixMyStreet::Geocode::lookup($input{pc}, $q); - } catch Error::Simple with { + ( $latitude, $longitude, $error ) = + FixMyStreet::Geocode::lookup( $input{pc}, $q ); + + debug 'Looked up postcode "%s": lat: "%s", lon: "%s", error: "%s"', + $input{pc}, $latitude, $longitude, $error; + } + catch Error::Simple with { $error = shift; }; - return FixMyStreet::Geocode::list_choices($error, '/', $q) if (ref($error) eq 'ARRAY'); - return front_page($q, $error) if $error; + return FixMyStreet::Geocode::list_choices( $error, '/', $q ) + if ( ref($error) eq 'ARRAY' ); + return front_page( $q, $error ) if $error; } + + warn "lat: $latitude, lon: $longitude"; # Check this location is okay to be displayed for the cobrand - my ($success, $error_msg) = Cobrand::council_check($cobrand, { e => $easting, n => $northing }, $q, 'display_location'); + my ($success, $error_msg) = Cobrand::council_check($cobrand, { lat => $latitude, lon => $longitude }, $q, 'display_location'); return front_page($q, $error_msg) unless $success; # Deal with pin hiding/age @@ -812,10 +855,10 @@ sub display_location { $interval = '6 months'; } - my ($on_map_all, $on_map, $around_map, $dist) = FixMyStreet::Map::map_features_easting_northing($q, $easting, $northing, $interval); + my ($on_map_all, $on_map, $around_map, $dist) = FixMyStreet::Map::map_features($q, $latitude, $longitude, $interval); my @pins; foreach (@$on_map_all) { - push @pins, [ $_->{easting}, $_->{northing}, $_->{state} eq 'fixed' ? 'green' : 'red' ]; + push @pins, [ $_->{latitude}, $_->{longitude}, $_->{state} eq 'fixed' ? 'green' : 'red' ]; } my $on_list = ''; foreach (@$on_map) { @@ -841,7 +884,7 @@ sub display_location { $around_list .= $dist . 'km)</small>'; $around_list .= ' <small>' . _('(fixed)') . '</small>' if $_->{state} eq 'fixed'; $around_list .= '</li>'; - push @pins, [ $_->{easting}, $_->{northing}, $_->{state} eq 'fixed' ? 'green' : 'red' ]; + push @pins, [ $_->{latitude}, $_->{longitude}, $_->{state} eq 'fixed' ? 'green' : 'red' ]; } $around_list = $q->li(_('No problems found.')) unless $around_list; @@ -858,21 +901,21 @@ sub display_location { my $url_skip = NewURL($q, -retain=>1, pc => undef, x => undef, 'y' => undef, - easting => $easting, northing => $northing, + latitude => $latitude, longitude => $longitude, 'submit_map'=>1, skipped=>1 ); my $pc_h = ent($q->param('pc') || ''); my %vars = ( 'map' => FixMyStreet::Map::display_map($q, - easting => $easting, northing => $northing, + latitude => $latitude, longitude => $longitude, type => 1, pins => \@pins, post => $map_links ), map_end => FixMyStreet::Map::display_map_end(1), url_home => Cobrand::url($cobrand, '/', $q), - url_rss => Cobrand::url($cobrand, NewURL($q, -retain => 1, -url=> "/rss/n/$easting,$northing", pc => undef, x => undef, 'y' => undef), $q), - url_email => Cobrand::url($cobrand, NewURL($q, -retain => 1, pc => undef, -url=>'/alert', e=>$easting, 'n'=>$northing, feed=>"local:$easting:$northing"), $q), + url_rss => Cobrand::url($cobrand, NewURL($q, -retain => 1, -url=> "/rss/l/$latitude,$longitude", pc => undef, x => undef, 'y' => undef), $q), + url_email => Cobrand::url($cobrand, NewURL($q, -retain => 1, pc => undef, -url=>'/alert', lat=>$latitude, lon=>$longitude, feed=>"local:$latitude:$longitude"), $q), url_skip => $url_skip, email_me => _('Email me new local problems'), rss_alt => _('RSS feed'), @@ -892,7 +935,7 @@ sub display_location { ); my %params = ( - rss => [ _('Recent local problems, FixMyStreet'), "/rss/n/$easting,$northing" ], + rss => [ _('Recent local problems, FixMyStreet'), "/rss/l/$latitude,$longitude" ], robots => 'noindex,nofollow', ); @@ -955,9 +998,9 @@ sub display_problem { my %vars = ( banner => $banner, map_start => FixMyStreet::Map::display_map($q, - easting => $problem->{easting}, northing => $problem->{northing}, + latitude => $problem->{latitude}, longitude => $problem->{longitude}, type => 0, - pins => [ [ $problem->{easting}, $problem->{northing}, 'blue' ] ], + pins => [ [ $problem->{latitude}, $problem->{longitude}, 'blue' ] ], post => $map_links ), map_end => FixMyStreet::Map::display_map_end(0), diff --git a/web/questionnaire.cgi b/web/questionnaire.cgi index 33e823560..cb5e553d3 100755 --- a/web/questionnaire.cgi +++ b/web/questionnaire.cgi @@ -220,9 +220,9 @@ sub display_questionnaire { my %vars = ( input_h => \%input_h, map_start => FixMyStreet::Map::display_map($q, - easting => $problem->{easting}, northing => $problem->{northing}, + latitude => $problem->{latitude}, longitude => $problem->{longitude}, pins => [ - [ $problem->{easting}, $problem->{northing}, $problem->{state} eq 'fixed'?'green':'red' ], + [ $problem->{latitude}, $problem->{longitude}, $problem->{state} eq 'fixed'?'green':'red' ], ], pre => $problem_text, post => $updates ), |