log all state changes in admin as comments

Create a comment on a problem when the admin is used to change the state
of the problem. If only the state is changed then create a comment with
blank text. If the category and state are changing then include details
of the category change in the comment.

Not all the state changes are displayed at the template level by
default.

Fixes #1846

9 files changed, 338 insertions, 20 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 054564646..bf9b222b3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,6 +19,7 @@
       - Character length limit can be placed on report detailed information #1848
       - Inspector panel shows nearest address if available #1850
       - Return a 200 rather than 404 for ref ID lookup.
+      - Public report page shows state changes made in admin interface #1846
 
 * v2.2 (13th September 2017)
     - New features:
diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm
index 719b04cf6..82041a6b1 100644
--- a/perllib/FixMyStreet/App/Controller/Admin.pm
+++ b/perllib/FixMyStreet/App/Controller/Admin.pm
@@ -913,7 +913,7 @@ sub report_edit : Path('report_edit') : Args(1) {
         }
         $problem->set_inflated_columns(\%columns);
 
-        $c->forward( '/admin/report_edit_category', [ $problem ] );
+        $c->forward( '/admin/report_edit_category', [ $problem, $problem->state ne $old_state ] );
         $c->forward('update_user', [ $problem ]);
 
         # Deal with photos
@@ -935,6 +935,27 @@ sub report_edit : Path('report_edit') : Args(1) {
 
         if ( $problem->state ne $old_state ) {
             $c->forward( 'log_edit', [ $id, 'problem', 'state_change' ] );
+
+            my $name = _('an adminstrator');
+            my $extra = { is_superuser => 1 };
+            if ($c->user->from_body) {
+                $name = $c->user->from_body->name;
+                delete $extra->{is_superuser};
+                $extra->{is_body_user} = $c->user->from_body->id;
+            }
+            my $timestamp = \'current_timestamp';
+            $problem->add_to_comments( {
+                text => $c->stash->{update_text} || '',
+                created => $timestamp,
+                confirmed => $timestamp,
+                user_id => $c->user->id,
+                name => $name,
+                mark_fixed => 0,
+                anonymous => 0,
+                state => 'confirmed',
+                problem_state => $problem->state,
+                extra => $extra
+            } );
         }
         $c->forward( 'log_edit', [ $id, 'problem', 'edit' ] );
 
diff --git a/perllib/FixMyStreet/App/Controller/Moderate.pm b/perllib/FixMyStreet/App/Controller/Moderate.pm
index 1313b5071..42dc759e5 100644
--- a/perllib/FixMyStreet/App/Controller/Moderate.pm
+++ b/perllib/FixMyStreet/App/Controller/Moderate.pm
@@ -85,7 +85,7 @@ sub moderate_report : Chained('report') : PathPart('') : Args(0) {
 
 sub moderating_user_name {
     my $user = shift;
-    return $user->from_body ? $user->from_body->name : 'a FixMyStreet administrator';
+    return $user->from_body ? $user->from_body->name : _('an administrator');
 }
 
 sub report_moderate_audit : Private {
diff --git a/perllib/FixMyStreet/DB/Result/Comment.pm b/perllib/FixMyStreet/DB/Result/Comment.pm
index 409a6e1ab..60fd31510 100644
--- a/perllib/FixMyStreet/DB/Result/Comment.pm
+++ b/perllib/FixMyStreet/DB/Result/Comment.pm
@@ -229,13 +229,24 @@ sub meta_line {
 
     if ($self->anonymous or !$self->name) {
         $meta = sprintf( _( 'Posted anonymously at %s' ), Utils::prettify_dt( $self->confirmed ) )
-    } elsif ($self->user->from_body) {
+    } elsif ($self->user->from_body || $self->get_extra_metadata('is_body_user') || $self->get_extra_metadata('is_superuser') ) {
         my $user_name = FixMyStreet::Template::html_filter($self->user->name);
-        my $body = $self->user->body;
-        if ($body eq 'Bromley Council') {
-            $body = "$body <img src='/cobrands/bromley/favicon.png' alt=''>";
-        } elsif ($body eq 'Royal Borough of Greenwich') {
-            $body = "$body <img src='/cobrands/greenwich/favicon.png' alt=''>";
+        my $body;
+        if ($self->get_extra_metadata('is_superuser')) {
+            $body = _('an administrator');
+        } else {
+            # use this meta data in preference to the user's from_body setting
+            # in case they are no longer with the body, or have changed body.
+            if (my $body_id = $self->get_extra_metadata('is_body_user')) {
+                $body = FixMyStreet::App->model('DB::Body')->find({id => $body_id})->name;
+            } else {
+                $body = $self->user->body;
+            }
+            if ($body eq 'Bromley Council') {
+                $body = "$body <img src='/cobrands/bromley/favicon.png' alt=''>";
+            } elsif ($body eq 'Royal Borough of Greenwich') {
+                $body = "$body <img src='/cobrands/greenwich/favicon.png' alt=''>";
+            }
         }
         my $can_view_contribute = $c->user_exists && $c->user->has_permission_to('view_body_contribute_details', $self->problem->bodies_str_ids);
         if ($self->text) {
diff --git a/t/app/controller/admin.t b/t/app/controller/admin.t
index 9f1b0cfb9..e6a8a34cf 100644
--- a/t/app/controller/admin.t
+++ b/t/app/controller/admin.t
@@ -1,4 +1,6 @@
 use FixMyStreet::TestMech;
+# avoid wide character warnings from the category change message
+use open ':std', ':encoding(UTF-8)';
 
 my $mech = FixMyStreet::TestMech->new;
 
@@ -406,6 +408,7 @@ foreach my $test (
             flagged    => 'on',
             non_public => undef,
         },
+        expect_comment => 1,
         changes   => { state => 'unconfirmed' },
         log_entries => [qw/edit state_change edit edit edit edit edit/],
         resend      => 0,
@@ -422,6 +425,7 @@ foreach my $test (
             flagged    => 'on',
             non_public => undef,
         },
+        expect_comment => 1,
         changes   => { state => 'confirmed' },
         log_entries => [qw/edit state_change edit state_change edit edit edit edit edit/],
         resend      => 0,
@@ -438,6 +442,7 @@ foreach my $test (
             flagged    => 'on',
             non_public => undef,
         },
+        expect_comment => 1,
         changes   => { state => 'fixed' },
         log_entries =>
           [qw/edit state_change edit state_change edit state_change edit edit edit edit edit/],
@@ -455,6 +460,7 @@ foreach my $test (
             flagged    => 'on',
             non_public => undef,
         },
+        expect_comment => 1,
         changes     => { state => 'hidden' },
         log_entries => [
             qw/edit state_change edit state_change edit state_change edit state_change edit edit edit edit edit/
@@ -473,6 +479,7 @@ foreach my $test (
             flagged    => 'on',
             non_public => undef,
         },
+        expect_comment => 1,
         changes => {
             state     => 'confirmed',
             anonymous => 1,
@@ -520,10 +527,58 @@ foreach my $test (
         ],
         resend => 0,
     },
+    {
+        description => 'change state to investigating as body superuser',
+        fields      => {
+            title      => 'Edited Report',
+            detail     => 'Edited Detail',
+            state      => 'confirmed',
+            name       => 'Edited User',
+            username   => $user2->email,
+            anonymous  => 1,
+            flagged    => 'on',
+            non_public => 'on',
+        },
+        expect_comment => 1,
+        user_body => $oxfordshire,
+        changes   => { state => 'investigating' },
+        log_entries => [
+            qw/edit state_change edit resend edit state_change edit state_change edit state_change edit state_change edit state_change edit edit edit edit edit/
+        ],
+        resend => 0,
+    },
+    {
+        description => 'change state to in progess and change category as body superuser',
+        fields      => {
+            title      => 'Edited Report',
+            detail     => 'Edited Detail',
+            state      => 'investigating',
+            name       => 'Edited User',
+            username   => $user2->email,
+            anonymous  => 1,
+            flagged    => 'on',
+            non_public => 'on',
+        },
+        expect_comment => 1,
+        expected_text => '*Category changed from ‘Other’ to ‘Potholes’*',
+        user_body => $oxfordshire,
+        changes   => { state => 'in progress', category => 'Potholes' },
+        log_entries => [
+            qw/edit state_change edit state_change edit resend edit state_change edit state_change edit state_change edit state_change edit state_change edit edit edit edit edit/
+        ],
+        resend => 0,
+    },
   )
 {
     subtest $test->{description} => sub {
+        $report->comments->delete;
         $log_entries->reset;
+
+        if ( $test->{user_body} ) {
+            $superuser->from_body( $test->{user_body}->id );
+            $superuser->update;
+        }
+
         $mech->get_ok("/admin/report_edit/$report_id");
 
         @{$test->{fields}}{'external_id', 'external_body', 'external_team', 'category'} = (13, "", "", "Other");
@@ -565,6 +620,31 @@ foreach my $test (
             $mech->content_contains( 'That problem will now be resent' );
             is $report->whensent, undef, 'mark report to resend';
         }
+
+        if ( $test->{expect_comment} ) {
+            my $comment = $report->comments->first;
+            ok $comment, 'report status change creates comment';
+            is $report->comments->count, 1, 'report only has one comment';
+            if ($test->{expected_text}) {
+                is $comment->text, $test->{expected_text}, 'comment has expected text';
+            } else {
+                is $comment->text, '', 'comment has no text';
+            }
+            if ( $test->{user_body} ) {
+                ok $comment->get_extra_metadata('is_body_user'), 'body user metadata set';
+                ok !$comment->get_extra_metadata('is_superuser'), 'superuser metadata not set';
+                is $comment->name, $test->{user_body}->name, 'comment name is body name';
+            } else {
+                ok !$comment->get_extra_metadata('is_body_user'), 'body user metadata not set';
+                ok $comment->get_extra_metadata('is_superuser'), 'superuser metadata set';
+                is $comment->name, _('an adminstrator'), 'comment name is admin';
+            }
+        } else {
+            is $report->comments->count, 0, 'report has no comments';
+        }
+
+        $superuser->from_body(undef);
+        $superuser->update;
     };
 }
 
@@ -586,6 +666,8 @@ subtest 'change report category' => sub {
     $ox_report->discard_changes;
     is $ox_report->category, 'Traffic lights';
     isnt $ox_report->whensent, undef;
+    is $ox_report->comments->count, 1, "Comment created for update";
+    is $ox_report->comments->first->text, '*Category changed from ‘Potholes’ to ‘Traffic lights’*', 'Comment text correct';
 
     $mech->submit_form_ok( { with_fields => { category => 'Graffiti' } }, 'form_submitted' );
     $ox_report->discard_changes;
@@ -604,7 +686,7 @@ subtest 'change email to new user' => sub {
         state  => $report->state,
         name   => $report->name,
         username => $report->user->email,
-        category => 'Other',
+        category => 'Potholes',
         anonymous => 1,
         flagged => 'on',
         non_public => 'on',
diff --git a/t/app/controller/moderate.t b/t/app/controller/moderate.t
index c3c77866b..c31f4880f 100644
--- a/t/app/controller/moderate.t
+++ b/t/app/controller/moderate.t
@@ -337,7 +337,7 @@ subtest 'And do it as a superuser' => sub {
         problem_title  => 'Good good',
         problem_detail => 'Good good improved',
     }});
-    $mech->content_contains('Moderated by a FixMyStreet administrator');
+    $mech->content_contains('Moderated by an administrator');
 };
 
 done_testing();
diff --git a/t/app/controller/report_updates.t b/t/app/controller/report_updates.t
index 7cb547081..7029365ff 100644
--- a/t/app/controller/report_updates.t
+++ b/t/app/controller/report_updates.t
@@ -918,16 +918,96 @@ subtest 'check meta correct for second comment marking as reopened' => sub {
     like $update_meta->[2], qr/Open/, 'update meta says reopened';
 };
 
-subtest "check first comment with status change but no text is displayed" => sub {
-    $user->from_body( $body->id );
-    $user->update;
+subtest 'check meta correct for comment after mark_fixed with not problem_state' => sub {
+    $report->comments->delete;
+    my $comment = FixMyStreet::App->model('DB::Comment')->create(
+        {
+            user          => $user,
+            problem_id    => $report->id,
+            text          => 'update text',
+            confirmed     => DateTime->now( time_zone => 'local'),
+            problem_state => '',
+            anonymous     => 0,
+            mark_open     => 0,
+            mark_fixed    => 1,
+            state         => 'confirmed',
+        }
+    );
+
+    $mech->get_ok( "/report/" . $report->id );
+    my $update_meta = $mech->extract_update_metas;
+    like $update_meta->[0], qr/fixed/i, 'update meta says fixed';
+
+    $comment = FixMyStreet::App->model('DB::Comment')->create(
+        {
+            user          => $user,
+            problem_id    => $report->id,
+            text          => 'update text',
+            confirmed     => DateTime->now( time_zone => 'local' ) + DateTime::Duration->new( minutes => 1 ),
+            problem_state => 'fixed - user',
+            anonymous     => 0,
+            mark_open     => 0,
+            mark_fixed    => 0,
+            state         => 'confirmed',
+        }
+    );
+
+    $mech->get_ok( "/report/" . $report->id );
+    $update_meta = $mech->extract_update_metas;
+    unlike $update_meta->[2], qr/Fixed/i, 'update meta does not say fixed';
+};
+
+for my $test(
+    {
+      user => $user2,
+      name => $body->name,
+      body => $body,
+      superuser => 0,
+      desc =>"check first comment from body user with status change but no text is displayed"
+    },
+    {
+      user => $user2,
+      name => $body->name,
+      superuser => 0,
+      bodyuser => 1,
+      desc =>"check first comment from ex body user with status change but no text is displayed"
+    },
+    {
+      user => $user2,
+      name => $body->name,
+      body => $body,
+      superuser => 1,
+      desc =>"check first comment from body super user with status change but no text is displayed"
+    },
+    {
+      user => $user2,
+      name => 'an adminstrator',
+      superuser => 1,
+      desc =>"check first comment from super user with status change but no text is displayed"
+    }
+) {
+subtest $test->{desc} => sub {
+    my $extra = {};
+    if ($test->{body}) {
+        $extra->{is_body_user} = $test->{body}->id;
+        $user2->from_body( $test->{body}->id );
+    } else {
+        if ($test->{superuser}) {
+            $extra->{is_superuser} = 1;
+        } elsif ($test->{bodyuser}) {
+            $extra->{is_body_user} = $body->id;
+        }
+        $user2->from_body(undef);
+    }
+    $user2->is_superuser($test->{superuser});
+    $user2->update;
 
     $report->comments->delete;
 
     my $comment = FixMyStreet::App->model('DB::Comment')->create(
         {
-            user          => $user,
-            name          => $user->from_body->name,
+            user          => $test->{user},
+            name          => $test->{name},
             problem_id    => $report->id,
             text          => '',
             confirmed     => DateTime->now( time_zone => 'local'),
@@ -936,17 +1016,30 @@ subtest "check first comment with status change but no text is displayed" => sub
             mark_open     => 0,
             mark_fixed    => 0,
             state         => 'confirmed',
+            extra         => $extra,
         }
     );
     $mech->log_in_ok( $user->email );
 
+
+    ok $user->user_body_permissions->search({
+      body_id => $body->id,
+      permission_type => 'view_body_contribute_details'
+    })->delete, 'Remove user view_body_contribute_details permissions';
+
     $mech->get_ok("/report/$report_id");
 
     my $update_meta = $mech->extract_update_metas;
     like $update_meta->[1], qr/Updated by/, 'updated by meta if no text';
-    unlike $update_meta->[1], qr/Test User/, 'commenter name not included';
+    unlike $update_meta->[1], qr/Commenter/, 'commenter name not included';
     like $update_meta->[0], qr/investigating/i, 'update meta includes state change';
 
+    if ($test->{body} || $test->{bodyuser}) {
+        like $update_meta->[1], qr/Westminster/, 'body user update uses body name';
+    } elsif ($test->{superuser}) {
+        like $update_meta->[1], qr/an administrator/, 'superuser update says an administrator';
+    }
+
     ok $user->user_body_permissions->create({
       body => $body,
       permission_type => 'view_body_contribute_details'
@@ -955,10 +1048,108 @@ subtest "check first comment with status change but no text is displayed" => sub
     $mech->get_ok("/report/$report_id");
     $update_meta = $mech->extract_update_metas;
     like $update_meta->[1], qr/Updated by/, 'updated by meta if no text';
-    like $update_meta->[1], qr/Test User/, 'commenter name included if user has view contribute permission';
+    like $update_meta->[1], qr/Commenter/, 'commenter name included if user has view contribute permission';
     like $update_meta->[0], qr/investigating/i, 'update meta includes state change';
 };
+}
+
+for my $test(
+    {
+      desc =>"check comment from super user hiding report is not displayed",
+      problem_state => 'hidden',
+    },
+    {
+      desc =>"check comment from super user unconfirming report is not displayed",
+      problem_state => 'unconfirmed',
+    }
+) {
+subtest $test->{desc} => sub {
+    my $extra = { is_superuser => 1 };
+    $user2->is_superuser(1);
+    $user2->update;
+
+    $report->comments->delete;
+
+    my $comment = FixMyStreet::App->model('DB::Comment')->create(
+        {
+            user          => $user2,
+            name          => 'an administrator',
+            problem_id    => $report->id,
+            text          => '',
+            confirmed     => DateTime->now( time_zone => 'local'),
+            problem_state => $test->{problem_state},
+            anonymous     => 0,
+            mark_open     => 0,
+            mark_fixed    => 0,
+            state         => 'confirmed',
+            extra         => $extra,
+        }
+    );
+    $mech->log_in_ok( $user->email );
+    $mech->get_ok("/report/$report_id");
+
+    my $update_meta = $mech->extract_update_metas;
+    is scalar(@$update_meta), 0, 'no comments on report';
+  };
+}
+
+for my $test(
+    {
+      desc =>"check comments from super user hiding and unhiding report are not displayed",
+      problem_states => [qw/hidden confirmed/],
+      comment_count => 0,
+    },
+    {
+      desc =>"check comment from super user unconfirming and confirming report are is not displayed",
+      problem_states => [qw/unconfirmed confirmed/],
+      comment_count => 0,
+    },
+    {
+      desc =>"check comment after unconfirming and confirming a report is displayed",
+      problem_states => [qw/unconfirmed confirmed investigating/],
+      comment_count => 2, # state change line + who updated line
+    },
+    {
+      desc =>"check comment after confirming a report after blank state is not displayed",
+      problem_states => ['unconfirmed', '', 'confirmed'],
+      comment_count => 0, # state change line + who updated line
+    },
+) {
+subtest $test->{desc} => sub {
+    my $extra = { is_superuser => 1 };
+    $user2->is_superuser(1);
+    $user2->update;
+
+    $report->comments->delete;
+
+    for my $state (@{$test->{problem_states}}) {
+        my $comment = FixMyStreet::App->model('DB::Comment')->create(
+            {
+                user          => $user2,
+                name          => 'an administrator',
+                problem_id    => $report->id,
+                text          => '',
+                confirmed     => DateTime->now( time_zone => 'local'),
+                problem_state => $state,
+                anonymous     => 0,
+                mark_open     => 0,
+                mark_fixed    => 0,
+                state         => 'confirmed',
+                extra         => $extra,
+            }
+        );
+    }
+    $mech->log_in_ok( $user->email );
+    $mech->get_ok("/report/$report_id");
+
+    my $update_meta = $mech->extract_update_metas;
+    is scalar(@$update_meta), $test->{comment_count}, 'expected number of comments on report';
+  };
+}
 
+$user2->is_superuser(0);
+$user2->from_body(undef);
+$user2->update;
 
 $user->from_body(undef);
 $user->update;
@@ -966,6 +1157,7 @@ $user->update;
 $report->state('confirmed');
 $report->bodies_str($body->id);
 $report->update;
+$report->comments->delete;
 
 for my $test (
     {
diff --git a/templates/web/base/report/update.html b/templates/web/base/report/update.html
index 85624669a..5691376e6 100644
--- a/templates/web/base/report/update.html
+++ b/templates/web/base/report/update.html
@@ -43,10 +43,15 @@
                     </div>
                     [% END %]
 
-                  [% SET update_state = update.problem_state_display(c) %]
-                  [% IF update_state AND update_state != global.last_state AND NOT (global.last_state == "" AND update.problem_state == 'confirmed') %]
-                    <p class="meta-2">[% loc('State changed to:') %] [% update_state %]</p>
+                  [% # Small chance of duplicates in the case of fixed - user followed by fixed - council %]
+                  [% SET update_state = update.problem_state %]
+                  [% IF ( update_state AND update_state != global.last_state AND NOT (global.last_state == "" AND update.problem_state == 'confirmed') ) OR
+                     update.mark_fixed OR update.mark_open
+                  %]
+                    <p class="meta-2">[% loc('State changed to:') %] [% update.problem_state_display(c) %]</p>
                     [%- 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 %]
 
                     <p class="meta-2">
diff --git a/templates/web/base/report/updates.html b/templates/web/base/report/updates.html
index 75e94b1d5..e8a2d4bd3 100644
--- a/templates/web/base/report/updates.html
+++ b/templates/web/base/report/updates.html
@@ -1,5 +1,11 @@
 [% global.last_state = '' %]
 [% FOREACH update IN updates %]
+[%- IF global.last_state == 'hidden' OR global.last_state == 'unconfirmed' OR update.problem_state == 'hidden' OR update.problem_state == 'unconfirmed' %]
+  [%- IF update.problem_state != '' %]
+  [%- global.last_state = update.problem_state %]
+  [%- END %]
+  [%- NEXT %]
+[%- END %]
 [% INCLUDE 'report/update.html' %]
 [% END %]