Add latest changes from gitlab-org/gitlab@master

app/assets/javascripts/project_find_file.js

@@ -81,7 +81,7 @@ export default class ProjectFindFile {
     // find file
   }
 
-  // files pathes load
+  // files paths load
   load(url) {
     axios
       .get(url)

app/assets/stylesheets/framework/sidebar.scss

@@ -171,7 +171,7 @@
       position: absolute;
       top: $gl-padding;
       bottom: $gl-padding;
-      left: map-get($spacers, 2) - 1px;
+      left: map-get($spacers, 2) - px-to-rem(1px);
     }
 
     &-row {
@@ -187,7 +187,7 @@
       * 2px extra is to give a little more height than needed
       * to hide timeline line before/after the element starts/ends
       */
-      height: map-get($spacers, 4) + 2px;
+      height: map-get($spacers, 4) + px-to-rem(2px);
       z-index: 1;
       position: relative;
       top: -3px;

app/controllers/projects/artifacts_controller.rb

@@ -8,10 +8,37 @@ class Projects::ArtifactsController < Projects::ApplicationController
   layout 'project'
   before_action :authorize_read_build!
   before_action :authorize_update_build!, only: [:keep]
+  before_action :authorize_destroy_artifacts!, only: [:destroy]
   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]
 
+  MAX_PER_PAGE = 20
+
+  def index
+    # Loading artifacts is very expensive in projects with a lot of artifacts.
+    # This feature flag prevents a DOS attack vector.
+    # It should be removed only after resolving the underlying performance
+    # issues: https://gitlab.com/gitlab-org/gitlab/issues/32281
+    return head :no_content unless Feature.enabled?(:artifacts_management_page, @project)
+
+    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
     return render_404 unless artifacts_file
 
@@ -74,6 +101,10 @@ class Projects::ArtifactsController < Projects::ApplicationController
     @ref_name, @path = extract_ref(params[:ref_name_and_path])
   end
 
+  def artifacts_params
+    params.permit(:sort)
+  end
+
   def validate_artifacts!
     render_404 unless build&.artifacts?
   end
@@ -85,6 +116,11 @@ class Projects::ArtifactsController < Projects::ApplicationController
     end
   end
 
+  def artifact
+    @artifact ||=
+      project.job_artifacts.find(params[:id])
+  end
+
   def build_from_id
     project.builds.find_by_id(params[:job_id]) if params[:job_id]
   end

app/finders/artifacts_finder.rb

+# frozen_string_literal: true
+
+class ArtifactsFinder
+  def initialize(project, params = {})
+    @project = project
+    @params = params
+  end
+
+  def execute
+    artifacts = @project.job_artifacts
+
+    sort(artifacts)
+  end
+
+  private
+
+  def sort_key
+    @params[:sort] || 'created_desc'
+  end
+
+  def sort(artifacts)
+    artifacts.order_by(sort_key)
+  end
+end

app/helpers/sorting_helper.rb

@@ -28,7 +28,9 @@ module SortingHelper
       sort_value_priority          => sort_title_priority,
       sort_value_upvotes           => sort_title_upvotes,
       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
 
@@ -406,6 +408,14 @@ module SortingHelper
     s_('SortOptions|Manual')
   end
 
+  def sort_title_size
+    s_('SortOptions|Size')
+  end
+
+  def sort_title_expire_date
+    s_('SortOptions|Expired date')
+  end
+
   # Values.
   def sort_value_access_level_asc
     'access_level_asc'
@@ -558,4 +568,12 @@ module SortingHelper
   def sort_value_relative_position
     'relative_position'
   end
+
+  def sort_value_size
+    'size_desc'
+  end
+
+  def sort_value_expire_date
+    'expired_asc'
+  end
 end

app/models/ci/job_artifact.rb

@@ -5,6 +5,7 @@ module Ci
     include AfterCommitQueue
     include ObjectStorage::BackgroundMove
     include UpdateProjectStatistics
+    include Sortable
     extend Gitlab::Ci::Model
 
     NotSupportedAdapterError = Class.new(StandardError)
@@ -143,6 +144,10 @@ module Ci
       self.update_column(:file_store, file.object_store)
     end
 
+    def self.total_size
+      self.sum(:size)
+    end
+
     def self.artifacts_size_for(project)
       self.where(project: project).sum(:size)
     end

app/models/project.rb

@@ -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 :build_trace_section_names, class_name: 'Ci::BuildTraceSectionName'
   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 :runners, through: :runner_projects, source: :runner, class_name: 'Ci::Runner'
   has_many :variables, class_name: 'Ci::Variable'

app/models/protected_branch.rb

@@ -40,11 +40,6 @@ class ProtectedBranch < ApplicationRecord
   def self.protected_refs(project)
     project.protected_branches.select(:name)
   end
-
-  def self.branch_requires_code_owner_approval?(project, branch_name)
-    # NOOP
-    #
-  end
 end
 
 ProtectedBranch.prepend_if_ee('EE::ProtectedBranch')

app/models/release.rb

@@ -22,7 +22,6 @@ class Release < ApplicationRecord
   accepts_nested_attributes_for :links, allow_destroy: true
 
   validates :description, :project, :tag, presence: true
-  validates :name, presence: true, on: :create
   validates_associated :milestone_releases, message: -> (_, obj) { obj[:value].map(&:errors).map(&:full_messages).join(",") }
 
   scope :sorted, -> { order(released_at: :desc) }

config/routes/project.rb

@@ -37,6 +37,8 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
       scope '-' do
         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
           collection do
             resources :artifacts, only: [] do

db/post_migrate/20190827102026_migrate_code_owner_approval_status_to_protected_branches_in_batches.rb

-# frozen_string_literal: true
-
-class MigrateCodeOwnerApprovalStatusToProtectedBranchesInBatches < ActiveRecord::Migration[5.2]
-  include Gitlab::Database::MigrationHelpers
-
-  disable_ddl_transaction!
-
-  DOWNTIME = false
-  BATCH_SIZE = 200
-
-  class Project < ActiveRecord::Base
-    include EachBatch
-
-    self.table_name = 'projects'
-    self.inheritance_column = :_type_disabled
-
-    has_many :protected_branches
-  end
-
-  class ProtectedBranch < ActiveRecord::Base
-    include EachBatch
-
-    self.table_name = 'protected_branches'
-    self.inheritance_column = :_type_disabled
-
-    belongs_to :project
-  end
-
-  def up
-    add_concurrent_index :projects, :id, name: "temp_active_projects_with_prot_branches", where: 'archived = false and pending_delete = false and merge_requests_require_code_owner_approval = true'
-
-    ProtectedBranch
-      .joins(:project)
-      .where(projects: { archived: false, pending_delete: false, merge_requests_require_code_owner_approval: true })
-      .each_batch(of: BATCH_SIZE) do |batch|
-        batch.update_all(code_owner_approval_required: true)
-      end
-
-    remove_concurrent_index_by_name(:projects, "temp_active_projects_with_prot_branches")
-  end
-
-  def down
-    # noop
-    #
-  end
-end

locale/gitlab.pot

@@ -1860,6 +1860,12 @@ msgstr ""
 msgid "Artifact ID"
 msgstr ""
 
+msgid "Artifact could not be deleted."
+msgstr ""
+
+msgid "Artifact was successfully deleted."
+msgstr ""
+
 msgid "Artifacts"
 msgstr ""
 
@@ -14483,6 +14489,9 @@ msgstr ""
 msgid "SortOptions|Due soon"
 msgstr ""
 
+msgid "SortOptions|Expired date"
+msgstr ""
+
 msgid "SortOptions|Label priority"
 msgstr ""
 
@@ -14573,6 +14582,9 @@ msgstr ""
 msgid "SortOptions|Recently starred"
 msgstr ""
 
+msgid "SortOptions|Size"
+msgstr ""
+
 msgid "SortOptions|Sort direction"
 msgstr ""
 
@@ -17962,7 +17974,7 @@ msgstr ""
 msgid "You don't have any recent searches"
 msgstr ""
 
-msgid "You don’t have acces to Productivity Analaytics in this group"
+msgid "You don’t have access to Productivity Analytics in this group"
 msgstr ""
 
 msgid "You have been granted %{access_level} access to the %{source_link} %{source_type}."

package.json

@@ -37,9 +37,9 @@
     "@babel/plugin-syntax-dynamic-import": "^7.2.0",
     "@babel/plugin-syntax-import-meta": "^7.2.0",
     "@babel/preset-env": "^7.5.5",
-    "@gitlab/svgs": "^1.73.0",
-    "@gitlab/ui": "5.25.2",
-    "@gitlab/visual-review-tools": "1.0.1",
+    "@gitlab/svgs": "^1.74.0",
+    "@gitlab/ui": "5.26.0",
+    "@gitlab/visual-review-tools": "1.0.2",
     "apollo-cache-inmemory": "^1.5.1",
     "apollo-client": "^2.5.1",
     "apollo-link": "^1.2.11",

spec/controllers/projects/artifacts_controller_spec.rb

@@ -6,7 +6,7 @@ describe Projects::ArtifactsController do
   let(:user) { project.owner }
   set(:project) { create(:project, :repository, :public) }
 
-  let(:pipeline) do
+  set(:pipeline) do
     create(:ci_pipeline,
             project: project,
             sha: project.commit.sha,
@@ -14,12 +14,119 @@ describe Projects::ArtifactsController do
             status: 'success')
   end
 
-  let(:job) { create(:ci_build, :success, :artifacts, pipeline: pipeline) }
+  let!(:job) { create(:ci_build, :success, :artifacts, pipeline: pipeline) }
 
   before do
     sign_in(user)
   end
 
+  describe 'GET index' do
+    subject { get :index, params: { namespace_id: project.namespace, project_id: project } }
+
+    context 'when feature flag is on' do
+      before do
+        stub_feature_flags(artifacts_management_page: true)
+      end
+
+      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
+
+    context 'when feature flag is off' do
+      before do
+        stub_feature_flags(artifacts_management_page: false)
+      end
+
+      it 'renders no content' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:no_content)
+      end
+
+      it 'does not set the artifacts variable' do
+        subject
+
+        expect(assigns(:artifacts)).to eq(nil)
+      end
+
+      it 'does not set the total size variable' do
+        subject
+
+        expect(assigns(:total_size)).to eq(nil)
+      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
     def download_artifact(extra_params = {})
       params = { namespace_id: project.namespace, project_id: project, job_id: job }.merge(extra_params)

spec/finders/artifacts_finder_spec.rb

+# 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
+  end
+end

spec/lib/gitlab/import_export/all_models.yml

@@ -349,6 +349,7 @@ project:
 - members_and_requesters
 - build_trace_section_names
 - build_trace_chunks
+- job_artifacts
 - root_of_fork_network
 - fork_network_member
 - fork_network

spec/migrations/migrate_code_owner_approval_status_to_protected_branches_in_batches_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-require Rails.root.join('db', 'post_migrate', '20190827102026_migrate_code_owner_approval_status_to_protected_branches_in_batches.rb')
-
-describe MigrateCodeOwnerApprovalStatusToProtectedBranchesInBatches, :migration do
-  let(:namespaces)         { table(:namespaces) }
-  let(:projects)           { table(:projects) }
-  let(:protected_branches) { table(:protected_branches) }
-
-  let(:namespace) do
-    namespaces.create!(
-      path: 'gitlab-instance-administrators',
-      name: 'GitLab Instance Administrators'
-    )
-  end
-
-  let(:project) do
-    projects.create!(
-      namespace_id: namespace.id,
-      name: 'GitLab Instance Administration'
-    )
-  end
-
-  let!(:protected_branch_1) do
-    protected_branches.create!(
-      name: "branch name",
-      project_id: project.id
-    )
-  end
-
-  describe '#up' do
-    context "when there's no projects needing approval" do
-      it "doesn't change any protected branch records" do
-        expect { migrate! }
-          .not_to change { ProtectedBranch.where(code_owner_approval_required: true).count }
-      end
-    end
-
-    context "when there's a project needing approval" do
-      let!(:project_needing_approval) do
-        projects.create!(
-          namespace_id: namespace.id,
-          name: 'GitLab Instance Administration',
-          merge_requests_require_code_owner_approval: true
-        )
-      end
-
-      let!(:protected_branch_2) do
-        protected_branches.create!(
-          name: "branch name",
-          project_id: project_needing_approval.id
-        )
-      end
-
-      it "changes N protected branch records" do
-        expect { migrate! }
-          .to change { ProtectedBranch.where(code_owner_approval_required: true).count }
-          .by(1)
-      end
-    end
-  end
-end

spec/models/release_spec.rb

@@ -20,7 +20,6 @@ RSpec.describe Release do
   describe 'validation' do
     it { is_expected.to validate_presence_of(:project) }
     it { is_expected.to validate_presence_of(:description) }
-    it { is_expected.to validate_presence_of(:name) }
 
     context 'when a release exists in the database without a name' do
       it 'does not require name' do

yarn.lock

@@ -991,15 +991,15 @@
   dependencies:
     vue-eslint-parser "^6.0.4"
 
-"@gitlab/svgs@^1.73.0":
-  version "1.73.0"
-  resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.73.0.tgz#e44b347e4be77b94474c80cf5c2ee26ca0325c2f"
-  integrity sha512-4on+l5CS8Ae8OOcrnxwkO5s2zq1kHl/YjnOrHaX7megr6jsTYsVzKGaEMe0ViMSIPXA2+QnGD6vElKMkeD2+YQ==
+"@gitlab/svgs@^1.74.0":
+  version "1.74.0"
+  resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.74.0.tgz#2883c47c476a08e8c9c3621117a544204f4c13a3"
+  integrity sha512-L/Jga3EzGgOWF1rdQrH8wNm4dBFXcAVPZnFOEvBoe5OoWZgR3Ac/5Bgz4fYXYyPEKYUSorO7eyE6OVSvjKoM7g==
 
-"@gitlab/ui@5.25.2":
-  version "5.25.2"
-  resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-5.25.2.tgz#599954fefcc228d31a398dbe3c1e2287a0fcdb3e"
-  integrity sha512-mwwvEhVTomnZQjG0dADD+9Kg1UHZXAIb4s5QwQxwpgTkemILYIb1r96oKWfmPe8Pl/xrzAoMUtGEPT3XbxDqYQ==
+"@gitlab/ui@5.26.0":
+  version "5.26.0"
+  resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-5.26.0.tgz#303dcb339947b04bd04378828bd6b6ee1509ea9e"
+  integrity sha512-F8zjN6oiXUy787/4xD+vApuqRxiNe5ZhWg96gT23cUk5SL1Oj4NyMETpAh0v9R9J/i70ETmBYW011EGogjlAVA==
   dependencies:
     "@babel/standalone" "^7.0.0"
     "@gitlab/vue-toasted" "^1.2.1"
@@ -1013,10 +1013,10 @@
     vue "^2.6.10"
     vue-loader "^15.4.2"
 
-"@gitlab/visual-review-tools@1.0.1":
-  version "1.0.1"
-  resolved "https://registry.yarnpkg.com/@gitlab/visual-review-tools/-/visual-review-tools-1.0.1.tgz#7e588328ed018d91560633d56865d65b72c3a11b"
-  integrity sha512-vNqpui0khtPi3crrrFtfuT+nw0SdD/nMyb+aurbJzc3RXuVJGCdgYwosvTLPcJkdMOVfTijizV5+ys75s8INBw==
+"@gitlab/visual-review-tools@1.0.2":
+  version "1.0.2"
+  resolved "https://registry.yarnpkg.com/@gitlab/visual-review-tools/-/visual-review-tools-1.0.2.tgz#d7b410d962cf32e6b6159207917134f7e6a90c68"
+  integrity sha512-U6cw/y/Hf9gYhpV9zBPv4SoIXf1hKye2Xrynj+1Yt2ZmJJG/+QnJfvS6MEuFcNcJRL42p1VDG98uzYMp3rJ7ww==
 
 "@gitlab/vue-toasted@^1.2.1":
   version "1.2.1"
@@ -11122,16 +11122,11 @@ sort-keys@^2.0.0:
   dependencies:
     is-plain-obj "^1.0.0"
 
-sortablejs@^1.10.0:
+sortablejs@^1.10.0, sortablejs@^1.9.0:
   version "1.10.0"
   resolved "https://registry.yarnpkg.com/sortablejs/-/sortablejs-1.10.0.tgz#0ebc054acff2486569194a2f975b2b145dd5e7d6"
   integrity sha512-+e0YakK1BxgEZpf9l9UiFaiQ8ZOBn1p/4qkkXr8QDVmYyCrUDTyDRRGm0AgW4E4cD0wtgxJ6yzIRkSPUwqhuhg==
 
-sortablejs@^1.9.0:
-  version "1.9.0"
-  resolved "https://registry.yarnpkg.com/sortablejs/-/sortablejs-1.9.0.tgz#2d1e74ae6bac2cb4ad0622908f340848969eb88d"
-  integrity sha512-Ot6bYJ6PoqPmpsqQYXjn1+RKrY2NWQvQt/o4jfd/UYwVWndyO5EPO8YHbnm5HIykf8ENsm4JUrdAvolPT86yYA==
-
 source-list-map@^2.0.0:
   version "2.0.0"
   resolved "https://registry.yarnpkg.com/source-list-map/-/source-list-map-2.0.0.tgz#aaa47403f7b245a92fbc97ea08f250d6087ed085"