diff options
-rw-r--r-- | CHANGELOG.md | 2 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Report.pm | 4 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Root.pm | 4 | ||||
-rw-r--r-- | perllib/FixMyStreet/Cobrand/Bexley.pm | 43 | ||||
-rw-r--r-- | perllib/FixMyStreet/Cobrand/Rutland.pm | 4 | ||||
-rw-r--r-- | perllib/Open311.pm | 11 | ||||
-rw-r--r-- | t/app/controller/contact_enquiry.t | 77 | ||||
-rw-r--r-- | t/app/controller/report_display.t | 4 | ||||
-rw-r--r-- | t/cobrand/bexley.t | 75 | ||||
-rwxr-xr-x | templates/web/base/errors/generic.html | 5 | ||||
-rw-r--r-- | web/cobrands/fixmystreet-uk-councils/council_validation_rules.js | 4 | ||||
-rw-r--r-- | web/cobrands/northamptonshire/assets.js | 16 |
12 files changed, 195 insertions, 54 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index b18762648..993bb3535 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ ## Releases * Unreleased + - Front end improvements: + - Improved 403 message, especially for private reports. - Admin improvements: - Add new roles system, to group permissions and apply to users. - Bugfixes: diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm index f2f411635..9b90da161 100644 --- a/perllib/FixMyStreet/App/Controller/Report.pm +++ b/perllib/FixMyStreet/App/Controller/Report.pm @@ -1,5 +1,6 @@ package FixMyStreet::App::Controller::Report; +use utf8; use Moose; use namespace::autoclean; use JSON::MaybeXS; @@ -156,9 +157,10 @@ sub load_problem_or_display_error : Private { my $permissions = $c->stash->{_permissions} = $c->forward( 'check_has_permission_to', [ qw/report_inspect report_edit_category report_edit_priority report_mark_private / ] ); if ( !$c->user || ($c->user->id != $problem->user->id && !($permissions->{report_inspect} || $permissions->{report_mark_private})) ) { + my $url = '/auth?r=report/' . $problem->id; $c->detach( '/page_error_403_access_denied', - [ sprintf(_('That report cannot be viewed on %s.'), $c->stash->{site_name}) ] + [ sprintf(_('Sorry, you don’t have permission to do that. If you are the problem reporter, or a member of staff, please <a href="%s">sign in</a> to view this report.'), $url) ] ); } } diff --git a/perllib/FixMyStreet/App/Controller/Root.pm b/perllib/FixMyStreet/App/Controller/Root.pm index 340c930c2..2c7e28e5f 100644 --- a/perllib/FixMyStreet/App/Controller/Root.pm +++ b/perllib/FixMyStreet/App/Controller/Root.pm @@ -122,7 +122,9 @@ sub page_error_410_gone : Private { sub page_error_403_access_denied : Private { my ( $self, $c, $error_msg ) = @_; - $c->detach('page_error', [ $error_msg || _("Sorry, you don't have permission to do that."), 403 ]); + $c->stash->{title} = _('Access denied'); + $error_msg ||= _("Sorry, you don't have permission to do that."); + $c->detach('page_error', [ $error_msg, 403 ]); } sub page_error_400_bad_request : Private { diff --git a/perllib/FixMyStreet/Cobrand/Bexley.pm b/perllib/FixMyStreet/Cobrand/Bexley.pm index d398bb670..d3787ef67 100644 --- a/perllib/FixMyStreet/Cobrand/Bexley.pm +++ b/perllib/FixMyStreet/Cobrand/Bexley.pm @@ -85,17 +85,44 @@ sub open311_post_send { # Check Open311 was successful return unless $row->external_id; - return unless $row->category eq 'Abandoned and untaxed vehicles' - || $row->category eq 'Dead animal'; + if ($row->category eq 'Abandoned and untaxed vehicles') { + my $burnt = $row->get_extra_field_value('burnt') || ''; + return unless $burnt eq 'Yes'; + } - my $mb = FixMyStreet->config('STAGING_SITE') ? 'digital-team' : 'P1sfromContactCentre'; - my $e = join('@', $mb, $self->admin_user_domain); - my $sender = FixMyStreet::SendReport::Email->new( to => [ [ $e, 'Bexley P1 email' ] ] ); + my @lighting = ( + 'Lamp post', + 'Light in multi-storey car park', + 'Light in outside car park', + 'Light in park or open space', + 'Traffic bollard', + 'Traffic sign light', + 'Underpass light', + 'Zebra crossing light', + ); + my %lighting = map { $_ => 1 } @lighting; + + my $emails = $self->feature('open311_email') || return; + my $dangerous = $row->get_extra_field_value('dangerous') || ''; + my $reportType = $row->get_extra_field_value('reportType') || ''; + + my $p1_email = 0; + if ($row->category eq 'Parks and open spaces') { + $p1_email = 1 if $reportType =~ /locked in a park|Wild animal/; + $p1_email = 1 if $dangerous eq 'Yes' && $reportType =~ /Playgrounds|park furniture|gates are broken|Vandalism|Other/; + } else { + $p1_email = 1 if $dangerous eq 'Yes'; + } - if ($row->category eq 'Abandoned and untaxed vehicles') { - my ($burnt) = grep { $_->{name} eq 'burnt' } @{$row->get_extra_fields}; - return unless $burnt && $burnt->{value} eq 'Yes'; + my @to; + if ($row->category eq 'Abandoned and untaxed vehicles' || $row->category eq 'Dead animal' || $p1_email) { + push @to, [ $emails->{p1}, 'Bexley P1 email' ] if $emails->{p1}; + } + if ($lighting{$row->category}) { + push @to, [ $emails->{lighting}, 'FixMyStreet Bexley Street Lighting' ] if $emails->{lighting}; } + return unless @to; + my $sender = FixMyStreet::SendReport::Email->new( to => \@to ); $self->open311_config($row); # Populate NSGRef again if needed diff --git a/perllib/FixMyStreet/Cobrand/Rutland.pm b/perllib/FixMyStreet/Cobrand/Rutland.pm index 407d43d14..97bcfe637 100644 --- a/perllib/FixMyStreet/Cobrand/Rutland.pm +++ b/perllib/FixMyStreet/Cobrand/Rutland.pm @@ -12,6 +12,10 @@ sub council_url { return 'rutland'; } sub report_validation { my ($self, $report, $errors) = @_; + if ( length( $report->title ) > 254 ) { + $errors->{title} = sprintf( _('Summaries are limited to %s characters in length. Please shorten your summary'), 254 ); + } + if ( length( $report->name ) > 40 ) { $errors->{name} = sprintf( _('Names are limited to %d characters in length.'), 40 ); } diff --git a/perllib/Open311.pm b/perllib/Open311.pm index a902a7213..ebf3ee987 100644 --- a/perllib/Open311.pm +++ b/perllib/Open311.pm @@ -8,6 +8,7 @@ use XML::Simple; use LWP::Simple; use LWP::UserAgent; use DateTime::Format::W3CDTF; +use Encode; use HTTP::Request::Common qw(GET POST); use FixMyStreet::Cobrand; use FixMyStreet::DB; @@ -495,11 +496,19 @@ sub _request { my $debug_request = $method . ' ' . $uri->as_string . "\n\n"; my $req = do { + $params = { + map { + my $value = $params->{$_}; + $_ => ref $value eq 'ARRAY' + ? [ map { encode('UTF-8', $_) } @$value ] + : encode('UTF-8', $value) + } keys %$params + }; if ($method eq 'GET') { $uri->query_form( $params ); GET $uri->as_string; } elsif ($method eq 'POST') { - if ($uploads) { + if ($uploads && %$uploads) { # HTTP::Request::Common needs to be constructed slightly # differently if there are files to upload. diff --git a/t/app/controller/contact_enquiry.t b/t/app/controller/contact_enquiry.t index fe3962c8e..3f2989695 100644 --- a/t/app/controller/contact_enquiry.t +++ b/t/app/controller/contact_enquiry.t @@ -1,6 +1,10 @@ +use utf8; +use Encode; use FixMyStreet::TestMech; use Path::Tiny; use File::Temp 'tempdir'; +use FixMyStreet::Script::Reports; +use Test::MockModule; # disable info logs for this test run FixMyStreet::App->log->disable('info'); @@ -8,7 +12,12 @@ END { FixMyStreet::App->log->enable('info'); } my $mech = FixMyStreet::TestMech->new; -my $body = $mech->create_body_ok( 2483, 'Hounslow Borough Council' ); +my $body = $mech->create_body_ok( 2483, 'Hounslow Borough Council', { + send_method => 'Open311', + endpoint => 'endpoint', + api_key => 'key', + jurisdiction => 'hounslow', +}); my $contact = $mech->create_contact_ok( body_id => $body->id, category => 'General Enquiry', @@ -163,17 +172,22 @@ ok $sample_jpeg->exists, "sample image $sample_jpeg exists"; my $sample_pdf = path(__FILE__)->parent->child("sample.pdf"); ok $sample_pdf->exists, "sample PDF $sample_pdf exists"; -subtest "Check photo & file upload works" => sub { - my $UPLOAD_DIR = tempdir( CLEANUP => 1 ); - - FixMyStreet::override_config { - ALLOWED_COBRANDS => [ 'hounslow' ], - MAPIT_URL => 'http://mapit.uk/', - PHOTO_STORAGE_BACKEND => 'FileSystem', - PHOTO_STORAGE_OPTIONS => { - UPLOAD_DIR => $UPLOAD_DIR, - }, - }, sub { +my $UPLOAD_DIR = tempdir( CLEANUP => 1 ); + +FixMyStreet::override_config { + ALLOWED_COBRANDS => [ 'hounslow' ], + STAGING_FLAGS => { send_reports => 1 }, + MAPIT_URL => 'http://mapit.uk/', + PHOTO_STORAGE_BACKEND => 'FileSystem', + PHOTO_STORAGE_OPTIONS => { + UPLOAD_DIR => $UPLOAD_DIR, + }, +}, sub { + + my $pdf_hash = '90f7a64043fb458d58de1a0703a6355e2856b15e.pdf'; + my $image_hash = '74e3362283b6ef0c48686fb0e161da4043bbcc97.jpeg'; + + subtest "Check photo & file upload works" => sub { my $problems = FixMyStreet::App->model('DB::Problem')->to_body( $body->id ); $problems->delete_all; @@ -190,7 +204,7 @@ subtest "Check photo & file upload works" => sub { name => 'Test User', username => 'testuser@example.org', category => 'Other', - detail => 'This is a general enquiry', + detail => encode_utf8('This is a general enquiry‽'), photo1 => [ $sample_jpeg, undef, Content_Type => 'image/jpeg' ], photo2 => [ $sample_pdf, undef, Content_Type => 'application/pdf' ], } @@ -199,7 +213,7 @@ subtest "Check photo & file upload works" => sub { is $problems->count, 1, 'problem created'; my $problem = $problems->first; - is $problem->detail, 'This is a general enquiry', 'problem has correct detail'; + is $problem->detail, 'This is a general enquiry‽', 'problem has correct detail'; is $problem->non_public, 1, 'problem created non_public'; is $problem->postcode, ''; is $problem->used_map, 0; @@ -207,20 +221,47 @@ subtest "Check photo & file upload works" => sub { is $problem->longitude, -0.35, 'Problem has correct longitude'; ok $problem->confirmed, 'problem confirmed'; - my $image_file = path($UPLOAD_DIR, '74e3362283b6ef0c48686fb0e161da4043bbcc97.jpeg'); + my $image_file = path($UPLOAD_DIR, $image_hash); ok $image_file->exists, 'Photo uploaded to temp'; my $photoset = $problem->get_photoset(); is $photoset->num_images, 1, 'Found just 1 image'; - is $photoset->data, '74e3362283b6ef0c48686fb0e161da4043bbcc97.jpeg'; + is $photoset->data, $image_hash; - my $pdf_file = path($UPLOAD_DIR, 'enquiry_files', '90f7a64043fb458d58de1a0703a6355e2856b15e.pdf'); + my $pdf_file = path($UPLOAD_DIR, 'enquiry_files', $pdf_hash); ok $pdf_file->exists, 'PDF uploaded to temp'; is_deeply $problem->get_extra_metadata('enquiry_files'), { - '90f7a64043fb458d58de1a0703a6355e2856b15e.pdf' => 'sample.pdf' + $pdf_hash => 'sample.pdf' }, 'enquiry file info stored OK'; + }; + subtest 'Check Open311 sending of the above report' => sub { + my $module = Test::MockModule->new('FixMyStreet::Cobrand::UKCouncils'); + $module->mock(get => sub ($) { '' }); + my $test_data = FixMyStreet::Script::Reports::send(); + my $req = $test_data->{test_req_used}; + my $found = 0; + foreach ($req->parts) { + my $cd = $_->header('Content-Disposition'); + if ($cd =~ /attribute\[description\]/) { + is decode_utf8($_->content), 'This is a general enquiry‽', 'Correct description'; + $found++; + } + if ($cd =~ /sample.pdf/) { + is $cd, 'form-data; name="file_' . $pdf_hash . '"; filename="sample.pdf"', 'Correct PDF header'; + is $_->header('Content-Type'), 'application/pdf', 'Correct PDF content type'; + $found++; + } + if ($cd =~ /jpeg/) { + is $cd, 'form-data; name="photo1"; filename="' . $image_hash . '"', 'Correct image header'; + is $_->header('Content-Type'), 'image/jpeg', 'Correct image content type'; + $found++; + } + } + is $found, 3, 'Found all tested headers'; }; + }; + done_testing(); diff --git a/t/app/controller/report_display.t b/t/app/controller/report_display.t index bde090dd1..bb5b0a72d 100644 --- a/t/app/controller/report_display.t +++ b/t/app/controller/report_display.t @@ -78,7 +78,7 @@ subtest "change report to non_public and check for 403 status" => sub { ok $mech->get("/report/$report_id"), "get '/report/$report_id'"; is $mech->res->code, 403, "access denied"; is $mech->uri->path, "/report/$report_id", "at /report/$report_id"; - $mech->content_contains('That report cannot be viewed on FixMyStreet.'); + $mech->content_contains('permission to do that. If you are the problem reporter'); ok $report->update( { non_public => 0 } ), 'make report public'; }; @@ -94,7 +94,7 @@ subtest "check owner of report can view non public reports" => sub { ok $mech->get("/report/$report_id"), "get '/report/$report_id'"; is $mech->res->code, 403, "access denied to user who is not report creator"; is $mech->uri->path, "/report/$report_id", "at /report/$report_id"; - $mech->content_contains('That report cannot be viewed on FixMyStreet.'); + $mech->content_contains('permission to do that. If you are the problem reporter'); $mech->log_out_ok; ok $report->update( { non_public => 0 } ), 'make report public'; }; diff --git a/t/cobrand/bexley.t b/t/cobrand/bexley.t index 2f74ac03a..07d7ed91b 100644 --- a/t/cobrand/bexley.t +++ b/t/cobrand/bexley.t @@ -30,11 +30,16 @@ my $mech = FixMyStreet::TestMech->new; my $body = $mech->create_body_ok(2494, 'London Borough of Bexley', { send_method => 'Open311', api_key => 'key', 'endpoint' => 'e', 'jurisdiction' => 'j' }); $mech->create_contact_ok(body_id => $body->id, category => 'Abandoned and untaxed vehicles', email => "ABAN"); +$mech->create_contact_ok(body_id => $body->id, category => 'Lamp post', email => "LAMP"); +$mech->create_contact_ok(body_id => $body->id, category => 'Parks and open spaces', email => "PARK"); +$mech->create_contact_ok(body_id => $body->id, category => 'Dead animal', email => "ANIM"); +$mech->create_contact_ok(body_id => $body->id, category => 'Something dangerous', email => "DANG"); FixMyStreet::override_config { ALLOWED_COBRANDS => [ 'bexley' ], MAPIT_URL => 'http://mapit.uk/', STAGING_FLAGS => { send_reports => 1, skip_checks => 0 }, + COBRAND_FEATURES => { open311_email => { bexley => { p1 => 'p1@bexley', lighting => 'thirdparty@notbexley.example.com' } } }, }, sub { subtest 'cobrand displays council name' => sub { @@ -48,24 +53,62 @@ FixMyStreet::override_config { $mech->content_contains('Bexley'); }; - my ($report) = $mech->create_problems_for_body(1, $body->id, 'On Road', { - category => 'Abandoned and untaxed vehicles', cobrand => 'bexley', - latitude => 51.408484, longitude => 0.074653, - }); - $report->set_extra_fields({ 'name' => 'burnt', description => 'Was it burnt?', 'value' => 'Yes' }); - $report->update; + foreach my $test ( + { category => 'Abandoned and untaxed vehicles', email => 1, code => 'ABAN', + extra => { 'name' => 'burnt', description => 'Was it burnt?', 'value' => 'Yes' } }, + { category => 'Abandoned and untaxed vehicles', code => 'ABAN', + extra => { 'name' => 'burnt', description => 'Was it burnt?', 'value' => 'No' } }, + { category => 'Dead animal', email => 1, code => 'ANIM' }, + { category => 'Something dangerous', email => 1, code => 'DANG', + extra => { 'name' => 'dangerous', description => 'Was it dangerous?', 'value' => 'Yes' } }, + { category => 'Something dangerous', code => 'DANG', + extra => { 'name' => 'dangerous', description => 'Was it dangerous?', 'value' => 'No' } }, + { category => 'Parks and open spaces', email => 1, code => 'PARK', + extra => { 'name' => 'reportType', description => 'Type of report', 'value' => 'Wild animal' } }, + { category => 'Parks and open spaces', code => 'PARK', + extra => { 'name' => 'reportType', description => 'Type of report', 'value' => 'Maintenance' } }, + { category => 'Parks and open spaces', code => 'PARK', + extra => { 'name' => 'dangerous', description => 'Was it dangerous?', 'value' => 'Yes' } }, + { category => 'Parks and open spaces', email => 1, code => 'PARK', + extra => [ + { 'name' => 'dangerous', description => 'Was it dangerous?', 'value' => 'Yes' }, + { 'name' => 'reportType', description => 'Type of report', 'value' => 'Vandalism' }, + ] }, + { category => 'Lamp post', code => 'LAMP', email => 'thirdparty', + extra => { 'name' => 'dangerous', description => 'Was it dangerous?', 'value' => 'No' } }, + { category => 'Lamp post', code => 'LAMP', email => 'p1.*thirdparty', + extra => { 'name' => 'dangerous', description => 'Was it dangerous?', 'value' => 'Yes' } }, + ) { + my ($report) = $mech->create_problems_for_body(1, $body->id, 'On Road', { + category => $test->{category}, cobrand => 'bexley', + latitude => 51.408484, longitude => 0.074653, + }); + if ($test->{extra}) { + $report->set_extra_fields(ref $test->{extra} eq 'ARRAY' ? @{$test->{extra}} : $test->{extra}); + $report->update; + } - subtest 'Server-side NSGRef included' => sub { - my $test_data = FixMyStreet::Script::Reports::send(); - my $req = $test_data->{test_req_used}; - my $c = CGI::Simple->new($req->content); - is $c->param('service_code'), 'ABAN'; - is $c->param('attribute[NSGRef]'), 'Road ID'; + subtest 'NSGRef and correct email config' => sub { + my $test_data = FixMyStreet::Script::Reports::send(); + my $req = $test_data->{test_req_used}; + my $c = CGI::Simple->new($req->content); + is $c->param('service_code'), $test->{code}; + is $c->param('attribute[NSGRef]'), 'Road ID'; - my $email = $mech->get_email; - like $email->header('To'), qr/"Bexley P1 email".*bexley/; - like $mech->get_text_body_from_email($email), qr/NSG Ref: Road ID/; - }; + if (my $t = $test->{email}) { + my $email = $mech->get_email; + if ($t eq 1) { + like $email->header('To'), qr/"Bexley P1 email".*bexley/; + } else { + like $email->header('To'), qr/$t/; + } + like $mech->get_text_body_from_email($email), qr/NSG Ref: Road ID/; + $mech->clear_emails_ok; + } else { + $mech->email_count_is(0); + } + }; + } }; diff --git a/templates/web/base/errors/generic.html b/templates/web/base/errors/generic.html index 241b310de..e5c2ca0c1 100755 --- a/templates/web/base/errors/generic.html +++ b/templates/web/base/errors/generic.html @@ -1,11 +1,12 @@ -[% INCLUDE 'header.html', bodyclass = 'fullwidthpage', title = loc('Error') %] +[% DEFAULT title = loc('Error') %] +[% INCLUDE 'header.html', bodyclass = 'fullwidthpage', title = title %] [% IF csrf_token ~%] <input type="hidden" name="token" value="[% csrf_token %]"> [% END ~%] <div class="confirmation-header confirmation-header--failure"> - <h1>[% loc('Error') %]</h1> + <h1>[% title %]</h1> <p>[% message %]</p> </div> diff --git a/web/cobrands/fixmystreet-uk-councils/council_validation_rules.js b/web/cobrands/fixmystreet-uk-councils/council_validation_rules.js index ee3dfde88..49c23db89 100644 --- a/web/cobrands/fixmystreet-uk-councils/council_validation_rules.js +++ b/web/cobrands/fixmystreet-uk-councils/council_validation_rules.js @@ -17,6 +17,10 @@ body_validation_rules = { 'Lincolnshire County Council': confirm_validation_rules, 'Bath and North East Somerset Council': confirm_validation_rules, 'Rutland County Council': { + title: { + required: true, + maxlength: 254 + }, name: { required: true, maxlength: 40 diff --git a/web/cobrands/northamptonshire/assets.js b/web/cobrands/northamptonshire/assets.js index fe726a1a8..07745ac8b 100644 --- a/web/cobrands/northamptonshire/assets.js +++ b/web/cobrands/northamptonshire/assets.js @@ -257,26 +257,28 @@ var layers = [ }, { "categories": [ "Bridge-Damaged/ Missing" ], - "item_name": "bridge", + "item_name": "bridge or right of way", "layer_name": "BRIDGES", "layer": 177, "version": "177.18-" }, { "categories": [ "Gate - Damaged/ Missing" ], + "item_name": "gate or right of way", "layer_name": "GATE", "layer": 181, "version": "181.3-" }, { "categories": [ "Stile-Damaged/Missing" ], + "item_name": "stile or right of way", "layer_name": "STILE", "layer": 185, "version": "185.3-" }, { "categories": [ "Sign/Waymarking - Damaged/Missing" ], - "item_name": "waymarking", + "item_name": "waymarking or right of way", "layer_name": "WAYMARK POST", "layer": 187, "version": "187.3-" @@ -485,8 +487,8 @@ var highways_style = new OpenLayers.Style({ fixmystreet.assets.add(northants_road_defaults, { protocol_class: OpenLayers.Protocol.Alloy, http_options: { - layerid: is_live ? 20 : 308, - layerVersion: is_live ? '20.123-' : '308.8-', + layerid: 20, + layerVersion: '20.143-', }, stylemap: new OpenLayers.StyleMap({ 'default': highways_style @@ -541,8 +543,12 @@ fixmystreet.assets.add(northants_road_defaults, { no_asset_msg_id: "#js-not-a-road", asset_item: 'right of way', asset_category: [ + "Bridge-Damaged/ Missing", + "Gate - Damaged/ Missing", "Livestock", - "Passage-Obstructed/Overgrown" + "Passage-Obstructed/Overgrown", + "Sign/Waymarking - Damaged/Missing", + "Stile-Damaged/Missing" ] }); |