From 91ce04678ea78f2042026e8584e5d1069853dbc4 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 5 Mar 2017 18:23:42 +0100 Subject: [PATCH] Make triggers to be user aware --- app/models/user.rb | 1 + .../ci/create_trigger_request_service.rb | 2 +- .../20170215164610_add_owner_id_to_triggers.rb | 2 +- spec/models/ci/trigger_spec.rb | 14 +++++++++++--- spec/models/user_spec.rb | 1 + .../ci/create_trigger_request_service_spec.rb | 18 ++++++++++++++++-- 6 files changed, 31 insertions(+), 7 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index d3bb04060bf..dfba51d3b00 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -95,6 +95,7 @@ class User < ActiveRecord::Base has_many :todos, dependent: :destroy has_many :notification_settings, dependent: :destroy has_many :award_emoji, dependent: :destroy + has_many :triggers, dependent: :destroy, class_name: 'Ci::Trigger', foreign_key: :owner_id has_many :assigned_issues, dependent: :nullify, foreign_key: :assignee_id, class_name: "Issue" has_many :assigned_merge_requests, dependent: :nullify, foreign_key: :assignee_id, class_name: "MergeRequest" diff --git a/app/services/ci/create_trigger_request_service.rb b/app/services/ci/create_trigger_request_service.rb index 6af3c1ca5b1..dca5aa9f5d7 100644 --- a/app/services/ci/create_trigger_request_service.rb +++ b/app/services/ci/create_trigger_request_service.rb @@ -3,7 +3,7 @@ module Ci def execute(project, trigger, ref, variables = nil) trigger_request = trigger.trigger_requests.create(variables: variables) - pipeline = Ci::CreatePipelineService.new(project, nil, ref: ref). + pipeline = Ci::CreatePipelineService.new(project, trigger.owner, ref: ref). execute(ignore_skip_ci: true, trigger_request: trigger_request) if pipeline.persisted? trigger_request diff --git a/db/migrate/20170215164610_add_owner_id_to_triggers.rb b/db/migrate/20170215164610_add_owner_id_to_triggers.rb index 02c77ee4b5e..845c89c703e 100644 --- a/db/migrate/20170215164610_add_owner_id_to_triggers.rb +++ b/db/migrate/20170215164610_add_owner_id_to_triggers.rb @@ -5,6 +5,6 @@ class AddOwnerIdToTriggers < ActiveRecord::Migration def change add_column :ci_triggers, :owner_id, :integer - add_foreign_key :ci_triggers, :users, column: :owner_id, on_delete: :nullify + add_foreign_key :ci_triggers, :users, column: :owner_id, on_delete: :cascade end end diff --git a/spec/models/ci/trigger_spec.rb b/spec/models/ci/trigger_spec.rb index 3ca9231f58e..074cf1a0bd7 100644 --- a/spec/models/ci/trigger_spec.rb +++ b/spec/models/ci/trigger_spec.rb @@ -1,16 +1,24 @@ require 'spec_helper' describe Ci::Trigger, models: true do - let(:project) { FactoryGirl.create :empty_project } + let(:project) { create :empty_project } + + describe 'associations' do + it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:owner) } + it { is_expected.to have_many(:trigger_requests) } + end describe 'before_validation' do it 'sets an random token if none provided' do - trigger = FactoryGirl.create :ci_trigger_without_token, project: project + trigger = create(:ci_trigger_without_token, project: project) + expect(trigger.token).not_to be_nil end it 'does not set an random token if one provided' do - trigger = FactoryGirl.create :ci_trigger, project: project + trigger = create(:ci_trigger, project: project) + expect(trigger.token).to eq('token') end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e86b4a761d9..b99cde64675 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -32,6 +32,7 @@ describe User, models: true do it { is_expected.to have_many(:spam_logs).dependent(:destroy) } it { is_expected.to have_many(:todos).dependent(:destroy) } it { is_expected.to have_many(:award_emoji).dependent(:destroy) } + it { is_expected.to have_many(:triggers).dependent(:destroy) } it { is_expected.to have_many(:builds).dependent(:nullify) } it { is_expected.to have_many(:pipelines).dependent(:nullify) } it { is_expected.to have_many(:chat_names).dependent(:destroy) } diff --git a/spec/services/ci/create_trigger_request_service_spec.rb b/spec/services/ci/create_trigger_request_service_spec.rb index d8c443d29d5..5e68343784d 100644 --- a/spec/services/ci/create_trigger_request_service_spec.rb +++ b/spec/services/ci/create_trigger_request_service_spec.rb @@ -13,8 +13,22 @@ describe Ci::CreateTriggerRequestService, services: true do context 'valid params' do subject { service.execute(project, trigger, 'master') } - it { expect(subject).to be_kind_of(Ci::TriggerRequest) } - it { expect(subject.builds.first).to be_kind_of(Ci::Build) } + context 'without owner' do + it { expect(subject).to be_kind_of(Ci::TriggerRequest) } + it { expect(subject.pipeline).to be_kind_of(Ci::Pipeline) } + it { expect(subject.builds.first).to be_kind_of(Ci::Build) } + end + + context 'with owner' do + let(:owner) { create(:user) } + let(:trigger) { create(:ci_trigger, project: project, owner: owner) } + + it { expect(subject).to be_kind_of(Ci::TriggerRequest) } + it { expect(subject.pipeline).to be_kind_of(Ci::Pipeline) } + it { expect(subject.pipeline.user).to eq(owner) } + it { expect(subject.builds.first).to be_kind_of(Ci::Build) } + it { expect(subject.builds.first.user).to eq(owner) } + end end context 'no commit for ref' do