diff options
author | Matthew Somerville <matthew-github@dracos.co.uk> | 2019-02-08 11:22:20 +0000 |
---|---|---|
committer | Matthew Somerville <matthew-github@dracos.co.uk> | 2019-02-11 17:02:25 +0000 |
commit | fb65300cbf6fb9ca9ad077b12d44f188294d8005 (patch) | |
tree | 5bbdddfa69978576f39ad57aa9691624459f266f | |
parent | d8d3da1119e41fd7a57a4664e92a644ba1a9c919 (diff) |
[fixmystreet.com] Improve two-tier unresponsive.
Similarly to 1f69e28c, we were previously only checking the first
matching entry, which led to confusing behaviour in places. Include
consequential amendments for e.g. one body being unresponsive, the
other not.
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Report/New.pm | 40 | ||||
-rw-r--r-- | t/app/controller/report_new.t | 6 | ||||
-rw-r--r-- | templates/web/fixmystreet.com/report/new/unresponsive_body.html | 11 |
3 files changed, 30 insertions, 27 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm index 2de1d8551..b6292facb 100644 --- a/perllib/FixMyStreet/App/Controller/Report/New.pm +++ b/perllib/FixMyStreet/App/Controller/Report/New.pm @@ -286,7 +286,9 @@ sub by_category_ajax_data : Private { # unresponsive must return empty string if okay, as that's what mobile app checks if ($type eq 'one' || ($type eq 'all' && $unresponsive)) { $body->{unresponsive} = $unresponsive; - if ($type eq 'all' && $unresponsive) { + # Check for no bodies here, because if there are any (say one + # unresponsive, one not), can use default display code for that. + if ($type eq 'all' && !@$bodies) { $body->{councils_text} = $c->render_fragment( 'report/new/councils_text.html', $vars); $body->{councils_text_private} = $c->render_fragment( 'report/new/councils_text_private.html'); } @@ -635,7 +637,6 @@ sub setup_categories_and_bodies : Private { my @bodies = $c->model('DB::Body')->active->for_areas(keys %$all_areas)->all; my %bodies = map { $_->id => $_ } @bodies; - my $first_body = ( values %bodies )[0]; my $contacts # = $c # @@ -654,17 +655,18 @@ sub setup_categories_and_bodies : Private { (); # categories for which the reports are not public $c->stash->{unresponsive} = {}; - if (keys %bodies == 1 && $first_body->send_method && $first_body->send_method eq 'Refused') { - # If there's only one body, and it's set to refused, we can show the + my @refused_bodies = grep { ($_->send_method || "") eq 'Refused' } values %bodies; + if (@refused_bodies && @refused_bodies == values %bodies) { + # If all bodies are set to Refused, we can show the # message immediately, before they select a category. + my $k = 'ALL'; if ($c->action->name eq 'category_extras_ajax' && $c->req->method eq 'POST') { # The mobile app doesn't currently use this, in which case make # sure the message is output, either below with a category, or when # a blank category call is made. - $c->stash->{unresponsive}{""} = $first_body->id; - } else { - $c->stash->{unresponsive}{ALL} = $first_body->id; + $k = ""; } + $c->stash->{unresponsive}{$k} = { map { $_ => 1 } keys %bodies }; } # keysort does not appear to obey locale so use strcoll (see i18n.t) @@ -694,14 +696,12 @@ sub setup_categories_and_bodies : Private { $non_public_categories{ $contact->category } = 1 if $contact->non_public; - unless ( $seen{$contact->category} ) { - push @category_options, $contact; + my $body_send_method = $contact->body->send_method || ''; + $c->stash->{unresponsive}{$contact->category}{$contact->body_id} = 1 + if !$c->stash->{unresponsive}{ALL} && + ($contact->email =~ /^REFUSED$/i || $body_send_method eq 'Refused'); - my $body_send_method = $bodies{$contact->body_id}->send_method || ''; - $c->stash->{unresponsive}{$contact->category} = $contact->body_id - if !$c->stash->{unresponsive}{ALL} && - ($contact->email =~ /^REFUSED$/i || $body_send_method eq 'Refused'); - } + push @category_options, $contact unless $seen{$contact->category}; $seen{$contact->category} = $contact; } @@ -1052,17 +1052,17 @@ sub contacts_to_bodies : Private { @contacts = @contacts_filtered if scalar @contacts_filtered; } - if ($c->stash->{unresponsive}{$category} || $c->stash->{unresponsive}{ALL} || !@contacts) { - []; - } else { + my $unresponsive = $c->stash->{unresponsive}{$category} || $c->stash->{unresponsive}{ALL}; + if ($unresponsive) { + @contacts = grep { !$unresponsive->{$_->body_id} } @contacts; + } elsif (@contacts) { if ( $c->cobrand->call_hook('singleton_bodies_str') ) { # Cobrands like Zurich can only ever have a single body: 'x', because some functionality # relies on string comparison against bodies_str. - [ $contacts[0]->body ]; - } else { - [ map { $_->body } @contacts ]; + @contacts = ($contacts[0]); } } + [ map { $_->body } @contacts ]; } sub set_report_extras : Private { diff --git a/t/app/controller/report_new.t b/t/app/controller/report_new.t index 66b802873..40264f073 100644 --- a/t/app/controller/report_new.t +++ b/t/app/controller/report_new.t @@ -1807,9 +1807,9 @@ subtest "unresponsive body handling works" => sub { my $body_id = $contact1->body->id; my $extra_details = $mech->get_ok_json('/report/new/ajax?latitude=55.952055&longitude=-3.189579'); like $extra_details->{top_message}, qr{Edinburgh.*accept reports.*/unresponsive\?body=$body_id}; - is $extra_details->{unresponsive}, $body_id, "unresponsive json set"; + is_deeply $extra_details->{unresponsive}, { $body_id => 1 }, "unresponsive json set"; $extra_details = $mech->get_ok_json('/report/new/category_extras?category=Street%20lighting&latitude=55.952055&longitude=-3.189579'); - is $extra_details->{unresponsive}, $body_id, "unresponsive json set"; + is_deeply $extra_details->{unresponsive}, { $body_id => 1 }, "unresponsive json set"; my $test_email = 'test-2@example.com'; $mech->log_out_ok; @@ -1886,7 +1886,7 @@ subtest "unresponsive body handling works" => sub { $extra_details = $mech->get_ok_json('/report/new/ajax?latitude=51.896268&longitude=-2.093063'); like $extra_details->{by_category}{$contact3->category}{category_extra}, qr/Cheltenham.*Trees.*unresponsive.*category=Trees/s; $extra_details = $mech->get_ok_json('/report/new/category_extras?category=Trees&latitude=51.896268&longitude=-2.093063'); - is $extra_details->{unresponsive}, $contact3->body->id, "unresponsive json set"; + is_deeply $extra_details->{unresponsive}, { $contact3->body->id => 1 }, "unresponsive json set"; $mech->get_ok('/around'); $mech->submit_form_ok( { with_fields => { pc => 'GL50 2PR', } }, "submit location" ); diff --git a/templates/web/fixmystreet.com/report/new/unresponsive_body.html b/templates/web/fixmystreet.com/report/new/unresponsive_body.html index 0a34475bc..54a9fb195 100644 --- a/templates/web/fixmystreet.com/report/new/unresponsive_body.html +++ b/templates/web/fixmystreet.com/report/new/unresponsive_body.html @@ -1,8 +1,10 @@ -[% SET soon = bodies.$body_id.name == 'Northamptonshire County Council' %] +[% +SET first = body_id.keys.first; # Might be more than one, but showing one will do +SET soon = bodies.$first.name == 'Northamptonshire County Council' %] <div class="box-warning"> <h1>Important message</h1> <p> - <span class="unresponsive-council">[% bodies.$body_id.name %]</span> doesn’t currently accept + <span class="unresponsive-council">[% bodies.$first.name %]</span> doesn’t currently accept [% IF soon %] FixMyStreet reports – though it will do so soon. [% ELSE %] @@ -12,6 +14,7 @@ reports from third party reporting sites such as FixMyStreet. [% END %] </p> - <p>We can make your report public, but [% 'at the moment' IF soon %] we can’t send it to the council.</p> - <a href="[% c.cobrand.base_url %]/unresponsive?body=[% body_id %][% IF category %];category=[% category | uri %][% END %]" class="btn">[% soon ? 'Find out more' : 'What can I do instead?' %]</a> + <p>We can make your report public, but [% 'at the moment' IF soon %] we can’t send it to + [% IF bodies.size > 1 %] that [% ELSE %] the [% END %] council.</p> + <a href="[% c.cobrand.base_url %]/unresponsive?body=[% first %][% IF category %];category=[% category | uri %][% END %]" class="btn">[% soon ? 'Find out more' : 'What can I do instead?' %]</a> </div> |