diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/My.pm | 18 | ||||
| -rw-r--r-- | t/app/controller/my_planned.t | 152 | ||||
| -rw-r--r-- | templates/web/base/report/_item.html | 2 |
4 files changed, 165 insertions, 8 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index b4a57c9c6..419dff1d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ - Do not include blank updates in email alerts #1857 - Redirect inspectors correctly on creation in two-tier. - Report status filter All option works for body users #1845 + - Always allow reports to be removed from shortlist #1882 - Admin improvements: - Character length limit can be placed on report detailed information #1848 - Inspector panel shows nearest address if available #1850 diff --git a/perllib/FixMyStreet/App/Controller/My.pm b/perllib/FixMyStreet/App/Controller/My.pm index 9647fae9a..1693766ba 100644 --- a/perllib/FixMyStreet/App/Controller/My.pm +++ b/perllib/FixMyStreet/App/Controller/My.pm @@ -189,17 +189,21 @@ sub planned_change : Path('planned/change') { $c->go('planned') if grep { /^shortlist-(up|down|\d+)$/ } keys %{$c->req->params}; my $id = $c->get_param('id'); - $c->forward( '/report/load_problem_or_display_error', [ $id ] ); - my $add = $c->get_param('shortlist-add'); my $remove = $c->get_param('shortlist-remove'); - $c->detach('/page_error_403_access_denied', []) - unless $add || $remove; - if ($add) { + # we can't lookup the report for removing via load_problem_or_display_error + # as then there is no way to remove a report that has been hidden or moved + # to another body by a category change from the shortlist. + if ($remove) { + my $report = $c->model('DB::Problem')->find({ id => $id }) + or $c->detach( '/page_error_404_not_found', [ _('Unknown problem ID') ] ); + $c->user->remove_from_planned_reports($report); + } elsif ($add) { + $c->forward( '/report/load_problem_or_display_error', [ $id ] ); $c->user->add_to_planned_reports($c->stash->{problem}); - } elsif ($remove) { - $c->user->remove_from_planned_reports($c->stash->{problem}); + } else { + $c->detach('/page_error_403_access_denied', []); } if ($c->get_param('ajax')) { diff --git a/t/app/controller/my_planned.t b/t/app/controller/my_planned.t index 51ea0297e..67d59e148 100644 --- a/t/app/controller/my_planned.t +++ b/t/app/controller/my_planned.t @@ -5,6 +5,7 @@ $mech->get_ok('/my/planned'); is $mech->uri->path, '/auth', "got sent to the sign in page"; my $body = $mech->create_body_ok(2237, 'Oxfordshire County Council'); +my $body2 = $mech->create_body_ok(2421, 'Oxford City Council'); my ($problem) = $mech->create_problems_for_body(1, $body->id, 'Test Title'); $mech->get_ok($problem->url); @@ -77,4 +78,155 @@ subtest "POSTing multiple problems to my/planned/change adds all to shortlist" = $mech->text_contains('Shortlisted'); }; +subtest "re-ordering shortlist on non shortlist page redirect to shortlist" => sub { + $user->user_planned_reports->remove(); + my ($problem1) = $mech->create_problems_for_body(1, $body->id, 'New Problem'); + + $mech->get_ok($problem1->url); + my ($csrf) = $mech->content =~ /meta content="([^"]*)" name="csrf-token"/; + + $mech->post_ok( '/my/planned/change', { + id => $problem1->id, + 'shortlist-up' => 1, + token => $csrf, + }, + ); + + $mech->content_contains('Your shortlist'); +}; + +subtest "shortlist with no action is forbidden" => sub { + $user->user_planned_reports->remove(); + my ($problem1) = $mech->create_problems_for_body(1, $body->id, 'New Problem'); + + $mech->get_ok($problem1->url); + my ($csrf) = $mech->content =~ /meta content="([^"]*)" name="csrf-token"/; + + my $result = $mech->post( '/my/planned/change', { + id => $problem1->id, + token => $csrf, + }, + ); + + is $result->code, 403, '403 response if no action'; +}; + +subtest "cannot remove non-existant problems from shortlist" => sub { + $user->user_planned_reports->remove(); + my ($problem1) = $mech->create_problems_for_body(1, $body->id, 'New Problem'); + + $mech->get_ok($problem1->url); + my ($csrf) = $mech->content =~ /meta content="([^"]*)" name="csrf-token"/; + + my $result = $mech->post( '/my/planned/change', { + id => 999, + 'shortlist-remove' => 1, + token => $csrf, + }, + ); + + is $result->code, 404, 'removing missing report returns 404'; +}; + +subtest "can remove problems from shortlist" => sub { + $user->user_planned_reports->remove(); + my ($problem1, $problem2) = $mech->create_problems_for_body(2, $body->id, 'New Problem'); + + $mech->get_ok($problem1->url); + my ($csrf) = $mech->content =~ /meta content="([^"]*)" name="csrf-token"/; + + $mech->post_ok( '/my/planned/change_multiple', { + 'ids[]' => [ + $problem1->id, + $problem2->id, + ], + token => $csrf, + } + ); + + $mech->get_ok($problem1->url); + $mech->text_contains('Shortlisted'); + + ($csrf) = $mech->content =~ /meta content="([^"]*)" name="csrf-token"/; + + $mech->post_ok( '/my/planned/change', { + id => $problem1->id, + 'shortlist-remove' => 1, + token => $csrf, + }, + 'Removed problem from shortlist'); + + $mech->get_ok($problem1->url); + $mech->text_lacks('Shortlisted'); + $mech->text_contains('Shortlist'); + + # check cases where problem has changed body due + # to e.g. change of category + $problem2->update({ + bodies_str => $body2->id, + title => 'Other body problem', + }); + + $mech->get_ok('/my/planned'); + $mech->text_contains('Other body problem'); + + ($csrf) = $mech->content =~ /meta content="([^"]*)" name="csrf-token"/; + + $mech->post_ok( '/my/planned/change', { + id => $problem2->id, + 'shortlist-remove' => 1, + token => $csrf, + }, + 'Removed problem for other body from shortlist'); + + $mech->get_ok('/my/planned'); + $mech->text_lacks('Other body problem'); +}; + +FixMyStreet::override_config { + ALLOWED_COBRANDS => [ 'oxfordshire' ], + BASE_URL => 'http://oxfordshire.fixmystreet.site', +}, sub { + subtest "can remove problems not displayed in cobrand from shortlist" => sub { + $user->user_planned_reports->remove(); + my ($problem1) = $mech->create_problems_for_body(2, $body->id, 'New Problem'); + + $mech->get_ok($problem1->url); + my ($csrf) = $mech->content =~ /meta content="([^"]*)" name="csrf-token"/; + + $mech->post_ok( '/my/planned/change_multiple', { + 'ids[]' => [ + $problem1->id, + ], + token => $csrf, + } + ); + + $mech->get_ok('/my/planned'); + $mech->text_contains('New Problem'); + $mech->content_contains('Remove from shortlist'); + + $problem1->update({ + bodies_str => $body2->id, + }); + + $mech->get_ok('/my/planned'); + $mech->text_contains('New Problem'); + $mech->content_contains('Remove from shortlist'); + + ($csrf) = $mech->content =~ /meta content="([^"]*)" name="csrf-token"/; + + $mech->post_ok( '/my/planned/change', { + id => $problem1->id, + 'shortlist-remove' => 1, + token => $csrf, + ajax => 1, + }, + 'Removed problem not displayed in this cobrand'); + + $mech->get_ok('/my/planned'); + $mech->text_lacks('New Problem', 'Problem no longer in shortlist'); + }; +}; + done_testing(); diff --git a/templates/web/base/report/_item.html b/templates/web/base/report/_item.html index 9449ca55d..576f2b30f 100644 --- a/templates/web/base/report/_item.html +++ b/templates/web/base/report/_item.html @@ -2,7 +2,7 @@ [% PROCESS 'admin/report_blocks.html' ~%] [% END ~%] -[% IF c.user.has_permission_to('planned_reports', problem.bodies_str_ids) ~%] +[% IF c.user.has_permission_to('planned_reports', problem.bodies_str_ids) OR c.user.is_planned_report(problem) ~%] [% item_extra_class = "item-list__item--indented" ~%] [% item_action = BLOCK ~%] <input type="submit" value="1" |