diff --git a/elixir/apps/domain/lib/domain/actors.ex b/elixir/apps/domain/lib/domain/actors.ex index bd632bf99..d8cfe5273 100644 --- a/elixir/apps/domain/lib/domain/actors.ex +++ b/elixir/apps/domain/lib/domain/actors.ex @@ -55,6 +55,16 @@ defmodule Domain.Actors do |> Repo.preload(preload) end + def all_editable_groups!(%Auth.Subject{} = subject, opts \\ []) do + {preload, _opts} = Keyword.pop(opts, :preload, []) + + Group.Query.not_deleted() + |> Group.Query.editable() + |> Authorizer.for_subject(subject) + |> Repo.all() + |> Repo.preload(preload) + end + def peek_group_actors(groups, limit, %Auth.Subject{} = subject) do with :ok <- Auth.ensure_has_permissions(subject, Authorizer.manage_actors_permission()) do ids = groups |> Enum.map(& &1.id) |> Enum.uniq() @@ -416,7 +426,7 @@ defmodule Domain.Actors do |> Authorizer.for_subject(subject) |> Repo.fetch_and_update(Actor.Query, with: fn actor -> - actor = maybe_preload_not_synced_memberships(actor, attrs) + actor = maybe_preload_editable_memberships(actor, attrs) synced_groups = list_readonly_groups(attrs) changeset = Actor.Changeset.update(actor, attrs, synced_groups, subject) @@ -424,6 +434,7 @@ defmodule Domain.Actors do changeset.data.type != :account_admin_user -> changeset Map.get(changeset.changes, :type) == :account_admin_user -> changeset other_enabled_admins_exist?(actor) -> changeset + is_nil(Map.get(changeset.changes, :type)) -> changeset true -> :cant_remove_admin_type end end, @@ -432,11 +443,11 @@ defmodule Domain.Actors do end end - defp maybe_preload_not_synced_memberships(%Actor{} = actor, attrs) do + defp maybe_preload_editable_memberships(%Actor{} = actor, attrs) do if Map.has_key?(attrs, "memberships") || Map.has_key?(attrs, :memberships) do memberships = Membership.Query.by_actor_id(actor.id) - |> Membership.Query.by_not_synced_group() + |> Membership.Query.only_editable_groups() |> Membership.Query.lock() |> Repo.all() diff --git a/elixir/apps/domain/lib/domain/actors/membership/query.ex b/elixir/apps/domain/lib/domain/actors/membership/query.ex index 0ce37c067..47a117985 100644 --- a/elixir/apps/domain/lib/domain/actors/membership/query.ex +++ b/elixir/apps/domain/lib/domain/actors/membership/query.ex @@ -6,6 +6,12 @@ defmodule Domain.Actors.Membership.Query do from(memberships in Membership, as: :memberships) end + def only_editable_groups(queryable \\ all()) do + queryable + |> with_joined_groups() + |> where([groups: groups], is_nil(groups.provider_id) and groups.type == :static) + end + def by_actor_id(queryable \\ all(), actor_id) do where(queryable, [memberships: memberships], memberships.actor_id == ^actor_id) end @@ -42,12 +48,6 @@ defmodule Domain.Actors.Membership.Query do |> where([groups: groups], groups.provider_id == ^provider_id) end - def by_not_synced_group(queryable \\ all()) do - queryable - |> with_joined_groups() - |> where([groups: groups], is_nil(groups.provider_id)) - end - def count_actors_by_group_id(queryable \\ all()) do queryable |> group_by([memberships: memberships], memberships.group_id) diff --git a/elixir/apps/web/lib/web/live/actors/edit.ex b/elixir/apps/web/lib/web/live/actors/edit.ex index cd60b7a80..bab5fad44 100644 --- a/elixir/apps/web/lib/web/live/actors/edit.ex +++ b/elixir/apps/web/lib/web/live/actors/edit.ex @@ -13,10 +13,7 @@ defmodule Web.Actors.Edit do ) do # TODO: unify this and dropdowns for filters groups = - Actors.all_groups!(socket.assigns.subject, - preload: [:provider], - filter: [editable?: true] - ) + Actors.all_editable_groups!(socket.assigns.subject, preload: [:provider]) changeset = Actors.change_actor(actor) diff --git a/elixir/apps/web/lib/web/live/actors/service_accounts/new.ex b/elixir/apps/web/lib/web/live/actors/service_accounts/new.ex index 1948a469d..810517f72 100644 --- a/elixir/apps/web/lib/web/live/actors/service_accounts/new.ex +++ b/elixir/apps/web/lib/web/live/actors/service_accounts/new.ex @@ -5,10 +5,7 @@ defmodule Web.Actors.ServiceAccounts.New do def mount(_params, _session, socket) do groups = - Actors.all_groups!(socket.assigns.subject, - preload: :provider, - filter: [editable?: true] - ) + Actors.all_editable_groups!(socket.assigns.subject, preload: :provider) changeset = Actors.new_actor(%{type: :service_account}) diff --git a/elixir/apps/web/lib/web/live/actors/users/new.ex b/elixir/apps/web/lib/web/live/actors/users/new.ex index 87c3a9e35..727f732fb 100644 --- a/elixir/apps/web/lib/web/live/actors/users/new.ex +++ b/elixir/apps/web/lib/web/live/actors/users/new.ex @@ -5,10 +5,7 @@ defmodule Web.Actors.Users.New do def mount(_params, _session, socket) do groups = - Actors.all_groups!(socket.assigns.subject, - preload: :provider, - filter: [editable?: true] - ) + Actors.all_editable_groups!(socket.assigns.subject, preload: :provider) changeset = Actors.new_actor() diff --git a/elixir/apps/web/test/web/live/actors/edit_test.exs b/elixir/apps/web/test/web/live/actors/edit_test.exs index 472fdc6e3..07d5c9e8b 100644 --- a/elixir/apps/web/test/web/live/actors/edit_test.exs +++ b/elixir/apps/web/test/web/live/actors/edit_test.exs @@ -166,7 +166,7 @@ defmodule Web.Live.Actors.EditTest do actor: actor, conn: conn } do - attrs = %{name: String.duplicate("X", 555)} + attrs = %{name: String.duplicate("X", 555), type: :account_user} {:ok, lv, _html} = conn @@ -184,6 +184,23 @@ defmodule Web.Live.Actors.EditTest do actor: actor, conn: conn } do + managed_group = Fixtures.Actors.create_managed_group(account: account) + + {provider, _bypass} = + Fixtures.Auth.start_and_create_google_workspace_provider(account: account) + + synced_group = Fixtures.Actors.create_group(account: account) + + synced_group + |> Ecto.Changeset.change( + created_by: :provider, + provider_id: provider.id, + provider_identifier: Ecto.UUID.generate() + ) + |> Repo.update!() + + Fixtures.Actors.create_membership(actor: actor, group: synced_group) + group1 = Fixtures.Actors.create_group(account: account) Fixtures.Actors.create_membership(actor: actor, group: group1) @@ -205,10 +222,15 @@ defmodule Web.Live.Actors.EditTest do assert_redirected(lv, ~p"/#{account}/actors/#{actor}") + expected_group_ids = [managed_group.id, synced_group.id, group2.id] + assert actor = Repo.get_by(Domain.Actors.Actor, id: actor.id) |> Repo.preload(:memberships) assert actor.name == attrs.name - assert [%{group_id: group_id}] = actor.memberships - assert group_id == group2.id + assert length(actor.memberships) == length(expected_group_ids) + + Enum.each(actor.memberships, fn membership -> + assert membership.group_id in expected_group_ids + end) end end