FIX for dynamic config

app/controllers/settings/providers_controller.rb

@@ -28,9 +28,6 @@ class Settings::ProvidersController < ApplicationController
 
     updated_fields = []
 
-    # This hash will store only the updates for dynamic (non-declared) fields
-    dynamic_updates = {}
-
     # Perform all updates within a transaction for consistency
     Setting.transaction do
       provider_params.each do |param_key, param_value|
@@ -57,32 +54,13 @@ class Settings::ProvidersController < ApplicationController
           # This is safe and uses the proper setter.
           Setting.public_send("#{key_str}=", value)
         else
-          # If it's a dynamic field, add it to our batch hash
-          # to avoid the Read-Modify-Write conflict.
-          dynamic_updates[key_str] = value
+          # If it's a dynamic field, set it as an individual entry
+          # Each field is stored independently, preventing race conditions
+          Setting[key_str] = value
         end
 
         updated_fields << param_key
       end
-
-      # Now, if we have any dynamic updates, apply them all at once
-      if dynamic_updates.any?
-        # 1. READ the current hash once
-        current_dynamic = Setting.dynamic_fields.dup
-
-        # 2. MODIFY by merging changes
-        # Treat nil values as deletions to keep the hash clean
-        dynamic_updates.each do |key, value|
-          if value.nil?
-            current_dynamic.delete(key)
-          else
-            current_dynamic[key] = value
-          end
-        end
-
-        # 3. WRITE the complete, merged hash back once
-        Setting.dynamic_fields = current_dynamic
-      end
     end
 
     if updated_fields.any?

app/models/provider/configurable.rb

@@ -1,8 +1,8 @@
 # Module for providers to declare their configuration requirements
 #
 # Providers can declare their own configuration fields without needing to modify
-# the Setting model. Settings are stored dynamically using RailsSettings::Base's
-# hash-style access (Setting[:key] = value).
+# the Setting model. Settings are stored dynamically as individual entries using
+# RailsSettings::Base's bracket-style access (Setting[:key] = value).
 #
 # Configuration fields are automatically registered and displayed in the UI at
 # /settings/providers. The system checks Setting storage first, then ENV variables,
@@ -186,8 +186,8 @@ module Provider::Configurable
 
     # Get the value for this field (Setting -> ENV -> default)
     def value
-      # First try Setting using dynamic hash-style access
-      # This works even without explicit field declarations in Setting model
+      # First try Setting using dynamic bracket-style access
+      # Each field is stored as an individual entry without explicit field declarations
       setting_value = Setting[setting_key]
       return normalize_value(setting_value) if setting_value.present?
 

app/models/setting.rb

@@ -11,9 +11,8 @@ class Setting < RailsSettings::Base
   field :openai_model, type: :string, default: ENV["OPENAI_MODEL"]
   field :brand_fetch_client_id, type: :string, default: ENV["BRAND_FETCH_CLIENT_ID"]
 
-  # Single hash field for all dynamic provider credentials and other dynamic settings
-  # This allows unlimited dynamic fields without declaring them upfront
-  field :dynamic_fields, type: :hash, default: {}
+  # Dynamic fields are now stored as individual entries with "dynamic:" prefix
+  # This prevents race conditions and ensures each field is independently managed
 
   # Onboarding and app settings
   ONBOARDING_STATES = %w[open closed invite_only].freeze
@@ -50,7 +49,7 @@ class Setting < RailsSettings::Base
     end
 
     # Support dynamic field access via bracket notation
-    # First checks if it's a declared field, then falls back to dynamic_fields hash
+    # First checks if it's a declared field, then falls back to individual dynamic entries
     def [](key)
       key_str = key.to_s
 
@@ -58,8 +57,8 @@ class Setting < RailsSettings::Base
       if respond_to?(key_str)
         public_send(key_str)
       else
-        # Fall back to dynamic_fields hash
-        dynamic_fields[key_str]
+        # Fall back to individual dynamic entry lookup
+        find_by(var: dynamic_key_name(key_str))&.value
       end
     end
 
@@ -70,21 +69,26 @@ class Setting < RailsSettings::Base
       if respond_to?("#{key_str}=")
         public_send("#{key_str}=", value)
       else
-        # Otherwise, manage in dynamic_fields hash
-        current_dynamic = dynamic_fields.dup
+        # Store as individual dynamic entry
+        dynamic_key = dynamic_key_name(key_str)
         if value.nil?
-          current_dynamic.delete(key_str)          # treat nil as delete
+          where(var: dynamic_key).destroy_all
         else
-          current_dynamic[key_str] = value
+          # Use upsert to avoid conflicts
+          record = find_or_initialize_by(var: dynamic_key)
+          record.value = value
+          record.save!
         end
-        self.dynamic_fields = current_dynamic      # persists & busts cache
       end
     end
 
     # Check if a dynamic field exists (useful to distinguish nil value vs missing key)
     def key?(key)
       key_str = key.to_s
-      respond_to?(key_str) || dynamic_fields.key?(key_str)
+      return true if respond_to?(key_str)
+
+      # Check if dynamic entry exists
+      where(var: dynamic_key_name(key_str)).exists?
     end
 
     # Delete a dynamic field
@@ -92,15 +96,21 @@ class Setting < RailsSettings::Base
       key_str = key.to_s
       return nil if respond_to?(key_str) # Can't delete declared fields
 
-      current_dynamic = dynamic_fields.dup
-      value = current_dynamic.delete(key_str)
-      self.dynamic_fields = current_dynamic
+      dynamic_key = dynamic_key_name(key_str)
+      value = self[key_str]
+      where(var: dynamic_key).destroy_all
       value
     end
 
     # List all dynamic field keys (excludes declared fields)
     def dynamic_keys
-      dynamic_fields.keys
+      where("var LIKE ?", "dynamic:%").pluck(:var).map { |var| var.sub(/^dynamic:/, "") }
+    end
+
+    private
+
+    def dynamic_key_name(key_str)
+      "dynamic:#{key_str}"
     end
   end
 

db/migrate/20251111094448_migrate_dynamic_fields_to_individual_entries.rb

@@ -0,0 +1,35 @@
+class MigrateDynamicFieldsToIndividualEntries < ActiveRecord::Migration[7.2]
+  def up
+    # Find the dynamic_fields setting record
+    dynamic_fields_record = Setting.find_by(var: "dynamic_fields")
+    return unless dynamic_fields_record
+
+    # Parse the hash and create individual entries
+    dynamic_fields_hash = dynamic_fields_record.value || {}
+    dynamic_fields_hash.each do |key, value|
+      Setting.create!(
+        var: "dynamic:#{key}",
+        value: value
+      )
+    end
+
+    # Delete the old dynamic_fields record
+    dynamic_fields_record.destroy
+  end
+
+  def down
+    # Collect all dynamic: entries back into a hash
+    dynamic_fields_hash = {}
+    Setting.where("var LIKE ?", "dynamic:%").find_each do |record|
+      key = record.var.sub(/^dynamic:/, "")
+      dynamic_fields_hash[key] = record.value
+      record.destroy
+    end
+
+    # Recreate the dynamic_fields record with the hash
+    Setting.create!(
+      var: "dynamic_fields",
+      value: dynamic_fields_hash
+    ) if dynamic_fields_hash.any?
+  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: 2025_11_10_104411) do
+ActiveRecord::Schema[7.2].define(version: 2025_11_11_094448) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pgcrypto"
   enable_extension "plpgsql"
@@ -687,6 +687,10 @@ ActiveRecord::Schema[7.2].define(version: 2025_11_10_104411) do
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
     t.string "name"
+    t.boolean "manual", default: false, null: false
+    t.decimal "expected_amount_min", precision: 19, scale: 4
+    t.decimal "expected_amount_max", precision: 19, scale: 4
+    t.decimal "expected_amount_avg", precision: 19, scale: 4
     t.index ["family_id", "merchant_id", "amount", "currency"], name: "idx_recurring_txns_merchant", unique: true, where: "(merchant_id IS NOT NULL)"
     t.index ["family_id", "name", "amount", "currency"], name: "idx_recurring_txns_name", unique: true, where: "((name IS NOT NULL) AND (merchant_id IS NULL))"
     t.index ["family_id", "status"], name: "index_recurring_transactions_on_family_id_and_status"

test/controllers/settings/providers_controller_test.rb

@@ -41,10 +41,10 @@ class Settings::ProvidersControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "updates dynamic provider fields using batch update" do
-    # plaid_client_id is a dynamic field, so it should go through dynamic_fields hash
+    # plaid_client_id is a dynamic field, stored as an individual entry
     with_self_hosting do
       # Clear any existing plaid settings
-      Setting.dynamic_fields = {}
+      Setting["plaid_client_id"] = nil
 
       patch settings_providers_url, params: {
         setting: { plaid_client_id: "test_client_id" }
@@ -52,14 +52,16 @@ class Settings::ProvidersControllerTest < ActionDispatch::IntegrationTest
 
       assert_redirected_to settings_providers_url
       assert_equal "test_client_id", Setting["plaid_client_id"]
-      assert_equal "test_client_id", Setting.dynamic_fields["plaid_client_id"]
     end
   end
 
   test "batches multiple dynamic fields from same provider atomically" do
-    # Test that multiple fields from Plaid are updated together in one write operation
+    # Test that multiple fields from Plaid are updated as individual entries
     with_self_hosting do
-      Setting.dynamic_fields = {}
+      # Clear existing fields
+      Setting["plaid_client_id"] = nil
+      Setting["plaid_secret"] = nil
+      Setting["plaid_environment"] = nil
 
       patch settings_providers_url, params: {
         setting: {
@@ -71,7 +73,7 @@ class Settings::ProvidersControllerTest < ActionDispatch::IntegrationTest
 
       assert_redirected_to settings_providers_url
 
-      # All three should be present in dynamic_fields
+      # All three should be present as individual entries
       assert_equal "new_client_id", Setting["plaid_client_id"]
       assert_equal "new_secret", Setting["plaid_secret"]
       assert_equal "production", Setting["plaid_environment"]
@@ -79,9 +81,14 @@ class Settings::ProvidersControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "batches dynamic fields from multiple providers atomically" do
-    # Test that fields from different providers are all batched together
+    # Test that fields from different providers are stored as individual entries
     with_self_hosting do
-      Setting.dynamic_fields = {}
+      # Clear existing fields
+      Setting["plaid_client_id"] = nil
+      Setting["plaid_secret"] = nil
+      Setting["plaid_eu_client_id"] = nil
+      Setting["plaid_eu_secret"] = nil
+      Setting["simplefin_setup_token"] = nil
 
       patch settings_providers_url, params: {
         setting: {
@@ -108,10 +115,8 @@ class Settings::ProvidersControllerTest < ActionDispatch::IntegrationTest
     # Test that updating some fields doesn't overwrite other existing fields
     with_self_hosting do
       # Set initial fields
-      Setting.dynamic_fields = {
-        "existing_field_1" => "value1",
-        "plaid_client_id" => "old_client_id"
-      }
+      Setting["existing_field_1"] = "value1"
+      Setting["plaid_client_id"] = "old_client_id"
 
       # Update one field and add a new one
       patch settings_providers_url, params: {
@@ -162,7 +167,7 @@ class Settings::ProvidersControllerTest < ActionDispatch::IntegrationTest
       # Set initial values
       Setting["plaid_client_id"] = "old_value"
       assert_equal "old_value", Setting["plaid_client_id"]
-      assert Setting.dynamic_fields.key?("plaid_client_id")
+      assert Setting.key?("plaid_client_id")
 
       patch settings_providers_url, params: {
         setting: { plaid_client_id: "  " }  # Blank string with spaces
@@ -170,18 +175,18 @@ class Settings::ProvidersControllerTest < ActionDispatch::IntegrationTest
 
       assert_redirected_to settings_providers_url
       assert_nil Setting["plaid_client_id"]
-      # Key should be removed from hash, not just set to nil
-      refute Setting.dynamic_fields.key?("plaid_client_id"),
-        "nil values should delete the key from dynamic_fields"
+      # Entry should be removed, not just set to nil
+      refute Setting.key?("plaid_client_id"),
+        "nil values should delete the entry"
     end
   end
 
   test "handles sequential updates to different dynamic fields safely" do
     # This test simulates what would happen if two requests tried to update
-    # different dynamic fields sequentially. With the batch update approach,
-    # all changes should be preserved.
+    # different dynamic fields sequentially. With individual entries,
+    # all changes should be preserved without conflicts.
     with_self_hosting do
-      Setting.dynamic_fields = { "existing_field" => "existing_value" }
+      Setting["existing_field"] = "existing_value"
 
       # Simulate first request updating plaid fields
       patch settings_providers_url, params: {
@@ -266,8 +271,8 @@ class Settings::ProvidersControllerTest < ActionDispatch::IntegrationTest
   test "logs errors when update fails" do
     with_self_hosting do
       # Test that errors during update are properly logged and handled gracefully
-      # We'll force an error by making the dynamic_fields= setter raise
-      Setting.expects(:dynamic_fields=).raises(StandardError.new("Database error")).once
+      # We'll force an error by making the []= method raise
+      Setting.expects(:[]=).with("plaid_client_id", "test").raises(StandardError.new("Database error")).once
 
       # Mock logger to verify error is logged
       Rails.logger.expects(:error).with(regexp_matches(/Failed to update provider settings.*Database error/)).once

test/models/setting_test.rb

@@ -7,6 +7,11 @@ class SettingTest < ActiveSupport::TestCase
     Setting.openai_model = nil
   end
 
+  teardown do
+    # Clean up dynamic fields after each test
+    Setting.where("var LIKE ?", "dynamic:%").destroy_all
+  end
+
   test "validate_openai_config! passes when both uri base and model are set" do
     assert_nothing_raised do
       Setting.validate_openai_config!(uri_base: "https://api.example.com", model: "gpt-4")
@@ -61,4 +66,69 @@ class SettingTest < ActiveSupport::TestCase
       Setting.validate_openai_config!(uri_base: "", model: nil)
     end
   end
+
+  # Dynamic field tests
+  test "can set and get dynamic fields" do
+    Setting["custom_key"] = "custom_value"
+    assert_equal "custom_value", Setting["custom_key"]
+  end
+
+  test "can set and get multiple dynamic fields independently" do
+    Setting["key1"] = "value1"
+    Setting["key2"] = "value2"
+    Setting["key3"] = "value3"
+
+    assert_equal "value1", Setting["key1"]
+    assert_equal "value2", Setting["key2"]
+    assert_equal "value3", Setting["key3"]
+  end
+
+  test "setting nil value deletes dynamic field" do
+    Setting["temp_key"] = "temp_value"
+    assert_equal "temp_value", Setting["temp_key"]
+
+    Setting["temp_key"] = nil
+    assert_nil Setting["temp_key"]
+  end
+
+  test "can delete dynamic field" do
+    Setting["delete_key"] = "delete_value"
+    assert_equal "delete_value", Setting["delete_key"]
+
+    value = Setting.delete("delete_key")
+    assert_equal "delete_value", value
+    assert_nil Setting["delete_key"]
+  end
+
+  test "key? returns true for existing dynamic field" do
+    Setting["exists_key"] = "exists_value"
+    assert Setting.key?("exists_key")
+  end
+
+  test "key? returns false for non-existing dynamic field" do
+    assert_not Setting.key?("nonexistent_key")
+  end
+
+  test "dynamic_keys returns all dynamic field keys" do
+    Setting["dynamic1"] = "value1"
+    Setting["dynamic2"] = "value2"
+
+    keys = Setting.dynamic_keys
+    assert_includes keys, "dynamic1"
+    assert_includes keys, "dynamic2"
+  end
+
+  test "declared fields take precedence over dynamic fields" do
+    # Try to set a declared field using bracket notation
+    Setting["openai_model"] = "custom-model"
+    assert_equal "custom-model", Setting["openai_model"]
+    assert_equal "custom-model", Setting.openai_model
+  end
+
+  test "cannot delete declared fields" do
+    Setting.openai_model = "test-model"
+    result = Setting.delete("openai_model")
+    assert_nil result
+    assert_equal "test-model", Setting.openai_model
+  end
 end