From 1903243461876327871aa289219d5fc8ee2221d9 Mon Sep 17 00:00:00 2001
From: Matthew Somerville <matthew@mysociety.org>
Date: Wed, 18 Sep 2024 14:06:08 +0100
Subject: [PATCH] Capture group from hoisted categories.

If a category is in a group on its own, the reporting form hoists it up
to the top level, to save a pointless extra step. In doing so, it was no
longer getting the associated group to store for submission. Rewrite the
input so it is clearer as to what we are receiving.
---
 .../cypress/integration/category_tests.js     |  6 +++---
 perllib/FixMyStreet/App/Controller/Report.pm  |  1 +
 .../FixMyStreet/App/Controller/Report/New.pm  | 21 +++++++++++--------
 t/app/controller/auth_social.t                |  6 +++---
 t/app/controller/report_new.t                 | 20 +++++++++++++-----
 t/cobrand/brent.t                             |  8 +++----
 t/cobrand/northumberland.t                    |  6 +++---
 templates/web/base/report/new/category.html   | 19 +++++++++++++----
 web/cobrands/fixmystreet/fixmystreet.js       |  6 +++---
 9 files changed, 59 insertions(+), 34 deletions(-)

diff --git a/.cypress/cypress/integration/category_tests.js b/.cypress/cypress/integration/category_tests.js
index 8cd72b50db3..bcb543a3cdf 100644
--- a/.cypress/cypress/integration/category_tests.js
+++ b/.cypress/cypress/integration/category_tests.js
@@ -13,7 +13,7 @@ describe('Basic categories', function() {
         'Footpath/bridleway away from road',
         'Graffiti',
         'Offensive graffiti',
-        'Licensing',
+        'G|Licensing',
         'Parks/landscapes',
         'Pavements',
         'Potholes',
@@ -75,7 +75,7 @@ describe('Basic categories', function() {
         cy.get('[value="Abandoned vehicles"]').should('not.be.visible');
         cy.get('[value="Bus stops"]').should('not.be.visible');
         cy.get('[value="Flyposting"]').should('not.be.visible');
-        cy.get('[value="Licensing"]').should('be.visible');
+        cy.get('[value="G|Licensing"]').should('be.visible');
         cy.get('[value="Dropped Kerbs"]').should('be.visible');
         cy.get('[value="Skips"]').should('be.visible');
         cy.get('[value="Street lighting"]').should('be.visible');
@@ -84,7 +84,7 @@ describe('Basic categories', function() {
         cy.get('[value="Abandoned vehicles"]').should('not.be.visible');
         cy.get('[value="Bus stops"]').should('be.visible');
         cy.get('[value="Flyposting"]').should('not.be.visible');
-        cy.get('[value="Licensing"]').should('be.visible');
+        cy.get('[value="G|Licensing"]').should('be.visible');
         cy.get('[value="Dropped Kerbs"]').should('not.be.visible');
         cy.get('[value="Skips"]').should('be.visible');
         cy.get('[value="Road traffic signs"]').should('be.visible');
diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm
index 1971087777a..785c4708d1f 100644
--- a/perllib/FixMyStreet/App/Controller/Report.pm
+++ b/perllib/FixMyStreet/App/Controller/Report.pm
@@ -843,6 +843,7 @@ sub stash_category_groups : Private {
             (my $id = $_) =~ s/[^a-zA-Z]+//g;
             if (@{$category_groups{$_}} == 1) {
                 my $contact = $category_groups{$_}[0];
+                $contact->set_extra_metadata(hoisted => $_);
                 push @list, [ $contact->category_display, $contact ];
             } else {
                 my $cats = $category_groups{$_};
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm
index 400548923f5..ad1b1731ae8 100644
--- a/perllib/FixMyStreet/App/Controller/Report/New.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/New.pm
@@ -1823,19 +1823,22 @@ sub generate_map : Private {
 sub check_for_category : Private {
     my ( $self, $c, $opts ) = @_;
 
-    my $category;
-    if (!$opts->{with_group}) {
-        $category = $c->get_param('category') || '';
-    } else {
-        # Group is either an actual group, or a category that wasn't in a group
-        my $group = $c->get_param('category') || $c->get_param('filter_group') || '';
-        if (any { $_->{name} && $group eq $_->{name} } @{$c->stash->{category_groups}}) {
+    my $category = $c->get_param('category') || '';
+    if ($opts->{with_group}) {
+        if (my ($group) = $category =~ /^G\|(.*)/) {
+            # A top-level group
             $c->stash->{group} = $c->stash->{filter_group} = $group;
             (my $group_id = $group) =~ s/[^a-zA-Z]+//g;
             my $cat_param = "category.$group_id";
             $category = $c->get_param($cat_param);
-        } else {
-            $category = $group;
+        } elsif ($category =~ /^H\|(.*?)\|(.*)/) {
+            # A hoisted to top-level category
+            ($group, $category) = ($1, $2);
+            $c->stash->{group} = $c->stash->{filter_group} = $group;
+        } elsif (!$category && ($group = $c->get_param('filter_group'))) {
+            if (any { $_->{name} && $group eq $_->{name} } @{$c->stash->{category_groups}}) {
+                $c->stash->{group} = $c->stash->{filter_group} = $group;
+            }
         }
     }
     $category ||= $c->stash->{report}->category || '';
diff --git a/t/app/controller/auth_social.t b/t/app/controller/auth_social.t
index 4211e560548..6a4fc89972b 100644
--- a/t/app/controller/auth_social.t
+++ b/t/app/controller/auth_social.t
@@ -129,7 +129,7 @@ for my $state ( 'refused', 'no email', 'existing UID', 'okay' ) {
                 $mech->submit_form_ok( { with_fields => { pc => $test->{pc} || 'SW1A1AA' } }, "submit location" );
                 $mech->follow_link_ok( { text_regex => qr/skip this step/i, }, "follow 'skip this step' link" );
                 $mech->submit_form(with_fields => {
-                    category => 'Bins',
+                    category => 'G|Bins',
                     'category.Bins' => 'Damaged bin',
                     title => 'Test title',
                     detail => 'Test detail',
@@ -173,7 +173,7 @@ for my $state ( 'refused', 'no email', 'existing UID', 'okay' ) {
             if ($page eq 'report') {
                 $mech->content_contains('/report/new');
                 $mech->content_contains('Salt bin');
-                $mech->content_contains('name="category" value="Bins" data-subcategory="Bins" checked');
+                $mech->content_like(qr{value="G|Bins"\s+data-subcategory="Bins" checked});
                 $mech->content_contains('name="category.Bins" data-category_display="Damaged bin" value=\'Damaged bin\' checked');
             } elsif ($page eq 'update') {
                 $mech->content_contains('/report/update');
@@ -319,7 +319,7 @@ for my $tw_state ( 'refused', 'existing UID', 'no email' ) {
                 $mech->submit_form_ok( { with_fields => { pc => 'SW1A1AA' } }, "submit location" );
                 $mech->follow_link_ok( { text_regex => qr/skip this step/i, }, "follow 'skip this step' link" );
                 $mech->submit_form(with_fields => {
-                    category => 'Bins',
+                    category => 'G|Bins',
                     'category.Bins' =>'Damaged bin',
                     title => 'Test title',
                     detail => 'Test detail',
diff --git a/t/app/controller/report_new.t b/t/app/controller/report_new.t
index 08c0496bb09..624b0abb63f 100644
--- a/t/app/controller/report_new.t
+++ b/t/app/controller/report_new.t
@@ -680,9 +680,9 @@ subtest "category groups" => sub {
         my $div = '<div[^>]*>\s*';
         my $div_end = '</div>\s*';
         my $pavements_label = '<label[^>]* for="category_Pavements">Pavements</label>\s*' . $div_end;
-        my $pavements_input = '<input[^>]* value="Pavements" data-subcategory="Pavements">\s*';
-        my $pavements_input_checked = '<input[^>]* value="Pavements" data-subcategory="Pavements" checked>\s*';
-        my $roads = $div . '<input[^>]* value="Roads" data-subcategory="Roads">\s*<label[^>]* for="category_Roads">Roads</label>\s*' . $div_end;
+        my $pavements_input = '<input[^>]* value="G|Pavements"\s+data-subcategory="Pavements">\s*';
+        my $pavements_input_checked = '<input[^>]* value="G|Pavements"\s+data-subcategory="Pavements" checked>\s*';
+        my $roads = $div . '<input[^>]* value="G|Roads"\s+data-subcategory="Roads">\s*<label[^>]* for="category_Roads">Roads</label>\s*' . $div_end;
         my $trees_label = '<label [^>]* for="category_\d+">Trees</label>\s*' . $div_end;
         my $trees_input = $div . '<input[^>]* value=\'Trees\'>\s*';
         my $trees_input_checked = $div . '<input[^>]* value=\'Trees\' checked>\s*';
@@ -708,18 +708,28 @@ subtest "category groups" => sub {
         $mech->content_like(qr{$fieldset_pavements$options});
         $mech->content_like(qr{$fieldset_roads$options});
         # Server submission of pavement subcategory
-        $mech->get_ok("/report/new?lat=$saved_lat&lon=$saved_lon&category=Pavements&category.Pavements=Potholes");
+        $mech->get_ok("/report/new?lat=$saved_lat&lon=$saved_lon&category=G|Pavements&category.Pavements=Potholes");
         $mech->content_like(qr{$pavements_input_checked$pavements_label$roads$trees_input$trees_label</fieldset>});
         $mech->content_like(qr{$fieldset_pavements$optionsS});
         $mech->content_like(qr{$fieldset_roads$options});
 
         $contact9->update( { extra => { group => 'Lights' } } );
         $mech->get_ok("/report/new?lat=$saved_lat&lon=$saved_lon");
-        $streetlighting = $div . '<input[^>]*value=\'Street lighting\'>\s*<label[^>]* for="category_\d+">Street lighting</label>\s*' . $div_end;
+        $streetlighting = $div . '<input[^>]*value=\'H|Lights\|Street lighting\'>\s*<label[^>]* for="category_\d+">Street lighting</label>\s*' . $div_end;
+        $potholes_input = $div . '<input[^>]* value=\'H|Pavements\|Potholes\'>\s*';
         $potholes_label = '<label[^>]* for="category_\d+">Potholes</label>\s*' . $div_end;
         $mech->content_like(qr{$potholes_input$potholes_label$roads$streetlighting$trees_input$trees_label</fieldset>});
         $mech->content_unlike(qr{$fieldset_pavements});
         $mech->content_like(qr{$fieldset_roads$options});
+
+        $mech->submit_form_ok({ with_fields => {
+            category => 'H|Lights|Street lighting',
+            title => 'Test Report',
+            detail => 'Test report details',
+            username_register => 'jo@example.org',
+            name => 'Jo Bloggs',
+        } });
+        $mech->content_contains('Now check your email');
     };
 };
 
diff --git a/t/cobrand/brent.t b/t/cobrand/brent.t
index f3b93eb03d3..799a8294249 100644
--- a/t/cobrand/brent.t
+++ b/t/cobrand/brent.t
@@ -927,7 +927,7 @@ FixMyStreet::override_config {
             with_fields => {
                 title => "Test Report",
                 detail => 'Test report details.',
-                category => 'Parks and open spaces',
+                category => 'G|Parks and open spaces',
                 'category.Parksandopenspaces' => 'Overgrown grass',
             }
         }, "submit details");
@@ -951,7 +951,7 @@ FixMyStreet::override_config {
             with_fields => {
                 title => "Test Report",
                 detail => 'Test report details.',
-                category => 'Parks and open spaces',
+                category => 'G|Parks and open spaces',
                 'category.Parksandopenspaces' => 'Overgrown grass',
             }
         }, "submit details");
@@ -970,7 +970,7 @@ FixMyStreet::override_config {
             with_fields => {
                 title => "Test Report",
                 detail => 'Test report details.',
-                category => 'Parks and open spaces',
+                category => 'G|Parks and open spaces',
                 'category.Parksandopenspaces' => 'Overgrown grass',
             }
         }, "submit details");
@@ -994,7 +994,7 @@ FixMyStreet::override_config {
             with_fields => {
                 title => "Test Report",
                 detail => 'Test report details.',
-                category => 'Parks and open spaces',
+                category => 'G|Parks and open spaces',
                 'category.Parksandopenspaces' => 'Ponds',
             }
         }, "submit details");
diff --git a/t/cobrand/northumberland.t b/t/cobrand/northumberland.t
index 0e5d51c38fb..7764597bcd9 100644
--- a/t/cobrand/northumberland.t
+++ b/t/cobrand/northumberland.t
@@ -87,7 +87,7 @@ FixMyStreet::override_config {
         $mech->content_contains('Main Carriageway');
         $mech->content_contains('Potholes');
         $mech->content_contains("Trees'>");
-        $mech->content_contains('value=\'Flytipping\' data-nh="1"');
+        $mech->content_contains('value=\'H|Staff Only - Out Of Hours|Flytipping\' data-nh="1"');
 
         # A-road where NH responsible for litter, council categories will also be present
         mock_road("A5103", 1);
@@ -97,7 +97,7 @@ FixMyStreet::override_config {
         $mech->content_contains('Main Carriageway');
         $mech->content_contains('Potholes');
         $mech->content_contains('Trees\'>');
-        $mech->content_contains('value=\'Flytipping\' data-nh="1"');
+        $mech->content_contains('value=\'H|Staff Only - Out Of Hours|Flytipping\' data-nh="1"');
 
         # A-road where NH not responsible for litter, no NH litter categories
         mock_road("A34", 0);
@@ -107,7 +107,7 @@ FixMyStreet::override_config {
         $mech->content_lacks('Main Carriageway');
         $mech->content_contains('Potholes');
         $mech->content_contains('Trees\'>');
-        $mech->content_contains('value=\'Flytipping\' data-nh="1"');
+        $mech->content_contains('value=\'H|Staff Only - Out Of Hours|Flytipping\' data-nh="1"');
     };
 };
 
diff --git a/templates/web/base/report/new/category.html b/templates/web/base/report/new/category.html
index e83a73a3b85..7efcd900ebf 100644
--- a/templates/web/base/report/new/category.html
+++ b/templates/web/base/report/new/category.html
@@ -24,7 +24,12 @@
         [%~ FOREACH group_or_cat IN category_groups ~%]
             <div class="govuk-radios__item">
           [% IF group_or_cat.name %]
-                <input class="govuk-radios__input" required id="category_[% group_or_cat.id %]" type="radio" name="category" value="[% group_or_cat.name %]" data-subcategory="[% group_or_cat.id %]"[% ' checked' IF filter_group == group_or_cat.name %]>
+                <input class="govuk-radios__input" required type="radio" name="category"
+                    id="category_[% group_or_cat.id %]"
+                    data-valuealone="[% group_or_cat.name %]"
+                    value="G|[% group_or_cat.name %]"
+                    data-subcategory="[% group_or_cat.id %]"
+                    [%~ ' checked' IF filter_group == group_or_cat.name %]>
             <label class="govuk-label govuk-radios__label" for="category_[% group_or_cat.id %]">[% group_or_cat.name %]</label>
             [% group_hint = group_or_cat.categories.first.get_extra_metadata('group_hint') %]
             [%~ IF group_hint %]
@@ -32,9 +37,15 @@
               [% group_hint | safe %]
             </div>
             [% END ~%]
-          [% ELSE # A category not in a group %]
-            [% cat_lc = group_or_cat.category | lower =%]
-            <input class="govuk-radios__input" required id="category_[% group_or_cat.id %]" type="radio" name="category" data-category_display="[% group_or_cat.category_display %]" value='[% group_or_cat.category %]'
+          [% ELSE # A category not in a group, or a hoisted category %]
+            [% cat_lc = group_or_cat.category | lower;
+                hoisted = group_or_cat.get_extra_metadata('hoisted');
+            =%]
+            <input class="govuk-radios__input" required type="radio" name="category"
+                id="category_[% group_or_cat.id %]"
+                data-category_display="[% group_or_cat.category_display %]"
+                data-valuealone="[% group_or_cat.category %]"
+                value='[% "H|" _ hoisted _ "|" IF hoisted %][% group_or_cat.category %]'
                 [%~ ' checked' IF ( report.category == group_or_cat.category || category_lc == cat_lc ) AND NOT filter_group ~%]
                 [%~ ' data-nh="1"' IF group_or_cat.get_extra_metadata('nh_council_cleaning') ~%]
                 >
diff --git a/web/cobrands/fixmystreet/fixmystreet.js b/web/cobrands/fixmystreet/fixmystreet.js
index de09f6df3da..0e7716e2f4e 100644
--- a/web/cobrands/fixmystreet/fixmystreet.js
+++ b/web/cobrands/fixmystreet/fixmystreet.js
@@ -1848,11 +1848,11 @@ function re_select(group, category) {
     var group_id = group.replace(/[^a-z]+/gi, '');
     var cat_in_group = $("#subcategory_" + group_id + " input[value=\"" + category + "\"]");
     if (cat_in_group.length) {
-        $('#form_category_fieldset input[value="' + group + '"]')[0].checked = true;
+        $('#form_category_fieldset input[data-valuealone="' + group + '"]')[0].checked = true;
         cat_in_group[0].checked = true;
     } else {
         var top_level = group || category;
-        var top_level_match = $("#form_category_fieldset input[value=\"" + top_level + "\"]");
+        var top_level_match = $("#form_category_fieldset input[data-valuealone=\"" + top_level + "\"]");
         if (top_level && top_level_match.length) {
             top_level_match[0].checked = true;
         }
@@ -1976,7 +1976,7 @@ fixmystreet.fetch_reporting_data = function() {
 fixmystreet.reporting = {};
 fixmystreet.reporting.selectedCategory = function() {
     var $group_or_cat_input = $('#form_category_fieldset input:checked'),
-        group_or_cat = $group_or_cat_input.val() || '',
+        group_or_cat = $group_or_cat_input.data('valuealone') || '',
         group_id = group_or_cat.replace(/[^a-z]+/gi, ''),
         $subcategory = $("#subcategory_" + group_id),
         $subcategory_input = $subcategory.find('input:checked'),