Commit c6db352c authored by Matija Čupić's avatar Matija Čupić

Port changes from FOSS

Ports the changes from
https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/32590
parent 4d8ed0b1
...@@ -8,10 +8,31 @@ class Projects::ArtifactsController < Projects::ApplicationController ...@@ -8,10 +8,31 @@ class Projects::ArtifactsController < Projects::ApplicationController
layout 'project' layout 'project'
before_action :authorize_read_build! before_action :authorize_read_build!
before_action :authorize_update_build!, only: [:keep] before_action :authorize_update_build!, only: [:keep]
before_action :authorize_destroy_artifacts!, only: [:destroy]
before_action :extract_ref_name_and_path before_action :extract_ref_name_and_path
before_action :validate_artifacts!, except: [:download] before_action :validate_artifacts!, except: [:index, :download, :destroy]
before_action :entry, only: [:file] before_action :entry, only: [:file]
MAX_PER_PAGE = 20
def index
finder = ArtifactsFinder.new(@project, artifacts_params)
all_artifacts = finder.execute
@artifacts = all_artifacts.page(params[:page]).per(MAX_PER_PAGE)
@total_size = all_artifacts.total_size
end
def destroy
notice = if artifact.destroy
_('Artifact was successfully deleted.')
else
_('Artifact could not be deleted.')
end
redirect_to project_artifacts_path(@project), status: :see_other, notice: notice
end
def download def download
return render_404 unless artifacts_file return render_404 unless artifacts_file
...@@ -74,6 +95,10 @@ class Projects::ArtifactsController < Projects::ApplicationController ...@@ -74,6 +95,10 @@ class Projects::ArtifactsController < Projects::ApplicationController
@ref_name, @path = extract_ref(params[:ref_name_and_path]) @ref_name, @path = extract_ref(params[:ref_name_and_path])
end end
def artifacts_params
params.permit(:sort, :search)
end
def validate_artifacts! def validate_artifacts!
render_404 unless build&.artifacts? render_404 unless build&.artifacts?
end end
...@@ -85,6 +110,11 @@ class Projects::ArtifactsController < Projects::ApplicationController ...@@ -85,6 +110,11 @@ class Projects::ArtifactsController < Projects::ApplicationController
end end
end end
def artifact
@artifact ||=
project.job_artifacts.find(params[:id])
end
def build_from_id def build_from_id
project.builds.find_by_id(params[:job_id]) if params[:job_id] project.builds.find_by_id(params[:job_id]) if params[:job_id]
end end
......
# frozen_string_literal: true
class ArtifactsFinder
def initialize(project, params = {})
@project = project
@params = params
end
def execute
artifacts = @project.job_artifacts
artifacts = by_job_name(artifacts)
sort(artifacts)
end
def by_job_name(artifacts)
return artifacts unless @params[:search].present?
artifacts.search_by_job_name(@params[:search])
end
private
def sort_key
@params[:sort] || 'created_desc'
end
def sort(artifacts)
artifacts.order_by(sort_key)
end
end
...@@ -28,7 +28,9 @@ module SortingHelper ...@@ -28,7 +28,9 @@ module SortingHelper
sort_value_priority => sort_title_priority, sort_value_priority => sort_title_priority,
sort_value_upvotes => sort_title_upvotes, sort_value_upvotes => sort_title_upvotes,
sort_value_contacted_date => sort_title_contacted_date, sort_value_contacted_date => sort_title_contacted_date,
sort_value_relative_position => sort_title_relative_position sort_value_relative_position => sort_title_relative_position,
sort_value_size => sort_title_size,
sort_value_expire_date => sort_title_expire_date
} }
end end
...@@ -406,6 +408,14 @@ module SortingHelper ...@@ -406,6 +408,14 @@ module SortingHelper
s_('SortOptions|Manual') s_('SortOptions|Manual')
end end
def sort_title_size
s_('SortOptions|Size')
end
def sort_title_expire_date
s_('SortOptions|Expired date')
end
# Values. # Values.
def sort_value_access_level_asc def sort_value_access_level_asc
'access_level_asc' 'access_level_asc'
...@@ -558,4 +568,12 @@ module SortingHelper ...@@ -558,4 +568,12 @@ module SortingHelper
def sort_value_relative_position def sort_value_relative_position
'relative_position' 'relative_position'
end end
def sort_value_size
'size_desc'
end
def sort_value_expire_date
'expired_asc'
end
end end
...@@ -5,6 +5,7 @@ module Ci ...@@ -5,6 +5,7 @@ module Ci
include AfterCommitQueue include AfterCommitQueue
include ObjectStorage::BackgroundMove include ObjectStorage::BackgroundMove
include UpdateProjectStatistics include UpdateProjectStatistics
include Sortable
extend Gitlab::Ci::Model extend Gitlab::Ci::Model
NotSupportedAdapterError = Class.new(StandardError) NotSupportedAdapterError = Class.new(StandardError)
...@@ -143,10 +144,18 @@ module Ci ...@@ -143,10 +144,18 @@ module Ci
self.update_column(:file_store, file.object_store) self.update_column(:file_store, file.object_store)
end end
def self.total_size
self.sum(:size)
end
def self.artifacts_size_for(project) def self.artifacts_size_for(project)
self.where(project: project).sum(:size) self.where(project: project).sum(:size)
end end
def self.search_by_job_name(job_name)
joins(:job).where(ci_builds: { name: job_name })
end
def local_store? def local_store?
[nil, ::JobArtifactUploader::Store::LOCAL].include?(self.file_store) [nil, ::JobArtifactUploader::Store::LOCAL].include?(self.file_store)
end end
......
...@@ -273,6 +273,7 @@ class Project < ApplicationRecord ...@@ -273,6 +273,7 @@ class Project < ApplicationRecord
has_many :builds, class_name: 'Ci::Build', inverse_of: :project, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :builds, class_name: 'Ci::Build', inverse_of: :project, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :build_trace_section_names, class_name: 'Ci::BuildTraceSectionName' has_many :build_trace_section_names, class_name: 'Ci::BuildTraceSectionName'
has_many :build_trace_chunks, class_name: 'Ci::BuildTraceChunk', through: :builds, source: :trace_chunks has_many :build_trace_chunks, class_name: 'Ci::BuildTraceChunk', through: :builds, source: :trace_chunks
has_many :job_artifacts, class_name: 'Ci::JobArtifact'
has_many :runner_projects, class_name: 'Ci::RunnerProject', inverse_of: :project has_many :runner_projects, class_name: 'Ci::RunnerProject', inverse_of: :project
has_many :runners, through: :runner_projects, source: :runner, class_name: 'Ci::Runner' has_many :runners, through: :runner_projects, source: :runner, class_name: 'Ci::Runner'
has_many :variables, class_name: 'Ci::Variable' has_many :variables, class_name: 'Ci::Variable'
......
...@@ -37,6 +37,8 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do ...@@ -37,6 +37,8 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
scope '-' do scope '-' do
get 'archive/*id', constraints: { format: Gitlab::PathRegex.archive_formats_regex, id: /.+?/ }, to: 'repositories#archive', as: 'archive' get 'archive/*id', constraints: { format: Gitlab::PathRegex.archive_formats_regex, id: /.+?/ }, to: 'repositories#archive', as: 'archive'
resources :artifacts, only: [:index, :destroy]
resources :jobs, only: [:index, :show], constraints: { id: /\d+/ } do resources :jobs, only: [:index, :show], constraints: { id: /\d+/ } do
collection do collection do
resources :artifacts, only: [] do resources :artifacts, only: [] do
......
...@@ -1860,6 +1860,12 @@ msgstr "" ...@@ -1860,6 +1860,12 @@ msgstr ""
msgid "Artifact ID" msgid "Artifact ID"
msgstr "" msgstr ""
msgid "Artifact could not be deleted."
msgstr ""
msgid "Artifact was successfully deleted."
msgstr ""
msgid "Artifacts" msgid "Artifacts"
msgstr "" msgstr ""
...@@ -14483,6 +14489,9 @@ msgstr "" ...@@ -14483,6 +14489,9 @@ msgstr ""
msgid "SortOptions|Due soon" msgid "SortOptions|Due soon"
msgstr "" msgstr ""
msgid "SortOptions|Expired date"
msgstr ""
msgid "SortOptions|Label priority" msgid "SortOptions|Label priority"
msgstr "" msgstr ""
...@@ -14573,6 +14582,9 @@ msgstr "" ...@@ -14573,6 +14582,9 @@ msgstr ""
msgid "SortOptions|Recently starred" msgid "SortOptions|Recently starred"
msgstr "" msgstr ""
msgid "SortOptions|Size"
msgstr ""
msgid "SortOptions|Sort direction" msgid "SortOptions|Sort direction"
msgstr "" msgstr ""
......
...@@ -6,7 +6,7 @@ describe Projects::ArtifactsController do ...@@ -6,7 +6,7 @@ describe Projects::ArtifactsController do
let(:user) { project.owner } let(:user) { project.owner }
set(:project) { create(:project, :repository, :public) } set(:project) { create(:project, :repository, :public) }
let(:pipeline) do set(:pipeline) do
create(:ci_pipeline, create(:ci_pipeline,
project: project, project: project,
sha: project.commit.sha, sha: project.commit.sha,
...@@ -14,12 +14,89 @@ describe Projects::ArtifactsController do ...@@ -14,12 +14,89 @@ describe Projects::ArtifactsController do
status: 'success') status: 'success')
end end
let(:job) { create(:ci_build, :success, :artifacts, pipeline: pipeline) } let!(:job) { create(:ci_build, :success, :artifacts, pipeline: pipeline) }
before do before do
sign_in(user) sign_in(user)
end end
describe 'GET index' do
subject { get :index, params: { namespace_id: project.namespace, project_id: project } }
it 'sets the artifacts variable' do
subject
expect(assigns(:artifacts)).to contain_exactly(*project.job_artifacts)
end
it 'sets the total size variable' do
subject
expect(assigns(:total_size)).to eq(project.job_artifacts.total_size)
end
describe 'pagination' do
before do
stub_const("#{described_class}::MAX_PER_PAGE", 1)
end
it 'paginates artifacts' do
subject
expect(assigns(:artifacts)).to contain_exactly(project.job_artifacts.last)
end
end
end
describe 'DELETE destroy' do
let!(:artifact) { job.job_artifacts.erasable.first }
subject { delete :destroy, params: { namespace_id: project.namespace, project_id: project, id: artifact } }
it 'deletes the artifact' do
expect { subject }.to change { Ci::JobArtifact.count }.by(-1)
expect(artifact).not_to exist
end
it 'redirects to artifacts index page' do
subject
expect(response).to redirect_to(project_artifacts_path(project))
end
it 'sets the notice' do
subject
expect(flash[:notice]).to eq('Artifact was successfully deleted.')
end
context 'when artifact deletion fails' do
before do
allow_any_instance_of(Ci::JobArtifact).to receive(:destroy).and_return(false)
end
it 'redirects to artifacts index page' do
subject
expect(response).to redirect_to(project_artifacts_path(project))
end
it 'sets the notice' do
subject
expect(flash[:notice]).to eq('Artifact could not be deleted.')
end
end
context 'when user is not authorized' do
let(:user) { create(:user) }
it 'does not delete the artifact' do
expect { subject }.not_to change { Ci::JobArtifact.count }
end
end
end
describe 'GET download' do describe 'GET download' do
def download_artifact(extra_params = {}) def download_artifact(extra_params = {})
params = { namespace_id: project.namespace, project_id: project, job_id: job }.merge(extra_params) params = { namespace_id: project.namespace, project_id: project, job_id: job }.merge(extra_params)
......
# frozen_string_literal: true
require 'spec_helper'
describe ArtifactsFinder do
let(:project) { create(:project) }
describe '#execute' do
before do
create(:ci_build, :artifacts, project: project)
end
subject { described_class.new(project, params).execute }
context 'with empty params' do
let(:params) { {} }
it 'returns all artifacts belonging to the project' do
expect(subject).to contain_exactly(*project.job_artifacts)
end
end
context 'with sort param' do
let(:params) { { sort: 'size_desc' } }
it 'sorts the artifacts' do
expect(subject).to eq(project.job_artifacts.order_by('size_desc'))
end
end
context 'with job_name param' do
let(:params) { { search: 'unique_name' } }
it 'filters the artifacts by job name' do
build = create(:ci_build, :artifacts, project: project, name: 'unique_name')
expect(subject).to contain_exactly(*build.job_artifacts)
end
end
end
end
...@@ -349,6 +349,7 @@ project: ...@@ -349,6 +349,7 @@ project:
- members_and_requesters - members_and_requesters
- build_trace_section_names - build_trace_section_names
- build_trace_chunks - build_trace_chunks
- job_artifacts
- root_of_fork_network - root_of_fork_network
- fork_network_member - fork_network_member
- fork_network - fork_network
......
...@@ -95,6 +95,17 @@ describe Ci::JobArtifact do ...@@ -95,6 +95,17 @@ describe Ci::JobArtifact do
end end
end end
describe '.search_by_job_name' do
subject { described_class.search_by_job_name('unique_name') }
it 'returns only artifacts for specified job name' do
create(:ci_build, :artifacts)
matching_build = create(:ci_build, :artifacts, name: 'unique_name')
expect(subject).to eq(matching_build.job_artifacts)
end
end
describe 'callbacks' do describe 'callbacks' do
subject { create(:ci_job_artifact, :archive) } subject { create(:ci_job_artifact, :archive) }
......
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