Merge remote-tracking branch 'origin/improve-all-reports-in-places'

perllib/FixMyStreet/App/Controller/Reports.pm

@@ -229,7 +229,7 @@ sub setup_map :Private {
 
     my $pins = $c->stash->{pins} || [];
 
-    my $areas = [ $c->stash->{wards} ? map { $_->{id} } @{$c->stash->{wards}} : keys %{$c->stash->{body}->areas} ];
+    my $areas = [ $c->stash->{wards} ? map { $_->{id} } @{$c->stash->{wards}} : $c->stash->{body}->areas_practical ];
     $c->cobrand->call_hook(munge_reports_area_list => $areas);
     my %map_params = (
         latitude  => @$pins ? $pins->[0]{latitude} : 0,
@@ -429,16 +429,16 @@ sub ward_check : Private {
         s{_}{/}g;
     }
     # Could be from RSS area, or body...
-    my $parent_id;
+
+    my $qw;
     if ( $c->stash->{body} ) {
-        $parent_id = $c->stash->{body}->body_areas->first;
-        $c->detach( 'redirect_body' ) unless $parent_id;
-        $parent_id = $parent_id->area_id;
+        $qw = $c->stash->{body}->area_children;
+        $c->detach( 'redirect_body' ) unless $qw;
     } else {
-        $parent_id = $c->stash->{area}->{id};
+        my $parent_id = $c->stash->{area}->{id};
+        $qw = $c->cobrand->fetch_area_children($parent_id);
     }
 
-    my $qw = $c->cobrand->fetch_area_children($parent_id);
     $c->cobrand->call_hook('add_parish_wards' => $qw);
 
     $qw = [values %$qw];

perllib/FixMyStreet/Cobrand/Default.pm

@@ -826,7 +826,7 @@ cobrand's area_types_children type.
 =cut
 
 sub fetch_area_children {
-    my ($self, $area_ids, $all_generations) = @_;
+    my ($self, $area_ids, $body, $all_generations) = @_;
 
     $area_ids = [ $area_ids ] unless ref $area_ids eq 'ARRAY';
 

perllib/FixMyStreet/Cobrand/FixMyStreet.pm

@@ -3,6 +3,7 @@ use base 'FixMyStreet::Cobrand::UK';
 
 use Moo;
 use JSON::MaybeXS;
+use List::Util qw(any);
 with 'FixMyStreet::Roles::BoroughEmails';
 
 use constant COUNCIL_ID_BROMLEY => 2482;
@@ -607,4 +608,30 @@ sub add_extra_area_types {
     return \@types;
 }
 
+=head2 fetch_area_children
+
+If we are looking at the All Reports page for one of the extra London (TfL)
+bodies (the bike providers), we want the children to be the London councils,
+not all the wards of London.
+
+=cut
+
+sub fetch_area_children {
+    my $self = shift;
+    my ($area_ids, $body, $all_generations) = @_;
+
+    my $features = FixMyStreet->config('COBRAND_FEATURES') || {};
+    my $bodies = $features->{categories_restriction_bodies} || {};
+    $bodies = $bodies->{tfl} || [];
+    if ($body && any { $_ eq $body->name } @$bodies) {
+        my $areas = FixMyStreet::MapIt::call('areas', 'LBO');
+        foreach (keys %$areas) {
+            $areas->{$_}->{name} =~ s/\s*(Borough|City|District|County) Council$//;
+        }
+        return $areas;
+    } else {
+        return $self->next::method(@_);
+    }
+}
+
 1;

perllib/FixMyStreet/Cobrand/UK.pm

@@ -195,6 +195,22 @@ sub reports_body_check {
     return;
 }
 
+sub munge_body_areas_practical {
+    my ($self, $body, $area_ids) = @_;
+
+    if (@$area_ids == 4 && $area_ids->[1] == 2488) { # Brent
+        @$area_ids = (2488);
+    } elsif ($area_ids->[0] == 2487 && $area_ids->[1] == 2488) { # Harrow
+        @$area_ids = (2487);
+    } elsif ($area_ids->[0] == 2488 && $area_ids->[1] == 2489) { # Barnet
+        @$area_ids = (2489);
+    } elsif ($area_ids->[0] == 2488 && $area_ids->[1] == 2505) { # Camden
+        @$area_ids = (2505);
+    } elsif (@$area_ids == 3 && $area_ids->[0] == 2561) { # Bristol
+        @$area_ids = (2561);
+    }
+}
+
 sub council_rss_alert_options {
     my $self = shift;
     my $all_areas = shift;

perllib/FixMyStreet/DB/Result/Body.pm

@@ -183,15 +183,36 @@ sub areas {
     return \%ids;
 }
 
+=head2 areas_practical
+
+Certain bodies will have associated areas for reasons such as out-of-area
+handling (edge cases, parks); whilst areas will return all areas in the
+database for a body, areas_practical should only return the areas you'd
+expect for the body.
+
+=cut
+
+sub areas_practical {
+    my $self = shift;
+    my @area_ids = sort keys %{$self->areas};
+
+    my $cobrand = $self->get_cobrand_handler;
+    $cobrand ||= $self->result_source->schema->cobrand;
+    $cobrand->call_hook(munge_body_areas_practical => $self, \@area_ids);
+
+    return @area_ids;
+}
+
 sub area_children {
     my ( $self, $all_generations ) = @_;
 
-    my @body_area_ids = map { $_->area_id } $self->body_areas->all;
+    my @body_area_ids = $self->areas_practical;
     return unless @body_area_ids;
 
-    my $cobrand = $self->result_source->schema->cobrand;
+    my $cobrand = $self->get_cobrand_handler;
+    $cobrand ||= $self->result_source->schema->cobrand;
 
-    return $cobrand->fetch_area_children(\@body_area_ids, $all_generations);
+    return $cobrand->fetch_area_children(\@body_area_ids, $self, $all_generations);
 }
 
 =head2 get_cobrand_handler

t/Mock/MapIt.pm

@@ -232,6 +232,30 @@ sub dispatch_request {
         if ($area eq '164186') {
             return $self->output({53296 => {parent_area => 164186, id => 53296, name => "Sulgrave", type => "CPC"}});
         }
+        if ($area eq '2457') { # Epsom Ewell
+            return $self->output({167337 => {parent_area => 2457, id => 167337, name => "Auriol", type => "DIW"}});
+        }
+        if ($area eq '2487') { # Harrow
+            return $self->output({165128 => {parent_area => 2487, id => 165128, name => "Belmont", type => "LBW"}});
+        }
+        if ($area eq '2488') { # Brent
+            return $self->output({165138 => {parent_area => 2488, id => 165138, name => "Alperton", type => "LBW"}});
+        }
+        if ($area eq '2489') { # Barnet
+            return $self->output({165692 => {parent_area => 2489, id => 165692, name => "Barnet Vale", type => "LBW"}});
+        }
+        if ($area eq '2505') { # Camden
+            return $self->output({165469 => {parent_area => 2505, id => 165469, name => "Belsize", type => "LBW"}});
+        }
+        if ($area eq '2561') { # Bristol
+            return $self->output({148666 => {parent_area => 2561, id => 148666, name => "Ashley", type => "UTW"}});
+        }
+        if ($area eq '2608') { # South Gloucestershire
+            return $self->output({153253 => {parent_area => 2608, id => 153253, name => "Bitton & Oldland Common", type => "UTW"}});
+        }
+        if ($area eq '2642') { # North Somerset
+            return $self->output({145739 => {parent_area => 2642, id => 145739, name => "Backwell", type => "UTW"}});
+        }
         my $response = {
             "60705" => { "parent_area" => 2245, "generation_high" => 25, "all_names" => { }, "id" => 60705, "codes" => { "ons" => "00HY226", "gss" => "E04011842", "unit_id" => "17101" }, "name" => "Trowbridge", "country" => "E", "type_name" => "Civil parish/community", "generation_low" => 12, "country_name" => "England", "type" => "CPC" },
             "62883" => { "parent_area" => 2245, "generation_high" => 25, "all_names" => { }, "id" => 62883, "codes" => { "ons" => "00HY026", "gss" => "E04011642", "unit_id" => "17205" }, "name" => "Bradford-on-Avon", "country" => "E", "type_name" => "Civil parish/community", "generation_low" => 12, "country_name" => "England", "type" => "CPC" },

t/cobrand/brent.t

@@ -95,10 +95,9 @@ my $brent = $mech->create_body_ok(2488, 'Brent Council', {
 });
 my $atak_contact = $mech->create_contact_ok(body_id => $brent->id, category => 'ATAK', email => 'ATAK');
 
-FixMyStreet::DB->resultset('BodyArea')->find_or_create({
-    area_id => 2505, # Camden
-    body_id => $brent->id,
-});
+FixMyStreet::DB->resultset('BodyArea')->find_or_create({ area_id => 2505, body_id => $brent->id }); # Camden
+FixMyStreet::DB->resultset('BodyArea')->find_or_create({ area_id => 2487, body_id => $brent->id }); # Harrow
+FixMyStreet::DB->resultset('BodyArea')->find_or_create({ area_id => 2489, body_id => $brent->id }); # Barnet
 
 my $camden = $mech->create_body_ok(2505, 'Camden Borough Council', {},{cobrand => 'camden'});
 my $barnet = $mech->create_body_ok(2489, 'Barnet Borough Council');
@@ -615,8 +614,44 @@ FixMyStreet::override_config {
                 };
             };
 
-            $mech->host("brent.fixmystreet.com");
-            undef $brent_mock; undef $camden_mock;
+    undef $brent_mock;
+    undef $camden_mock;
+
+    subtest "All reports page for Brent works appropriately" => sub {
+        $mech->host("brent.fixmystreet.com");
+        $mech->get_ok("/reports");
+        $mech->content_contains('data-area="2488"');
+        $mech->content_contains('Alperton');
+        $mech->content_lacks('Belsize');
+    };
+
+    subtest "All reports page for Camden works appropriately" => sub {
+        $mech->host("camden.fixmystreet.com");
+        $mech->get_ok("/reports");
+        $mech->content_contains('data-area="2505"');
+        $mech->content_contains('Belsize');
+        $mech->content_lacks('Alperton');
+        $mech->get_ok("/reports/Camden/Belsize");
+        is $mech->uri->path, '/reports/Camden/Belsize';
+    };
+
+    subtest "All reports on .com works appropriately" => sub {
+        $mech->host("fixmystreet.com");
+        $mech->get_ok("/reports/Brent");
+        $mech->content_contains('data-area="2488"');
+        $mech->content_contains('Alperton');
+        $mech->content_lacks('Belsize');
+        $mech->get_ok("/reports/Camden");
+        $mech->content_contains('data-area="2505"');
+        $mech->content_contains('Belsize');
+        $mech->content_lacks('Alperton');
+        $mech->get_ok("/reports/Harrow");
+        $mech->content_contains('data-area="2487"');
+        $mech->content_contains('Belmont');
+        $mech->content_lacks('Alperton');
+    };
+
+    $mech->host("brent.fixmystreet.com");
 };
 
 package SOAP::Result;

t/cobrand/bristol.t

@@ -75,6 +75,9 @@ subtest 'Reports page works with no reports', sub {
         MAP_TYPE => 'Bristol',
     }, sub {
         $mech->get_ok("/reports");
+        $mech->content_contains('Ashley');
+        $mech->content_lacks('Backwell');
+        $mech->content_lacks('Bitton');
     };
 };
 

t/cobrand/fixmystreet.t

@@ -20,6 +20,8 @@ $resolver->mock('address', sub { $_[1] });
 my $body = $mech->create_body_ok( 2514, 'Birmingham', {}, { cobrand => 'birmingham' } );
 $mech->create_body_ok( 2482, 'Bromley', {}, { cobrand => 'bromley' });
 
+$mech->create_body_ok(2482, 'Bike provider');
+
 my $contact = $mech->create_contact_ok(
     body_id => $body->id,
     category => 'Traffic lights',
@@ -40,6 +42,11 @@ FixMyStreet::override_config {
     MAPIT_URL => 'http://mapit.uk/',
     TEST_DASHBOARD_DATA => $data,
     ALLOWED_COBRANDS => [ 'fixmystreet', 'birmingham' ],
+    COBRAND_FEATURES => {
+        categories_restriction_bodies => {
+            tfl => [ 'Bike provider' ],
+        }
+    },
 }, sub {
     ok $mech->host('www.fixmystreet.com');
 
@@ -122,6 +129,14 @@ FixMyStreet::override_config {
         $mech->content_lacks('Where we send Birmingham');
     };
 
+    subtest 'Check All Reports page for bike bodies' => sub {
+        $mech->get_ok('/reports/Bike+provider');
+        $mech->content_contains('Bromley');
+        $mech->content_lacks('Trowbridge');
+        $mech->get_ok('/reports/Bike+provider/Bromley');
+        is $mech->uri->path, '/reports/Bike+provider/Bromley';
+    };
+
     subtest 'check average fix time respects cobrand cut-off date and non-standard reports' => sub {
         $mech->log_in_ok('[email protected]');
         my $user = FixMyStreet::DB->resultset('User')->find_or_create({ email => '[email protected]' });

t/cobrand/highwaysengland.t

@@ -168,6 +168,15 @@ FixMyStreet::override_config {
         $mech->get_ok('/report/' . $problem->id);
         $mech->content_lacks('Provide an update');
     };
+
+    subtest "check All Reports pages display councils correctly" => sub {
+        $mech->host('fixmystreet.com');
+        $mech->get_ok('/reports/National+Highways');
+        $mech->content_contains('Hackney'); # Mock has this returned
+        $mech->host('highwaysengland.example.org');
+        $mech->get_ok('/reports/National+Highways');
+        $mech->content_contains('Hackney');
+    };
 };
 
 subtest 'Dashboard CSV extra columns' => sub {

t/cobrand/northamptonshire.t

@@ -13,6 +13,12 @@ use open ':std', ':encoding(UTF-8)';
 
 my $nh = $mech->create_body_ok(164186, 'Northamptonshire Highways', {
     send_method => 'Open311', api_key => 'key', 'endpoint' => 'e', 'jurisdiction' => 'j', send_comments => 1, can_be_devolved => 1 }, { cobrand => 'northamptonshire' });
+# Associate body with North Northamptonshire area
+FixMyStreet::DB->resultset('BodyArea')->find_or_create({
+    area_id => 164185,
+    body_id => $nh->id,
+});
+
 my $wnc = $mech->create_body_ok(164186, 'West Northamptonshire Council');
 my $po = $mech->create_body_ok(164186, 'Northamptonshire Police');
 
@@ -405,13 +411,6 @@ subtest 'Old report cutoff' => sub {
     is $cobrand->should_skip_sending_update($update2), 0;
 };
 
-# Associate body with North Northamptonshire area
-# (It's associated with West when it's created at the top of this file)
-FixMyStreet::DB->resultset('BodyArea')->find_or_create({
-    area_id => 164185,
-    body_id => $nh->id,
-});
-
 subtest 'Dashboard wards contains North and West wards' => sub {
     my $staffuser = $mech->create_user_ok('[email protected]', name => 'Council User',
         from_body => $nh, password => 'password');
@@ -426,4 +425,21 @@ subtest 'Dashboard wards contains North and West wards' => sub {
     $mech->content_contains('Sulgrave');
 };
 
+FixMyStreet::override_config {
+    MAPIT_URL => 'http://mapit.uk/',
+    ALLOWED_COBRANDS => 'fixmystreet',
+}, sub {
+    subtest 'All reports page working' => sub {
+        $mech->get_ok("/reports/Northamptonshire+Highways");
+        $mech->content_contains('Sulgrave');
+        $mech->content_contains('Weston');
+        $mech->get_ok("/reports/Northamptonshire+Highways/Weston+By+Welland");
+        $mech->content_lacks('Sulgrave');
+        $mech->content_contains('Weston');
+        $mech->get_ok("/reports/Northamptonshire+Highways/Sulgrave");
+        $mech->content_contains('Sulgrave');
+        $mech->content_lacks('Weston');
+    };
+};
+
 done_testing();

t/cobrand/tfl.t

@@ -1176,7 +1176,7 @@ FixMyStreet::override_config {
 };
 
 FixMyStreet::override_config {
-    ALLOWED_COBRANDS => [ 'fixmystreet', 'tfl' ],
+    ALLOWED_COBRANDS => [ 'tfl', 'fixmystreet' ],
     MAPIT_URL => 'http://mapit.uk/'
 }, sub {
     foreach (qw(tfl.fixmystreet.com fixmystreet.com)) {
@@ -1250,4 +1250,26 @@ subtest 'check contact creation allows email from borough email addresses' => su
 
 }};
 
+FixMyStreet::override_config {
+    ALLOWED_COBRANDS => [ 'tfl', 'fixmystreet' ],
+    MAPIT_URL => 'http://mapit.uk/'
+}, sub {
+    subtest "check All Reports pages display councils correctly" => sub {
+        $mech->host('tfl.fixmystreet.com');
+        $mech->get_ok('/reports/TfL');
+        $mech->content_contains('Bromley');
+        $mech->content_contains('Hounslow');
+        $mech->content_lacks('Auriol'); # 2457
+        $mech->content_lacks('Brownswood'); # 2508
+        $mech->content_contains('data-area="2482,2483,2508"'); # No 2457
+        $mech->host('fixmystreet.com');
+        $mech->get_ok('/reports/TfL');
+        $mech->content_contains('Bromley');
+        $mech->content_contains('Hounslow');
+        $mech->content_lacks('Auriol'); # 2457
+        $mech->content_lacks('Brownswood'); # 2508
+        $mech->content_contains('data-area="2482,2483,2508"'); # No 2457
+    };
+};
+
 done_testing();