Skip to content

Fix exception for search when search_type is not a valid option

Ravi Kumarrequested to merge
217158-nomethoderror-undefined-method into master

What does this MR do and why?

  • For these files ee/app/services/ee/search/global_service.rb and ee/app/services/ee/search/group_service.rb, added an early return if the params[:search_type] = 'basic'. Before, it was adding the non-allowed scopes when the query param was added by a client who could use Elasticsearch. This was causing an exception later in the code flow.
  • For this file ee/app/services/ee/search_service.rb added a verification on params[:search_type] to be only one of these options: basic, advanced, or zoekt. Before, when a random search_type was passed, it was causing an exception.
  • For this file lib/gitlab/search_results.rb, I have just removed the unused code. We are never calling objects with without_count: false. So just removed this option. collection can never be nil. Even if it gets nil, the current code is wrong because Kaminari.paginate_array([]).without_count throws an exception.
  • The rest of the spec changes are Rubocop and spec optimizations.

AI Summary

This merge request improves the search functionality by making search type validation more efficient and flexible. The main changes include:

Search Type Handling: The system now only validates search types when they're actually provided in the request, avoiding unnecessary validation checks. It also adds support for a "basic" search type that bypasses advanced features like Elasticsearch and Zoekt.

Code Cleanup: Several code quality issues were fixed by removing a file from linting exception lists, indicating the code now meets style standards. The search results handling was simplified by removing an unused parameter for counting results.

Better Error Handling: The system now provides clearer error messages when invalid search types are used, listing the supported options (advanced, basic, zoekt).

Test Improvements: The test suite was updated to be more reliable by using before_all instead of before for setup, and tests were added to ensure the new search type validation works correctly.

Performance Optimization: The changes reduce unnecessary database queries and validation checks when basic search is used, making the search feature more efficient for common use cases.

Overall, this makes the search feature more robust, efficient, and user-friendly while maintaining backward compatibility.

References

Screenshots or screen recordings

Before After
Screenshot_2025-09-23_at_15.39.59 Screenshot_2025-09-23_at_14.45.34
Screenshot_2025-09-24_at_18.07.07 Screenshot_2025-09-24_at_18.11.42
Screenshot_2025-09-24_at_18.15.20 Screenshot_2025-09-24_at_18.14.47

How to set up and validate locally

  • You should have Advanced search enabled.
  • Perform a search with a random query param search_type=qwerty, like this: https://gdk.test:3443/search?group_id=24&scope=projects&search=test&search_type=qwerty&project_id=27
  • On master, you should get an exception
  • On this branch, you will see an error message
  • Perform a group search with these params scope=blobs&search_type=basic.
  • On master, you should get an exception.
  • On this branch, you will see the results get rendered for project search. That's how basic search behaves when you don't have zoekt or advanced available.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #217158 (closed)

Edited by Ravi Kumar

Merge request reports