Skip to content

waleedlatif1/hangzhou v2#3647

Merged
waleedlatif1 merged 2 commits intostagingfrom
waleedlatif1/hangzhou-v2
Mar 18, 2026
Merged

waleedlatif1/hangzhou v2#3647
waleedlatif1 merged 2 commits intostagingfrom
waleedlatif1/hangzhou-v2

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

  • Search: Replaces the Load Users button with a live search input. Email search uses listUsers with a contains operator; pasting a UUID switches to admin.getUser for exact ID lookup. Pagination resets on each new search.
  • Border fix: Removes the outer border-[var(--border-secondary)] on the user table container, which was rendering white in dark mode.

Test plan

  • Search by partial email returns matching users
  • Paste a full user UUID returns exactly that user
  • No white border around the user table in dark mode
  • Pagination still works across email search results

waleedlatif1 and others added 2 commits March 17, 2026 17:38
- Replace Load Users button with a live search input; query fires on any input
- Email search uses listUsers with contains operator
- User ID search (UUID format) uses admin.getUser directly for exact lookup
- Remove outer border on user table that rendered white in dark mode
- Reset pagination to page 0 on new search

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Split searchInput (controlled input) from searchQuery (committed value)
  so the hook only fires on Search click or Enter, not every keystroke
- Gate table render on searchQuery.length > 0 to prevent stale results
  showing after input is cleared

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@cursor
Copy link

cursor bot commented Mar 18, 2026

You have used all Bugbot PR reviews included in your free trial for your GitHub account on this workspace.

To continue using Bugbot reviews, enable Bugbot for your team in the Cursor dashboard.

@vercel
Copy link

vercel bot commented Mar 18, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Mar 18, 2026 1:00am

Request Review

@waleedlatif1 waleedlatif1 merged commit 2bc11a7 into staging Mar 18, 2026
18 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/hangzhou-v2 branch March 18, 2026 01:00
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 18, 2026

Greptile Summary

This PR replaces the manual "Load Users" button in the admin panel with a persistent search input that routes to admin.getUser for UUID lookups and admin.listUsers with a contains email filter for partial searches. It also removes an outer border-[var(--border-secondary)] that was rendering white in dark mode.

The implementation is clean and well-structured: dual searchInput/searchQuery state correctly decouples live typing from query execution, the UUID regex is robust, mapUser removes mapping duplication, and pagination resets appropriately on new searches.

Issues found:

  • The Search button remains enabled when searchInput is empty; clicking it (or pressing Enter on an empty field) calls handleSearch, resets usersOffset, sets searchQuery to '', and clears any visible results — likely unintended UX.
  • isLoading (isPending && isFetching) is used to disable the Search button. For background re-fetches of an already-cached query key (after the 30 s staleTime window), isLoading stays false while the request is in-flight, so the button is not disabled. Using isFetching would cover all in-flight states.

Confidence Score: 4/5

  • Safe to merge with minor UX polish — no data loss or security risk; issues are limited to edge-case button states.
  • Core search logic is correct, query key invalidation is sound, UUID vs email routing works as described, and the border fix is straightforward. The two flagged items (empty-input search triggering result clear, isLoading vs isFetching) are UX rough edges rather than functional bugs.
  • No files require special attention; the minor issues are both in admin.tsx around the Search button's disabled condition.

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/settings/components/admin/admin.tsx Replaces "Load Users" button with a search input + Search button, adds searchInput/searchQuery dual-state pattern for committing queries on Enter or button click, and removes the dark-mode-breaking outer border. Core logic is sound; the Search button should be disabled when the input is empty to avoid accidentally clearing visible results.
apps/sim/hooks/queries/admin-users.ts Adds UUID detection via regex to route to admin.getUser for exact lookup vs admin.listUsers with contains operator for email search; extracts mapUser helper to remove duplication; updates query key factory and hook signature to accept searchQuery. Implementation is clean; minor note that isLoading (vs isFetching) may not cover background re-fetch scenarios.

Sequence Diagram

sequenceDiagram
    actor Admin
    participant UI as Admin Component
    participant Hook as useAdminUsers
    participant API as Better-Auth Client

    Admin->>UI: Types in search input
    Admin->>UI: Presses Enter / clicks Search
    UI->>UI: handleSearch() — sets searchQuery, resets offset

    alt searchQuery matches UUID regex
        Hook->>API: admin.getUser({ id: searchQuery })
        API-->>Hook: { data: single user }
        Hook-->>UI: { users: [user], total: 1 }
    else searchQuery is email / partial
        Hook->>API: admin.listUsers({ searchField: 'email', searchOperator: 'contains', searchValue: searchQuery, limit, offset })
        API-->>Hook: { data: { users[], total } }
        Hook-->>UI: { users[], total }
    end

    UI->>Admin: Renders user table (gated on searchQuery.length > 0)

    opt Paginate
        Admin->>UI: Clicks Previous / Next
        UI->>UI: setUsersOffset(prev ± PAGE_SIZE)
        Hook->>API: admin.listUsers({ ..., offset: new offset })
        API-->>Hook: next page of results
        Hook-->>UI: updated users
    end
Loading

Last reviewed commit: "fix(admin): replace ..."

Comment on lines +144 to 146
<Button variant='primary' onClick={handleSearch} disabled={usersLoading}>
{usersLoading ? 'Searching...' : 'Search'}
</Button>
Copy link
Contributor

Choose a reason for hiding this comment

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

P2 Search button enabled on empty input

The Search button is currently enabled whenever usersLoading is false, even when searchInput is empty. Clicking Search with an empty (or whitespace-only) input will call handleSearch, reset usersOffset to 0, and set searchQuery to '' — causing any previously displayed results to immediately disappear (since the table is gated on searchQuery.length > 0). Adding || !searchInput.trim() to the disabled condition would prevent this accidental result-clearing and avoid unnecessary state updates.

Suggested change
<Button variant='primary' onClick={handleSearch} disabled={usersLoading}>
{usersLoading ? 'Searching...' : 'Search'}
</Button>
<Button variant='primary' onClick={handleSearch} disabled={usersLoading || !searchInput.trim()}>
{usersLoading ? 'Searching...' : 'Search'}

Comment on lines 77 to 83
return useQuery({
queryKey: adminUserKeys.list(offset, limit),
queryFn: () => fetchAdminUsers(offset, limit),
enabled,
queryKey: adminUserKeys.list(offset, limit, searchQuery),
queryFn: () => fetchAdminUsers(offset, limit, searchQuery),
enabled: searchQuery.length > 0,
staleTime: 30 * 1000,
placeholderData: keepPreviousData,
})
Copy link
Contributor

Choose a reason for hiding this comment

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

P2 isLoading misses background re-fetches on the Search button

usersLoading maps to isLoading (isPending && isFetching). For subsequent searches against a new query key this is fine (no cached data → isPending is true). However, if the exact same searchQuery is submitted again after the 30-second staleTime window elapses, the query re-fetches in the background: isFetching is true but isPending is false, so isLoading stays false and the button is not disabled. Using isFetching instead provides a consistent disabled state during every in-flight request:

Suggested change
return useQuery({
queryKey: adminUserKeys.list(offset, limit),
queryFn: () => fetchAdminUsers(offset, limit),
enabled,
queryKey: adminUserKeys.list(offset, limit, searchQuery),
queryFn: () => fetchAdminUsers(offset, limit, searchQuery),
enabled: searchQuery.length > 0,
staleTime: 30 * 1000,
placeholderData: keepPreviousData,
})
} = useAdminUsers(usersOffset, PAGE_SIZE, searchQuery)

In admin-users.ts, expose isFetching alongside isLoading (or return the full query object), and in the component use it:

const {
  data: usersData,
  isLoading: usersLoading,
  isFetching: usersFetching,
  error: usersError,
} = useAdminUsers(usersOffset, PAGE_SIZE, searchQuery)

// ...

<Button variant='primary' onClick={handleSearch} disabled={usersFetching || !searchInput.trim()}>
  {usersFetching ? 'Searching...' : 'Search'}

<EmcnInput
value={searchInput}
onChange={(e) => setSearchInput(e.target.value)}
onKeyDown={(e) => e.key === 'Enter' && handleSearch()}
Copy link
Contributor

Choose a reason for hiding this comment

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

P2 Enter key fires on empty input

The onKeyDown handler fires handleSearch when Enter is pressed regardless of whether searchInput has content. This has the same side-effect as the empty button click: it resets usersOffset to 0 and sets searchQuery to '', making any previously visible results disappear. Adding a searchInput.trim() guard before calling handleSearch keeps keyboard and click behaviour consistent.

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.

1 participant