Merge branch 'issues/forcouncils/201-show-all-report-updates'

5 files changed, 94 insertions, 22 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 789a1e456..291799e46 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,7 @@
         - Body users can filter reports by all states. #1790
     - Front end improvements:
         - Always show pagination figures even if only one page.
+        - Report pages list every update to a report. #1806
     - Admin improvements:
         - Highlight current shortlisted user in list tooltip.
         - Extra fields on contacts can be edited. #1743
diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm
index acc6f9a09..7bde226ef 100644
--- a/perllib/FixMyStreet/App/Controller/Report.pm
+++ b/perllib/FixMyStreet/App/Controller/Report.pm
@@ -320,7 +320,7 @@ sub inspect : Private {
         $c->forward('/auth/check_csrf_token');
 
         my $valid = 1;
-        my $update_text;
+        my $update_text = '';
         my $reputation_change = 0;
         my %update_params = ();
 
@@ -367,6 +367,8 @@ sub inspect : Private {
             if ( $problem->state ne $old_state ) {
                 $c->forward( '/admin/log_edit', [ $problem->id, 'problem', 'state_change' ] );
 
+                $update_params{problem_state} = $problem->state;
+
                 my $state = $problem->state;
                 $reputation_change = 1 if $c->cobrand->reputation_increment_states->{$state};
                 $reputation_change = -1 if $c->cobrand->reputation_decrement_states->{$state};
@@ -384,6 +386,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 };
                 $problem->set_extra_metadata( inspected => 1 );
                 $c->forward( '/admin/log_edit', [ $problem->id, 'problem', 'inspected' ] );
             }
@@ -419,24 +422,22 @@ sub inspect : Private {
             }
             $problem->lastupdate( \'current_timestamp' );
             $problem->update;
-            if ( defined($update_text) ) {
-                my $timestamp = \'current_timestamp';
-                if (my $saved_at = $c->get_param('saved_at')) {
-                    $timestamp = DateTime->from_epoch( epoch => $saved_at );
-                }
-                my $name = $c->user->from_body ? $c->user->from_body->name : $c->user->name;
-                $problem->add_to_comments( {
-                    text => $update_text,
-                    created => $timestamp,
-                    confirmed => $timestamp,
-                    user_id => $c->user->id,
-                    name => $name,
-                    state => 'confirmed',
-                    mark_fixed => 0,
-                    anonymous => 0,
-                    %update_params,
-                } );
+            my $timestamp = \'current_timestamp';
+            if (my $saved_at = $c->get_param('saved_at')) {
+                $timestamp = DateTime->from_epoch( epoch => $saved_at );
             }
+            my $name = $c->user->from_body ? $c->user->from_body->name : $c->user->name;
+            $problem->add_to_comments( {
+                text => $update_text,
+                created => $timestamp,
+                confirmed => $timestamp,
+                user_id => $c->user->id,
+                name => $name,
+                state => 'confirmed',
+                mark_fixed => 0,
+                anonymous => 0,
+                %update_params,
+            } );
             # This problem might no longer be visible on the current cobrand,
             # if its body has changed (e.g. by virtue of the category changing)
             # so redirect to a cobrand where it can be seen if necessary
diff --git a/perllib/FixMyStreet/DB/Result/Comment.pm b/perllib/FixMyStreet/DB/Result/Comment.pm
index 1218d2f76..d688eb8b9 100644
--- a/perllib/FixMyStreet/DB/Result/Comment.pm
+++ b/perllib/FixMyStreet/DB/Result/Comment.pm
@@ -101,6 +101,7 @@ use Moo;
 use namespace::clean -except => [ 'meta' ];
 
 with 'FixMyStreet::Roles::Abuser',
+     'FixMyStreet::Roles::Extra',
      'FixMyStreet::Roles::PhotoSet';
 
 my $stz = sub {
@@ -238,10 +239,19 @@ sub meta_line {
         } elsif ($body eq 'Royal Borough of Greenwich') {
             $body = "$body <img src='/cobrands/greenwich/favicon.png' alt=''>";
         }
-        if ($c->user_exists and $c->user->has_permission_to('view_body_contribute_details', $self->problem->bodies_str_ids)) {
-            $meta = sprintf( _( 'Posted by <strong>%s</strong> (%s) at %s' ), $body, $user_name, Utils::prettify_dt( $self->confirmed ) );
+        my $can_view_contribute = $c->user_exists && $c->user->has_permission_to('view_body_contribute_details', $self->problem->bodies_str_ids);
+        if ($self->text) {
+            if ($can_view_contribute) {
+                $meta = sprintf( _( 'Posted by <strong>%s</strong> (%s) at %s' ), $body, $user_name, Utils::prettify_dt( $self->confirmed ) );
+            } else {
+                $meta = sprintf( _( 'Posted by <strong>%s</strong> at %s' ), $body, Utils::prettify_dt( $self->confirmed ) );
+            }
         } else {
-            $meta = sprintf( _( 'Posted by <strong>%s</strong> at %s' ), $body, Utils::prettify_dt( $self->confirmed ) );
+            if ($can_view_contribute) {
+                $meta = sprintf( _( 'Updated by <strong>%s</strong> (%s) at %s' ), $body, $user_name, Utils::prettify_dt( $self->confirmed ) );
+            } else {
+                $meta = sprintf( _( 'Updated by <strong>%s</strong> at %s' ), $body, Utils::prettify_dt( $self->confirmed ) );
+            }
         }
     } else {
         $meta = sprintf( _( 'Posted by %s at %s' ), FixMyStreet::Template::html_filter($self->name), Utils::prettify_dt( $self->confirmed ) )
@@ -294,6 +304,10 @@ sub meta_line {
         $meta .= ", $update_state";
     }
 
+    if ($self->get_extra_metadata('defect_raised')) {
+        $meta .= ', ' . _( 'and a defect raised' );
+    }
+
     $c->stash->{last_state} = $update_state;
 
     return $meta;
diff --git a/t/app/controller/report_inspect.t b/t/app/controller/report_inspect.t
index e668fddbf..33486aa28 100644
--- a/t/app/controller/report_inspect.t
+++ b/t/app/controller/report_inspect.t
@@ -79,10 +79,21 @@ FixMyStreet::override_config {
             public_update => "This is a public update.", include_update => "1",
             state => 'action scheduled', raise_defect => 1,
         } });
+        $mech->get_ok("/report/$report_id");
+        $mech->submit_form_ok({ with_fields => {
+            update => "This is a second public update, of normal update form, no actual change.",
+        } });
         $report->discard_changes;
-        is $report->comments->first->text, "This is a public update.", 'Update was created';
+        my $comment = ($report->comments( undef, { order_by => { -desc => 'id' } } )->all)[1]->text;
+        is $comment, "This is a public update.", 'Update was created';
         is $report->get_extra_metadata('inspected'), 1, 'report marked as inspected';
         is $report->user->get_extra_metadata('reputation'), $reputation, "User reputation wasn't changed";
+        $mech->get_ok("/report/$report_id");
+        my $meta = $mech->extract_update_metas;
+        like $meta->[0], qr/Updated by .*action scheduled/, 'First update mentions action scheduled';
+        like $meta->[1], qr/Posted by .*defect raised/, 'Update mentions defect raised';
+        unlike $meta->[2], qr/Posted by .*action scheduled/, 'Update does not mention action scheduled';
+
         $user->unset_extra_metadata('categories');
         $user->update;
     };
@@ -245,6 +256,9 @@ FixMyStreet::override_config {
         } });
         $report->discard_changes;
         is $report->get_extra_metadata('inspected'), 1, 'report marked as inspected';
+        $mech->get_ok("/report/$report_id");
+        my $meta = $mech->extract_update_metas;
+        like $meta->[-1], qr/Updated by .*defect raised/, 'Update mentions defect raised';
     };
 
     subtest "Oxfordshire-specific traffic management options are shown" => sub {
diff --git a/t/app/controller/report_updates.t b/t/app/controller/report_updates.t
index 81174181c..4cb035bac 100644
--- a/t/app/controller/report_updates.t
+++ b/t/app/controller/report_updates.t
@@ -928,6 +928,48 @@ subtest 'check meta correct for second comment marking as reopened' => sub {
     like $update_meta->[1], qr/reopened/, 'update meta says reopened';
 };
 
+subtest "check first comment with status change but no text is displayed" => sub {
+    $user->from_body( $body->id );
+    $user->update;
+
+    $report->comments->delete;
+
+    my $comment = FixMyStreet::App->model('DB::Comment')->create(
+        {
+            user          => $user,
+            name          => $user->from_body->name,
+            problem_id    => $report->id,
+            text          => '',
+            confirmed     => DateTime->now( time_zone => 'local'),
+            problem_state => 'investigating',
+            anonymous     => 0,
+            mark_open     => 0,
+            mark_fixed    => 0,
+            state         => 'confirmed',
+        }
+    );
+    $mech->log_in_ok( $user->email );
+
+    $mech->get_ok("/report/$report_id");
+
+    my $update_meta = $mech->extract_update_metas;
+    like $update_meta->[0], qr/Updated by/, 'updated by meta if no text';
+    unlike $update_meta->[0], qr/Test User/, 'commenter name not included';
+    like $update_meta->[0], qr/investigating/, 'update meta includes state change';
+
+    ok $user->user_body_permissions->create({
+      body => $body,
+      permission_type => 'view_body_contribute_details'
+    }), 'Give user view_body_contribute_details permissions';
+
+    $mech->get_ok("/report/$report_id");
+    $update_meta = $mech->extract_update_metas;
+    like $update_meta->[0], qr/Updated by/, 'updated by meta if no text';
+    like $update_meta->[0], qr/Test User/, 'commenter name included if user has view contribute permission';
+    like $update_meta->[0], qr/investigating/, 'update meta includes state change';
+};
+
+
 $user->from_body(undef);
 $user->update;