Refactor nearby() to use named parameters not list

6 files changed, 32 insertions, 34 deletions

diff --git a/perllib/FixMyStreet/App/Controller/Around.pm b/perllib/FixMyStreet/App/Controller/Around.pm
index ec8df4450..4c68ce414 100644
--- a/perllib/FixMyStreet/App/Controller/Around.pm
+++ b/perllib/FixMyStreet/App/Controller/Around.pm
@@ -269,7 +269,7 @@ sub map_features : Private {
     # Allow the cobrand to add in any additional query parameters
     my $extra_params = $c->cobrand->call_hook('display_location_extra_params');
 
-    my ( $on_map, $nearby, $distance ) =
+    my ( $on_map, $nearby ) =
       FixMyStreet::Map::map_features(
         $c, %$extra,
         categories => [ keys %{$c->stash->{filter_category}} ],
@@ -290,7 +290,6 @@ sub map_features : Private {
     $c->stash->{pins} = \@pins;
     $c->stash->{on_map} = $on_map;
     $c->stash->{around_map} = $nearby;
-    $c->stash->{distance} = $distance;
 }
 
 =head2 ajax
diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm
index 12f6ec1d1..796c2a42b 100644
--- a/perllib/FixMyStreet/App/Controller/Report.pm
+++ b/perllib/FixMyStreet/App/Controller/Report.pm
@@ -600,16 +600,22 @@ sub nearby_json :PathPart('nearby.json') :Chained('id') :Args(0) {
     my ($self, $c) = @_;
 
     my $p = $c->stash->{problem};
-    my $dist = 1;
+    my $params = {
+        latitude => $p->latitude,
+        longitude => $p->longitude,
+        categories => [ $p->category ],
+        ids => [ $p->id ],
+        distance => 1,
+    };
 
     # This is for the list template, this is a list on that page.
     $c->stash->{page} = 'report';
 
-    my $extra_params = $c->cobrand->call_hook('display_location_extra_params');
+    $params->{extra} = $c->cobrand->call_hook('display_location_extra_params');
+    $params->{limit} = 5;
+
+    my $nearby = $c->model('DB::Nearby')->nearby($c, %$params);
 
-    my $nearby = $c->model('DB::Nearby')->nearby(
-        $c, $dist, [ $p->id ], 5, $p->latitude, $p->longitude, [ $p->category ], undef, $extra_params
-    );
     # Want to treat these as if they were on map
     $nearby = [ map { $_->problem } @$nearby ];
     my @pins = map {
diff --git a/perllib/FixMyStreet/DB/ResultSet/Nearby.pm b/perllib/FixMyStreet/DB/ResultSet/Nearby.pm
index 3d8f87b9f..2ebe309e3 100644
--- a/perllib/FixMyStreet/DB/ResultSet/Nearby.pm
+++ b/perllib/FixMyStreet/DB/ResultSet/Nearby.pm
@@ -10,34 +10,34 @@ sub to_body {
 }
 
 sub nearby {
-    my ( $rs, $c, $dist, $ids, $limit, $mid_lat, $mid_lon, $categories, $states, $extra_params, $report_age ) = @_;
+    my ( $rs, $c, %args ) = @_;
 
-    unless ( $states ) {
-        $states = FixMyStreet::DB::Result::Problem->visible_states();
+    unless ( $args{states} ) {
+        $args{states} = FixMyStreet::DB::Result::Problem->visible_states();
     }
 
     my $params = {
-        state => [ keys %$states ],
+        state => [ keys %{$args{states}} ],
     };
-    $params->{id} = { -not_in => $ids }
-        if $ids;
-    $params->{category} = $categories if $categories && @$categories;
+    $params->{id} = { -not_in => $args{ids} }
+        if $args{ids};
+    $params->{category} = $args{categories} if $args{categories} && @{$args{categories}};
 
-    $params->{$c->stash->{report_age_field}} = { '>=', \"current_timestamp-'$report_age'::interval" }
-        if $report_age;
+    $params->{$c->stash->{report_age_field}} = { '>=', \"current_timestamp-'$args{report_age}'::interval" }
+        if $args{report_age};
 
     FixMyStreet::DB::ResultSet::Problem->non_public_if_possible($params, $c);
 
     $rs = $c->cobrand->problems_restriction($rs);
 
     # Add in any optional extra query parameters
-    $params = { %$params, %$extra_params } if $extra_params;
+    $params = { %$params, %{$args{extra}} } if $args{extra};
 
     my $attrs = {
         prefetch => 'problem',
-        bind => [ $mid_lat, $mid_lon, $dist ],
+        bind => [ $args{latitude}, $args{longitude}, $args{distance} ],
         order_by => [ 'distance', { -desc => 'created' } ],
-        rows => $limit,
+        rows => $args{limit},
     };
 
     my @problems = mySociety::Locale::in_gb_locale { $rs->search( $params, $attrs )->all };
diff --git a/perllib/FixMyStreet/Map.pm b/perllib/FixMyStreet/Map.pm
index 8ed0c4b37..8b8cfe82c 100644
--- a/perllib/FixMyStreet/Map.pm
+++ b/perllib/FixMyStreet/Map.pm
@@ -104,26 +104,21 @@ sub map_features {
         # if show_old_reports is on then there must be old reports
         $c->stash->{num_old_reports} = 1;
     } else {
-        $p{report_age} = undef;
-        $p{page} = 1;
-        my $older = $c->cobrand->problems_on_map->around_map( $c, %p );
+        my $older = $c->cobrand->problems_on_map->around_map( $c, %p, report_age => undef, page => 1 );
         $c->stash->{num_old_reports} = $older->pager->total_entries - $pager->total_entries;
     }
 
-    my $dist = FixMyStreet::Gaze::get_radius_containing_population( $p{latitude}, $p{longitude} );
-
     # if there are fewer entries than our paging limit on the map then
     # also return nearby entries for display
     my $nearby;
     if (@$on_map < $pager->entries_per_page && $pager->current_page == 1) {
-        my $limit = 20;
-        my @ids = map { $_->id } @$on_map;
-        $nearby = $c->model('DB::Nearby')->nearby(
-            $c, $dist, \@ids, $limit, @p{"latitude", "longitude", "categories", "states", "extra"}, $report_age
-        );
+        $p{limit} = 20;
+        $p{ids} = [ map { $_->id } @$on_map ];
+        $p{distance} = FixMyStreet::Gaze::get_radius_containing_population( $p{latitude}, $p{longitude} );
+        $nearby = $c->model('DB::Nearby')->nearby($c, %p);
     }
 
-    return ( $on_map, $nearby, $dist );
+    return ( $on_map, $nearby );
 }
 
 sub click_to_wgs84 {
diff --git a/t/cobrand/restriction.t b/t/cobrand/restriction.t
index 9e3018625..63fe326b1 100644
--- a/t/cobrand/restriction.t
+++ b/t/cobrand/restriction.t
@@ -47,7 +47,7 @@ is($c->model('DB::Problem')->count, 4, 'Four reports in database');
 is($cobrand->problems->count, 2, 'Two reports in the right cobrand');
 is($cobrand->updates->count, 1, 'One update in the right cobrand');
 
-my $nearby = $c->model('DB::Nearby')->nearby($c, 5, [], 10, 0.003, 0.004);
+my $nearby = $c->model('DB::Nearby')->nearby($c, distance => 5, ids => [], limit => 10, latitude => 0.003, longitude => 0.004);
 is(@$nearby, 1, 'One report close to the origin point');
 
 done_testing();
diff --git a/t/map/tilma/original.t b/t/map/tilma/original.t
index 42cbbd9f2..9a404a2c9 100644
--- a/t/map/tilma/original.t
+++ b/t/map/tilma/original.t
@@ -93,12 +93,10 @@ for my $test (
         $report->update;
         $c->stash->{report_age_field} = 'lastupdate';
 
-        my ( $on_map, $nearby, $dist ) =
-            FixMyStreet::Map::map_features($c, bbox => "0,0,0,0");
+        my ($on_map, $nearby) = FixMyStreet::Map::map_features($c, bbox => "0,0,0,0");
 
         ok $on_map;
         ok $nearby;
-        ok $dist;
 
         my $id = $report->id;
         my $colour = $test->{colour};