refactor(portal): Move actor groups to own table in actor show page (#4392)

Why:

* When viewing an actor in the portal, all of the groups were listed in
the top info table. This works for a small number of groups, but becomes
difficult to use when an actor is in a large number of groups. This
commit moves that information to it's own `live_table` element so that
it's easier to parse and can be paginated.
This commit is contained in:
Brian Manifold
2024-03-29 16:50:04 -04:00
committed by GitHub
parent 595e058e47
commit f5bb02d36e
5 changed files with 161 additions and 20 deletions

View File

@@ -46,6 +46,15 @@ defmodule Domain.Actors do
end
end
def list_groups_for(%Actor{} = actor, %Auth.Subject{} = subject, opts \\ []) do
with :ok <- Auth.ensure_has_permissions(subject, Authorizer.manage_actors_permission()) do
Group.Query.not_deleted()
|> Group.Query.by_actor_id(actor.id)
|> Authorizer.for_subject(subject)
|> Repo.list(Group.Query, opts)
end
end
def all_groups!(%Auth.Subject{} = subject, opts \\ []) do
{preload, _opts} = Keyword.pop(opts, :preload, [])

View File

@@ -42,6 +42,13 @@ defmodule Domain.Actors.Group.Query do
where(queryable, [groups: groups], groups.account_id == ^account_id)
end
def by_actor_id(queryable, actor_id) do
join(queryable, :left, [groups: groups], memberships in assoc(groups, :memberships),
as: :memberships
)
|> where([memberships: memberships], memberships.actor_id == ^actor_id)
end
def by_provider_id(queryable, provider_id) do
where(queryable, [groups: groups], groups.provider_id == ^provider_id)
end

View File

@@ -189,6 +189,83 @@ defmodule Domain.ActorsTest do
end
end
describe "list_groups_for/2" do
setup do
account = Fixtures.Accounts.create_account()
actor = Fixtures.Actors.create_actor(type: :account_admin_user, account: account)
identity = Fixtures.Auth.create_identity(account: account, actor: actor)
subject = Fixtures.Auth.create_subject(identity: identity)
%{
account: account,
actor: actor,
identity: identity,
subject: subject
}
end
test "returns empty list when there are no groups", %{actor: actor, subject: subject} do
assert {:ok, [], _metadata} = list_groups_for(actor, subject)
end
test "does not list groups from other accounts", %{actor: actor, subject: subject} do
Fixtures.Actors.create_group()
assert {:ok, [], _metadata} = list_groups_for(actor, subject)
end
test "does not list groups from other actors in account", %{
account: account,
actor: actor,
subject: subject
} do
actor2 = Fixtures.Actors.create_actor(account: account)
group = Fixtures.Actors.create_group(account: account)
Fixtures.Actors.create_membership(account: account, actor: actor2, group: group)
assert {:ok, [], _metadata} = list_groups_for(actor, subject)
end
test "does not list deleted groups", %{account: account, actor: actor, subject: subject} do
group = Fixtures.Actors.create_group(account: account)
Fixtures.Actors.create_membership(account: account, actor: actor, group: group)
Fixtures.Actors.delete_group(group)
assert {:ok, [], _metadata} = list_groups_for(actor, subject)
end
test "returns all groups for actor", %{
account: account,
actor: actor,
subject: subject
} do
group1 = Fixtures.Actors.create_group(account: account)
Fixtures.Actors.create_membership(account: account, actor: actor, group: group1)
group2 = Fixtures.Actors.create_group(account: account)
Fixtures.Actors.create_membership(account: account, actor: actor, group: group2)
Fixtures.Actors.create_group(account: account)
Fixtures.Actors.create_group()
assert {:ok, groups, _metadata} = list_groups_for(actor, subject)
assert length(groups) == 2
end
test "returns error when subject has no permission to manage groups", %{
actor: actor,
subject: subject
} do
subject = Fixtures.Auth.remove_permissions(subject)
assert list_groups_for(actor, subject) ==
{:error,
{:unauthorized,
reason: :missing_permissions,
missing_permissions: [Actors.Authorizer.manage_actors_permission()]}}
end
end
describe "peek_group_actors/3" do
setup do
account = Fixtures.Accounts.create_account()

View File

@@ -42,6 +42,13 @@ defmodule Web.Actors.Show do
limit: 10,
callback: &handle_flows_update!/2
)
|> assign_live_table("groups",
query_module: Actors.Group.Query,
sortable_fields: [],
hide_filters: [:provider_id],
limit: 15,
callback: &handle_groups_update!/2
)
{:ok, socket}
else
@@ -67,6 +74,19 @@ defmodule Web.Actors.Show do
end
end
def handle_groups_update!(socket, list_opts) do
list_opts = Keyword.put(list_opts, :preload, [:provider])
with {:ok, groups, metadata} <-
Actors.list_groups_for(socket.assigns.actor, socket.assigns.subject, list_opts) do
{:ok,
assign(socket,
groups: groups,
groups_metadata: metadata
)}
end
end
def handle_tokens_update!(socket, list_opts) do
list_opts =
Keyword.put(list_opts, :preload,
@@ -171,18 +191,6 @@ defmodule Web.Actors.Show do
</:value>
</.vertical_table_row>
<.vertical_table_row>
<:label>Groups</:label>
<:value>
<div class="flex flex-wrap gap-y-2">
<span :if={Enum.empty?(@actor.groups)}>none</span>
<span :for={group <- @actor.groups}>
<.group account={@account} group={group} />
</span>
</div>
</:value>
</.vertical_table_row>
<.vertical_table_row>
<:label>Last Signed In</:label>
<:value><.relative_datetime datetime={@actor.last_seen_at} /></:value>
@@ -450,6 +458,31 @@ defmodule Web.Actors.Show do
</:content>
</.section>
<.section>
<:title>Groups</:title>
<:content>
<.live_table
id="groups"
rows={@groups}
row_id={&"group-#{&1.id}"}
filters={@filters_by_table_id["groups"]}
filter={@filter_form_by_table_id["groups"]}
ordered_by={@order_by_table_id["groups"]}
metadata={@groups_metadata}
>
<:col :let={group} label="NAME">
<.link navigate={~p"/#{@account}/groups/#{group.id}"} class={[link_style()]}>
<%= group.name %>
</.link>
</:col>
<:empty>
<div class="text-center text-neutral-500 p-4">No Groups to display.</div>
</:empty>
</.live_table>
</:content>
</.section>
<.danger_zone :if={is_nil(@actor.deleted_at)}>
<:action :if={not Actors.actor_synced?(@actor) or @identities == []}>
<.delete_button

View File

@@ -222,6 +222,29 @@ defmodule Web.Live.Actors.ShowTest do
"#{flow.gateway.group.name}-#{flow.gateway.name} (#{flow.gateway.last_seen_remote_ip})"
end
test "renders groups table", %{
conn: conn
} do
account = Fixtures.Accounts.create_account()
actor = Fixtures.Actors.create_actor(type: :account_admin_user, account: account)
identity = Fixtures.Auth.create_identity(account: account, actor: actor)
group = Fixtures.Actors.create_group(account: account)
Fixtures.Actors.create_membership(account: account, actor: actor, group: group)
{:ok, lv, _html} =
conn
|> authorize_conn(identity)
|> live(~p"/#{account}/actors/#{actor}")
[row] =
lv
|> element("#groups")
|> render()
|> table_to_map()
assert row["name"] == group.name
end
describe "users" do
setup do
account = Fixtures.Accounts.create_account()
@@ -268,9 +291,6 @@ defmodule Web.Live.Actors.ShowTest do
identity: identity,
conn: conn
} do
group = Fixtures.Actors.create_group(account: account)
Fixtures.Actors.create_membership(account: account, actor: actor, group: group)
{:ok, lv, html} =
conn
|> authorize_conn(identity)
@@ -285,7 +305,6 @@ defmodule Web.Live.Actors.ShowTest do
|> render()
|> vertical_table_to_map()
assert table["groups"] == group.name
assert table["name"] == actor.name
assert table["role"] == "admin"
assert around_now?(table["last signed in"])
@@ -779,9 +798,6 @@ defmodule Web.Live.Actors.ShowTest do
identity: identity,
conn: conn
} do
group = Fixtures.Actors.create_group(account: account)
Fixtures.Actors.create_membership(account: account, actor: actor, group: group)
{:ok, lv, html} =
conn
|> authorize_conn(identity)
@@ -793,7 +809,6 @@ defmodule Web.Live.Actors.ShowTest do
|> element("#actor")
|> render()
|> vertical_table_to_map() == %{
"groups" => group.name,
"last signed in" => "never",
"name" => actor.name,
"role" => "service account"