Repository navigation
feat(review-desk): Page the Review item list with a cursor - #2480
sentry-junior[bot] wants to merge 2 commits into
Conversation
The review desk list returned at most 100 items with no way to read more, so the archive cut off old items without a sign. The list API now takes `cursor` and `limit` and returns `nextCursor`, in the order updated_at desc, id desc. The cursor names the list filters, so it cannot page a different list. The review desk shows Load more. Co-Authored-By: David Cramer <david@sentry.io>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9547393. Configure here.
| const items = useMemo( | ||
| () => (query.data?.pages ?? []).flatMap((page) => page.items), | ||
| [query.data?.pages], | ||
| ); |
There was a problem hiding this comment.
Paged list can show duplicate items
Medium Severity
useReviewItemsData concatenates every loaded page with no id dedupe. After a review action, invalidateQueries refetches each page with its original cursor, so a removed item shifts page 1 down and the same item can appear again from page 2. The shared memory pager already collapses records by id for this reason.
Reviewed by Cursor Bugbot for commit 9547393. Configure here.
There was a problem hiding this comment.
Declining this one. On invalidate, TanStack Query v5 refetches an infinite query one page at a time. It takes the cursor of each page after the first from the page it just fetched, with getNextPageParam (see infiniteQueryBehavior.js). It does not reuse the old cursors. So when an item leaves page 1, page 2 starts after the new last item of page 1. Also, updated_at only goes up, so a changed item can only move above an old cursor, never below it. A dedupe by id would have nothing to remove.
The bar counted only the first page of open items. It now shows 50+ when more items follow, and its side panel can load them.
|
1 screenshot change — 1 changed · 0 added · 0 removed Review screenshots in Frameshift
|
The review desk list returned at most 100 items and had no way to read more. So the archive cut off old items, and nothing showed that it did.
GET /api/review-itemsnow pages:cursorandlimit(default 50, max 100). It returnsnextCursorwhen more items follow.updated_at desc, id desc, as before.searchReviewItemstakes anafterposition in that order.useInfiniteQueryand the sharedLoadMorePaginationbutton, like the memory page. The Conversation review bar uses the same hook.store.tschanges only touch search and order, so this PR can merge in either order with the work that moves the visibility rules intoaccess.ts.An item that changes while a person reads page 1 moves to the top, so page 2 does not show it. A refresh shows it again. Because
updated_atonly goes up, an item cannot show on two pages.Mock reporting still returns
{ items: [] }, which is valid with the optionalnextCursor, somock-reporting/routes.tsdoes not change.Checks:
pnpm typecheckandpnpm lintpass.vitest:tests/integration/api/review-items/actions.test.ts(extended with the paging contract) andtests/integration/review-item-tools.test.tspass.review-desk.spec.ts. Playwright could not find Chrome in the sandbox, so CI runs it.Part of the review desk cleanup for #2401.
via David Cramer.
--
View Session [Sentry]