Less private methods, more test ordering

lib/gitlab/ldap/access.rb

@@ -127,7 +127,6 @@ module Gitlab
         end
       end
 
-      private
       def ldap_groups
         @ldap_groups ||= ::LdapGroupLink.distinct(:cn).pluck(:cn).map do |cn|
           Gitlab::LDAP::Group.find_by_cn(cn, adapter)
@@ -141,6 +140,7 @@ module Gitlab
         end.map(&:cn)
       end
 
+      private
       def gitlab_groups_with_ldap_link
         ::Group.includes(:ldap_group_links).references(:ldap_group_links).
           where.not(ldap_group_links: { id: nil })

spec/lib/gitlab/ldap/access_spec.rb

@@ -186,6 +186,66 @@ objectclass: posixGroup
     end
   end
 
+
+  describe :update_ldap_group_links do
+    let(:cns_with_access) { %w(ldap-group1 ldap-group2) }
+    let(:gitlab_group_1) { create :group }
+    let(:gitlab_group_2) { create :group }
+
+    before do
+      access.stub(:get_ldap_user)
+      access.stub(cns_with_access: cns_with_access)
+    end
+
+    context "non existing access for group-1, allowed via ldap-group1 as MASTER" do
+      before do
+        gitlab_group_1.ldap_group_links.create cn: 'ldap-group1', group_access: Gitlab::Access::MASTER
+      end
+
+      it "gives the user master access for group 1" do
+        access.update_ldap_group_links(user)
+        expect( gitlab_group_1.has_master?(user) ).to be_true
+      end
+    end
+
+    context "existing access as guest for group-1, allowed via ldap-group1 as DEVELOPER" do
+      before do
+        gitlab_group_1.users_groups.guests.create(user_id: user.id)
+        gitlab_group_1.ldap_group_links.create cn: 'ldap-group1', group_access: Gitlab::Access::MASTER
+      end
+
+      it "upgrades the users access to master for group 1" do
+        expect { access.update_ldap_group_links(user) }.to \
+          change{ gitlab_group_1.has_master?(user) }.from(false).to(true)
+      end
+    end
+
+    context "existing access as MASTER for group-1, allowed via ldap-group1 as DEVELOPER" do
+      before do
+        gitlab_group_1.users_groups.masters.create(user_id: user.id)
+        gitlab_group_1.ldap_group_links.create cn: 'ldap-group1', group_access: Gitlab::Access::DEVELOPER
+      end
+
+      it "keeps the users master access for group 1" do
+        expect { access.update_ldap_group_links(user) }.not_to \
+          change{ gitlab_group_1.has_master?(user) }
+      end
+    end
+
+    context "existing access as master for group-1, not allowed" do
+      before do
+        gitlab_group_1.users_groups.masters.create(user_id: user.id)
+        gitlab_group_1.ldap_group_links.create cn: 'ldap-group1', group_access: Gitlab::Access::MASTER
+        access.stub(cns_with_access: ['ldap-group2'])
+      end
+
+      it "removes user from gitlab_group_1" do
+        expect { access.update_ldap_group_links(user) }.to \
+          change{ gitlab_group_1.members.where(user_id: user).any? }.from(true).to(false)
+      end
+    end
+  end
+
   describe 'ldap_groups' do
     let(:ldap_group_1) do
       Net::LDAP::Entry.from_single_ldif_string(
@@ -255,64 +315,5 @@ objectclass: posixGroup
       expect(access.cns_with_access(ldap_user)).to eql ['group1']
     end
   end
-
-  describe :update_ldap_group_links do
-    let(:cns_with_access) { %w(ldap-group1 ldap-group2) }
-    let(:gitlab_group_1) { create :group }
-    let(:gitlab_group_2) { create :group }
-
-    before do
-      access.stub(:get_ldap_user)
-      access.stub(cns_with_access: cns_with_access)
-    end
-
-    context "non existing access for group-1, allowed via ldap-group1 as MASTER" do
-      before do
-        gitlab_group_1.ldap_group_links.create cn: 'ldap-group1', group_access: Gitlab::Access::MASTER
-      end
-
-      it "gives the user master access for group 1" do
-        access.update_ldap_group_links(user)
-        expect( gitlab_group_1.has_master?(user) ).to be_true
-      end
-    end
-
-    context "existing access as guest for group-1, allowed via ldap-group1 as DEVELOPER" do
-      before do
-        gitlab_group_1.users_groups.guests.create(user_id: user.id)
-        gitlab_group_1.ldap_group_links.create cn: 'ldap-group1', group_access: Gitlab::Access::MASTER
-      end
-
-      it "upgrades the users access to master for group 1" do
-        expect { access.update_ldap_group_links(user) }.to \
-          change{ gitlab_group_1.has_master?(user) }.from(false).to(true)
-      end
-    end
-
-    context "existing access as MASTER for group-1, allowed via ldap-group1 as DEVELOPER" do
-      before do
-        gitlab_group_1.users_groups.masters.create(user_id: user.id)
-        gitlab_group_1.ldap_group_links.create cn: 'ldap-group1', group_access: Gitlab::Access::DEVELOPER
-      end
-
-      it "keeps the users master access for group 1" do
-        expect { access.update_ldap_group_links(user) }.not_to \
-          change{ gitlab_group_1.has_master?(user) }
-      end
-    end
-
-    context "existing access as master for group-1, not allowed" do
-      before do
-        gitlab_group_1.users_groups.masters.create(user_id: user.id)
-        gitlab_group_1.ldap_group_links.create cn: 'ldap-group1', group_access: Gitlab::Access::MASTER
-        access.stub(cns_with_access: ['ldap-group2'])
-      end
-
-      it "removes user from gitlab_group_1" do
-        expect { access.update_ldap_group_links(user) }.to \
-          change{ gitlab_group_1.members.where(user_id: user).any? }.from(true).to(false)
-      end
-    end
-  end
 end