aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMatthew Somerville <matthew-github@dracos.co.uk>2019-02-08 11:22:20 +0000
committerMatthew Somerville <matthew-github@dracos.co.uk>2019-02-11 17:02:25 +0000
commitfb65300cbf6fb9ca9ad077b12d44f188294d8005 (patch)
tree5bbdddfa69978576f39ad57aa9691624459f266f
parentd8d3da1119e41fd7a57a4664e92a644ba1a9c919 (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.pm40
-rw-r--r--t/app/controller/report_new.t6
-rw-r--r--templates/web/fixmystreet.com/report/new/unresponsive_body.html11
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>