Fix SMS settings page: toggle-driven visibility, password merging, background save

- Wrap provider section in #sms_provider_section, hidden when SMS disabled
- Extend embedded JS to sync section visibility with enable toggle
- Add Turbo frame attribute for background saves without full page reload
- Fix password field merging: preserve existing secrets when blank submitted
- Expand system spec with behavioral tests: toggle visibility, provider switching, saving
- Add request spec for controller actions: auth, password retention, test_message

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

app/controllers/sms_settings_controller.rb

@@ -1,6 +1,9 @@
 # frozen_string_literal: true
 
 class SmsSettingsController < ApplicationController
+  PASSWORD_FIELDS = %w[basic_auth_token twilio_auth_token voipms_api_password signalwire_api_token].freeze
+  private_constant :PASSWORD_FIELDS
+
   before_action :load_encrypted_config
   authorize_resource :encrypted_config, only: :index
   authorize_resource :encrypted_config, parent: false, only: %i[create test_message]
@@ -46,7 +49,7 @@ class SmsSettingsController < ApplicationController
   end
 
   def build_sms_value
-    params.require(:encrypted_config).require(:value).permit(
+    permitted = params.require(:encrypted_config).require(:value).permit(
       :enabled,
       :provider,
       :basic_auth_token,
@@ -62,6 +65,14 @@ class SmsSettingsController < ApplicationController
       :signalwire_project_id,
       :signalwire_api_token,
       :signalwire_from
-    )
+    ).to_h
+
+    existing = @encrypted_config.value.is_a?(Hash) ? @encrypted_config.value : {}
+
+    PASSWORD_FIELDS.each do |field|
+      permitted[field] = existing[field] if permitted[field].blank? && existing[field].present?
+    end
+
+    permitted
   end
 end

app/views/sms_settings/index.html.erb

@@ -60,7 +60,9 @@
       </div>
     <% end %>
 
-    <%= form_for @encrypted_config, url: settings_sms_path, method: :post, html: { autocomplete: 'off', class: 'space-y-4', id: 'sms_settings_form' } do |f| %>
+    <%= form_for @encrypted_config, url: settings_sms_path, method: :post,
+          html: { autocomplete: 'off', class: 'space-y-4', id: 'sms_settings_form' },
+          data: { turbo_frame: :_top } do |f| %>
       <%= f.fields_for :value do |ff| %>
         <div class="form-control">
           <label class="label cursor-pointer" for="encrypted_config_value_enabled">
@@ -68,6 +70,7 @@
             <%= ff.check_box :enabled, { class: 'toggle', checked: value['enabled'] == true || value['enabled'] == '1' || value['enabled'] == 'true' }, '1', '0' %>
           </label>
         </div>
+        <div id="sms_provider_section" class="space-y-4<%= ' hidden' unless value['enabled'] == true || value['enabled'] == '1' || value['enabled'] == 'true' %>">
           <div class="form-control">
             <%= ff.label :provider, 'Provider', class: 'label' %>
             <%= ff.select :provider,
@@ -169,6 +172,7 @@
               <span class="label-text-alt mt-1 opacity-70">A SignalWire number from <strong>Phone Numbers</strong>. Full E.164 with leading <code>+</code>.</span>
             </div>
           </div>
+        </div>
       <% end %>
       <div class="form-control pt-2">
         <%= f.button button_title(title: t('save'), disabled_with: t('saving')), class: 'base-button' %>
@@ -206,16 +210,28 @@
     }
   }
   ready(function() {
+    var toggle  = document.getElementById('encrypted_config_value_enabled');
+    var section = document.getElementById('sms_provider_section');
     var select  = document.getElementById('sms_provider_select');
     if (!select) return;
-    function sync() {
+
+    function syncSection() {
+      if (!section || !toggle) return;
+      section.classList.toggle('hidden', !toggle.checked);
+    }
+
+    function syncProvider() {
       var v = select.value;
       document.querySelectorAll('[data-provider-block]').forEach(function(b) {
         b.classList.toggle('hidden', b.dataset.providerBlock !== v);
       });
     }
-    select.addEventListener('change', sync);
+
-    sync();
+    if (toggle) toggle.addEventListener('change', syncSection);
+    select.addEventListener('change', syncProvider);
+
+    syncSection();
+    syncProvider();
   });
 })();
 </script>

spec/requests/sms_settings_spec.rb

@@ -0,0 +1,170 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe 'SMS Settings', type: :request do
+  let!(:account) { create(:account) }
+  let!(:admin)   { create(:user, account:, role: User::ADMIN_ROLE, email: 'admin@wabo.cc') }
+  let!(:editor)  { create(:user, account:, role: User::EDITOR_ROLE, email: 'editor@wabo.cc') }
+
+  describe 'GET /settings/sms' do
+    it 'renders ok for admin' do
+      sign_in admin
+      get settings_sms_path
+
+      expect(response).to have_http_status(:ok)
+    end
+
+    it 'redirects editor to root' do
+      sign_in editor
+      get settings_sms_path
+
+      expect(response).to redirect_to(root_path)
+    end
+  end
+
+  describe 'POST /settings/sms (create)' do
+    before { sign_in admin }
+
+    it 'creates a new SMS config and redirects with notice' do
+      expect {
+        post settings_sms_path, params: {
+          encrypted_config: {
+            value: {
+              enabled: '1',
+              provider: 'bulkvs',
+              basic_auth_token: 'tok123',
+              from_number: '15551234567',
+              delivery_webhook_url: ''
+            }
+          }
+        }
+      }.to change(EncryptedConfig, :count).by(1)
+
+      expect(response).to redirect_to(settings_sms_path)
+      follow_redirect!
+      expect(response.body).to include(I18n.t('changes_have_been_saved'))
+
+      config = EncryptedConfig.find_by(account:, key: EncryptedConfig::SMS_CONFIGS_KEY)
+      expect(config.value['basic_auth_token']).to eq('tok123')
+      expect(config.value['from_number']).to eq('15551234567')
+    end
+
+    it 'preserves existing Twilio auth token when blank is submitted' do
+      create(:encrypted_config, account:, key: EncryptedConfig::SMS_CONFIGS_KEY,
+                                 value: { 'enabled' => true, 'provider' => 'twilio',
+                                          'twilio_account_sid' => 'AC123',
+                                          'twilio_auth_token' => 'keep_me',
+                                          'twilio_from' => '+15551234567' })
+
+      post settings_sms_path, params: {
+        encrypted_config: {
+          value: {
+            enabled: '1',
+            provider: 'twilio',
+            twilio_account_sid: 'AC123',
+            twilio_auth_token: '',
+            twilio_from: '+15551234567'
+          }
+        }
+      }
+
+      config = EncryptedConfig.find_by(account:, key: EncryptedConfig::SMS_CONFIGS_KEY)
+      expect(config.value['twilio_auth_token']).to eq('keep_me')
+    end
+
+    it 'preserves existing BulkVS basic_auth_token when blank is submitted' do
+      create(:encrypted_config, account:, key: EncryptedConfig::SMS_CONFIGS_KEY,
+                                 value: { 'enabled' => true, 'provider' => 'bulkvs',
+                                          'basic_auth_token' => 'keep_me',
+                                          'from_number' => '15551234567' })
+
+      post settings_sms_path, params: {
+        encrypted_config: {
+          value: { enabled: '1', provider: 'bulkvs', basic_auth_token: '', from_number: '15551234567' }
+        }
+      }
+
+      config = EncryptedConfig.find_by(account:, key: EncryptedConfig::SMS_CONFIGS_KEY)
+      expect(config.value['basic_auth_token']).to eq('keep_me')
+    end
+
+    it 'preserves existing VoIP.ms API password when blank is submitted' do
+      create(:encrypted_config, account:, key: EncryptedConfig::SMS_CONFIGS_KEY,
+                                 value: { 'enabled' => true, 'provider' => 'voipms',
+                                          'voipms_api_username' => 'user@example.com',
+                                          'voipms_api_password' => 'keep_me',
+                                          'voipms_did' => '5551234567' })
+
+      post settings_sms_path, params: {
+        encrypted_config: {
+          value: {
+            enabled: '1', provider: 'voipms',
+            voipms_api_username: 'user@example.com', voipms_api_password: '', voipms_did: '5551234567'
+          }
+        }
+      }
+
+      config = EncryptedConfig.find_by(account:, key: EncryptedConfig::SMS_CONFIGS_KEY)
+      expect(config.value['voipms_api_password']).to eq('keep_me')
+    end
+
+    it 'preserves existing SignalWire API token when blank is submitted' do
+      create(:encrypted_config, account:, key: EncryptedConfig::SMS_CONFIGS_KEY,
+                                 value: { 'enabled' => true, 'provider' => 'signalwire',
+                                          'signalwire_space_url' => 'test.signalwire.com',
+                                          'signalwire_project_id' => 'uuid-1234',
+                                          'signalwire_api_token' => 'keep_me',
+                                          'signalwire_from' => '+15551234567' })
+
+      post settings_sms_path, params: {
+        encrypted_config: {
+          value: {
+            enabled: '1', provider: 'signalwire',
+            signalwire_space_url: 'test.signalwire.com', signalwire_project_id: 'uuid-1234',
+            signalwire_api_token: '', signalwire_from: '+15551234567'
+          }
+        }
+      }
+
+      config = EncryptedConfig.find_by(account:, key: EncryptedConfig::SMS_CONFIGS_KEY)
+      expect(config.value['signalwire_api_token']).to eq('keep_me')
+    end
+
+    it 'redirects editor to root' do
+      sign_in editor
+      post settings_sms_path, params: {
+        encrypted_config: { value: { enabled: '1', provider: 'bulkvs' } }
+      }
+
+      expect(response).to redirect_to(root_path)
+    end
+  end
+
+  describe 'POST /settings/sms/test_message' do
+    before { sign_in admin }
+
+    it 'redirects with alert when phone is blank' do
+      post test_message_settings_sms_path, params: { phone: '' }
+
+      expect(response).to redirect_to(settings_sms_path)
+      follow_redirect!
+      expect(response.body).to include('Enter a phone number')
+    end
+
+    it 'redirects with alert when SMS is not configured' do
+      post test_message_settings_sms_path, params: { phone: '15551234567' }
+
+      expect(response).to redirect_to(settings_sms_path)
+      follow_redirect!
+      expect(response.body).to include('Test failed')
+    end
+
+    it 'redirects editor to root' do
+      sign_in editor
+      post test_message_settings_sms_path, params: { phone: '15551234567' }
+
+      expect(response).to redirect_to(root_path)
+    end
+  end
+end

spec/system/sms_settings_spec.rb

@@ -42,4 +42,132 @@ RSpec.describe 'SMS Settings' do
     expect(page).to have_content('SMS is enabled')
     expect(page).to have_content('Send a test SMS')
   end
+
+  describe 'enable toggle visibility' do
+    context 'when SMS is disabled (no saved config)' do
+      it 'hides the provider section on page load' do
+        visit settings_sms_path
+
+        expect(page).to have_css('#sms_provider_section.hidden', visible: :hidden)
+      end
+    end
+
+    context 'when SMS is enabled' do
+      before do
+        create(:encrypted_config, account:, key: EncryptedConfig::SMS_CONFIGS_KEY,
+                                   value: { 'enabled' => true, 'provider' => 'bulkvs',
+                                            'basic_auth_token' => 'tok', 'from_number' => '15551234567' })
+      end
+
+      it 'shows the provider section on page load' do
+        visit settings_sms_path
+
+        expect(page).to have_css('#sms_provider_section', visible: :visible)
+      end
+    end
+
+    it 'shows the provider section when the toggle is turned on' do
+      visit settings_sms_path
+
+      expect(page).to have_css('#sms_provider_section.hidden', visible: :hidden)
+
+      find('#encrypted_config_value_enabled').click
+
+      expect(page).to have_css('#sms_provider_section', visible: :visible)
+    end
+
+    it 'hides the provider section when the toggle is turned off' do
+      create(:encrypted_config, account:, key: EncryptedConfig::SMS_CONFIGS_KEY,
+                                 value: { 'enabled' => true, 'provider' => 'bulkvs',
+                                          'basic_auth_token' => 'tok', 'from_number' => '15551234567' })
+
+      visit settings_sms_path
+
+      expect(page).to have_css('#sms_provider_section', visible: :visible)
+
+      find('#encrypted_config_value_enabled').click
+
+      expect(page).to have_css('#sms_provider_section.hidden', visible: :hidden)
+    end
+  end
+
+  describe 'provider switching' do
+    before do
+      create(:encrypted_config, account:, key: EncryptedConfig::SMS_CONFIGS_KEY,
+                                 value: { 'enabled' => true, 'provider' => 'bulkvs',
+                                          'basic_auth_token' => 'tok', 'from_number' => '15551234567' })
+      visit settings_sms_path
+    end
+
+    it 'shows only the BulkVS block when BulkVS is the saved provider' do
+      expect(page).to have_css('[data-provider-block="bulkvs"]', visible: :visible)
+      expect(page).to have_css('[data-provider-block="twilio"]', visible: :hidden)
+      expect(page).to have_css('[data-provider-block="voipms"]', visible: :hidden)
+      expect(page).to have_css('[data-provider-block="signalwire"]', visible: :hidden)
+    end
+
+    it 'switches to Twilio fields when Twilio is selected' do
+      select 'Twilio', from: 'encrypted_config[value][provider]'
+
+      expect(page).to have_css('[data-provider-block="twilio"]', visible: :visible)
+      expect(page).to have_css('[data-provider-block="bulkvs"]', visible: :hidden)
+    end
+
+    it 'switches to VoIP.ms fields when VoIP.ms is selected' do
+      select 'VoIP.ms', from: 'encrypted_config[value][provider]'
+
+      expect(page).to have_css('[data-provider-block="voipms"]', visible: :visible)
+      expect(page).to have_css('[data-provider-block="twilio"]', visible: :hidden)
+    end
+
+    it 'switches to SignalWire fields when SignalWire is selected' do
+      select 'SignalWire', from: 'encrypted_config[value][provider]'
+
+      expect(page).to have_css('[data-provider-block="signalwire"]', visible: :visible)
+      expect(page).to have_css('[data-provider-block="bulkvs"]', visible: :hidden)
+    end
+  end
+
+  describe 'saving settings' do
+    it 'saves BulkVS configuration and shows success flash' do
+      visit settings_sms_path
+
+      find('#encrypted_config_value_enabled').click
+
+      within('[data-provider-block="bulkvs"]') do
+        fill_in 'BulkVS Basic Auth Token', with: 'mytoken123'
+        fill_in 'From Number', with: '15551234567'
+      end
+
+      expect { click_button 'Save' }.to change(EncryptedConfig, :count).by(1)
+
+      expect(page).to have_content('Changes have been saved')
+
+      config = EncryptedConfig.find_by(account:, key: EncryptedConfig::SMS_CONFIGS_KEY)
+      expect(config.value['enabled']).to eq('1')
+      expect(config.value['basic_auth_token']).to eq('mytoken123')
+      expect(config.value['from_number']).to eq('15551234567')
+    end
+
+    it 'retains existing Twilio auth token when left blank on re-save' do
+      create(:encrypted_config, account:, key: EncryptedConfig::SMS_CONFIGS_KEY,
+                                 value: { 'enabled' => true, 'provider' => 'twilio',
+                                          'twilio_account_sid' => 'AC123',
+                                          'twilio_auth_token' => 'secret_token',
+                                          'twilio_from' => '+15551234567' })
+
+      visit settings_sms_path
+
+      within('[data-provider-block="twilio"]') do
+        fill_in 'Twilio Account SID', with: 'AC123'
+        fill_in 'From Number', with: '+15551234567'
+        # Auth Token intentionally left blank — should be preserved from saved value
+      end
+
+      click_button 'Save'
+
+      config = EncryptedConfig.find_by(account:, key: EncryptedConfig::SMS_CONFIGS_KEY)
+      expect(config.value['twilio_auth_token']).to eq('secret_token')
+    end
+  end
 end