Added more testsjprovazn-subgroup-labels

4 files changed, 78 insertions, 32 deletions

diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index 80b30e1f39c..e87fd49d193 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -106,7 +106,7 @@ class IssuableBaseService < BaseService
   end
 
   def available_labels
-    @avialable_lables ||= LabelsFinder.new(current_user, project_id: @project.id).execute
+    @available_labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute
   end
 
   def merge_quick_actions_into_params!(issuable)
@@ -308,7 +308,6 @@ class IssuableBaseService < BaseService
     issuable.state_changed?
   end
 
-  # this allows us to easily change label's parent from project to group
   def parent
     project
   end
diff --git a/app/services/labels/find_or_create_service.rb b/app/services/labels/find_or_create_service.rb
index dd97f68ced9..079f611b3f3 100644
--- a/app/services/labels/find_or_create_service.rb
+++ b/app/services/labels/find_or_create_service.rb
@@ -19,7 +19,8 @@ module Labels
     def available_labels
       @available_labels ||= LabelsFinder.new(
         current_user,
-        "#{parent_type}_id".to_sym => parent.id
+        "#{parent_type}_id".to_sym => parent.id,
+        only_group_labels: parent_is_group?
       ).execute(skip_authorization: skip_authorization)
     end
 
@@ -42,5 +43,9 @@ module Labels
     def parent_type
       parent.model_name.param_key
     end
+
+    def parent_is_group?
+      parent_type == "group"
+    end
   end
 end
diff --git a/spec/finders/labels_finder_spec.rb b/spec/finders/labels_finder_spec.rb
index 594c052c583..dc76efea35b 100644
--- a/spec/finders/labels_finder_spec.rb
+++ b/spec/finders/labels_finder_spec.rb
@@ -5,7 +5,8 @@ describe LabelsFinder do
     let(:group_1) { create(:group) }
     let(:group_2) { create(:group) }
     let(:group_3) { create(:group) }
-    let(:subgroup_1) { create(:group, parent: group_1) }
+    let(:private_group_1) { create(:group, :private) }
+    let(:private_subgroup_1) { create(:group, :private, parent: private_group_1) }
 
     let(:project_1) { create(:project, namespace: group_1) }
     let(:project_2) { create(:project, namespace: group_2) }
@@ -21,7 +22,8 @@ describe LabelsFinder do
     let!(:group_label_1) { create(:group_label, group: group_1, title: 'Label 1 (group)') }
     let!(:group_label_2) { create(:group_label, group: group_1, title: 'Group Label 2') }
     let!(:group_label_3) { create(:group_label, group: group_2, title: 'Group Label 3') }
-    let!(:subgroup_label_1) { create(:group_label, group: subgroup_1, title: 'Subgroup Label 1') }
+    let!(:private_group_label_1) { create(:group_label, group: private_group_1, title: 'Private Group Label 1') }
+    let!(:private_subgroup_label_1) { create(:group_label, group: private_subgroup_1, title: 'Private Sub Group Label 1') }
 
     let(:user) { create(:user) }
 
@@ -71,12 +73,20 @@ describe LabelsFinder do
 
       context 'when including labels from group ancestors', :nested_groups do
         it 'returns labels from group and its ancestors' do
-          group_1.add_developer(user)
-          subgroup_1.add_developer(user)
+          private_group_1.add_developer(user)
+          private_subgroup_1.add_developer(user)
+
+          finder = described_class.new(user, group_id: private_subgroup_1.id, only_group_labels: true, include_ancestor_groups: true)
+
+          expect(finder.execute).to eq [private_group_label_1, private_subgroup_label_1]
+        end
+
+        it 'ignores labels from groups which user can not read' do
+          private_subgroup_1.add_developer(user)
 
-          finder = described_class.new(user, group_id: subgroup_1.id, only_group_labels: true, include_ancestor_groups: true)
+          finder = described_class.new(user, group_id: private_subgroup_1.id, only_group_labels: true, include_ancestor_groups: true)
 
-          expect(finder.execute).to eq [group_label_2, group_label_1, subgroup_label_1]
+          expect(finder.execute).to eq [private_subgroup_label_1]
         end
       end
     end
diff --git a/spec/services/labels/find_or_create_service_spec.rb b/spec/services/labels/find_or_create_service_spec.rb
index 78aa5d442e7..68d5660445a 100644
--- a/spec/services/labels/find_or_create_service_spec.rb
+++ b/spec/services/labels/find_or_create_service_spec.rb
@@ -15,47 +15,79 @@ describe Labels::FindOrCreateService do
 
     context 'when acting on behalf of a specific user' do
       let(:user) { create(:user) }
-      subject(:service) { described_class.new(user, project, params) }
-      before do
-        project.add_developer(user)
-      end
 
-      context 'when label does not exist at group level' do
-        it 'creates a new label at project level' do
-          expect { service.execute }.to change(project.labels, :count).by(1)
+      context 'when finding labels on project level' do
+        subject(:service) { described_class.new(user, project, params) }
+
+        before do
+          project.add_developer(user)
         end
-      end
 
-      context 'when label exists at group level' do
-        it 'returns the group label' do
-          group_label = create(:group_label, group: group, title: 'Security')
+        context 'when label does not exist at group level' do
+          it 'creates a new label at project level' do
+            expect { service.execute }.to change(project.labels, :count).by(1)
+          end
+        end
 
-          expect(service.execute).to eq group_label
+        context 'when label exists at group level' do
+          it 'returns the group label' do
+            group_label = create(:group_label, group: group, title: 'Security')
+
+            expect(service.execute).to eq group_label
+          end
+        end
+
+        context 'when label exists at project level' do
+          it 'returns the project label' do
+            project_label = create(:label, project: project, title: 'Security')
+
+            expect(service.execute).to eq project_label
+          end
         end
       end
 
-      context 'when label does not exist at group level' do
-        it 'creates a new label at project leve' do
-          expect { service.execute }.to change(project.labels, :count).by(1)
+      context 'when finding labels on group level' do
+        subject(:service) { described_class.new(user, group, params) }
+
+        before do
+          group.add_developer(user)
+        end
+
+        context 'when label does not exist at group level' do
+          it 'creates a new label at group level' do
+            expect { service.execute }.to change(group.labels, :count).by(1)
+          end
+        end
+
+        context 'when label exists at group level' do
+          it 'returns the group label' do
+            group_label = create(:group_label, group: group, title: 'Security')
+
+            expect(service.execute).to eq group_label
+          end
         end
       end
+    end
+
+    context 'when authorization is not required' do
+      context 'when finding labels on project level' do
+        subject(:service) { described_class.new(nil, project, params) }
 
-      context 'when label exists at project level' do
         it 'returns the project label' do
           project_label = create(:label, project: project, title: 'Security')
 
-          expect(service.execute).to eq project_label
+          expect(service.execute(skip_authorization: true)).to eq project_label
         end
       end
-    end
 
-    context 'when authorization is not required' do
-      subject(:service) { described_class.new(nil, project, params) }
+      context 'when finding labels on group level' do
+        subject(:service) { described_class.new(nil, group, params) }
 
-      it 'returns the project label' do
-        project_label = create(:label, project: project, title: 'Security')
+        it 'returns the group label' do
+          group_label = create(:group_label, group: group, title: 'Security')
 
-        expect(service.execute(skip_authorization: true)).to eq project_label
+          expect(service.execute(skip_authorization: true)).to eq group_label
+        end
       end
     end
   end