diff options
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Report/New.pm | 22 | ||||
-rw-r--r-- | perllib/FixMyStreet/Cobrand/BathNES.pm | 32 | ||||
-rw-r--r-- | perllib/FixMyStreet/Cobrand/Buckinghamshire.pm | 28 | ||||
-rw-r--r-- | perllib/FixMyStreet/Cobrand/Hounslow.pm | 32 | ||||
-rw-r--r-- | perllib/FixMyStreet/Cobrand/IsleOfWight.pm | 26 | ||||
-rw-r--r-- | perllib/FixMyStreet/Cobrand/Lincolnshire.pm | 28 | ||||
-rw-r--r-- | perllib/FixMyStreet/Cobrand/Peterborough.pm | 32 | ||||
-rw-r--r-- | perllib/FixMyStreet/Roles/ConfirmOpen311.pm | 39 | ||||
-rw-r--r-- | perllib/FixMyStreet/Roles/ConfirmValidation.pm | 2 | ||||
-rw-r--r-- | t/app/controller/auth_profile.t | 4 | ||||
-rw-r--r-- | t/cobrand/fixmystreet.t | 4 | ||||
-rw-r--r-- | t/cobrand/isleofwight.t | 12 | ||||
-rw-r--r-- | t/cobrand/westminster.t | 2 | ||||
-rw-r--r-- | web/cobrands/bristol/assets.js | 3 | ||||
-rw-r--r-- | web/cobrands/fixmystreet/assets.js | 13 | ||||
-rw-r--r-- | web/cobrands/lincolnshire/assets.js | 76 |
16 files changed, 118 insertions, 237 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm index cac50e34d..612c76c0c 100644 --- a/perllib/FixMyStreet/App/Controller/Report/New.pm +++ b/perllib/FixMyStreet/App/Controller/Report/New.pm @@ -1699,12 +1699,24 @@ sub generate_category_extra_json : Private { my $false = JSON->false; my @fields = map { - { - %$_, - required => ($_->{required} || '') eq "true" ? $true : $false, - variable => ($_->{variable} || '') eq "true" ? $true : $false, - order => int($_->{order} || 0), + my %data = %$_; + + # Mobile app still looks in datatype_description + if (($_->{variable} || '') eq 'true' && @{$_->{values} || []}) { + foreach my $opt (@{$_->{values}}) { + if ($opt->{disable}) { + my $message = $opt->{disable_message} || $_->{datatype_description}; + $data{datatype_description} = $message; + } + } } + + # Remove unneeded + delete $data{$_} for qw(datatype protected variable order disable_form); + delete $data{datatype_description} unless $data{datatype_description}; + + $data{required} = ($_->{required} || '') eq "true" ? $true : $false; + \%data; } @{ $c->stash->{category_extras}->{$c->stash->{category}} }; return \@fields; diff --git a/perllib/FixMyStreet/Cobrand/BathNES.pm b/perllib/FixMyStreet/Cobrand/BathNES.pm index 6de28bca8..dd1b93764 100644 --- a/perllib/FixMyStreet/Cobrand/BathNES.pm +++ b/perllib/FixMyStreet/Cobrand/BathNES.pm @@ -6,6 +6,7 @@ use warnings; use Moo; with 'FixMyStreet::Roles::ConfirmValidation'; +with 'FixMyStreet::Roles::ConfirmOpen311'; use LWP::Simple; use URI; @@ -34,7 +35,7 @@ sub contact_extra_fields_validation { return unless $contact->get_extra_metadata('display_name'); my @contacts = $contact->body->contacts->not_deleted->search({ id => { '!=', $contact->id } }); - my %display_names = map { $_->get_extra_metadata('display_name') => 1 } @contacts; + my %display_names = map { ($_->get_extra_metadata('display_name') || '') => 1 } @contacts; if ($display_names{$contact->get_extra_metadata('display_name')}) { $errors->{display_name} = 'That display name is already in use'; } @@ -97,33 +98,6 @@ sub category_extra_hidden { return $self->SUPER::category_extra_hidden($meta); } -sub open311_config { - my ($self, $row, $h, $params) = @_; - - my $extra = $row->get_extra_fields; - push @$extra, - { name => 'report_url', - value => $h->{url} }, - { name => 'title', - value => $row->title }, - { name => 'description', - value => $row->detail }; - - # Reports made via FMS.com or the app probably won't have a USRN - # value because we don't display the adopted highways layer on those - # frontends. Instead we'll look up the closest asset from the WFS - # service at the point we're sending the report over Open311. - if (!$row->get_extra_field_value('site_code')) { - if (my $usrn = $self->lookup_usrn($row)) { - push @$extra, - { name => 'site_code', - value => $usrn }; - } - } - - $row->set_extra_fields(@$extra); -} - sub available_permissions { my $self = shift; @@ -137,7 +111,7 @@ sub available_permissions { sub report_sent_confirmation_email { 'id' } -sub lookup_usrn { +sub lookup_site_code { my $self = shift; my $row = shift; diff --git a/perllib/FixMyStreet/Cobrand/Buckinghamshire.pm b/perllib/FixMyStreet/Cobrand/Buckinghamshire.pm index e210e7d09..b13f4eed1 100644 --- a/perllib/FixMyStreet/Cobrand/Buckinghamshire.pm +++ b/perllib/FixMyStreet/Cobrand/Buckinghamshire.pm @@ -5,6 +5,7 @@ use strict; use warnings; use Moo; +with 'FixMyStreet::Roles::ConfirmOpen311'; with 'FixMyStreet::Roles::ConfirmValidation'; sub council_area_id { return 2217; } @@ -43,33 +44,6 @@ sub send_questionnaires { return 0; } -sub open311_config { - my ($self, $row, $h, $params) = @_; - - my $extra = $row->get_extra_fields; - push @$extra, - { name => 'report_url', - value => $h->{url} }, - { name => 'title', - value => $row->title }, - { name => 'description', - value => $row->detail }; - - # Reports made via FMS.com or the app probably won't have a site code - # value because we don't display the adopted highways layer on those - # frontends. Instead we'll look up the closest asset from the WFS - # service at the point we're sending the report over Open311. - if (!$row->get_extra_field_value('site_code')) { - if (my $site_code = $self->lookup_site_code($row)) { - push @$extra, - { name => 'site_code', - value => $site_code }; - } - } - - $row->set_extra_fields(@$extra); -} - sub open311_pre_send { my ($self, $row, $open311) = @_; diff --git a/perllib/FixMyStreet/Cobrand/Hounslow.pm b/perllib/FixMyStreet/Cobrand/Hounslow.pm index 2d5c2bf0f..ab131cf84 100644 --- a/perllib/FixMyStreet/Cobrand/Hounslow.pm +++ b/perllib/FixMyStreet/Cobrand/Hounslow.pm @@ -5,6 +5,7 @@ use strict; use warnings; use Moo; +with 'FixMyStreet::Roles::ConfirmOpen311'; with 'FixMyStreet::Roles::ConfirmValidation'; sub council_area_id { 2483 } @@ -89,35 +90,12 @@ sub open311_post_send { } } -sub open311_config { - my ($self, $row, $h, $params) = @_; - - my $extra = $row->get_extra_fields; - push @$extra, - { name => 'report_url', - value => $h->{url} }, - { name => 'title', - value => $row->title }, - { name => 'description', - value => $row->detail }; - - # Reports made via FMS.com or the app probably won't have a site code - # value because we don't display the adopted highways layer on those - # frontends. Instead we'll look up the closest asset from the WFS - # service at the point we're sending the report over Open311. - if (!$row->get_extra_field_value('site_code')) { - if (my $site_code = $self->lookup_site_code($row)) { - push @$extra, - { name => 'site_code', - value => $site_code }; - } - } +around 'open311_config' => sub { + my ($orig, $self, $row, $h, $params) = @_; - $row->set_extra_fields(@$extra); - - $params->{multi_photos} = 1; $params->{upload_files} = 1; -} + $self->$orig($row, $h, $params); +}; sub open311_munge_update_params { my ($self, $params, $comment, $body) = @_; diff --git a/perllib/FixMyStreet/Cobrand/IsleOfWight.pm b/perllib/FixMyStreet/Cobrand/IsleOfWight.pm index 26e3ba474..83431c532 100644 --- a/perllib/FixMyStreet/Cobrand/IsleOfWight.pm +++ b/perllib/FixMyStreet/Cobrand/IsleOfWight.pm @@ -4,6 +4,9 @@ use parent 'FixMyStreet::Cobrand::Whitelabel'; use strict; use warnings; +use Moo; +with 'FixMyStreet::Roles::ConfirmOpen311'; + sub council_area_id { 2636 } sub council_area { 'Isle of Wight' } sub council_name { 'Island Roads' } @@ -78,29 +81,6 @@ sub open311_pre_send { } } -sub open311_config { - my ($self, $row, $h, $params) = @_; - - my $extra = $row->get_extra_fields; - push @$extra, - { name => 'report_url', - value => $h->{url} }, - { name => 'title', - value => $row->title }, - { name => 'description', - value => $row->detail }; - - if (!$row->get_extra_field_value('site_code')) { - if (my $site_code = $self->lookup_site_code($row)) { - push @$extra, - { name => 'site_code', - value => $site_code }; - } - } - - $row->set_extra_fields(@$extra); -} - # Make sure fetched report description isn't shown. sub filter_report_description { "" } diff --git a/perllib/FixMyStreet/Cobrand/Lincolnshire.pm b/perllib/FixMyStreet/Cobrand/Lincolnshire.pm index ca88f6b8e..eaed35118 100644 --- a/perllib/FixMyStreet/Cobrand/Lincolnshire.pm +++ b/perllib/FixMyStreet/Cobrand/Lincolnshire.pm @@ -10,6 +10,7 @@ use Try::Tiny; use JSON::MaybeXS; use Moo; +with 'FixMyStreet::Roles::ConfirmOpen311'; with 'FixMyStreet::Roles::ConfirmValidation'; sub council_area_id { return 2232; } @@ -41,33 +42,6 @@ sub disambiguate_location { } -sub open311_config { - my ($self, $row, $h, $params) = @_; - - my $extra = $row->get_extra_fields; - push @$extra, - { name => 'report_url', - value => $h->{url} }, - { name => 'title', - value => $row->title }, - { name => 'description', - value => $row->detail }; - - # Reports made via FMS.com or the app probably won't have a site code - # value because we don't display the adopted highways layer on those - # frontends. Instead we'll look up the closest asset from the WFS - # service at the point we're sending the report over Open311. - if (!$row->get_extra_field_value('site_code')) { - if (my $site_code = $self->lookup_site_code($row)) { - push @$extra, - { name => 'site_code', - value => $site_code }; - } - } - - $row->set_extra_fields(@$extra); -} - sub lookup_site_code_config { { buffer => 200, # metres url => "https://tilma.mysociety.org/mapserver/lincs", diff --git a/perllib/FixMyStreet/Cobrand/Peterborough.pm b/perllib/FixMyStreet/Cobrand/Peterborough.pm index 9aa807cda..dd9bb0670 100644 --- a/perllib/FixMyStreet/Cobrand/Peterborough.pm +++ b/perllib/FixMyStreet/Cobrand/Peterborough.pm @@ -5,6 +5,7 @@ use strict; use warnings; use Moo; +with 'FixMyStreet::Roles::ConfirmOpen311'; with 'FixMyStreet::Roles::ConfirmValidation'; sub council_area_id { 2566 } @@ -34,35 +35,16 @@ sub geocoder_munge_results { sub admin_user_domain { "peterborough.gov.uk" } -sub open311_config { - my ($self, $row, $h, $params) = @_; - - my $extra = $row->get_extra_fields; - push @$extra, - { name => 'report_url', - value => $h->{url} }, - { name => 'title', - value => $row->title }, - { name => 'description', - value => $row->detail }; +around 'open311_config' => sub { + my ($orig, $self, $row, $h, $params) = @_; # remove the emergency category which is informational only + my $extra = $row->get_extra_fields; @$extra = grep { $_->{name} ne 'emergency' } @$extra; - - # Reports made via FMS.com or the app probably won't have a site code - # value because we don't display the adopted highways layer on those - # frontends. Instead we'll look up the closest asset from the WFS - # service at the point we're sending the report over Open311. - if (!$row->get_extra_field_value('site_code')) { - if (my $site_code = $self->lookup_site_code($row)) { - push @$extra, - { name => 'site_code', - value => $site_code }; - } - } - $row->set_extra_fields(@$extra); -} + + $self->$orig($row, $h, $params); +}; sub lookup_site_code_config { { buffer => 50, # metres diff --git a/perllib/FixMyStreet/Roles/ConfirmOpen311.pm b/perllib/FixMyStreet/Roles/ConfirmOpen311.pm new file mode 100644 index 000000000..b9e424d4f --- /dev/null +++ b/perllib/FixMyStreet/Roles/ConfirmOpen311.pm @@ -0,0 +1,39 @@ +package FixMyStreet::Roles::ConfirmOpen311; +use Moo::Role; + +=head1 NAME + +FixMyStreet::Roles::ConfirmOpen311 - role for adding various Open311 things specific to Confirm + +=cut + +sub open311_config { + my ($self, $row, $h, $params) = @_; + + $params->{multi_photos} = 1; + + my $extra = $row->get_extra_fields; + push @$extra, + { name => 'report_url', + value => $h->{url} }, + { name => 'title', + value => $row->title }, + { name => 'description', + value => $row->detail }; + + # Reports made via FMS.com or the app probably won't have a USRN + # value because we don't display the adopted highways layer on those + # frontends. Instead we'll look up the closest asset from the WFS + # service at the point we're sending the report over Open311. + if (!$row->get_extra_field_value('site_code')) { + if (my $site_code = $self->lookup_site_code($row)) { + push @$extra, + { name => 'site_code', + value => $site_code }; + } + } + + $row->set_extra_fields(@$extra); +} + +1; diff --git a/perllib/FixMyStreet/Roles/ConfirmValidation.pm b/perllib/FixMyStreet/Roles/ConfirmValidation.pm index 776230287..6474c94d1 100644 --- a/perllib/FixMyStreet/Roles/ConfirmValidation.pm +++ b/perllib/FixMyStreet/Roles/ConfirmValidation.pm @@ -24,7 +24,7 @@ sub report_validation { $errors->{name} = sprintf( _('Names are limited to %d characters in length.'), 50 ); } - if ( length( $report->user->phone ) > 20 ) { + if ( $report->user->phone && length( $report->user->phone ) > 20 ) { $errors->{phone} = sprintf( _('Phone numbers are limited to %s characters in length.'), 20 ); } diff --git a/t/app/controller/auth_profile.t b/t/app/controller/auth_profile.t index f68b64835..4b7db19b5 100644 --- a/t/app/controller/auth_profile.t +++ b/t/app/controller/auth_profile.t @@ -5,9 +5,13 @@ sub must_have_2fa { 1 } package main; +use Test::MockModule; use FixMyStreet::TestMech; my $mech = FixMyStreet::TestMech->new; +my $resolver = Test::MockModule->new('Email::Valid'); +$resolver->mock('address', sub { $_[1] }); + use t::Mock::Twilio; my $twilio = t::Mock::Twilio->new; diff --git a/t/cobrand/fixmystreet.t b/t/cobrand/fixmystreet.t index fedb06f40..a54e782f2 100644 --- a/t/cobrand/fixmystreet.t +++ b/t/cobrand/fixmystreet.t @@ -1,9 +1,13 @@ use utf8; use FixMyStreet::Script::UpdateAllReports; +use Test::MockModule; use FixMyStreet::TestMech; my $mech = FixMyStreet::TestMech->new; +my $resolver = Test::MockModule->new('Email::Valid'); +$resolver->mock('address', sub { $_[1] }); + my $body = $mech->create_body_ok( 2514, 'Birmingham' ); my $contact = $mech->create_contact_ok( diff --git a/t/cobrand/isleofwight.t b/t/cobrand/isleofwight.t index fd08ce455..f945dc924 100644 --- a/t/cobrand/isleofwight.t +++ b/t/cobrand/isleofwight.t @@ -1,5 +1,6 @@ use CGI::Simple; use DateTime; +use Test::MockModule; use FixMyStreet::TestMech; use Open311; use Open311::GetServiceRequests; @@ -8,6 +9,16 @@ use Open311::PostServiceRequestUpdates; use FixMyStreet::Script::Alerts; use FixMyStreet::Script::Reports; +# disable info logs for this test run +FixMyStreet::App->log->disable('info'); +END { FixMyStreet::App->log->enable('info'); } + +my $cobrand = Test::MockModule->new('FixMyStreet::Cobrand::IsleOfWight'); +$cobrand->mock('lookup_site_code', sub { + my ($self, $row) = @_; + return "Road ID" if $row->latitude == 50.7108; +}); + ok( my $mech = FixMyStreet::TestMech->new, 'Created mech object' ); my $params = { @@ -290,6 +301,7 @@ subtest 'Check special Open311 request handling', sub { my $req = $test_data->{test_req_used}; my $c = CGI::Simple->new($req->content); is $c->param('attribute[urgent]'), undef, 'no urgent param sent'; + is $c->param('attribute[site_code]'), 'Road ID', 'road ID set'; $mech->email_count_is(1); my $email = $mech->get_email; diff --git a/t/cobrand/westminster.t b/t/cobrand/westminster.t index 84def0917..54de5a160 100644 --- a/t/cobrand/westminster.t +++ b/t/cobrand/westminster.t @@ -79,7 +79,7 @@ FixMyStreet::override_config { $mech->content_contains($comment3->text); }; - subtest 'Reports don’t show updates from fixmystreet.com cobrand' => sub { + subtest "Reports don't show updates from fixmystreet.com cobrand" => sub { $mech->get_ok('/report/' . $report->id); $mech->content_lacks($comment2->text); }; diff --git a/web/cobrands/bristol/assets.js b/web/cobrands/bristol/assets.js index 0e22e8f19..0e1ac603f 100644 --- a/web/cobrands/bristol/assets.js +++ b/web/cobrands/bristol/assets.js @@ -19,7 +19,7 @@ var options = { wfs_url: "https://maps.bristol.gov.uk/arcgis/services/ext/FixMyStreetSupportData/MapServer/WFSServer", wfs_feature: "COD_ASSETS_POINT", asset_id_field: 'COD_ASSET_ID', - propertyNames: [ 'COD_ASSET_ID', 'COD_USRN', 'COD_ASSET_TYPE', 'SHAPE' ], + propertyNames: [ 'COD_ASSET_ID', 'COD_USRN', 'SHAPE' ], filter_key: 'COD_ASSET_TYPE', attributes: { asset_id: 'COD_ASSET_ID', @@ -50,7 +50,6 @@ fixmystreet.assets.add(options, { fixmystreet.assets.add(options, { asset_category: "Flooding", asset_item: 'flood risk structure', - filter_key: 'COD_ASSET_TYPE', filter_value: 'FRST' }); diff --git a/web/cobrands/fixmystreet/assets.js b/web/cobrands/fixmystreet/assets.js index ad832e67f..25c1ac08e 100644 --- a/web/cobrands/fixmystreet/assets.js +++ b/web/cobrands/fixmystreet/assets.js @@ -596,8 +596,8 @@ function construct_layer_options(options, protocol) { } if (options.filter_key) { - // Add this filter to the layer, so it can potentially be used - // in the request (though only Bristol currently does this). + // Add this filter to the layer, so it can potentially be + // used in the request if non-HTTP WFS if (OpenLayers.Util.isArray(options.filter_value)) { layer_options.filter = new OpenLayers.Filter.Logical({ type: OpenLayers.Filter.Logical.OR, @@ -621,10 +621,11 @@ function construct_layer_options(options, protocol) { value: options.filter_value }); } - // Add a strategy filter to the layer, to filter the incoming results - // after they are received. Bristol does not need this, but has to ask - // for the filter data in its response so it doesn't then disappear. - layer_options.strategies.push(new OpenLayers.Strategy.Filter({filter: layer_options.filter})); + // If using HTTP WFS, add a strategy filter to the layer, + // to filter the incoming results after being received. + if (options.http_options) { + layer_options.strategies.push(new OpenLayers.Strategy.Filter({filter: layer_options.filter})); + } } return layer_options; diff --git a/web/cobrands/lincolnshire/assets.js b/web/cobrands/lincolnshire/assets.js index cb85f8fc6..ac1ae8cd5 100644 --- a/web/cobrands/lincolnshire/assets.js +++ b/web/cobrands/lincolnshire/assets.js @@ -5,15 +5,7 @@ if (!fixmystreet.maps) { } var defaults = { - http_options: { - url: "https://tilma.mysociety.org/mapserver/lincs", - params: { - SERVICE: "WFS", - VERSION: "1.1.0", - REQUEST: "GetFeature", - SRSNAME: "urn:ogc:def:crs:EPSG::3857" - } - }, + wfs_url: "https://tilma.mysociety.org/mapserver/lincs", asset_type: 'spot', max_resolution: 2.388657133579254, min_resolution: 0.5971642833948135, @@ -29,21 +21,13 @@ var defaults = { }; fixmystreet.assets.add(defaults, { - http_options: { - params: { - TYPENAME: "SL_Bollards" - } - }, + wfs_feature: "SL_Bollards", asset_category: "Bollards (lit)", asset_item: 'bollard' }); fixmystreet.assets.add(defaults, { - http_options: { - params: { - TYPENAME: "SL_Street_Light_Units" - } - }, + wfs_feature: "SL_Street_Light_Units", asset_category: "Street light", asset_item: 'street light', filter_key: 'Type', @@ -53,11 +37,7 @@ fixmystreet.assets.add(defaults, { }); fixmystreet.assets.add(defaults, { - http_options: { - params: { - TYPENAME: "SL_Street_Light_Units" - } - }, + wfs_feature: "SL_Street_Light_Units", asset_category: "Subway light", asset_item: 'light', filter_key: 'Type', @@ -86,11 +66,7 @@ function get_barrier_stylemap() { } fixmystreet.assets.add(defaults, { - http_options: { - params: { - TYPENAME: "Safety_Barriers" - } - }, + wfs_feature: "Safety_Barriers", asset_category: ["Roadside safety barrier", "Missing safety fence"], asset_item: 'barrier or fence', filter_key: 'Type', @@ -100,21 +76,13 @@ fixmystreet.assets.add(defaults, { }); fixmystreet.assets.add(defaults, { - http_options: { - params: { - TYPENAME: "LCC_Drainage-GulliesOffletsManholes" - } - }, + wfs_feature: "LCC_Drainage-GulliesOffletsManholes", asset_category: "Blocked drain", asset_item: 'drain' }); fixmystreet.assets.add(defaults, { - http_options: { - params: { - TYPENAME: "ST_All_Structures" - } - }, + wfs_feature: "ST_All_Structures", asset_category: "Damaged dyke, ditch or culvert", asset_item: 'culvert', filter_key: 'Type', @@ -124,21 +92,13 @@ fixmystreet.assets.add(defaults, { }); fixmystreet.assets.add(defaults, { - http_options: { - params: { - TYPENAME: "SL_Lit_Signs" - } - }, + wfs_feature: "SL_Lit_Signs", asset_category: "Sign (lit)", asset_item: 'street sign' }); fixmystreet.assets.add(defaults, { - http_options: { - params: { - TYPENAME: "ST_All_Structures" - } - }, + wfs_feature: "ST_All_Structures", asset_category: [ "Bridge", "Bridge or Structure" @@ -153,11 +113,7 @@ fixmystreet.assets.add(defaults, { }); fixmystreet.assets.add(defaults, { - http_options: { - params: { - TYPENAME: "Carriageway" - } - }, + wfs_feature: "Carriageway", asset_category: [ "Damaged/missing cats eye", "Damaged road edge, encroaches less than 100mm", @@ -176,11 +132,7 @@ fixmystreet.assets.add(defaults, { }); fixmystreet.assets.add(defaults, { - http_options: { - params: { - TYPENAME: "NSG" - } - }, + wfs_feature: "NSG", always_visible: true, non_interactive: true, usrn: { @@ -207,11 +159,7 @@ var llpg_stylemap = new OpenLayers.StyleMap({ }); fixmystreet.assets.add(defaults, { - http_options: { - params: { - TYPENAME: "LLPG" - } - }, + wfs_feature: "LLPG", // LLPG is only to be shown when fully zoomed in max_resolution: 0.5971642833948135, stylemap: llpg_stylemap, |