Merge branch 'ce-to-ee-2018-08-23' into 'master'

CE upstream - 2018-08-23 09:21 UTC

See merge request gitlab-org/gitlab-ee!6976

app/controllers/concerns/renders_commits.rb

 module RendersCommits
+  def limited_commits(commits)
+    if commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE
+      [
+        commits.first(MergeRequestDiff::COMMITS_SAFE_SIZE),
+        commits.size - MergeRequestDiff::COMMITS_SAFE_SIZE
+      ]
+    else
+      [commits, 0]
+    end
+  end
+
+  # This is used as a helper method in a controller.
+  # rubocop: disable Gitlab/ModuleWithInstanceVariables
+  def set_commits_for_rendering(commits)
+    @total_commit_count = commits.size
+    limited, @hidden_commit_count = limited_commits(commits)
+    prepare_commits_for_rendering(limited)
+  end
+  # rubocop: enable Gitlab/ModuleWithInstanceVariables
+
   def prepare_commits_for_rendering(commits)
     Banzai::CommitRenderer.render(commits, @project, current_user) # rubocop:disable Gitlab/ModuleWithInstanceVariables
 

app/controllers/projects/commits_controller.rb

@@ -63,7 +63,7 @@ class Projects::CommitsController < Projects::ApplicationController
       end
 
     @commits = @commits.with_pipeline_status
-    @commits = prepare_commits_for_rendering(@commits)
+    @commits = set_commits_for_rendering(@commits)
   end
 
   # Rails 5 sets request.format from the extension.

app/controllers/projects/compare_controller.rb

@@ -78,7 +78,7 @@ class Projects::CompareController < Projects::ApplicationController
   end
 
   def define_commits
-    @commits = compare.present? ? prepare_commits_for_rendering(compare.commits) : []
+    @commits = compare.present? ? set_commits_for_rendering(@compare.commits) : []
   end
 
   def define_diffs

app/controllers/projects/merge_requests/creations_controller.rb

@@ -103,7 +103,7 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap
 
     @target_project = @merge_request.target_project
     @source_project = @merge_request.source_project
-    @commits = prepare_commits_for_rendering(@merge_request.commits)
+    @commits = set_commits_for_rendering(@merge_request.commits)
     @commit = @merge_request.diff_head_commit
 
     @labels = LabelsFinder.new(current_user, project_id: @project.id).execute

app/controllers/projects/merge_requests_controller.rb

@@ -82,7 +82,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     # Get commits from repository
     # or from cache if already merged
     @commits =
-      prepare_commits_for_rendering(@merge_request.commits.with_pipeline_status)
+      set_commits_for_rendering(@merge_request.commits.with_pipeline_status)
 
     render json: { html: view_to_html_string('projects/merge_requests/_commits') }
   end

app/helpers/commits_helper.rb

@@ -210,17 +210,6 @@ module CommitsHelper
     Sanitize.clean(string, remove_contents: true)
   end
 
-  def limited_commits(commits)
-    if commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE
-      [
-        commits.first(MergeRequestDiff::COMMITS_SAFE_SIZE),
-        commits.size - MergeRequestDiff::COMMITS_SAFE_SIZE
-      ]
-    else
-      [commits, 0]
-    end
-  end
-
   def commit_path(project, commit, merge_request: nil)
     if merge_request&.persisted?
       diffs_project_merge_request_path(project, merge_request, commit_id: commit.id)

app/views/projects/commits/_commit_list.html.haml

-- commits, hidden = limited_commits(@commits)
+- commits = @commits
+- hidden = @hidden_commit_count
 - commits = Commit.decorate(commits, @project)
 
 .card
   .card-header
-    Commits (#{@commits.count})
+    Commits (#{@total_commit_count})
   - if hidden > 0
     %ul.content-list
       - commits.each do |commit|

app/views/projects/commits/_commits.html.haml

@@ -2,7 +2,8 @@
 - project = local_assigns.fetch(:project) { merge_request&.project }
 - ref = local_assigns.fetch(:ref) { merge_request&.source_branch }
 
-- commits, hidden = limited_commits(@commits)
+- commits = @commits
+- hidden = @hidden_commit_count
 
 - commits.chunk { |c| c.committed_date.in_time_zone.to_date }.each do |day, commits|
   %li.commit-header.js-commit-header{ data: { day: day } }

app/views/projects/merge_requests/creations/_new_submit.html.haml

@@ -33,7 +33,7 @@
             %li.commits-tab.new-tab
               = link_to url_for(safe_params), data: {target: 'div#commits', action: 'new', toggle: 'tabvue'} do
                 Commits
-                %span.badge.badge-pill= @commits.size
+                %span.badge.badge-pill= @total_commit_count
             - if @pipelines.any?
               %li.builds-tab
                 = link_to url_for(safe_params.merge(action: 'pipelines')), data: {target: 'div#pipelines', action: 'pipelines', toggle: 'tabvue'} do

changelogs/unreleased/ccr-48800-ping_for_boards.yml

+---
+title: Adds count for different board list types (label lists, assignee lists, and
+  milestone lists) to usage statistics.
+merge_request: 21208
+author:
+type: changed

changelogs/unreleased/jprovazn-fix-form-uploads.yml

+---
+title: Accept upload files in public/uplaods/tmp when using accelerated uploads.
+merge_request:
+author:
+type: fixed

changelogs/unreleased/sh-limit-commit-renderering.yml

+---
+title: Speed up diff comparisons by limiting number of commit messages rendered
+merge_request: 21335
+author:
+type: performance

db/migrate/20180815175440_add_index_on_list_type.rb

+# frozen_string_literal: true
+class AddIndexOnListType < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :lists, :list_type
+  end
+
+  def down
+    remove_concurrent_index :lists, :list_type
+  end
+end

db/schema.rb

@@ -1559,6 +1559,7 @@ ActiveRecord::Schema.define(version: 20180816193530) do
 
   add_index "lists", ["board_id", "label_id"], name: "index_lists_on_board_id_and_label_id", unique: true, using: :btree
   add_index "lists", ["label_id"], name: "index_lists_on_label_id", using: :btree
+  add_index "lists", ["list_type"], name: "index_lists_on_list_type", using: :btree
   add_index "lists", ["milestone_id"], name: "index_lists_on_milestone_id", using: :btree
   add_index "lists", ["user_id"], name: "index_lists_on_user_id", using: :btree
 

lib/gitlab/middleware/multipart.rb

@@ -82,9 +82,13 @@ module Gitlab
         end
 
         def open_file(params, key)
-          ::UploadedFile.from_params(
-            params, key,
-            [FileUploader.root, Gitlab.config.uploads.storage_path])
+          allowed_paths = [
+            FileUploader.root,
+            Gitlab.config.uploads.storage_path,
+            File.join(Rails.root, 'public/uploads/tmp')
+          ]
+
+          ::UploadedFile.from_params(params, key, allowed_paths)
         end
       end
 

lib/gitlab/usage_data.rb

@@ -36,6 +36,7 @@ module Gitlab
       def system_usage_data
         {
           counts: {
+            assignee_lists: List.assignee.count,
             boards: Board.count,
             ci_builds: ::Ci::Build.count,
             ci_internal_pipelines: ::Ci::Pipeline.internal.count,
@@ -63,9 +64,11 @@ module Gitlab
             groups: Group.count,
             issues: Issue.count,
             keys: Key.count,
+            label_lists: List.label.count,
             labels: Label.count,
             lfs_objects: LfsObject.count,
             merge_requests: MergeRequest.count,
+            milestone_lists: List.milestone.count,
             milestones: Milestone.count,
             notes: Note.count,
             pages_domains: PagesDomain.count,

spec/controllers/projects/merge_requests/creations_controller_spec.rb

@@ -29,6 +29,55 @@ describe Projects::MergeRequests::CreationsController do
         expect(response).to be_success
       end
     end
+
+    context 'merge request with some commits' do
+      render_views
+
+      let(:large_diff_params) do
+        {
+          namespace_id: fork_project.namespace.to_param,
+          project_id: fork_project,
+          merge_request: {
+            source_branch: 'master',
+            target_branch: 'fix'
+          }
+        }
+      end
+
+      describe 'with artificial limits' do
+        before do
+          # Load MergeRequestdiff so stub_const won't override it with its own definition
+          # See https://github.com/rspec/rspec-mocks/issues/1079
+          stub_const("#{MergeRequestDiff}::COMMITS_SAFE_SIZE", 2)
+        end
+
+        it 'limits total commits' do
+          get :new, large_diff_params
+
+          expect(response).to be_success
+
+          total = assigns(:total_commit_count)
+          expect(assigns(:commits)).to be_an Array
+          expect(total).to be > 0
+          expect(assigns(:hidden_commit_count)).to be > 0
+          expect(response).to have_gitlab_http_status(200)
+          expect(response.body).to match %r(<span class="commits-count">2 commits</span>)
+        end
+      end
+
+      it 'shows total commits' do
+        get :new, large_diff_params
+
+        expect(response).to be_success
+
+        total = assigns(:total_commit_count)
+        expect(assigns(:commits)).to be_an Array
+        expect(total).to be > 0
+        expect(assigns(:hidden_commit_count)).to eq(0)
+        expect(response).to have_gitlab_http_status(200)
+        expect(response.body).to match %r(<span class="commits-count">#{total} commits</span>)
+      end
+    end
   end
 
   describe 'GET diffs' do

spec/lib/gitlab/middleware/multipart_spec.rb

@@ -75,6 +75,26 @@ describe Gitlab::Middleware::Multipart do
     it_behaves_like 'multipart upload files'
   end
 
+  it 'allows files in uploads/tmp directory' do
+    Dir.mktmpdir do |dir|
+      uploads_dir = File.join(dir, 'public/uploads/tmp')
+      FileUtils.mkdir_p(uploads_dir)
+
+      allow(Rails).to receive(:root).and_return(dir)
+      allow(Dir).to receive(:tmpdir).and_return(File.join(Dir.tmpdir, 'tmpsubdir'))
+
+      Tempfile.open('top-level', uploads_dir) do |tempfile|
+        env = post_env({ 'file' => tempfile.path }, { 'file.name' => original_filename, 'file.path' => tempfile.path }, Gitlab::Workhorse.secret, 'gitlab-workhorse')
+
+        expect(app).to receive(:call) do |env|
+          expect(Rack::Request.new(env).params['file']).to be_a(::UploadedFile)
+        end
+
+        middleware.call(env)
+      end
+    end
+  end
+
   it 'allows symlinks for uploads dir' do
     Tempfile.open('two-levels') do |tempfile|
       symlinked_dir = '/some/dir/uploads'

spec/lib/gitlab/usage_data_spec.rb

@@ -57,6 +57,7 @@ describe Gitlab::UsageData do
       expect(count_data[:projects]).to eq(3)
 
       expect(count_data.keys).to include(*%i(
+        assignee_lists
         boards
         ci_builds
         ci_internal_pipelines
@@ -84,9 +85,11 @@ describe Gitlab::UsageData do
         groups
         issues
         keys
+        label_lists
         labels
         lfs_objects
         merge_requests
+        milestone_lists
         milestones
         notes
         projects

spec/views/projects/merge_requests/_commits.html.haml_spec.rb

@@ -20,6 +20,7 @@ describe 'projects/merge_requests/_commits.html.haml' do
 
     assign(:merge_request, merge_request)
     assign(:commits, merge_request.commits)
+    assign(:hidden_commit_count, 0)
   end
 
   it 'shows commits from source project' do
@@ -30,4 +31,16 @@ describe 'projects/merge_requests/_commits.html.haml' do
 
     expect(rendered).to have_link(href: href)
   end
+
+  context 'when there are hidden commits' do
+    before do
+      assign(:hidden_commit_count, 1)
+    end
+
+    it 'shows notice about omitted commits' do
+      render
+
+      expect(rendered).to match(/1 additional commit has been omitted to prevent performance issues/)
+    end
+  end
 end

spec/views/projects/merge_requests/creations/_new_submit.html.haml_spec.rb

@@ -9,6 +9,8 @@ describe 'projects/merge_requests/creations/_new_submit.html.haml' do
 
     assign(:merge_request, merge_request)
     assign(:commits, merge_request.commits)
+    assign(:hidden_commit_count, 0)
+    assign(:total_commit_count, merge_request.commits.count)
     assign(:project, merge_request.target_project)
 
     allow(view).to receive(:can?).and_return(true)
@@ -29,4 +31,17 @@ describe 'projects/merge_requests/creations/_new_submit.html.haml' do
       expect(rendered).not_to have_text('Builds')
     end
   end
+
+  context 'when there are hidden commits' do
+    before do
+      assign(:pipelines, Ci::Pipeline.none)
+      assign(:hidden_commit_count, 2)
+    end
+
+    it 'shows notice about omitted commits' do
+      render
+
+      expect(rendered).to match(/2 additional commits have been omitted to prevent performance issues/)
+    end
+  end
 end