From 920aa2dfce86d1eb5f7e961cb1eb177e35a68efd Mon Sep 17 00:00:00 2001
From: Ted Johansson <ted@discourse.org>
Date: Thu, 20 Jun 2024 16:24:48 +0800
Subject: [PATCH] FIX: Prevent field type migration from poisoning AR cache
 (#27549)

We previously migrated field_type from a string to an integer backed enum. Part of this involved renaming a column in a post migration, swapping out field_type:string for field_type:integer. This borks the ActiveRecord cache since the application is already running. Rebooting fixes it, but we want to avoid having this happen in the first place.
---
 app/models/user_field.rb                      |  6 ++++--
 ...add_back_field_type_enum_to_user_fields.rb | 21 +++++++++++++++++++
 ...ype_with_field_type_enum_on_user_fields.rb | 19 ++++++++++-------
 3 files changed, 36 insertions(+), 10 deletions(-)
 create mode 100644 db/migrate/20240620024938_add_back_field_type_enum_to_user_fields.rb

diff --git a/app/models/user_field.rb b/app/models/user_field.rb
index d99696afa82..9feb3636864 100644
--- a/app/models/user_field.rb
+++ b/app/models/user_field.rb
@@ -6,6 +6,7 @@ class UserField < ActiveRecord::Base
   include HasSanitizableFields
 
   deprecate_column :required, drop_from: "3.3"
+  self.ignored_columns += %i[field_type]
 
   validates_presence_of :description
   validates_presence_of :name, unless: -> { field_type == "confirm" }
@@ -19,7 +20,8 @@ class UserField < ActiveRecord::Base
   scope :public_fields, -> { where(show_on_profile: true).or(where(show_on_user_card: true)) }
 
   enum :requirement, { optional: 0, for_all_users: 1, on_signup: 2 }.freeze
-  enum :field_type, { text: 0, confirm: 1, dropdown: 2, multiselect: 3 }.freeze
+  enum :field_type_enum, { text: 0, confirm: 1, dropdown: 2, multiselect: 3 }.freeze
+  alias_attribute :field_type, :field_type_enum
 
   def self.max_length
     2048
@@ -60,5 +62,5 @@ end
 #  external_type     :string
 #  searchable        :boolean          default(FALSE), not null
 #  requirement       :integer          default("optional"), not null
-#  field_type        :integer          not null
+#  field_type_enum   :integer          not null
 #
diff --git a/db/migrate/20240620024938_add_back_field_type_enum_to_user_fields.rb b/db/migrate/20240620024938_add_back_field_type_enum_to_user_fields.rb
new file mode 100644
index 00000000000..b42d361002c
--- /dev/null
+++ b/db/migrate/20240620024938_add_back_field_type_enum_to_user_fields.rb
@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+class AddBackFieldTypeEnumToUserFields < ActiveRecord::Migration[7.0]
+  def change
+    # NOTE: This is here to undo the swap done in SwapFieldTypeWithFieldTypeEnumOnUserFields,
+    #       as that change was breaking the AR cache until the application is rebooted.
+    #       The condition here is to ensure it's only executed if that post-migration has been
+    #       applied.
+    if !ActiveRecord::Base.connection.column_exists?(:user_fields, :field_type_enum)
+      add_column :user_fields, :field_type_enum, :integer
+      change_column_null :user_fields, :field_type, true
+
+      execute(<<~SQL)
+        UPDATE user_fields
+        SET field_type_enum = field_type
+      SQL
+
+      change_column_null :user_fields, :field_type_enum, false
+    end
+  end
+end
diff --git a/db/post_migrate/20240612073116_swap_field_type_with_field_type_enum_on_user_fields.rb b/db/post_migrate/20240612073116_swap_field_type_with_field_type_enum_on_user_fields.rb
index 1143cd8eb49..8b98bc48c08 100644
--- a/db/post_migrate/20240612073116_swap_field_type_with_field_type_enum_on_user_fields.rb
+++ b/db/post_migrate/20240612073116_swap_field_type_with_field_type_enum_on_user_fields.rb
@@ -1,14 +1,17 @@
 # frozen_string_literal: true
 
 class SwapFieldTypeWithFieldTypeEnumOnUserFields < ActiveRecord::Migration[7.0]
-  DROPPED_COLUMNS ||= { user_fields: %i[field_type] }
+  # DROPPED_COLUMNS ||= { user_fields: %i[field_type] }
 
-  def up
-    DROPPED_COLUMNS.each { |table, columns| Migration::ColumnDropper.execute_drop(table, columns) }
-    rename_column :user_fields, :field_type_enum, :field_type
-  end
+  # def up
+  #   # WARNING: Swapping in a column of a different type in a post-migration will break the AR
+  #   #          cache, since the application is already booted, requiring a restart.
+  #   #
+  #   DROPPED_COLUMNS.each { |table, columns| Migration::ColumnDropper.execute_drop(table, columns) }
+  #   rename_column :user_fields, :field_type_enum, :field_type
+  # end
 
-  def down
-    raise ActiveRecord::IrreversibleMigration
-  end
+  # def down
+  #   raise ActiveRecord::IrreversibleMigration
+  # end
 end