aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMatthew Somerville <matthew@mysociety.org>2020-07-15 16:02:21 +0100
committerMatthew Somerville <matthew@mysociety.org>2020-07-16 14:58:12 +0100
commit88c607fa378951491c7ec0f3384bc8f27fbca467 (patch)
tree3697d45c0ce00625f416c8fc61c0e0cd87fb4cd7
parent8a37b57398e09ff13a7370b5119de734ecbc772a (diff)
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 527d763b.
-rw-r--r--CHANGELOG.md1
-rw-r--r--t/app/controller/around.t4
-rw-r--r--t/app/controller/report_inspect.t23
-rw-r--r--t/cobrand/hackney.t7
-rw-r--r--templates/web/base/admin/report-category.html3
-rw-r--r--templates/web/base/admin/triage/_list-filters.html5
-rw-r--r--templates/web/base/report/_inspect.html7
-rw-r--r--templates/web/base/report/_item.html5
-rw-r--r--templates/web/base/report/new/form_user_loggedin.html5
-rw-r--r--templates/web/base/reports/_list-filters.html3
10 files changed, 37 insertions, 26 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2af6f052c..f724b8f98 100644
--- a/CHANGELOG.md
+++ b/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
diff --git a/t/app/controller/around.t b/t/app/controller/around.t
index 784801517..e35406bb6 100644
--- a/t/app/controller/around.t
+++ b/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);
diff --git a/t/app/controller/report_inspect.t b/t/app/controller/report_inspect.t
index 2852f8d18..de9d689cd 100644
--- a/t/app/controller/report_inspect.t
+++ b/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 &amp; 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";
};
diff --git a/t/cobrand/hackney.t b/t/cobrand/hackney.t
index b5a629e33..374bf2ea9 100644
--- a/t/cobrand/hackney.t
+++ b/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" );
diff --git a/templates/web/base/admin/report-category.html b/templates/web/base/admin/report-category.html
index e76106f7f..55b9e2ea5 100644
--- a/templates/web/base/admin/report-category.html
+++ b/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>
diff --git a/templates/web/base/admin/triage/_list-filters.html b/templates/web/base/admin/triage/_list-filters.html
index dd3e17875..29f48f0bb 100644
--- a/templates/web/base/admin/triage/_list-filters.html
+++ b/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 %]
diff --git a/templates/web/base/report/_inspect.html b/templates/web/base/report/_inspect.html
index a8be342d0..88fbdd70b 100644
--- a/templates/web/base/report/_inspect.html
+++ b/templates/web/base/report/_inspect.html
@@ -18,13 +18,14 @@
[% 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 %]
diff --git a/templates/web/base/report/_item.html b/templates/web/base/report/_item.html
index 6f567de01..9a5168986 100644
--- a/templates/web/base/report/_item.html
+++ b/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;
diff --git a/templates/web/base/report/new/form_user_loggedin.html b/templates/web/base/report/new/form_user_loggedin.html
index 0d259e695..d26948fc2 100644
--- a/templates/web/base/report/new/form_user_loggedin.html
+++ b/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 %]
diff --git a/templates/web/base/reports/_list-filters.html b/templates/web/base/reports/_list-filters.html
index c5a8eaa75..ee9815157 100644
--- a/templates/web/base/reports/_list-filters.html
+++ b/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>