Commit 6776fac6 authored by Kamil Trzciński's avatar Kamil Trzciński

Respect permissions when showing Failed Jobs

parent 927a9b13
...@@ -87,7 +87,7 @@ class Projects::PipelinesController < Projects::ApplicationController ...@@ -87,7 +87,7 @@ class Projects::PipelinesController < Projects::ApplicationController
end end
def failures def failures
if @pipeline.statuses.latest.failed.present? if @pipeline.failed_builds.present?
render_show render_show
else else
redirect_to pipeline_path(@pipeline) redirect_to pipeline_path(@pipeline)
......
module Ci module Ci
class PipelinePresenter < Gitlab::View::Presenter::Delegated class PipelinePresenter < Gitlab::View::Presenter::Delegated
include Gitlab::Utils::StrongMemoize
FAILURE_REASONS = { FAILURE_REASONS = {
config_error: 'CI/CD YAML configuration error!' config_error: 'CI/CD YAML configuration error!'
}.freeze }.freeze
presents :pipeline presents :pipeline
def failed_builds
return [] unless can?(current_user, :read_build, pipeline)
strong_memoize(:failed_builds) do
pipeline.builds.latest.failed
end
end
def failure_reason def failure_reason
return unless pipeline.failure_reason? return unless pipeline.failure_reason?
......
- failed_builds = @pipeline.statuses.latest.failed
.tabs-holder .tabs-holder
%ul.pipelines-tabs.nav-links.no-top.no-bottom.mobile-separator %ul.pipelines-tabs.nav-links.no-top.no-bottom.mobile-separator
%li.js-pipeline-tab-link %li.js-pipeline-tab-link
...@@ -9,11 +7,11 @@ ...@@ -9,11 +7,11 @@
= link_to builds_project_pipeline_path(@project, @pipeline), data: { target: '#js-tab-builds', action: 'builds', toggle: 'tab' }, class: 'builds-tab' do = link_to builds_project_pipeline_path(@project, @pipeline), data: { target: '#js-tab-builds', action: 'builds', toggle: 'tab' }, class: 'builds-tab' do
= _("Jobs") = _("Jobs")
%span.badge.js-builds-counter= pipeline.total_size %span.badge.js-builds-counter= pipeline.total_size
- if failed_builds.present? - if @pipeline.failed_builds.present?
%li.js-failures-tab-link %li.js-failures-tab-link
= link_to failures_project_pipeline_path(@project, @pipeline), data: { target: '#js-tab-failures', action: 'failures', toggle: 'tab' }, class: 'failures-tab' do = link_to failures_project_pipeline_path(@project, @pipeline), data: { target: '#js-tab-failures', action: 'failures', toggle: 'tab' }, class: 'failures-tab' do
= _("Failed Jobs") = _("Failed Jobs")
%span.badge.js-failures-counter= failed_builds.count %span.badge.js-failures-counter= @pipeline.failed_builds.count
.tab-content .tab-content
#js-tab-pipeline.tab-pane #js-tab-pipeline.tab-pane
...@@ -43,9 +41,10 @@ ...@@ -43,9 +41,10 @@
%th Coverage %th Coverage
%th %th
= render partial: "projects/stage/stage", collection: pipeline.legacy_stages, as: :stage = render partial: "projects/stage/stage", collection: pipeline.legacy_stages, as: :stage
- if failed_builds.present?
- if @pipeline.failed_builds.present?
#js-tab-failures.build-failures.tab-pane #js-tab-failures.build-failures.tab-pane
- failed_builds.each_with_index do |build, index| - @pipeline.failed_builds.each_with_index do |build, index|
.build-state .build-state
%span.ci-status-icon-failed= custom_icon('icon_status_failed') %span.ci-status-icon-failed= custom_icon('icon_status_failed')
%span.stage %span.stage
......
...@@ -3,10 +3,11 @@ require 'spec_helper' ...@@ -3,10 +3,11 @@ require 'spec_helper'
describe 'Pipeline', :js do describe 'Pipeline', :js do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:role) { :developer }
before do before do
sign_in(user) sign_in(user)
project.add_developer(user) project.add_role(user, role)
end end
shared_context 'pipeline builds' do shared_context 'pipeline builds' do
...@@ -153,9 +154,10 @@ describe 'Pipeline', :js do ...@@ -153,9 +154,10 @@ describe 'Pipeline', :js do
end end
context 'page tabs' do context 'page tabs' do
it 'shows Pipeline and Jobs tabs with link' do it 'shows Pipeline, Jobs and Failed Jobs tabs with link' do
expect(page).to have_link('Pipeline') expect(page).to have_link('Pipeline')
expect(page).to have_link('Jobs') expect(page).to have_link('Jobs')
expect(page).to have_link('Failed Jobs')
end end
it 'shows counter in Jobs tab' do it 'shows counter in Jobs tab' do
...@@ -165,6 +167,16 @@ describe 'Pipeline', :js do ...@@ -165,6 +167,16 @@ describe 'Pipeline', :js do
it 'shows Pipeline tab as active' do it 'shows Pipeline tab as active' do
expect(page).to have_css('.js-pipeline-tab-link.active') expect(page).to have_css('.js-pipeline-tab-link.active')
end end
context 'without permission to access builds' do
let(:project) { create(:project, :public, :repository, public_builds: false) }
let(:role) { :guest }
it 'does not show failed jobs tab pane' do
expect(page).to have_link('Pipeline')
expect(page).not_to have_content('Failed Jobs')
end
end
end end
context 'retrying jobs' do context 'retrying jobs' do
...@@ -308,8 +320,7 @@ describe 'Pipeline', :js do ...@@ -308,8 +320,7 @@ describe 'Pipeline', :js do
end end
describe 'GET /:project/pipelines/:id/failures' do describe 'GET /:project/pipelines/:id/failures' do
let(:project) { create(:project, :repository) } let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master', sha: '1234') }
let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master', sha: project.commit.id) }
let(:pipeline_failures_page) { failures_project_pipeline_path(project, pipeline) } let(:pipeline_failures_page) { failures_project_pipeline_path(project, pipeline) }
let!(:failed_build) { create(:ci_build, :failed, pipeline: pipeline) } let!(:failed_build) { create(:ci_build, :failed, pipeline: pipeline) }
...@@ -340,11 +351,39 @@ describe 'Pipeline', :js do ...@@ -340,11 +351,39 @@ describe 'Pipeline', :js do
visit pipeline_failures_page visit pipeline_failures_page
end end
it 'includes failed jobs' do it 'shows jobs tab pane as active' do
expect(page).to have_content('Failed Jobs')
expect(page).to have_css('#js-tab-failures.active')
end
it 'lists failed builds' do
expect(page).to have_content(failed_build.name)
expect(page).to have_content(failed_build.stage)
end
it 'does not show trace' do
expect(page).to have_content('No job trace') expect(page).to have_content('No job trace')
end end
end end
context 'without permission to access builds' do
let(:role) { :guest }
before do
project.update(public_builds: false)
end
context 'when accessing failed jobs page' do
before do
visit pipeline_failures_page
end
it 'fails to access the page' do
expect(page).to have_content('Access Denied')
end
end
end
context 'without failures' do context 'without failures' do
before do before do
failed_build.update!(status: :success) failed_build.update!(status: :success)
......
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