Skip to content
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ and this project adheres to

- Editors can now provision and merge sandboxes; merge checks editor+ role on
the target project [#4384](https://github.com/OpenFn/lightning/issues/4384)
- Superuser users and projects settings lists now use server-side pagination,
sorting, and search for better performance
[#2913](https://github.com/OpenFn/lightning/issues/2913)
- Show specific workflow names in sandbox merge dialog when target project has
diverged, instead of generic warning message
[#4001](https://github.com/OpenFn/lightning/issues/4001)
Expand Down
128 changes: 128 additions & 0 deletions lib/lightning/accounts.ex
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,31 @@ defmodule Lightning.Accounts do
Repo.all(User)
end

@admin_user_default_sort "email"
@admin_user_allowed_sorts ~w(first_name last_name email role enabled support_user scheduled_deletion)
@admin_user_default_page_size 10
@admin_user_max_page_size 100

@doc """
Returns a paginated list of users for the superuser admin table with
server-side filtering and sorting.
"""
@spec list_users_for_admin(map()) :: Scrivener.Page.t()
def list_users_for_admin(params \\ %{}) do
Comment thread
sakibsadmanshajib marked this conversation as resolved.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
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

%{
filter: filter,
sort: sort,
dir: dir,
page: page,
page_size: page_size
} = normalize_admin_user_params(params)

User
|> filter_admin_users(filter)
|> order_admin_users(sort, dir)
|> Repo.paginate(page: page, page_size: page_size)
end

@doc """
Returns the list of users with the given emails
"""
Expand Down Expand Up @@ -160,6 +185,109 @@ defmodule Lightning.Accounts do
if User.valid_password?(user, password), do: user
end

defp normalize_admin_user_params(params) do
params = stringify_param_keys(params)

sort = normalize_admin_sort(Map.get(params, "sort"))
dir = normalize_admin_dir(Map.get(params, "dir"))
page = parse_positive_int(Map.get(params, "page"), 1)

page_size =
Map.get(params, "page_size")
|> parse_positive_int(@admin_user_default_page_size)
|> min(@admin_user_max_page_size)

%{
filter: normalize_admin_filter(Map.get(params, "filter")),
sort: sort,
dir: dir,
page: page,
page_size: page_size
}
end

defp filter_admin_users(query, ""), do: query

defp filter_admin_users(query, filter) do
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
defp filter_admin_users(query, filter) do
defp search_users_query(query, search_term) do

search = "%#{filter}%"

where(
query,
[u],
ilike(u.first_name, ^search) or
ilike(u.last_name, ^search) or
ilike(fragment("?::text", u.email), ^search) or
ilike(fragment("?::text", u.role), ^search)
)
end

defp order_admin_users(query, "enabled", "asc"),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

do: order_by(query, [u], desc: u.disabled)

defp order_admin_users(query, "enabled", "desc"),
do: order_by(query, [u], asc: u.disabled)

defp order_admin_users(query, "scheduled_deletion", "asc"),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
defp order_admin_users(query, "scheduled_deletion", "asc"),
defp sort_users_query(query, "scheduled_deletion", "asc"),

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)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
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


defp order_admin_users(query, "role", dir) do
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

direction = dir_to_atom(dir)
order_by(query, [u], [{^direction, fragment("?::text", u.role)}])
end

defp order_admin_users(query, sort, dir) do
direction = dir_to_atom(dir)
sort_field = String.to_existing_atom(sort)

order_by(query, [u], [{^direction, field(u, ^sort_field)}])
end

defp normalize_admin_sort(sort) when is_binary(sort) do
if sort in @admin_user_allowed_sorts,
do: sort,
else: @admin_user_default_sort
end

defp normalize_admin_sort(sort) when is_atom(sort) do
sort
|> Atom.to_string()
|> normalize_admin_sort()
end

defp normalize_admin_sort(_), do: @admin_user_default_sort

defp normalize_admin_dir(dir) when dir in ["asc", :asc], do: "asc"
defp normalize_admin_dir(dir) when dir in ["desc", :desc], do: "desc"
defp normalize_admin_dir(_), do: "asc"

defp normalize_admin_filter(nil), do: ""

defp normalize_admin_filter(filter) do
filter
|> to_string()
|> String.trim()
end

defp stringify_param_keys(params) when is_map(params) do
Map.new(params, fn {key, value} -> {to_string(key), value} end)
end

defp parse_positive_int(value, _default) when is_integer(value) and value > 0,
do: value

defp parse_positive_int(value, default) do
case Integer.parse(to_string(value || "")) do
{int, ""} when int > 0 -> int
_ -> default
end
end

defp dir_to_atom("asc"), do: :asc
defp dir_to_atom("desc"), do: :desc

@doc """
Gets a single user.

Expand Down
135 changes: 135 additions & 0 deletions lib/lightning/projects.ex
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,34 @@ defmodule Lightning.Projects do
Repo.all(from(p in Project, order_by: p.name))
end

@admin_project_default_sort "name"
@admin_project_allowed_sorts ~w(name inserted_at description owner scheduled_deletion)
@admin_project_default_page_size 10
@admin_project_max_page_size 100

@doc """
Returns a paginated list of projects for the superuser admin table with
server-side filtering and sorting.
"""
@spec list_projects_for_admin(map()) :: Scrivener.Page.t()
def list_projects_for_admin(params \\ %{}) do
%{
filter: filter,
sort: sort,
dir: dir,
page: page,
page_size: page_size
} = normalize_admin_project_params(params)

Project
|> join(:left, [p], pu in assoc(p, :project_users), on: pu.role == :owner)
|> join(:left, [_p, pu], owner in assoc(pu, :user))
|> preload([_p, _pu, _owner], project_users: :user)
|> filter_admin_projects(filter)
|> order_admin_projects(sort, dir)
|> Repo.paginate(page: page, page_size: page_size)
end

@doc """
Lists all projects that have history retention
"""
Expand Down Expand Up @@ -220,6 +248,113 @@ defmodule Lightning.Projects do
end
end

defp normalize_admin_project_params(params) do
params = stringify_param_keys(params)

sort = normalize_admin_project_sort(Map.get(params, "sort"))
dir = normalize_admin_project_dir(Map.get(params, "dir"))
page = parse_positive_int(Map.get(params, "page"), 1)

page_size =
Map.get(params, "page_size")
|> parse_positive_int(@admin_project_default_page_size)
|> min(@admin_project_max_page_size)

%{
filter: normalize_admin_project_filter(Map.get(params, "filter")),
sort: sort,
dir: dir,
page: page,
page_size: page_size
}
end

defp filter_admin_projects(query, ""), do: query

defp filter_admin_projects(query, filter) do
search = "%#{filter}%"

where(
query,
[p, _pu, owner],
ilike(p.name, ^search) or
ilike(p.description, ^search) or
ilike(
fragment("concat_ws(' ', ?, ?)", owner.first_name, owner.last_name),
^search
)
)
end

defp order_admin_projects(query, "scheduled_deletion", "asc"),
do: order_by(query, [p, _pu, _owner], asc_nulls_last: p.scheduled_deletion)

defp order_admin_projects(query, "scheduled_deletion", "desc"),
do: order_by(query, [p, _pu, _owner], desc_nulls_first: p.scheduled_deletion)

defp order_admin_projects(query, "owner", dir) do
direction = project_dir_to_atom(dir)

order_by(
query,
[_p, _pu, owner],
[
{^direction,
fragment("concat_ws(' ', ?, ?)", owner.first_name, owner.last_name)}
]
)
end

defp order_admin_projects(query, sort, dir) do
direction = project_dir_to_atom(dir)
sort_field = String.to_existing_atom(sort)

order_by(query, [p, _pu, _owner], [{^direction, field(p, ^sort_field)}])
end

defp normalize_admin_project_sort(sort) when is_binary(sort) do
if sort in @admin_project_allowed_sorts,
do: sort,
else: @admin_project_default_sort
end

defp normalize_admin_project_sort(sort) when is_atom(sort) do
sort
|> Atom.to_string()
|> normalize_admin_project_sort()
end

defp normalize_admin_project_sort(_), do: @admin_project_default_sort

defp normalize_admin_project_dir(dir) when dir in ["asc", :asc], do: "asc"
defp normalize_admin_project_dir(dir) when dir in ["desc", :desc], do: "desc"
defp normalize_admin_project_dir(_), do: "asc"

defp normalize_admin_project_filter(nil), do: ""

defp normalize_admin_project_filter(filter) do
filter
|> to_string()
|> String.trim()
end

defp stringify_param_keys(params) when is_map(params) do
Map.new(params, fn {key, value} -> {to_string(key), value} end)
end

defp parse_positive_int(value, _default) when is_integer(value) and value > 0,
do: value

defp parse_positive_int(value, default) do
case Integer.parse(to_string(value || "")) do
{int, ""} when int > 0 -> int
_ -> default
end
end

defp project_dir_to_atom("asc"), do: :asc
defp project_dir_to_atom("desc"), do: :desc

@doc """
Gets the project associated with a run.
Traverses Run → WorkOrder → Workflow → Project.
Expand Down
Loading