Merge branch 'sh-check-group-ldap-sync-validity' into 'master'

Notify owner that group is invalid when LDAP "Sync now" fails

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

ee/app/controllers/groups/ldaps_controller.rb

@@ -6,11 +6,19 @@ class Groups::LdapsController < Groups::ApplicationController
   before_action :check_enabled_extras!
 
   def sync
+    # A group can transition to pending if it is in the ready or failed
+    # state. If it is in the started or pending state, then that means
+    # it is already running. If the group doesn't validate, then it's
+    # likely the group will never transition out of its current state,
+    # so we should tell the group owner.
     if @group.pending_ldap_sync
       LdapGroupSyncWorker.perform_async(@group.id)
       message = 'The group sync has been scheduled'
-    else
+    elsif @group.valid?
       message = 'The group sync is already scheduled'
+    else
+      message = "This group is in an invalid state: #{@group.errors.full_messages.join(', ')}"
+      return redirect_to group_group_members_path(@group), alert: message
     end
 
     redirect_to group_group_members_path(@group), notice: message

ee/changelogs/unreleased/sh-check-group-ldap-sync-validity.yml

+---
+title: Notify owner that group is invalid when LDAP "Sync now" fails
+merge_request: 10509
+author:
+type: fixed

ee/spec/controllers/groups/ldaps_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Groups::LdapsController do
+  include LdapHelpers
+
+  let(:group) { create(:group) }
+  let(:user)  { create(:user) }
+
+  before do
+    stub_ldap_setting(enabled: true)
+    group.add_owner(user)
+
+    sign_in(user)
+  end
+
+  describe 'POST #sync' do
+    subject do
+      Sidekiq::Testing.fake! do
+        post :sync, params: { group_id: group.to_param }
+      end
+    end
+
+    it 'transitions to the pending state' do
+      subject
+
+      expect(group.reload.ldap_sync_pending?).to be_truthy
+
+      expect(controller).to redirect_to(group_group_members_path(group))
+    end
+
+    it 'notifies user that the group is already pending' do
+      group.update_columns(ldap_sync_status: 'pending')
+
+      subject
+
+      expect(flash[:notice]).to eq('The group sync is already scheduled')
+      expect(controller).to redirect_to(group_group_members_path(group))
+    end
+
+    it 'returns an error if the group does not validate' do
+      group.update_columns(repository_size_limit: -1)
+
+      expect(group).not_to be_valid
+
+      subject
+
+      expect(flash[:alert]).to include('This group is in an invalid state')
+      expect(controller).to redirect_to(group_group_members_path(group))
+    end
+  end
+end