From 14237476d84ab87db4ac88ff54859f36a9d8f881 Mon Sep 17 00:00:00 2001
From: Matt Jankowski <matt@jankowski.online>
Date: Wed, 18 Sep 2024 13:22:45 -0400
Subject: [PATCH] Remove `thing_type` and `thing_id` columns from `settings`
 table

---
 app/models/setting.rb                         | 10 ++++------
 ...4195405_migrate_hide_network_preference.rb |  7 +++++++
 .../20240918170954_remove_settings_thing.rb   | 19 +++++++++++++++++++
 db/schema.rb                                  |  6 ++----
 spec/fabricators/setting_fabricator.rb        |  2 +-
 5 files changed, 33 insertions(+), 11 deletions(-)
 create mode 100644 db/post_migrate/20240918170954_remove_settings_thing.rb

diff --git a/app/models/setting.rb b/app/models/setting.rb
index 6af7a98c6d..4f0d02e7ac 100644
--- a/app/models/setting.rb
+++ b/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
-  # used for users too. This can be dropped later with a database
-  # migration dropping any scoped setting.
-  default_scope { where(thing_type: nil, thing_id: nil) }
+  self.ignored_columns += %w(
+    thing_type
+    thing_id
+  )
 
   class << self
     # get or set a variable with the variable as the called method
diff --git a/db/migrate/20220304195405_migrate_hide_network_preference.rb b/db/migrate/20220304195405_migrate_hide_network_preference.rb
index 0083e0422a..996b7d78ab 100644
--- a/db/migrate/20220304195405_migrate_hide_network_preference.rb
+++ b/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
 
diff --git a/db/post_migrate/20240918170954_remove_settings_thing.rb b/db/post_migrate/20240918170954_remove_settings_thing.rb
new file mode 100644
index 0000000000..cf9d1bbd79
--- /dev/null
+++ b/db/post_migrate/20240918170954_remove_settings_thing.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+class RemoveSettingsThing < ActiveRecord::Migration[7.1]
+  disable_ddl_transaction!
+
+  def change
+    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
+end
diff --git a/db/schema.rb b/db/schema.rb
index f9f2d97395..018e209bf6 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.1].define(version: 2024_09_16_190140) do
+ActiveRecord::Schema[7.1].define(version: 2024_09_18_170954) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -935,11 +935,9 @@ ActiveRecord::Schema[7.1].define(version: 2024_09_16_190140) 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 ["thing_type", "thing_id", "var"], name: "index_settings_on_thing_type_and_thing_id_and_var", unique: true
+    t.index ["var"], name: "index_settings_on_var", unique: true
   end
 
   create_table "severed_relationships", force: :cascade do |t|
diff --git a/spec/fabricators/setting_fabricator.rb b/spec/fabricators/setting_fabricator.rb
index ce9a48e901..689a0de002 100644
--- a/spec/fabricators/setting_fabricator.rb
+++ b/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