Improve group visibility level feature

8 files changed, 54 insertions, 57 deletions

diff --git a/spec/factories/groups.rb b/spec/factories/groups.rb
index 4a3a155d7ff..2d47a6f6c4c 100644
--- a/spec/factories/groups.rb
+++ b/spec/factories/groups.rb
@@ -3,5 +3,17 @@ FactoryGirl.define do
     sequence(:name) { |n| "group#{n}" }
     path { name.downcase.gsub(/\s/, '_') }
     type 'Group'
+
+    trait :public do
+      visibility_level Gitlab::VisibilityLevel::PUBLIC
+    end
+
+    trait :internal do
+      visibility_level Gitlab::VisibilityLevel::INTERNAL
+    end
+
+    trait :private do
+      visibility_level Gitlab::VisibilityLevel::PRIVATE
+    end
   end
 end
diff --git a/spec/finders/groups_finder_spec.rb b/spec/finders/groups_finder_spec.rb
index d0fd7af8cc3..d5d111e8d15 100644
--- a/spec/finders/groups_finder_spec.rb
+++ b/spec/finders/groups_finder_spec.rb
@@ -2,11 +2,11 @@ require 'spec_helper'
 
 describe GroupsFinder do
   describe '#execute' do
-    let(:user) { create(:user) }
-    let!(:private_group) { create(:group, visibility_level: 0) }
-    let!(:internal_group) { create(:group, visibility_level: 10) }
-    let!(:public_group) { create(:group, visibility_level: 20) }
-    let(:finder) { described_class.new }
+    let(:user)            { create(:user) }
+    let!(:private_group)  { create(:group, :private) }
+    let!(:internal_group) { create(:group, :internal) }
+    let!(:public_group)   { create(:group, :public) }
+    let(:finder)          { described_class.new }
 
     describe 'execute' do
       describe 'without a user' do
@@ -23,7 +23,8 @@ describe GroupsFinder do
         end
 
         context 'external user' do
-          before { user.update_attribute(external: true) }
+          let(:user) { create(:user, external: true) }
+
           it { is_expected.to eq([public_group]) }
         end
       end
diff --git a/spec/finders/personal_projects_finder_spec.rb b/spec/finders/personal_projects_finder_spec.rb
index 8758f61903c..a4681fe59d8 100644
--- a/spec/finders/personal_projects_finder_spec.rb
+++ b/spec/finders/personal_projects_finder_spec.rb
@@ -1,24 +1,17 @@
 require 'spec_helper'
 
 describe PersonalProjectsFinder do
-  let(:source_user) { create(:user) }
-  let(:current_user) { create(:user) }
-
-  let(:finder) { described_class.new(source_user) }
-
-  let!(:public_project) do
-    create(:project, :public, namespace: source_user.namespace, name: 'A',
-                              path: 'A')
-  end
+  let(:source_user)     { create(:user) }
+  let(:current_user)    { create(:user) }
+  let(:finder)          { described_class.new(source_user) }
+  let!(:public_project) { create(:project, :public, namespace: source_user.namespace) }
 
   let!(:private_project) do
-    create(:project, :private, namespace: source_user.namespace, name: 'B',
-                               path: 'B')
+    create(:project, :private, namespace: source_user.namespace, path: 'mepmep')
   end
 
   let!(:internal_project) do
-    create(:project, :internal, namespace: source_user.namespace, name: 'c',
-                               path: 'C')
+    create(:project, :internal, namespace: source_user.namespace, path: 'C')
   end
 
   before do
diff --git a/spec/helpers/visibility_level_helper_spec.rb b/spec/helpers/visibility_level_helper_spec.rb
index ef60ef75fbe..ff98249570d 100644
--- a/spec/helpers/visibility_level_helper_spec.rb
+++ b/spec/helpers/visibility_level_helper_spec.rb
@@ -66,13 +66,8 @@ describe VisibilityLevelHelper do
 
   describe "skip_level?" do
     describe "forks" do
-      let(:project) { create(:project, :internal) }
-      let(:fork_project) { create(:forked_project_with_submodules) }
-
-      before do
-        fork_project.build_forked_project_link(forked_to_project_id: fork_project.id, forked_from_project_id: project.id)
-        fork_project.save
-      end
+      let(:project)       { create(:project, :internal) }
+      let(:fork_project)  { create(:project, forked_from_project: project) }
 
       it "skips levels" do
         expect(skip_level?(fork_project, Gitlab::VisibilityLevel::PUBLIC)).to be_truthy
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 135d298e10f..0591aa089d8 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -57,18 +57,18 @@ describe Group, models: true do
   end
 
   describe 'scopes' do
-    let!(:private_group)  { create(:group, visibility_level: 0)  }
-    let!(:internal_group) { create(:group, visibility_level: 10) }
-    let!(:public_group)   { create(:group, visibility_level: 20) }
+    let!(:private_group)  { create(:group, :private)  }
+    let!(:internal_group) { create(:group, :internal) }
+    let!(:public_group)   { create(:group, :public) }
 
     describe 'public_only' do
-      subject { described_class.public_only }
+      subject { described_class.public_only.to_a }
 
       it{ is_expected.to eq([public_group]) }
     end
 
     describe 'public_and_internal_only' do
-      subject { described_class.public_and_internal_only }
+      subject { described_class.public_and_internal_only.to_a }
 
       it{ is_expected.to eq([public_group, internal_group]) }
     end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 7fd3726c6ad..74383204250 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -571,12 +571,8 @@ describe Project, models: true do
     end
 
     context 'when checking on forked project' do
-      let(:forked_project) { create :forked_project_with_submodules }
-
-      before do
-        forked_project.build_forked_project_link(forked_to_project_id: forked_project.id, forked_from_project_id: project.id)
-        forked_project.save
-      end
+      let(:project)        { create(:project, :internal) }
+      let(:forked_project) { create(:project, forked_from_project: project) }
 
       it { expect(forked_project.visibility_level_allowed?(Gitlab::VisibilityLevel::PRIVATE)).to be_truthy }
       it { expect(forked_project.visibility_level_allowed?(Gitlab::VisibilityLevel::INTERNAL)).to be_truthy }
diff --git a/spec/services/groups/create_service_spec.rb b/spec/services/groups/create_service_spec.rb
index b938a2f0c05..6aefb48a4e8 100644
--- a/spec/services/groups/create_service_spec.rb
+++ b/spec/services/groups/create_service_spec.rb
@@ -1,8 +1,8 @@
 require 'spec_helper'
 
 describe Groups::CreateService, services: true do
-    let!(:user)         { create(:user) }
-    let!(:group_params) { { path: "group_path", visibility_level: Gitlab::VisibilityLevel::PUBLIC } }
+  let!(:user)         { create(:user) }
+  let!(:group_params) { { path: "group_path", visibility_level: Gitlab::VisibilityLevel::PUBLIC } }
 
   describe "execute" do
     let!(:service) { described_class.new(user, group_params ) }
diff --git a/spec/services/groups/update_service_spec.rb b/spec/services/groups/update_service_spec.rb
index c759e32342d..9d427ff2d90 100644
--- a/spec/services/groups/update_service_spec.rb
+++ b/spec/services/groups/update_service_spec.rb
@@ -1,10 +1,10 @@
 require 'spec_helper'
 
 describe Groups::UpdateService, services: true do
-    let!(:user)    { create(:user) }
-    let!(:private_group)    { create(:group, visibility_level: Gitlab::VisibilityLevel::PRIVATE) }
-    let!(:internal_group)   { create(:group, visibility_level: Gitlab::VisibilityLevel::INTERNAL) }
-    let!(:public_group)     { create(:group, visibility_level: Gitlab::VisibilityLevel::PUBLIC) }
+  let!(:user)             { create(:user) }
+  let!(:private_group)    { create(:group, :private) }
+  let!(:internal_group)   { create(:group, :internal) }
+  let!(:public_group)     { create(:group, :public) }
 
   describe "execute" do
     context "project visibility_level validation" do
@@ -14,28 +14,28 @@ describe Groups::UpdateService, services: true do
 
         before do
           public_group.add_user(user, Gitlab::Access::MASTER)
-          create(:project, :public, group: public_group, name: 'B', path: 'B')
+          create(:project, :public, group: public_group)
         end
 
         it "cant downgrade permission level" do
           expect(service.execute).to be_falsy
-          expect(public_group.errors.count).to eq(1)
+          expect(public_group.errors.count).to eq(2)
         end
       end
 
-    context "internal group with internal project" do
-        let!(:service) { described_class.new(internal_group, user, visibility_level: Gitlab::VisibilityLevel::PRIVATE ) }
-
-        before do
-          internal_group.add_user(user, Gitlab::Access::MASTER)
-          create(:project, :internal, group: internal_group, name: 'B', path: 'B')
+      context "internal group with internal project" do
+          let!(:service) { described_class.new(internal_group, user, visibility_level: Gitlab::VisibilityLevel::PRIVATE ) }
+  
+          before do
+            internal_group.add_user(user, Gitlab::Access::MASTER)
+            create(:project, :internal, group: internal_group)
+          end
+  
+          it "cant downgrade permission level" do
+            expect(service.execute).to be_falsy
+            expect(internal_group.errors.count).to eq(2)
+          end
         end
-
-        it "cant downgrade permission level" do
-          expect(service.execute).to be_falsy
-          expect(internal_group.errors.count).to eq(1)
-        end
-      end
     end
   end