Tidy up find_closest* functions.

Allow find_closest to be called multiple times with only one lookup,
and to return just its data, not a compiled string.

10 files changed, 50 insertions, 49 deletions

diff --git a/perllib/FixMyStreet/App/Controller/Admin/ExorDefects.pm b/perllib/FixMyStreet/App/Controller/Admin/ExorDefects.pm
index 201742c81..fd4b2be3c 100644
--- a/perllib/FixMyStreet/App/Controller/Admin/ExorDefects.pm
+++ b/perllib/FixMyStreet/App/Controller/Admin/ExorDefects.pm
@@ -140,6 +140,7 @@ sub download : Path('download') : Args(0) {
 
         foreach my $report (@$inspections) {
             my ($eastings, $northings) = $report->local_coords;
+            my $location = "${eastings}E ${northings}N";
             my $description = sprintf("%s %s", $report->external_id || "", $report->get_extra_metadata('detailed_information') || "");
             my $activity_code = $report->defect_type ?
                 $report->defect_type->get_extra_metadata('activity_code')
@@ -153,7 +154,7 @@ sub download : Path('download') : Args(0) {
                 $activity_code, # activity code - minor carriageway, also FC (footway)
                 "", # empty field, can also be A (seen on MC) or B (seen on FC)
                 sprintf("%03d", ++$i), # randomised sequence number
-                "${eastings}E ${northings}N", # defect location field, which we don't capture from inspectors
+                $location, # defect location field, which we don't capture from inspectors
                 $report->inspection_log_entry->whenedited->strftime("%H%M"), # defect time raised
                 "","","","","","","", # empty fields
                 $traffic_information,
@@ -227,4 +228,4 @@ sub download : Path('download') : Args(0) {
     $c->res->body( join "", map { "\"$_\"\r\n" } @body );
 }
 
-1;
\ No newline at end of file
+1;
diff --git a/perllib/FixMyStreet/App/Controller/Rss.pm b/perllib/FixMyStreet/App/Controller/Rss.pm
index 28f1aba43..0e299d2c0 100755
--- a/perllib/FixMyStreet/App/Controller/Rss.pm
+++ b/perllib/FixMyStreet/App/Controller/Rss.pm
@@ -289,7 +289,7 @@ sub add_row : Private {
     }
 
     if ( $row->{used_map} ) {
-        my $address = $c->cobrand->find_closest_address_for_rss( $row->{latitude}, $row->{longitude}, $row );
+        my $address = $c->cobrand->find_closest_address_for_rss($row);
         $item{description} .= ent("\n<br>$address") if $address;
     }
 
diff --git a/perllib/FixMyStreet/Cobrand/Default.pm b/perllib/FixMyStreet/Cobrand/Default.pm
index ac70fff08..ad84bd496 100644
--- a/perllib/FixMyStreet/Cobrand/Default.pm
+++ b/perllib/FixMyStreet/Cobrand/Default.pm
@@ -509,27 +509,35 @@ sub geocoded_string_check { return 1; }
 
 =head2 find_closest
 
-Used by send-reports to attach nearest things to the bottom of the report
+Used by send-reports and similar to attach nearest things to the bottom of the
+report.
 
 =cut
 
 sub find_closest {
-    my ( $self, $latitude, $longitude, $problem ) = @_;
-    my $str = '';
+    my ( $self, $problem, $as_data ) = @_;
 
-    if ( my $j = FixMyStreet::Geocode::Bing::reverse( $latitude, $longitude, disambiguate_location()->{bing_culture} ) ) {
+    my $j = $problem->geocode;
+    if (!$j) {
+        $j = FixMyStreet::Geocode::Bing::reverse( $problem->latitude, $problem->longitude,
+            disambiguate_location()->{bing_culture} );
         # cache the bing results for use in alerts
-        if ( $problem ) {
-            $problem->geocode( $j );
-            $problem->update;
-        }
-        if ($j->{resourceSets}[0]{resources}[0]{name}) {
-            $str .= sprintf(_("Nearest road to the pin placed on the map (automatically generated by Bing Maps): %s"),
-                $j->{resourceSets}[0]{resources}[0]{name}) . "\n\n";
+        $problem->geocode( $j );
+        $problem->update;
+    }
+
+    my $data = $as_data ? {} : '';
+    if ($j && $j->{resourceSets}[0]{resources}[0]{name}) {
+        my $str = $j->{resourceSets}[0]{resources}[0]{name};
+        if ($as_data) {
+            $data->{road} = $str;
+        } else {
+            $data .= sprintf(_("Nearest road to the pin placed on the map (automatically generated by Bing Maps): %s"),
+                $str) . "\n\n";
         }
     }
 
-    return $str;
+    return $data;
 }
 
 =head2 find_closest_address_for_rss
@@ -539,26 +547,14 @@ Used by rss feeds to provide a bit more context
 =cut
 
 sub find_closest_address_for_rss {
-    my ( $self, $latitude, $longitude, $problem ) = @_;
-    my $str = '';
+    my ( $self, $problem ) = @_;
 
-    my $j;
-    if ( $problem && ref($problem) =~ /FixMyStreet/ && $problem->can( 'geocode' ) ) {
-       $j = $problem->geocode;
-    } else {
+    if (ref($problem) eq 'HASH') {
         $problem = FixMyStreet::App->model('DB::Problem')->find( { id => $problem->{id} } );
-        $j = $problem->geocode;
     }
+    my $j = $problem->geocode;
 
-    # if we've not cached it then we don't want to look it up in order to avoid
-    # hammering the bing api
-    # if ( !$j ) {
-    #     $j = FixMyStreet::Geocode::Bing::reverse( $latitude, $longitude, disambiguate_location()->{bing_culture}, 1 );
-
-    #     $problem->geocode( $j );
-    #     $problem->update;
-    # }
-
+    my $str = '';
     if ($j && $j->{resourceSets}[0]{resources}[0]{name}) {
         my $address = $j->{resourceSets}[0]{resources}[0]{address};
         my @address;
diff --git a/perllib/FixMyStreet/Cobrand/FiksGataMi.pm b/perllib/FixMyStreet/Cobrand/FiksGataMi.pm
index cf0d72f8e..e49d5bd47 100644
--- a/perllib/FixMyStreet/Cobrand/FiksGataMi.pm
+++ b/perllib/FixMyStreet/Cobrand/FiksGataMi.pm
@@ -61,8 +61,8 @@ sub geocoded_string_check {
 }
 
 sub find_closest {
-    my ( $self, $latitude, $longitude ) = @_;
-    return FixMyStreet::Geocode::OSM::closest_road_text( $self, $latitude, $longitude );
+    my ( $self, $problem ) = @_;
+    return FixMyStreet::Geocode::OSM::closest_road_text( $self, $problem->latitude, $problem->longitude );
 }
 
 # Used by send-reports, calling find_closest, calling OSM geocoding
diff --git a/perllib/FixMyStreet/Cobrand/FixaMinGata.pm b/perllib/FixMyStreet/Cobrand/FixaMinGata.pm
index 324811008..07a4ef920 100644
--- a/perllib/FixMyStreet/Cobrand/FixaMinGata.pm
+++ b/perllib/FixMyStreet/Cobrand/FixaMinGata.pm
@@ -67,8 +67,8 @@ sub geocoded_string_check {
 }
 
 sub find_closest {
-    my ( $self, $latitude, $longitude ) = @_;
-    return FixMyStreet::Geocode::OSM::closest_road_text( $self, $latitude, $longitude );
+    my ( $self, $problem ) = @_;
+    return FixMyStreet::Geocode::OSM::closest_road_text( $self, $problem->latitude, $problem->longitude );
 }
 
 # Used by send-reports, calling find_closest, calling OSM geocoding
diff --git a/perllib/FixMyStreet/Cobrand/UK.pm b/perllib/FixMyStreet/Cobrand/UK.pm
index 945af48f8..2d4a4d891 100644
--- a/perllib/FixMyStreet/Cobrand/UK.pm
+++ b/perllib/FixMyStreet/Cobrand/UK.pm
@@ -117,21 +117,25 @@ sub short_name {
 }
 
 sub find_closest {
-    my ( $self, $latitude, $longitude, $problem ) = @_;
+    my ( $self, $problem, $as_data ) = @_;
 
-    my $str = $self->SUPER::find_closest( $latitude, $longitude, $problem );
+    my $data = $self->SUPER::find_closest($problem, $as_data);
 
-    my $url = "http://mapit.mysociety.org/nearest/4326/$longitude,$latitude";
+    my $url = "https://mapit.mysociety.org/nearest/4326/" . $problem->longitude . ',' . $problem->latitude;
     my $j = LWP::Simple::get($url);
     if ($j) {
         $j = JSON->new->utf8->allow_nonref->decode($j);
         if ($j->{postcode}) {
-            $str .= sprintf(_("Nearest postcode to the pin placed on the map (automatically generated): %s (%sm away)"),
-                $j->{postcode}{postcode}, $j->{postcode}{distance}) . "\n\n";
+            if ($as_data) {
+                $data->{postcode} = $j->{postcode};
+            } else {
+                $data .= sprintf(_("Nearest postcode to the pin placed on the map (automatically generated): %s (%sm away)"),
+                    $j->{postcode}{postcode}, $j->{postcode}{distance}) . "\n\n";
+            }
         }
     }
 
-    return $str;
+    return $data;
 }
 
 sub reports_body_check {
diff --git a/perllib/FixMyStreet/Cobrand/Zurich.pm b/perllib/FixMyStreet/Cobrand/Zurich.pm
index dca722224..776350b25 100644
--- a/perllib/FixMyStreet/Cobrand/Zurich.pm
+++ b/perllib/FixMyStreet/Cobrand/Zurich.pm
@@ -67,7 +67,7 @@ sub pin_colour {
 
 # This isn't used
 sub find_closest {
-    my ( $self, $latitude, $longitude, $problem ) = @_;
+    my ( $self, $problem ) = @_;
     return '';
 }
 
diff --git a/perllib/FixMyStreet/Script/Alerts.pm b/perllib/FixMyStreet/Script/Alerts.pm
index 1a760a0c1..56550563e 100644
--- a/perllib/FixMyStreet/Script/Alerts.pm
+++ b/perllib/FixMyStreet/Script/Alerts.pm
@@ -143,7 +143,7 @@ sub send() {
             # this is ward and council problems
             } else {
                 if ( exists $row->{geocode} && $row->{geocode} && $ref =~ /ward|council/ ) {
-                    my $nearest_st = _get_address_from_gecode( $row->{geocode} );
+                    my $nearest_st = _get_address_from_geocode( $row->{geocode} );
                     $row->{nearest} = $nearest_st;
                 }
 
@@ -236,7 +236,7 @@ sub send() {
                 parameter => $row->{id},
             } );
             if ( exists $row->{geocode} && $row->{geocode} ) {
-                my $nearest_st = _get_address_from_gecode( $row->{geocode} );
+                my $nearest_st = _get_address_from_geocode( $row->{geocode} );
                 $row->{nearest} = $nearest_st;
             }
             my $dt = $parser->parse_timestamp( $row->{confirmed} );
@@ -304,7 +304,7 @@ sub _send_aggregated_alert_email(%) {
     }
 }
 
-sub _get_address_from_gecode {
+sub _get_address_from_geocode {
     my $geocode = shift;
 
     return '' unless defined $geocode;
diff --git a/perllib/FixMyStreet/Script/Reports.pm b/perllib/FixMyStreet/Script/Reports.pm
index 6057807de..1e5fd55bb 100644
--- a/perllib/FixMyStreet/Script/Reports.pm
+++ b/perllib/FixMyStreet/Script/Reports.pm
@@ -98,7 +98,7 @@ sub send(;$) {
 
         $h{osm_url} = Utils::OpenStreetMap::short_url($h{latitude}, $h{longitude});
         if ( $row->used_map ) {
-            $h{closest_address} = $cobrand->find_closest( $h{latitude}, $h{longitude}, $row );
+            $h{closest_address} = $cobrand->find_closest($row);
             $h{osm_url} .= '?m';
         }
 
diff --git a/t/cobrand/closest.t b/t/cobrand/closest.t
index 43b36f608..0d6a772ba 100644
--- a/t/cobrand/closest.t
+++ b/t/cobrand/closest.t
@@ -59,7 +59,7 @@ $report->geocode( undef );
 
 ok !$report->geocode, 'no geocode entry for report';
 
-my $near = $c->find_closest( $report->latitude, $report->longitude, $report );
+my $near = $c->find_closest($report);
 
 SKIP: {
     if (!FixMyStreet->config('BING_MAPS_API_KEY')) {
@@ -72,13 +72,13 @@ SKIP: {
     like $near, qr/Constitution Hill/i, 'nearest street looks right';
     like $near, qr/Nearest postcode .*: SW1A 1AA/i, 'nearest postcode looks right';
 
-    $near = $c->find_closest_address_for_rss( $report->latitude, $report->longitude, $report );
+    $near = $c->find_closest_address_for_rss($report);
 
     like $near, qr/Constitution Hill/i, 'nearest street for RSS looks right';
     unlike $near, qr/Nearest postcode/i, 'no nearest postcode in RSS text';
 
     $report->geocode( undef );
-    $near = $c->find_closest_address_for_rss( $report->latitude, $report->longitude, $report );
+    $near = $c->find_closest_address_for_rss($report);
 
     ok !$near, 'no closest address for RSS if not cached';
 }