refactor group_finder to not break the API and add tests for it

3 files changed, 37 insertions, 2 deletions

diff --git a/app/finders/groups_finder.rb b/app/finders/groups_finder.rb
index e8439b35b76..5056e381805 100644
--- a/app/finders/groups_finder.rb
+++ b/app/finders/groups_finder.rb
@@ -39,7 +39,7 @@ class GroupsFinder < UnionFinder
 
   def all_groups
     return [owned_groups] if params[:owned]
-    return [Group.all] if current_user&.full_private_access? && params[:all_available]
+    return [Group.all] if current_user&.full_private_access? && all_available?
 
     groups = []
     groups << Gitlab::GroupHierarchy.new(groups_for_ancestors, groups_for_descendants).all_groups if current_user
@@ -69,4 +69,8 @@ class GroupsFinder < UnionFinder
   def include_public_groups?
     current_user.nil? || params.fetch(:all_available, true)
   end
+
+  def all_available?
+    params[:all_available].nil? ? true : params[:all_available]
+  end
 end
diff --git a/changelogs/unreleased/feature-show-only-groups-user-is-member-of-in-dashboard.yml b/changelogs/unreleased/feature-show-only-groups-user-is-member-of-in-dashboard.yml
index 29a069f1d99..a26a37d69e5 100644
--- a/changelogs/unreleased/feature-show-only-groups-user-is-member-of-in-dashboard.yml
+++ b/changelogs/unreleased/feature-show-only-groups-user-is-member-of-in-dashboard.yml
@@ -1,5 +1,5 @@
 ---
-title: In the groups dashboard, Gitlab now only shows the groups which the user is a member of (applies also to admins)
+title: For group dashboard and other places, we no longer show groups which the visitor is not a member of (this applies to admins and auditors)
 merge_request: 17884
 author: Roger Rüttimann
 type: changed
diff --git a/spec/finders/groups_finder_spec.rb b/spec/finders/groups_finder_spec.rb
index abc470788e1..3cf3f78be5f 100644
--- a/spec/finders/groups_finder_spec.rb
+++ b/spec/finders/groups_finder_spec.rb
@@ -116,4 +116,35 @@ describe GroupsFinder do
       end
     end
   end
+
+  describe '#all_available?' do
+    let(:params) { nil }
+    let(:group_finder) { described_class.new(nil, params) }
+
+    subject { group_finder.send(:all_available?) }
+
+    context 'with params[:all_available] = true' do
+      let(:params) { { all_available: true } }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'with params[:all_available] = false' do
+      let(:params) { { all_available: false } }
+
+      it { is_expected.to be_falsey }
+    end
+
+    context 'with param[:all_available] = nil' do
+      let(:group_finder) { described_class.new(nil) }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'without key :all_available' do
+      let(:group_finder) { described_class.new(nil) }
+
+      it { is_expected.to be_truthy }
+    end
+  end
 end