Merge branch 'master' into 'bootstrap4'

# Conflicts:
#   app/views/projects/branches/_branch.html.haml

17 files changed, 132 insertions, 102 deletions

diff --git a/app/assets/javascripts/notes/components/discussion_counter.vue b/app/assets/javascripts/notes/components/discussion_counter.vue
index a75af1e5322..a6d5adaff12 100644
--- a/app/assets/javascripts/notes/components/discussion_counter.vue
+++ b/app/assets/javascripts/notes/components/discussion_counter.vue
@@ -86,7 +86,7 @@ export default {
             v-html="resolveSvg"
           ></span>
         </span>
-        <span class=".line-resolve-text">
+        <span class="line-resolve-text">
           {{ resolvedDiscussionCount }}/{{ discussionCount }} {{ countText }} resolved
         </span>
       </div>
diff --git a/app/assets/javascripts/sidebar/lib/sidebar_move_issue.js b/app/assets/javascripts/sidebar/lib/sidebar_move_issue.js
index 1eadebc7004..b267422cd97 100644
--- a/app/assets/javascripts/sidebar/lib/sidebar_move_issue.js
+++ b/app/assets/javascripts/sidebar/lib/sidebar_move_issue.js
@@ -1,4 +1,5 @@
 import $ from 'jquery';
+import _ from 'underscore';
 
 function isValidProjectId(id) {
   return id > 0;
@@ -43,7 +44,7 @@ class SidebarMoveIssue {
       renderRow: project => `
         <li>
           <a href="#" class="js-move-issue-dropdown-item">
-            ${project.name_with_namespace}
+            ${_.escape(project.name_with_namespace)}
           </a>
         </li>
       `,
diff --git a/app/assets/javascripts/vue_shared/components/identicon.vue b/app/assets/javascripts/vue_shared/components/identicon.vue
index 0a30f467b08..23010f40f26 100644
--- a/app/assets/javascripts/vue_shared/components/identicon.vue
+++ b/app/assets/javascripts/vue_shared/components/identicon.vue
@@ -17,7 +17,7 @@ export default {
   },
   computed: {
     /**
-     * This method is based on app/helpers/application_helper.rb#project_identicon
+     * This method is based on app/helpers/avatars_helper.rb#project_identicon
      */
     identiconStyles() {
       const allowedColors = [
diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss
index 08a14f0a8a8..f568c5d7036 100644
--- a/app/assets/stylesheets/pages/notes.scss
+++ b/app/assets/stylesheets/pages/notes.scss
@@ -776,7 +776,3 @@ ul.notes {
     height: auto;
   }
 }
-
-.line-resolve-text {
-  vertical-align: middle;
-}
diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb
index 9137bc92810..40d9fa18a10 100644
--- a/app/controllers/omniauth_callbacks_controller.rb
+++ b/app/controllers/omniauth_callbacks_controller.rb
@@ -8,8 +8,8 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
     omniauth_flow(Gitlab::Auth::OAuth)
   end
 
-  Gitlab.config.omniauth.providers.each do |provider|
-    alias_method provider['name'], :handle_omniauth
+  AuthHelper.providers_for_base_controller.each do |provider|
+    alias_method provider, :handle_omniauth
   end
 
   # Extend the standard implementation to also increment
diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb
index 86c50d88a2a..bc13b8ad7ba 100644
--- a/app/controllers/projects/notes_controller.rb
+++ b/app/controllers/projects/notes_controller.rb
@@ -33,9 +33,7 @@ class Projects::NotesController < Projects::ApplicationController
   def resolve
     return render_404 unless note.resolvable?
 
-    note.resolve!(current_user)
-
-    MergeRequests::ResolvedDiscussionNotificationService.new(project, current_user).execute(note.noteable)
+    Notes::ResolveService.new(project, current_user).execute(note)
 
     discussion = note.discussion
 
diff --git a/app/finders/groups_finder.rb b/app/finders/groups_finder.rb
index 0282b378d88..0754123a3cf 100644
--- a/app/finders/groups_finder.rb
+++ b/app/finders/groups_finder.rb
@@ -39,7 +39,7 @@ class GroupsFinder < UnionFinder
 
   def all_groups
     return [owned_groups] if params[:owned]
-    return [Group.all] if current_user&.full_private_access?
+    return [Group.all] if current_user&.full_private_access? && all_available?
 
     groups = []
     groups << Gitlab::GroupHierarchy.new(groups_for_ancestors, groups_for_descendants).all_groups if current_user
@@ -67,6 +67,10 @@ class GroupsFinder < UnionFinder
   end
 
   def include_public_groups?
-    current_user.nil? || params.fetch(:all_available, true)
+    current_user.nil? || all_available?
+  end
+
+  def all_available?
+    params.fetch(:all_available, true)
   end
 end
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 228c8d2e8f9..6aa307b4db4 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -32,80 +32,6 @@ module ApplicationHelper
     args.any? { |v| v.to_s.downcase == action_name }
   end
 
-  def project_icon(project_id, options = {})
-    project =
-      if project_id.respond_to?(:avatar_url)
-        project_id
-      else
-        Project.find_by_full_path(project_id)
-      end
-
-    if project.avatar_url
-      image_tag project.avatar_url, options
-    else # generated icon
-      project_identicon(project, options)
-    end
-  end
-
-  def project_identicon(project, options = {})
-    allowed_colors = {
-      red: 'FFEBEE',
-      purple: 'F3E5F5',
-      indigo: 'E8EAF6',
-      blue: 'E3F2FD',
-      teal: 'E0F2F1',
-      orange: 'FBE9E7',
-      gray: 'EEEEEE'
-    }
-
-    options[:class] ||= ''
-    options[:class] << ' identicon'
-    bg_key = project.id % 7
-    style = "background-color: ##{allowed_colors.values[bg_key]}; color: #555"
-
-    content_tag(:div, class: options[:class], style: style) do
-      project.name[0, 1].upcase
-    end
-  end
-
-  # Takes both user and email and returns the avatar_icon by
-  # user (preferred) or email.
-  def avatar_icon_for(user = nil, email = nil, size = nil, scale = 2, only_path: true)
-    if user
-      avatar_icon_for_user(user, size, scale, only_path: only_path)
-    elsif email
-      avatar_icon_for_email(email, size, scale, only_path: only_path)
-    else
-      default_avatar
-    end
-  end
-
-  def avatar_icon_for_email(email = nil, size = nil, scale = 2, only_path: true)
-    user = User.find_by_any_email(email.try(:downcase))
-    if user
-      avatar_icon_for_user(user, size, scale, only_path: only_path)
-    else
-      gravatar_icon(email, size, scale)
-    end
-  end
-
-  def avatar_icon_for_user(user = nil, size = nil, scale = 2, only_path: true)
-    if user
-      user.avatar_url(size: size, only_path: only_path) || default_avatar
-    else
-      gravatar_icon(nil, size, scale)
-    end
-  end
-
-  def gravatar_icon(user_email = '', size = nil, scale = 2)
-    GravatarService.new.execute(user_email, size, scale) ||
-      default_avatar
-  end
-
-  def default_avatar
-    asset_path('no_avatar.png')
-  end
-
   def last_commit(project)
     if project.repo_exists?
       time_ago_with_tooltip(project.repository.commit.committed_date)
diff --git a/app/helpers/auth_helper.rb b/app/helpers/auth_helper.rb
index c109954f3a3..d2daee22aba 100644
--- a/app/helpers/auth_helper.rb
+++ b/app/helpers/auth_helper.rb
@@ -1,6 +1,6 @@
 module AuthHelper
   PROVIDERS_WITH_ICONS = %w(twitter github gitlab bitbucket google_oauth2 facebook azure_oauth2 authentiq).freeze
-  FORM_BASED_PROVIDERS = [/\Aldap/, 'crowd'].freeze
+  LDAP_PROVIDER = /\Aldap/
 
   def ldap_enabled?
     Gitlab::Auth::LDAP::Config.enabled?
@@ -23,7 +23,7 @@ module AuthHelper
   end
 
   def form_based_provider?(name)
-    FORM_BASED_PROVIDERS.any? { |pattern| pattern === name.to_s }
+    [LDAP_PROVIDER, 'crowd'].any? { |pattern| pattern === name.to_s }
   end
 
   def form_based_providers
@@ -38,6 +38,10 @@ module AuthHelper
     auth_providers.reject { |provider| form_based_provider?(provider) }
   end
 
+  def providers_for_base_controller
+    auth_providers.reject { |provider| LDAP_PROVIDER === provider }
+  end
+
   def enabled_button_based_providers
     disabled_providers = Gitlab::CurrentSettings.disabled_oauth_sign_in_sources || []
 
diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb
index 8d96569bbb0..43d92bde064 100644
--- a/app/helpers/avatars_helper.rb
+++ b/app/helpers/avatars_helper.rb
@@ -1,4 +1,78 @@
 module AvatarsHelper
+  def project_icon(project_id, options = {})
+    project =
+      if project_id.respond_to?(:avatar_url)
+        project_id
+      else
+        Project.find_by_full_path(project_id)
+      end
+
+    if project.avatar_url
+      image_tag project.avatar_url, options
+    else # generated icon
+      project_identicon(project, options)
+    end
+  end
+
+  def project_identicon(project, options = {})
+    allowed_colors = {
+      red: 'FFEBEE',
+      purple: 'F3E5F5',
+      indigo: 'E8EAF6',
+      blue: 'E3F2FD',
+      teal: 'E0F2F1',
+      orange: 'FBE9E7',
+      gray: 'EEEEEE'
+    }
+
+    options[:class] ||= ''
+    options[:class] << ' identicon'
+    bg_key = project.id % 7
+    style = "background-color: ##{allowed_colors.values[bg_key]}; color: #555"
+
+    content_tag(:div, class: options[:class], style: style) do
+      project.name[0, 1].upcase
+    end
+  end
+
+  # Takes both user and email and returns the avatar_icon by
+  # user (preferred) or email.
+  def avatar_icon_for(user = nil, email = nil, size = nil, scale = 2, only_path: true)
+    if user
+      avatar_icon_for_user(user, size, scale, only_path: only_path)
+    elsif email
+      avatar_icon_for_email(email, size, scale, only_path: only_path)
+    else
+      default_avatar
+    end
+  end
+
+  def avatar_icon_for_email(email = nil, size = nil, scale = 2, only_path: true)
+    user = User.find_by_any_email(email.try(:downcase))
+    if user
+      avatar_icon_for_user(user, size, scale, only_path: only_path)
+    else
+      gravatar_icon(email, size, scale)
+    end
+  end
+
+  def avatar_icon_for_user(user = nil, size = nil, scale = 2, only_path: true)
+    if user
+      user.avatar_url(size: size, only_path: only_path) || default_avatar
+    else
+      gravatar_icon(nil, size, scale)
+    end
+  end
+
+  def gravatar_icon(user_email = '', size = nil, scale = 2)
+    GravatarService.new.execute(user_email, size, scale) ||
+      default_avatar
+  end
+
+  def default_avatar
+    ActionController::Base.helpers.image_path('no_avatar.png')
+  end
+
   def author_avatar(commit_or_event, options = {})
     user_avatar(options.merge({
       user: commit_or_event.author,
diff --git a/app/helpers/system_note_helper.rb b/app/helpers/system_note_helper.rb
index 00fe67d6ffb..5b4a141dbcf 100644
--- a/app/helpers/system_note_helper.rb
+++ b/app/helpers/system_note_helper.rb
@@ -1,14 +1,14 @@
 module SystemNoteHelper
   ICON_NAMES_BY_ACTION = {
     'commit' => 'commit',
-    'description' => 'pencil',
+    'description' => 'pencil-square',
     'merge' => 'git-merge',
     'merged' => 'git-merge',
     'opened' => 'issue-open',
     'closed' => 'issue-close',
     'time_tracking' => 'timer',
     'assignee' => 'user',
-    'title' => 'pencil',
+    'title' => 'pencil-square',
     'task' => 'task-done',
     'label' => 'label',
     'cross_reference' => 'comment-dots',
@@ -18,7 +18,7 @@ module SystemNoteHelper
     'milestone' => 'clock',
     'discussion' => 'comment',
     'moved' => 'arrow-right',
-    'outdated' => 'pencil',
+    'outdated' => 'pencil-square',
     'duplicate' => 'issue-duplicate',
     'locked' => 'lock',
     'unlocked' => 'lock-open'
diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb
index e4212775956..3646e08a15f 100644
--- a/app/mailers/notify.rb
+++ b/app/mailers/notify.rb
@@ -16,6 +16,7 @@ class Notify < BaseMailer
   helper BlobHelper
   helper EmailsHelper
   helper MembersHelper
+  helper AvatarsHelper
   helper GitlabRoutingHelper
 
   def test_email(recipient_email, subject, body)
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 9750e9298ec..32f3d90595a 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -105,6 +105,10 @@ class Commit
         end
       end
     end
+
+    def parent_class
+      ::Project
+    end
   end
 
   attr_accessor :raw
diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb
index 15122cbc693..616a626419b 100644
--- a/app/models/diff_note.rb
+++ b/app/models/diff_note.rb
@@ -54,7 +54,20 @@ class DiffNote < Note
   end
 
   def diff_file
-    @diff_file ||= self.original_position.diff_file(self.project.repository)
+    @diff_file ||=
+      begin
+        if created_at_diff?(noteable.diff_refs)
+          # We're able to use the already persisted diffs (Postgres) if we're
+          # presenting a "current version" of the MR discussion diff.
+          # So no need to make an extra Gitaly diff request for it.
+          # As an extra benefit, the returned `diff_file` already
+          # has `highlighted_diff_lines` data set from Redis on
+          # `Diff::FileCollection::MergeRequestDiff`.
+          noteable.diffs(paths: original_position.paths, expanded: true).diff_files.first
+        else
+          original_position.diff_file(self.project.repository)
+        end
+      end
   end
 
   def diff_line
diff --git a/app/services/notes/resolve_service.rb b/app/services/notes/resolve_service.rb
new file mode 100644
index 00000000000..0db8ee809a9
--- /dev/null
+++ b/app/services/notes/resolve_service.rb
@@ -0,0 +1,9 @@
+module Notes
+  class ResolveService < ::BaseService
+    def execute(note)
+      note.resolve!(current_user)
+
+      ::MergeRequests::ResolvedDiscussionNotificationService.new(project, current_user).execute(note.noteable)
+    end
+  end
+end
diff --git a/app/services/repository_archive_clean_up_service.rb b/app/services/repository_archive_clean_up_service.rb
index 9a88459b511..ba7be4b3f89 100644
--- a/app/services/repository_archive_clean_up_service.rb
+++ b/app/services/repository_archive_clean_up_service.rb
@@ -20,11 +20,12 @@ class RepositoryArchiveCleanUpService
   private
 
   def clean_up_old_archives
-    run(%W(find #{path} -not -path #{path} -type f \( -name \*.tar -o -name \*.bz2 -o -name \*.tar.gz -o -name \*.zip \) -maxdepth 2 -mmin +#{mmin} -delete))
+    run(%W(find #{path} -mindepth 1 -maxdepth 3 -type f \( -name \*.tar -o -name \*.bz2 -o -name \*.tar.gz -o -name \*.zip \) -mmin +#{mmin} -delete))
   end
 
   def clean_up_empty_directories
-    run(%W(find #{path} -not -path #{path} -type d -empty -name \*.git -maxdepth 1 -delete))
+    run(%W(find #{path} -mindepth 2 -maxdepth 2 -type d -empty -delete))
+    run(%W(find #{path} -mindepth 1 -maxdepth 1 -type d -empty -delete))
   end
 
   def run(cmd)
diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml
index 54767bdf757..85cde721607 100644
--- a/app/views/projects/branches/_branch.html.haml
+++ b/app/views/projects/branches/_branch.html.haml
@@ -8,18 +8,17 @@
 %li{ class: "branch-item js-branch-#{branch.name}" }
   .branch-info
     .branch-title
-      = link_to project_tree_path(@project, branch.name), class: 'item-title str-truncated-100 ref-name' do
-        = sprite_icon('fork', size: 12)
+      = sprite_icon('fork', size: 12)
+      = link_to project_tree_path(@project, branch.name), class: 'item-title str-truncated-100 ref-name prepend-left-8' do
         = branch.name
-      &nbsp;
       - if branch.name == @repository.root_ref
-        %span.badge.badge-primary default
+        %span.badge.badge-primary.prepend-left-5 default
       - elsif merged
-        %span.badge.badge-info.has-tooltip{ title: s_('Branches|Merged into %{default_branch}') % { default_branch: @repository.root_ref } }
+        %span.badge.badge-info.has-tooltip.prepend-left-5{ title: s_('Branches|Merged into %{default_branch}') % { default_branch: @repository.root_ref } }
           = s_('Branches|merged')
 
       - if protected_branch?(@project, branch)
-        %span.badge.badge-success
+        %span.badge.badge-success.prepend-left-5
           = s_('Branches|protected')
 
     .block-truncated