From c93ebfa26b864c26b9219b59f6676371e434ea0a Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Thu, 6 Jul 2017 14:04:23 +0100 Subject: [Oxfordshire] Separate defect creation from state. Revert the behaviour from 36baff2d, so that everyone can use the 'action scheduled' state, and instead if someone with report_instruct permission has the state set to 'action scheduled', add an extra mandatory question asking whether they want to raise a defect or not. --- perllib/FixMyStreet/App/Controller/Report.pm | 19 ++++---- t/app/controller/report_inspect.t | 52 +++++++++++----------- templates/web/base/report/_inspect.html | 11 +++++ .../report/inspect/state_groups_select.html | 5 +-- web/cobrands/fixmystreet/staff.js | 12 +++++ 5 files changed, 59 insertions(+), 40 deletions(-) diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm index ef10dc32e..81abba3ac 100644 --- a/perllib/FixMyStreet/App/Controller/Report.pm +++ b/perllib/FixMyStreet/App/Controller/Report.pm @@ -367,15 +367,9 @@ sub inspect : Private { if ( $problem->state ne $old_state ) { $c->forward( '/admin/log_edit', [ $problem->id, 'problem', 'state_change' ] ); - # If the state has been changed by an inspector, consider the - # report to be inspected. - unless ($problem->get_extra_metadata('inspected')) { - $problem->set_extra_metadata( inspected => 1 ); - $c->forward( '/admin/log_edit', [ $problem->id, 'problem', 'inspected' ] ); - my $state = $problem->state; - $reputation_change = 1 if $c->cobrand->reputation_increment_states->{$state}; - $reputation_change = -1 if $c->cobrand->reputation_decrement_states->{$state}; - } + my $state = $problem->state; + $reputation_change = 1 if $c->cobrand->reputation_increment_states->{$state}; + $reputation_change = -1 if $c->cobrand->reputation_decrement_states->{$state}; # If an inspector has changed the state, subscribe them to # updates @@ -386,6 +380,13 @@ sub inspect : Private { }; $problem->user->create_alert($problem->id, $options); } + + # If the state has been changed to action scheduled and they've said + # they want to raise a defect, consider the report to be inspected. + if ($problem->state eq 'action scheduled' && $c->get_param('raise_defect') && !$problem->get_extra_metadata('inspected')) { + $problem->set_extra_metadata( inspected => 1 ); + $c->forward( '/admin/log_edit', [ $problem->id, 'problem', 'inspected' ] ); + } } if ( !$c->forward( '/admin/report_edit_location', [ $problem ] ) ) { diff --git a/t/app/controller/report_inspect.t b/t/app/controller/report_inspect.t index e3d24e9a9..481d8dc87 100644 --- a/t/app/controller/report_inspect.t +++ b/t/app/controller/report_inspect.t @@ -65,13 +65,15 @@ FixMyStreet::override_config { }; subtest "test inspect & instruct submission" => sub { - $report->unset_extra_metadata('inspected'); + $user->user_body_permissions->create({ body => $oxon, permission_type => 'report_instruct' }); $report->state('confirmed'); $report->update; - $report->inspection_log_entry->delete; my $reputation = $report->user->get_extra_metadata("reputation"); $mech->get_ok("/report/$report_id"); - $mech->submit_form_ok({ button => 'save', with_fields => { public_update => "This is a public update.", include_update => "1", state => 'action scheduled' } }); + $mech->submit_form_ok({ button => 'save', with_fields => { + public_update => "This is a public update.", include_update => "1", + state => 'action scheduled', raise_defect => 1, + } }); $report->discard_changes; is $report->comments->first->text, "This is a public update.", 'Update was created'; is $report->get_extra_metadata('inspected'), 1, 'report marked as inspected'; @@ -203,46 +205,42 @@ FixMyStreet::override_config { }; subtest "test positive reputation" => sub { + $user->user_body_permissions->create({ body => $oxon, permission_type => 'report_instruct' }); $report->unset_extra_metadata('inspected'); $report->update; $report->inspection_log_entry->delete if $report->inspection_log_entry; my $reputation = $report->user->get_extra_metadata("reputation") || 0; $mech->get_ok("/report/$report_id"); - $mech->submit_form_ok({ button => 'save', with_fields => { state => 'action scheduled', include_update => undef } }); + $mech->submit_form_ok({ button => 'save', with_fields => { + state => 'in progress', include_update => undef, + } }); $report->discard_changes; - is $report->get_extra_metadata('inspected'), 1, 'report marked as inspected'; + is $report->get_extra_metadata('inspected'), undef, 'report not marked as inspected'; + + $mech->submit_form_ok({ button => 'save', with_fields => { + state => 'action scheduled', include_update => undef, + } }); + $report->discard_changes; + is $report->get_extra_metadata('inspected'), undef, 'report not marked as inspected'; is $report->user->get_extra_metadata('reputation'), $reputation+1, "User reputation was increased"; + + $mech->submit_form_ok({ button => 'save', with_fields => { + state => 'action scheduled', include_update => undef, + raise_defect => 1, + } }); + $report->discard_changes; + is $report->get_extra_metadata('inspected'), 1, 'report marked as inspected'; }; subtest "Oxfordshire-specific traffic management options are shown" => sub { $report->update({ state => 'confirmed' }); $mech->get_ok("/report/$report_id"); - $mech->submit_form_ok({ button => 'save', with_fields => { traffic_information => 'Signs and Cones', state => 'Investigating', include_update => undef } }); + $mech->submit_form_ok({ button => 'save', with_fields => { traffic_information => 'Signs and Cones', state => 'Action Scheduled', include_update => undef } }); $report->discard_changes; - is $report->state, 'investigating', 'report state changed'; + is $report->state, 'action scheduled', 'report state changed'; is $report->get_extra_metadata('traffic_information'), 'Signs and Cones', 'report data changed'; }; - subtest "Action scheduled only shown appropriately" => sub { - $report->update({ state => 'confirmed' }); - $mech->get_ok("/report/$report_id"); - $mech->content_lacks('action scheduled'); - - # If the report is already in that state, though, we should show it - $report->update({ state => 'action scheduled' }); - $mech->get_ok("/report/$report_id"); - $mech->content_unlike(qr/\s*'); - - $user->user_body_permissions->create({ body => $oxon, permission_type => 'report_instruct' }); - $mech->get_ok("/report/$report_id"); - $mech->content_like(qr/\s*\s*