Have FixMyStreet::Geocode only deal in real co-ordinates.

5 files changed, 35 insertions, 40 deletions

diff --git a/perllib/FixMyStreet/Geocode.pm b/perllib/FixMyStreet/Geocode.pm
index 04e440cc6..9e89b4f7b 100644
--- a/perllib/FixMyStreet/Geocode.pm
+++ b/perllib/FixMyStreet/Geocode.pm
@@ -36,7 +36,7 @@ BEGIN {
 # of the site to diambiguate locations.
 sub lookup {
     my ($s, $q) = @_;
-    my ($x, $y, $easting, $northing, $error);
+    my ($easting, $northing, $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)) {
@@ -44,22 +44,16 @@ sub lookup {
         unless ($error = Page::mapit_check_error($location)) {
             $easting = $location->{easting};
             $northing = $location->{northing};
-            my $xx = FixMyStreet::Map::os_to_tile($easting);
-            my $yy = FixMyStreet::Map::os_to_tile($northing);
-            $x = int($xx);
-            $y = int($yy);
-            $x += 1 if ($xx - $x > 0.5);
-            $y += 1 if ($yy - $y > 0.5);
         }
     } else {
-        ($x, $y, $easting, $northing, $error) = FixMyStreet::Geocode::string($s, $q);
+        ($easting, $northing, $error) = FixMyStreet::Geocode::string($s, $q);
     }
-    return ($x, $y, $easting, $northing, $error);
+    return ($easting, $northing, $error);
 }
 
 sub geocoded_string_coordinates {
     my ($js, $q) = @_;
-    my ($x, $y, $easting, $northing, $error);
+    my ($easting, $northing, $error);
     my ($accuracy) = $js =~ /"Accuracy" *: *(\d)/;
     if ($accuracy < 4) {  
         $error = _('Sorry, that location appears to be too general; please be more specific.');
@@ -69,19 +63,13 @@ sub geocoded_string_coordinates {
          my $lon = $1; my $lat = $2;
          try {
               ($easting, $northing) = mySociety::GeoUtil::wgs84_to_national_grid($lat, $lon, 'G');
-              my $xx = FixMyStreet::Map::os_to_tile($easting);
-              my $yy = FixMyStreet::Map::os_to_tile($northing);
-              $x = int($xx);
-              $y = int($yy);
-              $x += 1 if ($xx - $x > 0.5);
-              $y += 1 if ($yy - $y > 0.5);
           } 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 ($x, $y, $easting, $northing, $error);
+    return ($easting, $northing, $error);
 }
 
 # string STRING QUERY
@@ -101,7 +89,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, $x, $y, $easting, $northing);
+    my ($js, $error, $easting, $northing);
     if (-s $cache_file) {
         $js = File::Slurp::read_file($cache_file);
     } else {
@@ -133,9 +121,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 {
-        ($x, $y, $easting, $northing, $error) = geocoded_string_coordinates($js, $q);
+        ($easting, $northing, $error) = geocoded_string_coordinates($js, $q);
     }
-    return ($x, $y, $easting, $northing, $error);
+    return ($easting, $northing, $error);
 }
 
 # list_choices
diff --git a/t/Page.t b/t/Page.t
index 831986f80..30925ff1b 100755
--- a/t/Page.t
+++ b/t/Page.t
@@ -41,14 +41,12 @@ sub test_geocode_string() {
     my $q = new MockQuery('nosite', \%params);
     
     # geocode a straightforward string, expect success 
-    my ($x, $y, $easting, $northing, $error) = FixMyStreet::Geocode::string('Buckingham Palace', $q);
-    ok($x == 3280, 'example x coordinate generated') or diag("Got $x");
-    ok($y == 1114, 'example y coordinate generated') or diag("Got $y");;
+    my ($easting, $northing, $error) = FixMyStreet::Geocode::string('Buckingham Palace', $q);
     ok($easting == 529044, 'example easting generated') or diag("Got $easting");
     ok($northing == 179619, 'example northing generated') or diag("Got $northing");
     ok(! defined($error), 'should not generate error for simple example') or diag("Got $error");
     # expect a failure message for Northern Ireland
-    ($x, $y, $easting, $northing, $error) = FixMyStreet::Geocode::string('Falls Road, Belfast', $q);
+    ($easting, $northing, $error) = FixMyStreet::Geocode::string('Falls Road, Belfast', $q);
     ok($error eq "We do not cover Northern Ireland, I'm afraid, as our licence doesn't include any maps for the region.", 'error message produced for NI location') or diag("Got $error");
 
 }
diff --git a/web/alert.cgi b/web/alert.cgi
index 70b9d1873..3dc52c677 100755
--- a/web/alert.cgi
+++ b/web/alert.cgi
@@ -72,20 +72,17 @@ Page::do_fastcgi(\&main);
 
 sub alert_list {
     my ($q, @errors) = @_;
-    my @vars = qw(pc rznvy x y);
+    my @vars = qw(pc rznvy e n);
     my %input = map { $_ => scalar $q->param($_) } @vars;
     my %input_h = map { $_ => $q->param($_) ? ent($q->param($_)) : '' } @vars;
 
     my($error, $e, $n);
-    my $x = $input{x}; my $y = $input{y};
-    $x ||= 0; $x += 0;
-    $y ||= 0; $y += 0;
-    if ($x || $y) {
-        $e = FixMyStreet::Map::tile_to_os($input{x});
-        $n = FixMyStreet::Map::tile_to_os($input{y});
+    if ($input{e} || $input{n}) {
+        $e = $input{e};
+        $n = $input{n};
     } else {
         try {
-            ($x, $y, $e, $n, $error) = FixMyStreet::Geocode::lookup($input{pc}, $q);
+            ($e, $n, $error) = FixMyStreet::Geocode::lookup($input{pc}, $q);
         } catch Error::Simple with {
             $error = shift;
         };
@@ -274,8 +271,8 @@ EOF
                 rss_feed_5k => $rss_feed_5k,   
                 rss_feed_10k => $rss_feed_10k,   
                 rss_feed_20k => $rss_feed_20k, 
-                x => $x, 
-                y => $y, 
+                e => $e, 
+                n => $n, 
                 options => $options );
     my $cobrand_page = Page::template_include('alert-options', $q, Page::template_root($q), %vars);
     $out = $cobrand_page if ($cobrand_page);
diff --git a/web/index.cgi b/web/index.cgi
index 4c1366cbb..63b1e938b 100755
--- a/web/index.cgi
+++ b/web/index.cgi
@@ -470,7 +470,7 @@ sub display_form {
             $easting = FixMyStreet::Map::tile_to_os($input{x});
             $northing = FixMyStreet::Map::tile_to_os($input{y});
         } else {
-            my ($x, $y, $e, $n, $error) = FixMyStreet::Geocode::lookup($input{pc}, $q);
+            my ($e, $n, $error) = FixMyStreet::Geocode::lookup($input{pc}, $q);
             $easting = $e; $northing = $n;
         }
     } elsif ($pin_x && $pin_y) {
@@ -484,9 +484,9 @@ sub display_form {
         $easting = FixMyStreet::Map::tile_to_os($pin_x);
         $northing = FixMyStreet::Map::tile_to_os($pin_y);
     } elsif ($input{partial} && $input{pc} && !$input{easting} && !$input{northing}) {
-        my ($x, $y, $error);
+        my $error;
         try {
-            ($x, $y, $easting, $northing, $error) = FixMyStreet::Geocode::lookup($input{pc}, $q);
+            ($easting, $northing, $error) = FixMyStreet::Geocode::lookup($input{pc}, $q);
         } catch Error::Simple with {
             $error = shift;
         };
@@ -795,7 +795,13 @@ sub display_location {
     return front_page($q, @errors) unless $x || $y || $input{pc};
     if (!$x && !$y) {
         try {
-            ($x, $y, $easting, $northing, $error) = FixMyStreet::Geocode::lookup($input{pc}, $q);
+            ($easting, $northing, $error) = FixMyStreet::Geocode::lookup($input{pc}, $q);
+            my $xx = FixMyStreet::Map::os_to_tile($easting);
+            my $yy = FixMyStreet::Map::os_to_tile($northing);
+            $x = int($xx);
+            $y = int($yy);
+            $x += 1 if ($xx - $x > 0.5);
+            $y += 1 if ($yy - $y > 0.5);
         } catch Error::Simple with {
             $error = shift;
         };
@@ -866,7 +872,7 @@ sub display_location {
         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', x=>$x, 'y'=>$y, feed=>"local:$easting:$northing"), $q),
+        url_email => Cobrand::url($cobrand, NewURL($q, -retain => 1, pc => undef, -url=>'/alert', e=>$easting, 'n'=>$northing, feed=>"local:$easting:$northing"), $q),
         url_skip => $url_skip,
         email_me => _('Email me new local problems'),
         rss_alt => _('RSS feed'),
diff --git a/web/rss.cgi b/web/rss.cgi
index 623314e7f..867f050fd 100755
--- a/web/rss.cgi
+++ b/web/rss.cgi
@@ -94,7 +94,13 @@ sub rss_local_problems {
     } elsif ($pc) {
         my $error;
         try {
-            ($x, $y, $e, $n, $error) = FixMyStreet::Geocode::lookup($pc, $q);
+            ($e, $n, $error) = FixMyStreet::Geocode::lookup($pc, $q);
+            my $xx = FixMyStreet::Map::os_to_tile($e);¬
+            my $yy = FixMyStreet::Map::os_to_tile($n);¬
+            $x = int($xx);¬
+            $y = int($yy);¬
+            $x += 1 if ($xx - $x > 0.5);¬
+            $y += 1 if ($yy - $y > 0.5);¬
         } catch Error::Simple with {
             $error = shift;
         };