Commit fa0b4321 authored by Etienne Baqué's avatar Etienne Baqué Committed by Shinya Maeda

Added validation to AfterCreateService

Removed previous solution for this MR.
Set up call to Sentry when validation fails in service.
parent b7fa9dc4
...@@ -135,7 +135,7 @@ class Deployment < ApplicationRecord ...@@ -135,7 +135,7 @@ class Deployment < ApplicationRecord
end end
def create_ref def create_ref
project.repository.create_ref(ref, ref_path) project.repository.create_ref(sha, ref_path)
end end
def invalidate_cache def invalidate_cache
...@@ -280,12 +280,12 @@ class Deployment < ApplicationRecord ...@@ -280,12 +280,12 @@ class Deployment < ApplicationRecord
errors.add(:ref, _('The branch or tag does not exist')) errors.add(:ref, _('The branch or tag does not exist'))
end end
private
def ref_path def ref_path
File.join(environment.ref_path, 'deployments', iid.to_s) File.join(environment.ref_path, 'deployments', iid.to_s)
end end
private
def legacy_finished_at def legacy_finished_at
self.created_at if success? && !read_attribute(:finished_at) self.created_at if success? && !read_attribute(:finished_at)
end end
......
...@@ -193,15 +193,6 @@ class Environment < ApplicationRecord ...@@ -193,15 +193,6 @@ class Environment < ApplicationRecord
folder_name == "production" folder_name == "production"
end end
def first_deployment_for(commit_sha)
ref = project.repository.ref_name_for_sha(ref_path, commit_sha)
return unless ref
deployment_iid = ref.split('/').last
deployments.find_by(iid: deployment_iid)
end
def ref_path def ref_path
"refs/#{Repository::REF_ENVIRONMENTS}/#{slug}" "refs/#{Repository::REF_ENVIRONMENTS}/#{slug}"
end end
......
---
title: Use of sha instead of ref when creating a new ref on deployment creation.
merge_request: 23170
author:
type: fixed
...@@ -520,6 +520,21 @@ describe Deployment do ...@@ -520,6 +520,21 @@ describe Deployment do
end end
end end
describe '#create_ref' do
let(:deployment) { build(:deployment) }
subject { deployment.create_ref }
it 'creates a ref using the sha' do
expect(deployment.project.repository).to receive(:create_ref).with(
deployment.sha,
"refs/environments/#{deployment.environment.name}/deployments/#{deployment.iid}"
)
subject
end
end
describe '#playable_build' do describe '#playable_build' do
subject { deployment.playable_build } subject { deployment.playable_build }
......
...@@ -325,26 +325,6 @@ describe Environment, :use_clean_rails_memory_store_caching do ...@@ -325,26 +325,6 @@ describe Environment, :use_clean_rails_memory_store_caching do
end end
end end
describe '#first_deployment_for' do
let(:project) { create(:project, :repository) }
let!(:deployment) { create(:deployment, :succeed, environment: environment, ref: commit.parent.id) }
let!(:deployment1) { create(:deployment, :succeed, environment: environment, ref: commit.id) }
let(:head_commit) { project.commit }
let(:commit) { project.commit.parent }
it 'returns deployment id for the environment', :sidekiq_might_not_need_inline do
expect(environment.first_deployment_for(commit.id)).to eq deployment1
end
it 'return nil when no deployment is found' do
expect(environment.first_deployment_for(head_commit.id)).to eq nil
end
it 'returns a UTF-8 ref', :sidekiq_might_not_need_inline do
expect(environment.first_deployment_for(commit.id).ref).to be_utf8
end
end
describe '#environment_type' do describe '#environment_type' do
subject { environment.environment_type } subject { environment.environment_type }
......
...@@ -49,7 +49,7 @@ describe Deployments::AfterCreateService do ...@@ -49,7 +49,7 @@ describe Deployments::AfterCreateService do
it 'creates ref' do it 'creates ref' do
expect_any_instance_of(Repository) expect_any_instance_of(Repository)
.to receive(:create_ref) .to receive(:create_ref)
.with(deployment.ref, deployment.send(:ref_path)) .with(deployment.sha, deployment.send(:ref_path))
service.execute service.execute
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