Skip to content

fix(users): use cursor-paginated server-side search in user and task pickers - #551

Merged
pikann merged 2 commits into
masterfrom
fix/use-cursor-paginated-search-in-user-and-task-pickers
Oct 6, 2026
Merged

pikann merged 2 commits into
masterfrom
fix/use-cursor-paginated-search-in-user-and-task-pickers

Conversation

@pikann

@pikann pikann commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #547. The Add Member dialog only filtered the first 20 loaded users in the browser, so anyone beyond the first page could never be found. This adds a cursor-paginated users endpoint and moves the search to the server. The task-link modal had the same flaw in milder form (it preloaded up to 5,000 tasks and filtered them client-side) and gets the same treatment.

Changes

API

  • New GET /admin/users/cursor (requires users.read). Query params: page_size (default 20, max 100), cursor, search, role. Returns { items, page_size, next_cursor }; next_cursor is null on the last page.
  • The cursor is opaque and holds only the last user's id. The repository resumes after that row in the existing name ordering, so it stays valid if that user is deleted between pages.
  • An invalid cursor returns 400 USER_INVALID_CURSOR.
  • GET /admin/users is unchanged, because the admin users page relies on its total and must_change_password_count.

Web

  • Add Member dialog: debounced server-side search, infinite scroll via next_cursor, and it keeps loading pages until a few non-member rows are visible.
  • Task-link modal: cursor-paginated infinite scroll (20 per page) with debounced server-side search, replacing the load-everything-then-filter approach.

Behaviour change

The task-link modal's search now matches on the server against title and #<number> (e.g. #12). Typing the full prefixed id such as PRJ-12 no longer matches, which the old client-side filter did.

Testing

  • Go: handler tests for the new route (next cursor, last page, invalid cursor), cursor codec tests; go build, go vet and the unit and integration suites pass.
  • Verified ListAfter against a dev Postgres: paging 29 users in pages of 7, with and without a search filter, returns the same order as the offset-based List.
  • Web: tsc -b, biome and the full vitest suite pass (970 tests), including new admin-api cursor tests.
  • Not run: the dialogs in a browser, and e2e specs.

@pikann pikann changed the title fix(users): use cursor-paginated server-side search in user and task … fix(users): use cursor-paginated server-side search in user and task pickers Oct 6, 2026

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — one minor suggestion inline, plus a test-coverage nitpick.

Reviewed changes

  • New cursor users endpoint — GET /admin/users/cursor (requires users.read) returns { items, page_size, next_cursor } backed by a new UserRepository.ListAfter keyset query; invalid cursors map to 400 USER_INVALID_CURSOR. GET /admin/users is left intact for the admin page's total/must_change_password_count.
  • Add Member dialog moved to server-side search — the user infinite query is now keyed on a debounced search string and paginates the cursor endpoint, with an effect that keeps loading until a few non-member rows are visible.
  • Task-link modal rewritten — replaces the 5,000-task preload/client filter with a cursor-paginated useInfiniteQuery over listAllTasks using server-side search, cursor, and 20-per-page.

The keyset ordering (LOWER(COALESCE(NULLIF(full_name,''),username)), LOWER(username), id) matches List, the cursor pivot is looked up without the deleted_at filter so soft-deletes don't invalidate it, and limit+1 correctly drives hasMore. Argument placeholder math checks out in both branches.

ℹ️ Nitpicks

  • No repository-level test covers ListAfter. The keyset SQL — the most failure-prone piece, and the one the PR notes was only checked by hand against dev Postgres — has no automated coverage (user_repository_test.go tests List but not ListAfter). A paging test there would lock in the "same order as List, no gaps/dupes across pages" contract.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

Reviewed the delta since the prior pullfrog review (c3956dab → 2331c34).

  • Cancelled pending debounces on close — both the Add Member dialog and the task-link modal call their debounced setter with "" on close, superseding a queued timer so it can no longer restore a stale search after reset. This resolves the prior inline concern.
  • Hardened the users cursor — ListAfter now returns ErrInvalidCursor (400 USER_INVALID_CURSOR) for a decodable but unknown id, while still treating soft-deleted rows as existing; previously such a cursor silently yielded an empty page.
  • Added error and paging guards — the task-link modal distinguishes load error from empty, and both pickers gate their auto-load-to-5 effect on isFetchNextPageError and isPlaceholderData, with keepPreviousData smoothing search transitions.
  • Restored prefixed-id search — toServerSearch rewrites a display id like PRJ-12 to #12 before the server call, matching the backend's title / #<number> search.

Note: that last change means the PR description's "Behaviour change" paragraph (which states PRJ-12 no longer matches) is now stale.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@pikann
pikann merged commit 6783194 into master Oct 6, 2026
7 checks passed
@pikann
pikann deleted the fix/use-cursor-paginated-search-in-user-and-task-pickers branch October 6, 2026 04:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Add Member search can't find users beyond the first 20

1 participant