Refactor feature flags controller

app/controllers/superadmin/feature_flags_controller.rb

@@ -3,32 +3,34 @@ class Superadmin::FeatureFlagsController < Superadmin::SuperadminController
 
   ssl_required :create, :update, :destroy
 
-  before_filter :get_feature_flag, only: [:create, :update, :destroy]
+  before_filter :load_feature_flag, only: [:update, :destroy]
 
   def create
-    @feature_flag.save
-    render json: @feature_flag, status: 204
+    Carto::FeatureFlag.create!(feature_flag_params)
+
+    render json: @feature_flag, status: :no_content
   end
 
   def update
-    @feature_flag.save
-    render json: @feature_flag, status: 204
+    @feature_flag.update!(feature_flag_params)
+
+    render json: @feature_flag, status: :no_content
   end
 
   def destroy
-    if @feature_flag.present?
-      @feature_flag.destroy
-    end
-    render json: @feature_flag, status: 204
+    @feature_flag.destroy!
+
+    render json: @feature_flag, status: :no_content
   end
 
   private
 
-  def get_feature_flag
-    feature_flag_params = params[:feature_flag]
-    @feature_flag = Carto::FeatureFlag.find_or_initialize_by(id: feature_flag_params[:id])
-    @feature_flag.name = feature_flag_params[:name]
-    @feature_flag.restricted = feature_flag_params[:restricted]
+  def load_feature_flag
+    @feature_flag = Carto::FeatureFlag.find(params[:id])
+  end
+
+  def feature_flag_params
+    params.require(:feature_flag).permit(:id, :name, :restricted)
   end
 
 end

spec/requests/superadmin/feature_flag_spec.rb

@@ -1,55 +1,57 @@
 require_relative '../../acceptance_helper'
 
 describe Carto::FeatureFlag do
+  let(:feature_flag) { create(:feature_flag) }
+
   describe '#create' do
-    it 'should create feature flag' do
-      feature_flag = FactoryGirl.build(:feature_flag)
+    let(:feature_flag) { build(:feature_flag) }
 
-      expect {
-        post superadmin_feature_flags_url, { feature_flag: feature_flag }.to_json, superadmin_headers
+    context 'when everything is OK' do
+      it 'should create feature flag' do
+        payload = { feature_flag: feature_flag.attributes }.to_json
+        expect {
+          post superadmin_feature_flags_url, payload, superadmin_headers
+        }.to change(Carto::FeatureFlag, :count).by(1)
+
+        expect(response.status).to eq(204)
+      end
+    end
+
+    context 'when an error occurs' do
+      it 'returns an error' do
+        post superadmin_feature_flags_url, {}.to_json, superadmin_headers
 
-        response.status.should == 204
-      }.to change(Carto::FeatureFlag, :count).by(1)
+        expect(response.status).to eq(500)
+      end
     end
   end
 
   describe '#update' do
     it 'should update feature flag name' do
-      feature_flag      = FactoryGirl.build(:feature_flag)
-      test_feature_flag = FactoryGirl.build(:feature_flag)
-
-      test_feature_flag.id   = feature_flag.id
-      test_feature_flag.name = "test_new_name"
-      test_feature_flag.save
+      payload = { feature_flag: feature_flag.attributes.merge('name' => 'new_name') }.to_json
+      put superadmin_feature_flag_url(feature_flag.id), payload, superadmin_headers
 
-      old_name = feature_flag.name
-
-      expect {
-        put superadmin_feature_flag_url(feature_flag.id), { feature_flag: test_feature_flag }.to_json, superadmin_headers
-
-        feature_flag.reload
-      }.to change(feature_flag, :name).from(old_name).to(test_feature_flag.name)
+      expect(feature_flag.reload.name).to eq('new_name')
     end
   end
 
   describe '#destroy' do
+    let(:payload) { {}.to_json }
+    let(:user) { create(:user) }
 
-    it 'should destroy feature flag' do
-      feature_flag = FactoryGirl.create(:feature_flag)
+    before { feature_flag.save! }
 
+    it 'should destroy feature flag' do
       expect {
-        delete superadmin_feature_flag_url(feature_flag.id), { feature_flag: feature_flag }.to_json, superadmin_headers
+        delete superadmin_feature_flag_url(feature_flag.id), payload, superadmin_headers
       }.to change(Carto::FeatureFlag, :count).by(-1)
     end
 
     it 'should destroy feature flag user relations' do
-      feature_flag = FactoryGirl.create(:feature_flag)
-      user         = FactoryGirl.create(:user)
-
-      feature_flag_user = FactoryGirl.create(:feature_flags_user, feature_flag_id: feature_flag.id, user_id: user.id)
+      user.self_feature_flags_user.create!(feature_flag: feature_flag)
 
       expect {
-        delete superadmin_feature_flag_url(feature_flag.id), { feature_flag: feature_flag }.to_json, superadmin_headers
+        delete superadmin_feature_flag_url(feature_flag.id), payload, superadmin_headers
       }.to change(Carto::FeatureFlagsUser, :count).by(-1)
     end
   end