diff options
author | Struan Donald <struan@exo.org.uk> | 2018-10-23 17:01:40 +0100 |
---|---|---|
committer | Struan Donald <struan@exo.org.uk> | 2018-11-12 11:24:09 +0000 |
commit | 9e9460b8ff4bdccf9dc0166331688f2f0818b29f (patch) | |
tree | 4d07226516cfc61782d12a4f76480fb559509a88 | |
parent | 6c2fa7f8e55283d1595ac7f293de5266f2b8fed7 (diff) |
add report_mark_private permission
Allows user's to see the inspector panel to mark reports as Private, and
also to view those non-public reports. Useful for call centre staff who
want to record private reports but don't need to other permissions.
Fixes mysociety/fixmystreet-commercial#1213
-rw-r--r-- | CHANGELOG.md | 1 | ||||
-rw-r--r-- | docs/_includes/admin-tasks-content.md | 23 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Report.pm | 6 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Reports.pm | 28 | ||||
-rw-r--r-- | perllib/FixMyStreet/Cobrand/Default.pm | 1 | ||||
-rw-r--r-- | perllib/FixMyStreet/DB/ResultSet/Problem.pm | 3 | ||||
-rw-r--r-- | t/app/controller/admin/users.t | 1 | ||||
-rw-r--r-- | t/app/controller/around.t | 51 | ||||
-rw-r--r-- | t/app/controller/report_inspect.t | 32 | ||||
-rw-r--r-- | t/app/controller/report_new.t | 44 | ||||
-rw-r--r-- | t/app/controller/reports.t | 41 | ||||
-rw-r--r-- | templates/web/base/report/new/form_user_loggedin.html | 2 |
12 files changed, 219 insertions, 14 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index a1d45dc70..9443c7afa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Simplify /auth sign in page. #2208 - Admin improvements: - Allow moderation to potentially change category. #2320 + - Add Mark/View private reports permission #2306 - Open311 improvements: - Fix bug in contact group handling. #2323 diff --git a/docs/_includes/admin-tasks-content.md b/docs/_includes/admin-tasks-content.md index 2fe85ad4c..712da5b1e 100644 --- a/docs/_includes/admin-tasks-content.md +++ b/docs/_includes/admin-tasks-content.md @@ -250,6 +250,29 @@ were made during the period when the user was banned — these remain unsent. </div> +<div class="admin-task" markdown="1" id="create-reports-private"> + +### Creating/Viewing private reports + +<span class="admin-task__permissions">Permissions required: User must be marked +as staff; one or more of ‘View/Mark private reports’ and ‘Markup problem +details’ must be ticked.</span> + +If a you are creating a report that has to contain information that should +not be make public, e.g. Names and addresses, then you can create a +Private report. This will still be visible to staff members with the +relevant permissions and will be sent as normal but will not be visible +to members of the public. + +You can also mark an existing report as private by visiting the report +page while logged in, checking "Private" and clicking "Save Changes". + +In such cases, staff should make a new report just as a member of the public would — see ‘[The +citizen’s experience](/pro-manual/citizens-experience/)'. Those with the appropriate permissions +will see a "Private" checkbox underneath the user details which they should select. + +</div> + <div class="admin-task" markdown="1" id="correct-reporter-errors"> ### Correcting reporter errors diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm index 5718bc021..1951028c8 100644 --- a/perllib/FixMyStreet/App/Controller/Report.pm +++ b/perllib/FixMyStreet/App/Controller/Report.pm @@ -85,7 +85,7 @@ sub display :PathPart('') :Chained('id') :Args(0) { $c->forward( 'format_problem_for_display' ); my $permissions = $c->stash->{_permissions} ||= $c->forward( 'check_has_permission_to', - [ qw/report_inspect report_edit_category report_edit_priority/ ] ); + [ qw/report_inspect report_edit_category report_edit_priority report_mark_private/ ] ); if (any { $_ } values %$permissions) { $c->stash->{template} = 'report/inspect.html'; $c->forward('inspect'); @@ -131,8 +131,8 @@ sub load_problem_or_display_error : Private { # Creator, and inspection users can see non_public reports $c->stash->{problem} = $problem; my $permissions = $c->stash->{_permissions} = $c->forward( 'check_has_permission_to', - [ qw/report_inspect report_edit_category report_edit_priority/ ] ); - if ( !$c->user || ($c->user->id != $problem->user->id && !$permissions->{report_inspect}) ) { + [ 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})) ) { $c->detach( '/page_error_403_access_denied', [ sprintf(_('That report cannot be viewed on %s.'), $c->stash->{site_name}) ] diff --git a/perllib/FixMyStreet/App/Controller/Reports.pm b/perllib/FixMyStreet/App/Controller/Reports.pm index 1ca4cbb09..2508b822f 100644 --- a/perllib/FixMyStreet/App/Controller/Reports.pm +++ b/perllib/FixMyStreet/App/Controller/Reports.pm @@ -556,13 +556,9 @@ sub load_and_group_problems : Private { state => [ keys %$states ] }; - my $body = $c->stash->{body}; # Might be undef + $c->forward('check_non_public_reports_permission', [ $where ] ); - if ($c->user_exists && ($c->user->is_superuser || ($body && $c->user->has_permission_to('report_inspect', $body->id)))) { - # See all reports, no restriction - } else { - $where->{non_public} = 0; - } + my $body = $c->stash->{body}; # Might be undef my $filter = { order_by => $c->stash->{sort_order}, @@ -653,6 +649,26 @@ sub load_and_group_problems : Private { return 1; } + +sub check_non_public_reports_permission : Private { + my ($self, $c, $where) = @_; + + if ( $c->user_exists ) { + return if $c->user->is_super_user; + + my $body = $c->stash->{body}; + + my $user_has_permission = $body && ( + $c->user->has_permission_to('report_inspect', $body->id) || + $c->user->has_permission_to('report_mark_private', $body->id) + ); + + $where->{non_public} = 0 unless $user_has_permission; + } else { + $where->{non_public} = 0; + } +} + sub redirect_index : Private { my ( $self, $c ) = @_; my $url = '/reports'; diff --git a/perllib/FixMyStreet/Cobrand/Default.pm b/perllib/FixMyStreet/Cobrand/Default.pm index e7ab77bfd..d71b96392 100644 --- a/perllib/FixMyStreet/Cobrand/Default.pm +++ b/perllib/FixMyStreet/Cobrand/Default.pm @@ -717,6 +717,7 @@ sub available_permissions { report_edit => _("Edit reports"), report_edit_category => _("Edit report category"), # future use report_edit_priority => _("Edit report priority"), # future use + report_mark_private => _("View/Mark private reports"), report_inspect => _("Markup problem details"), report_instruct => _("Instruct contractors to fix problems"), # future use planned_reports => _("Manage shortlist"), diff --git a/perllib/FixMyStreet/DB/ResultSet/Problem.pm b/perllib/FixMyStreet/DB/ResultSet/Problem.pm index cc28e4c33..0a180f8e3 100644 --- a/perllib/FixMyStreet/DB/ResultSet/Problem.pm +++ b/perllib/FixMyStreet/DB/ResultSet/Problem.pm @@ -30,7 +30,8 @@ sub non_public_if_possible { if ($c->user_exists) { if ($c->user->is_superuser) { # See all reports, no restriction - } elsif ($c->user->has_body_permission_to('report_inspect')) { + } elsif ($c->user->has_body_permission_to('report_inspect') || + $c->user->has_body_permission_to('report_mark_private')) { $params->{'-or'} = [ non_public => 0, $rs->body_query($c->user->from_body->id), diff --git a/t/app/controller/admin/users.t b/t/app/controller/admin/users.t index d9e984454..903ec3104 100644 --- a/t/app/controller/admin/users.t +++ b/t/app/controller/admin/users.t @@ -168,6 +168,7 @@ for my $test ( my %default_perms = ( "permissions[moderate]" => undef, "permissions[planned_reports]" => undef, + "permissions[report_mark_private]" => undef, "permissions[report_edit]" => undef, "permissions[report_edit_category]" => undef, "permissions[report_edit_priority]" => undef, diff --git a/t/app/controller/around.t b/t/app/controller/around.t index 8eeafec7f..18281396a 100644 --- a/t/app/controller/around.t +++ b/t/app/controller/around.t @@ -104,7 +104,10 @@ foreach my $test ( }; } -my @edinburgh_problems = $mech->create_problems_for_body( 5, 2651, 'Around page', { +my $body_edin_id = $mech->create_body_ok(2651, 'City of Edinburgh Council')->id; +my $body_west_id = $mech->create_body_ok(2504, 'Westminster City Council')->id; + +my @edinburgh_problems = $mech->create_problems_for_body( 5, $body_edin_id, 'Around page', { postcode => 'EH1 1BB', latitude => 55.9519637512, longitude => -3.17492254484, @@ -128,7 +131,7 @@ subtest 'check non public reports are not displayed on around page' => sub { $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } }, "good location" ); }; - $mech->content_contains( 'Around page Test 3 for 2651', + $mech->content_contains( "Around page Test 3 for $body_edin_id", 'problem to be marked non public visible' ); my $private = $edinburgh_problems[2]; @@ -142,10 +145,52 @@ subtest 'check non public reports are not displayed on around page' => sub { $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } }, "good location" ); }; - $mech->content_lacks( 'Around page Test 3 for 2651', + $mech->content_lacks( "Around page Test 3 for $body_edin_id", 'problem marked non public is not visible' ); }; +for my $permission ( qw/ report_inspect report_mark_private/ ) { + subtest 'check non public reports are displayed on around page with $permission permission' => sub { + my $body = FixMyStreet::DB->resultset('Body')->find( $body_edin_id ); + my $body2 = FixMyStreet::DB->resultset('Body')->find( $body_west_id ); + my $user = $mech->log_in_ok( 'test@example.com' ); + $user->user_body_permissions->delete(); + $user->update({ from_body => $body }); + $user->user_body_permissions->find_or_create({ + body => $body, + permission_type => $permission, + }); + + $mech->get_ok('/'); + FixMyStreet::override_config { + ALLOWED_COBRANDS => [ { 'fixmystreet' => '.' } ], + MAPIT_URL => 'http://mapit.uk/', + }, sub { + $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } }, + "good location" ); + }; + $mech->content_contains( "Around page Test 3 for $body_edin_id", + 'problem marked non public is visible' ); + + $user->user_body_permissions->delete(); + $user->update({ from_body => $body2 }); + $user->user_body_permissions->find_or_create({ + body => $body2, + permission_type => $permission, + }); + + $mech->get_ok('/'); + FixMyStreet::override_config { + ALLOWED_COBRANDS => [ { 'fixmystreet' => '.' } ], + MAPIT_URL => 'http://mapit.uk/', + }, sub { + $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } }, + "good location" ); + }; + $mech->content_lacks( "Around page Test 3 for $body_edin_id", + 'problem marked non public is not visible' ); + }; +} my $body = $mech->create_body_ok(2237, "Oxfordshire"); diff --git a/t/app/controller/report_inspect.t b/t/app/controller/report_inspect.t index e3ca33f0f..397dd1b00 100644 --- a/t/app/controller/report_inspect.t +++ b/t/app/controller/report_inspect.t @@ -57,12 +57,22 @@ FixMyStreet::override_config { subtest "test inspect page" => sub { $mech->get_ok("/report/$report_id"); $mech->content_lacks('Save changes'); + $mech->content_lacks('Private'); + $mech->content_lacks('Priority'); + $mech->content_lacks('Traffic management'); + $mech->content_lacks('/admin/report_edit/'.$report_id.'">admin</a>)'); + + $user->user_body_permissions->create({ body => $oxon, permission_type => 'report_mark_private' }); + $mech->get_ok("/report/$report_id"); + $mech->content_contains('Private'); + $mech->content_contains('Save changes'); $mech->content_lacks('Priority'); $mech->content_lacks('Traffic management'); $mech->content_lacks('/admin/report_edit/'.$report_id.'">admin</a>)'); $user->user_body_permissions->create({ body => $oxon, permission_type => 'report_edit_priority' }); $mech->get_ok("/report/$report_id"); + $mech->content_contains('Private'); $mech->content_contains('Save changes'); $mech->content_contains('Priority'); $mech->content_lacks('Traffic management'); @@ -71,6 +81,7 @@ FixMyStreet::override_config { $user->user_body_permissions->create({ body => $oxon, permission_type => 'report_inspect' }); $mech->get_ok("/report/$report_id"); $mech->content_contains('Save changes'); + $mech->content_contains('Private'); $mech->content_contains('Priority'); $mech->content_contains('Traffic management'); $mech->content_lacks('/admin/report_edit/'.$report_id.'">admin</a>)'); @@ -91,7 +102,28 @@ FixMyStreet::override_config { $user->update({is_superuser => 0}); }; + subtest "test mark private submission" => sub { + $user->user_body_permissions->delete; + $user->user_body_permissions->create({ body => $oxon, permission_type => 'report_mark_private' }); + + $mech->get_ok("/report/$report_id"); + $mech->submit_form_ok({ button => 'save', with_fields => { non_public => 1 } }); + $report->discard_changes; + my $alert = FixMyStreet::App->model('DB::Alert')->find( + { user => $user, alert_type => 'new_updates', confirmed => 1, } + ); + + is $report->state, 'confirmed', 'report state not changed'; + ok $report->non_public, 'report not public'; + ok !defined( $alert ) , 'not signed up for alerts'; + + $report->update( { non_public => 0 } ); + }; subtest "test basic inspect submission" => sub { + $user->user_body_permissions->create({ body => $oxon, permission_type => 'report_edit_priority' }); + $user->user_body_permissions->create({ body => $oxon, permission_type => 'report_inspect' }); + + $mech->get_ok("/report/$report_id"); $mech->submit_form_ok({ button => 'save', with_fields => { traffic_information => 'Yes', state => 'Action scheduled', include_update => undef } }); $report->discard_changes; my $alert = FixMyStreet::App->model('DB::Alert')->find( diff --git a/t/app/controller/report_new.t b/t/app/controller/report_new.t index 86d058287..6dce6711b 100644 --- a/t/app/controller/report_new.t +++ b/t/app/controller/report_new.t @@ -1919,6 +1919,50 @@ subtest "extra google analytics code displayed on email confirmation problem cre }; }; + +my $private_perms = $mech->create_user_ok('private_perms@example.org', name => 'private', from_body => $bodies[0]); +subtest "report_mark_private allows users to mark reports as private" => sub { + FixMyStreet::override_config { + ALLOWED_COBRANDS => [ { fixmystreet => '.' } ], + BASE_URL => 'https://www.fixmystreet.com', + MAPIT_URL => 'http://mapit.uk/', + }, sub { + $mech->log_out_ok; + + $private_perms->user_body_permissions->find_or_create({ + body => $bodies[0], + permission_type => 'report_mark_private', + }); + + $mech->log_in_ok('private_perms@example.org'); + $mech->get_ok('/'); + $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB' } }, + "submit location" ); + $mech->follow_link_ok( + { text_regex => qr/skip this step/i, }, + "follow 'skip this step' link" + ); + + $mech->submit_form_ok( + { + with_fields => { + title => "Private report", + detail => 'Private report details.', + photo1 => '', + name => 'Joe Bloggs', + may_show_name => '1', + phone => '07903 123 456', + category => 'Trees', + non_public => 1, + } + }, + "submit good details" + ); + + $mech->content_contains('Great work. Now spread the word', 'shown confirmation page'); + } +}; + my $inspector = $mech->create_user_ok('inspector@example.org', name => 'inspector', from_body => $bodies[0]); foreach my $test ( { non_public => 0 }, diff --git a/t/app/controller/reports.t b/t/app/controller/reports.t index 66af2778d..3ba90c062 100644 --- a/t/app/controller/reports.t +++ b/t/app/controller/reports.t @@ -215,6 +215,47 @@ is scalar @$problems, 4, 'only public problems are displayed'; $mech->content_lacks('All reports Test 3 for ' . $body_west_id, 'non public problem is not visible'); +for my $permission( qw/ report_inspect report_mark_private / ) { + subtest "user with $permission permission can see non public reports" => sub { + my $body = FixMyStreet::DB->resultset('Body')->find( $body_west_id ); + my $body2 = FixMyStreet::DB->resultset('Body')->find( $body_edin_id ); + my $user = $mech->log_in_ok( 'test@example.com' ); + $user->user_body_permissions->delete(); + $user->update({ from_body => $body }); + $user->user_body_permissions->find_or_create({ + body => $body, + permission_type => $permission, + }); + + FixMyStreet::override_config { + MAPIT_URL => 'http://mapit.uk/', + }, sub { + $mech->get_ok('/reports/Westminster'); + }; + $problems = $mech->extract_problem_list; + is scalar @$problems, 5, 'only public problems are displayed'; + + $mech->content_contains('All reports Test 3 for ' . $body_west_id, 'non public problem is visible'); + + $user->user_body_permissions->delete(); + $user->update({ from_body => $body2 }); + $user->user_body_permissions->find_or_create({ + body => $body2, + permission_type => $permission, + }); + + FixMyStreet::override_config { + MAPIT_URL => 'http://mapit.uk/', + }, sub { + $mech->get_ok('/reports/Westminster'); + }; + $problems = $mech->extract_problem_list; + is scalar @$problems, 4, 'only public problems are displayed'; + + $mech->content_lacks('All reports Test 3 for ' . $body_west_id, 'non public problem is not visible'); + }; +} + # No change to numbers if report is non-public FixMyStreet::override_config { TEST_DASHBOARD_DATA => $data, diff --git a/templates/web/base/report/new/form_user_loggedin.html b/templates/web/base/report/new/form_user_loggedin.html index 7f81764be..41cbc4a54 100644 --- a/templates/web/base/report/new/form_user_loggedin.html +++ b/templates/web/base/report/new/form_user_loggedin.html @@ -57,7 +57,7 @@ <input class="form-control" type="text" value="[% report.user.email | html %]" name="email" id="form_email"> [% END %] -[% IF c.user.has_permission_to("report_inspect", bodies.keys) %] +[% IF c.user.has_permission_to("report_inspect", bodies.keys) OR c.user.has_permission_to("report_mark_private", bodies.keys) %] <div class="checkbox-group"> <input type="checkbox" name="non_public" id="form_non_public" value="1"[% ' checked' IF report.non_public %]> <label class="inline" for="form_non_public">[% loc('Private') %] </label> |