From f9cb3dc032d1a118e340eb4dd1db318a5138c899 Mon Sep 17 00:00:00 2001
From: Moray Jones <moray@mysociety.org>
Date: Fri, 22 Mar 2024 17:54:45 +0000
Subject: [PATCH] [Northumberland] Being able to filter on only one category in
 multiple parent categories

Set up categories in index to include name of their group.

Use this group name to check if a report is in the
selected category (through extra->group).

If there are reports that don't identify as being in that category, then
still use a 'Multiple' category.

https://github.com/mysociety/societyworks/issues/4039
---
 .../FixMyStreet/App/Controller/Dashboard.pm   | 91 ++++++++++++++-----
 perllib/FixMyStreet/Reporting.pm              |  2 +-
 t/app/controller/dashboard.t                  | 35 +++++--
 templates/web/base/dashboard/index.html       | 14 +--
 4 files changed, 104 insertions(+), 38 deletions(-)

diff --git a/perllib/FixMyStreet/App/Controller/Dashboard.pm b/perllib/FixMyStreet/App/Controller/Dashboard.pm
index c354d51e200..05dcdcc0fdc 100644
--- a/perllib/FixMyStreet/App/Controller/Dashboard.pm
+++ b/perllib/FixMyStreet/App/Controller/Dashboard.pm
@@ -136,24 +136,42 @@ sub index : Path : Args(0) {
         }
 
         my %group_names = map { $_->{name} => $_->{categories} } @{$c->stash->{category_groups}};
-        # See if we've had anything from the body dropdowns
-        $c->stash->{category} = [ $c->get_param_list('category') ];
-        my @remove_from_display;
-
-        foreach (@{$c->stash->{category}}) {
-            next unless /^group-(.*)/;
-            for my $contact (@{$group_names{$1}}) {
-                push @{ $c->stash->{category} }, $contact->category;
-                push @remove_from_display, $contact->category;
+
+        # Categories being received are expected to carry group information
+        # from the interface. A category can be:
+        # 'group-Roads' -> A master category 'Roads' which will contain subcategories
+        # 'Potholes-group-Roads' -> A 'Potholes' category under roads
+        # 'Potholes-group-' -> A 'Potholes' category not under any other category
+        # We need to reformat any categories that may be manually put into the url
+        # which may be bookmarks from before grouping was added
+
+        my %display_categories;
+        foreach my $display ($c->get_param_list('category')) {
+            if ($display !~ /group-/) {
+                $display = $display . '-group-';
             }
+            $display_categories{$display} = 1;
         }
 
-        my %display_categories = map { $_ => 1 } @{$c->stash->{category}};
-        delete $display_categories{$_} for (@remove_from_display);
-        $c->stash->{display_categories} = \%display_categories;
-
-        @{$c->stash->{category}} = grep { $_ !~ /^group-/} @{$c->stash->{category}};
+        my (@categories, @groups_selected_from);
+        for my $param (keys %display_categories) {
+            if ($param =~ /\-group\-/) {
+                my ($category, $group) = split(/\-group\-/, $param);
+                push @categories, $category;
+                push @groups_selected_from, $group if $group;
+                $display_categories{$param} = $category;
+            } elsif ($param =~ /^group-(.*)/) {
+                push @groups_selected_from, $1;
+                $display_categories{$param} = 'group-' . $1;
+                for my $contact (@{$group_names{$1}}) {
+                    push @categories, $contact->category;
+                }
+            }
+        };
 
+        $c->stash->{category} = \@categories;
+        $c->stash->{groups_selected_from} = \@groups_selected_from;
+        $c->stash->{display_categories} = { reverse %display_categories };
         $c->stash->{ward} = [ $c->get_param_list('ward') ];
 
         if ($c->user_exists) {
@@ -256,18 +274,28 @@ sub generate_grouped_data : Private {
         %grouped = map { $_->category => {} } @{$c->stash->{contacts}};
     }
     my $problems = $c->stash->{objects_rs}->search(undef, {
-        group_by => [ map { ref $_ ? $_->{-as} : $_ } @groups ],
-        select   => [ @groups, { count => 'me.id' } ],
-        as       => [ @groups == 2 ? qw/key1 key2 count/ : qw/key1 count/ ],
+        group_by => [ map { ref $_ ? $_->{-as} : $_ } (@groups, 'me.extra') ],
+        select   => [ @groups, { count => 'me.id' }, 'me.extra' ],
+        as       => [ @groups == 2 ? qw/key1 key2 count extra/ : qw/key1 count extra/ ],
     } );
     $c->stash->{group_by} = $group_by;
 
     my %columns;
     while (my $p = $problems->next) {
         my %cols = $p->get_columns;
-        my ($col1, $col2) = ($cols{key1}, $cols{key2});
+        my ($col1, $col2, $col3) = ($cols{key1}, $cols{key2}, $cols{extra});
         if ($group_by eq 'category+state') {
             $col2 = $state_map->{$cols{key2}};
+        }
+        if ($group_by eq 'category+state' || $group_by eq 'category') {
+            if ($col3 && decode_json($col3)->{group}) {
+                my $group = decode_json($col3)->{group};
+                if (grep { /$group/ } @{$c->stash->{groups_selected_from}} ) {
+                    $col1 = decode_json($col3)->{group} . "::" . $col1;
+                } else {
+                    next;
+                }
+            }
         } elsif ($group_by eq 'month') {
             $col1 = Time::Piece->strptime("2017-$cols{key1}-01", '%Y-%m-%d')->fullmonth;
         }
@@ -295,6 +323,7 @@ sub generate_grouped_data : Private {
             $am <=> $bm;
         } @rows;
     } elsif ($group_by eq 'category+state' || $group_by eq 'category') {
+        my @subcategory_specific = grep { $_ =~ /::/ } @rows;
         @rows = ();
         my @sorting_categories;
         my %category_to_group;
@@ -302,16 +331,30 @@ sub generate_grouped_data : Private {
             for my $category (@{$group->{categories}}) {
                 push @sorting_categories, $category->category;
                 if (!$category_to_group{$category->category}) {
-                    $category_to_group{$category->category} = $group->{name};
+                    $category_to_group{$category->category}{group} = $group->{name} || '';
+                    $category_to_group{$category->category}{display_name} = $category->category;
                 } else {
-                    $category_to_group{$category->category} = 'Multiple';
+                    $category_to_group{$category->category}{group} = 'Multiple';
+                    $category_to_group{$category->category}{display_name} = $category->category;
                 }
             }
         };
-        my ($single_group, $multiple_groups) = part { $category_to_group{$_} eq 'Multiple'} @sorting_categories;
-        my @multiple = sort (uniq(@$multiple_groups));
-
-        push @rows, @$single_group if $single_group;
+        for (@subcategory_specific) {
+            my ($group, $display_name) = split(/::/, $_);
+            push @sorting_categories, $_;
+            $category_to_group{$_}{group} = $group;
+            $category_to_group{$_}{display_name} = $display_name;
+        }
+        my ($single_group, $multiple_groups) = part { $category_to_group{$_}{group} eq 'Multiple'} @sorting_categories;
+        my @single_group = sort {
+            $category_to_group{$a}{group} cmp $category_to_group{$b}{group}
+            || $category_to_group{$a}{display_name} cmp $category_to_group{$b}{display_name}
+            } (@$single_group);
+        my @multiple = sort {
+            $category_to_group{$a}{group} cmp $category_to_group{$b}{group}
+            || $category_to_group{$a}{display_name} cmp $category_to_group{$b}{display_name}
+            } (uniq(@$multiple_groups));
+        push @rows, @single_group if scalar @single_group;
         push @rows, @multiple if scalar @multiple;
         $c->stash->{category_to_group} = \%category_to_group;
     } else {
diff --git a/perllib/FixMyStreet/Reporting.pm b/perllib/FixMyStreet/Reporting.pm
index 0adc06314e0..a8ac077419a 100644
--- a/perllib/FixMyStreet/Reporting.pm
+++ b/perllib/FixMyStreet/Reporting.pm
@@ -95,7 +95,7 @@ has filename => ( is => 'rw', isa => Str, lazy => 1, default => sub {
         start_date => $self->start_date,
         end_date => $self->end_date,
     );
-    $where{category} = @{$self->category} < 3 ? join(',', @{$self->category}) : 'multiple-categories';
+    $where{category} = @{$self->category} < 3 ? join(',', sort @{$self->category}) : 'multiple-categories';
     $where{body} = $self->body->id if $self->body;
     $where{role} = $self->role_id if $self->role_id;
     my $host = URI->new($self->cobrand->base_url)->host;
diff --git a/t/app/controller/dashboard.t b/t/app/controller/dashboard.t
index afc7ba76e7f..3292d2ba5ba 100644
--- a/t/app/controller/dashboard.t
+++ b/t/app/controller/dashboard.t
@@ -37,7 +37,7 @@ my @cats = ('Litter', 'Other', 'Potholes', 'Traffic lights & bells', 'White line
 for my $contact ( @cats ) {
     my $c = $mech->create_contact_ok(body_id => $body->id, category => $contact, email => "$contact\@example.org");
     if ($contact eq 'Potholes' || $contact eq 'White lines') {
-        $c->set_extra_metadata(group => ['Road & more']);
+        $c->set_extra_metadata(group => ['Road & more', 'Pavements']);
         $c->update;
     }
 }
@@ -161,7 +161,7 @@ FixMyStreet::override_config {
 
     subtest 'The correct categories and totals shown by default' => sub {
         $mech->get_ok("/dashboard");
-        my $expected_cats = [ 'Litter', 'Other', 'Traffic lights & bells', 'All Road & more', 'Potholes', 'White lines' ];
+        my $expected_cats = [ 'Litter', 'Other', 'Traffic lights & bells', 'All Pavements', 'Potholes', 'White lines', 'All Road & more', 'Potholes', 'White lines' ];
         my $res = $categories->scrape( $mech->content );
         $mech->content_contains('<optgroup label="Road &amp; more">');
         $mech->content_contains('<option value="group-Road &amp; more"');
@@ -182,21 +182,42 @@ FixMyStreet::override_config {
         my $end = DateTime->now->subtract(months => 1)->strftime('%Y-%m-%d');
         $mech->submit_form_ok({ with_fields => { state => '', start_date => $start, end_date => $end } });
         test_table($mech->content, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 3);
-        $mech->get_ok("/dashboard?category=Litter&category=Potholes");
+        $mech->get_ok("/dashboard?category=Litter-group-&category=Potholes-group-");
         test_table($mech->content, 1, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 4, 6, 0, 0, 0, 0, 3, 0, 4, 7);
-        $mech->get_ok("/dashboard?category=Traffic+lights+%26+bells");
-        $mech->content_contains("<option value='Traffic lights &amp; bells' selected>");
+        $mech->get_ok("/dashboard?category=Traffic+lights+%26+bells-group-");
+        $mech->content_contains("<option value='Traffic lights &amp; bells-group-' selected>");
         test_table($mech->content, 0, 0, 0, 0, 0, 0, 0, 0, 7, 3, 0, 10, 0, 0, 0, 0, 0, 0, 0, 0, 7, 3, 0, 10);
         $mech->get_ok("/dashboard?category=group-Road+%26+more");
         $mech->content_contains('<option value="group-Road &amp; more" selected>');
         test_table($mech->content, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 4, 6, 1, 0, 0, 1, 3, 0, 4, 7);
-        $mech->get_ok("/dashboard?category=group-Road+%26+more&category=Potholes");
+        $mech->get_ok("/dashboard?category=group-Road+%26+more&category=Potholes-group-");
         test_table($mech->content, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 4, 6, 1, 0, 0, 1, 3, 0, 4, 7);
-        $mech->get_ok("/dashboard?category=group-Road+%26+more&category=Litter");
+        $mech->get_ok("/dashboard?category=group-Road+%26+more&category=Litter-group-");
         test_table($mech->content, 1, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 4, 6, 1, 0, 0, 1, 4, 0, 4, 8);
+
+        my ($sub_pothole_pavement) = $mech->create_problems_for_body(1, $body->id, 'Title', { areas => ",$area_id,2651,", category => 'Potholes', cobrand => 'no2fat' });
+        $sub_pothole_pavement->set_extra_metadata( group => 'Pavements');
+        $sub_pothole_pavement->update;
+        $mech->get_ok("/dashboard?category=group-Road+%26+more&category=Litter-group-");
+        test_table($mech->content, 1, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 4, 6, 1, 0, 0, 1, 4, 0, 4, 8);
+        my ($sub_pothole_road) = $mech->create_problems_for_body(1, $body->id, 'Title', { areas => ",$area_id,2651,", category => 'Potholes', cobrand => 'no2fat' });
+        $sub_pothole_road->set_extra_metadata( group => 'Road & more');
+        $sub_pothole_road->state('closed');
+        $sub_pothole_road->update;
+        $mech->get_ok("/dashboard?category=group-Road+%26+more&category=Litter-group-");
+        test_table($mech->content, 1, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 1, 2, 0, 4, 6, 1, 0, 0, 1, 4, 1, 4, 9);
+        $sub_pothole_pavement->delete; $sub_pothole_pavement->update;
+        $sub_pothole_road->delete; $sub_pothole_road->update;
     };
 
     subtest 'test grouping' => sub {
+        my $contacts = FixMyStreet::DB->resultset('Contact')->search( { body_id => $body->id } );
+        while (my $contact = $contacts->next) {
+            if ($contact->category eq 'Potholes' || $contact->category eq 'White lines') {
+                $contact->set_extra_metadata(group => ['Road & more']);
+                $contact->update;
+            }
+        }
         $mech->get_ok("/dashboard?group_by=category");
         my $top_level = test_table($mech->content, 1, 0, 10, 6, 1, 18);
         is_deeply $top_level, ['Road & more'], 'Road group created';
diff --git a/templates/web/base/dashboard/index.html b/templates/web/base/dashboard/index.html
index ca07f52d259..0f928115f32 100644
--- a/templates/web/base/dashboard/index.html
+++ b/templates/web/base/dashboard/index.html
@@ -54,7 +54,7 @@ <h1>[% loc('Summary statistics') %]</h1>
         <select class="form-control js-multiple" multiple name="category" id="category">
             [% BLOCK category_option %]
             [% SET category_safe = mark_safe(cat.category) %]
-                <option value='[% cat.category | html %]'[% ' selected' IF display_categories.$category_safe %]>[% cat.category_display | html %]</option>
+                <option value='[% cat.category | html %]-[% group.group_id | html %]'[% ' selected' IF display_categories.$category_safe %]>[% cat.category_display | html %]</option>
             [% END %]
             [%~ INCLUDE 'report/new/_category_select.html' include_group_option=1 ~%]
         </select>
@@ -156,19 +156,21 @@ <h1>[% loc('Summary statistics') %]</h1>
     </tr>
   [% SET current_category = '' %]
   [% FOR k IN rows %][% k_safe = mark_safe(k) %]
-    [% IF category_to_group.$k_safe AND category_to_group.$k_safe != current_category %]
-    [% SET current_category = category_to_group.$k_safe %]
+    [% IF category_to_group.$k_safe.group AND category_to_group.$k_safe.group != current_category %]
+    [% SET current_category = category_to_group.$k_safe.group %]
       <tr>
-        [% IF group_by == 'category+state' %]
-        <th colspan="5" scope="colgroup">[% category_to_group.$k_safe %]</th>
+        [% IF group_by == 'category+state' OR group_by == 'category' %]
+        <th colspan="5" scope="colgroup">[% category_to_group.$k_safe.group %]</th>
         [% ELSE %]
-        <th colspan="2" scope="colgroup">[% category_to_group.$k_safe %]</th>
+        <th colspan="2" scope="colgroup">[% $k_safe %]</th>
         [% END %]
       </tr>
     [% END %]
     <tr>
       [% IF group_by == 'state' %]
         <th scope="row">[% prettify_state(k) %]</th>
+      [% ELSIF group_by == 'category' OR group_by == 'category+state' %]
+        <th scope="row">[% category_to_group.$k_safe.display_name %]</th>
       [% ELSE %]
         <th scope="row">[% k %]</th>
       [% END %]