diff options
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Admin.pm | 10 | ||||
| -rw-r--r-- | perllib/FixMyStreet/Cobrand/Zurich.pm | 58 | ||||
| -rw-r--r-- | t/cobrand/zurich.t | 38 | ||||
| -rw-r--r-- | templates/web/zurich/admin/report_edit.html | 101 | ||||
| -rw-r--r-- | templates/web/zurich/report/updates.html | 2 |
5 files changed, 144 insertions, 65 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm index 5ebeffc11..677aa66d7 100644 --- a/perllib/FixMyStreet/App/Controller/Admin.pm +++ b/perllib/FixMyStreet/App/Controller/Admin.pm @@ -683,7 +683,15 @@ sub report_edit : Path('report_edit') : Args(1) { if (my $rotate_photo_param = $self->_get_rotate_photo_param($c)) { $self->rotate_photo($c, @$rotate_photo_param); - return 1; + if ( $c->cobrand->moniker eq 'zurich' ) { + # Clicking the photo rotation buttons should do nothing + # except for rotating the photo, so return the user + # to the report screen now. + $c->res->redirect( $c->uri_for( 'report_edit', $problem->id ) ); + return; + } else { + return 1; + } } if ( $c->cobrand->moniker eq 'zurich' ) { diff --git a/perllib/FixMyStreet/Cobrand/Zurich.pm b/perllib/FixMyStreet/Cobrand/Zurich.pm index 1a9963a60..0e75d71b9 100644 --- a/perllib/FixMyStreet/Cobrand/Zurich.pm +++ b/perllib/FixMyStreet/Cobrand/Zurich.pm @@ -123,6 +123,26 @@ sub problem_is_closed { return exists $self->zurich_closed_states->{ $problem->state } ? 1 : 0; } +sub zurich_public_response_states { + my $states = { + 'fixed - council' => 1, + 'closed' => 1, # extern + 'unable to fix' => 1, # jurisdiction unknown + + # e.g. as above, but WITHOUT the following (as they are hidden) + # 'hidden' => 1, + # 'investigating' => 1, # wish + # 'partial' => 1, # not contactable + }; + + return wantarray ? keys %{ $states } : $states; +} + +sub problem_has_public_response { + my ($self, $problem) = @_; + return exists $self->zurich_public_response_states->{ $problem->state } ? 1 : 0; +} + sub problem_as_hashref { my $self = shift; my $problem = shift; @@ -570,13 +590,8 @@ sub admin_report_edit { my $state = $c->get_param('state') || ''; my $oldstate = $problem->state; - my %closure_states = ( - 'closed' => 1, - 'investigating' => 1, - 'hidden' => 1, - 'partial' => 1, - 'unable to fix' => 1, - ); + my $closure_states = $self->zurich_closed_states; + delete $closure_states->{'fixed - council'}; # may not be needed? my $old_closure_state = $problem->get_extra_metadata('closure_status'); @@ -595,7 +610,7 @@ sub admin_report_edit { $internal_note_text = "Weitergeleitet von $old_cat an $new_cat"; $self->update_admin_log($c, $problem, "Changed category from $old_cat to $new_cat"); $redirect = 1 if $cat->body_id ne $body->id; - } elsif ( $closure_states{$state} and + } elsif ( $closure_states->{$state} and ( $oldstate ne 'planned' ) || (($old_closure_state ||'') ne $state)) { @@ -610,7 +625,7 @@ sub admin_report_edit { $problem->set_extra_metadata( closure_status => $state ); $self->set_problem_state($c, $problem, 'planned'); $state = 'planned'; - $redirect = 1; + $problem->set_extra_metadata_if_undefined( moderated_overdue => $self->overdue( $problem ) ); } elsif ( my $subdiv = $c->get_param('body_subdivision') ) { $problem->set_extra_metadata_if_undefined( moderated_overdue => $self->overdue( $problem ) ); @@ -628,12 +643,8 @@ sub admin_report_edit { } $self->set_problem_state($c, $problem, $state) - unless $closure_states{$state}; # in closure-states case, we'll defer to clause below - - if ($self->problem_is_closed($problem)) { - $problem->set_extra_metadata_if_undefined( closed_overdue => $self->overdue( $problem ) ); - $c->stash->{problem_is_closed} = 1; - } + unless $closure_states->{$state}; + # we'll defer to 'planned' clause below to change the state } } @@ -641,15 +652,17 @@ sub admin_report_edit { # Rueckmeldung ausstehend # override $state from the metadata set above $state = $problem->get_extra_metadata('closure_status') || ''; - my $closed = 0; + my ($moderated, $closed) = (0, 0); if ($state eq 'hidden' && $c->req->params->{publish_response} ) { _admin_send_email( $c, 'problem-rejected.txt', $problem ); - $problem->set_extra_metadata_if_undefined( moderated_overdue => $self->overdue( $problem ) ); + $self->set_problem_state($c, $problem, $state); + $moderated++; $closed++; } elsif ($state =~/^(closed|investigating)$/) { # Extern | Wish + $moderated++; # Nested if instead of `and` because in these cases, we *don't* # want to close unless we have body_external (so we don't want # the final elsif clause below to kick in on publish_response) @@ -659,7 +672,6 @@ sub admin_report_edit { $problem->external_body( $external ); } if ($problem->external_body && $c->req->params->{publish_response}) { - $problem->set_extra_metadata_if_undefined( moderated_overdue => $self->overdue( $problem ) ); $problem->whensent( undef ); $self->set_problem_state($c, $problem, $state); my $template = ($state eq 'investigating') ? 'problem-wish.txt' : 'problem-external.txt'; @@ -671,14 +683,20 @@ sub admin_report_edit { } elsif ($c->req->params->{publish_response}) { # otherwise we only set the state if publish_response is set + # # if $state wasn't set, then we are simply closing the message as fixed $state ||= 'fixed - council'; _admin_send_email( $c, 'problem-closed.txt', $problem ); $redirect = 1; + $moderated++; $closed++; } + if ($moderated) { + $problem->set_extra_metadata_if_undefined( moderated_overdue => $self->overdue( $problem ) ); + } + if ($closed) { # set to either the closure_status from metadata or 'fixed - council' as above $self->set_problem_state($c, $problem, $state); @@ -757,7 +775,9 @@ sub admin_report_edit { # (this will only happen if no other update_admin_log has already been called) $self->update_admin_log($c, $problem); - if ( $redirect ) { + if ( $redirect and $type eq 'dm' ) { + # only redirect for DM + $c->stash->{status_message} ||= '<p><em>' . _('Updated!') . '</em></p>'; $c->go('index'); } diff --git a/t/cobrand/zurich.t b/t/cobrand/zurich.t index 0a622cb2b..a8a1d973e 100644 --- a/t/cobrand/zurich.t +++ b/t/cobrand/zurich.t @@ -782,7 +782,7 @@ subtest "test stats" => sub { $mech->get_ok( '/admin/stats' ); is $mech->res->code, 200, "superuser should be able to see stats page"; - $mech->content_contains('Innerhalb eines Arbeitstages moderiert: 2'); # now including hidden + $mech->content_contains('Innerhalb eines Arbeitstages moderiert: 3'); $mech->content_contains('Innerhalb von fünf Arbeitstagen abgeschlossen: 3'); # my @data = $mech->content =~ /(?:moderiert|abgeschlossen): \d+/g; # diag Dumper(\@data); use Data::Dumper; @@ -854,6 +854,42 @@ subtest 'email images to external partners' => sub { }; }; +subtest 'Status update shown as appropriate' => sub { + FixMyStreet::override_config { + ALLOWED_COBRANDS => [ 'zurich' ], + }, sub { + # ALL closed states must hide the public_response edit, and public ones + # must show the answer in blue. + for (['planned', 1, 0], + ['fixed - council', 0, 1], + ['closed', 0, 1], + ['hidden', 0, 0]) + { + my ($state, $update, $public) = @$_; + $report->update({ state => $state }); + $mech->get_ok( '/admin/report_edit/' . $report->id ); + contains_or_lacks($mech, $update, "<textarea name='status_update'"); + contains_or_lacks($mech, $public, '<div class="admin-official-answer">'); + + if ($public) { + $mech->get_ok( '/report/' . $report->id ); + $mech->content_contains('Antwort</h4>'); + } + } + }; +}; + +# TODO refactor into FixMyStreet::TestMech; +sub contains_or_lacks { + my ($mech, $contains, $text) = @_; + if ($contains) { + $mech->content_contains($text); + } + else { + $mech->content_lacks($text); + } +} + subtest 'time_spent' => sub { FixMyStreet::override_config { ALLOWED_COBRANDS => [ 'zurich' ], diff --git a/templates/web/zurich/admin/report_edit.html b/templates/web/zurich/admin/report_edit.html index 95aa5d311..802ac466d 100644 --- a/templates/web/zurich/admin/report_edit.html +++ b/templates/web/zurich/admin/report_edit.html @@ -25,7 +25,7 @@ <li class="report-edit-action">» <a href="http://webgis.intra.stzh.ch/AV_Online/Direct.asp?Map=AV&Search=Koord&West=[% problem.local_coords.0 %]&Nord=[% problem.local_coords.1 %]&B=300" target="_blank">Standort in AV-Online anzeigen</a></li> -[% IF problem_is_closed %] +[% IF c.cobrand.problem_is_closed(problem) %] <li><span class="mock-label">[% loc('Details:') %]</span> [% problem.detail | html %] [% IF problem.extra.original_detail %] <br>[% @@ -133,12 +133,15 @@ </script> </p> -<p><span class="mock-label">[% loc('State:') %]</span> <select name="state" id="state"> - <option value="">--</option> - [% FOREACH s IN states %] - <option [% 'selected ' IF s.state == pstate %] value="[% s.state %]">[% s.trans %]</option> - [% END %] -</select></p> +<p> + <span class="mock-label">[% loc('State:') %]</span> + <select name="state" id="state"> + <option value="">--</option> + [% FOREACH s IN states %] + <option [% 'selected ' IF s.state == pstate %] value="[% s.state %]">[% s.trans %]</option> + [% END %] + </select> +</p> <script type="text/javascript"> $(function(){ @@ -157,35 +160,33 @@ $(function(){ // Show or hide the automatic reply field var state = $(this).val(); - // Show or hide the categories - if (state === 'confirmed') { - $('#assignation__category').show(); - $('#assignation__subdivision').show(); - } else { - $('#assignation__category').hide(); - $('#assignation__subdivision').hide(); - $('#assignation__category select').val(''); - $('#assignation__subdivision select').val(''); - } - - if ((state === 'closed') || (state === 'investigating')) { - $('#assignation__external').show(); - } else { - $('#assignation__external select').val(''); - $('#assignation__external').hide(); - } - - // disable the templates, public_response, publish if we've selected a - // different state to the one we started on + // show or disable assignation, templates, public_response, publish if + // same or different state to the one we started on if ((state === '[% pstate %]')) { $('input[name=publish_response]').show(); $('.response_templates_select').show(); - $('#status_update').show(); + $('#status_update_container').show(); + + if (state === 'confirmed') { + $('#assignation__category').show(); + $('#assignation__subdivision').show(); + } + if ((state === 'closed') || (state === 'investigating')) { + $('#assignation__external').show(); + } } else { $('input[name=publish_response]').hide(); $('.response_templates_select').hide(); - $('#status_update').hide(); + $('#status_update_container').hide(); + + $('#assignation__category').hide(); + $('#assignation__subdivision').hide(); + $('#assignation__category select').val(''); + $('#assignation__subdivision select').val(''); + + $('#assignation__external select').val(''); + $('#assignation__external').hide(); } }).change(); @@ -256,28 +257,42 @@ $(function(){ </ul> [% END %] -[%# Public response field shown for Ruckmeldung ausstehend/Extern/ - # Zustandigkeit unbekannt/Wunsch/Nicht kontaktierbar/Unsichtbar %] -[% SWITCH pstate %] - [% CASE ['planned','closed','unable to fix','investigating','partial','hidden'] %] - <ul class="no-bullets report-edit-action"> - <li><label for="status_update">[% loc('Public response:') %]</label> - [% INCLUDE 'admin/response_templates_select.html' for='status_update' %] - <textarea name='status_update' id='status_update' cols=60 rows=5> - [%- problem.extra.public_response || default_public_response | html -%] - </textarea> - </li> - </ul> +[%# Public response field shown for Ruckmeldung ausstehend states + # (e.g. various pstates) %] +[% IF problem.state == 'planned' %] + <ul class="no-bullets report-edit-action"> + <li id="status_update_container"><label for="status_update"> + [% SWITCH pstate %] + [% CASE ['hidden', 'investigating', 'partial'] %][%# Hidden/Wish/Not contactable %] + [% loc('Reply to user:') %] + [% CASE DEFAULT %] + [% loc('Public response:') %] + [% END %] + </label> + [% INCLUDE 'admin/response_templates_select.html' for='status_update' %] + <textarea name='status_update' id='status_update' cols=60 rows=5> + [%- problem.extra.public_response || default_public_response | html -%] + </textarea> + </li> + </ul> [% END %] <p align="right" class="report-edit-action"> [% IF show_publish_response %] -<input type="submit" name="publish_response" value="[% loc('Publish the response') %]"> + [%# While we call this 'publish_response', the response will not actually + # be "published" for these cases: Wish / Hidden / Not contactable (for these, + # only a private email will be sent to the user. However, in all cases, + # this is the end of processing, so we mark this with the same text used + # for 'No further updates %] +<input type="submit" name="publish_response" value="[% loc('No further updates') %]"> [% END %] + + [%# This button simply saves changes, but does NOT close the report (though + # it may trigger other workflow %] <input type="submit" name="Submit changes" value="[% loc('Submit changes') %]" > </p> -[% IF problem.state == 'fixed - council' %] +[% IF c.cobrand.problem_has_public_response(problem) %] <h2>[% loc('Public response:') %]</h2> <div class="admin-official-answer"> [% problem.extra.public_response | html_para %] diff --git a/templates/web/zurich/report/updates.html b/templates/web/zurich/report/updates.html index ea71f2b1c..cb7644694 100644 --- a/templates/web/zurich/report/updates.html +++ b/templates/web/zurich/report/updates.html @@ -8,7 +8,7 @@ [%# XXX following should honour zurich_closed_states instead? %] [% IF problem.state == 'fixed - council' || ( problem.external_body AND problem.state == 'closed' ) %] - [% add_links( problem.extra.public_response ) | html_para %] + [% add_links( problem.extra.public_response ) | html_para %] [% END %] </div> </div> |