From e9862a9900c6269a41b65ca543035e57b49fede3 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 20 Jul 2017 20:17:42 +0800 Subject: [PATCH] Use struct instead of hash --- .../ci/create_trigger_request_service.rb | 5 ++-- lib/api/triggers.rb | 2 +- lib/api/v3/triggers.rb | 4 ++-- lib/ci/api/triggers.rb | 4 ++-- .../ci/create_trigger_request_service_spec.rb | 24 +++++++++---------- 5 files changed, 20 insertions(+), 19 deletions(-) diff --git a/app/services/ci/create_trigger_request_service.rb b/app/services/ci/create_trigger_request_service.rb index 1674830a41a..a43d0e4593c 100644 --- a/app/services/ci/create_trigger_request_service.rb +++ b/app/services/ci/create_trigger_request_service.rb @@ -1,13 +1,14 @@ module Ci module CreateTriggerRequestService + Result = Struct.new(:trigger_request, :pipeline) + def self.execute(project, trigger, ref, variables = nil) trigger_request = trigger.trigger_requests.create(variables: variables) pipeline = Ci::CreatePipelineService.new(project, trigger.owner, ref: ref) .execute(:trigger, ignore_skip_ci: true, trigger_request: trigger_request) - { trigger_request: trigger_request, - pipeline: pipeline } + Result.new(trigger_request, pipeline) end end end diff --git a/lib/api/triggers.rb b/lib/api/triggers.rb index 55528101f15..280fe72ae47 100644 --- a/lib/api/triggers.rb +++ b/lib/api/triggers.rb @@ -28,7 +28,7 @@ module API # create request and trigger builds result = Ci::CreateTriggerRequestService.execute(project, trigger, params[:ref].to_s, variables) - pipeline = result[:pipeline] + pipeline = result.pipeline if pipeline.persisted? present pipeline, with: Entities::Pipeline diff --git a/lib/api/v3/triggers.rb b/lib/api/v3/triggers.rb index 0e236423b8c..e9d4c35307b 100644 --- a/lib/api/v3/triggers.rb +++ b/lib/api/v3/triggers.rb @@ -29,10 +29,10 @@ module API # create request and trigger builds result = Ci::CreateTriggerRequestService.execute(project, trigger, params[:ref].to_s, variables) - pipeline = result[:pipeline] + pipeline = result.pipeline if pipeline.persisted? - present result[:trigger_request], with: ::API::V3::Entities::TriggerRequest + present result.trigger_request, with: ::API::V3::Entities::TriggerRequest else render_validation_error!(pipeline) end diff --git a/lib/ci/api/triggers.rb b/lib/ci/api/triggers.rb index ce0ef95b186..6225203f223 100644 --- a/lib/ci/api/triggers.rb +++ b/lib/ci/api/triggers.rb @@ -25,10 +25,10 @@ module Ci # create request and trigger builds result = Ci::CreateTriggerRequestService.execute(project, trigger, params[:ref], variables) - pipeline = result[:pipeline] + pipeline = result.pipeline if pipeline.persisted? - present result[:trigger_request], with: Entities::TriggerRequest + present result.trigger_request, with: Entities::TriggerRequest else render_validation_error!(pipeline) end diff --git a/spec/services/ci/create_trigger_request_service_spec.rb b/spec/services/ci/create_trigger_request_service_spec.rb index 48d9b0844f1..37ca9804f56 100644 --- a/spec/services/ci/create_trigger_request_service_spec.rb +++ b/spec/services/ci/create_trigger_request_service_spec.rb @@ -17,26 +17,26 @@ describe Ci::CreateTriggerRequestService, services: true do subject { service.execute(project, trigger, 'master') } context 'without owner' do - it { expect(subject[:trigger_request]).to be_kind_of(Ci::TriggerRequest) } - it { expect(subject[:trigger_request].builds.first).to be_kind_of(Ci::Build) } - it { expect(subject[:pipeline]).to be_kind_of(Ci::Pipeline) } - it { expect(subject[:pipeline]).to be_trigger } + it { expect(subject.trigger_request).to be_kind_of(Ci::TriggerRequest) } + it { expect(subject.trigger_request.builds.first).to be_kind_of(Ci::Build) } + it { expect(subject.pipeline).to be_kind_of(Ci::Pipeline) } + it { expect(subject.pipeline).to be_trigger } end context 'with owner' do - it { expect(subject[:trigger_request]).to be_kind_of(Ci::TriggerRequest) } - it { expect(subject[:trigger_request].builds.first).to be_kind_of(Ci::Build) } - it { expect(subject[:trigger_request].builds.first.user).to eq(owner) } - it { expect(subject[:pipeline]).to be_kind_of(Ci::Pipeline) } - it { expect(subject[:pipeline]).to be_trigger } - it { expect(subject[:pipeline].user).to eq(owner) } + it { expect(subject.trigger_request).to be_kind_of(Ci::TriggerRequest) } + it { expect(subject.trigger_request.builds.first).to be_kind_of(Ci::Build) } + it { expect(subject.trigger_request.builds.first.user).to eq(owner) } + it { expect(subject.pipeline).to be_kind_of(Ci::Pipeline) } + it { expect(subject.pipeline).to be_trigger } + it { expect(subject.pipeline.user).to eq(owner) } end end context 'no commit for ref' do subject { service.execute(project, trigger, 'other-branch') } - it { expect(subject[:pipeline]).not_to be_persisted } + it { expect(subject.pipeline).not_to be_persisted } end context 'no builds created' do @@ -46,7 +46,7 @@ describe Ci::CreateTriggerRequestService, services: true do stub_ci_pipeline_yaml_file('script: { only: [develop], script: hello World }') end - it { expect(subject[:pipeline]).not_to be_persisted } + it { expect(subject.pipeline).not_to be_persisted } end end end