Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1430,6 +1430,13 @@ Migrations in this release, all in `priv/repo/migrations/`:

### Added

- Pagination (10 items per page) for credentials, keychain credentials, and
OAuth clients on the credentials page (`/credentials`) and the project
settings credentials tab. Each table paginates independently via URL
parameters. OAuth clients are behind a collapsible section, collapsed by
default. Tables are ordered: credentials first, keychain credentials second
(project settings only), OAuth clients last.
[#4301](https://github.com/OpenFn/lightning/issues/4301)
- Add support for sync v2 protocol
[#4523](https://github.com/OpenFn/lightning/issues/4523)
- Support collections in sandboxes. Collection names are now scoped per project,
Expand Down
38 changes: 38 additions & 0 deletions lib/lightning/credentials.ex
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,32 @@ defmodule Lightning.Credentials do
|> Repo.all()
end

@spec list_credentials(Project.t(), map()) :: Scrivener.Page.t()
def list_credentials(%Project{} = project, page_params) do
query =
from c in Credential,
join: pc in assoc(c, :project_credentials),
on: pc.project_id == ^project.id,
preload: [
:user,
:project_credentials,
:projects,
:credential_bodies,
:oauth_client
],
order_by: [asc: fragment("lower(?)", c.name)],
group_by: c.id

Repo.paginate(query, page_params)
end

@spec list_credentials(User.t(), map()) :: Scrivener.Page.t()
def list_credentials(%User{id: user_id}, page_params) do
list_credentials_query(user_id)
|> order_by([c], asc: fragment("lower(?)", c.name))
|> Repo.paginate(page_params)
end

defp list_credentials_query(user_id) do
from(c in Credential,
where: c.user_id == ^user_id,
Expand Down Expand Up @@ -1918,6 +1944,18 @@ defmodule Lightning.Credentials do
|> Repo.all()
end

def list_keychain_credentials_for_project(
%Project{id: project_id},
page_params
) do
from(kc in KeychainCredential,
where: kc.project_id == ^project_id,
order_by: [asc: fragment("lower(?)", kc.name)],
preload: [:project, :created_by, :default_credential]
)
|> Repo.paginate(page_params)
end

@doc """
Gets a single keychain credential.

Expand Down
31 changes: 31 additions & 0 deletions lib/lightning/oauth_clients.ex
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,37 @@ defmodule Lightning.OauthClients do
|> Repo.all()
end

def list_clients(%Project{} = project, page_params) do
global_clients_subquery =
from(c in OauthClient,
where: c.global == true,
select: c.id
)

clients_query =
from(c in OauthClient,
left_join: poc in ProjectOauthClient,
on: poc.oauth_client_id == c.id,
where:
poc.project_id == ^project.id or
c.id in subquery(global_clients_subquery),
preload: [:user, :project_oauth_clients, :projects],
order_by: [asc: fragment("lower(?)", c.name)],
group_by: c.id
)

Repo.paginate(clients_query, page_params)
end

def list_clients(%User{id: user_id}, page_params) do
from(c in OauthClient,
where: c.user_id == ^user_id or c.global,
preload: :projects,
order_by: [asc: fragment("lower(?)", c.name)]
)
|> Repo.paginate(page_params)
end

@doc """
Retrieves a single OAuth client by its ID, raising an error if not found.

Expand Down
2 changes: 1 addition & 1 deletion lib/lightning/scrivener/query_paginater.ex
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ defimpl Scrivener.Paginater, for: Ecto.Query do
defp aggregate(
%{
group_bys: [
%Ecto.Query.QueryExpr{
%{

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.

This one I'd like to think through with you. Loosening %Ecto.Query.QueryExpr{} to a bare %{expr: ...} works, but it now matches any map with an expr key rather than the actual struct. I think the precise fix is %Ecto.Query.ByExpr{}, that's what group_by builds in Ecto 3.13, which keeps the type guard. It's also a shared file every grouped-paginated query flows through, with no test hitting it directly. Could we name the struct and add a small test for the grouped-count path? More a robustness worry than a blocker.

expr: [
{{:., [], [{:&, [], [source_index]}, field]}, [], []} | _
]
Expand Down
6 changes: 6 additions & 0 deletions lib/lightning_web/live/components/credentials.ex
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ defmodule LightningWeb.Components.Credentials do
attr :can_create_project_credential, :any, required: true
attr :show_owner_in_tables, :boolean, default: false
attr :return_to, :string, required: true
attr :credentials_page, :map, default: nil
attr :keychain_credentials_page, :map, default: nil
attr :oauth_clients_page, :map, default: nil
attr :credentials_url, :any, default: nil
attr :keychain_url, :any, default: nil
attr :oauth_clients_url, :any, default: nil

def credentials_index_live_component(assigns) do
~H"""
Expand Down
15 changes: 11 additions & 4 deletions lib/lightning_web/live/components/data_tables.ex
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ defmodule LightningWeb.Components.DataTables do
attr :title, :string, required: true
attr :display_table_title, :boolean, default: true
attr :show_owner, :boolean, default: false
attr :page, :map, default: nil
attr :url, :any, default: nil

slot :actions,
doc: "the slot for showing user actions in the last table column"
Expand All @@ -30,7 +32,7 @@ defmodule LightningWeb.Components.DataTables do
<%= if Enum.empty?(@credentials) do %>
{render_slot(@empty_state)}
<% else %>
<.table id={"#{@id}-table"}>
<.table id={"#{@id}-table"} page={@page} url={@url}>
<:header>
<.tr>
<.th>Name</.th>
Expand Down Expand Up @@ -125,6 +127,8 @@ defmodule LightningWeb.Components.DataTables do
attr :title, :string, required: true
attr :display_table_title, :boolean, default: true
attr :show_owner, :boolean, default: false
attr :page, :map, default: nil
attr :url, :any, default: nil

slot :actions,
doc: "the slot for showing user actions in the last table column"
Expand All @@ -141,7 +145,7 @@ defmodule LightningWeb.Components.DataTables do
<%= if Enum.empty?(@keychain_credentials) do %>
{render_slot(@empty_state)}
<% else %>
<.table id={"#{@id}-table"}>
<.table id={"#{@id}-table"} page={@page} url={@url}>
<:header>
<.tr>
<.th>Name</.th>
Expand Down Expand Up @@ -198,7 +202,10 @@ defmodule LightningWeb.Components.DataTables do
attr :id, :string, required: true
attr :clients, :list, required: true
attr :title, :string, required: true
attr :display_table_title, :boolean, default: true
attr :show_owner, :boolean, default: false
attr :page, :map, default: nil
attr :url, :any, default: nil

slot :actions,
doc: "the slot for showing user actions in the last table column"
Expand All @@ -209,13 +216,13 @@ defmodule LightningWeb.Components.DataTables do
def oauth_clients_table(assigns) do
~H"""
<div id={"#{@id}-table-container"}>
<div class="leading-loose pb-2">
<div :if={@display_table_title} class="leading-loose pb-2">
<h6 class="font-normal text-black">{@title}</h6>
</div>
<%= if Enum.empty?(@clients) do %>
{render_slot(@empty_state)}
<% else %>
<.table id={"#{@id}-table"}>
<.table id={"#{@id}-table"} page={@page} url={@url}>
<:header>
<.tr>
<.th>Name</.th>
Expand Down
Loading