feat: User pagination using offsets #1062
Conversation
- Currently "before" does not work - Filters not added yet
coderd/database/queries/users.sql
Outdated
| users | ||
| WHERE | ||
| CASE | ||
| -- This allows using the last element on a page as effectively a cursor. | ||
| -- This is an important option for scripts that need to paginate without | ||
| -- duplicating or missing data. | ||
| WHEN @created_after :: timestamp with time zone != '0001-01-01 00:00:00+00' THEN created_at > @created_after | ||
| ELSE true | ||
| END | ||
| AND CASE | ||
| WHEN @search_email :: text != '' THEN email LIKE concat('%', @search_email, '%') | ||
| ELSE true | ||
| END | ||
| ORDER BY | ||
| created_at ASC OFFSET @offset_opt | ||
| LIMIT | ||
| -- A null limit means "no limit", so -1 means return all | ||
| NULLIF(@limit_opt :: int, -1); |
There was a problem hiding this comment.
The sql formatter was pretty set on how this looks. I'm not super happy with it, but I couldn't get the formatter to look different even with comments. Some things I don't like:
- Would prefer
order byandoffsetto be on different lines. They kinda merge together - The Case
WHEN/THENis all on 1 line making it quite long.
Codecov Report
@@ Coverage Diff @@
## main #1062 +/- ##
==========================================
+ Coverage 66.53% 66.65% +0.11%
==========================================
Files 255 255
Lines 15675 15743 +68
Branches 152 152
==========================================
+ Hits 10430 10494 +64
- Misses 4178 4183 +5
+ Partials 1067 1066 -1
Continue to review full report at Codecov.
|
coderd/coderd.go
Outdated
| // APIRateLimit is the minutely throughput rate limit per user or ip. | ||
| // Setting a rate limit <0 will disable the rate limiter across the entire | ||
| // app. Specific routes may have their own limiters. | ||
| APIRateLimit int |
There was a problem hiding this comment.
Helpful to disable during a unit test.
codersdk/users.go
Outdated
| @@ -13,6 +14,26 @@ import ( | |||
| // Me is used as a replacement for your own ID. | |||
| var Me = uuid.Nil | |||
|
|
|||
| type PaginatedUsersRequest struct { | |||
There was a problem hiding this comment.
What do you think about renaming all the PaginatedUsers stuff to Users? Since the endpoint is paginated anyways, seems like it could be implicit.
codersdk/users.go
Outdated
| } | ||
|
|
||
| type PaginatedUsers struct { | ||
| Pager PagerFields `json:"pager"` |
There was a problem hiding this comment.
Do we need to return this at all? Or could we keep it similar to before as a top-level array? I believe both parameters are specified by the user anyways. Once we add the totals we may have to increase it, but it's possible things like TotalUsers will be added to Organization instead.
There was a problem hiding this comment.
As for the counts, unfortunately a count like that is not sufficient, as you need to match the search and after_user params. Those affect the resulting count. So if it's a separate query, you have to accept the same arguments and make two calls 🤷. Not terrible, just not ideal.
Bruno is flying right now, but what I can do is drop this and return []users. When the UI comes, if there is a need to display pages, we'll revisit. It's not hard to come back to.
tl;dr: I'll drop it and go []users as the return for now
What does this do
Adds offset pagination to the user query. Also has
search_namesearch field (v1 parity) that searches usernames and emails.Supports cursor pagination using
after_userquery param.This also drops sql linting. Sql linting was not working with sqlc type arguments and kept destroying my queries making them invalid sqlc queries.
Request GET
/api/v2/usersQuery params
after_user: a uuid that returns all users after the argument. All users are sorted in a global order by their creation time.limit: maximum number of users returnedoffset: returns the[offset:]users from the list. This allows pagination, pagenisoffset = <page> * <limit>search_name: a string that filters all users, only showing users that have this substring in theirusername,email, ornameResponse