Merge branch 'master' of gitlab.com:gitlab-org/gitlab-ee into feature/improve-weight-filter

# Conflicts:
#	app/views/shared/issuable/_sidebar.html.haml

CHANGELOG-EE

@@ -15,6 +15,9 @@ v 8.5.0
   - Fix of Elastic indexer. Stabilze indexer when serialized data is corrupted
   - [Elastic] Don't index unnecessary data into elastic
 
+v 8.4.5
+  - Update LDAP groups asynchronously
+
 v 8.4.4
   - Re-introduce "Send email to users" link in Admin area
   - Fix category values for Jenkins and JenkinsDeprecated services

app/assets/stylesheets/pages/issuable.scss

@@ -203,14 +203,7 @@
       overflow: hidden;
     }
 
-    .issuable-count,
-    .issuable-nav,
-    .assignee > *,
-    .milestone > *,
-    .labels > *,
-    .participants > *,
-    .light > *,
-    .project-reference > * {
+    .hide-collapsed {
       display: none;
     }
 

app/views/shared/issuable/_participants.html.haml

@@ -3,7 +3,8 @@
     = icon('users')
     %span
       = participants.count
-  .title
+  .title.hide-collapsed
     = pluralize participants.count, "participant"
   - participants.each do |participant|
-    = link_to_member(@project, participant, name: false, size: 24)
+    %span.hide-collapsed
+      = link_to_member(@project, participant, name: false, size: 24)

app/views/shared/issuable/_sidebar.html.haml

 %aside.right-sidebar{ class: sidebar_gutter_collapsed_class }
   .issuable-sidebar
     .block
-      %span.issuable-count.pull-left
+      %span.issuable-count.pull-left.hide-collapsed
         = issuable.iid
         of
         = issuables_count(issuable)
       %span.pull-right
         %a.gutter-toggle{href: '#'}
           = sidebar_gutter_toggle_icon
-      .issuable-nav.pull-right.btn-group{role: 'group', "aria-label" => '...'}
+      .issuable-nav.pull-right.btn-group.hide-collapsed{role: 'group', "aria-label" => '...'}
         - if prev_issuable = prev_issuable_for(issuable)
           = link_to 'Prev', [@project.namespace.becomes(Namespace), @project, prev_issuable], class: 'btn btn-default prev-btn'
         - else
@@ -27,13 +27,13 @@
             = link_to_member_avatar(issuable.assignee, size: 24)
           - else
             = icon('user')
-        .title
+        .title.hide-collapsed
           %label
             Assignee
           - if can?(current_user, :"admin_#{issuable.to_ability_name}", @project)
             .pull-right
               = link_to 'Edit', '#', class: 'edit-link'
-        .value
+        .value.hide-collapsed
           - if issuable.assignee
             %strong= link_to_member(@project, issuable.assignee, size: 24)
             - if issuable.instance_of?(MergeRequest) && !issuable.can_be_merged_by?(issuable.assignee)
@@ -42,7 +42,7 @@
           - else
             .light None
 
-        .selectbox
+        .selectbox.hide-collapsed
           = users_select_tag("#{issuable.class.table_name.singularize}[assignee_id]", placeholder: 'Select assignee', class: 'custom-form-control js-select2 js-assignee', selected: issuable.assignee_id, project: @target_project, null_user: true, current_user: true, first_user: true)
 
       .block.milestone
@@ -53,13 +53,13 @@
               = issuable.milestone.title
             - else
               No
-        .title
+        .title.hide-collapsed
           %label
             Milestone
           - if can?(current_user, :"admin_#{issuable.to_ability_name}", @project)
             .pull-right
               = link_to 'Edit', '#', class: 'edit-link'
-        .value
+        .value.hide-collapsed
           - if issuable.milestone
             %span.back-to-milestone
               = link_to namespace_project_milestone_path(@project.namespace, @project, issuable.milestone) do
@@ -68,7 +68,7 @@
                   = issuable.milestone.title
           - else
             .light None
-        .selectbox
+        .selectbox.hide-collapsed
           = f.select(:milestone_id, milestone_options(issuable), { include_blank: true }, { class: 'select2 select2-compact js-select2 js-milestone', data: { placeholder: 'Select milestone' }})
           = hidden_field_tag :issuable_context
           = f.submit class: 'btn hide'
@@ -79,18 +79,18 @@
             = icon('tags')
             %span
               = issuable.labels.count
-          .title
+          .title.hide-collapsed
             %label Labels
             - if can?(current_user, :"admin_#{issuable.to_ability_name}", @project)
               .pull-right
                 = link_to 'Edit', '#', class: 'edit-link'
-          .value.issuable-show-labels
+          .value.issuable-show-labels.hide-collapsed
             - if issuable.labels.any?
               - issuable.labels.each do |label|
                 = link_to_label(label, type: issuable.to_ability_name)
             - else
               .light None
-          .selectbox
+          .selectbox.hide-collapsed
             = f.collection_select :label_ids, issuable.project.labels.all, :id, :name,
               { selected: issuable.label_ids }, multiple: true, class: 'select2 js-select2', data: { placeholder: "Select labels" }
 
@@ -103,18 +103,18 @@
                 = issuable.weight
               - else
                 No
-          .title
+          .title.hide-collapsed
             %label Weight
             - if can?(current_user, :"admin_#{issuable.to_ability_name}", @project)
               .pull-right
                 = link_to 'Edit', '#', class: 'edit-link'
-          .value
+          .value.hide-collapsed
             - if issuable.weight
               = icon('balance-scale')
               = issuable.weight
             - else
               .light None
-          .selectbox
+          .selectbox.hide-collapsed
             = f.select :weight, issues_weight_options(issuable.weight, edit: true), { include_blank: true },
               { class: 'select2 js-select2', data: { placeholder: "Select weight" }}
 
@@ -125,12 +125,12 @@
         .block.light
           .sidebar-collapsed-icon
             = icon('rss')
-          .title
+          .title.hide-collapsed
             %label.light Notifications
           - subscribtion_status = subscribed ? 'subscribed' : 'unsubscribed'
-          %button.btn.btn-block.btn-gray.subscribe-button{:type => 'button'}
+          %button.btn.btn-block.btn-gray.subscribe-button.hide-collapsed{:type => 'button'}
             %span= subscribed ? 'Unsubscribe' : 'Subscribe'
-          .subscription-status{data: {status: subscribtion_status}}
+          .subscription-status.hide-collapsed{data: {status: subscribtion_status}}
             .unsubscribed{class: ( 'hidden' if subscribed )}
               You're not receiving notifications from this thread.
             .subscribed{class: ( 'hidden' unless subscribed )}
@@ -140,8 +140,7 @@
       .block.project-reference
         .sidebar-collapsed-icon
           = clipboard_button(clipboard_text: project_ref)
-        .title
-        .cross-project-reference
+        .cross-project-reference.hide-collapsed
           %span
             Reference:
             %cite{title: project_ref}

app/workers/ldap_group_links_worker.rb

+class LdapGroupLinksWorker
+  include Sidekiq::Worker
+
+  def perform(user_id)
+    user = User.find(user_id)
+    logger.info "Updating LDAP group memberships for user #{user.id} (#{user.email})"
+    access = Gitlab::LDAP::Access.new(user)
+    access.update_ldap_group_links
+  end
+end

app/workers/ldap_sync_worker.rb

@@ -8,7 +8,9 @@ class LdapSyncWorker
     Rails.logger.info "Performing daily LDAP sync task."
     User.ldap.find_each(batch_size: 100).each do |ldap_user|
       Rails.logger.debug "Syncing user #{ldap_user.username}, #{ldap_user.email}"
-      Gitlab::LDAP::Access.allowed?(ldap_user)
+      # Use the 'update_ldap_group_links_synchronously' option to avoid creating a ton
+      # of new Sidekiq jobs all at once.
+      Gitlab::LDAP::Access.allowed?(ldap_user, update_ldap_group_links_synchronously: true)
     end
   end
 end

lib/gitlab/ldap/access.rb

@@ -14,11 +14,11 @@ module Gitlab
         end
       end
 
-      def self.allowed?(user)
+      def self.allowed?(user, options={})
         self.open(user) do |access|
           # Whether user is allowed, or not, we should update
           # permissions to keep things clean
-          access.update_permissions
+          access.update_permissions(options)
           if access.allowed?
             access.update_user
             user.last_credential_check_at = Time.now
@@ -75,8 +75,14 @@ module Gitlab
         update_kerberos_identity if import_kerberos_identities?
       end
 
-      def update_permissions
-        update_ldap_group_links if group_base.present?
+      def update_permissions(options)
+        if group_base.present?
+          if options[:update_ldap_group_links_synchronously]
+            update_ldap_group_links
+          else
+            LdapGroupLinksWorker.perform_async(user.id)
+          end
+        end
         update_admin_status if admin_group.present?
       end
 

spec/lib/gitlab/ldap/access_spec.rb

@@ -117,11 +117,12 @@ describe Gitlab::LDAP::Access, lib: true do
   end
 
   describe '#update_permissions' do
-    subject { access.update_permissions }
+    subject { access.update_permissions({}) }
 
     it 'does update group permissions with a group base configured' do
       allow(access).to receive_messages(group_base: 'my-group-base')
-      expect(access).to receive(:update_ldap_group_links)
+      expect(access).not_to receive(:update_ldap_group_links)
+      expect(LdapGroupLinksWorker).to receive(:perform_async).with(user.id)
 
       subject
     end
@@ -129,6 +130,7 @@ describe Gitlab::LDAP::Access, lib: true do
     it 'does not update group permissions without a group base configured' do
       allow(access).to receive_messages(group_base: '')
       expect(access).not_to receive(:update_ldap_group_links)
+      expect(LdapGroupLinksWorker).not_to receive(:perform_async)
 
       subject
     end
@@ -146,6 +148,16 @@ describe Gitlab::LDAP::Access, lib: true do
 
       subject
     end
+
+    context 'when synchronously updating group permissions' do
+      it 'updates group permissions directly' do
+        allow(access).to receive_messages(group_base: 'my-group-base')
+        expect(LdapGroupLinksWorker).not_to receive(:perform_async)
+        expect(access).to receive(:update_ldap_group_links)
+  
+        access.update_permissions(update_ldap_group_links_synchronously: true)
+      end      
+    end
   end
 
   describe :update_kerberos_identity do