Remove `thing_type` and `thing_id` columns from settings table (#31971)

app/models/setting.rb

@@ -7,10 +7,8 @@
 #  id         :bigint(8)        not null, primary key
 #  var        :string           not null
 #  value      :text
-#  thing_type :string
 #  created_at :datetime
 #  updated_at :datetime
-#  thing_id   :bigint(8)
 #
 
 # This file is derived from a fork of the `rails-settings-cached` gem available at
@@ -46,10 +44,10 @@ class Setting < ApplicationRecord
   after_commit :rewrite_cache, on: %i(create update)
   after_commit :expire_cache, on: %i(destroy)
 
-  # Settings are server-wide settings only, but they were previously
+  self.ignored_columns += %w(
-  # used for users too. This can be dropped later with a database
+    thing_id
-  # migration dropping any scoped setting.
+    thing_type
-  default_scope { where(thing_type: nil, thing_id: nil) }
+  )
 
   class << self
     # get or set a variable with the variable as the called method

db/migrate/20220304195405_migrate_hide_network_preference.rb

@@ -13,6 +13,13 @@ class MigrateHideNetworkPreference < ActiveRecord::Migration[6.1]
     belongs_to :account
   end
 
+  class Setting < ApplicationRecord
+    # Mirror the behavior of the `Setting` model at this point in db history
+    def value
+      YAML.safe_load(self[:value], permitted_classes: [ActiveSupport::HashWithIndifferentAccess, Symbol]) if self[:value].present?
+    end
+  end
+
   def up
     Account.reset_column_information
 

db/migrate/20241205135901_remove_legacy_user_settings_data.rb

@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+class RemoveLegacyUserSettingsData < ActiveRecord::Migration[7.2]
+  def up
+    connection.execute(<<~SQL.squish)
+      DELETE FROM settings
+      WHERE
+        thing_type IS NOT NULL
+        AND thing_id IS NOT NULL
+    SQL
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

db/post_migrate/20241205135925_remove_legacy_user_settings_columns.rb

@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+class RemoveLegacyUserSettingsColumns < ActiveRecord::Migration[7.2]
+  disable_ddl_transaction!
+
+  def up
+    # In normal usage this should not find anything to delete
+    # Deletion here is already done in RemoveLegacyUserSettingsData migration
+    # and no data like this should be created from app at this point
+    # Deleting again out of caution
+    connection.execute(<<~SQL.squish)
+      DELETE FROM settings
+      WHERE
+        thing_type IS NOT NULL
+        AND thing_id IS NOT NULL
+    SQL
+
+    add_index :settings, :var, unique: true, algorithm: :concurrently
+    remove_index :settings, [:thing_type, :thing_id, :var], name: :index_settings_on_thing_type_and_thing_id_and_var, unique: true
+
+    safety_assured do
+      remove_column :settings, :thing_type, :string
+      remove_column :settings, :thing_id, :bigint
+    end
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.2].define(version: 2024_11_23_160722) do
+ActiveRecord::Schema[7.2].define(version: 2024_12_05_135925) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -939,11 +939,9 @@ ActiveRecord::Schema[7.2].define(version: 2024_11_23_160722) do
   create_table "settings", force: :cascade do |t|
     t.string "var", null: false
     t.text "value"
-    t.string "thing_type"
     t.datetime "created_at", precision: nil
     t.datetime "updated_at", precision: nil
-    t.bigint "thing_id"
+    t.index ["var"], name: "index_settings_on_var", unique: true
-    t.index ["thing_type", "thing_id", "var"], name: "index_settings_on_thing_type_and_thing_id_and_var", unique: true
   end
 
   create_table "severed_relationships", force: :cascade do |t|

spec/fabricators/setting_fabricator.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 Fabricator(:setting) do
-  var 'var'
+  var { sequence(:var) { |n| "var_#{n}" } }
 end