Spot moderation conflicts.

If someone tries to moderate after someone else has, re-show the
form and an error.

6 files changed, 53 insertions, 10 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 054b8c3ce..485806de4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,7 @@
     - Admin improvements:
         - Include moderation history in report updates. #2379
         - Allow moderation to potentially change state. #2381
+        - Spot moderation conflicts and raise an error. #2384
     - Bugfixes
         - Check cached reports do still have photos before being shown. #2374
         - Delete cache photos upon photo moderation. #2374
diff --git a/perllib/FixMyStreet/App/Controller/Moderate.pm b/perllib/FixMyStreet/App/Controller/Moderate.pm
index d5bd2f3a9..0ec6cbb63 100644
--- a/perllib/FixMyStreet/App/Controller/Moderate.pm
+++ b/perllib/FixMyStreet/App/Controller/Moderate.pm
@@ -90,6 +90,7 @@ sub moderate_report : Chained('report') : PathPart('') : Args(0) {
     # Make sure user can moderate this report
     $c->detach unless $c->user->can_moderate($problem);
 
+    $c->forward('check_edited_elsewhere');
     $c->forward('report_moderate_hide');
 
     my @types = grep $_,
@@ -127,6 +128,24 @@ sub moderate_report : Chained('report') : PathPart('') : Args(0) {
     $c->detach( 'report_moderate_audit', \@types );
 }
 
+sub check_edited_elsewhere : Private {
+    my ($self, $c) = @_;
+
+    my $problem = $c->stash->{problem};
+    my $last_moderation = $problem->latest_moderation;
+    return unless $last_moderation;
+
+    my $form_started = $c->get_param('form_started') || 0;
+    if ($form_started && $form_started < $last_moderation->created->epoch) {
+        $c->stash->{moderate_errors} ||= [];
+        push @{$c->stash->{moderate_errors}},
+            _('Someone has moderated this report since you started.') . ' ' .
+            sprintf(_('Please <a href="#%s">check their changes</a> and resolve any differences.'),
+            'update_m' . $last_moderation->id);
+        $c->detach;
+    }
+}
+
 sub moderating_user_name {
     my $user = shift;
     return $user->from_body ? $user->from_body->name : _('an administrator');
diff --git a/perllib/FixMyStreet/App/Controller/Report.pm b/perllib/FixMyStreet/App/Controller/Report.pm
index 1eac76c9a..a7a3328dc 100644
--- a/perllib/FixMyStreet/App/Controller/Report.pm
+++ b/perllib/FixMyStreet/App/Controller/Report.pm
@@ -243,6 +243,7 @@ sub load_updates : Private {
         my $last_history = $problem;
         foreach my $history (@history) {
             push @combined, [ $history->created, {
+                id => 'm' . $history->id,
                 type => 'moderation',
                 last => $last_history,
                 entry => $history,
diff --git a/perllib/FixMyStreet/Roles/Moderation.pm b/perllib/FixMyStreet/Roles/Moderation.pm
index f43b65208..fb9ea3a70 100644
--- a/perllib/FixMyStreet/Roles/Moderation.pm
+++ b/perllib/FixMyStreet/Roles/Moderation.pm
@@ -7,12 +7,18 @@ Return most recent AdminLog object concerning moderation
 
 =cut
 
-sub latest_moderation_log_entry {
+sub latest_moderation {
     my $self = shift;
 
-    my $latest = $self->moderation_original_datas->search(
+    return $self->moderation_original_datas->search(
         $self->moderation_filter,
         { order_by => { -desc => 'id' } })->first;
+}
+
+sub latest_moderation_log_entry {
+    my $self = shift;
+
+    my $latest = $self->latest_moderation;
     return unless $latest;
 
     my $rs = $self->result_source->schema->resultset("AdminLog");
diff --git a/t/app/controller/moderate.t b/t/app/controller/moderate.t
index 438a81b25..7ef24bbe8 100644
--- a/t/app/controller/moderate.t
+++ b/t/app/controller/moderate.t
@@ -139,6 +139,20 @@ subtest 'Problem moderation' => sub {
         is $history[1]->title, 'Good good', 'Correct second title';
     };
 
+    subtest 'Post modified title after edited elsewhere' => sub {
+        $mech->submit_form_ok({ with_fields => {
+            %problem_prepopulated,
+            problem_title => 'Good good',
+            form_started => 1, # January 1970!
+        }});
+        $mech->base_like( qr{\Q/moderate$REPORT_URL\E} );
+        $mech->content_contains('Good bad good'); # Displayed title
+        $mech->content_contains('Good good'); # Form edit
+
+        $report->discard_changes;
+        is $report->title, 'Good bad good', 'title unchanged';
+    };
+
     subtest 'Make anonymous' => sub {
         $mech->content_lacks('Reported anonymously');
 
diff --git a/templates/web/base/report/_main.html b/templates/web/base/report/_main.html
index ed4805b36..e4612cc4a 100644
--- a/templates/web/base/report/_main.html
+++ b/templates/web/base/report/_main.html
@@ -1,4 +1,5 @@
 [%
+USE date;
 can_moderate = permissions.moderate OR c.user.can_moderate(problem, staff = permissions.moderate)
 can_moderate_title = c.user.can_moderate_title(problem, can_moderate)
 %]
@@ -43,6 +44,7 @@ can_moderate_title = c.user.can_moderate_title(problem, can_moderate)
     [% original = problem_original %]
     <form method="post" action="/moderate/report/[% problem.id %]">
         <input type="hidden" name="token" value="[% csrf_token %]">
+        <input type="hidden" name="form_started" value="[% date.now %]">
   [% END %]
 
   [% FOR error IN moderate_errors %]
@@ -54,11 +56,11 @@ can_moderate_title = c.user.can_moderate_title(problem, can_moderate)
     <div class="moderate-edit">
       [% IF problem.title != original.title %]
         <label>
-            <input type="checkbox" name="problem_revert_title" class="revert-title">
+            <input type="checkbox" name="problem_revert_title" class="revert-title"[% " checked" IF c.req.params.problem_revert_title %]>
             [% loc('Revert to original title') %]
         </label>
       [% END %]
-        <h1><input class="form-control" type="text" name="problem_title" value="[% problem.title | html %]" data-original-value="[% original.title | html %]"></h1>
+        <h1><input class="form-control" type="text" name="problem_title" value="[% (c.req.params.problem_title || problem.title) | html %]" data-original-value="[% original.title | html %]"></h1>
     </div>
   [% ELSE %]
     <h1>[% problem.title | html %]</h1>
@@ -66,7 +68,7 @@ can_moderate_title = c.user.can_moderate_title(problem, can_moderate)
 
     <div class="moderate-edit">
         <label>
-            <input type="checkbox" name="problem_show_name" [% 'checked' UNLESS problem.anonymous %]>
+            <input type="checkbox" name="problem_show_name" [% 'checked' IF !problem.anonymous OR c.req.params.problem_show_name %]>
             [% loc('Show reporter&rsquo;s name') %]
         </label>
     </div>
@@ -89,7 +91,7 @@ can_moderate_title = c.user.can_moderate_title(problem, can_moderate)
     [% IF problem.photo or original.photo %]
         <p class="moderate-edit">
             <label>
-                <input type="checkbox" name="problem_photo" [% problem.photo ? 'checked' : '' %]>
+                <input type="checkbox" name="problem_photo" [% 'checked' IF problem.photo OR c.req.params.problem_photo %]>
                 [% loc('Show photo') %]
             </label>
         </p>
@@ -105,23 +107,23 @@ can_moderate_title = c.user.can_moderate_title(problem, can_moderate)
     <p class="moderate-edit">
       [% IF problem.detail != original.detail %]
         <label>
-            <input type="checkbox" name="problem_revert_detail" class="revert-textarea">
+            <input type="checkbox" name="problem_revert_detail" class="revert-textarea"[% ' checked' IF c.req.params.problem_revert_detail %]>
             [% loc('Revert to original text') %]
         </label>
       [% END %]
-        <textarea class="form-control" name="problem_detail" data-original-value="[% original.detail | html %]">[% problem.detail | html %]</textarea>
+        <textarea class="form-control" name="problem_detail" data-original-value="[% original.detail | html %]">[% (c.req.params.problem_detail || problem.detail) | html %]</textarea>
     </p>
 
     <div class="moderate-edit">
         <p>
             <label>
-                <input type="checkbox" class="hide-document" name="problem_hide" [% problem.hidden ? 'checked' : '' %]>
+                <input type="checkbox" class="hide-document" name="problem_hide" [% 'checked' IF problem.hidden OR c.req.params.problem_hide %]>
                 [% loc('Hide entire report') %]
             </label>
         </p>
         <p>
             <label for="moderation_reason">[% loc('Describe why you are moderating this') %]</label>
-            <input type="text" class="form-control" name="moderation_reason">
+            <input type="text" class="form-control" name="moderation_reason" value="[% (c.req.params.moderation_reason || '') | html %]">
         </p>
         <p>
             <input type="submit" class="green-btn" value="[% loc('Save changes') %]">