Commit 30cd4fca authored by Steve Abrams's avatar Steve Abrams Committed by Dmytro Zaporozhets

Fix deploy token API to delete deploy_token record

Update deploy_tokens api to delete DeployToken
instead of ProjectDeployToken or GroupDeployToken
to allow proper fk cascade delete.
parent 919fd2dc
......@@ -8,4 +8,10 @@ module DeployTokenMethods
deploy_token.username = params[:username].presence
end
end
def destroy_deploy_token(entity, params)
deploy_token = entity.deploy_tokens.find_by_id!(params[:token_id])
deploy_token.destroy
end
end
# frozen_string_literal: true
module Groups
module DeployTokens
class DestroyService < BaseService
include DeployTokenMethods
def execute
destroy_deploy_token(@group, params)
end
end
end
end
# frozen_string_literal: true
module Projects
module DeployTokens
class DestroyService < BaseService
include DeployTokenMethods
def execute
destroy_deploy_token(@project, params)
end
end
end
end
---
title: Fix deploy token API to properly delete all associated deploy token records
merge_request: 28156
author:
type: fixed
......@@ -85,12 +85,10 @@ module API
delete ':id/deploy_tokens/:token_id' do
authorize!(:destroy_deploy_token, user_project)
deploy_token = user_project.project_deploy_tokens
.find_by_deploy_token_id(params[:token_id])
not_found!('Deploy Token') unless deploy_token
::Projects::DeployTokens::DestroyService.new(
user_project, current_user, token_id: params[:token_id]
).execute
deploy_token.destroy
no_content!
end
end
......@@ -144,13 +142,17 @@ module API
desc 'Delete a group deploy token' do
detail 'This feature was introduced in GitLab 12.9'
end
params do
requires :token_id, type: Integer, desc: 'The deploy token ID'
end
delete ':id/deploy_tokens/:token_id' do
authorize!(:destroy_deploy_token, user_group)
deploy_token = user_group.group_deploy_tokens
.find_by_deploy_token_id!(params[:token_id])
::Groups::DeployTokens::DestroyService.new(
user_group, current_user, token_id: params[:token_id]
).execute
destroy_conditionally!(deploy_token)
no_content!
end
end
end
......
......@@ -175,8 +175,12 @@ describe API::DeployTokens do
it { is_expected.to have_gitlab_http_status(:no_content) }
it 'deletes the deploy token' do
expect { subject }.to change { project.deploy_tokens.count }.by(-1)
it 'calls the deploy token destroy service' do
expect(::Projects::DeployTokens::DestroyService).to receive(:new)
.with(project, user, token_id: deploy_token.id)
.and_return(true)
subject
end
context 'invalid request' do
......@@ -187,9 +191,13 @@ describe API::DeployTokens do
end
it 'returns bad_request with invalid token id' do
delete api("/projects/#{project.id}/deploy_tokens/123abc", user)
expect(::Projects::DeployTokens::DestroyService).to receive(:new)
.with(project, user, token_id: 999)
.and_raise(ActiveRecord::RecordNotFound)
delete api("/projects/#{project.id}/deploy_tokens/999", user)
expect(response).to have_gitlab_http_status(:bad_request)
expect(response).to have_gitlab_http_status(:not_found)
end
end
end
......@@ -307,10 +315,12 @@ describe API::DeployTokens do
group.add_maintainer(user)
end
it 'deletes the deploy token' do
expect { subject }.to change { group.deploy_tokens.count }.by(-1)
it 'calls the deploy token destroy service' do
expect(::Groups::DeployTokens::DestroyService).to receive(:new)
.with(group, user, token_id: group_deploy_token.id)
.and_return(true)
expect(group.deploy_tokens).to be_empty
subject
end
context 'invalid request' do
......@@ -321,7 +331,11 @@ describe API::DeployTokens do
end
it 'returns not found with invalid deploy token id' do
delete api("/groups/#{group.id}/deploy_tokens/bad_id", user)
expect(::Groups::DeployTokens::DestroyService).to receive(:new)
.with(group, user, token_id: 999)
.and_raise(ActiveRecord::RecordNotFound)
delete api("/groups/#{group.id}/deploy_tokens/999", user)
expect(response).to have_gitlab_http_status(:not_found)
end
......
# frozen_string_literal: true
require 'spec_helper'
describe Groups::DeployTokens::DestroyService do
it_behaves_like 'a deploy token deletion service' do
let_it_be(:entity) { create(:group) }
let_it_be(:deploy_token_class) { GroupDeployToken }
let_it_be(:deploy_token) { create(:deploy_token, :group, groups: [entity]) }
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Projects::DeployTokens::DestroyService do
it_behaves_like 'a deploy token deletion service' do
let_it_be(:entity) { create(:project) }
let_it_be(:deploy_token_class) { ProjectDeployToken }
let_it_be(:deploy_token) { create(:deploy_token, projects: [entity]) }
end
end
......@@ -58,3 +58,25 @@ RSpec.shared_examples 'a deploy token creation service' do
end
end
end
RSpec.shared_examples 'a deploy token deletion service' do
let(:user) { create(:user) }
let(:deploy_token_params) { { token_id: deploy_token.id } }
describe '#execute' do
subject { described_class.new(entity, user, deploy_token_params).execute }
it "destroys a token record and it's associated DeployToken" do
expect { subject }.to change { deploy_token_class.count }.by(-1)
.and change { DeployToken.count }.by(-1)
end
context 'invalid token id' do
let(:deploy_token_params) { { token_id: 9999 } }
it 'raises an error' do
expect { subject }.to raise_error(ActiveRecord::RecordNotFound)
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment