Commit ee97aba0 authored by Stan Hu's avatar Stan Hu

Merge branch 'eb-no-keep-report-artifacts-take-2' into 'master'

Don't show keep path for non archive artifacts

See merge request gitlab-org/gitlab!23762
parents 37e32369 63676473
......@@ -763,8 +763,8 @@ module Ci
end
end
def has_expiring_artifacts?
artifacts_expire_at.present? && artifacts_expire_at > Time.now
def has_expiring_archive_artifacts?
has_expiring_artifacts? && job_artifacts_archive.present?
end
def keep_artifacts!
......@@ -979,6 +979,10 @@ module Ci
value.with_indifferent_access
end
end
def has_expiring_artifacts?
artifacts_expire_at.present? && artifacts_expire_at > Time.now
end
end
end
......
......@@ -15,7 +15,7 @@ class BuildArtifactEntity < Grape::Entity
fast_download_project_job_artifacts_path(project, job)
end
expose :keep_path, if: -> (*) { job.has_expiring_artifacts? } do |job|
expose :keep_path, if: -> (*) { job.has_expiring_archive_artifacts? } do |job|
fast_keep_project_job_artifacts_path(project, job)
end
......
......@@ -31,7 +31,7 @@ class BuildDetailsEntity < JobEntity
browse_project_job_artifacts_path(project, build)
end
expose :keep_path, if: -> (*) { build.has_expiring_artifacts? && can?(current_user, :update_build, build) } do |build|
expose :keep_path, if: -> (*) { build.has_expiring_archive_artifacts? && can?(current_user, :update_build, build) } do |build|
keep_project_job_artifacts_path(project, build)
end
......
......@@ -58,7 +58,8 @@ class PipelineSerializer < BaseSerializer
pending_builds: :project,
project: [:route, { namespace: :route }],
artifacts: {
project: [:route, { namespace: :route }]
project: [:route, { namespace: :route }],
job_artifacts_archive: []
}
},
{ triggered_by_pipeline: [:project, :user] },
......
---
title: Remove keep button for non archive artifacts
merge_request: 23762
author:
type: fixed
......@@ -2338,14 +2338,24 @@ describe Ci::Build do
end
end
describe '#has_expiring_artifacts?' do
describe '#has_expiring_archive_artifacts?' do
context 'when artifacts have expiration date set' do
before do
build.update(artifacts_expire_at: 1.day.from_now)
end
context 'and job artifacts archive record exists' do
let!(:archive) { create(:ci_job_artifact, :archive, job: build) }
it 'has expiring artifacts' do
expect(build).to have_expiring_artifacts
expect(build).to have_expiring_archive_artifacts
end
end
context 'and job artifacts archive record does not exist' do
it 'does not have expiring artifacts' do
expect(build).not_to have_expiring_archive_artifacts
end
end
end
......@@ -2355,7 +2365,7 @@ describe Ci::Build do
end
it 'does not have expiring artifacts' do
expect(build).not_to have_expiring_artifacts
expect(build).not_to have_expiring_archive_artifacts
end
end
end
......
......@@ -3,7 +3,7 @@
require 'spec_helper'
describe BuildArtifactEntity do
let(:job) { create(:ci_build, name: 'test:job', artifacts_expire_at: 1.hour.from_now) }
let(:job) { create(:ci_build, :artifacts, name: 'test:job', artifacts_expire_at: 1.hour.from_now) }
let(:entity) do
described_class.new(job, request: double)
......
......@@ -176,5 +176,22 @@ describe BuildDetailsEntity do
expect(subject[:reports].first[:file_type]).to eq('codequality')
end
end
context 'when the build has no archive type artifacts' do
let!(:report) { create(:ci_job_artifact, :codequality, job: build) }
it 'does not expose any artifact actions path' do
expect(subject[:artifact].keys).not_to include(:download_path, :browse_path, :keep_path)
end
end
context 'when the build has archive type artifacts' do
let!(:build) { create(:ci_build, :artifacts, artifacts_expire_at: 7.days.from_now) }
let!(:report) { create(:ci_job_artifact, :codequality, job: build) }
it 'exposes artifact details' do
expect(subject[:artifact].keys).to include(:download_path, :browse_path, :keep_path, :expire_at, :expired)
end
end
end
end
......@@ -159,7 +159,7 @@ describe PipelineSerializer do
it 'verifies number of queries', :request_store do
recorded = ActiveRecord::QueryRecorder.new { subject }
expected_queries = Gitlab.ee? ? 42 : 39
expected_queries = Gitlab.ee? ? 43 : 40
expect(recorded.count).to be_within(2).of(expected_queries)
expect(recorded.cached_count).to eq(0)
......@@ -180,7 +180,7 @@ describe PipelineSerializer do
# pipeline. With the same ref this check is cached but if refs are
# different then there is an extra query per ref
# https://gitlab.com/gitlab-org/gitlab-foss/issues/46368
expected_queries = Gitlab.ee? ? 45 : 42
expected_queries = Gitlab.ee? ? 46 : 43
expect(recorded.count).to be_within(2).of(expected_queries)
expect(recorded.cached_count).to eq(0)
......
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