Paginate superuser users/projects lists with server-side search#4474
Paginate superuser users/projects lists with server-side search#4474sakibsadmanshajib wants to merge 12 commits intoOpenFn:mainfrom
Conversation
a7f7859 to
fcf4b25
Compare
There was a problem hiding this comment.
Pull request overview
This PR improves superuser admin list performance by moving /settings/users and /settings/projects filtering/sorting/pagination from in-memory operations to database-backed queries with URL-param-driven state, and adds trigram indexes to support efficient search.
Changes:
- Added
Accounts.list_users_for_admin/1andProjects.list_projects_for_admin/1usingRepo.paginate/2with normalized params for safe filtering/sorting. - Updated Users/Projects LiveViews to treat URL params (
page,filter,sort,dir,page_size) as the source of truth and to use Scrivener pages in table rendering. - Added
pg_trgm+ GIN trigram indexes for key search columns, plus tests covering pagination and param safety.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/lightning_web/live/user_live_test.exs | Adds LiveView pagination navigation coverage for users index. |
| test/lightning_web/live/project_live_test.exs | Adds LiveView pagination navigation coverage for projects index. |
| test/lightning/projects_test.exs | Adds coverage for admin projects search/sort and invalid param fallback. |
| test/lightning/accounts_test.exs | Adds coverage for admin users search/sort and invalid param fallback. |
| priv/repo/migrations/20260226162000_add_admin_search_trgm_indexes.exs | Adds pg_trgm extension and trigram indexes for admin search paths. |
| lib/lightning_web/live/user_live/table_component.ex | Switches users table component to Scrivener-page-driven rendering + URL patching. |
| lib/lightning_web/live/user_live/index.html.heex | Passes normalized table_params down to the table component. |
| lib/lightning_web/live/user_live/index.ex | Normalizes/validates table URL params (sort/dir/page/page_size/filter). |
| lib/lightning_web/live/user_live/components.ex | Updates users table component API to accept page + pagination_path. |
| lib/lightning_web/live/project_live/index.html.heex | Switches projects table to use Scrivener page entries + pagination URL. |
| lib/lightning_web/live/project_live/index.ex | Adds param normalization and uses Projects.list_projects_for_admin/1 for server-side table data. |
| lib/lightning/projects.ex | Implements paginated/sortable/searchable admin projects query. |
| lib/lightning/accounts.ex | Implements paginated/sortable/searchable admin users query. |
| docs/plans/2026-02-26-superuser-users-projects-pagination-search-design.md | Documents design/approach and rollout considerations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
midigofrank
left a comment
There was a problem hiding this comment.
Hey @sakibsadmanshajib , great job here. The superuser interface needs some love, thank you for picking this up.
I have left some comments:
- I don't see a need for indexes on those tables. They're usually tiny
- Consider using using an embedded schema to have a single place for validations, like: https://github.com/OpenFn/lightning/blob/main/lib/lightning/workorders/search_params.ex
- With the embedded schema in place, you can try to refactor the list_users function, like: https://github.com/OpenFn/lightning/blob/main/lib/lightning/invocation.ex#L482
priv/repo/migrations/20260226162000_add_admin_search_trgm_indexes.exs
Outdated
Show resolved
Hide resolved
queries/live views refactored to use them
midigofrank
left a comment
There was a problem hiding this comment.
Hey @sakibsadmanshajib , sorry I took so long to get back to you. You're on the right track. I have left inline comments, but here is the tldr:
- Use standard field names, like those in https://github.com/OpenFn/lightning/blob/main/lib/lightning/workorders/search_params.ex . Use
search_terminstead offilter,sort --> sort_by,dir --> sort_direction - Boolean state fields like
role,enabled, andsupport_userdon't need to be in the sort list, these are better handled as toggles in the form. We can track that work in a separate follow-up issue. - Ecto changesets can handle all the normalizations you need, so the custom
normalize_valuefunction isn't necessary. - Context functions should receive the struct directly rather than raw map params
- I've suggested some renaming —
adminconflicts with theProjectUserrole, so we'll want to resolve that
Please go through the comments and apply the same changes to the projects context. Note that I haven't reviewed the LiveViews yet, II'll do that in the next round once the contexts are updated. In the meantime, it's worth looking at how the workorders page uses the filter changeset (https://github.com/OpenFn/lightning/tree/main/lib/lightning_web/live/run_live) and applying the same pattern to these pages.
Thanks again for picking this up, don't hesitate to reach out if anything is unclear.
There was a problem hiding this comment.
You don't need to stringify and normalize while using ecto changesets. The changesets will do this for you, look at https://github.com/OpenFn/lightning/blob/main/lib/lightning/workorders/search_params.ex. You can read more about Ecto changesets in https://hexdocs.pm/ecto/Ecto.Changeset.html
There was a problem hiding this comment.
Also, could you use the same field names as the workorders search params? The filter --> search_term, sort --> sort_by, dir --> sort_direction
There was a problem hiding this comment.
You don't need to normalize these values while using Ecto Changeset
| server-side filtering and sorting. | ||
| """ | ||
| @spec list_users_for_admin(map()) :: Scrivener.Page.t() | ||
| def list_users_for_admin(params \\ %{}) do |
There was a problem hiding this comment.
| def list_users_for_admin(params \\ %{}) do | |
| def list_all_users(%AdminSearchParams{} = params) do |
The context function should take the typed params instead of building it inside the body. This way we establish a "contract" with the callers.
We should also rename the function, to list_all_users or list_users_for_superuser. admin conflicts with the ProjectUser role
| do: order_by(query, [u], asc_nulls_last: u.scheduled_deletion) | ||
|
|
||
| defp order_admin_users(query, "scheduled_deletion", "desc"), | ||
| do: order_by(query, [u], desc_nulls_first: u.scheduled_deletion) |
There was a problem hiding this comment.
| do: order_by(query, [u], desc_nulls_first: u.scheduled_deletion) | |
| do: order_by(query, [u], desc_nulls_last: u.scheduled_deletion) |
Given null are non deleted users, we should show them last
| ) | ||
| end | ||
|
|
||
| defp order_admin_users(query, "enabled", "asc"), |
There was a problem hiding this comment.
I don't think we need to sort by a boolean. This should be toggle somewhere on the form. Let's skip it for now
|
|
||
| User | ||
| |> filter_admin_users(params.filter) | ||
| |> order_admin_users(params.sort, params.dir) |
There was a problem hiding this comment.
| |> order_admin_users(params.sort, params.dir) | |
| |> sort_users_query(params.sort, params.dir) |
|
|
||
| defp filter_admin_users(query, ""), do: query | ||
|
|
||
| defp filter_admin_users(query, filter) do |
There was a problem hiding this comment.
| defp filter_admin_users(query, filter) do | |
| defp search_users_query(query, search_term) do |
| defp order_admin_users(query, "scheduled_deletion", "desc"), | ||
| do: order_by(query, [u], desc_nulls_first: u.scheduled_deletion) | ||
|
|
||
| defp order_admin_users(query, "role", dir) do |
There was a problem hiding this comment.
I also don't see the need to sort by role. There are only 2 types of users, :user and :superuser. This should be a toggle
| @primary_key false | ||
|
|
||
| @default_sort "email" | ||
| @allowed_sorts ~w(first_name last_name email role enabled support_user scheduled_deletion) |
There was a problem hiding this comment.
| @allowed_sorts ~w(first_name last_name email role enabled support_user scheduled_deletion) | |
| @allowed_sorts ~w(first_name last_name email scheduled_deletion) |
| defp order_admin_users(query, "enabled", "desc"), | ||
| do: order_by(query, [u], asc: u.disabled) | ||
|
|
||
| defp order_admin_users(query, "scheduled_deletion", "asc"), |
There was a problem hiding this comment.
| defp order_admin_users(query, "scheduled_deletion", "asc"), | |
| defp sort_users_query(query, "scheduled_deletion", "asc"), |
Description
This PR adds server-side pagination, sorting, and search to the superuser users/projects settings list views, and moves those list operations out of in-memory filtering into SQL-backed queries.
Closes #2913
Validation steps
mix formatmix test test/lightning/accounts_test.exs test/lightning/projects_test.exs test/lightning_web/live/user_live_test.exs test/lightning_web/live/project_live_test.exs --seed 0mix test test/lightning_web/live/audit_live_test.exs --seed 0Additional notes for the reviewer
20260226162000_add_admin_search_trgm_indexes.exs.page,filter,sort,dir) for both users and projects.AI Usage
Please disclose whether you've used AI anywhere in this PR (it's cool, we just
want to know!):
Pre-submission checklist
/reviewwith Claude Code):owner,:admin,:editor,:viewer)