From 0f7ddeb0e484239596b8882bbdb49c27a404028d Mon Sep 17 00:00:00 2001 From: Moray Jones Date: Wed, 1 Nov 2023 15:16:32 +0000 Subject: [PATCH 1/2] [FMS] Make dashboard categories multi-select Switch categories list on dashborad to multi checkbox rather than single option. First step towards: https://github.com/mysociety/societyworks/issues/1458 --- bin/csv-export | 4 +++- perllib/FixMyStreet/App/Controller/Dashboard.pm | 5 ++++- perllib/FixMyStreet/Cobrand/TfL.pm | 5 +++-- perllib/FixMyStreet/Reporting.pm | 13 ++++++++----- t/app/controller/dashboard.t | 14 +++++++++++++- templates/web/base/dashboard/index.html | 5 +++-- 6 files changed, 34 insertions(+), 12 deletions(-) diff --git a/bin/csv-export b/bin/csv-export index a0930ae6185..ef06ea80283 100755 --- a/bin/csv-export +++ b/bin/csv-export @@ -60,10 +60,12 @@ my $user = FixMyStreet::DB->resultset("User")->find($opts->user) if $opts->user; my $body = FixMyStreet::DB->resultset("Body")->find($opts->body) if $opts->body; my $wards = $opts->wards ? [split',', $opts->wards] : []; +my $category = $opts->category ? [split'::', $opts->category] : []; + my $reporting = FixMyStreet::Reporting->new( type => $opts->type, user => $user, - category => $opts->category, + category => $category, state => $opts->state, role_id => $opts->role_id, wards => $wards, diff --git a/perllib/FixMyStreet/App/Controller/Dashboard.pm b/perllib/FixMyStreet/App/Controller/Dashboard.pm index 88ebe2b6bfb..8202a471bbd 100644 --- a/perllib/FixMyStreet/App/Controller/Dashboard.pm +++ b/perllib/FixMyStreet/App/Controller/Dashboard.pm @@ -127,7 +127,10 @@ sub index : Path : Args(0) { $c->forward('/report/stash_category_groups', [ $c->stash->{contacts} ]); # See if we've had anything from the body dropdowns - $c->stash->{category} = $c->get_param('category'); + $c->stash->{category} = [ $c->get_param_list('category') ]; + my %display_categories = map { $_ => 1 } @{$c->stash->{category}}; + $c->stash->{display_categories} = \%display_categories; + $c->stash->{ward} = [ $c->get_param_list('ward') ]; if ($c->user_exists) { diff --git a/perllib/FixMyStreet/Cobrand/TfL.pm b/perllib/FixMyStreet/Cobrand/TfL.pm index 06ae2d0cc00..6597159ad94 100644 --- a/perllib/FixMyStreet/Cobrand/TfL.pm +++ b/perllib/FixMyStreet/Cobrand/TfL.pm @@ -237,8 +237,9 @@ sub dashboard_export_problems_add_columns { my @contacts = $csv->body->contacts->search(undef, { order_by => [ 'category' ] } )->all; my %extra_columns; - if ($csv->category) { - @contacts = grep { $_->category eq $csv->category } @contacts; + if (@{$csv->category}) { + my %picked_cats = map { $_ => 1} @{$csv->category}; + @contacts = grep { $picked_cats{$_->category} } @contacts; } foreach my $contact (@contacts) { foreach (@{$contact->get_metadata_for_storage}) { diff --git a/perllib/FixMyStreet/Reporting.pm b/perllib/FixMyStreet/Reporting.pm index 0c942e8f6a3..215526f737b 100644 --- a/perllib/FixMyStreet/Reporting.pm +++ b/perllib/FixMyStreet/Reporting.pm @@ -18,7 +18,7 @@ has on_updates => ( is => 'lazy', default => sub { $_[0]->type eq 'updates' } ); has body => ( is => 'ro', isa => Maybe[InstanceOf['FixMyStreet::DB::Result::Body']] ); has wards => ( is => 'ro', isa => ArrayRef[Int], default => sub { [] } ); -has category => ( is => 'ro', isa => Maybe[Str] ); +has category => ( is => 'ro', isa => Maybe[ArrayRef[Maybe[Str]]], default => sub { [] } ); has state => ( is => 'ro', isa => Maybe[Str] ); has start_date => ( is => 'ro', isa => Str, @@ -90,12 +90,14 @@ has csv_extra_data => ( is => 'rw', isa => CodeRef ); has filename => ( is => 'rw', isa => Str, lazy => 1, default => sub { my $self = shift; my %where = ( - category => $self->category, state => $self->state, ward => join(',', @{$self->wards}), start_date => $self->start_date, end_date => $self->end_date, ); + if ($self->category) { + $where{category} = @{$self->category} < 3 ? join(',', @{$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; @@ -121,7 +123,7 @@ sub construct_rs_filter { $where{areas} = [ map { { 'like', "%,$_,%" } } @{$self->wards} ] if @{$self->wards}; $where{"$table_name.category"} = $self->category - if $self->category; + if $self->category && @{$self->category}; my $all_states = $self->cobrand->call_hook('dashboard_export_include_all_states'); if ( $self->state && FixMyStreet::DB::Result::Problem->fixed_states->{$self->state} ) { # Probably fixed - council @@ -375,9 +377,10 @@ sub kick_off_process { my $cmd = FixMyStreet->path_to('bin/csv-export'); $cmd .= ' --cobrand ' . $self->cobrand->moniker; $cmd .= " --out \Q$out\E"; - foreach (qw(type category state start_date end_date)) { + foreach (qw(type state start_date end_date)) { $cmd .= " --$_ " . quotemeta($self->$_) if $self->$_; } + $cmd .= " --category " . join('::', map { quotemeta } @{$self->category}) if ($self->category && @{$self->category}); foreach (qw(body user)) { $cmd .= " --$_ " . $self->$_->id if $self->$_; } @@ -490,7 +493,7 @@ sub filter_premade_csv { } my $category = $row->{Subcategory} || $row->{Category}; - next if $self->category && $category ne $self->category; + next if ($self->category && @{$self->category}) && !grep { /$category/ } @{$self->category}; if ( $self->state && $fixed_states->{$self->state} ) { # Probably fixed - council next unless $fixed_states->{$row->{$state_column}}; diff --git a/t/app/controller/dashboard.t b/t/app/controller/dashboard.t index 1487a9f3a3a..5ee0c109e47 100644 --- a/t/app/controller/dashboard.t +++ b/t/app/controller/dashboard.t @@ -159,7 +159,7 @@ FixMyStreet::override_config { subtest 'The correct categories and totals shown by default' => sub { $mech->get_ok("/dashboard"); - my $expected_cats = [ 'All', 'Litter', 'Other', 'Traffic lights & bells', 'Potholes' ]; + my $expected_cats = [ 'Litter', 'Other', 'Traffic lights & bells', 'Potholes' ]; my $res = $categories->scrape( $mech->content ); $mech->content_contains(''); is_deeply( $res->{cats}, $expected_cats, 'correct list of categories' ); @@ -179,6 +179,8 @@ 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, 0, 0, 0, 0, 3, 0, 0, 3, 3, 0, 0, 3); + $mech->get_ok("/dashboard?category=Litter&category=Potholes"); + test_table($mech->content, 1, 0, 0, 1, 0, 0, 0, 0, 2, 0, 4, 6, 0, 0, 0, 0, 3, 0, 4, 7); }; subtest 'test grouping' => sub { @@ -198,6 +200,16 @@ FixMyStreet::override_config { $mech->content_contains('role-' . $role->id, "File link created with role"); }; + subtest 'csv for multiple categories' => sub { + $mech->get_ok("/dashboard?category=Litter&category=Potholes&export=2"); + $mech->content_contains('www.example.org-body-' . $body->id . '-category-Litter,Potholes-start_date-2014-01-02.csv'); + $mech->get_ok("/dashboard?category=Litter&category=Potholes&category=Traffic+lights+&+bells&export=2"); + $mech->content_contains('www.example.org-body-' . $body->id . '-category-multiple-categories-start_date-2014-01-02.csv'); + $mech->get_ok("/dashboard?category=Litter&category=Potholes&export=1"); + my @rows = $mech->content_as_csv; + is scalar @rows, 8, '1 (header) + 7 (reports) found = 8 lines'; + }; + subtest 'export as csv' => sub { $mech->create_problems_for_body(1, $body->id, 'Title', { detail => "this report\nis split across\nseveral lines", diff --git a/templates/web/base/dashboard/index.html b/templates/web/base/dashboard/index.html index 6e5d201ebdb..24a99c7c8dd 100644 --- a/templates/web/base/dashboard/index.html +++ b/templates/web/base/dashboard/index.html @@ -51,9 +51,10 @@

[% loc('Summary statistics') %]

- [% BLOCK category_option %] - + [% SET category = cat.category | html %] + [% END %] [%~ INCLUDE 'report/new/_category_select.html' ~%] From 5b34f3c762e0b66e477d98a1488b972cfb24844a Mon Sep 17 00:00:00 2001 From: Moray Jones Date: Wed, 1 Nov 2023 15:16:32 +0000 Subject: [PATCH 2/2] [FMS] Add parent categories to dashboard categories view Add an 'All' subcategories checkbox to select all the categories in one group Arbitrary decision to choose the first parent category from the groups method of a contact. https://github.com/mysociety/societyworks/issues/1458 --- .../FixMyStreet/App/Controller/Dashboard.pm | 16 ++++++- perllib/FixMyStreet/Reporting.pm | 12 ++--- t/app/controller/admin/report_edit.t | 9 ++-- t/app/controller/dashboard.t | 47 +++++++++++++------ templates/web/base/dashboard/index.html | 2 +- .../web/base/report/new/_category_select.html | 7 ++- 6 files changed, 65 insertions(+), 28 deletions(-) diff --git a/perllib/FixMyStreet/App/Controller/Dashboard.pm b/perllib/FixMyStreet/App/Controller/Dashboard.pm index 8202a471bbd..c3cb07f6074 100644 --- a/perllib/FixMyStreet/App/Controller/Dashboard.pm +++ b/perllib/FixMyStreet/App/Controller/Dashboard.pm @@ -126,11 +126,25 @@ sub index : Path : Args(0) { $c->stash->{contacts} = [ $c->stash->{contacts}->all ]; $c->forward('/report/stash_category_groups', [ $c->stash->{contacts} ]); + 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; + } + } + 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}}; + $c->stash->{ward} = [ $c->get_param_list('ward') ]; if ($c->user_exists) { @@ -190,7 +204,7 @@ sub construct_rs_filter : Private { my $reporting = FixMyStreet::Reporting->new( type => $updates ? 'updates' : 'problems', - category => $c->stash->{category}, + category => $c->stash->{category} || [], state => $c->stash->{q_state}, wards => $c->stash->{ward}, body => $c->stash->{body} || undef, diff --git a/perllib/FixMyStreet/Reporting.pm b/perllib/FixMyStreet/Reporting.pm index 215526f737b..5807286164e 100644 --- a/perllib/FixMyStreet/Reporting.pm +++ b/perllib/FixMyStreet/Reporting.pm @@ -18,7 +18,7 @@ has on_updates => ( is => 'lazy', default => sub { $_[0]->type eq 'updates' } ); has body => ( is => 'ro', isa => Maybe[InstanceOf['FixMyStreet::DB::Result::Body']] ); has wards => ( is => 'ro', isa => ArrayRef[Int], default => sub { [] } ); -has category => ( is => 'ro', isa => Maybe[ArrayRef[Maybe[Str]]], default => sub { [] } ); +has category => ( is => 'ro', isa => ArrayRef[Str], default => sub { [] } ); has state => ( is => 'ro', isa => Maybe[Str] ); has start_date => ( is => 'ro', isa => Str, @@ -95,9 +95,7 @@ has filename => ( is => 'rw', isa => Str, lazy => 1, default => sub { start_date => $self->start_date, end_date => $self->end_date, ); - if ($self->category) { - $where{category} = @{$self->category} < 3 ? join(',', @{$self->category}) : 'multiple-categories'; - } + $where{category} = @{$self->category} < 3 ? join(',', @{$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; @@ -123,7 +121,7 @@ sub construct_rs_filter { $where{areas} = [ map { { 'like', "%,$_,%" } } @{$self->wards} ] if @{$self->wards}; $where{"$table_name.category"} = $self->category - if $self->category && @{$self->category}; + if @{$self->category}; my $all_states = $self->cobrand->call_hook('dashboard_export_include_all_states'); if ( $self->state && FixMyStreet::DB::Result::Problem->fixed_states->{$self->state} ) { # Probably fixed - council @@ -380,7 +378,7 @@ sub kick_off_process { foreach (qw(type state start_date end_date)) { $cmd .= " --$_ " . quotemeta($self->$_) if $self->$_; } - $cmd .= " --category " . join('::', map { quotemeta } @{$self->category}) if ($self->category && @{$self->category}); + $cmd .= " --category " . join('::', map { quotemeta } @{$self->category}) if @{$self->category}; foreach (qw(body user)) { $cmd .= " --$_ " . $self->$_->id if $self->$_; } @@ -493,7 +491,7 @@ sub filter_premade_csv { } my $category = $row->{Subcategory} || $row->{Category}; - next if ($self->category && @{$self->category}) && !grep { /$category/ } @{$self->category}; + next if @{$self->category} && !grep { /$category/ } @{$self->category}; if ( $self->state && $fixed_states->{$self->state} ) { # Probably fixed - council next unless $fixed_states->{$row->{$state_column}}; diff --git a/t/app/controller/admin/report_edit.t b/t/app/controller/admin/report_edit.t index bb7cc26372b..a2c820225b5 100644 --- a/t/app/controller/admin/report_edit.t +++ b/t/app/controller/admin/report_edit.t @@ -10,8 +10,10 @@ my $superuser = $mech->create_user_ok('superuser@example.com', name => 'Super Us my $oxfordshire = $mech->create_body_ok(2237, 'Oxfordshire County Council'); my $user3 = $mech->create_user_ok('body_user@example.com', name => 'Body User', from_body => $oxfordshire); -my $oxfordshirecontact = $mech->create_contact_ok( body_id => $oxfordshire->id, category => 'Potholes', email => 'potholes@example.com' ); +my $oxfordshirecontact = $mech->create_contact_ok( body_id => $oxfordshire->id, category => 'Potholes', email => 'potholes@example.com', extra => { group => 'Road' } ); $mech->create_contact_ok( body_id => $oxfordshire->id, category => 'Traffic lights', email => 'lights@example.com' ); +$mech->create_contact_ok( body_id => $oxfordshire->id, category => 'Yellow lines', email => 'yellow@example.com', extra => { group => 'Road' } ); + my $oxford = $mech->create_body_ok(2421, 'Oxford City Council'); $mech->create_contact_ok( body_id => $oxford->id, category => 'Graffiti', email => 'graffiti@example.net' ); @@ -58,7 +60,7 @@ my $log_entries = FixMyStreet::DB->resultset('AdminLog')->search( object_type => 'problem', object_id => $report->id }, - { + { order_by => { -desc => 'id' }, } ); @@ -455,7 +457,8 @@ subtest 'change report category' => sub { whensent => \'current_timestamp', }); $mech->get_ok("/admin/report_edit/" . $ox_report->id); - + $mech->content_contains(''); + $mech->content_lacks(''); + $mech->content_contains('

[% loc('Summary statistics') %]

[% SET category = cat.category | html %] [% END %] - [%~ INCLUDE 'report/new/_category_select.html' ~%] + [%~ INCLUDE 'report/new/_category_select.html' include_group_option=1 ~%]

diff --git a/templates/web/base/report/new/_category_select.html b/templates/web/base/report/new/_category_select.html index 263b2432797..6f2c9c18d58 100644 --- a/templates/web/base/report/new/_category_select.html +++ b/templates/web/base/report/new/_category_select.html @@ -1,5 +1,10 @@ [%~ FOREACH group IN category_groups ~%] - [% IF group.name %][% END %] + [% IF group.name AND include_group_option %] + [% SET pseudo_category = "group-" _ group.name %] + + [% ELSIF group.name %] + + [% END %] [%~ FOREACH cat IN group.categories ~%] [% INCLUDE category_option %] [%~ END ~%]