diff options
-rw-r--r-- | perllib/FixMyStreet/Cobrand/FixMyStreet.pm | 11 | ||||
-rw-r--r-- | perllib/FixMyStreet/Cobrand/TfL.pm | 92 | ||||
-rw-r--r-- | perllib/FixMyStreet/Cobrand/UKCouncils.pm | 30 | ||||
-rw-r--r-- | perllib/FixMyStreet/Cobrand/Westminster.pm | 4 | ||||
-rw-r--r-- | t/Mock/MapIt.pm | 1 | ||||
-rw-r--r-- | t/Mock/Tilma.pm | 39 | ||||
-rw-r--r-- | t/app/controller/admin/templates.t | 2 | ||||
-rw-r--r-- | t/cobrand/hounslow.t | 10 | ||||
-rw-r--r-- | t/cobrand/tfl.t | 130 | ||||
-rw-r--r-- | web/cobrands/tfl/assets.js | 73 |
10 files changed, 299 insertions, 93 deletions
diff --git a/perllib/FixMyStreet/Cobrand/FixMyStreet.pm b/perllib/FixMyStreet/Cobrand/FixMyStreet.pm index a6161b570..6e0a0e2a5 100644 --- a/perllib/FixMyStreet/Cobrand/FixMyStreet.pm +++ b/perllib/FixMyStreet/Cobrand/FixMyStreet.pm @@ -86,11 +86,7 @@ sub munge_reports_categories_list { sub munge_report_new_category_list { my ($self, $options, $contacts, $extras) = @_; - # No TfL Traffic Lights category in Hounslow my %bodies = map { $_->body->name => $_->body } @$contacts; - if ( $bodies{'Hounslow Borough Council'} ) { - @$options = grep { ($_->{category} || $_->category) !~ /^Traffic lights$/i } @$options; - } if ( $bodies{'Isle of Wight Council'} ) { my $user = $self->{c}->user; @@ -105,6 +101,13 @@ sub munge_report_new_category_list { my $seen = { map { $_->category => 1 } @$contacts }; @$options = grep { my $c = ($_->{category} || $_->category); $c =~ 'Pick a category' || $seen->{ $c } } @$options; } + + if ( $bodies{'TfL'} ) { + # Presented categories vary if we're on/off a red route + my $tfl = FixMyStreet::Cobrand->get_class_for_moniker( 'tfl' )->new({ c => $self->{c} }); + $tfl->munge_red_route_categories($options, $contacts); + } + } sub munge_load_and_group_problems { diff --git a/perllib/FixMyStreet/Cobrand/TfL.pm b/perllib/FixMyStreet/Cobrand/TfL.pm index c91b8a79c..b8c03c7a9 100644 --- a/perllib/FixMyStreet/Cobrand/TfL.pm +++ b/perllib/FixMyStreet/Cobrand/TfL.pm @@ -7,6 +7,8 @@ use warnings; use POSIX qw(strcoll); use FixMyStreet::MapIt; +use mySociety::ArrayUtils; +use Utils; sub council_area_id { return [ 2511, 2489, 2494, 2488, 2482, 2505, 2512, 2481, 2484, 2495, @@ -362,4 +364,94 @@ sub munge_sendreport_params { $params->{To} = \@munged_to; } +sub report_new_is_on_tlrn { + my ( $self ) = @_; + + my ($x, $y) = Utils::convert_latlon_to_en( + $self->{c}->stash->{latitude}, + $self->{c}->stash->{longitude}, + 'G' + ); + + my $cfg = { + url => "https://tilma.mysociety.org/mapserver/tfl", + srsname => "urn:ogc:def:crs:EPSG::27700", + typename => "RedRoutes", + filter => "<Filter><Contains><PropertyName>geom</PropertyName><gml:Point><gml:coordinates>$x,$y</gml:coordinates></gml:Point></Contains></Filter>", + }; + + my $features = $self->_fetch_features($cfg, $x, $y); + return scalar @$features ? 1 : 0; +} + +sub munge_report_new_category_list { } + +sub munge_red_route_categories { + my ($self, $options, $contacts) = @_; + if ( $self->report_new_is_on_tlrn ) { + # We're on a red route - only send TfL categories (except the disabled + # one that directs the user to borough for street cleaning XXX TODO: make sure this is included when on the TfL cobrand) and borough + # street cleaning categories. + my %cleaning_cats = map { $_ => 1 } @{ $self->_cleaning_categories }; + @$contacts = grep { + ( $_->body->name eq 'TfL' && $_->category ne $self->_tfl_council_category ) + || $cleaning_cats{$_->category} + || @{ mySociety::ArrayUtils::intersection( $self->_cleaning_groups, $_->groups ) } + } @$contacts; + } else { + # We're not on a red route - send all categories except + # TfL red-route-only. + my %tlrn_cats = map { $_ => 1 } @{ $self->_tlrn_categories }; + @$contacts = grep { !( $_->body->name eq 'TfL' && $tlrn_cats{$_->category } ) } @$contacts; + } + my $seen = { map { $_->category => 1 } @$contacts }; + @$options = grep { my $c = ($_->{category} || $_->category); $c =~ 'Pick a category' || $seen->{ $c } } @$options; +} + +# Reports in these categories can only be made on a red route +sub _tlrn_categories { [ + "All out - three or more street lights in a row", + "Blocked drain", + "Damage - general (Trees)", + "Dead animal in the carriageway or footway", + "Debris in the carriageway", + "Fallen Tree", + "Flooding", + "Flytipping (TfL)", + "Graffiti / Flyposting (non-offensive)", + "Graffiti / Flyposting (offensive)", + "Graffiti / Flyposting on street light (non-offensive)", + "Graffiti / Flyposting on street light (offensive)", + "Grass Cutting and Hedges", + "Hoardings blocking carriageway or footway", + "Light on during daylight hours", + "Lights out in Pedestrian Subway", + "Low hanging branches and general maintenance", + "Manhole Cover - Damaged (rocking or noisy)", + "Manhole Cover - Missing", + "Mobile Crane Operation", + "Other (TfL)", + "Pavement Defect (uneven surface / cracked paving slab)", + "Pothole", + "Roadworks", + "Scaffolding blocking carriageway or footway", + "Single Light out (street light)", + "Standing water", + "Unstable hoardings", + "Unstable scaffolding", + "Worn out road markings", +] } + +sub _cleaning_categories { [ + 'Street cleaning', + 'Street Cleaning', + 'Accumulated Litter', + 'Street Cleaning Enquiry', + 'Street Cleansing', +] } + +sub _cleaning_groups { [ 'Street cleaning' ] } + +sub _tfl_council_category { 'General Litter / Rubbish Collection' } + 1; diff --git a/perllib/FixMyStreet/Cobrand/UKCouncils.pm b/perllib/FixMyStreet/Cobrand/UKCouncils.pm index ef0bcf4fb..97c8b6c81 100644 --- a/perllib/FixMyStreet/Cobrand/UKCouncils.pm +++ b/perllib/FixMyStreet/Cobrand/UKCouncils.pm @@ -289,6 +289,18 @@ sub prefill_report_fields_for_inspector { 1 } sub social_auth_disabled { 1 } +sub munge_report_new_category_list { + my ($self, $options, $contacts, $extras) = @_; + + my %bodies = map { $_->body->name => $_->body } @$contacts; + if ( $bodies{'TfL'} ) { + # Presented categories vary if we're on/off a red route + my $tfl = FixMyStreet::Cobrand->get_class_for_moniker( 'tfl' )->new({ c => $self->{c} }); + $tfl->munge_red_route_categories($options, $contacts); + } +} + + =head2 lookup_site_code Reports made via FMS.com or the app probably won't have a site code @@ -317,10 +329,16 @@ sub lookup_site_code { sub _fetch_features { my ($self, $cfg, $x, $y) = @_; - my $buffer = $cfg->{buffer}; - my ($w, $s, $e, $n) = ($x-$buffer, $y-$buffer, $x+$buffer, $y+$buffer); - my $uri = $self->_fetch_features_url($cfg, $w, $s, $e,$n); + # default to a buffered bounding box around the given point unless + # a custom filter parameter has been specified. + unless ( $cfg->{filter} ) { + my $buffer = $cfg->{buffer}; + my ($w, $s, $e, $n) = ($x-$buffer, $y-$buffer, $x+$buffer, $y+$buffer); + $cfg->{bbox} = "$w,$s,$e,$n"; + } + + my $uri = $self->_fetch_features_url($cfg); my $response = get($uri) or return; my $j = JSON->new->utf8->allow_nonref; @@ -336,7 +354,7 @@ sub _fetch_features { } sub _fetch_features_url { - my ($self, $cfg, $w, $s, $e, $n) = @_; + my ($self, $cfg) = @_; my $uri = URI->new($cfg->{url}); $uri->query_form( @@ -346,7 +364,7 @@ sub _fetch_features_url { TYPENAME => $cfg->{typename}, VERSION => "1.1.0", outputformat => "geojson", - BBOX => "$w,$s,$e,$n" + $cfg->{filter} ? ( Filter => $cfg->{filter} ) : ( BBOX => $cfg->{bbox} ), ); return $uri; @@ -372,7 +390,7 @@ sub _nearest_feature { next unless $accept_types->{$feature->{geometry}->{type}}; my @linestrings = @{ $feature->{geometry}->{coordinates} }; - if ( $feature->{geometry}->{type} eq 'LineString') { + if ( $feature->{geometry}->{type} eq 'LineString' ) { @linestrings = ([ @linestrings ]); } # If it is a point, upgrade it to a one-segment zero-length diff --git a/perllib/FixMyStreet/Cobrand/Westminster.pm b/perllib/FixMyStreet/Cobrand/Westminster.pm index 8adb7eb2c..7a3d7bbcd 100644 --- a/perllib/FixMyStreet/Cobrand/Westminster.pm +++ b/perllib/FixMyStreet/Cobrand/Westminster.pm @@ -122,7 +122,7 @@ sub lookup_site_code_config { } sub _fetch_features_url { - my ($self, $cfg, $w, $s, $e, $n) = @_; + my ($self, $cfg) = @_; # Westminster's asset proxy has a slightly different calling style to # a standard WFS server. @@ -132,7 +132,7 @@ sub _fetch_features_url { outSR => "27700", f => "geojson", outFields => $cfg->{property}, - geometry => "$w,$s,$e,$n", + geometry => $cfg->{bbox}, ); return $cfg->{proxy_url} . "?" . $uri->as_string; diff --git a/t/Mock/MapIt.pm b/t/Mock/MapIt.pm index 96be429e4..cd441856a 100644 --- a/t/Mock/MapIt.pm +++ b/t/Mock/MapIt.pm @@ -39,6 +39,7 @@ my @PLACES = ( [ 'LE15 0GJ', 52.670447, -0.727877, 2600, 'Rutland County Council', 'CTY'], [ 'BR1 3UH', 51.4021, 0.01578, 2482, 'Bromley Council', 'LBO' ], [ 'BR1 3UH', 51.402096, 0.015784, 2482, 'Bromley Council', 'LBO' ], + [ 'BR1 3EF', 51.4039, 0.018697, 2482, 'Bromley Council', 'LBO' ], [ 'NN1 1NS', 52.236251, -0.892052, 2234, 'Northamptonshire County Council', 'CTY', 2397, 'Northampton Borough Council', 'DIS' ], [ 'NN1 2NS', 52.238301, -0.889992, 2234, 'Northamptonshire County Council', 'CTY', 2397, 'Northampton Borough Council', 'DIS' ], [ '?', 52.238827, -0.894970, 2234, 'Northamptonshire County Council', 'CTY', 2397, 'Northampton Borough Council', 'DIS' ], diff --git a/t/Mock/Tilma.pm b/t/Mock/Tilma.pm new file mode 100644 index 000000000..5a11209e3 --- /dev/null +++ b/t/Mock/Tilma.pm @@ -0,0 +1,39 @@ +package t::Mock::Tilma; + +use JSON::MaybeXS; +use Web::Simple; + +has json => ( + is => 'lazy', + default => sub { + JSON->new->utf8->pretty->allow_blessed->convert_blessed; + }, +); + +sub dispatch_request { + my $self = shift; + + sub (GET + /mapserver/tfl + ?*) { + my ($self, $args) = @_; + my $features = []; + if ($args->{Filter} =~ /540512,169141/) { + $features = [ + { type => "Feature", properties => { HA_ID => "19" }, geometry => { type => "Polygon", coordinates => [ [ + [ 539408.94, 170607.58 ], + [ 539432.81, 170627.93 ], + [ 539437.24, 170623.48 ], + [ 539408.94, 170607.58 ], + ] ] } } ]; + } + my $json = mySociety::Locale::in_gb_locale { + $self->json->encode({ + type => "FeatureCollection", + crs => { type => "name", properties => { name => "urn:ogc:def:crs:EPSG::27700" } }, + features => $features, + }); + }; + return [ 200, [ 'Content-Type' => 'application/json' ], [ $json ] ]; + }, +} + +__PACKAGE__->run_if_script; diff --git a/t/app/controller/admin/templates.t b/t/app/controller/admin/templates.t index 200fbd727..ad5b3e77b 100644 --- a/t/app/controller/admin/templates.t +++ b/t/app/controller/admin/templates.t @@ -347,7 +347,7 @@ subtest "TfL cobrand only shows TfL templates" => sub { subtest "Bromley cobrand only shows Bromley templates" => sub { FixMyStreet::override_config { - ALLOWED_COBRANDS => [ 'bromley' ], + ALLOWED_COBRANDS => [ 'bromley', 'tfl' ], }, sub { $report->update({ category => $bromleycontact->category, bodies_str => $bromley->id }); $mech->log_in_ok( $bromleyuser->email ); diff --git a/t/cobrand/hounslow.t b/t/cobrand/hounslow.t index cb67ad397..91c1cb455 100644 --- a/t/cobrand/hounslow.t +++ b/t/cobrand/hounslow.t @@ -75,16 +75,6 @@ subtest "it shows the right things on an /around page" => sub { }; }; -subtest "does not show TfL traffic lights category" => sub { - FixMyStreet::override_config { - MAPIT_URL => 'http://mapit.uk/', - ALLOWED_COBRANDS => 'fixmystreet', - }, sub { - my $json = $mech->get_ok_json('/report/new/ajax?latitude=51.482286&longitude=-0.328163'); - is $json->{by_category}{"Traffic lights"}, undef; - }; -}; - subtest "Shows external ID on report page to staff users only" => sub { FixMyStreet::override_config { ALLOWED_COBRANDS => 'hounslow', diff --git a/t/cobrand/tfl.t b/t/cobrand/tfl.t index a327a00ef..33506e8c4 100644 --- a/t/cobrand/tfl.t +++ b/t/cobrand/tfl.t @@ -8,6 +8,11 @@ END { FixMyStreet::App->log->enable('info'); } my $mech = FixMyStreet::TestMech->new; +use t::Mock::Tilma; +my $tilma = t::Mock::Tilma->new; +LWP::Protocol::PSGI->register($tilma->to_psgi_app, host => 'tilma.mysociety.org'); + + my $body = $mech->create_body_ok(2482, 'TfL'); FixMyStreet::DB->resultset('BodyArea')->find_or_create({ area_id => 2483, # Hounslow @@ -27,7 +32,26 @@ my $user = $mech->create_user_ok('londonresident@example.com'); my $bromley = $mech->create_body_ok(2482, 'Bromley'); my $bromleyuser = $mech->create_user_ok('bromleyuser@bromley.example.com', name => 'Bromley Staff', from_body => $bromley); +$mech->create_contact_ok( + body_id => $bromley->id, + category => 'Accumulated Litter', + email => 'litter-bromley@example.com', +); +my $bromley_flooding = $mech->create_contact_ok( + body_id => $bromley->id, + category => 'Flooding (Bromley)', + email => 'litter-bromley@example.com', +); +$bromley_flooding->set_extra_metadata(display_name => 'Flooding'); +$bromley_flooding->update; +my $bromley_flytipping = $mech->create_contact_ok( + body_id => $bromley->id, + category => 'Flytipping (Bromley)', + email => 'flytipping-bromley@example.com', +); +$bromley_flytipping->set_extra_metadata(group => [ 'Street cleaning' ]); +$bromley_flytipping->update; my $contact1 = $mech->create_contact_ok( body_id => $body->id, @@ -660,6 +684,106 @@ FixMyStreet::override_config { }, }, sub { +for my $test ( + { + host => 'www.fixmystreet.com', + name => "test red route categories", + lat => 51.4039, + lon => 0.018697, + expected => [ + 'Accumulated Litter', # Tests TfL->_cleaning_categories + 'Bus stops', + 'Flooding', + 'Flytipping (Bromley)', # In the 'Street cleaning' group + 'Grit bins', + 'Pothole', + 'Traffic lights', + 'Trees' + ], + }, + { + host => 'www.fixmystreet.com', + name => "test non-red route categories", + lat => 51.4021, + lon => 0.01578, + expected => [ + 'Accumulated Litter', # Tests TfL->_cleaning_categories + 'Bus stops', + 'Flooding (Bromley)', + 'Flytipping (Bromley)', # In the 'Street cleaning' group + 'Grit bins', + 'Traffic lights', + 'Trees' + ], + }, + { + host => 'tfl.fixmystreet.com', + name => "test red route categories", + lat => 51.4039, + lon => 0.018697, + expected => [ + 'Bus stops', + 'Flooding', + 'Grit bins', + 'Pothole', + 'Traffic lights', + 'Trees' + ], + }, + { + host => 'tfl.fixmystreet.com', + name => "test non-red route categories", + lat => 51.4021, + lon => 0.01578, + expected => [ + 'Bus stops', + 'Flooding', + 'Grit bins', + 'Pothole', + 'Traffic lights', + 'Trees' + ], + }, + { + host => 'bromley.fixmystreet.com', + name => "test red route categories", + lat => 51.4039, + lon => 0.018697, + expected => [ + 'Accumulated Litter', + 'Bus stops', + 'Flooding', + 'Flytipping (Bromley)', + 'Grit bins', + 'Pothole', + 'Traffic lights', + 'Trees' + ], + }, + { + host => 'bromley.fixmystreet.com', + name => "test non-red route categories", + lat => 51.4021, + lon => 0.01578, + expected => [ + 'Accumulated Litter', + 'Bus stops', + 'Flooding (Bromley)', + 'Flytipping (Bromley)', + 'Grit bins', + 'Traffic lights', + 'Trees' + ], + }, +) { + subtest $test->{name} . ' on ' . $test->{host} => sub { + $mech->host($test->{host}); + my $resp = $mech->get_ok_json( '/report/new/ajax?latitude=' . $test->{lat} . '&longitude=' . $test->{lon} ); + my @actual = sort keys %{ $resp->{by_category} }; + is_deeply \@actual, $test->{expected}; + }; +} + for my $host ( 'tfl.fixmystreet.com', 'www.fixmystreet.com', 'bromley.fixmystreet.com' ) { for my $test ( { @@ -673,6 +797,7 @@ for my $host ( 'tfl.fixmystreet.com', 'www.fixmystreet.com', 'bromley.fixmystree safety_critical => 'yes', category => "Pothole", subject => "Dangerous Pothole Report: Test Report", + pc => "BR1 3EF", # this is on a red route (according to Mock::MapIt and Mock::Tilma anyway) }, { name => "test category extra field - safety critical", @@ -682,6 +807,7 @@ for my $host ( 'tfl.fixmystreet.com', 'www.fixmystreet.com', 'bromley.fixmystree location => "carriageway", }, subject => "Dangerous Flooding Report: Test Report", + pc => "BR1 3EF", # this is on a red route (according to Mock::MapIt and Mock::Tilma anyway) }, { name => "test category extra field - non-safety critical", @@ -691,13 +817,15 @@ for my $host ( 'tfl.fixmystreet.com', 'www.fixmystreet.com', 'bromley.fixmystree location => "footway", }, subject => "Problem Report: Test Report", + pc => "BR1 3EF", # this is on a red route (according to Mock::MapIt and Mock::Tilma anyway) }, ) { subtest $test->{name} . ' on ' . $host => sub { $mech->log_in_ok( $user->email ); $mech->host($host); $mech->get_ok('/around'); - $mech->submit_form_ok( { with_fields => { pc => 'BR1 3UH', } }, "submit location" ); + my $pc = $test->{pc} || 'BR1 3UH'; + $mech->submit_form_ok( { with_fields => { pc => $pc, } }, "submit location ($pc)" ); $mech->follow_link_ok( { text_regex => qr/skip this step/i, }, "follow 'skip this step' link" ); $mech->submit_form_ok( { diff --git a/web/cobrands/tfl/assets.js b/web/cobrands/tfl/assets.js index 96927600f..ca712934b 100644 --- a/web/cobrands/tfl/assets.js +++ b/web/cobrands/tfl/assets.js @@ -4,18 +4,6 @@ if (!fixmystreet.maps) { return; } -var cleaning_categories = [ - 'Street cleaning', - 'Street Cleaning', - 'Accumulated Litter', - 'Street Cleaning Enquiry', - 'Street Cleansing' -]; -var cleaning_groups = [ - 'Street cleaning' -]; -var tfl_council_category = 'General Litter / Rubbish Collection'; - var defaults = { http_options: { url: "https://tilma.mysociety.org/mapserver/tfl", @@ -94,12 +82,6 @@ function is_tlrn_category_only(category, bodies) { bodies.length <= 1; } -function regenerate_category_groups() { - var old_category_group = $('#category_group').val() || $('#filter_group').val(); - $('#category_group').remove(); - fixmystreet.set_up.category_groups(old_category_group, true); -} - var red_routes_layer = fixmystreet.assets.add(defaults, { http_options: { url: "https://tilma.mysociety.org/mapserver/tfl", @@ -117,65 +99,18 @@ var red_routes_layer = fixmystreet.assets.add(defaults, { stylemap: tlrn_stylemap, no_asset_msg_id: '#js-not-tfl-road', actions: { - found: function(layer, feature) { - fixmystreet.message_controller.road_found(layer, feature); - - if (fixmystreet.cobrand === 'tfl' || !fixmystreet.reporting_data) { + found: fixmystreet.message_controller.road_found, + not_found: function(layer) { + // Only care about this on TfL cobrand + if (fixmystreet.cobrand !== 'tfl') { return; } - - // On a TfL road, remove any council categories, except street cleaning - var changed = false; - $('#form_category').find('option').each(function(i, option) { - var val = option.value; - if (OpenLayers.Util.indexOf(cleaning_categories, val) > -1) { - return; - } - var optgroup = $(option).closest('optgroup').attr('label'); - if (OpenLayers.Util.indexOf(cleaning_groups, optgroup) > -1) { - return; - } - if (val === tfl_council_category) { - changed = true; - $(option).remove(); - } - var data = fixmystreet.reporting_data.by_category[val]; - if (data && OpenLayers.Util.indexOf(data.bodies, 'TfL') === -1) { - changed = true; - $(option).remove(); - } - }); - if (changed) { - regenerate_category_groups(); - } - }, - not_found: function(layer) { var category = $('#form_category').val(); if (is_tlrn_category_only(category, fixmystreet.bodies)) { fixmystreet.message_controller.road_not_found(layer); } else { fixmystreet.message_controller.road_found(layer); } - - if (fixmystreet.cobrand === 'tfl' || !fixmystreet.reporting_data) { - return; - } - - // On non-TfL, remove any TfL-road-only categories - var changed = false; - $('#form_category').find('option').each(function(i, option) { - if (option.value === tfl_council_category) { - $(option).remove(); - } - var data = fixmystreet.reporting_data.by_category[option.value]; - if (data && is_tlrn_category_only(option.value, data.bodies)) { - changed = true; - $(option).remove(); - } - }); - if (changed) { - regenerate_category_groups(); - } } } }); |