From 489edbd60e4298758b9cc4542e44ece206fa15aa Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Fri, 16 Aug 2024 11:18:03 +0100 Subject: [PATCH] Perform comment state check in join, not query. The old way meant that reports that had updates that were only unconfirmed or hidden were being ignored entirely (as the left join did match). --- CHANGELOG.md | 1 + perllib/FixMyStreet/Cobrand/HighwaysEngland.pm | 8 ++++---- perllib/FixMyStreet/DB/Result/Problem.pm | 12 ++++++++++++ perllib/FixMyStreet/Reporting.pm | 8 +++----- perllib/FixMyStreet/Script/CSVExport.pm | 4 +--- t/cobrand/northumberland.t | 2 ++ 6 files changed, 23 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d27298be9..cfb1200658 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ - Create reporter alert before creating first unconfirmed auto-update. - Fix display of user in assignment dropdown. #4855 - Fix setting of fixed timestamp in CSV export. #5119 + - Fix CSV export of reports with only hidden/unconfirmed updates. #5119 - Admin improvements: - Rename emergency message to site message. - Added a category control for overriding the text of the new report details field. diff --git a/perllib/FixMyStreet/Cobrand/HighwaysEngland.pm b/perllib/FixMyStreet/Cobrand/HighwaysEngland.pm index 95b855b7bd..48468fa77b 100644 --- a/perllib/FixMyStreet/Cobrand/HighwaysEngland.pm +++ b/perllib/FixMyStreet/Cobrand/HighwaysEngland.pm @@ -305,12 +305,12 @@ sub dashboard_export_problems_add_columns { $csv->objects_attrs({ '+columns' => [ - 'comments.text', 'comments.extra', - {'comments.user.name' => 'user.name'}, + 'confirmed_comments.text', 'confirmed_comments.extra', + {'confirmed_comments.user.name' => 'user.name'}, {'user.email' => 'user_2.email'}, {'user.phone' => 'user_2.phone'}, ], - join => ['user', { comments => 'user' }], + join => ['user', { confirmed_comments => 'user' }], }); $csv->add_csv_columns( @@ -371,7 +371,7 @@ sub dashboard_export_problems_add_columns { $fields->{user_phone} = $report->user ? $report->user->phone : ''; my $i = 1; - my @updates = $report->comments->all; + my @updates = $report->confirmed_comments->all; @updates = sort { $a->confirmed <=> $b->confirmed || $a->id <=> $b->id } @updates; for my $update (@updates) { last if $i > 5; diff --git a/perllib/FixMyStreet/DB/Result/Problem.pm b/perllib/FixMyStreet/DB/Result/Problem.pm index 5ea34f85a3..d1a1c0b602 100644 --- a/perllib/FixMyStreet/DB/Result/Problem.pm +++ b/perllib/FixMyStreet/DB/Result/Problem.pm @@ -241,6 +241,18 @@ __PACKAGE__->has_many( }, ); +__PACKAGE__->has_many( + confirmed_comments => "FixMyStreet::DB::Result::Comment", + sub { + my $args = shift; + return { + "$args->{foreign_alias}.problem_id" => { -ident => "$args->{self_alias}.id" }, + "$args->{foreign_alias}.state" => 'confirmed', + }; + }, + { cascade_copy => 0, cascade_delete => 0 }, +); + __PACKAGE__->might_have( contributed_by => "FixMyStreet::DB::Result::User", sub { diff --git a/perllib/FixMyStreet/Reporting.pm b/perllib/FixMyStreet/Reporting.pm index cfe5f4a874..54c45a0e0c 100644 --- a/perllib/FixMyStreet/Reporting.pm +++ b/perllib/FixMyStreet/Reporting.pm @@ -197,16 +197,14 @@ sub _csv_parameters_problems { my $self = shift; my $groups = $self->cobrand->enable_category_groups ? 1 : 0; - my $join = ['comments']; - my $columns = ['comments.id', 'comments.problem_state', 'comments.confirmed', 'comments.mark_fixed']; + my $join = ['confirmed_comments']; + my $columns = ['confirmed_comments.id', 'confirmed_comments.problem_state', 'confirmed_comments.confirmed', 'confirmed_comments.mark_fixed']; if ($groups) { push @$join, 'contact'; push @$columns, 'contact.id', 'contact.extra'; } - my $rs = $self->objects_rs->search({ - "comments.state" => ['confirmed', undef], - }, { + my $rs = $self->objects_rs->search(undef, { join => $join, collapse => 1, '+columns' => $columns, diff --git a/perllib/FixMyStreet/Script/CSVExport.pm b/perllib/FixMyStreet/Script/CSVExport.pm index c476c4e19e..42111ee1cf 100644 --- a/perllib/FixMyStreet/Script/CSVExport.pm +++ b/perllib/FixMyStreet/Script/CSVExport.pm @@ -196,7 +196,7 @@ staff_roles AS ( EOF my @sql_join = ( '"contacts" "contact" ON CAST( "contact"."body_id" AS text ) = (regexp_split_to_array( "me"."bodies_str", \',\'))[1] AND "contact"."category" = "me"."category"', - '"comment" "comments" ON "comments"."problem_id" = "me"."id"', + '"comment" "comments" ON "comments"."problem_id" = "me"."id" AND "comments"."state" = \'confirmed\'', '"users" "contributed_by_user" ON "contributed_by_user"."id" = ("me"."extra"->>\'contributed_by\')::integer', 'staff_roles ON contributed_by_user.id = staff_roles.user_id', ); @@ -239,8 +239,6 @@ EOF return <create_comment_for_problem( ); $update->update({ problem_state => '' }); # simulate a questionnaire response which has mark_fixed true and no problem_state +# Have it so problem1 only has hidden updates +$mech->create_comment_for_problem($problem1, $staffuser, 'Title', 'text', 0, 'hidden', ''); my $UPLOAD_DIR = tempdir( CLEANUP => 1 ); FixMyStreet::override_config {