EDM-3777: Filter repositories via the API#630
EDM-3777: Filter repositories via the API#630celdrake wants to merge 1 commit intoflightctl:mainfrom
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 28 minutes and 56 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
WalkthroughThis pull request removes the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/ui-components/src/components/ResourceSync/RepositoryResourceSyncList.tsx (1)
276-286:⚠️ Potential issue | 🟡 MinorGuard
ResourceSyncEmptyStatewhen a search filter is active.Now that
nameSearchdrives a server-side query, an active search with zero matches will renderResourceSyncEmptyState("No resource syncs here!") even though the repository may actually contain resource syncs that just don't match the filter. This is also inconsistent withRepositoryList.tsx(line 231), which gates its empty state with!nameSearch. It may additionally collide with theTable's own "no results for the applied filters" state driven byhasFilters/emptyData/clearFilters.🛠️ Proposed fix
- {resourceSyncs.length === 0 && ( + {resourceSyncs.length === 0 && !nameSearch && ( <ResourceSyncEmptyState addResourceSync={ canCreate ? () => { setIsAddRsModalOpen(true); } : undefined } /> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-components/src/components/ResourceSync/RepositoryResourceSyncList.tsx` around lines 276 - 286, The empty-state component ResourceSyncEmptyState is rendered even when a nameSearch filter is active; change the render guard so the empty state only appears when there are zero resourceSyncs AND no active nameSearch (mirror RepositoryList behavior), i.e. update the condition around ResourceSyncEmptyState to include !nameSearch while keeping the existing canCreate -> setIsAddRsModalOpen logic intact so the addResourceSync prop remains unchanged.
🧹 Nitpick comments (1)
libs/ui-components/src/utils/query.ts (1)
68-88: Object-parameter refactor looks good; minor note on rsName interpolation.The new signature is clean and mirrors the
repo + optional name containspattern elsewhere. One small thing to keep in mind:rsNameis spliced into a comma-joinedfieldSelectorwithout escaping, so a literal comma inrsNamewould be interpreted as a selector separator by the API.TableTextSearchonly strips whitespace, not commas. Not blocking given resource-sync names shouldn't contain commas, but worth noting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-components/src/utils/query.ts` around lines 68 - 88, getResourceSyncsByRepo currently injects rsName directly into the fieldSelector which treats commas as selector separators; modify the function so you escape commas in rsName before building the selector (e.g., compute const safeRsName = rsName.replaceAll(',', '\\,') and use safeRsName in the selectors.push call), keeping the rest of the URLSearchParams logic unchanged; this ensures commas in the provided rsName are treated literally when constructing the fieldSelector.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@libs/ui-components/src/components/ResourceSync/RepositoryResourceSyncList.tsx`:
- Around line 276-286: The empty-state component ResourceSyncEmptyState is
rendered even when a nameSearch filter is active; change the render guard so the
empty state only appears when there are zero resourceSyncs AND no active
nameSearch (mirror RepositoryList behavior), i.e. update the condition around
ResourceSyncEmptyState to include !nameSearch while keeping the existing
canCreate -> setIsAddRsModalOpen logic intact so the addResourceSync prop
remains unchanged.
---
Nitpick comments:
In `@libs/ui-components/src/utils/query.ts`:
- Around line 68-88: getResourceSyncsByRepo currently injects rsName directly
into the fieldSelector which treats commas as selector separators; modify the
function so you escape commas in rsName before building the selector (e.g.,
compute const safeRsName = rsName.replaceAll(',', '\\,') and use safeRsName in
the selectors.push call), keeping the rest of the URLSearchParams logic
unchanged; this ensures commas in the provided rsName are treated literally when
constructing the fieldSelector.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bacd6e19-bcdc-4a1b-9cbf-d8ba19934f76
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
apps/ocp-plugin/package.jsonapps/standalone/package.jsonlibs/ui-components/package.jsonlibs/ui-components/src/components/Repository/CreateRepository/CreateRepository.tsxlibs/ui-components/src/components/Repository/RepositoryDetails/DeleteRepositoryModal.tsxlibs/ui-components/src/components/Repository/RepositoryList.tsxlibs/ui-components/src/components/Repository/useRepositories.tslibs/ui-components/src/components/ResourceSync/RepositoryResourceSyncList.tsxlibs/ui-components/src/components/modals/massModals/MassDeleteRepositoryModal/MassDeleteRepositoryModal.tsxlibs/ui-components/src/hooks/useTableTextSearch.tslibs/ui-components/src/utils/query.tslibs/ui-components/src/utils/search.ts
💤 Files with no reviewable changes (5)
- libs/ui-components/package.json
- apps/standalone/package.json
- apps/ocp-plugin/package.json
- libs/ui-components/src/utils/search.ts
- libs/ui-components/src/hooks/useTableTextSearch.ts
celdrake
force-pushed
the
EDM-3777-filter-repos-via-api
branch
from
b651fb8 to
f70f339
Compare
| const prevNameRef = React.useRef(name); | ||
| React.useEffect(() => { | ||
| if (prevNameRef.current !== name) { | ||
| prevNameRef.current = name; | ||
| setCurrentPage(1); | ||
| } | ||
| }, [name, setCurrentPage]); |
There was a problem hiding this comment.
why are you using name Ref ? Would this be sufficient ?
| const prevNameRef = React.useRef(name); | |
| React.useEffect(() => { | |
| if (prevNameRef.current !== name) { | |
| prevNameRef.current = name; | |
| setCurrentPage(1); | |
| } | |
| }, [name, setCurrentPage]); | |
| React.useEffect(() => { | |
| setCurrentPage(1); | |
| }, [name, setCurrentPage]); |
There was a problem hiding this comment.
The only difference is that with the ref we avoid a re-render in the first mount.
I can remove it, since it's not very clear in any case.
2 participants
Filtering by name in Repositories page worked differently to the rest of the pages, since it was done client-side and used fuzzy search.
Now the filtering in Repositories / ResourceSyncs will be done via the API with "contains"
Summary by CodeRabbit