diff options
author | Dave Arter <davea@mysociety.org> | 2020-02-11 15:16:23 +0000 |
---|---|---|
committer | Dave Arter <davea@mysociety.org> | 2020-02-12 16:13:36 +0000 |
commit | 527d763b77598b656642a47d224e0411264a4c53 (patch) | |
tree | 939dc61ff3216350bbd653bcabebe161725afe0a | |
parent | 4ceb95be74b48776b14d0155d3a14323d12a8acf (diff) |
Fix category extra AJAX call for categories with ampersands
The `IF category_extras.$category.size` condition in category_extras.html
was using an escaped string for `$category`, meaning the lookup would
always fail for categories that had an ampersand in their name. The result
of this was an empty `<div id="category_meta">`, and no extra fields shown
to the user. This caused issues when one or more of the fields were
required, as they were always empty and the error message wasn't being
rendered and shown to the user. This meant the user would see their report
form over and over again when submitting instead of useful feedback.
We also ensure $category is always a SafeString when used as a key into
a hashref in other templates.
Fixes https://github.com/mysociety/fixmystreet-freshdesk/issues/126
7 files changed, 31 insertions, 0 deletions
diff --git a/t/app/controller/report_new_open311.t b/t/app/controller/report_new_open311.t index dcad10899..08435fb2b 100644 --- a/t/app/controller/report_new_open311.t +++ b/t/app/controller/report_new_open311.t @@ -360,6 +360,31 @@ subtest "Category extras includes description label for user" => sub { }; }; +subtest "Category extras are correct even if category has an ampersand in it" => sub { + FixMyStreet::override_config { + ALLOWED_COBRANDS => [ { fixmystreet => '.' } ], + MAPIT_URL => 'http://mapit.uk/', + }, sub { + for ( + { url => '/report/new/ajax?' }, + { url => '/report/new/category_extras?category=Potholes+%26+Road+Defects' }, + ) { + my $category = "Potholes & Road Defects"; + $contact4->update({ category => $category }); + my $json = $mech->get_ok_json($_->{url} . '&latitude=55.952055&longitude=-3.189579'); + my $category_extra = $json->{by_category} ? $json->{by_category}{$category}{category_extra} : $json->{category_extra}; + contains_string($category_extra, "usrn") or diag $mech->content; + contains_string($category_extra, "central_asset_id"); + lacks_string($category_extra, "USRN", "Lacks 'USRN' label"); + lacks_string($category_extra, "Asset ID", "Lacks 'Asset ID' label"); + contains_string($category_extra, "Size?"); + lacks_string($category_extra, '<option value=""'); + contains_string($category_extra, "resolve your problem quicker, by providing some extra detail", "Contains description text"); + $contact4->update({ category => "Pothole" }); + } + }; +}; + subtest "Category extras includes form disabling string" => sub { FixMyStreet::override_config { ALLOWED_COBRANDS => 'fixmystreet', diff --git a/templates/web/base/report/new/category_extras.html b/templates/web/base/report/new/category_extras.html index 5cef391ce..9b4149119 100644 --- a/templates/web/base/report/new/category_extras.html +++ b/templates/web/base/report/new/category_extras.html @@ -1,5 +1,6 @@ [% SET default_list = [] %][% FOR b IN bodies_to_list.values %][% default_list.push(b.cobrand_name) %][% END %] [% DEFAULT list_of_names = default_list %] +[% category = mark_safe(category) %] <div id="category_meta"> [%- IF unresponsive.$category %] diff --git a/templates/web/base/report/new/councils_text.html b/templates/web/base/report/new/councils_text.html index 5f4b113ca..9b3d6fce7 100644 --- a/templates/web/base/report/new/councils_text.html +++ b/templates/web/base/report/new/councils_text.html @@ -1,4 +1,5 @@ [% FILTER collapse %] +[% category = mark_safe(category) %] [% IF unresponsive.$category OR unresponsive.ALL OR bodies_to_list.size == 0 %] [% tprintf( loc('These will be published online for others to see, in accordance with our <a href="%s">privacy policy</a>.'), diff --git a/templates/web/base/report/new/councils_text_all.html b/templates/web/base/report/new/councils_text_all.html index da512c980..08e27e8b3 100644 --- a/templates/web/base/report/new/councils_text_all.html +++ b/templates/web/base/report/new/councils_text_all.html @@ -1,5 +1,6 @@ [% SET default_list = [] %][% FOR b IN bodies_to_list.values %][% default_list.push(b.cobrand_name) %][% END %] [% DEFAULT list_of_names = default_list %] +[% category = mark_safe(category) %] <p> [% UNLESS non_public_categories.$category; diff --git a/templates/web/base/report/new/councils_text_private.html b/templates/web/base/report/new/councils_text_private.html index cab38c97e..80bcfed74 100644 --- a/templates/web/base/report/new/councils_text_private.html +++ b/templates/web/base/report/new/councils_text_private.html @@ -1,4 +1,5 @@ [% FILTER collapse %] +[% category = mark_safe(category) %] [% IF unresponsive.$category OR unresponsive.ALL OR bodies_to_list.size == 0 %] [% loc('These details will never be shown online without your permission.') %] [% ELSE %] diff --git a/templates/web/oxfordshire/report/new/councils_text_all.html b/templates/web/oxfordshire/report/new/councils_text_all.html index 08852a772..38459b059 100644 --- a/templates/web/oxfordshire/report/new/councils_text_all.html +++ b/templates/web/oxfordshire/report/new/councils_text_all.html @@ -1,3 +1,4 @@ +[% category = mark_safe(category) %] <p> All the information you provide here will be sent to the [% # NB this empty class attribute is so fixmystreet.update_public_councils_text diff --git a/templates/web/tfl/report/new/councils_text_all.html b/templates/web/tfl/report/new/councils_text_all.html index 22e130347..ae9fb3307 100644 --- a/templates/web/tfl/report/new/councils_text_all.html +++ b/templates/web/tfl/report/new/councils_text_all.html @@ -1,4 +1,5 @@ <p> +[% category = mark_safe(category) %] [% UNLESS non_public_categories.$category; tprintf( |