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 mysociety/fixmystreet-freshdesk#126
mysociety · Feb 12, 2020 · 527d763 · 527d763
1 parent 4ceb95b
commit 527d763
Show file tree

Hide file tree

Showing 7 changed files with 31 additions and 0 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -1,4 +1,5 @@
 <p>
+[% category = mark_safe(category) %]
 [% UNLESS non_public_categories.$category;
 
     tprintf(