Merge branch '20371-artifacts_present_db_check' into 'master'

Check for Ci::Build artifacts at database level

## What does this MR do?

Check for the presence of artifacts not expired at database level

## Are there points in the code the reviewer needs to double check?

N/A

## Why was this MR needed?

To try to improve a little the performance of the pipelines page

## What are the relevant issue numbers?

Closes #20371

## Screenshots (if relevant)

N/A

## Does this MR meet the acceptance criteria?

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- ~~[ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~
- ~~[ ] API support added~~
- Tests
  - [x] Added for this feature/bug
  - [ ] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)


See merge request !5543

5 files changed, 29 insertions, 1 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 5e181e865a6..88f37735c69 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -25,6 +25,7 @@ v 8.11.0 (unreleased)
   - Load project invited groups and members eagerly in `ProjectTeam#fetch_members`
   - Bump gitlab_git to speedup DiffCollection iterations
   - Make branches sortable without push permission !5462 (winniehell)
+  - Check for Ci::Build artifacts at database level on pipeline partial
   - Add GitLab Workhorse version to admin dashboard (Katarzyna Kobierska Ula Budziszewska)
   - Add the `sprockets-es6` gem
   - Multiple trigger variables show in separate lines (Katarzyna Kobierska Ula Budziszewska)
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index aac78d75f57..08f396210c9 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -13,6 +13,7 @@ module Ci
     scope :unstarted, ->() { where(runner_id: nil) }
     scope :ignore_failures, ->() { where(allow_failure: false) }
     scope :with_artifacts, ->() { where.not(artifacts_file: [nil, '']) }
+    scope :with_artifacts_not_expired, ->() { with_artifacts.where('artifacts_expire_at IS NULL OR artifacts_expire_at > ?', Time.now) }
     scope :with_expired_artifacts, ->() { with_artifacts.where('artifacts_expire_at < ?', Time.now) }
     scope :last_month, ->() { where('created_at > ?', Date.today - 1.month) }
     scope :manual_actions, ->() { where(when: :manual) }
diff --git a/app/views/projects/ci/pipelines/_pipeline.html.haml b/app/views/projects/ci/pipelines/_pipeline.html.haml
index 2f7d54f0bdd..558c35553da 100644
--- a/app/views/projects/ci/pipelines/_pipeline.html.haml
+++ b/app/views/projects/ci/pipelines/_pipeline.html.haml
@@ -57,7 +57,7 @@
 
   %td.pipeline-actions
     .controls.hidden-xs.pull-right
-      - artifacts = pipeline.builds.latest.select { |b| b.artifacts? }
+      - artifacts = pipeline.builds.latest.with_artifacts_not_expired
       - actions = pipeline.manual_actions
       - if artifacts.present? || actions.any?
         .btn-group.inline
diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb
index 5e19e403c6b..1b32d560b16 100644
--- a/spec/factories/ci/builds.rb
+++ b/spec/factories/ci/builds.rb
@@ -90,5 +90,21 @@ FactoryGirl.define do
         build.save!
       end
     end
+
+    trait :artifacts_expired do
+      after(:create) do |build, _|
+        build.artifacts_file =
+          fixture_file_upload(Rails.root.join('spec/fixtures/ci_build_artifacts.zip'),
+            'application/zip')
+
+        build.artifacts_metadata =
+          fixture_file_upload(Rails.root.join('spec/fixtures/ci_build_artifacts_metadata.gz'),
+            'application/x-gzip')
+
+        build.artifacts_expire_at = 1.minute.ago
+
+        build.save!
+      end
+    end
   end
 end
diff --git a/spec/features/pipelines_spec.rb b/spec/features/pipelines_spec.rb
index 7f861db1969..377a9aba60d 100644
--- a/spec/features/pipelines_spec.rb
+++ b/spec/features/pipelines_spec.rb
@@ -116,9 +116,19 @@ describe "Pipelines" do
         it { expect(page).to have_link(with_artifacts.name) }
       end
 
+      context 'with artifacts expired' do
+        let!(:with_artifacts_expired) { create(:ci_build, :artifacts_expired, :success, pipeline: pipeline, name: 'rspec', stage: 'test') }
+
+        before { visit namespace_project_pipelines_path(project.namespace, project) }
+
+        it { expect(page).not_to have_selector('.build-artifacts') }
+      end
+
       context 'without artifacts' do
         let!(:without_artifacts) { create(:ci_build, :success, pipeline: pipeline, name: 'rspec', stage: 'test') }
 
+        before { visit namespace_project_pipelines_path(project.namespace, project) }
+
         it { expect(page).not_to have_selector('.build-artifacts') }
       end
     end