diff options
author | Matthew Somerville <matthew@mysociety.org> | 2020-07-15 16:02:21 +0100 |
---|---|---|
committer | Matthew Somerville <matthew@mysociety.org> | 2020-07-16 14:58:12 +0100 |
commit | 88c607fa378951491c7ec0f3384bc8f27fbca467 (patch) | |
tree | 3697d45c0ce00625f416c8fc61c0e0cd87fb4cd7 | |
parent | 8a37b57398e09ff13a7370b5119de734ecbc772a (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.md | 1 | ||||
-rw-r--r-- | t/app/controller/around.t | 4 | ||||
-rw-r--r-- | t/app/controller/report_inspect.t | 23 | ||||
-rw-r--r-- | t/cobrand/hackney.t | 7 | ||||
-rw-r--r-- | templates/web/base/admin/report-category.html | 3 | ||||
-rw-r--r-- | templates/web/base/admin/triage/_list-filters.html | 5 | ||||
-rw-r--r-- | templates/web/base/report/_inspect.html | 7 | ||||
-rw-r--r-- | templates/web/base/report/_item.html | 5 | ||||
-rw-r--r-- | templates/web/base/report/new/form_user_loggedin.html | 5 | ||||
-rw-r--r-- | templates/web/base/reports/_list-filters.html | 3 |
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 & 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 & 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> |