aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--CHANGELOG.md2
-rw-r--r--perllib/FixMyStreet/App/Controller/Report.pm4
-rw-r--r--perllib/FixMyStreet/App/Controller/Root.pm4
-rw-r--r--perllib/FixMyStreet/Cobrand/Bexley.pm43
-rw-r--r--perllib/FixMyStreet/Cobrand/Rutland.pm4
-rw-r--r--perllib/Open311.pm11
-rw-r--r--t/app/controller/contact_enquiry.t77
-rw-r--r--t/app/controller/report_display.t4
-rw-r--r--t/cobrand/bexley.t75
-rwxr-xr-xtemplates/web/base/errors/generic.html5
-rw-r--r--web/cobrands/fixmystreet-uk-councils/council_validation_rules.js4
-rw-r--r--web/cobrands/northamptonshire/assets.js16
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"
]
});