aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorChris Mytton <chrism@mysociety.org>2020-01-23 17:16:07 +0000
committerChris Mytton <chrism@mysociety.org>2020-01-28 11:01:37 +0000
commit86c5ce6b9b0ac945d4939f132bcf7235ff3ce25f (patch)
tree99c3b15d6ccce38b3e69e26c8cfe1257fd6f344a
parent2d6af993630d9e43594975e093507ee62ed53b97 (diff)
Allow multiple question to disable the new report form
This changes the existing logic to account for the fact that multiple questions might have the disable form checkbox ticked for one of their options. At the moment this just displays one of the messages at a time, with the first one taking priority. So if you have a "Is this an emergency" question and a "Is this on private land" question which both disable the form, and the user has selected "Yes" to both, then only the message for the first question is displayed. This can potentially be improved in the future, but seemed out of scope for this change.
-rw-r--r--perllib/FixMyStreet/App/Controller/Report/New.pm27
-rw-r--r--t/app/controller/report_new_open311.t10
-rw-r--r--web/cobrands/fixmystreet/fixmystreet.js22
3 files changed, 38 insertions, 21 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm
index ba37a25fa..6f6428089 100644
--- a/perllib/FixMyStreet/App/Controller/Report/New.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/New.pm
@@ -328,13 +328,15 @@ sub disable_form_message : Private {
$out{all} .= ' ' if $out{all};
$out{all} .= $_->{description};
} elsif (($_->{variable} || '') eq 'true' && @{$_->{values} || []}) {
+ my %category;
foreach my $opt (@{$_->{values}}) {
if ($opt->{disable}) {
- $out{message} = $opt->{disable_message} || $_->{datatype_description};
- $out{code} = $_->{code};
- push @{$out{answers}}, $opt->{key};
+ $category{message} = $opt->{disable_message} || $_->{datatype_description};
+ $category{code} = $_->{code};
+ push @{$category{answers}}, $opt->{key};
}
}
+ push @{$out{questions}}, \%category if %category;
}
}
@@ -1575,16 +1577,19 @@ sub check_for_category : Private {
my $disable_form_messages = $c->forward('disable_form_message');
if ($disable_form_messages->{all}) {
$c->stash->{disable_form_message} = $disable_form_messages->{all};
- } elsif (my $code = $disable_form_messages->{code}) {
- my $answer = $c->get_param($code);
- my $message = $disable_form_messages->{message};
- if ($answer) {
- foreach (@{$disable_form_messages->{answers}}) {
- if ($answer eq $_) {
- $c->stash->{disable_form_message} = $message;
+ } elsif (my $questions = $disable_form_messages->{questions}) {
+ foreach my $question (@$questions) {
+ my $answer = $c->get_param($question->{code});
+ my $message = $question->{message};
+ if ($answer) {
+ foreach (@{$question->{answers}}) {
+ if ($answer eq $_) {
+ $c->stash->{disable_form_message} = $message;
+ }
}
}
- } else {
+ }
+ if (!$c->stash->{disable_form_message}) {
$c->stash->{have_disable_qn_to_answer} = 1;
}
}
diff --git a/t/app/controller/report_new_open311.t b/t/app/controller/report_new_open311.t
index 3488b96ce..dcad10899 100644
--- a/t/app/controller/report_new_open311.t
+++ b/t/app/controller/report_new_open311.t
@@ -378,9 +378,13 @@ subtest "Category extras includes form disabling string" => sub {
my $output = $json->{by_category} ? $json->{by_category}{Pothole}{disable_form} : $json->{disable_form};
is_deeply $output, {
all => 'Please ring us!',
- message => 'Please please ring',
- code => 'dangerous',
- answers => [ 'yes' ],
+ questions => [
+ {
+ message => 'Please please ring',
+ code => 'dangerous',
+ answers => [ 'yes' ],
+ },
+ ],
};
}
diff --git a/web/cobrands/fixmystreet/fixmystreet.js b/web/cobrands/fixmystreet/fixmystreet.js
index 1a513a1a5..a5fd90ccb 100644
--- a/web/cobrands/fixmystreet/fixmystreet.js
+++ b/web/cobrands/fixmystreet/fixmystreet.js
@@ -459,9 +459,13 @@ $.extend(fixmystreet.set_up, {
$(".js-hide-if-public-category").hide();
}
- if (fixmystreet.message_controller && data && data.disable_form && data.disable_form.answers) {
- $('#form_' + data.disable_form.code).on('change.category', function() {
- $(fixmystreet).trigger('report_new:category_change');
+ if (fixmystreet.message_controller && data && data.disable_form && data.disable_form.questions) {
+ $.each(data.disable_form.questions, function(_, question) {
+ if (question.message && question.code) {
+ $('#form_' + question.code).on('change.category', function() {
+ $(fixmystreet).trigger('report_new:category_change');
+ });
+ }
});
}
@@ -1334,10 +1338,14 @@ fixmystreet.fetch_reporting_data = function() {
message: details.disable_form.all
});
}
- if (details.disable_form.answers) {
- details.disable_form.category = category;
- details.disable_form.keep_category_extras = true;
- fixmystreet.message_controller.register_category(details.disable_form);
+ if (details.disable_form.questions) {
+ $.each(details.disable_form.questions, function(_, question) {
+ if (question.message && question.code) {
+ question.category = category;
+ question.keep_category_extras = true;
+ fixmystreet.message_controller.register_category(question);
+ }
+ });
}
});
}