Allow to include also descendant group labels

Because epic index page includes also epics from subgroups
it's necessary to also get descendant group labels for filtering.
https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/4773#note_61236542

4 files changed, 39 insertions, 11 deletions

diff --git a/app/controllers/groups/labels_controller.rb b/app/controllers/groups/labels_controller.rb
index 2e4cec830bb..ac1d97dc54a 100644
--- a/app/controllers/groups/labels_controller.rb
+++ b/app/controllers/groups/labels_controller.rb
@@ -18,7 +18,8 @@ class Groups::LabelsController < Groups::ApplicationController
           current_user,
           group_id: @group.id,
           only_group_labels: params[:only_group_labels],
-          include_ancestor_groups: params[:include_ancestor_groups]
+          include_ancestor_groups: params[:include_ancestor_groups],
+          include_descendant_groups: params[:include_descendant_groups]
         ).execute
 
         render json: LabelSerializer.new.represent_appearance(available_labels)
diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb
index 5c9fce211ec..780c0fdb03e 100644
--- a/app/finders/labels_finder.rb
+++ b/app/finders/labels_finder.rb
@@ -61,12 +61,20 @@ class LabelsFinder < UnionFinder
 
   def group_ids
     strong_memoize(:group_ids) do
-      group = Group.find(params[:group_id])
-      groups = params[:include_ancestor_groups].present? ? group.self_and_ancestors : [group]
-      groups_user_can_read_labels(groups).map(&:id)
+      groups_user_can_read_labels(groups_to_include).map(&:id)
     end
   end
 
+  def groups_to_include
+    group = Group.find(params[:group_id])
+    groups = [group]
+
+    groups += group.ancestors if params[:include_ancestor_groups].present?
+    groups += group.descendants if params[:include_descendant_groups].present?
+
+    groups
+  end
+
   def group?
     params[:group_id].present?
   end
diff --git a/spec/controllers/groups/labels_controller_spec.rb b/spec/controllers/groups/labels_controller_spec.rb
index 3269f47f327..185b6b4ce57 100644
--- a/spec/controllers/groups/labels_controller_spec.rb
+++ b/spec/controllers/groups/labels_controller_spec.rb
@@ -1,9 +1,9 @@
 require 'spec_helper'
 
 describe Groups::LabelsController do
-  let(:group) { create(:group) }
-  let(:user)  { create(:user) }
-  let(:project) { create(:project, namespace: group) }
+  set(:group) { create(:group) }
+  set(:user)  { create(:user) }
+  set(:project) { create(:project, namespace: group) }
 
   before do
     group.add_owner(user)
@@ -12,8 +12,8 @@ describe Groups::LabelsController do
   end
 
   describe 'GET #index' do
-    let!(:label_1) { create(:label, project: project, title: 'label_1') }
-    let!(:group_label_1) { create(:group_label, group: group, title: 'group_label_1') }
+    set(:label_1) { create(:label, project: project, title: 'label_1') }
+    set(:group_label_1) { create(:group_label, group: group, title: 'group_label_1') }
 
     it 'returns group and project labels by default' do
       get :index, group_id: group, format: :json
@@ -23,8 +23,8 @@ describe Groups::LabelsController do
     end
 
     context 'with ancestor group', :nested_groups do
-      let(:subgroup) { create(:group, parent: group) }
-      let!(:subgroup_label_1) { create(:group_label, group: subgroup, title: 'subgroup_label_1') }
+      set(:subgroup) { create(:group, parent: group) }
+      set(:subgroup_label_1) { create(:group_label, group: subgroup, title: 'subgroup_label_1') }
 
       before do
         subgroup.add_owner(user)
diff --git a/spec/finders/labels_finder_spec.rb b/spec/finders/labels_finder_spec.rb
index dc76efea35b..d434c501110 100644
--- a/spec/finders/labels_finder_spec.rb
+++ b/spec/finders/labels_finder_spec.rb
@@ -89,6 +89,25 @@ describe LabelsFinder do
           expect(finder.execute).to eq [private_subgroup_label_1]
         end
       end
+
+      context 'when including labels from group descendants', :nested_groups do
+        it 'returns labels from group and its descendants' do
+          private_group_1.add_developer(user)
+          private_subgroup_1.add_developer(user)
+
+          finder = described_class.new(user, group_id: private_group_1.id, only_group_labels: true, include_descendant_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: private_group_1.id, only_group_labels: true, include_descendant_groups: true)
+
+          expect(finder.execute).to eq [private_subgroup_label_1]
+        end
+      end
     end
 
     context 'filtering by project_id' do