diff options
| -rw-r--r-- | app/models/commit_status.rb | 4 | ||||
| -rw-r--r-- | app/services/ci/create_job_service.rb | 12 | ||||
| -rw-r--r-- | app/services/projects/update_pages_service.rb | 18 | ||||
| -rw-r--r-- | db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb | 19 | ||||
| -rw-r--r-- | db/post_migrate/20180125111139_add_unique_pipeline_stage_name_index.rb | 17 | ||||
| -rw-r--r-- | lib/api/commit_statuses.rb | 10 | ||||
| -rw-r--r-- | spec/services/ci/create_job_service_spec.rb | 26 |
7 files changed, 25 insertions, 81 deletions
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 47af20975ec..c0263c0b4e2 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -50,9 +50,7 @@ class CommitStatus < ActiveRecord::Base ## # We still create some CommitStatuses outside of CreatePipelineService. # - # These are pages deployments and external statuses. We now handle these - # jobs using CreateJobService, but we still need to ensure that a job has - # a stage assigned. TODO, In future releases we will add model validation. + # These are pages deployments and external statuses. # before_create unless: :importing? do Ci::EnsureStageService.new(project, user).execute(self) do |stage| diff --git a/app/services/ci/create_job_service.rb b/app/services/ci/create_job_service.rb deleted file mode 100644 index 7cb77edd690..00000000000 --- a/app/services/ci/create_job_service.rb +++ /dev/null @@ -1,12 +0,0 @@ -module Ci - class CreateJobService < BaseService - def execute(subject = nil) - (subject || yield).tap do |subject| - Ci::EnsureStageService.new(project, current_user) - .execute(subject) - - subject.save - end - end - end -end diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb index 63d2b6c76e6..a773222bf17 100644 --- a/app/services/projects/update_pages_service.rb +++ b/app/services/projects/update_pages_service.rb @@ -58,16 +58,14 @@ module Projects end def create_status - Ci::CreateJobService.new(project, build.user).execute do - GenericCommitStatus.new( - project: project, - pipeline: build.pipeline, - user: build.user, - ref: build.ref, - stage: 'deploy', - name: 'pages:deploy' - ) - end + GenericCommitStatus.new( + project: project, + pipeline: build.pipeline, + user: build.user, + ref: build.ref, + stage: 'deploy', + name: 'pages:deploy' + ) end def extract_archive!(temp_path) diff --git a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb index 89ed422ea1c..be5a80261c0 100644 --- a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb +++ b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb @@ -4,6 +4,21 @@ class RemoveRedundantPipelineStages < ActiveRecord::Migration DOWNTIME = false def up + remove_concurrent_index :ci_stages, [:pipeline_id, :name] + + remove_redundant_pipeline_stages! + + add_concurrent_index :ci_stages, [:pipeline_id, :name], unique: true + end + + def down + remove_concurrent_index :ci_stages, [:pipeline_id, :name], unique: true + add_concurrent_index :ci_stages, [:pipeline_id, :name] + end + + private + + def remove_redundant_pipeline_stages! redundant_stages_ids = <<~SQL SELECT id FROM ci_stages WHERE (pipeline_id, name) IN ( SELECT pipeline_id, name FROM ci_stages @@ -27,8 +42,4 @@ class RemoveRedundantPipelineStages < ActiveRecord::Migration SQL end end - - def down - # noop - end end diff --git a/db/post_migrate/20180125111139_add_unique_pipeline_stage_name_index.rb b/db/post_migrate/20180125111139_add_unique_pipeline_stage_name_index.rb deleted file mode 100644 index 4fa148b2446..00000000000 --- a/db/post_migrate/20180125111139_add_unique_pipeline_stage_name_index.rb +++ /dev/null @@ -1,17 +0,0 @@ -class AddUniquePipelineStageNameIndex < ActiveRecord::Migration - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - - disable_ddl_transaction! - - def up - remove_concurrent_index :ci_stages, [:pipeline_id, :name] - add_concurrent_index :ci_stages, [:pipeline_id, :name], unique: true - end - - def down - remove_concurrent_index :ci_stages, [:pipeline_id, :name], unique: true - add_concurrent_index :ci_stages, [:pipeline_id, :name] - end -end diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb index 3ec17c4d9f5..829eef18795 100644 --- a/lib/api/commit_statuses.rb +++ b/lib/api/commit_statuses.rb @@ -69,7 +69,6 @@ module API name = params[:name] || params[:context] || 'default' pipeline = @project.pipeline_for(ref, commit.sha) - unless pipeline pipeline = @project.pipelines.create!( source: :external, @@ -91,14 +90,7 @@ module API optional_attributes = attributes_for_keys(%w[target_url description coverage]) - status.assign_attributes(optional_attributes) if optional_attributes.any? - - if status.new_record? - Ci::CreateJobService.new(@project, current_user).execute(status) - else - status.save if status.changed? - end - + status.update(optional_attributes) if optional_attributes.any? render_validation_error!(status) if status.invalid? begin diff --git a/spec/services/ci/create_job_service_spec.rb b/spec/services/ci/create_job_service_spec.rb deleted file mode 100644 index d9dd1a40325..00000000000 --- a/spec/services/ci/create_job_service_spec.rb +++ /dev/null @@ -1,26 +0,0 @@ -require 'spec_helper' - -describe Ci::CreateJobService, '#execute' do - set(:project) { create(:project, :repository) } - let(:user) { create(:admin) } - let(:status) { build(:ci_build) } - let(:service) { described_class.new(project, user) } - - it 'persists job object instantiated in the block' do - expect(service.execute { status }).to be_persisted - end - - it 'persists a job instance passed as an argument' do - expect(service.execute(status)).to be_persisted - end - - it 'ensures that a job has a stage assigned' do - expect(service.execute(status).stage_id).to be_present - end - - it 'does not raise error if status is invalid' do - status.name = nil - - expect(service.execute(status)).not_to be_persisted - end -end |