diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Moderate.pm | 4 | ||||
| -rw-r--r-- | perllib/FixMyStreet/DB/Result/Comment.pm | 7 | ||||
| -rw-r--r-- | perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm | 116 | ||||
| -rw-r--r-- | perllib/FixMyStreet/DB/Result/Problem.pm | 7 | ||||
| -rw-r--r-- | t/app/controller/moderate.t | 3 | ||||
| -rw-r--r-- | templates/web/base/admin/report_edit.html | 23 | ||||
| -rw-r--r-- | templates/web/base/admin/update_edit.html | 18 |
8 files changed, 177 insertions, 2 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 600cc7348..f0899fd2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ - Store more original stuff on moderation. - Sort user updates in reverse date order. - Improve update display on admin report edit page. + - Keep all moderation history, and show in report/update admin. #2329 - Open311 improvements: - Fix bug in contact group handling. #2323 - Improve validation of fetched reports timestamps. #2327 diff --git a/perllib/FixMyStreet/App/Controller/Moderate.pm b/perllib/FixMyStreet/App/Controller/Moderate.pm index a6eb59ca1..3ed2022ee 100644 --- a/perllib/FixMyStreet/App/Controller/Moderate.pm +++ b/perllib/FixMyStreet/App/Controller/Moderate.pm @@ -63,7 +63,7 @@ sub report : Chained('moderate') : PathPart('report') : CaptureArgs(1) { longitude => $problem->longitude, latitude => $problem->latitude, category => $problem->category, - extra => $problem->extra, + $problem->extra ? (extra => $problem->extra) : (), }); $c->stash->{original} = $problem->moderation_original_data || $c->stash->{history}; $c->stash->{problem} = $problem; @@ -299,7 +299,7 @@ sub update : Chained('report') : PathPart('update') : CaptureArgs(1) { detail => $comment->text, photo => $comment->photo, anonymous => $comment->anonymous, - extra => $comment->extra, + $comment->extra ? (extra => $comment->extra) : (), }); $c->stash->{comment} = $comment; $c->stash->{original} = $comment->moderation_original_data || $c->stash->{history}; diff --git a/perllib/FixMyStreet/DB/Result/Comment.pm b/perllib/FixMyStreet/DB/Result/Comment.pm index d3551956f..3a2b7d84b 100644 --- a/perllib/FixMyStreet/DB/Result/Comment.pm +++ b/perllib/FixMyStreet/DB/Result/Comment.pm @@ -197,6 +197,13 @@ __PACKAGE__->has_many( } ); +sub moderation_history { + my $self = shift; + return $self->moderation_original_datas->search({ + problem_id => $self->problem_id, + }, { order_by => { -desc => 'id' } })->all; +} + # This will return the oldest moderation_original_data, if any. # The plural can be used to return all entries. __PACKAGE__->might_have( diff --git a/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm b/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm index 7478eca87..42ff75295 100644 --- a/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm +++ b/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm @@ -70,10 +70,126 @@ __PACKAGE__->belongs_to( # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:zFOhQnS4WfVzD7qHxaAr6w use Moo; +use Text::Diff; +use Data::Dumper; with 'FixMyStreet::Roles::Extra'; __PACKAGE__->load_components("+FixMyStreet::DB::RABXColumn"); __PACKAGE__->rabx_column('extra'); +sub compare_with { + my ($self, $other) = @_; + if ($self->comment_id) { + my $new_detail = $other->can('text') ? $other->text : $other->detail; + return { + detail => string_diff($self->detail, $new_detail), + photo => $self->compare_photo($other), + anonymous => $self->compare_anonymous($other), + extra => $self->compare_extra($other), + }; + } + return { + title => string_diff($self->title, $other->title), + detail => string_diff($self->detail, $other->detail), + photo => $self->compare_photo($other), + anonymous => $self->compare_anonymous($other), + coords => $self->compare_coords($other), + category => string_diff($self->category, $other->category, single => 1), + extra => $self->compare_extra($other), + } +} + +sub compare_anonymous { + my ($self, $other) = @_; + string_diff( + $self->anonymous ? _('Yes') : _('No'), + $other->anonymous ? _('Yes') : _('No'), + ); +} + +sub compare_coords { + my ($self, $other) = @_; + my $old = join ',', $self->latitude, $self->longitude; + my $new = join ',', $other->latitude, $other->longitude; + string_diff($old, $new, single => 1); +} + +sub compare_photo { + my ($self, $other) = @_; + + my $old = $self->photo || ''; + my $new = $other->photo || ''; + return '' if $old eq $new; + + $old = [ split /,/, $old ]; + $new = [ split /,/, $new ]; + + my $diff = Algorithm::Diff->new( $old, $new ); + my (@added, @deleted); + while ( $diff->Next ) { + next if $diff->Same; + push @deleted, $diff->Items(1); + push @added, $diff->Items(2); + } + return (join ', ', map { + "<del style='background-color:#fcc'>$_</del>"; + } @deleted) . (join ', ', map { + "<ins style='background-color:#cfc'>$_</ins>"; + } @added); +} + +sub compare_extra { + my ($self, $other) = @_; + + my $old = $self->get_extra_metadata; + my $new = $other->get_extra_metadata; + + my $both = { %$old, %$new }; + my @all_keys = sort keys %$both; + my @s; + foreach (@all_keys) { + if ($old->{$_} && $new->{$_}) { + push @s, string_diff("$_ = $old->{$_}", "$_ = $new->{$_}"); + } elsif ($new->{$_}) { + push @s, string_diff("", "$_ = $new->{$_}"); + } else { + push @s, string_diff("$_ = $old->{$_}", ""); + } + } + return join ', ', @s; +} + +sub string_diff { + my ($old, $new, %options) = @_; + + return '' if $old eq $new; + + $old = FixMyStreet::Template::html_filter($old); + $new = FixMyStreet::Template::html_filter($new); + + if ($options{single}) { + $old = [ $old ]; + $new = [ $new ]; + } + $old = [ split //, $old ] unless ref $old; + $new = [ split //, $new ] unless ref $new; + my $diff = Algorithm::Diff->new( $old, $new ); + my $string; + while ($diff->Next) { + my $d = $diff->Diff; + if ($d & 1) { + my $deleted = join '', $diff->Items(1); + $string .= "<del style='background-color:#fcc'>$deleted</del>"; + } + my $inserted = join '', $diff->Items(2); + if ($d & 2) { + $string .= "<ins style='background-color:#cfc'>$inserted</ins>"; + } else { + $string .= $inserted; + } + } + return $string; +} + 1; diff --git a/perllib/FixMyStreet/DB/Result/Problem.pm b/perllib/FixMyStreet/DB/Result/Problem.pm index ffc6aba93..a7884f7d2 100644 --- a/perllib/FixMyStreet/DB/Result/Problem.pm +++ b/perllib/FixMyStreet/DB/Result/Problem.pm @@ -973,6 +973,13 @@ sub latest_moderation_log_entry { return $self->admin_log_entries->search({ action => 'moderation' }, { order_by => { -desc => 'id' } })->first; } +sub moderation_history { + my $self = shift; + return $self->moderation_original_datas->search({ + comment_id => undef, + }, { order_by => { -desc => 'id' } })->all; +} + __PACKAGE__->has_many( "admin_log_entries", "FixMyStreet::DB::Result::AdminLog", diff --git a/t/app/controller/moderate.t b/t/app/controller/moderate.t index 57ae26047..a064a88d4 100644 --- a/t/app/controller/moderate.t +++ b/t/app/controller/moderate.t @@ -459,4 +459,7 @@ subtest 'And do it as a superuser' => sub { $mech->content_contains('Moderated by an administrator'); }; +subtest 'Check moderation history in admin' => sub { + $mech->get_ok('/admin/report_edit/' . $report->id); +}; done_testing(); diff --git a/templates/web/base/admin/report_edit.html b/templates/web/base/admin/report_edit.html index 794e19e4c..a38bacb00 100644 --- a/templates/web/base/admin/report_edit.html +++ b/templates/web/base/admin/report_edit.html @@ -182,6 +182,29 @@ class="admin-offsite-link">[% problem.latitude %], [% problem.longitude %]</a> </div> +[% moderation_history = problem.moderation_history %] +[% IF moderation_history %] +<div class="full-width" style="margin-bottom:-2em; padding-bottom: 2em"> + <h2>[% loc('Moderation history') %]</h2> + [% last_history = problem %] + <ul> + [% FOR history IN moderation_history %] + [% SET diff = history.compare_with(last_history) %] + <li>[% PROCESS format_time time=history.created %]: + [% IF diff.title %]<br>[% loc('Subject:') %] [% diff.title %][% END %] + [% IF diff.detail %]<br>[% loc('Details:') %] [% diff.detail %][% END %] + [% IF diff.photo %]<br>[% loc('Photo') %]: [% diff.photo %][% END %] + [% IF diff.anonymous %]<br>[% loc('Anonymous:') %] [% diff.anonymous %][% END %] + [% IF diff.coords %]<br>[% loc('Latitude/Longitude:') %] [% diff.coords %][% END %] + [% IF diff.category %]<br>[% loc('Category:') %] [% diff.category %][% END %] + [% IF diff.extra %]<br>[% loc('Extra data:') %] [% diff.extra %][% END %] + </li> + [% last_history = history %] + [% END %] + </ul> +</div> +[% END %] + <div class="full-width full-width--bottom"> [% INCLUDE 'admin/list_updates.html' %] </div> diff --git a/templates/web/base/admin/update_edit.html b/templates/web/base/admin/update_edit.html index cb420fc67..d997b89e8 100644 --- a/templates/web/base/admin/update_edit.html +++ b/templates/web/base/admin/update_edit.html @@ -81,4 +81,22 @@ </ul> <input type="submit" class="btn" name="Submit changes" value="[% loc('Submit changes') %]" ></form> +[% moderation_history = update.moderation_history %] +[% IF moderation_history %] + <h2>[% loc('Moderation history') %]</h2> + [% last_history = update %] + <ul> + [% FOR history IN moderation_history %] + [% SET diff = history.compare_with(last_history) %] + <li>[% PROCESS format_time time=history.created %]: + [% IF diff.detail %]<br>[% loc('Text:') %] [% diff.detail %][% END %] + [% IF diff.photo %]<br>[% loc('Photo') %]: [% diff.photo %][% END %] + [% IF diff.anonymous %]<br>[% loc('Anonymous:') %] [% diff.anonymous %][% END %] + [% IF diff.extra %]<br>[% loc('Extra data:') %] [% diff.extra %][% END %] + </li> + [% last_history = history %] + [% END %] + </ul> +[% END %] + [% INCLUDE 'admin/footer.html' %] |