Merge branch '3110-deep-linking-category-group'

13 files changed, 67 insertions, 35 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9f7dc615b..df449a480 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,7 @@
         - Improve Bing geocoder results.
         - Add option of checking passwords against Have I Been Pwned.
         - Add aerial maps option to Bing and OSM maps.
+        - Select matches for both filter category and group. #3110
     - Changes:
         - Mark user as active when sent an email alert.
     - Bugfixes:
@@ -26,6 +27,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..ba5f8c48a 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);
@@ -250,7 +250,10 @@ subtest 'check category, status and extra filtering works on /around' => sub {
     # Create one open and one fixed report in each category
     foreach my $category ( @$categories ) {
         my $contact = $mech->create_contact_ok( category => $category, body_id => $body->id, email => "$category\@example.org" );
-        if ($category ne 'Pothole') {
+        if ($category eq 'Vegetation') {
+            $contact->set_extra_metadata(group => ['Environment', 'Green']);
+            $contact->update;
+        } elsif ($category eq 'Flytipping') {
             $contact->set_extra_metadata(group => ['Environment']);
             $contact->update;
         }
@@ -281,6 +284,9 @@ subtest 'check category, status and extra filtering works on /around' => sub {
 
         $mech->get_ok( '/around?filter_group=Environment&bbox=' . $bbox );
         $mech->content_contains('<option value="Flytipping" selected>');
+
+        $mech->get_ok( '/around?filter_group=Environment&filter_category=Vegetation&bbox=' . $bbox );
+        $mech->content_like(qr/<optgroup label="Environment">.*?<option value="Vegetation" selected>.*?<optgroup label="Green">.*?<option value="Vegetation">/s);
     };
 
     $json = $mech->get_ok_json( '/around?ajax=1&filter_category=Pothole&bbox=' . $bbox );
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/_category_select.html b/templates/web/base/report/new/_category_select.html
index d5aa9842b..61353647d 100644
--- a/templates/web/base/report/new/_category_select.html
+++ b/templates/web/base/report/new/_category_select.html
@@ -1,3 +1,4 @@
+[% SET filter_group = c.get_param('filter_group') %]
 [%~ IF category_groups.size ~%]
     [%~ FOREACH group IN category_groups ~%]
         [% IF group.name %]<optgroup label="[% group.name %]">[% END %]
diff --git a/templates/web/base/report/new/category.html b/templates/web/base/report/new/category.html
index b5bfd0251..37479e4a5 100644
--- a/templates/web/base/report/new/category.html
+++ b/templates/web/base/report/new/category.html
@@ -10,7 +10,7 @@ END
 [% IF category_options.size OR category_groups.size ~%]
     [%~ BLOCK category_option ~%]
     [% cat_lc = cat.category | lower =%]
-    <option value='[% cat.category | html %]'[% ' selected' IF report.category == cat.category || category_lc == cat_lc ~%]
+    <option value='[% cat.category | html %]'[% ' selected' IF ( report.category == cat.category || category_lc == cat_lc ) AND ( NOT filter_group OR filter_group == group.name ) ~%]
     >[% IF loop.first %][% cat.category_display %][% ELSE %][% cat.category_display | html %][% END %]
     [%~ IF cat.get_extra_metadata('help_text') %] ([% cat.get_extra_metadata('help_text') %])[% END ~%]
     </option>
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..9973a0c1f 100644
--- a/templates/web/base/reports/_list-filters.html
+++ b/templates/web/base/reports/_list-filters.html
@@ -1,8 +1,10 @@
 [% select_status = PROCESS 'reports/_list-filter-status.html' %]
 
+[%# We want to only select things that match all filters, if filters are provided ~%]
 [% 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.size OR filter_group ) AND ( NOT filter_category.size OR filter_category.$cat_safe ) AND ( NOT filter_group OR filter_group == group.name ) %]>
             [% cat.category_display %]
             [%~ IF cat.get_extra_metadata('help_text') %] ([% cat.get_extra_metadata('help_text') %])[% END ~%]
         </option>
diff --git a/web/cobrands/fixmystreet/fixmystreet.js b/web/cobrands/fixmystreet/fixmystreet.js
index d79f17049..ca36d5b46 100644
--- a/web/cobrands/fixmystreet/fixmystreet.js
+++ b/web/cobrands/fixmystreet/fixmystreet.js
@@ -1310,6 +1310,20 @@ fixmystreet.update_pin = function(lonlat, savePushState) {
     }
 };
 
+(function() { // fetch_reporting_data closure
+
+function re_select(group, category) {
+    var cat_in_group = $("#form_category optgroup[label=\"" + group + "\"] option[value=\"" + category + "\"]");
+    if (cat_in_group.length) {
+        cat_in_group.prop({selected:true});
+        return true;
+    } else if ($("#form_category option[value=\"" + category + "\"]").length) {
+        $("#form_category").val(category);
+        return true;
+    }
+    return false;
+}
+
 fixmystreet.fetch_reporting_data = function() {
     $.getJSON('/report/new/ajax', {
         latitude: $('#fixmystreet\\.latitude').val(),
@@ -1364,16 +1378,12 @@ fixmystreet.fetch_reporting_data = function() {
         }
 
         $('#form_category_row').html(data.category);
-        var cat_in_group = $("#form_category optgroup[label=\"" + old_category_group + "\"] option[value=\"" + old_category + "\"]");
-        if (cat_in_group.length) {
-            cat_in_group.prop({selected:true});
-        } else if ($("#form_category option[value=\"" + old_category + "\"]").length) {
-            $("#form_category").val(old_category);
-        } else if (filter_category !== undefined && $("#form_category option[value='" + filter_category + "']").length) {
+        var reselected = re_select(old_category_group, old_category);
+        if (!reselected && filter_category !== undefined) {
             // If the category filter appears on the map and the user has selected
             // something from it, then pre-fill the category field in the report,
             // if it's a value already present in the drop-down.
-            $("#form_category").val(filter_category);
+            re_select(old_category_group, filter_category);
         }
 
         fixmystreet.set_up.category_groups(old_category_group);
@@ -1408,6 +1418,8 @@ fixmystreet.fetch_reporting_data = function() {
     });
 };
 
+})(); // fetch_reporting_data closure
+
 fixmystreet.display = {
   begin_report: function(lonlat, saveHistoryState) {
     lonlat = fixmystreet.maps.begin_report(lonlat);