feat(gitlab): add server-side search to project picker - #8553
MathiasMonstrey wants to merge 1 commit into
Conversation
Thread a `search_text` query param from the project picker through to GitLab's `GET /projects?search=`, replacing the previous fetch-100-and- filter-client-side behaviour. - `fetch_gitlab_projects` accepts optional `search_text`, omitting the `search` param when blank - `ProjectSearchQueryParamsSerializer` validates the param on `BrowseGitLabProjects` - `GitLabProjectSelect` drives a debounced server-side search via `useInfiniteScroll`, with `filterOption` disabling react-select's client-side filtering - `useInfiniteScroll` exposes `isError` - Replace the select's `isDisabled` on fetch failure with `noOptionsMessage`, keeping the input usable for retry-by-search - Hold the selected option rather than deriving it from the mutating result set, so the label survives a subsequent search Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@MathiasMonstrey is attempting to deploy a commit to the Flagsmith Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe backend now accepts optional Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🔵 Low · up to Closing or selecting in the project picker can replace typed search results with the unfiltered list. The localized callback fix should be applied before merge. ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: cded3326-f122-4661-b40a-de75605ce6f5
📒 Files selected for processing (10)
api/integrations/gitlab/client/api.pyapi/integrations/gitlab/serializers.pyapi/integrations/gitlab/views/browse_gitlab.pyapi/tests/unit/integrations/gitlab/test_client.pyapi/tests/unit/integrations/gitlab/test_proxy_views.pydocs/docs/deployment-self-hosting/observability/_events-catalogue.mdfrontend/common/services/useGitlab.tsfrontend/common/useInfiniteScroll.tsfrontend/web/components/GitLabLinkSection.tsxfrontend/web/components/GitLabProjectSelect.tsx
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
| import GitLabProjectSelect from './GitLabProjectSelect' | ||
| import GitLabProjectSelect, { | ||
| type GitLabProjectOption, | ||
| } from './GitLabProjectSelect' |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use the components/ import path.
Replace the relative GitLabProjectSelect import with its components/ path. The changed import violates the rule: “Use only common/, components/, and project/ import paths; do not use relative imports.”
Source: Coding guidelines
| placeholder={isBusy ? 'Loading...' : 'Select GitLab Project'} | ||
| value={value} | ||
| onChange={(v: GitLabProjectOption) => onChange(v)} | ||
| onInputChange={(e: string) => onInputChange(e)} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '1,130p' frontend/web/components/GitLabProjectSelect.tsx
sed -n '35,125p' frontend/web/components/GitLabLinkSection.tsx
sed -n '1,130p' frontend/common/useInfiniteScroll.ts
rg -n 'onInputChange|input-blur|menu-close|set-value' frontend/node_modules/react-select 2>/dev/null | head -100
cat frontend/package.json | grep -n -A2 -B2 '"react-select"'Repository: Flagsmith/flagsmith
Length of output: 7333
🌐 Web query:
react-select 5.10.2 source Select.tsx onInputChange input-blur menu-close set-value
💡 Result:
<search_synthesis>
In react-select 5.10.2, the Select component handles input interactions through a series of actions that trigger the onInputChange prop [1][2]. These actions—specifically input-blur, menu-close, and set-value—are emitted by the internal Select.tsx logic to signal state changes in the input field [1]. Key behavioral details for these actions: 1. input-blur: This action is triggered when the input element loses focus [1]. By default, the component cleans up the input state, which often results in the search text being cleared [1][3]. 2. menu-close: This action is triggered when the dropdown menu closes [1]. Like input-blur, this often triggers an internal state update that may clear the input value [1][4]. 3. set-value: This action is triggered when a user selects an option [1]. The internal setValue method updates the component's internal state and calls onInputChange with this action to notify the controlled input of the change [1]. Because these actions are built-in behaviors of the library, many developers manage controlled input persistence by wrapping the onInputChange handler with logic to ignore or selectively process specific actions [5][3][6][7]. Common implementation pattern: If you want to keep the input text from being cleared when the menu closes or the input blurs, you can check the action metadata within your onInputChange handler [5][7]: onInputChange={(newValue, actionMeta) => { // Only update your component state if the action is a standard input change if (actionMeta.action!== 'input-blur' && actionMeta.action!== 'menu-close') { setInputValue(newValue); } }} This pattern ensures that your controlled inputValue only responds to direct user typing (input-change) while ignoring the automatic resets triggered by internal lifecycle events [5][3][6].
</search_synthesis>
<source_evidence>
Citations:
- 1: https://github.com/JedWatson/react-select/blob/master/packages/react-select/src/Select.tsx
- 2: https://cdn.jsdelivr.net/npm/react-select@5.10.2/dist/declarations/src/Select.d.ts
- 3: GitHub issue 3189 in JedWatson/react-select (link omitted to avoid creating a cross-reference)
- 4: GitHub issue 1826 in JedWatson/react-select (link omitted to avoid creating a cross-reference)
- 5: GitHub issue 3210 in JedWatson/react-select (link omitted to avoid creating a cross-reference)
- 6: https://stackoverflow.com/questions/64298547/dont-clear-input-on-select-using-react-select
- 7: https://komelin.com/blog/developing-custom-search-box-with-react-select
🏁 Script executed:
sed -n '1,70p' frontend/web/components/GitLabProjectSelect.tsx
sed -n '1,65p' frontend/web/components/experiments/EventNameSelect/EventNameSelect.tsx
sed -n '75,90p' frontend/package-lock.json
sed -n '19815,19830p' frontend/package-lock.jsonRepository: Flagsmith/flagsmith
Length of output: 5280
Filter non-typing onInputChange actions.
React Select 5.10.2 emits an empty value with actions such as input-blur, menu-close, and set-value. This handler forwards every value to searchProjects. The debounced useInfiniteScroll search stores the empty value in q, which sends an unfiltered request and can replace the typed project results. Selecting an option emits set-value before the menu-close configuration is checked, so the current configuration does not prevent this path.
Pass the action metadata and handle only input-change.
import type { InputActionMeta } from 'react-select'
// ...
onInputChange={(
value: string,
{ action }: InputActionMeta,
) => {
if (action === 'input-change') onInputChange(value)
}}|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8553 +/- ##
==========================================
- Coverage 98.81% 98.67% -0.14%
==========================================
Files 1620 1620
Lines 66234 66251 +17
==========================================
- Hits 65449 65376 -73
- Misses 785 875 +90 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Closes #8552
The GitLab project picker fetched the first 100 projects once and filtered them client-side, so projects beyond that page were unreachable. Search is now performed by GitLab.
Backend
fetch_gitlab_projectstakes an optionalsearch_textand forwards it as GitLab'ssearchparam. A blank value omits the param, matchingsearch_gitlab_issuesandsearch_gitlab_merge_requests.ProjectSearchQueryParamsSerializervalidatessearch_textonBrowseGitLabProjects.Frontend
GitLabProjectSelectdrives a debounced server-side search viauseInfiniteScroll.filterOptiondisables react-select's client-side filtering so server results are not filtered twice.useInfiniteScrollnow exposesisError, so consumers do not have to reach into the raw RTK response.isDisabled; the error surfaces throughnoOptionsMessageinstead, keeping the input usable so the user can retry by typing.docs/change is the auto-regenerated structlog events catalogue (a line-number shift from the added import), produced by thegenerate-docspre-commit hook.Paging is intentionally not wired up: server-side search narrows the list, so the picker shows the first page of matches.
How did you test this code?
Unit tests added:
test_fetch_gitlab_projects__with_search_text__sends_search_paramtest_fetch_gitlab_projects__without_search_text__omits_search_param(usesstrict_match=Trueto provesearchis absent)test_gitlab_project_list__with_search_text__forwards_search_to_gitlabRun locally from
api/:make test opts='tests/unit/integrations/gitlab -n0': 114 passed, 2 failed (2 tests failed locally that had nothing to do with the changes, and where DNS related)make typecheck: passmake lint: passThe 2 failures are
test_configuration.py::test_update_configuration__valid_data__persists_and_masks_token, which also fail on a cleanmainand are unrelated to this change. Cause: the fixture URLhttps://gitlab.updated.comis a real registrable domain that resolves to0.0.0.0on filtering/ad-blocking DNS resolvers.ipaddressclassifies that as private, soNoSSRFURLField'svalidate_no_internal_addressrejects it with a 400. The sibling tests pass only becausegitlab.example.comandgitlab.other.comdo not resolve at all, andis_internal_addressreturnsFalsefor unresolvable hosts. Switching that fixture to a reserved domain such asgitlab.updated.example.comwould fix it; I left out of this PR as unrelated and maybe I am the only one with that specific problem 😅Frontend
npm run typecheckandnpx eslintare clean on the changed files.Manual: selected a project, typed a different search term, and confirmed the selected label persists and the dropdown shows a loading state while the search is in flight.
Disclaimer: I did use claude code to help me with this PR, because I am not familiar enough with the code of this project yet. It was mainly used for discussing and reviewing the changes.