diff --git a/app/controllers/api/v1/split_configs_controller.rb b/app/controllers/api/v1/split_configs_controller.rb index 1b8f1631..40da5913 100644 --- a/app/controllers/api/v1/split_configs_controller.rb +++ b/app/controllers/api/v1/split_configs_controller.rb @@ -9,7 +9,7 @@ def create end def destroy - split = Split.find_by!(name: params[:id]) + split = current_app.splits.find_by!(name: params[:id]) split.update!(finished_at: Time.zone.now) unless split.finished? head :no_content end diff --git a/app/controllers/authenticated_api_controller.rb b/app/controllers/authenticated_api_controller.rb index 5832fa9b..2c268e5c 100644 --- a/app/controllers/authenticated_api_controller.rb +++ b/app/controllers/authenticated_api_controller.rb @@ -5,7 +5,7 @@ class AuthenticatedApiController < UnauthenticatedApiController attr_reader :current_app def authenticate - authenticate_with_http_basic do |app_name, auth_secret| + authenticate_or_request_with_http_basic do |app_name, auth_secret| app = App.find_by(name: app_name) if app && ActiveSupport::SecurityUtils.secure_compare(app.auth_secret, auth_secret) @current_app = app diff --git a/spec/controllers/api/v1/split_configs_controller_spec.rb b/spec/controllers/api/v1/split_configs_controller_spec.rb index a644ca36..813a89a9 100644 --- a/spec/controllers/api/v1/split_configs_controller_spec.rb +++ b/spec/controllers/api/v1/split_configs_controller_spec.rb @@ -3,53 +3,73 @@ RSpec.describe Api::V1::SplitConfigsController, type: :controller do let(:default_app) { FactoryGirl.create :app, name: "default_app", auth_secret: "6Sd6T7T6Q8hKcoo0t8CTzV0IdN1EEHqXB2Ig4raZsOU" } - before do - http_authenticate username: default_app.name, auth_secret: default_app.auth_secret - end - describe '#create' do - it "creates a new split with desired weightings" do + it "doesn't create when unauthenticated" do post :create, name: 'foobar', weighting_registry: { foo: 10, bar: 90 } - expect(response).to have_http_status(:no_content) - split = Split.find_by(name: 'foobar', owner_app: default_app) - expect(split).to be_truthy - expect(split.registry).to eq 'foo' => 10, 'bar' => 90 + expect(response).to have_http_status(:unauthorized) + expect(Split.where(name: 'foobar')).to be_empty end + end - it 'returns errors when invalid' do - post :create, name: 'fooBar', weighting_registry: { foo: 10, bar: 90 } - expect(response).to have_http_status(:unprocessable_entity) - expect(response_json['errors']['name'].first).to include("snake_case") + describe "while authenticated" do + before do + http_authenticate username: default_app.name, auth_secret: default_app.auth_secret end - end - describe '#destroy' do - it "sets the finished_at time on the split" do - split = FactoryGirl.create(:split, name: "old_split", owner_app: default_app, finished_at: nil) + describe '#create' do + it "creates a new split with desired weightings" do + post :create, name: 'foobar', weighting_registry: { foo: 10, bar: 90 } - delete :destroy, id: "old_split" + expect(response).to have_http_status(:no_content) + split = Split.find_by(name: 'foobar', owner_app: default_app) + expect(split).to be_truthy + expect(split.registry).to eq 'foo' => 10, 'bar' => 90 + end - expect(response).to have_http_status(:no_content) - expect(split.reload).to be_finished + it 'returns errors when invalid' do + post :create, name: 'fooBar', weighting_registry: { foo: 10, bar: 90 } + expect(response).to have_http_status(:unprocessable_entity) + expect(response_json['errors']['name'].first).to include("snake_case") + end end - it "is idempotent" do - split = FactoryGirl.create(:split, name: "old_split", owner_app: default_app, finished_at: nil) + describe '#destroy' do + it "sets the finished_at time on the split" do + split = FactoryGirl.create(:split, name: "old_split", owner_app: default_app, finished_at: nil) - Timecop.freeze(Time.zone.parse('2011-01-01')) do delete :destroy, id: "old_split" + + expect(response).to have_http_status(:no_content) + expect(split.reload).to be_finished end - expect(response).to have_http_status(:no_content) - expect(split.reload.finished_at).to eq Time.zone.parse('2011-01-01') + it "can't delete another app's split" do + other_app = FactoryGirl.create :app, name: "other_app" + split = FactoryGirl.create(:split, name: "other_split", owner_app: other_app, finished_at: nil) - Timecop.freeze(Time.zone.parse('2011-01-02')) do - delete :destroy, id: "old_split" + expect { delete :destroy, id: "other_split" }.to raise_error(ActiveRecord::RecordNotFound) + + expect(split.reload).not_to be_finished end - expect(response).to have_http_status(:no_content) - expect(split.reload.finished_at).to eq Time.zone.parse('2011-01-01') + it "is idempotent" do + split = FactoryGirl.create(:split, name: "old_split", owner_app: default_app, finished_at: nil) + + Timecop.freeze(Time.zone.parse('2011-01-01')) do + delete :destroy, id: "old_split" + end + + expect(response).to have_http_status(:no_content) + expect(split.reload.finished_at).to eq Time.zone.parse('2011-01-01') + + Timecop.freeze(Time.zone.parse('2011-01-02')) do + delete :destroy, id: "old_split" + end + + expect(response).to have_http_status(:no_content) + expect(split.reload.finished_at).to eq Time.zone.parse('2011-01-01') + end end end end