Store all moderation change history in database.

Currently keeping the same front end functionality of only reverting to
the original.

11 files changed, 74 insertions, 70 deletions

diff --git a/bin/update-schema b/bin/update-schema
index a2f20c306..55a052ccc 100755
--- a/bin/update-schema
+++ b/bin/update-schema
@@ -212,6 +212,7 @@ else {
 # (assuming schema change files are never half-applied, which should be the case)
 sub get_db_version {
     return 'EMPTY' if ! table_exists('problem');
+    return '0064' if index_exists('moderation_original_data_problem_id_comment_id_idx');
     return '0063' if column_exists('moderation_original_data', 'extra');
     return '0062' if column_exists('users', 'created');
     return '0061' if column_exists('body', 'extra');
@@ -320,3 +321,9 @@ sub function_exists {
     my $fn = shift;
     return $db->dbh->selectrow_array('select count(*) from pg_proc where proname = ?', {}, $fn);
 }
+
+# Returns true if an index exists
+sub index_exists {
+    my $idx = shift;
+    return $db->dbh->selectrow_array('select count(*) from pg_indexes where indexname = ?', {}, $idx);
+}
diff --git a/db/downgrade_0064---0063.sql b/db/downgrade_0064---0063.sql
new file mode 100644
index 000000000..7cce67680
--- /dev/null
+++ b/db/downgrade_0064---0063.sql
@@ -0,0 +1,7 @@
+BEGIN;
+
+DROP INDEX moderation_original_data_problem_id_comment_id_idx;
+ALTER TABLE moderation_original_data
+    ADD CONSTRAINT moderation_original_data_comment_id_key UNIQUE (comment_id);
+
+COMMIT;
diff --git a/db/schema.sql b/db/schema.sql
index a448e05fa..d5d63f478 100644
--- a/db/schema.sql
+++ b/db/schema.sql
@@ -441,7 +441,7 @@ create table moderation_original_data (
 
     -- Problem details
     problem_id int references problem(id) ON DELETE CASCADE not null,
-    comment_id int references comment(id) ON DELETE CASCADE unique,
+    comment_id int references comment(id) ON DELETE CASCADE,
 
     title text null,
     detail text null, -- or text for comment
@@ -456,6 +456,7 @@ create table moderation_original_data (
     latitude double precision,
     longitude double precision
 );
+create index moderation_original_data_problem_id_comment_id_idx on moderation_original_data(problem_id, comment_id);
 
 create table user_body_permissions (
     id serial not null primary key,
diff --git a/db/schema_0064-allow-multiple-moderations.sql b/db/schema_0064-allow-multiple-moderations.sql
new file mode 100644
index 000000000..5fee96282
--- /dev/null
+++ b/db/schema_0064-allow-multiple-moderations.sql
@@ -0,0 +1,8 @@
+BEGIN;
+
+ALTER TABLE moderation_original_data
+    DROP CONSTRAINT moderation_original_data_comment_id_key;
+CREATE INDEX moderation_original_data_problem_id_comment_id_idx
+    ON moderation_original_data(problem_id, comment_id);
+
+COMMIT;
diff --git a/perllib/FixMyStreet/App/Controller/Moderate.pm b/perllib/FixMyStreet/App/Controller/Moderate.pm
index 90396fa07..a6eb59ca1 100644
--- a/perllib/FixMyStreet/App/Controller/Moderate.pm
+++ b/perllib/FixMyStreet/App/Controller/Moderate.pm
@@ -23,9 +23,9 @@ data to change.
   - user to be from_body
   - user to have a "moderate" record in user_body_permissions
 
-The original data of the report is stored in moderation_original_data, so
-that it can be reverted/consulted if required.  All moderation events are
-stored in admin_log.
+The original and previous data of the report is stored in
+moderation_original_data, so that it can be reverted/consulted if required.
+All moderation events are stored in admin_log.
 
 =head1 SEE ALSO
 
@@ -55,7 +55,7 @@ sub report : Chained('moderate') : PathPart('report') : CaptureArgs(1) {
 
     $c->forward('/auth/check_csrf_token');
 
-    my $original = $problem->find_or_new_related( moderation_original_data => {
+    $c->stash->{history} = $problem->new_related( moderation_original_data => {
         title => $problem->title,
         detail => $problem->detail,
         photo => $problem->photo,
@@ -65,8 +65,8 @@ sub report : Chained('moderate') : PathPart('report') : CaptureArgs(1) {
         category => $problem->category,
         extra => $problem->extra,
     });
+    $c->stash->{original} = $problem->moderation_original_data || $c->stash->{history};
     $c->stash->{problem} = $problem;
-    $c->stash->{problem_original} = $original;
     $c->stash->{moderation_reason} = $c->get_param('moderation_reason') // '';
 }
 
@@ -150,6 +150,7 @@ sub report_moderate_audit : Private {
             problem => $problem,
             report_uri => $c->stash->{report_uri},
             report_complain_uri => $c->stash->{cobrand_base} . '/contact?m=' . $token->token,
+            moderated_data => $c->stash->{history},
         });
     }
 }
@@ -172,29 +173,22 @@ sub report_moderate_hide : Private {
 sub moderate_text : Private {
     my ($self, $c, $thing) = @_;
 
-    my ($object, $original, $param);
+    my $object = $c->stash->{comment} || $c->stash->{problem};
+    my $param = $c->stash->{comment} ? 'update_' : 'problem_';
+
     my $thing_for_original_table = $thing;
-    if (my $comment = $c->stash->{comment}) {
-        $object = $comment;
-        $original = $c->stash->{comment_original};
-        $param = 'update_';
-        # Update 'text' field is stored in original table's 'detail' field
-        $thing_for_original_table = 'detail' if $thing eq 'text';
-    } else {
-        $object = $c->stash->{problem};
-        $original = $c->stash->{problem_original};
-        $param = 'problem_';
-    }
+    # Update 'text' field is stored in original table's 'detail' field
+    $thing_for_original_table = 'detail' if $c->stash->{comment} && $thing eq 'text';
 
     my $old = $object->$thing;
-    my $original_thing = $original->$thing_for_original_table;
+    my $original_thing = $c->stash->{original}->$thing_for_original_table;
 
     my $new = $c->get_param($param . 'revert_' . $thing) ?
         $original_thing
         : $c->get_param($param . $thing);
 
     if ($new ne $old) {
-        $original->insert unless $original->in_storage;
+        $c->stash->{history}->insert;
         $object->update({ $thing => $new });
         return $thing_for_original_table;
     }
@@ -205,18 +199,11 @@ sub moderate_text : Private {
 sub moderate_boolean : Private {
     my ( $self, $c, $thing, $reverse ) = @_;
 
-    my ($object, $original, $param);
-    if (my $comment = $c->stash->{comment}) {
-        $object = $comment;
-        $original = $c->stash->{comment_original};
-        $param = 'update_';
-    } else {
-        $object = $c->stash->{problem};
-        $original = $c->stash->{problem_original};
-        $param = 'problem_';
-    }
+    my $object = $c->stash->{comment} || $c->stash->{problem};
+    my $param = $c->stash->{comment} ? 'update_' : 'problem_';
+    my $original = $c->stash->{original}->photo;
 
-    return if $thing eq 'photo' && !$original->photo;
+    return if $thing eq 'photo' && !$original;
 
     my $new;
     if ($reverse) {
@@ -227,9 +214,9 @@ sub moderate_boolean : Private {
     my $old = $object->$thing ? 1 : 0;
 
     if ($new != $old) {
-        $original->insert unless $original->in_storage;
+        $c->stash->{history}->insert;
         if ($thing eq 'photo') {
-            $object->update({ $thing => $new ? $original->photo : undef });
+            $object->update({ $thing => $new ? $original : undef });
         } else {
             $object->update({ $thing => $new });
         }
@@ -241,14 +228,7 @@ sub moderate_boolean : Private {
 sub moderate_extra : Private {
     my ($self, $c) = @_;
 
-    my ($object, $original);
-    if (my $comment = $c->stash->{comment}) {
-        $object = $comment;
-        $original = $c->stash->{comment_original};
-    } else {
-        $object = $c->stash->{problem};
-        $original = $c->stash->{problem_original};
-    }
+    my $object = $c->stash->{comment} || $c->stash->{problem};
 
     my $changed;
     my @extra = grep { /^extra\./ } keys %{$c->req->params};
@@ -257,12 +237,12 @@ sub moderate_extra : Private {
         my $old = $object->get_extra_metadata($field_name) || '';
         my $new = $c->get_param($_);
         if ($new ne $old) {
-            $original->insert unless $original->in_storage;
             $object->set_extra_metadata($field_name, $new);
             $changed = 1;
         }
     }
     if ($changed) {
+        $c->stash->{history}->insert;
         $object->update;
         return 'extra';
     }
@@ -272,7 +252,6 @@ sub moderate_location : Private {
     my ($self, $c) = @_;
 
     my $problem = $c->stash->{problem};
-    my $original = $c->stash->{problem_original};
 
     my $moved = $c->forward('/admin/report_edit_location', [ $problem ]);
     if (!$moved) {
@@ -283,7 +262,7 @@ sub moderate_location : Private {
     }
 
     if ($moved == 2) {
-        $original->insert unless $original->in_storage;
+        $c->stash->{history}->insert;
         $problem->update;
         return 'location';
     }
@@ -299,12 +278,11 @@ sub moderate_category : Private {
     $c->forward('/admin/categories_for_point');
 
     my $problem = $c->stash->{problem};
-    my $original = $c->stash->{problem_original};
 
     my $changed = $c->forward( '/admin/report_edit_category', [ $problem, 1 ] );
     # It might need to set_report_extras in future
     if ($changed) {
-        $original->insert unless $original->in_storage;
+        $c->stash->{history}->insert;
         $problem->update;
         return 'category';
     }
@@ -317,14 +295,14 @@ sub update : Chained('report') : PathPart('update') : CaptureArgs(1) {
     # Make sure user can moderate this update
     $c->detach unless $comment && $c->user->can_moderate($comment);
 
-    my $original = $comment->find_or_new_related( moderation_original_data => {
+    $c->stash->{history} = $comment->new_related( moderation_original_data => {
         detail => $comment->text,
         photo => $comment->photo,
         anonymous => $comment->anonymous,
         extra => $comment->extra,
     });
     $c->stash->{comment} = $comment;
-    $c->stash->{comment_original} = $original;
+    $c->stash->{original} = $comment->moderation_original_data || $c->stash->{history};
 }
 
 sub moderate_update : Chained('update') : PathPart('') : Args(0) {
diff --git a/perllib/FixMyStreet/DB/Result/Comment.pm b/perllib/FixMyStreet/DB/Result/Comment.pm
index 738b58f82..d3551956f 100644
--- a/perllib/FixMyStreet/DB/Result/Comment.pm
+++ b/perllib/FixMyStreet/DB/Result/Comment.pm
@@ -6,7 +6,6 @@ package FixMyStreet::DB::Result::Comment;
 
 use strict;
 use warnings;
-use FixMyStreet::Template;
 
 use base 'DBIx::Class::Core';
 __PACKAGE__->load_components("FilterColumn", "InflateColumn::DateTime", "EncodedColumn");
@@ -70,8 +69,8 @@ __PACKAGE__->add_columns(
   { data_type => "timestamp", is_nullable => 1 },
 );
 __PACKAGE__->set_primary_key("id");
-__PACKAGE__->might_have(
-  "moderation_original_data",
+__PACKAGE__->has_many(
+  "moderation_original_datas",
   "FixMyStreet::DB::Result::ModerationOriginalData",
   { "foreign.comment_id" => "self.id" },
   { cascade_copy => 0, cascade_delete => 0 },
@@ -90,8 +89,8 @@ __PACKAGE__->belongs_to(
 );
 
 
-# Created by DBIx::Class::Schema::Loader v0.07035 @ 2015-08-13 16:33:38
-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:ZR+YNA1Jej3s+8mr52iq6Q
+# Created by DBIx::Class::Schema::Loader v0.07035 @ 2018-11-20 16:13:59
+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:5w/4Og9uCy54lGyyJiLzxA
 #
 
 __PACKAGE__->load_components("+FixMyStreet::DB::RABXColumn");
@@ -99,6 +98,7 @@ __PACKAGE__->rabx_column('extra');
 
 use Moo;
 use namespace::clean -except => [ 'meta' ];
+use FixMyStreet::Template;
 
 with 'FixMyStreet::Roles::Abuser',
      'FixMyStreet::Roles::Extra',
@@ -197,22 +197,17 @@ __PACKAGE__->has_many(
   }
 );
 
-# we already had the `moderation_original_data` rel above, as inferred by
-# Schema::Loader, but that doesn't know about the problem_id mapping, so we now
-# (slightly hackishly) redefine here:
-#
-# we also add cascade_delete, though this seems to be insufficient.
-#
-# TODO: should add FK on moderation_original_data field for this, to get S::L to
-# pick up without hacks.
-
+# This will return the oldest moderation_original_data, if any.
+# The plural can be used to return all entries.
 __PACKAGE__->might_have(
   "moderation_original_data",
   "FixMyStreet::DB::Result::ModerationOriginalData",
   { "foreign.comment_id" => "self.id",
     "foreign.problem_id" => "self.problem_id",
   },
-  { cascade_copy => 0, cascade_delete => 1 },
+  { order_by => 'id',
+    rows => 1,
+    cascade_copy => 0, cascade_delete => 1 },
 );
 
 =head2 meta_line
diff --git a/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm b/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm
index 3e6482658..7478eca87 100644
--- a/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm
+++ b/perllib/FixMyStreet/DB/Result/ModerationOriginalData.pm
@@ -47,7 +47,6 @@ __PACKAGE__->add_columns(
   { data_type => "double precision", is_nullable => 1 },
 );
 __PACKAGE__->set_primary_key("id");
-__PACKAGE__->add_unique_constraint("moderation_original_data_comment_id_key", ["comment_id"]);
 __PACKAGE__->belongs_to(
   "comment",
   "FixMyStreet::DB::Result::Comment",
@@ -67,8 +66,8 @@ __PACKAGE__->belongs_to(
 );
 
 
-# Created by DBIx::Class::Schema::Loader v0.07035 @ 2018-11-13 10:48:41
-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:OQAXPriTc3G2jKFPw0TqdQ
+# Created by DBIx::Class::Schema::Loader v0.07035 @ 2018-11-20 16:13:59
+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:zFOhQnS4WfVzD7qHxaAr6w
 
 use Moo;
 
diff --git a/perllib/FixMyStreet/DB/Result/Problem.pm b/perllib/FixMyStreet/DB/Result/Problem.pm
index 0b8bc4de0..ffc6aba93 100644
--- a/perllib/FixMyStreet/DB/Result/Problem.pm
+++ b/perllib/FixMyStreet/DB/Result/Problem.pm
@@ -177,11 +177,15 @@ __PACKAGE__->has_one(
   { cascade_copy => 0, cascade_delete => 0 },
 );
 
+# This will return the oldest moderation_original_data, if any.
+# The plural can be used to return all entries.
 __PACKAGE__->might_have(
   "moderation_original_data",
   "FixMyStreet::DB::Result::ModerationOriginalData",
   { "foreign.problem_id" => "self.id" },
   { where => { 'comment_id' => undef },
+    order_by => 'id',
+    rows => 1,
     cascade_copy => 0, cascade_delete => 1 },
 );
 
diff --git a/t/app/controller/moderate.t b/t/app/controller/moderate.t
index e80355075..57ae26047 100644
--- a/t/app/controller/moderate.t
+++ b/t/app/controller/moderate.t
@@ -126,6 +126,11 @@ subtest 'Problem moderation' => sub {
         $report->discard_changes;
         is $report->title, 'Good bad good';
         is $report->detail, 'Good bad bad bad good bad';
+
+        my @history = $report->moderation_original_datas->search(undef, { order_by => 'id' })->all;
+        is @history, 2, 'Right number of entries';
+        is $history[0]->title, 'Good bad good', 'Correct original title';
+        is $history[1]->title, 'Good good', 'Correct second title';
     };
 
     subtest 'Make anonymous' => sub {
diff --git a/templates/email/default/problem-moderated.html b/templates/email/default/problem-moderated.html
index 1a12446d0..142f27fc2 100644
--- a/templates/email/default/problem-moderated.html
+++ b/templates/email/default/problem-moderated.html
@@ -25,8 +25,8 @@ the team at <a href="[% report_complain_uri %]">[% report_complain_uri %]</a></p
   [% end_padded_box %]
 </th>
 [% WRAPPER '_email_sidebar.html' object = problem %]
-    <h2 style="[% h2_style %]">[% problem.moderation_original_data.title | html %]</h2>
-    <p style="[% secondary_p_style %]">[% problem.moderation_original_data.detail | html %]</p>
+    <h2 style="[% h2_style %]">[% moderated_data.title | html %]</h2>
+    <p style="[% secondary_p_style %]">[% moderated_data.detail | html %]</p>
 [% END %]
 
 [% INCLUDE '_email_bottom.html' %]
diff --git a/templates/email/default/problem-moderated.txt b/templates/email/default/problem-moderated.txt
index 7c1c3b5c2..ee219672f 100644
--- a/templates/email/default/problem-moderated.txt
+++ b/templates/email/default/problem-moderated.txt
@@ -15,11 +15,11 @@ The following data has been changed:
 
 Your report had the title:
 
-[% problem.moderation_original_data.title %]
+[% moderated_data.title %]
 
 And details:
 
-[% problem.moderation_original_data.detail %]
+[% moderated_data.detail %]
 
 [% UNLESS types == 'hide' %]
 You can see the report at [% report_uri %]