From dcc552068e93f78309479dea53b70ae7c79d4a95 Mon Sep 17 00:00:00 2001 From: Victoria Mihell-Hale Date: Thu, 12 Sep 2024 17:44:10 +0100 Subject: [PATCH 1/6] Allow unsetting of extra details field on report --- perllib/FixMyStreet/App/Controller/Report.pm | 2 ++ 1 file changed, 2 insertions(+) diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm index 1971087777..618f9f6a20 100644 --- a/perllib/FixMyStreet/App/Controller/Report.pm +++ b/perllib/FixMyStreet/App/Controller/Report.pm @@ -472,6 +472,8 @@ sub inspect : Private { $c->cobrand->max_detailed_info_length ); } + } elsif ( $c->get_param('detailed_information') eq '' ) { + $problem->unset_extra_metadata('detailed_information'); } if ( $c->get_param('include_update') or $c->get_param('raise_defect') ) { From d7078a7cb9d0c02b554d9dbdb19113d6e288a91f Mon Sep 17 00:00:00 2001 From: Victoria Mihell-Hale Date: Thu, 29 Aug 2024 18:06:16 +0100 Subject: [PATCH 2/6] [Northumberland] Pull 'detailed_information' (extra details) from Alloy --- perllib/Open311/UpdatesBase.pm | 62 ++++++++++++++----------- t/app/model/problem.t | 82 +++++++++++++++++++++++++--------- 2 files changed, 96 insertions(+), 48 deletions(-) diff --git a/perllib/Open311/UpdatesBase.pm b/perllib/Open311/UpdatesBase.pm index 7bb0f92d67..a981431f7e 100644 --- a/perllib/Open311/UpdatesBase.pm +++ b/perllib/Open311/UpdatesBase.pm @@ -188,34 +188,42 @@ sub _process_update { $request->{comment_time} = $p->whensent + DateTime::Duration->new( seconds => 1 ); } - # Assign admin user to report if 'assigned_user_*' fields supplied - if ( $request->{extras} && $request->{extras}{assigned_user_email} ) { - my $assigned_user_email = $request->{extras}{assigned_user_email}; - my $assigned_user_name = $request->{extras}{assigned_user_name}; - - my $assigned_user - = FixMyStreet::DB->resultset('User') - ->find( { email => $assigned_user_email } ); - - unless ($assigned_user) { - $assigned_user = FixMyStreet::DB->resultset('User')->create( - { email => $assigned_user_email, - name => $assigned_user_name, - from_body => $body->id, - email_verified => 1, - }, - ); - - # Make them an inspector - # TODO Other permissions required? - $assigned_user->user_body_permissions->create( - { body_id => $body->id, - permission_type => 'report_inspect', - } - ); - } + if ( $request->{extras} ) { + # Assign admin user to report if 'assigned_user_*' fields supplied + if ( $request->{extras}{assigned_user_email} ) { + my $assigned_user_email = $request->{extras}{assigned_user_email}; + my $assigned_user_name = $request->{extras}{assigned_user_name}; + + my $assigned_user + = FixMyStreet::DB->resultset('User') + ->find( { email => $assigned_user_email } ); + + unless ($assigned_user) { + $assigned_user = FixMyStreet::DB->resultset('User')->create( + { email => $assigned_user_email, + name => $assigned_user_name, + from_body => $body->id, + email_verified => 1, + }, + ); + + # Make them an inspector + # TODO Other permissions required? + $assigned_user->user_body_permissions->create( + { body_id => $body->id, + permission_type => 'report_inspect', + } + ); + } - $assigned_user->add_to_planned_reports($p); + $assigned_user->add_to_planned_reports($p); + } + if ( exists $request->{extras}{detailed_information} ) { + $request->{extras}{detailed_information} + ? $p->set_extra_metadata( detailed_information => + $request->{extras}{detailed_information} ) + : $p->unset_extra_metadata('detailed_information'); + } } my $comment = $self->schema->resultset('Comment')->new( diff --git a/t/app/model/problem.t b/t/app/model/problem.t index 2704ca709b..ae7f7ecc5d 100644 --- a/t/app/model/problem.t +++ b/t/app/model/problem.t @@ -203,6 +203,31 @@ for my $test ( }, state => 'confirmed', }, + { + desc => 'adding detailed information', + request => { + comment_time => $comment_time, + status => 'open', + description => 'Adding detailed information', + extras => { + detailed_information => 'Hello there', + }, + }, + state => 'confirmed', + }, + { + desc => 'deleting detailed information', + request => { + comment_time => $comment_time, + status => 'open', + description => 'Deleting detailed information', + extras => { + detailed_information => '', + }, + }, + state => 'confirmed', + start_detailed_information => 'Goodbye here', + }, ) { subtest $test->{desc} => sub { # makes testing easier; @@ -210,6 +235,10 @@ for my $test ( $problem->created( DateTime->now()->subtract( days => 1 ) ); $problem->lastupdate( DateTime->now()->subtract( days => 1 ) ); $problem->state( $test->{start_state} || 'confirmed' ); + $problem->unset_extra_metadata('detailed_information'); + $problem->set_extra_metadata( 'detailed_information', + $test->{start_detailed_information} ) + if $test->{start_detailed_information}; $problem->update; my $w3c = DateTime::Format::W3CDTF->new(); @@ -228,29 +257,40 @@ for my $test ( is $problem->state, $test->{state}, 'problem state'; is $update->text, $test->{request}->{description}, 'update text'; - if ( my $assigned = $test->{request}{extras} ) { - my $assigned_user = FixMyStreet::DB->resultset('User') - ->search( { email => $assigned->{assigned_user_email} } ) - ->first; - - # Check certain fields set for new user - if ( $assigned->{assigned_user_email} ne $existing_user->email ) { - is $assigned_user->from_body->id, $body->id, - 'assigned user body'; - is $assigned_user->email_verified, 1, - 'assigned user email verified'; - is_deeply $assigned_user->body_permissions, - [ { body_id => $body->id, permission => 'report_inspect' } - ], - 'assigned user permissions'; + if ( my $extras = $test->{request}{extras} ) { + my $assigned_user_name = $extras->{assigned_user_name}; + my $assigned_user_email = $extras->{assigned_user_email}; + + if ( $assigned_user_email ) { + my $assigned_user = FixMyStreet::DB->resultset('User') + ->search( { email => $assigned_user_email } ) + ->first; + + # Check certain fields set for new user + if ( $assigned_user_email ne $existing_user->email ) { + is $assigned_user->from_body->id, $body->id, + 'assigned user body'; + is $assigned_user->email_verified, 1, + 'assigned user email verified'; + is_deeply $assigned_user->body_permissions, + [ { body_id => $body->id, permission => 'report_inspect' } + ], + 'assigned user permissions'; + } + + is $assigned_user->name, $assigned_user_name, + 'assigned user name'; + + is $problem->shortlisted_user->email, + $assigned_user_email, + 'assigned user actually assigned to problem'; } - is $assigned_user->name, $assigned->{assigned_user_name}, - 'assigned user name'; - - is $problem->shortlisted_user->email, - $assigned->{assigned_user_email}, - 'assigned user actually assigned to problem'; + my $detailed_information = $extras->{detailed_information}; + if ( defined $detailed_information ) { + is $problem->get_extra_metadata('detailed_information'), + $detailed_information || undef; + } } }; } From cd016b0abd9f7ea5018d1b3a15be7e4b4a914193 Mon Sep 17 00:00:00 2001 From: Victoria Mihell-Hale Date: Tue, 3 Sep 2024 20:34:45 +0100 Subject: [PATCH 3/6] [Northumberland] Send 'detailed_information' (extra details) in updates to Alloy --- perllib/FixMyStreet/Cobrand/Northumberland.pm | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/perllib/FixMyStreet/Cobrand/Northumberland.pm b/perllib/FixMyStreet/Cobrand/Northumberland.pm index df0362d733..08acfc710e 100644 --- a/perllib/FixMyStreet/Cobrand/Northumberland.pm +++ b/perllib/FixMyStreet/Cobrand/Northumberland.pm @@ -227,4 +227,21 @@ sub should_skip_sending_update { =cut +=head2 open311_munge_update_params + +We pass a report's 'detailed_information' (from its +extra_metadata) to Alloy, as an 'extra_details' attribute. + +=cut + +sub open311_munge_update_params { + my ( $self, $params, $comment, undef ) = @_; + + my $p = $comment->problem; + my $detailed_information + = $p->get_extra_metadata('detailed_information') || ''; + + $params->{extra_details} = $detailed_information; +} + 1; From 3a44bbdbca51ad83c11cb0f70e7e45069ee06042 Mon Sep 17 00:00:00 2001 From: Victoria Mihell-Hale Date: Thu, 5 Sep 2024 00:23:50 +0100 Subject: [PATCH 4/6] [Northumberland] Pass assigned user to Alloy --- perllib/FixMyStreet/Cobrand/Northumberland.pm | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/perllib/FixMyStreet/Cobrand/Northumberland.pm b/perllib/FixMyStreet/Cobrand/Northumberland.pm index 08acfc710e..0a60e027d9 100644 --- a/perllib/FixMyStreet/Cobrand/Northumberland.pm +++ b/perllib/FixMyStreet/Cobrand/Northumberland.pm @@ -232,16 +232,25 @@ sub should_skip_sending_update { We pass a report's 'detailed_information' (from its extra_metadata) to Alloy, as an 'extra_details' attribute. +We pass the name and email address of the user assigned to the report (the +user who has shortlisted the report). + =cut sub open311_munge_update_params { my ( $self, $params, $comment, undef ) = @_; my $p = $comment->problem; - my $detailed_information - = $p->get_extra_metadata('detailed_information') || ''; - $params->{extra_details} = $detailed_information; + my $detailed_information + = $p->get_extra_metadata('detailed_information') // ''; + $params->{'attribute[extra_details]'} = $detailed_information; + + my $assigned_to = $p->shortlisted_user; + $params->{'attribute[assigned_to_user_email]'} + = $assigned_to + ? $assigned_to->email + : ''; } 1; From 11cc647a69dd35e1efe64f135eacbc47dbfc8339 Mon Sep 17 00:00:00 2001 From: Victoria Mihell-Hale Date: Thu, 12 Sep 2024 17:45:16 +0100 Subject: [PATCH 5/6] [Northumberland] Add comment when assigned user or extra details updated --- perllib/FixMyStreet/App/Controller/Report.pm | 18 ++- perllib/FixMyStreet/Cobrand/Northumberland.pm | 13 ++ perllib/FixMyStreet/DB/Result/User.pm | 38 ++++++ perllib/Open311/UpdatesBase.pm | 4 +- t/app/model/problem.t | 10 ++ t/cobrand/northumberland.t | 119 ++++++++++++++++++ 6 files changed, 200 insertions(+), 2 deletions(-) diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm index 618f9f6a20..4ab1f97cba 100644 --- a/perllib/FixMyStreet/App/Controller/Report.pm +++ b/perllib/FixMyStreet/App/Controller/Report.pm @@ -460,6 +460,8 @@ sub inspect : Private { if ($permissions->{report_inspect}) { $problem->set_extra_metadata( traffic_information => $c->get_param('traffic_information') ); + my $old_detailed_information + = $problem->get_extra_metadata('detailed_information') // ''; if ( my $info = $c->get_param('detailed_information') ) { $problem->set_extra_metadata( detailed_information => $info ); if ($c->cobrand->max_detailed_info_length && @@ -476,6 +478,20 @@ sub inspect : Private { $problem->unset_extra_metadata('detailed_information'); } + my $handler + = $c->cobrand->call_hook( + get_body_handler_for_problem => $problem + ) || $c->cobrand; + my $record_extra + = $handler->call_hook('record_update_extra_fields'); + if ( + $record_extra->{detailed_information} + && ( $problem->get_extra_metadata('detailed_information') // '' ) + ne $old_detailed_information + ) { + $update_params{extra}{detailed_information} = 1; + } + if ( $c->get_param('include_update') or $c->get_param('raise_defect') ) { $update_text = Utils::cleanup_text( $c->get_param('public_update'), { allow_multiline => 1 } ); if (!$update_text) { @@ -523,7 +539,7 @@ sub inspect : Private { # If the state has been changed to action scheduled and they've said # they want to raise a defect, consider the report to be inspected. if ($problem->state eq 'action scheduled' && $c->get_param('raise_defect') && !$problem->get_extra_metadata('inspected')) { - $update_params{extra} = { 'defect_raised' => 1 }; + $update_params{extra}{defect_raised} = 1; $problem->set_extra_metadata( inspected => 1 ); $c->forward( '/admin/log_edit', [ $problem->id, 'problem', 'inspected' ] ); } diff --git a/perllib/FixMyStreet/Cobrand/Northumberland.pm b/perllib/FixMyStreet/Cobrand/Northumberland.pm index 0a60e027d9..5d621cbacd 100644 --- a/perllib/FixMyStreet/Cobrand/Northumberland.pm +++ b/perllib/FixMyStreet/Cobrand/Northumberland.pm @@ -227,6 +227,19 @@ sub should_skip_sending_update { =cut +=head2 record_update_extra_fields + +We want to create comments when assigned (= shortlisted) user or +extra details (= detail_information) are updated for a report. + +=cut + +sub record_update_extra_fields { + { shortlisted_user => 1, + detailed_information => 1, + }; +} + =head2 open311_munge_update_params We pass a report's 'detailed_information' (from its diff --git a/perllib/FixMyStreet/DB/Result/User.pm b/perllib/FixMyStreet/DB/Result/User.pm index 782fd54b02..ad2d93214b 100644 --- a/perllib/FixMyStreet/DB/Result/User.pm +++ b/perllib/FixMyStreet/DB/Result/User.pm @@ -648,6 +648,25 @@ around add_to_planned_reports => sub { return $self->$orig(@_); }; +after add_to_planned_reports => sub { + my ( $self, $report, $no_comment ) = @_; + + unless ($no_comment) { + my $cobrand = $report->get_cobrand_logged; + $cobrand + = $cobrand->call_hook( get_body_handler_for_problem => $report ) + || $cobrand; + + my $report_extra = $cobrand->call_hook('record_update_extra_fields'); + $report->add_to_comments( + { text => '', + user => $self, + extra => { shortlisted_user => $self->email }, + } + ) if $report_extra->{shortlisted_user}; + } +}; + # Override the default auto-created function as we don't want to ever delete anything around remove_from_planned_reports => sub { my ($orig, $self, $report) = @_; @@ -656,6 +675,25 @@ around remove_from_planned_reports => sub { $report->update; }; +after remove_from_planned_reports => sub { + my ( $self, $report, $no_comment ) = @_; + + unless ($no_comment) { + my $cobrand = $report->get_cobrand_logged; + $cobrand + = $cobrand->call_hook( get_body_handler_for_problem => $report ) + || $cobrand; + + my $report_extra = $cobrand->call_hook('record_update_extra_fields'); + $report->add_to_comments( + { text => '', + user => $self, + extra => { shortlisted_user => undef }, + } + ) if $report_extra->{shortlisted_user}; + } +}; + sub active_planned_reports { my $self = shift; $self->planned_reports->search({ removed => undef }); diff --git a/perllib/Open311/UpdatesBase.pm b/perllib/Open311/UpdatesBase.pm index a981431f7e..24dc99cf11 100644 --- a/perllib/Open311/UpdatesBase.pm +++ b/perllib/Open311/UpdatesBase.pm @@ -216,7 +216,9 @@ sub _process_update { ); } - $assigned_user->add_to_planned_reports($p); + $assigned_user->add_to_planned_reports($p, 'no_comment'); + + # TODO Unassign? } if ( exists $request->{extras}{detailed_information} ) { $request->{extras}{detailed_information} diff --git a/t/app/model/problem.t b/t/app/model/problem.t index ae7f7ecc5d..50b5699982 100644 --- a/t/app/model/problem.t +++ b/t/app/model/problem.t @@ -5,6 +5,12 @@ use FixMyStreet::DB; use FixMyStreet::Script::Reports; use Open311::GetUpdates; use Sub::Override; +use Test::MockModule; + +my $cobrand = Test::MockModule->new('FixMyStreet::Cobrand::Default'); +$cobrand->mock( + record_update_extra_fields => sub { { shortlisted_user => 1 } } +); my $problem_rs = FixMyStreet::DB->resultset('Problem'); @@ -284,6 +290,10 @@ for my $test ( is $problem->shortlisted_user->email, $assigned_user_email, 'assigned user actually assigned to problem'; + is $problem->comments->count, 1, 'initial comment only'; + for ( $problem->comments ) { + is $_->extra, undef, 'No extra data'; + } } my $detailed_information = $extras->{detailed_information}; diff --git a/t/cobrand/northumberland.t b/t/cobrand/northumberland.t index 0e5d51c38f..73dec22287 100644 --- a/t/cobrand/northumberland.t +++ b/t/cobrand/northumberland.t @@ -1,3 +1,4 @@ +use CGI::Simple; use FixMyStreet::TestMech; use FixMyStreet::Script::CSVExport; use Test::MockModule; @@ -111,4 +112,122 @@ FixMyStreet::override_config { }; }; +FixMyStreet::override_config { + ALLOWED_COBRANDS => [ 'northumberland', 'fixmystreet' ], + MAPIT_URL => 'http://mapit.uk/', +}, sub { + my $o = Open311->new( fixmystreet_body => $body ); + + my $superuser = $mech->create_user_ok( + 'superuser@example.com', + name => 'Super User', + is_superuser => 1, + ); + $mech->log_in_ok( $superuser->email ); + + for my $host ( 'northumberland', 'fixmystreet' ) { + ok $mech->host($host), "change host to $host"; + + my ($problem_to_update) = $mech->create_problems_for_body( + 1, + $body->id, + 'Test', + { cobrand => $host }, + ); + + subtest "User assignment on $host site" => sub { + $mech->get_ok( '/report/' . $problem_to_update->id ); + $mech->click_ok('.btn--shortlist'); + my $comment + = $problem_to_update->comments->order_by('-id')->first; + is_deeply $comment->get_extra_metadata, { + shortlisted_user => $superuser->email, + is_superuser => 1, + }, 'Comment created for user assignment'; + + my $id = $o->post_service_request_update($comment); + my $cgi = CGI::Simple->new($o->test_req_used->content); + is $cgi->param('attribute[assigned_to_user_email]'), + $superuser->email, + 'correct assigned_to_user_email attribute'; + is $cgi->param('attribute[extra_details]'), + '', + 'correct extra_details attribute'; + + $mech->get_ok( '/report/' . $problem_to_update->id ); + $mech->click_ok('.btn--shortlisted'); + $comment + = $problem_to_update->comments->order_by('-id')->first; + is_deeply $comment->get_extra_metadata, { + shortlisted_user => undef, + is_superuser => 1, + }, 'Comment created for user un-assignment'; + + $id = $o->post_service_request_update($comment); + $cgi = CGI::Simple->new($o->test_req_used->content); + is $cgi->param('attribute[assigned_to_user_email]'), + '', + 'correct assigned_to_user_email attribute'; + is $cgi->param('attribute[extra_details]'), + '', + 'correct extra_details attribute'; + }; + + subtest "Extra details on $host site" => sub { + $mech->get_ok( '/report/' . $problem_to_update->id ); + $mech->submit_form( + button => 'save', + form_id => 'report_inspect_form', + fields => { + detailed_information => 'ABC', + include_update => 0, + }, + ); + my $comment + = $problem_to_update->comments->order_by('-id')->first; + is_deeply $comment->get_extra_metadata, { + detailed_information => 1, + is_superuser => 1, + }, 'Comment created for extra details'; + + my $id = $o->post_service_request_update($comment); + my $cgi = CGI::Simple->new($o->test_req_used->content); + is $cgi->param('attribute[assigned_to_user_email]'), + '', + 'correct assigned_to_user_email attribute'; + is $cgi->param('attribute[extra_details]'), + 'ABC', + 'correct extra_details attribute'; + + $mech->get_ok( '/report/' . $problem_to_update->id ); + $mech->submit_form( + button => 'save', + form_id => 'report_inspect_form', + fields => { + detailed_information => '', + include_update => 0, + }, + ); + $comment + = $problem_to_update->comments->order_by('-id')->first; + is_deeply $comment->get_extra_metadata, { + detailed_information => 1, + is_superuser => 1, + }, 'Comment created for extra details unsetting'; + is_deeply $problem_to_update->get_extra_metadata, + {}, + 'Extra details unset on problem'; + + $id = $o->post_service_request_update($comment); + $cgi = CGI::Simple->new($o->test_req_used->content); + is $cgi->param('attribute[assigned_to_user_email]'), + '', + 'correct assigned_to_user_email attribute'; + is $cgi->param('attribute[extra_details]'), + '', + 'correct extra_details attribute'; + }; + } +}; + done_testing(); From 702d1078e4ac633e785d3bb605950d2004436143 Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Wed, 18 Sep 2024 15:26:55 +0100 Subject: [PATCH 6/6] =?UTF-8?q?Don=E2=80=99t=20show=20empty=20updates=20in?= =?UTF-8?q?=20front=20end.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- templates/web/base/report/_update_state.html | 9 +-------- templates/web/base/report/updates.html | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/templates/web/base/report/_update_state.html b/templates/web/base/report/_update_state.html index d484dcb051..7f78dc5503 100644 --- a/templates/web/base/report/_update_state.html +++ b/templates/web/base/report/_update_state.html @@ -1,13 +1,6 @@ [% # Small chance of duplicates in the case of fixed - user followed by fixed - council %] -[% SET update_state = update.problem_state_processed %] -[% IF ( update_state AND update_state != global.last_state AND NOT (global.last_state == "" AND update.problem_state_processed == 'confirmed') ) - OR update.mark_fixed - OR update.mark_open -%] +[% IF state_change %]

[% loc('State changed to:') %] [% update.problem_state_display %]

- [%- global.last_state = update_state %] - [%- IF update_state == "" AND update.mark_fixed %][% global.last_state = 'fixed - user' %][% END %] - [%- IF update_state == "" AND update.mark_open %][% global.last_state = 'confirmed' %][% END %] [% END %]

diff --git a/templates/web/base/report/updates.html b/templates/web/base/report/updates.html index d65f1a2872..03a398860a 100644 --- a/templates/web/base/report/updates.html +++ b/templates/web/base/report/updates.html @@ -9,6 +9,20 @@ [%- IF update.get_extra_metadata('triage_report') == 1 AND ( NOT c.user OR ( NOT c.user.from_body AND NOT c.user.is_superuser ) ) %] [%- NEXT %] [%- END %] + +[%- IF NOT update.whenanswered AND update.type != 'moderation' %] + [%- SET update_state = update.problem_state_processed %] + [%- SET state_change = ( update_state AND update_state != global.last_state AND NOT (global.last_state == "" AND update.problem_state_processed == 'confirmed') ) OR update.mark_fixed OR update.mark_open %] + [%- IF NOT update.photo AND NOT update.text AND NOT state_change %] + [%- NEXT %] + [%- END %] + [%- IF state_change %] + [%- global.last_state = update_state %] + [%- IF update_state == "" AND update.mark_fixed %][% global.last_state = 'fixed - user' %][% END %] + [%- IF update_state == "" AND update.mark_open %][% global.last_state = 'confirmed' %][% END %] + [%- END %] +[%- END %] + [% INCLUDE 'report/update.html' %] [% END %]