fix: pagination added#5
Merged
Merged
Conversation
Signed-off-by: Arnab Nandy <arnab_nandy7@yahoo.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe issue search flow now accepts a ChangesIssue Search Pagination
Sequence Diagram(s)sequenceDiagram
participant IssueFinder
participant SearchRoute as "/api/search route"
participant searchGitHubIssues
participant GitHubSearchAPI as "GitHub /search/issues"
IssueFinder->>SearchRoute: GET tech, label, sort, linkedPr, page
SearchRoute->>searchGitHubIssues: searchGitHubIssues({ ...page })
searchGitHubIssues->>GitHubSearchAPI: GET page query parameter
GitHubSearchAPI-->>searchGitHubIssues: Search results page
searchGitHubIssues-->>SearchRoute: SearchResponse with page
SearchRoute-->>IssueFinder: JSON issues, totalCount, page
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Signed-off-by: Arnab Nandy <arnab_nandy7@yahoo.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
tests/app/api/search/route.test.ts (1)
52-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an explicit
pagequery test.The route’s new behavior is forwarding
?page=...; this only covers the default. Add a case for?tech=React&page=3expectingpage: 3, plus an invalid value if you want to lock normalization.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/app/api/search/route.test.ts` at line 52, The search route tests only cover the default page value, so add an explicit query case for the route handler in the search API tests. Update the existing test suite around the route’s request parsing to include a request with ?tech=React&page=3 and assert the downstream call receives page: 3, and optionally add an invalid page case to verify normalization behavior in the same handler path.src/features/issues/components/issue-finder.tsx (1)
135-135: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDedupe appended pages by issue ID.
Paginated live search results can overlap as issues move between pages; appending blindly can render duplicate cards and duplicate React keys.
Proposed append guard
- setIssues((prev) => [...prev, ...payload.issues]); + setIssues((prev) => { + const seen = new Set(prev.map((issue) => issue.id)); + return [...prev, ...payload.issues.filter((issue) => !seen.has(issue.id))]; + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/issues/components/issue-finder.tsx` at line 135, The issue list append in issue-finder should dedupe paginated results by issue ID instead of blindly concatenating pages, since overlapping pages can create duplicate cards and React keys. Update the setIssues callback to merge prev and payload.issues while filtering out items whose issue ID already exists in the accumulated list, using the unique issue identifier exposed by the issue objects in IssueFinder.tests/features/issues/server/github-search.test.ts (1)
103-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover a non-default page value.
This asserts the default path, but the pagination contract also needs a case like
page: 2to prove the service forwards caller-provided pages instead of always returning/requesting1.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/features/issues/server/github-search.test.ts` around lines 103 - 104, The github-search pagination test only covers the default page value, so add coverage for a caller-provided non-default page in the relevant test case for the search service. Update the assertions around the github search URL and result handling in github-search.test.ts to use a page like 2, and verify the service forwards that value into the request URL and preserves it on the returned result instead of always using 1.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/features/issues/components/issue-finder.tsx`:
- Around line 75-79: The new-search reset in issue-finder is only clearing the
rendered issues list, so stale `data` can still drive the query, total, and
`hasMore` state if the next request fails. Update the search reset path in
`IssueFinder` to also clear or invalidate the previous response state alongside
`setIssues([])` and `setPage(1)`, so the component no longer reads old `data`
after a fresh search begins.
- Around line 119-124: The “Load More” flow in issue-finder is reading the
current editable form values instead of the last submitted search criteria.
Update the search state in issue-finder.tsx so the initial submit snapshots the
submitted params, and have the load-more handler build its request from that
stored snapshot rather than tech/label/sort/linkedPr form state. Use the
existing issue-finder component handlers and URLSearchParams construction to
locate the submit/load-more logic and keep page increments tied to the last
successful search.
---
Nitpick comments:
In `@src/features/issues/components/issue-finder.tsx`:
- Line 135: The issue list append in issue-finder should dedupe paginated
results by issue ID instead of blindly concatenating pages, since overlapping
pages can create duplicate cards and React keys. Update the setIssues callback
to merge prev and payload.issues while filtering out items whose issue ID
already exists in the accumulated list, using the unique issue identifier
exposed by the issue objects in IssueFinder.
In `@tests/app/api/search/route.test.ts`:
- Line 52: The search route tests only cover the default page value, so add an
explicit query case for the route handler in the search API tests. Update the
existing test suite around the route’s request parsing to include a request with
?tech=React&page=3 and assert the downstream call receives page: 3, and
optionally add an invalid page case to verify normalization behavior in the same
handler path.
In `@tests/features/issues/server/github-search.test.ts`:
- Around line 103-104: The github-search pagination test only covers the default
page value, so add coverage for a caller-provided non-default page in the
relevant test case for the search service. Update the assertions around the
github search URL and result handling in github-search.test.ts to use a page
like 2, and verify the service forwards that value into the request URL and
preserves it on the returned result instead of always using 1.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 39c1ccff-da01-4a69-b4ee-0fa50daaaaae
📒 Files selected for processing (6)
src/app/api/search/route.tssrc/features/issues/components/issue-finder.tsxsrc/features/issues/server/github-search.tssrc/features/issues/types/search.tstests/app/api/search/route.test.tstests/features/issues/server/github-search.test.ts
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes