REFACTOR: admin site settings controller specs to requests (#5966)

lib/site_setting_extension.rb

@@ -118,7 +118,7 @@ module SiteSettingExtension
     name = name_arg.to_sym
 
     if name == :default_locale
-      raise ArgumentError.new("Other settings depend on default locale, you can not configure it like this")
+      raise Discourse::InvalidParameters.new("Other settings depend on default locale, you can not configure it like this")
     end
 
     shadowed_val = nil
@@ -347,7 +347,7 @@ module SiteSettingExtension
       self.send("#{name}=", value)
       Discourse.request_refresh! if requires_refresh?(name)
     else
-      raise ArgumentError.new("Either no setting named '#{name}' exists or value provided is invalid")
+      raise Discourse::InvalidParameters.new("Either no setting named '#{name}' exists or value provided is invalid")
     end
   end
 

spec/requests/admin/site_settings_controller_spec.rb

@@ -7,16 +7,17 @@ describe Admin::SiteSettingsController do
   end
 
   context 'while logged in as an admin' do
+    let(:admin) { Fabricate(:admin) }
+
     before do
-      @user = log_in(:admin)
+      sign_in(admin)
     end
 
-    context 'index' do
+    describe '#index' do
       it 'returns valid info' do
-        get :index, format: :json
-        json = ::JSON.parse(response.body)
-        expect(json).to be_present
+        get "/admin/site_settings.json"
         expect(response.status).to eq(200)
+        json = ::JSON.parse(response.body)
         expect(json["site_settings"].length).to be > 100
 
         locale = json["site_settings"].select do |s|
@@ -27,37 +28,38 @@ describe Admin::SiteSettingsController do
       end
     end
 
-    context 'update' do
-
+    describe '#update' do
       before do
         SiteSetting.setting(:test_setting, "default")
         SiteSetting.refresh!
       end
 
       it 'sets the value when the param is present' do
-        put :update, params: {
-          id: 'test_setting', test_setting: 'hello'
-        }, format: :json
-
+        put "/admin/site_settings/test_setting.json", params: {
+          test_setting: 'hello'
+        }
+        expect(response.status).to eq(200)
         expect(SiteSetting.test_setting).to eq('hello')
       end
 
       it 'allows value to be a blank string' do
-        put :update, params: {
-          id: 'test_setting', test_setting: ''
-        }, format: :json
-
+        put "/admin/site_settings/test_setting.json", params: {
+          test_setting: ''
+        }
+        expect(response.status).to eq(200)
         expect(SiteSetting.test_setting).to eq('')
       end
 
       it 'logs the change' do
         SiteSetting.test_setting = 'previous'
-        StaffActionLogger.any_instance.expects(:log_site_setting_change).with('test_setting', 'previous', 'hello')
 
-        put :update, params: {
-          id: 'test_setting', test_setting: 'hello'
-        }, format: :json
+        expect do
+          put "/admin/site_settings/test_setting.json", params: {
+            test_setting: 'hello'
+          }
+        end.to change { UserHistory.where(action: UserHistory.actions[:change_site_setting]).count }.by(1)
 
+        expect(response.status).to eq(200)
         expect(SiteSetting.test_setting).to eq('hello')
       end
 
@@ -65,21 +67,18 @@ describe Admin::SiteSettingsController do
         SiteSetting.setting(:hidden_setting, "hidden", hidden: true)
         SiteSetting.refresh!
 
-        put :update, params: {
-          id: 'hidden_setting', hidden_setting: 'not allowed'
-        }, format: :json
+        put "/admin/site_settings/hidden_setting.json", params: {
+          hidden_setting: 'not allowed'
+        }
 
         expect(SiteSetting.hidden_setting).to eq("hidden")
         expect(response.status).to eq(422)
       end
 
       it 'fails when a setting does not exist' do
-        expect {
-          put :update, params: { id: 'provider', provider: 'gotcha' }, format: :json
-        }.to raise_error(ArgumentError)
+        put "/admin/site_settings/provider.json", params: { provider: 'gotcha' }
+        expect(response.status).to eq(422)
       end
     end
-
   end
-
 end