Merge branch 'master' into 11-1-stable-prepare-rc5

.gitlab-ci.yml

@@ -269,10 +269,10 @@ package-and-qa:
     <<: *single-script-job-variables
     SCRIPT_NAME: trigger-build-docs
   environment:
-    name: review-docs/$CI_COMMIT_REF_NAME
+    name: review-docs/$CI_COMMIT_REF_SLUG
     # DOCS_REVIEW_APPS_DOMAIN and DOCS_GITLAB_REPO_SUFFIX are secret variables
     # Discussion: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14236/diffs#note_40140693
-    url: http://$DOCS_GITLAB_REPO_SUFFIX-$CI_ENVIRONMENT_SLUG.$DOCS_REVIEW_APPS_DOMAIN/$DOCS_GITLAB_REPO_SUFFIX
+    url: http://$CI_ENVIRONMENT_SLUG.$DOCS_REVIEW_APPS_DOMAIN/$DOCS_GITLAB_REPO_SUFFIX
     on_stop: review-docs-cleanup
 
 # Trigger a manual docs build in gitlab-docs only on non docs-only branches.
@@ -307,7 +307,7 @@ review-docs-cleanup:
   <<: *review-docs
   stage: post-cleanup
   environment:
-    name: review-docs/$CI_COMMIT_REF_NAME
+    name: review-docs/$CI_COMMIT_REF_SLUG
     action: stop
   when: manual
   script:

app/serializers/diff_file_entity.rb

@@ -25,6 +25,8 @@ class DiffFileEntity < Grape::Entity
   expose :can_modify_blob do |diff_file|
     merge_request = options[:merge_request]
 
+    next unless diff_file.blob
+
     if merge_request&.source_project && current_user
       can_modify_blob?(diff_file.blob, merge_request.source_project, merge_request.source_branch)
     else
@@ -108,6 +110,7 @@ class DiffFileEntity < Grape::Entity
     project = merge_request.target_project
 
     next unless project
+    next unless diff_file.content_sha
 
     project_blob_path(project, tree_join(diff_file.content_sha, diff_file.new_path))
   end
@@ -125,6 +128,8 @@ class DiffFileEntity < Grape::Entity
   end
 
   expose :context_lines_path, if: -> (diff_file, _) { diff_file.text? } do |diff_file|
+    next unless diff_file.content_sha
+
     project_blob_diff_path(diff_file.repository.project, tree_join(diff_file.content_sha, diff_file.file_path))
   end
 

app/serializers/discussion_entity.rb

@@ -3,7 +3,7 @@ class DiscussionEntity < Grape::Entity
   include NotesHelper
 
   expose :id, :reply_id
-  expose :position, if: -> (d, _) { d.diff_discussion? }
+  expose :position, if: -> (d, _) { d.diff_discussion? && !d.legacy_diff_discussion? }
   expose :line_code, if: -> (d, _) { d.diff_discussion? }
   expose :expanded?, as: :expanded
   expose :active?, as: :active, if: -> (d, _) { d.diff_discussion? }

app/uploaders/attachment_uploader.rb

+# frozen_string_literal: true
+
 class AttachmentUploader < GitlabUploader
   include RecordsUploads::Concern
   include ObjectStorage::Concern

app/uploaders/avatar_uploader.rb

+# frozen_string_literal: true
+
 class AvatarUploader < GitlabUploader
   include UploaderHelper
   include RecordsUploads::Concern

app/uploaders/favicon_uploader.rb

+# frozen_string_literal: true
+
 class FaviconUploader < AttachmentUploader
   EXTENSION_WHITELIST = %w[png ico].freeze
 

app/uploaders/file_mover.rb

+# frozen_string_literal: true
+
 class FileMover
   attr_reader :secret, :file_name, :model, :update_field
 

app/uploaders/file_uploader.rb

+# frozen_string_literal: true
+
 # This class breaks the actual CarrierWave concept.
 # Every uploader should use a base_dir that is model agnostic so we can build
 # back URLs from base_dir-relative paths saved in the `Upload` model.
@@ -117,7 +119,7 @@ class FileUploader < GitlabUploader
   end
 
   def markdown_link
-    markdown = "[#{markdown_name}](#{secure_url})"
+    markdown = +"[#{markdown_name}](#{secure_url})"
     markdown.prepend("!") if image_or_video? || dangerous?
     markdown
   end

app/uploaders/gitlab_uploader.rb

+# frozen_string_literal: true
+
 class GitlabUploader < CarrierWave::Uploader::Base
   class_attribute :options
 

app/uploaders/job_artifact_uploader.rb

+# frozen_string_literal: true
+
 class JobArtifactUploader < GitlabUploader
   extend Workhorse::UploadPath
   include ObjectStorage::Concern

app/uploaders/legacy_artifact_uploader.rb

+# frozen_string_literal: true
+
 class LegacyArtifactUploader < GitlabUploader
   extend Workhorse::UploadPath
   include ObjectStorage::Concern

app/uploaders/lfs_object_uploader.rb

+# frozen_string_literal: true
+
 class LfsObjectUploader < GitlabUploader
   extend Workhorse::UploadPath
   include ObjectStorage::Concern

app/uploaders/namespace_file_uploader.rb

+# frozen_string_literal: true
+
 class NamespaceFileUploader < FileUploader
   # Re-Override
   def self.root

app/uploaders/object_storage.rb

+# frozen_string_literal: true
+
 require 'fog/aws'
 require 'carrierwave/storage/fog'
 

app/uploaders/personal_file_uploader.rb

+# frozen_string_literal: true
+
 class PersonalFileUploader < FileUploader
   # Re-Override
   def self.root

app/uploaders/records_uploads.rb

+# frozen_string_literal: true
+
 module RecordsUploads
   module Concern
     extend ActiveSupport::Concern

app/uploaders/uploader_helper.rb

+# frozen_string_literal: true
+
 # Extra methods for uploader
 module UploaderHelper
   IMAGE_EXT = %w[png jpg jpeg gif bmp tiff ico].freeze

app/uploaders/workhorse.rb

+# frozen_string_literal: true
+
 module Workhorse
   module UploadPath
     def workhorse_upload_path

app/views/projects/snippets/_actions.html.haml

@@ -3,34 +3,34 @@
 .d-none.d-sm-block
   - if can?(current_user, :update_project_snippet, @snippet)
     = link_to edit_project_snippet_path(@project, @snippet), class: "btn btn-grouped" do
-      Edit
+      = _('Edit')
   - if can?(current_user, :update_project_snippet, @snippet)
-    = link_to project_snippet_path(@project, @snippet), method: :delete, data: { confirm: "Are you sure?" }, class: "btn btn-grouped btn-inverted btn-remove", title: 'Delete Snippet' do
-      Delete
+    = link_to project_snippet_path(@project, @snippet), method: :delete, data: { confirm: _("Are you sure?") }, class: "btn btn-grouped btn-inverted btn-remove", title: _('Delete Snippet') do
+      = _('Delete')
   - if can?(current_user, :create_project_snippet, @project)
-    = link_to new_project_snippet_path(@project), class: 'btn btn-grouped btn-inverted btn-create', title: "New snippet" do
-      New snippet
+    = link_to new_project_snippet_path(@project), class: 'btn btn-grouped btn-inverted btn-create', title: _("New snippet") do
+      = _('New snippet')
   - if @snippet.submittable_as_spam_by?(current_user)
-    = link_to 'Submit as spam', mark_as_spam_project_snippet_path(@project, @snippet), method: :post, class: 'btn btn-grouped btn-spam', title: 'Submit as spam'
+    = link_to _('Submit as spam'), mark_as_spam_project_snippet_path(@project, @snippet), method: :post, class: 'btn btn-grouped btn-spam', title: _('Submit as spam')
 - if can?(current_user, :create_project_snippet, @project) || can?(current_user, :update_project_snippet, @snippet)
   .d-block.d-sm-none.dropdown
     %button.btn.btn-default.btn-block.append-bottom-0.prepend-top-5{ data: { toggle: "dropdown" } }
-      Options
+      = _('Options')
       = icon('caret-down')
     .dropdown-menu.dropdown-menu-full-width
       %ul
         - if can?(current_user, :create_project_snippet, @project)
           %li
-            = link_to new_project_snippet_path(@project), title: "New snippet" do
-              New snippet
+            = link_to new_project_snippet_path(@project), title: _("New snippet") do
+              = _('New snippet')
         - if can?(current_user, :update_project_snippet, @snippet)
           %li
-            = link_to project_snippet_path(@project, @snippet), method: :delete, data: { confirm: "Are you sure?" }, title: 'Delete Snippet' do
-              Delete
+            = link_to project_snippet_path(@project, @snippet), method: :delete, data: { confirm: _("Are you sure?") }, title: _('Delete Snippet') do
+              = _('Delete')
         - if can?(current_user, :update_project_snippet, @snippet)
           %li
             = link_to edit_project_snippet_path(@project, @snippet) do
-              Edit
+              = _('Edit')
         - if @snippet.submittable_as_spam_by?(current_user)
           %li
-            = link_to 'Submit as spam', mark_as_spam_project_snippet_path(@project, @snippet), method: :post
+            = link_to _('Submit as spam'), mark_as_spam_project_snippet_path(@project, @snippet), method: :post

app/views/projects/snippets/edit.html.haml

-- add_to_breadcrumbs "Snippets", project_snippets_path(@project)
+- add_to_breadcrumbs _("Snippets"), project_snippets_path(@project)
 - breadcrumb_title @snippet.to_reference
-- page_title "Edit", "#{@snippet.title} (#{@snippet.to_reference})", "Snippets"
+- page_title _("Edit"), "#{@snippet.title} (#{@snippet.to_reference})", _("Snippets")
 
 %h3.page-title
-  Edit Snippet
+  = _("Edit Snippet")
 %hr
 = render "shared/snippets/form", url: project_snippet_path(@project, @snippet)

app/views/projects/snippets/index.html.haml

-- page_title "Snippets"
+- page_title _("Snippets")
 
 - if current_user
   .top-area
@@ -7,6 +7,6 @@
 
     .nav-controls
       - if can?(current_user, :create_project_snippet, @project)
-        = link_to "New snippet", new_project_snippet_path(@project), class: "btn btn-new", title: "New snippet"
+        = link_to _("New snippet"), new_project_snippet_path(@project), class: "btn btn-new", title: _("New snippet")
 
 = render 'snippets/snippets'

app/views/projects/snippets/new.html.haml

-- add_to_breadcrumbs "Snippets", project_snippets_path(@project)
-- breadcrumb_title "New"
-- page_title "New Snippets"
+- add_to_breadcrumbs _("Snippets"), project_snippets_path(@project)
+- breadcrumb_title _("New")
+- page_title _("New Snippets")
 
 %h3.page-title
-  New Snippet
+  = _('New Snippet')
 %hr
 = render "shared/snippets/form", url: project_snippets_path(@project, @snippet)

app/views/projects/snippets/show.html.haml

 - @content_class = "limit-container-width limited-inner-width-container" unless fluid_layout
-- add_to_breadcrumbs "Snippets", project_snippets_path(@project)
+- add_to_breadcrumbs _("Snippets"), project_snippets_path(@project)
 - breadcrumb_title @snippet.to_reference
-- page_title "#{@snippet.title} (#{@snippet.to_reference})", "Snippets"
+- page_title "#{@snippet.title} (#{@snippet.to_reference})", _("Snippets")
 
 = render 'shared/snippets/header'
 

changelogs/no-rm-rf-gitlab-basics.yml

----
-  title: Do not use '-f' with 'rm' in gitlab-basics docs
-  merge_request: 18027
-  author: Elias Werberich
-  type: changed

changelogs/unreleased/frozen-string-enable-app-uploaders.yml

+---
+title: Enable frozen string in apps/validators/*.rb
+merge_request: 20382
+author: gfyoung
+type: other

lib/gitlab/diff/file.rb

@@ -247,6 +247,7 @@ module Gitlab
         lines = highlighted_diff_lines
 
         return if lines.empty?
+        return if blob.nil?
 
         last_line = lines.last
 

lib/gitlab/gitaly_client.rb

@@ -429,7 +429,7 @@ module Gitlab
     def self.count_stack
       return unless RequestStore.active?
 
-      stack_string = caller.drop(1).join("\n")
+      stack_string = Gitlab::Profiler.clean_backtrace(caller).drop(1).join("\n")
 
       RequestStore.store[:stack_counter] ||= Hash.new
 

locale/gitlab.pot

@@ -8,8 +8,8 @@ msgid ""
 msgstr ""
 "Project-Id-Version: gitlab 1.0.0\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2018-07-01 16:35+1000\n"
-"PO-Revision-Date: 2018-07-01 16:35+1000\n"
+"POT-Creation-Date: 2018-07-01 21:24+1000\n"
+"PO-Revision-Date: 2018-07-01 21:24+1000\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"
 "Language: \n"
@@ -1815,6 +1815,9 @@ msgstr ""
 msgid "Delete"
 msgstr ""
 
+msgid "Delete Snippet"
+msgstr ""
+
 msgid "Delete list"
 msgstr ""
 
@@ -2036,6 +2039,9 @@ msgstr ""
 msgid "Edit Pipeline Schedule %{id}"
 msgstr ""
 
+msgid "Edit Snippet"
+msgstr ""
+
 msgid "Edit files in the editor and commit changes here"
 msgstr ""
 
@@ -2978,6 +2984,9 @@ msgstr ""
 msgid "Nav|Sign out and sign in with a different account"
 msgstr ""
 
+msgid "New"
+msgstr ""
+
 msgid "New Identity"
 msgstr ""
 
@@ -2998,6 +3007,12 @@ msgstr ""
 msgid "New Pipeline Schedule"
 msgstr ""
 
+msgid "New Snippet"
+msgstr ""
+
+msgid "New Snippets"
+msgstr ""
+
 msgid "New branch"
 msgstr ""
 
@@ -4301,6 +4316,9 @@ msgstr ""
 msgid "Subgroups"
 msgstr ""
 
+msgid "Submit as spam"
+msgstr ""
+
 msgid "Subscribe"
 msgstr ""
 

qa/qa/page/merge_request/show.rb

@@ -20,14 +20,6 @@ module QA
           element :squash_checkbox
         end
 
-        def rebase!
-          click_element :mr_rebase_button
-
-          wait(reload: false) do
-            has_text?('Fast-forward merge without a merge commit')
-          end
-        end
-
         def fast_forward_possible?
           !has_text?('Fast-forward merge is not possible')
         end
@@ -38,7 +30,35 @@ module QA
           has_selector?('.accept-merge-request')
         end
 
+        def rebase!
+          # The rebase button is disabled on load
+          wait do
+            has_css?(element_selector_css(:mr_rebase_button))
+          end
+
+          # The rebase button is enabled via JS
+          wait(reload: false) do
+            !first(element_selector_css(:mr_rebase_button)).disabled?
+          end
+
+          click_element :mr_rebase_button
+
+          wait(reload: false) do
+            has_text?('Fast-forward merge without a merge commit')
+          end
+        end
+
         def merge!
+          # The merge button is disabled on load
+          wait do
+            has_css?(element_selector_css(:merge_button))
+          end
+
+          # The merge button is enabled via JS
+          wait(reload: false) do
+            !first(element_selector_css(:merge_button)).disabled?
+          end
+
           click_element :merge_button
 
           wait(reload: false) do
@@ -47,10 +67,16 @@ module QA
         end
 
         def mark_to_squash
-          wait(reload: true) do
+          # The squash checkbox is disabled on load
+          wait do
             has_css?(element_selector_css(:squash_checkbox))
           end
 
+          # The squash checkbox is enabled via JS
+          wait(reload: false) do
+            !first(element_selector_css(:squash_checkbox)).disabled?
+          end
+
           click_element :squash_checkbox
         end
       end

qa/qa/specs/features/merge_request/squash_spec.rb

@@ -24,6 +24,8 @@ module QA
 
       merge_request.visit!
 
+      expect(page).to have_text('to be squashed')
+
       Page::MergeRequest::Show.perform do |merge_request_page|
         merge_request_page.mark_to_squash
         merge_request_page.merge!

scripts/trigger-build-docs

@@ -16,18 +16,14 @@ end
 GITLAB_DOCS_REPO = 'gitlab-com/gitlab-docs'.freeze
 
 #
-# Truncate the remote docs branch name if it's more than 63 characters
-# otherwise we hit the filesystem limit and the directory name where
-# NGINX serves the site won't match the branch name.
+# Truncate the remote docs branch name otherwise we hit the filesystem
+# limit and the directory name where NGINX serves the site won't match
+# the branch name.
 #
 def docs_branch
   # The maximum string length a file can have on a filesystem (ext4)
-  # is 63 characters. Let's use something smaller to be 100% sure.
-  max = 42
-  # Prefix the remote branch with the slug of the project in order
-  # to avoid name conflicts in the rare case the branch name already
-  # exists in the docs repo and truncate to max length.
-  "#{slug}-#{ENV["CI_ENVIRONMENT_SLUG"]}"[0...max]
+  # is 63 characters. CI_ENVIRONMENT_SLUG is limited to 24 characters.
+  ENV["CI_ENVIRONMENT_SLUG"]
 end
 
 #

spec/serializers/diff_file_entity_spec.rb

@@ -25,6 +25,20 @@ describe DiffFileEntity do
         :context_lines_path
       )
     end
+
+    # Converted diff files from GitHub import does not contain blob file
+    # and content sha.
+    context 'when diff file does not have a blob and content sha' do
+      it 'exposes some attributes as nil' do
+        allow(diff_file).to receive(:content_sha).and_return(nil)
+        allow(diff_file).to receive(:blob).and_return(nil)
+
+        expect(subject[:context_lines_path]).to be_nil
+        expect(subject[:view_path]).to be_nil
+        expect(subject[:highlighted_diff_lines]).to be_nil
+        expect(subject[:can_modify_blob]).to be_nil
+      end
+    end
   end
 
   context 'when there is no merge request' do

spec/serializers/discussion_entity_spec.rb

@@ -36,6 +36,25 @@ describe DiscussionEntity do
     )
   end
 
+  context 'when is LegacyDiffDiscussion' do
+    let(:project) { create(:project) }
+    let(:merge_request) { create(:merge_request, source_project: project) }
+    let(:discussion) { create(:legacy_diff_note_on_merge_request, noteable: merge_request, project: project).to_discussion }
+
+    it 'exposes correct attributes' do
+      expect(subject.keys.sort).to include(
+        :diff_discussion,
+        :expanded,
+        :id,
+        :individual_note,
+        :notes,
+        :discussion_path,
+        :for_commit,
+        :commit_id
+      )
+    end
+  end
+
   context 'when diff file is present' do
     let(:note) { create(:diff_note_on_merge_request) }
 

spec/services/notification_recipient_service_spec.rb

@@ -12,15 +12,15 @@ describe NotificationRecipientService do
 
     def create_watcher
       watcher = create(:user)
-      create(:notification_setting, project: project, user: watcher, level: :watch)
+      create(:notification_setting, source: project, user: watcher, level: :watch)
 
       other_projects.each do |other_project|
-        create(:notification_setting, project: other_project, user: watcher, level: :watch)
+        create(:notification_setting, source: other_project, user: watcher, level: :watch)
       end
     end
 
     it 'avoids N+1 queries', :request_store do
-      Gitlab::GitalyClient.allow_n_plus_1_calls { create_watcher }
+      create_watcher
 
       service.build_new_note_recipients(note)
 
@@ -28,7 +28,7 @@ describe NotificationRecipientService do
         service.build_new_note_recipients(note)
       end
 
-      Gitlab::GitalyClient.allow_n_plus_1_calls { create_watcher }
+      create_watcher
 
       expect { service.build_new_note_recipients(note) }.not_to exceed_query_limit(control_count)
     end