Merge branch 'issues/forcouncils/191-action-scheduled-instruct'

6 files changed, 50 insertions, 29 deletions

diff --git a/perllib/FixMyStreet/App/Controller/Report/Update.pm b/perllib/FixMyStreet/App/Controller/Report/Update.pm
index 6e94547f3..81a50dfa2 100644
--- a/perllib/FixMyStreet/App/Controller/Report/Update.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/Update.pm
@@ -286,8 +286,6 @@ sub process_update : Private {
     }
 
     if ( $params{state} ) {
-        $params{state} = 'fixed - council'
-            if $params{state} eq 'fixed' && $c->user && $c->user->belongs_to_body( $update->problem->bodies_str );
         $update->problem_state( $params{state} );
     } else {
         # we do this so we have a record of the state of the problem at this point
@@ -344,14 +342,10 @@ sub check_for_errors : Private {
     my ( $self, $c ) = @_;
 
     # they have to be an authority user to update the state
-    if ( $c->get_param('state') ) {
+    if ( my $state = $c->get_param('state') ) {
         my $error = 0;
         $error = 1 unless $c->user && $c->user->belongs_to_body( $c->stash->{update}->problem->bodies_str );
-
-        my $state = $c->get_param('state');
-        $state = 'fixed - council' if $state eq 'fixed';
-        $error = 1 unless ( grep { $state eq $_ } ( FixMyStreet::DB::Result::Problem->council_states() ) );
-
+        $error = 1 unless grep { $state eq $_ } FixMyStreet::DB::Result::Problem->council_states();
         if ( $error ) {
             $c->stash->{errors} ||= [];
             push @{ $c->stash->{errors} }, _('There was a problem with your update. Please try again.');
diff --git a/t/app/controller/report_inspect.t b/t/app/controller/report_inspect.t
index 00b4e3b38..e1723b0cd 100644
--- a/t/app/controller/report_inspect.t
+++ b/t/app/controller/report_inspect.t
@@ -221,12 +221,32 @@ FixMyStreet::override_config {
     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 => 'Action Scheduled', include_update => undef } });
+        $mech->submit_form_ok({ button => 'save', with_fields => { traffic_information => 'Signs and Cones', state => 'Investigating', include_update => undef } });
         $report->discard_changes;
-        is $report->state, 'action scheduled', 'report state changed';
+        is $report->state, 'investigating', '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/<optgroup label="Scheduled">\s*<option value="action scheduled">Action Scheduled<\/option>/);
+        $mech->content_contains('<option selected value="action scheduled">Action Scheduled</option>');
+
+        $user->user_body_permissions->create({ body => $oxon, permission_type => 'report_instruct' });
+        $mech->get_ok("/report/$report_id");
+        $mech->content_like(qr/<optgroup label="Scheduled">\s*<option selected value="action scheduled">Action Scheduled<\/option>/);
+
+        $report->update({ state => 'confirmed' });
+        $mech->get_ok("/report/$report_id");
+        $mech->content_like(qr/<optgroup label="Scheduled">\s*<option value="action scheduled">Action Scheduled<\/option>/);
+    };
+
 };
 
 FixMyStreet::override_config {
diff --git a/t/app/controller/report_updates.t b/t/app/controller/report_updates.t
index de153978b..a1e26476f 100644
--- a/t/app/controller/report_updates.t
+++ b/t/app/controller/report_updates.t
@@ -134,7 +134,7 @@ for my $test (
 subtest "updates displayed on report with empty bodies_str" => sub {
     my $old_bodies_str = $report->bodies_str;
     $report->update({ bodies_str => undef });
-    $comment->update({ problem_state => 'fixed' , mark_open => 'false', mark_fixed => 'false' });
+    $comment->update({ problem_state => 'fixed - user' , mark_open => 'false', mark_fixed => 'false' });
 
     $mech->get_ok("/report/$report_id");
 
@@ -587,9 +587,10 @@ for my $test (
             name => $user->name,
             may_show_name => 1,
             update => 'Set state to fixed',
-            state => 'fixed',
+            state => 'fixed - council',
         },
         state => 'fixed - council',
+        meta => 'fixed',
     },
     {
         desc => 'from authority user marks report as action scheduled',
@@ -661,9 +662,10 @@ for my $test (
             name => $user->name,
             may_show_name => 1,
             update => 'Set state to fixed',
-            state => 'fixed',
+            state => 'fixed - council',
         },
         state => 'fixed - council',
+        meta => 'fixed',
         report_bodies => $body->id . ',2505',
     },
     {
@@ -672,9 +674,10 @@ for my $test (
           name => $user->name,
           may_show_name => 1,
           update => 'Set state to fixed',
-          state => 'fixed',
+          state => 'fixed - council',
       },
       state => 'fixed - council',
+      meta => 'fixed',
       report_bodies => $body->id . ',2505',
       view_username => 1
     },
@@ -1662,14 +1665,14 @@ for my $test (
     },
     {
         desc => 'update fixed without marking as open leaves state unchanged',
-        initial_state => 'fixed',
+        initial_state => 'fixed - user',
         expected_form_fields => {
             reopen => undef,
         },
         submitted_form_fields => {
             reopen => 0,
         },
-        end_state => 'fixed',
+        end_state => 'fixed - user',
     },
     {
         desc => 'update unable to fix without marking as fixed leaves state unchanged',
@@ -1761,7 +1764,7 @@ for my $test (
     },
     {
         desc => 'cannot mark fixed as fixed, can mark as not fixed',
-        initial_state => 'fixed',
+        initial_state => 'fixed - user',
         expected_form_fields => {
             reopen => undef,
         },
diff --git a/templates/web/base/report/_state_select_field.html b/templates/web/base/report/_state_select_field.html
index 9b3ed18df..adec1ff06 100644
--- a/templates/web/base/report/_state_select_field.html
+++ b/templates/web/base/report/_state_select_field.html
@@ -2,10 +2,19 @@
 [% IF include_empty %]
     <option value="">---</option>
 [% END %]
+[% SET found = 0 ~%]
+[% FOREACH group IN state_groups ~%]
+    [% FOREACH state IN group.1 ~%]
+        [% SET found = 1 IF state == current_state ~%]
+    [% END ~%]
+[% END ~%]
+[% IF NOT found ~%]
+    <option selected value="[% current_state %]">[% state_pretty.$current_state %]</option>
+[% END ~%]
 [% FOREACH group IN state_groups %]
     <optgroup label="[% group.0 %]">
     [% FOREACH state IN group.1 %]
-        <option [% 'selected ' IF state == current_state %] value="[% state %]">[% state_pretty.$state %]</option>
+        <option [% 'selected ' IF state == current_state %]value="[% state %]">[% state_pretty.$state %]</option>
     [% END %]
     </optgroup>
 [% END %]
diff --git a/templates/web/base/report/update/form_update.html b/templates/web/base/report/update/form_update.html
index f15a1f74b..34abf53c5 100644
--- a/templates/web/base/report/update/form_update.html
+++ b/templates/web/base/report/update/form_update.html
@@ -36,16 +36,8 @@
 <textarea rows="7" cols="30" name="update" class="form-control" id="form_update" placeholder="[% loc('Please write your update here') %]" required>[% update.text | html %]</textarea>
 
 [% IF c.user && c.user.belongs_to_body( problem.bodies_str ) %]
-    <label for="form_state">[% loc( 'State' ) %]</label>
-    <select name="state" id="form_state" class="form-control">
-    [% FOREACH state IN [ ['confirmed', loc('Open')], ['investigating',
-    loc('Investigating')], ['action scheduled', loc('Action Scheduled')],
-    ['in progress', loc('In Progress')], ['duplicate', loc('Duplicate')],
-    ['unable to fix', loc('No further action')], ['not responsible', loc('Not Responsible')],
-    ['fixed', loc('Fixed')] ] %]
-        <option [% 'selected ' IF state.0 == problem.state_display %] value="[% state.0 %]">[% state.1 %]</option>
-    [% END %]
-    </select>
+    <label for="state">[% loc( 'State' ) %]</label>
+    [% INCLUDE 'report/inspect/state_groups_select.html' %]
 [% ELSE %]
   [% IF (problem.is_fixed OR problem.state == 'closed') AND ((c.user_exists AND c.user.id == problem.user_id) OR alert_to_reporter) %]
 
diff --git a/templates/web/oxfordshire/report/inspect/state_groups_select.html b/templates/web/oxfordshire/report/inspect/state_groups_select.html
index d9db3864a..529e6f403 100644
--- a/templates/web/oxfordshire/report/inspect/state_groups_select.html
+++ b/templates/web/oxfordshire/report/inspect/state_groups_select.html
@@ -11,11 +11,14 @@ SET state_pretty = {
 
 SET state_groups = [
     [ loc('New'), [ 'confirmed', 'investigating' ] ],
-    [ loc('Scheduled'), [ 'action scheduled' ] ],
     [ loc('Fixed'), [ 'fixed - council' ] ],
     [ loc('Closed'), [ 'not responsible', 'duplicate', 'unable to fix' ] ]
 ];
 
+IF c.user_exists AND c.user.has_body_permission_to('report_instruct');
+    CALL state_groups.splice(1, 0, [ [ loc('Scheduled'), [ 'action scheduled' ] ] ]);
+END
+
 %]
 [% DEFAULT current_state = problem.state %]
 [% INCLUDE 'report/_state_select_field.html' %]