Ignore searching in full path in GroupsFinder

When searching for groups, if parent parameter is used, it's more useful
to ignore matches in the full path (as we are searching in a
group hierarchy), otherwise of the search string matches full path
substring, we get list of all descendant groups.

Changelog: fixed

app/finders/groups_finder.rb

@@ -100,8 +100,7 @@ class GroupsFinder < UnionFinder
   def by_search(groups)
     return groups unless params[:search].present?
 
-    search_in_descendant_groups = params[:parent].present? && include_parent_descendants?
-    groups.search(params[:search], include_parents: !search_in_descendant_groups)
+    groups.search(params[:search], include_parents: !params[:parent].present?)
   end
   # rubocop: enable CodeReuse/ActiveRecord
 

doc/api/groups.md

@@ -131,7 +131,7 @@ Parameters:
 | `id`                     | integer/string    | yes      | The ID or [URL-encoded path of the group](index.md#namespaced-path-encoding) of the immediate parent group |
 | `skip_groups`            | array of integers | no       | Skip the group IDs passed |
 | `all_available`          | boolean           | no       | Show all the groups you have access to (defaults to `false` for authenticated users, `true` for administrators); Attributes `owned` and `min_access_level` have precedence |
-| `search`                 | string            | no       | Return the list of authorized groups matching the search criteria |
+| `search`                 | string            | no       | Return the list of authorized groups matching the search criteria. Only subgroup short paths are searched (not full paths) |
 | `order_by`               | string            | no       | Order groups by `name`, `path` or `id`. Default is `name` |
 | `sort`                   | string            | no       | Order groups in `asc` or `desc` order. Default is `asc` |
 | `statistics`             | boolean           | no       | Include group statistics (administrators only) |
@@ -189,7 +189,7 @@ Parameters:
 | `id`                     | integer/string    | yes      | The ID or [URL-encoded path of the group](index.md#namespaced-path-encoding) of the immediate parent group |
 | `skip_groups`            | array of integers | no       | Skip the group IDs passed |
 | `all_available`          | boolean           | no       | Show all the groups you have access to (defaults to `false` for authenticated users, `true` for administrators). Attributes `owned` and `min_access_level` have precedence |
-| `search`                 | string            | no       | Return the list of authorized groups matching the search criteria |
+| `search`                 | string            | no       | Return the list of authorized groups matching the search criteria. Only descendant group short paths are searched (not full paths) |
 | `order_by`               | string            | no       | Order groups by `name`, `path`, or `id`. Default is `name` |
 | `sort`                   | string            | no       | Order groups in `asc` or `desc` order. Default is `asc` |
 | `statistics`             | boolean           | no       | Include group statistics (administrators only) |

lib/api/groups.rb

@@ -35,7 +35,8 @@ module API
           :all_available,
           :custom_attributes,
           :owned, :min_access_level,
-          :include_parent_descendants
+          :include_parent_descendants,
+          :search
         )
 
         find_params[:parent] = if params[:top_level_only]
@@ -48,7 +49,6 @@ module API
           find_params.fetch(:all_available, current_user&.can_read_all_resources?)
 
         groups = GroupsFinder.new(current_user, find_params).execute
-        groups = groups.search(params[:search], include_parents: true) if params[:search].present?
         groups = groups.where.not(id: params[:skip_groups]) if params[:skip_groups].present?
 
         order_groups(groups)

spec/finders/groups_finder_spec.rb

@@ -228,13 +228,6 @@ RSpec.describe GroupsFinder do
           )
         end
       end
-
-      context 'with search' do
-        it 'does not search in full path' do
-          params[:search] = public_subgroup.path
-          expect(described_class.new(user, params).execute).to contain_exactly(public_subgroup)
-        end
-      end
     end
 
     context 'with search' do
@@ -249,6 +242,10 @@ RSpec.describe GroupsFinder do
         expect(described_class.new(user, { search: 'test' }).execute).to contain_exactly(test_group)
       end
 
+      it 'does not search in full path if parent is set' do
+        expect(described_class.new(user, { search: 'parent', parent: parent_group }).execute).to be_empty
+      end
+
       context 'with group descendants' do
         let_it_be(:sub_group) { create(:group, :public, name: 'Sub Group', parent: parent_group) }
 

spec/requests/api/groups_spec.rb

@@ -63,6 +63,19 @@ RSpec.describe API::Groups do
     end
   end
 
+  shared_examples 'skips searching in full path' do
+    it 'does not find groups by full path' do
+      subgroup = create(:group, parent: parent, path: "#{parent.path}-subgroup")
+      create(:group, parent: parent, path: 'not_matching_path')
+
+      get endpoint, params: { search: parent.path }
+
+      expect(json_response).to be_an Array
+      expect(json_response.length).to eq(1)
+      expect(json_response.first['id']).to eq(subgroup.id)
+    end
+  end
+
   describe "GET /groups" do
     context "when unauthenticated" do
       it "returns public groups" do
@@ -406,6 +419,22 @@ RSpec.describe API::Groups do
         expect(response_groups).to contain_exactly(group2.id, group3.id)
       end
     end
+
+    context 'when searching' do
+      let_it_be(:subgroup1) { create(:group, parent: group1, path: 'some_path') }
+
+      let(:response_groups) { json_response.map { |group| group['id'] } }
+
+      subject { get api('/groups', user1), params: { search: group1.path } }
+
+      it 'finds also groups with full path matching search param' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response).to be_an Array
+        expect(response_groups).to match_array([group1.id, subgroup1.id])
+      end
+    end
   end
 
   describe "GET /groups/:id" do
@@ -1424,6 +1453,11 @@ RSpec.describe API::Groups do
         expect(json_response.first).to include('statistics')
       end
     end
+
+    it_behaves_like 'skips searching in full path' do
+      let(:parent) { group1 }
+      let(:endpoint) { api("/groups/#{group1.id}/subgroups", user1) }
+    end
   end
 
   describe 'GET /groups/:id/descendant_groups' do
@@ -1558,6 +1592,11 @@ RSpec.describe API::Groups do
         expect(json_response.first).to include('statistics')
       end
     end
+
+    it_behaves_like 'skips searching in full path' do
+      let(:parent) { group1 }
+      let(:endpoint) { api("/groups/#{group1.id}/descendant_groups", user1) }
+    end
   end
 
   describe "POST /groups" do