diff options
24 files changed, 175 insertions, 43 deletions
diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index cb4bd0ad5f5..603a51266da 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -80,10 +80,6 @@ class Projects::ApplicationController < ApplicationController cookies.permanent[:diff_view] = params.delete(:view) if params[:view].present? end - def builds_enabled - return render_404 unless @project.feature_available?(:builds, current_user) - end - def require_pages_enabled! not_found unless Gitlab.config.pages.enabled end diff --git a/app/controllers/projects/graphs_controller.rb b/app/controllers/projects/graphs_controller.rb index 43fc0c39801..df5221fe95f 100644 --- a/app/controllers/projects/graphs_controller.rb +++ b/app/controllers/projects/graphs_controller.rb @@ -5,7 +5,6 @@ class Projects::GraphsController < Projects::ApplicationController before_action :require_non_empty_project before_action :assign_ref_vars before_action :authorize_download_code! - before_action :builds_enabled, only: :ci def show respond_to do |format| diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 6223e7943f8..8effb792689 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -4,7 +4,6 @@ class Projects::PipelinesController < Projects::ApplicationController before_action :authorize_read_pipeline! before_action :authorize_create_pipeline!, only: [:new, :create] before_action :authorize_update_pipeline!, only: [:retry, :cancel] - before_action :builds_enabled, only: :charts wrap_parameters Ci::Pipeline diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 7441b58fddb..c11dd49f4a7 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -218,6 +218,10 @@ module ProjectsHelper nav_tabs << :container_registry end + if project.builds_enabled? && can?(current_user, :read_pipeline, project) + nav_tabs << :pipelines + end + tab_ability_map.each do |tab, ability| if can?(current_user, ability, project) nav_tabs << tab @@ -231,7 +235,6 @@ module ProjectsHelper { environments: :read_environment, milestones: :read_milestone, - pipelines: :read_pipeline, snippets: :read_project_snippet, settings: :admin_project, builds: :read_build, diff --git a/app/models/generic_commit_status.rb b/app/models/generic_commit_status.rb index 8867ba0d2ff..532b8f4ad69 100644 --- a/app/models/generic_commit_status.rb +++ b/app/models/generic_commit_status.rb @@ -11,6 +11,7 @@ class GenericCommitStatus < CommitStatus def set_default_values self.context ||= 'default' self.stage ||= 'external' + self.stage_idx ||= 1000000 end def tags diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 3959b895f44..47518dddb61 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -203,7 +203,7 @@ class ProjectPolicy < BasePolicy unless project.feature_available?(:builds, user) && repository_enabled cannot!(*named_abilities(:build)) - cannot!(*named_abilities(:pipeline)) + cannot!(*named_abilities(:pipeline) - [:read_pipeline]) cannot!(*named_abilities(:pipeline_schedule)) cannot!(*named_abilities(:environment)) cannot!(*named_abilities(:deployment)) diff --git a/app/serializers/build_details_entity.rb b/app/serializers/build_details_entity.rb index 0eddbaaaebf..cb6d3dfec89 100644 --- a/app/serializers/build_details_entity.rb +++ b/app/serializers/build_details_entity.rb @@ -1,4 +1,4 @@ -class BuildDetailsEntity < BuildEntity +class BuildDetailsEntity < JobEntity expose :coverage, :erased_at, :duration expose :tag_list, as: :tags expose :user, using: UserEntity diff --git a/app/serializers/build_serializer.rb b/app/serializers/build_serializer.rb index 79b67001199..bae9932847f 100644 --- a/app/serializers/build_serializer.rb +++ b/app/serializers/build_serializer.rb @@ -1,5 +1,5 @@ class BuildSerializer < BaseSerializer - entity BuildEntity + entity JobEntity def represent_status(resource) data = represent(resource, { only: [:status] }) diff --git a/app/serializers/deployment_entity.rb b/app/serializers/deployment_entity.rb index 8b3de1bed0f..e493c9162fd 100644 --- a/app/serializers/deployment_entity.rb +++ b/app/serializers/deployment_entity.rb @@ -24,6 +24,6 @@ class DeploymentEntity < Grape::Entity expose :user, using: UserEntity expose :commit, using: CommitEntity - expose :deployable, using: BuildEntity - expose :manual_actions, using: BuildEntity + expose :deployable, using: JobEntity + expose :manual_actions, using: JobEntity end diff --git a/app/serializers/build_entity.rb b/app/serializers/job_entity.rb index 67001f4547d..d6de43bcbcb 100644 --- a/app/serializers/build_entity.rb +++ b/app/serializers/job_entity.rb @@ -1,11 +1,11 @@ -class BuildEntity < Grape::Entity +class JobEntity < Grape::Entity include RequestAwareEntity expose :id expose :name expose :build_path do |build| - path_to(:namespace_project_job, build) + build.target_url || path_to(:namespace_project_job, build) end expose :retry_path, if: -> (*) { retryable? } do |build| diff --git a/app/serializers/job_group_entity.rb b/app/serializers/job_group_entity.rb index 04487e59009..8554de55517 100644 --- a/app/serializers/job_group_entity.rb +++ b/app/serializers/job_group_entity.rb @@ -4,7 +4,7 @@ class JobGroupEntity < Grape::Entity expose :name expose :size expose :detailed_status, as: :status, with: StatusEntity - expose :jobs, with: BuildEntity + expose :jobs, with: JobEntity private diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index f080e6326a1..fb1d4aed58b 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -101,12 +101,12 @@ class GitPushService < BaseService UpdateMergeRequestsWorker .perform_async(@project.id, current_user.id, params[:oldrev], params[:newrev], params[:ref]) - SystemHookPushWorker.perform_async(build_push_data.dup, :push_hooks) - EventCreateService.new.push(@project, current_user, build_push_data) + Ci::CreatePipelineService.new(@project, current_user, build_push_data).execute(:push) + + SystemHookPushWorker.perform_async(build_push_data.dup, :push_hooks) @project.execute_hooks(build_push_data.dup, :push_hooks) @project.execute_services(build_push_data.dup, :push_hooks) - Ci::CreatePipelineService.new(@project, current_user, build_push_data).execute(:push) if push_remove_branch? AfterBranchDeleteService diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb index 7c424fba428..9917a39b795 100644 --- a/app/services/git_tag_push_service.rb +++ b/app/services/git_tag_push_service.rb @@ -8,10 +8,12 @@ class GitTagPushService < BaseService @push_data = build_push_data EventCreateService.new.push(project, current_user, @push_data) + Ci::CreatePipelineService.new(project, current_user, @push_data).execute(:push) + SystemHooksService.new.execute_hooks(build_system_push_data.dup, :tag_push_hooks) project.execute_hooks(@push_data.dup, :tag_push_hooks) project.execute_services(@push_data.dup, :tag_push_hooks) - Ci::CreatePipelineService.new(project, current_user, @push_data).execute(:push) + ProjectCacheWorker.perform_async(project.id, [], [:commit_count, :repository_size]) true diff --git a/changelogs/unreleased/fix-support-for-external-ci-services.yml b/changelogs/unreleased/fix-support-for-external-ci-services.yml new file mode 100644 index 00000000000..eecb4519259 --- /dev/null +++ b/changelogs/unreleased/fix-support-for-external-ci-services.yml @@ -0,0 +1,4 @@ +--- +title: Fix support for external CI services +merge_request: 11176 +author: diff --git a/lib/gitlab/ci/status/external/common.rb b/lib/gitlab/ci/status/external/common.rb index 4969a350862..9307545b5b1 100644 --- a/lib/gitlab/ci/status/external/common.rb +++ b/lib/gitlab/ci/status/external/common.rb @@ -3,6 +3,10 @@ module Gitlab module Status module External module Common + def label + subject.description + end + def has_details? subject.target_url.present? && can?(user, :read_commit_status, subject) diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 954f89e3854..734532668d3 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -5,9 +5,12 @@ describe Projects::PipelinesController do let(:user) { create(:user) } let(:project) { create(:empty_project, :public) } + let(:feature) { ProjectFeature::DISABLED } before do project.add_developer(user) + project.project_feature.update( + builds_access_level: feature) sign_in(user) end @@ -153,16 +156,26 @@ describe Projects::PipelinesController do format: :json end - it 'retries a pipeline without returning any content' do - expect(response).to have_http_status(:no_content) - expect(build.reload).to be_retried + context 'when builds are enabled' do + let(:feature) { ProjectFeature::ENABLED } + + it 'retries a pipeline without returning any content' do + expect(response).to have_http_status(:no_content) + expect(build.reload).to be_retried + end + end + + context 'when builds are disabled' do + it 'fails to retry pipeline' do + expect(response).to have_http_status(:not_found) + end end end describe 'POST cancel.json' do let!(:pipeline) { create(:ci_pipeline, project: project) } let!(:build) { create(:ci_build, :running, pipeline: pipeline) } - + before do post :cancel, namespace_id: project.namespace, project_id: project, @@ -170,9 +183,19 @@ describe Projects::PipelinesController do format: :json end - it 'cancels a pipeline without returning any content' do - expect(response).to have_http_status(:no_content) - expect(pipeline.reload).to be_canceled + context 'when builds are enabled' do + let(:feature) { ProjectFeature::ENABLED } + + it 'cancels a pipeline without returning any content' do + expect(response).to have_http_status(:no_content) + expect(pipeline.reload).to be_canceled + end + end + + context 'when builds are disabled' do + it 'fails to retry pipeline' do + expect(response).to have_http_status(:not_found) + end end end end diff --git a/spec/features/projects/features_visibility_spec.rb b/spec/features/projects/features_visibility_spec.rb index c49648f54bd..d76b5e4ef1b 100644 --- a/spec/features/projects/features_visibility_spec.rb +++ b/spec/features/projects/features_visibility_spec.rb @@ -68,9 +68,12 @@ describe 'Edit Project Settings', feature: true do end describe 'project features visibility pages' do + let(:pipeline) { create(:ci_empty_pipeline, project: project) } + let(:job) { create(:ci_build, pipeline: pipeline) } + let(:tools) do { - builds: namespace_project_pipelines_path(project.namespace, project), + builds: namespace_project_job_path(project.namespace, project, job), issues: namespace_project_issues_path(project.namespace, project), wiki: namespace_project_wiki_path(project.namespace, project, :home), snippets: namespace_project_snippets_path(project.namespace, project), diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index a695621b87a..3ed0b0a756b 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -300,4 +300,37 @@ describe ProjectsHelper do expect(helper.send(:visibility_select_options, project, Gitlab::VisibilityLevel::PRIVATE)).to include('Private') end end + + describe '#get_project_nav_tabs' do + let(:project) { create(:empty_project) } + let(:user) { create(:user) } + + before do + allow(helper).to receive(:can?) { true } + end + + subject do + helper.send(:get_project_nav_tabs, project, user) + end + + context 'when builds feature is enabled' do + before do + allow(project).to receive(:builds_enabled?).and_return(true) + end + + it "does include pipelines tab" do + is_expected.to include(:pipelines) + end + end + + context 'when builds feature is disabled' do + before do + allow(project).to receive(:builds_enabled?).and_return(false) + end + + it "do not include pipelines tab" do + is_expected.not_to include(:pipelines) + end + end + end end diff --git a/spec/lib/gitlab/ci/status/external/common_spec.rb b/spec/lib/gitlab/ci/status/external/common_spec.rb index 5a97d98b55f..e58f5d8d4df 100644 --- a/spec/lib/gitlab/ci/status/external/common_spec.rb +++ b/spec/lib/gitlab/ci/status/external/common_spec.rb @@ -4,9 +4,10 @@ describe Gitlab::Ci::Status::External::Common do let(:user) { create(:user) } let(:project) { external_status.project } let(:external_target_url) { 'http://example.gitlab.com/status' } + let(:external_description) { 'my description' } let(:external_status) do - create(:generic_commit_status, target_url: external_target_url) + create(:generic_commit_status, target_url: external_target_url, description: external_description) end subject do @@ -15,6 +16,12 @@ describe Gitlab::Ci::Status::External::Common do .extend(described_class) end + describe '#label' do + it 'returns description' do + expect(subject.label).to eq external_description + end + end + describe '#has_action?' do it { is_expected.not_to have_action } end diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 0d3af1f4499..848fd547e10 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -139,6 +139,18 @@ describe ProjectPolicy, models: true do is_expected.not_to include(:read_build, :read_pipeline) end end + + context 'when builds are disabled' do + before do + project.project_feature.update( + builds_access_level: ProjectFeature::DISABLED) + end + + it do + is_expected.not_to include(:read_build) + is_expected.to include(:read_pipeline) + end + end end context 'reporter' do diff --git a/spec/serializers/build_details_entity_spec.rb b/spec/serializers/build_details_entity_spec.rb index 396ba96e9b3..b92c1c28ba8 100644 --- a/spec/serializers/build_details_entity_spec.rb +++ b/spec/serializers/build_details_entity_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' describe BuildDetailsEntity do set(:user) { create(:admin) } - it 'inherits from BuildEntity' do - expect(described_class).to be < BuildEntity + it 'inherits from JobEntity' do + expect(described_class).to be < JobEntity end describe '#as_json' do diff --git a/spec/serializers/build_entity_spec.rb b/spec/serializers/job_entity_spec.rb index e51ff9fc709..5ca7bf2fcaf 100644 --- a/spec/serializers/build_entity_spec.rb +++ b/spec/serializers/job_entity_spec.rb @@ -1,9 +1,9 @@ require 'spec_helper' -describe BuildEntity do +describe JobEntity do let(:user) { create(:user) } - let(:build) { create(:ci_build) } - let(:project) { build.project } + let(:job) { create(:ci_build) } + let(:project) { job.project } let(:request) { double('request') } before do @@ -12,12 +12,12 @@ describe BuildEntity do end let(:entity) do - described_class.new(build, request: request) + described_class.new(job, request: request) end subject { entity.as_json } - it 'contains paths to build page action' do + it 'contains paths to job page action' do expect(subject).to include(:build_path) end @@ -27,7 +27,7 @@ describe BuildEntity do end it 'contains whether it is playable' do - expect(subject[:playable]).to eq build.playable? + expect(subject[:playable]).to eq job.playable? end it 'contains timestamps' do @@ -39,9 +39,9 @@ describe BuildEntity do expect(subject[:status]).to include :icon, :favicon, :text, :label end - context 'when build is retryable' do + context 'when job is retryable' do before do - build.update(status: :failed) + job.update(status: :failed) end it 'contains cancel path' do @@ -49,9 +49,9 @@ describe BuildEntity do end end - context 'when build is cancelable' do + context 'when job is cancelable' do before do - build.update(status: :running) + job.update(status: :running) end it 'contains cancel path' do @@ -59,7 +59,7 @@ describe BuildEntity do end end - context 'when build is a regular build' do + context 'when job is a regular job' do it 'does not contain path to play action' do expect(subject).not_to include(:play_path) end @@ -69,8 +69,8 @@ describe BuildEntity do end end - context 'when build is a manual action' do - let(:build) { create(:ci_build, :manual) } + context 'when job is a manual action' do + let(:job) { create(:ci_build, :manual) } context 'when user is allowed to trigger action' do before do @@ -99,4 +99,25 @@ describe BuildEntity do end end end + + context 'when job is generic commit status' do + let(:job) { create(:generic_commit_status, target_url: 'http://google.com') } + + it 'contains paths to target action' do + expect(subject).to include(:build_path) + end + + it 'does not contain paths to other action paths' do + expect(subject).not_to include(:retry_path, :cancel_path, :play_path) + end + + it 'contains timestamps' do + expect(subject).to include(:created_at, :updated_at) + end + + it 'contains details' do + expect(subject).to include :status + expect(subject[:status]).to include :icon, :favicon, :text, :label + end + end end diff --git a/spec/serializers/pipeline_details_entity_spec.rb b/spec/serializers/pipeline_details_entity_spec.rb index 03cc5ae9b63..5cb9b9945b6 100644 --- a/spec/serializers/pipeline_details_entity_spec.rb +++ b/spec/serializers/pipeline_details_entity_spec.rb @@ -91,6 +91,20 @@ describe PipelineDetailsEntity do end end + context 'when pipeline has commit statuses' do + let(:pipeline) { create(:ci_empty_pipeline) } + + before do + create(:generic_commit_status, pipeline: pipeline) + end + + it 'contains stages' do + expect(subject).to include(:details) + expect(subject[:details]).to include(:stages) + expect(subject[:details][:stages].first).to include(name: 'external') + end + end + context 'when pipeline has YAML errors' do let(:pipeline) do create(:ci_pipeline, config: { rspec: { invalid: :value } }) diff --git a/spec/serializers/stage_entity_spec.rb b/spec/serializers/stage_entity_spec.rb index 64b3217b809..40e303f7b89 100644 --- a/spec/serializers/stage_entity_spec.rb +++ b/spec/serializers/stage_entity_spec.rb @@ -54,6 +54,17 @@ describe StageEntity do it 'exposes the group key' do expect(subject).to include :groups end + + context 'and contains commit status' do + before do + create(:generic_commit_status, pipeline: pipeline, stage: 'test') + end + + it 'contains commit status' do + groups = subject[:groups].map { |group| group[:name] } + expect(groups).to include('generic') + end + end end end end |