Fix lookups in templates of categories with &s.

A hash lookup in a template is escaping the key used, meaning the lookup
is failing anywhere we are using a category containing an ampersand as a
key. A continuation of the changes made in 527d763.

CHANGELOG.md

@@ -26,6 +26,7 @@
         - Fix duplicate asset message after dismissing duplicate suggestions.
         - Improve moderation diff display in a few small ways. #3105
         - Do not have bootstrap run sudo commands. #2930
+        - Fix lookups in templates of categories with &s.
     - Admin improvements:
         - Display user name/email for contributed as reports. #2990
         - Interface for enabling anonymous reports for certain categories. #2989

t/app/controller/around.t

@@ -217,8 +217,8 @@ for my $permission ( qw/ report_inspect report_mark_private/ ) {
 
 subtest 'check assigned-only list items do not display shortlist buttons' => sub {
     my $body = FixMyStreet::DB->resultset('Body')->find( $body_edin_id );
-    my $contact = $mech->create_contact_ok( category => 'Horses', body_id => $body->id, email => "horses\@example.org" );
-    $edinburgh_problems[4]->update({ category => 'Horses' });
+    my $contact = $mech->create_contact_ok( category => 'Horses & Ponies', body_id => $body->id, email => "horses\@example.org" );
+    $edinburgh_problems[4]->update({ category => 'Horses & Ponies' });
 
     my $user = $mech->log_in_ok( 'test@example.com' );
     $user->set_extra_metadata(assigned_categories_only => 1);

t/app/controller/report_inspect.t

@@ -7,7 +7,7 @@ my $brum = $mech->create_body_ok(2514, 'Birmingham City Council');
 my $oxon = $mech->create_body_ok(2237, 'Oxfordshire County Council', { can_be_devolved => 1 } );
 my $contact = $mech->create_contact_ok( body_id => $oxon->id, category => 'Cows', email => 'cows@example.net' );
 my $contact2 = $mech->create_contact_ok( body_id => $oxon->id, category => 'Sheep', email => 'SHEEP', send_method => 'Open311' );
-my $contact3 = $mech->create_contact_ok( body_id => $oxon->id, category => 'Badgers', email => 'badgers@example.net' );
+my $contact3 = $mech->create_contact_ok( body_id => $oxon->id, category => 'Badgers & Voles', email => 'badgers@example.net' );
 my $rp = FixMyStreet::DB->resultset("ResponsePriority")->create({
     body => $oxon,
     name => 'High Priority',
@@ -683,19 +683,20 @@ FixMyStreet::override_config {
 
 FixMyStreet::override_config {
     MAPIT_URL => 'http://mapit.uk/',
-    ALLOWED_COBRANDS => 'fixmystreet',
+    ALLOWED_COBRANDS => 'oxfordshire',
 }, sub {
     subtest "test category not updated if fail to include public update" => sub {
         $mech->get_ok("/report/$report_id");
-        $mech->submit_form(button => 'save', with_fields => { category => 'Badgers' });
+        $mech->submit_form(button => 'save', with_fields => { category => 'Badgers & Voles' });
 
         $report->discard_changes;
         is $report->category, "Cows", "Report in correct category";
-        $mech->content_contains('Badgers" selected', 'Changed category still selected');
+        $mech->content_contains('Badgers & Voles" selected', 'Changed category still selected');
     };
 
     subtest "test invalid form maintains Category and priority" => sub {
         $mech->get_ok("/report/$report_id");
+        $mech->content_like(qr/data-priorities='[^']*?Low Priority/);
         my $expected_fields = {
           state => 'action scheduled',
           category => 'Cows',
@@ -709,9 +710,9 @@ FixMyStreet::override_config {
         my $values = $mech->visible_form_values('report_inspect_form');
         is_deeply $values, $expected_fields, 'correct form fields present';
 
-        $mech->submit_form(button => 'save', with_fields => { category => 'Badgers', priority => $rp2->id });
+        $mech->submit_form(button => 'save', with_fields => { category => 'Badgers & Voles', priority => $rp2->id });
 
-        $expected_fields->{category} = 'Badgers';
+        $expected_fields->{category} = 'Badgers & Voles';
         $expected_fields->{priority} = $rp2->id;
 
         my $new_values = $mech->visible_form_values('report_inspect_form');
@@ -724,15 +725,15 @@ FixMyStreet::override_config {
         $mech->submit_form(
             button => 'save',
             with_fields => {
-                category => 'Badgers',
+                category => 'Badgers & Voles',
                 include_update => 1,
                 public_update => 'This is a public update',
         });
 
         $report->discard_changes;
-        is $report->category, "Badgers", "Report in correct category";
+        is $report->category, "Badgers & Voles", "Report in correct category";
         is $report->comments->count, 1, "Only leaves one update";
-        like $report->comments->first->text, qr/Category changed.*Badgers/, 'update text included category change';
+        like $report->comments->first->text, qr/Category changed.*Badgers & Voles/, 'update text included category change';
     };
 
     subtest "test non-public changing" => sub {
@@ -779,10 +780,10 @@ FixMyStreet::override_config {
     });
     subtest "test report not resent when category changes if send_method doesn't change" => sub {
         $mech->get_ok("/report/$report3_id");
-        $mech->submit_form(button => 'save', with_fields => { category => 'Badgers', include_update => undef, });
+        $mech->submit_form(button => 'save', with_fields => { category => 'Badgers & Voles', include_update => undef, });
 
         $report3->discard_changes;
-        is $report3->category, "Badgers", "Report in correct category";
+        is $report3->category, "Badgers & Voles", "Report in correct category";
         isnt $report3->whensent, undef, "Report not marked as unsent";
         is $report3->bodies_str, $oxon->id, "Reported to OCC";
     };

t/cobrand/hackney.t

@@ -28,7 +28,7 @@ my $params = {
 my $hackney = $mech->create_body_ok(2508, 'Hackney Council', $params);
 my $contact = $mech->create_contact_ok(
     body_id => $hackney->id,
-    category => 'Potholes',
+    category => 'Potholes & stuff',
     email => 'pothole@example.org',
 );
 $contact->set_extra_fields( ( {
@@ -143,7 +143,7 @@ $p->delete;
 subtest "sends branded confirmation emails" => sub {
     $mech->log_out_ok;
     $mech->clear_emails_ok;
-    $mech->get_ok('/around');
+    $mech->get_ok('/?filter_category=Potholes+%26+stuff');
     FixMyStreet::override_config {
         ALLOWED_COBRANDS => [ 'hackney' ],
         MAPIT_URL => 'http://mapit.uk/',
@@ -159,6 +159,9 @@ subtest "sends branded confirmation emails" => sub {
         $mech->submit_form_ok( { with_fields => { pc => 'E8 1DY', } },
             "submit location" );
 
+        # While we're here, check the category with an ampersand (regression test)
+        $mech->content_contains('<option value="Potholes &amp; stuff" selected>');
+
         # click through to the report page
         $mech->follow_link_ok( { text_regex => qr/skip this step/i, },
             "follow 'skip this step' link" );

templates/web/base/admin/report-category.html

@@ -6,7 +6,8 @@
 [%~ END ~%]
 
 <select class="form-control" name="[% select_name %]" id="[% select_name %]">
-  [% IF NOT problem.category OR NOT categories_hash.${problem.category} %]
+  [% SET category_safe = mark_safe(problem.category) ~%]
+  [% IF NOT problem.category OR NOT categories_hash.$category_safe %]
     <optgroup label="[% loc('Existing category') %]">
         <option selected value="[% problem.category | html %]">[% (problem.category_display OR '-') | html %]</option>
     </optgroup>

templates/web/base/admin/triage/_list-filters.html

@@ -2,8 +2,9 @@
   [% IF filter_categories.size %]
     <select class="form-control js-multiple" name="filter_category" id="filter_categories" multiple data-all="[% loc('Everything') %]">
         [% FOR cat IN filter_categories %]
-            <option value="[% cat.category | html %]"[% ' selected' IF filter_category.${cat.category} %]>
-                [% cat.category_display | html %]
+          [%~ SET cat_safe = mark_safe(cat.category) %]
+            <option value="[% cat.category %]"[% ' selected' IF filter_category.$cat_safe %]>
+                [% cat.category_display %]
                 [%~ IF cat.get_extra_metadata('help_text') %] ([% cat.get_extra_metadata('help_text') %])[% END ~%]
             </option>
         [% END %]

templates/web/base/report/_inspect.html

@@ -18,13 +18,14 @@ <h2 class="inspect-form-heading">[% loc('Inspect report') %]</h2>
 
         [% FOREACH category IN category_options %]
           [% cat_name = category.category;
+             cat_safe = mark_safe(cat_name);
              cat_prefix = cat_name | lower | replace('[^a-z]', '');
              cat_prefix = "category_" _ cat_prefix _ "_" %]
             <p data-category="[% cat_name | html %]"
                [%~ IF cat_name != problem.category %] class="hidden"[% END %]
-               data-priorities='[% priorities_by_category.$cat_name | html %]'
-               data-templates='[% templates_by_category.$cat_name | html %]'>
-                [% INCLUDE 'report/new/category_extras_fields.html' metas=category_extras.$cat_name hide_notices=1 show_hidden=1 %]
+               data-priorities='[% priorities_by_category.$cat_safe | html %]'
+               data-templates='[% templates_by_category.$cat_safe | html %]'>
+                [% INCLUDE 'report/new/category_extras_fields.html' metas=category_extras.$cat_safe hide_notices=1 show_hidden=1 %]
             </p>
         [% END %]
 

templates/web/base/report/_item.html

@@ -5,8 +5,9 @@
 [%
 
 SET relevant_staff = 1;
-SET is_user_category = user_categories.${problem.category};
-IF (assigned_users_only.${problem.category} OR assigned_categories_only) AND NOT is_user_category;
+SET category_safe = mark_safe(problem.category);
+SET is_user_category = user_categories.$category_safe;
+IF (assigned_users_only.$category_safe OR assigned_categories_only) AND NOT is_user_category;
     SET relevant_staff = 0;
 END;
 

templates/web/base/report/new/form_user_loggedin.html

@@ -64,8 +64,9 @@
 [% IF c.user.has_permission_to("report_inspect", bodies_ids) OR c.user.has_permission_to("report_mark_private", bodies_ids) %]
     <div class="checkbox-group">
         <input type="checkbox" name="non_public" id="form_non_public" value="1"
-            [%~ ' checked' IF report.non_public OR non_public_categories.$category %]
-            [%~ ' disabled' IF non_public_categories.$category %]>
+            [%~ SET category_safe = mark_safe(category) %]
+            [%~ ' checked' IF report.non_public OR non_public_categories.$category_safe %]
+            [%~ ' disabled' IF non_public_categories.$category_safe %]>
         <label class="inline" for="form_non_public">[% loc('Private') %] </label>
     </div>
 [% END %]

templates/web/base/reports/_list-filters.html

@@ -2,7 +2,8 @@
 
 [% BLOCK category_options %]
     [% FOR cat IN categories %]
-        <option value="[% cat.category %]"[% ' selected' IF filter_category.${cat.category} OR ( filter_group AND ( cat.groups.grep(filter_group).size OR cat.category == filter_group ) ) %]>
+      [% SET cat_safe = mark_safe(cat.category) %]
+        <option value="[% cat.category %]"[% ' selected' IF filter_category.$cat_safe OR ( filter_group AND ( cat.groups.grep(filter_group).size OR cat.category == filter_group ) ) %]>
             [% cat.category_display %]
             [%~ IF cat.get_extra_metadata('help_text') %] ([% cat.get_extra_metadata('help_text') %])[% END ~%]
         </option>