From 4116094e546a3139d6210799e82c8c665aeac7d3 Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Tue, 8 Nov 2022 09:06:13 +1000
Subject: [PATCH] FIX: Make chat editor IDs not null (#18903)

Follow up to 766bcbc6840c9d665055441bcd77616b3a96e10e

Makes ChatMessage.last_editor_id and ChatMessageRevision.user_id
NOT NULL since they are always filled in now and the last commit
had a migration to backfill this data.
---
 plugins/chat/app/models/chat_message.rb                   | 2 +-
 plugins/chat/app/models/chat_message_revision.rb          | 2 +-
 .../20221107034541_make_chat_editor_ids_not_null.rb       | 8 ++++++++
 plugins/chat/lib/message_mover.rb                         | 5 ++++-
 plugins/chat/spec/fabricators/chat_fabricator.rb          | 1 +
 plugins/chat/spec/jobs/chat_channel_delete_spec.rb        | 3 ++-
 .../chat/spec/jobs/regular/chat_notify_mentioned_spec.rb  | 2 +-
 7 files changed, 18 insertions(+), 5 deletions(-)
 create mode 100644 plugins/chat/db/migrate/20221107034541_make_chat_editor_ids_not_null.rb

diff --git a/plugins/chat/app/models/chat_message.rb b/plugins/chat/app/models/chat_message.rb
index f54c4627381..9b4159b3efa 100644
--- a/plugins/chat/app/models/chat_message.rb
+++ b/plugins/chat/app/models/chat_message.rb
@@ -210,7 +210,7 @@ end
 #  message         :text
 #  cooked          :text
 #  cooked_version  :integer
-#  last_editor_id  :integer
+#  last_editor_id  :integer          not null
 #
 # Indexes
 #
diff --git a/plugins/chat/app/models/chat_message_revision.rb b/plugins/chat/app/models/chat_message_revision.rb
index a0caed17fbd..e13cf507e17 100644
--- a/plugins/chat/app/models/chat_message_revision.rb
+++ b/plugins/chat/app/models/chat_message_revision.rb
@@ -15,7 +15,7 @@ end
 #  new_message     :text             not null
 #  created_at      :datetime         not null
 #  updated_at      :datetime         not null
-#  user_id         :integer
+#  user_id         :integer          not null
 #
 # Indexes
 #
diff --git a/plugins/chat/db/migrate/20221107034541_make_chat_editor_ids_not_null.rb b/plugins/chat/db/migrate/20221107034541_make_chat_editor_ids_not_null.rb
new file mode 100644
index 00000000000..5dc971d0119
--- /dev/null
+++ b/plugins/chat/db/migrate/20221107034541_make_chat_editor_ids_not_null.rb
@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+class MakeChatEditorIdsNotNull < ActiveRecord::Migration[7.0]
+  def change
+    change_column_null :chat_messages, :last_editor_id, false
+    change_column_null :chat_message_revisions, :user_id, false
+  end
+end
diff --git a/plugins/chat/lib/message_mover.rb b/plugins/chat/lib/message_mover.rb
index 8e9a80e87bc..fc290f757ca 100644
--- a/plugins/chat/lib/message_mover.rb
+++ b/plugins/chat/lib/message_mover.rb
@@ -93,9 +93,12 @@ class Chat::MessageMover
       destination_channel_id: destination_channel.id,
     }
     moved_message_ids = DB.query_single(<<~SQL, query_args)
-      INSERT INTO chat_messages(chat_channel_id, user_id, message, cooked, cooked_version, created_at, updated_at)
+      INSERT INTO chat_messages(
+        chat_channel_id, user_id, last_editor_id, message, cooked, cooked_version, created_at, updated_at
+      )
       SELECT :destination_channel_id,
              user_id,
+             last_editor_id,
              message,
              cooked,
              cooked_version,
diff --git a/plugins/chat/spec/fabricators/chat_fabricator.rb b/plugins/chat/spec/fabricators/chat_fabricator.rb
index 42dbf855cee..5a6f7f231dc 100644
--- a/plugins/chat/spec/fabricators/chat_fabricator.rb
+++ b/plugins/chat/spec/fabricators/chat_fabricator.rb
@@ -47,6 +47,7 @@ Fabricator(:chat_message_revision) do
   chat_message { Fabricate(:chat_message) }
   old_message { "something old" }
   new_message { "something new" }
+  user { |attrs| attrs[:chat_message].user }
 end
 
 Fabricator(:reviewable_chat_message) do
diff --git a/plugins/chat/spec/jobs/chat_channel_delete_spec.rb b/plugins/chat/spec/jobs/chat_channel_delete_spec.rb
index 1922fbd5f60..55176fdd606 100644
--- a/plugins/chat/spec/jobs/chat_channel_delete_spec.rb
+++ b/plugins/chat/spec/jobs/chat_channel_delete_spec.rb
@@ -36,7 +36,8 @@ describe Jobs::ChatChannelDelete do
     )
 
     revision_message = messages.sample
-    ChatMessageRevision.create(
+    Fabricate(
+      :chat_message_revision,
       chat_message: revision_message,
       old_message: "some old message",
       new_message: revision_message.message,
diff --git a/plugins/chat/spec/jobs/regular/chat_notify_mentioned_spec.rb b/plugins/chat/spec/jobs/regular/chat_notify_mentioned_spec.rb
index b69e3cc4406..0693f32ca61 100644
--- a/plugins/chat/spec/jobs/regular/chat_notify_mentioned_spec.rb
+++ b/plugins/chat/spec/jobs/regular/chat_notify_mentioned_spec.rb
@@ -58,7 +58,7 @@ describe Jobs::ChatNotifyMentioned do
 
     it "does nothing if there is a newer version of the message" do
       message = create_chat_message
-      ChatMessageRevision.create!(chat_message: message, old_message: "a", new_message: "b")
+      Fabricate(:chat_message_revision, chat_message: message, old_message: "a", new_message: "b")
 
       PostAlerter.expects(:push_notification).never