Merge remote-tracking branch 'upstream/master' into ce-to-ee-2018-07-03

.gitlab-ci.yml

@@ -391,7 +391,8 @@ review-docs-deploy-manual:
     - ./$SCRIPT_NAME deploy
   when: manual
   only:
-    - branches
+    - branches@gitlab-org/gitlab-ce
+    - branches@gitlab-org/gitlab-ee
   <<: *except-docs-and-qa
 
 # Always trigger a docs build in gitlab-docs only on docs-only branches.
@@ -404,6 +405,8 @@ review-docs-deploy:
     - ./$SCRIPT_NAME deploy
   only:
     - /(^docs[\/-].*|.*-docs$)/
+    - branches@gitlab-org/gitlab-ce
+    - branches@gitlab-org/gitlab-ee
   <<: *except-qa
 
 # Cleanup remote environment of gitlab-docs

.gitlab/issue_templates/Security Developer Workflow.md → .gitlab/issue_templates/Security developer workflow.md

@@ -39,6 +39,7 @@ Set the title to: `[Security] Description of the original issue`
 - [ ] Add the nickname of the external user who found the issue (and/or HackerOne profile) to the Thanks row in the [details section](#details)
 
 ### Summary
+
 #### Links
 
 | Description | Link |

.gitlab/merge_request_templates/Database Changes.md → .gitlab/merge_request_templates/Database changes.md

 Add a description of your merge request here. Merge requests without an adequate
 description will not be reviewed until one is added.
 
-## Database Checklist
+## Database checklist
 
 When adding migrations:
 
@@ -31,7 +31,7 @@ When removing columns, tables, indexes or other structures:
 - [ ] Removed these in a post-deployment migration
 - [ ] Made sure the application no longer uses (or ignores) these structures
 
-## General Checklist
+## General checklist
 
 - [ ] [Changelog entry](https://docs.gitlab.com/ee/development/changelog.html) added, if necessary
 - [ ] [Documentation created/updated](https://docs.gitlab.com/ee/development/doc_styleguide.html)

app/presenters/project_presenter.rb

@@ -27,6 +27,7 @@ class ProjectPresenter < Gitlab::View::Presenter::Delegated
 
   def statistics_buttons(show_auto_devops_callout:)
     [
+      readme_anchor_data,
       changelog_anchor_data,
       license_anchor_data,
       contribution_guide_anchor_data,
@@ -212,11 +213,11 @@ class ProjectPresenter < Gitlab::View::Presenter::Delegated
   end
 
   def readme_anchor_data
-    if current_user && can_current_user_push_to_default_branch? && repository.readme.blank?
+    if current_user && can_current_user_push_to_default_branch? && repository.readme.nil?
       OpenStruct.new(enabled: false,
                      label: _('Add Readme'),
                      link: add_readme_path)
-    elsif repository.readme.present?
+    elsif repository.readme
       OpenStruct.new(enabled: true,
                      label: _('Readme'),
                      link: default_view != 'readme' ? readme_path : '#readme')

app/serializers/build_details_entity.rb

@@ -35,7 +35,7 @@ class BuildDetailsEntity < JobEntity
 
   def build_failed_issue_options
     { title: "Job Failed ##{build.id}",
-      description: "Job [##{build.id}](#{project_job_path(project, build)}) failed for #{build.sha}:\n" }
+      description: "Job [##{build.id}](#{project_job_url(project, build)}) failed for #{build.sha}:\n" }
   end
 
   def current_user

changelogs/unreleased/36907-fix-new-issue-link-from-failed-job.yml

+---
+title: Fix link to job when creating a new issue from a failed job
+merge_request: 20328
+author:
+type: fixed

changelogs/unreleased/46963-add_readme_button_for_non_empty_project.yml

+---
+title: Add readme button to non-empty project page
+merge_request: 20104
+author:
+type: fixed

db/fixtures/development/20_nested_groups.rb

 require './spec/support/sidekiq'
 
-def create_group_with_parents(user, full_path)
-  parent_path = nil
-  group = nil
-
-  until full_path.blank?
-    path, _, full_path = full_path.partition('/')
-
-    if parent_path
-      parent = Group.find_by_full_path(parent_path)
-
-      parent_path += '/'
-      parent_path += path
-
-      group = Groups::CreateService.new(user, path: path, parent_id: parent.id).execute
-    else
-      parent_path = path
-
-      group = Group.find_by_full_path(parent_path) ||
-        Groups::CreateService.new(user, path: path).execute
-    end
-  end
-
-  group
-end
-
 Sidekiq::Testing.inline! do
   Gitlab::Seeder.quiet do
     flag = 'SEED_NESTED_GROUPS'
@@ -48,7 +23,8 @@ Sidekiq::Testing.inline! do
         full_path = url.sub('https://android.googlesource.com/', '')
         full_path = full_path.sub(/\.git\z/, '')
         full_path, _, project_path = full_path.rpartition('/')
-        group = Group.find_by_full_path(full_path) || create_group_with_parents(user, full_path)
+        group = Group.find_by_full_path(full_path) ||
+          Groups::NestedCreateService.new(user, group_path: full_path).execute
 
         params = {
           import_url: url,

db/post_migrate/20180619121030_enqueue_delete_diff_files_workers.rb

-class EnqueueDeleteDiffFilesWorkers < ActiveRecord::Migration
-  include Gitlab::Database::MigrationHelpers
-
-  class MergeRequestDiff < ActiveRecord::Base
-    self.table_name = 'merge_request_diffs'
-
-    belongs_to :merge_request
-
-    include EachBatch
-  end
-
-  DOWNTIME = false
-  BATCH_SIZE = 1000
-  MIGRATION = 'DeleteDiffFiles'
-  DELAY_INTERVAL = 8.minutes
-  TMP_INDEX = 'tmp_partial_diff_id_with_files_index'.freeze
-
-  disable_ddl_transaction!
-
-  def up
-    # We add temporary index, to make iteration over batches more performant.
-    # Conditional here is to avoid the need of doing that in a separate
-    # migration file to make this operation idempotent.
-    #
-    unless index_exists_by_name?(:merge_request_diffs, TMP_INDEX)
-      add_concurrent_index(:merge_request_diffs, :id, where: "(state NOT IN ('without_files', 'empty'))", name: TMP_INDEX)
-    end
-
-
-    diffs_with_files = MergeRequestDiff.where.not(state: ['without_files', 'empty'])
-
-    # explain (analyze, buffers) example for the iteration:
-    #
-    # Index Only Scan using tmp_index_20013 on merge_request_diffs  (cost=0.43..1630.19 rows=60567 width=4) (actual time=0.047..9.572 rows=56976 loops=1)
-    #   Index Cond: ((id >= 764586) AND (id < 835298))
-    #   Heap Fetches: 8
-    #   Buffers: shared hit=18188
-    # Planning time: 0.752 ms
-    # Execution time: 12.430 ms
-    #
-    diffs_with_files.each_batch(of: BATCH_SIZE) do |relation, outer_index|
-      ids = relation.pluck(:id)
-
-      ids.each_with_index do |diff_id, inner_index|
-        # This will give some space between batches of workers.
-        interval = DELAY_INTERVAL * outer_index + inner_index.minutes
-
-        # A single `merge_request_diff` can be associated with way too many
-        # `merge_request_diff_files`. It's better to avoid batching these and
-        # schedule one at a time.
-        #
-        # Considering roughly 6M jobs, this should take ~30 days to process all
-        # of them.
-        #
-        BackgroundMigrationWorker.perform_in(interval, MIGRATION, [diff_id])
-      end
-    end
-
-    # We remove temporary index, because it is not required during standard
-    # operations and runtime.
-    #
-    remove_concurrent_index_by_name(:merge_request_diffs, TMP_INDEX)
-  end
-
-  def down
-    if index_exists_by_name?(:merge_request_diffs, TMP_INDEX)
-      remove_concurrent_index_by_name(:merge_request_diffs, TMP_INDEX)
-    end
-  end
-end

spec/features/projects/jobs_spec.rb

@@ -165,7 +165,7 @@ feature 'Jobs', :clean_gitlab_redis_shared_state do
 
         it 'links to issues/new with the title and description filled in' do
           button_title = "Job Failed ##{job.id}"
-          job_url = project_job_path(project, job)
+          job_url = project_job_url(project, job, host: page.server.host, port: page.server.port)
           options = { issue: { title: button_title, description: "Job [##{job.id}](#{job_url}) failed for #{job.sha}:\n" } }
 
           href = new_project_issue_path(project, options)

spec/features/projects/show/user_sees_setup_shortcut_buttons_spec.rb

@@ -5,6 +5,8 @@ describe 'Projects > Show > User sees setup shortcut buttons' do
   # see spec/features/projects/files/project_owner_creates_license_file_spec.rb
   # see spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb
 
+  include FakeBlobHelpers
+
   let(:user) { create(:user) }
 
   describe 'empty project' do
@@ -141,11 +143,57 @@ describe 'Projects > Show > User sees setup shortcut buttons' do
         allow_any_instance_of(AutoDevopsHelper).to receive(:show_auto_devops_callout?).and_return(false)
         project.add_master(user)
         sign_in(user)
+      end
 
-        visit project_path(project)
+      context 'Readme button' do
+        before do
+          allow(Project).to receive(:find_by_full_path)
+                              .with(project.full_path, follow_redirects: true)
+                              .and_return(project)
+        end
+
+        context 'when the project has a populated Readme' do
+          it 'show the "Readme" anchor' do
+            visit project_path(project)
+
+            expect(project.repository.readme).not_to be_nil
+
+            page.within('.project-stats') do
+              expect(page).not_to have_link('Add Readme', href: presenter.add_readme_path)
+              expect(page).to have_link('Readme', href: presenter.readme_path)
+            end
+          end
+
+          context 'when the project has an empty Readme' do
+            it 'show the "Readme" anchor' do
+              allow(project.repository).to receive(:readme).and_return(fake_blob(path: 'README.md', data: '', size: 0))
+
+              visit project_path(project)
+
+              page.within('.project-stats') do
+                expect(page).not_to have_link('Add Readme', href: presenter.add_readme_path)
+                expect(page).to have_link('Readme', href: presenter.readme_path)
+              end
+            end
+          end
+        end
+
+        context 'when the project does not have a Readme' do
+          it 'shows the "Add Readme" button' do
+            allow(project.repository).to receive(:readme).and_return(nil)
+
+            visit project_path(project)
+
+            page.within('.project-stats') do
+              expect(page).to have_link('Add Readme', href: presenter.add_readme_path)
+            end
+          end
+        end
       end
 
       it 'no "Add Changelog" button if the project already has a changelog' do
+        visit project_path(project)
+
         expect(project.repository.changelog).not_to be_nil
 
         page.within('.project-stats') do
@@ -154,6 +202,8 @@ describe 'Projects > Show > User sees setup shortcut buttons' do
       end
 
       it 'no "Add License" button if the project already has a license' do
+        visit project_path(project)
+
         expect(project.repository.license_blob).not_to be_nil
 
         page.within('.project-stats') do
@@ -162,6 +212,8 @@ describe 'Projects > Show > User sees setup shortcut buttons' do
       end
 
       it 'no "Add Contribution guide" button if the project already has a contribution guide' do
+        visit project_path(project)
+
         expect(project.repository.contribution_guide).not_to be_nil
 
         page.within('.project-stats') do
@@ -171,6 +223,8 @@ describe 'Projects > Show > User sees setup shortcut buttons' do
 
       describe 'GitLab CI configuration button' do
         it '"Set up CI/CD" button linked to new file populated for a .gitlab-ci.yml' do
+          visit project_path(project)
+
           expect(project.repository.gitlab_ci_yml).to be_nil
 
           page.within('.project-stats') do
@@ -211,6 +265,8 @@ describe 'Projects > Show > User sees setup shortcut buttons' do
 
       describe 'Auto DevOps button' do
         it '"Enable Auto DevOps" button linked to settings page' do
+          visit project_path(project)
+
           page.within('.project-stats') do
             expect(page).to have_link('Enable Auto DevOps', href: project_settings_ci_cd_path(project, anchor: 'autodevops-settings'))
           end
@@ -263,6 +319,8 @@ describe 'Projects > Show > User sees setup shortcut buttons' do
 
       describe 'Kubernetes cluster button' do
         it '"Add Kubernetes cluster" button linked to clusters page' do
+          visit project_path(project)
+
           page.within('.project-stats') do
             expect(page).to have_link('Add Kubernetes cluster', href: new_project_cluster_path(project))
           end

spec/lib/gitlab/background_migration/delete_diff_files_spec.rb

 require 'spec_helper'
 
-describe Gitlab::BackgroundMigration::DeleteDiffFiles, :migration, schema: 20180619121030 do
+describe Gitlab::BackgroundMigration::DeleteDiffFiles, :migration, schema: 20180626125654 do
   describe '#perform' do
     context 'when diff files can be deleted' do
       let(:merge_request) { create(:merge_request, :merged) }

spec/migrations/enqueue_delete_diff_files_workers_spec.rb

-require 'spec_helper'
-require Rails.root.join('db', 'post_migrate', '20180619121030_enqueue_delete_diff_files_workers.rb')
-
-describe EnqueueDeleteDiffFilesWorkers, :migration, :sidekiq do
-  let(:merge_request_diffs) { table(:merge_request_diffs) }
-  let(:merge_requests) { table(:merge_requests) }
-  let(:namespaces) { table(:namespaces) }
-  let(:projects) { table(:projects) }
-
-  before do
-    stub_const("#{described_class.name}::BATCH_SIZE", 2)
-
-    namespaces.create!(id: 1, name: 'gitlab', path: 'gitlab')
-    projects.create!(id: 1, namespace_id: 1, name: 'gitlab', path: 'gitlab')
-
-    merge_requests.create!(id: 1, target_project_id: 1, source_project_id: 1, target_branch: 'feature', source_branch: 'master', state: 'merged')
-
-    merge_request_diffs.create!(id: 1, merge_request_id: 1, state: 'collected')
-    merge_request_diffs.create!(id: 2, merge_request_id: 1, state: 'without_files')
-    merge_request_diffs.create!(id: 3, merge_request_id: 1, state: 'collected')
-    merge_request_diffs.create!(id: 4, merge_request_id: 1, state: 'collected')
-    merge_request_diffs.create!(id: 5, merge_request_id: 1, state: 'empty')
-    merge_request_diffs.create!(id: 6, merge_request_id: 1, state: 'collected')
-
-    merge_requests.update(1, latest_merge_request_diff_id: 6)
-  end
-
-  it 'correctly schedules diff file deletion workers' do
-    Sidekiq::Testing.fake! do
-      Timecop.freeze do
-        migrate!
-
-        # 1st batch
-        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(8.minutes, 1)
-        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(9.minutes, 3)
-        # 2nd batch
-        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(16.minutes, 4)
-        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(17.minutes, 6)
-        expect(BackgroundMigrationWorker.jobs.size).to eq(4)
-      end
-    end
-  end
-
-  it 'migrates the data' do
-    expect { migrate! }.to change { merge_request_diffs.where(state: 'without_files').count }
-      .from(1).to(4)
-  end
-end