FIX: Allow re-flagging of ninja-edited posts (#21360)

What is the problem?

Consider the following timeline:

1. OP starts a topic.
2. Troll responds snarkily.
3. Flagger flags the post as “inappropriate”.
4. Admin agrees and hides the post.
5. Troll ninja-edits the post within the grace period, but still snarky.
6. Flagger flags the post as inappropriate again.

The current behaviour is that the flagger is met with an error saying the post has been reviewed and can't be flagged again for the same reason.

The desired behaviour is after someone has edited a post, it should be flaggable again.

Why is this happening?

This is related to the ninja-edit feature, where within a set grace period no new revision is created, but a new revision is required to flag the same post for the same reason.

So essentially there is a window between the naughty corner cooldown where a flagged post can't be edited, and the ninja-edit grace period, where an edit can be made without a new revision. Posts that are edited within this window can't be re-flagged by the same user.

|-----------------|-------------------------------|
^ Flag accepted   | ~~~~~~~~~~~~~ 🥷🏻 ~~~~~~~~~~~~ |
                  |                               ^ Editing grace period over
                  ^ Naughty corner cooldown over

How does this fix it?

We already create a new revision when ninja-editing a post with a pending flag. The issue above happens only in the case where the flag is already accepted.

This change extends the existing behaviour so that a new revision is created when ninja-editing any flagged post, regardless of the status of the flag. (Deleted flags excluded.)

This should also help with posterity, avoiding situations where a successfully flagged post looks innocuous in the history because it was ninja-edited, and vice versa.

lib/post_revisor.rb

@@ -371,14 +371,18 @@ class PostRevisor
 
   def should_create_new_version?
     return false if @skip_revision
-    edited_by_another_user? || !grace_period_edit? || owner_changed? || force_new_version? ||
-      edit_reason_specified?
+    edited_by_another_user? || flagged? || !grace_period_edit? || owner_changed? ||
+      force_new_version? || edit_reason_specified?
   end
 
   def edit_reason_specified?
     @fields[:edit_reason].present? && @fields[:edit_reason] != @post.edit_reason
   end
 
+  def flagged?
+    @post.is_flagged?
+  end
+
   def edited_by_another_user?
     @post.last_editor_id != @editor.id
   end
@@ -422,7 +426,6 @@ class PostRevisor
 
   def grace_period_edit?
     return false if (@revised_at - @last_version_at) > SiteSetting.editing_grace_period.to_i
-    return false if @post.reviewable_flag.present?
 
     if new_raw = @fields[:raw]
       max_diff = SiteSetting.editing_grace_period_max_diff.to_i

spec/lib/post_revisor_spec.rb

@@ -388,6 +388,25 @@ RSpec.describe PostRevisor do
         expect(post.revisions.count).to eq(1)
       end
 
+      it "creates a new version when the post is flagged" do
+        SiteSetting.editing_grace_period = 1.minute
+
+        post = Fabricate(:post, raw: "hello world")
+
+        Fabricate(:flag, post: post, user: user)
+
+        revisor = PostRevisor.new(post)
+        revisor.revise!(
+          post.user,
+          { raw: "hello world, JK" },
+          revised_at: post.updated_at + 1.second,
+        )
+
+        post.reload
+        expect(post.version).to eq(2)
+        expect(post.revisions.count).to eq(1)
+      end
+
       it "doesn't create a new version" do
         SiteSetting.editing_grace_period = 1.minute
         SiteSetting.editing_grace_period_max_diff = 100