Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch AcceptInvitation ops to read from team schemas behind FF #4847

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
11 changes: 9 additions & 2 deletions lib/plausible/site/admin.ex
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,16 @@ defmodule Plausible.SiteAdmin do
{:error, "Please select at least one site from the list"}
end

defp transfer_ownership_direct(_conn, sites, %{"email" => email}) do
defp transfer_ownership_direct(conn, sites, %{"email" => email}) do
current_user = conn.assigns.current_user

with {:ok, new_owner} <- Plausible.Auth.get_user_by(email: email),
{:ok, _} <- Plausible.Site.Memberships.bulk_transfer_ownership_direct(sites, new_owner) do
{:ok, _} <-
Plausible.Site.Memberships.bulk_transfer_ownership_direct(
current_user,
sites,
new_owner
) do
:ok
else
{:error, :user_not_found} ->
Expand Down
4 changes: 2 additions & 2 deletions lib/plausible/site/memberships.ex
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ defmodule Plausible.Site.Memberships do
alias Plausible.Repo
alias Plausible.Site.Memberships

defdelegate transfer_ownership(site, user), to: Memberships.AcceptInvitation
defdelegate accept_invitation(invitation_id, user), to: Memberships.AcceptInvitation
defdelegate reject_invitation(invitation_id, user), to: Memberships.RejectInvitation
defdelegate remove_invitation(invitation_id, site), to: Memberships.RemoveInvitation
Expand All @@ -20,7 +19,8 @@ defmodule Plausible.Site.Memberships do
defdelegate bulk_create_invitation(sites, inviter, invitee_email, role, opts),
to: Memberships.CreateInvitation

defdelegate bulk_transfer_ownership_direct(sites, new_owner), to: Memberships.CreateInvitation
defdelegate bulk_transfer_ownership_direct(current_user, sites, new_owner),
to: Memberships.AcceptInvitation

@spec any?(Auth.User.t()) :: boolean()
def any?(user) do
Expand Down
99 changes: 63 additions & 36 deletions lib/plausible/site/memberships/accept_invitation.ex
Original file line number Diff line number Diff line change
Expand Up @@ -23,43 +23,36 @@ defmodule Plausible.Site.Memberships.AcceptInvitation do

require Logger

@spec transfer_ownership(Site.t(), Auth.User.t()) ::
{:ok, Site.Membership.t()}
| {:error,
Billing.Quota.Limits.over_limits_error()
| Ecto.Changeset.t()
| :transfer_to_self
| :no_plan}
def transfer_ownership(site, user) do
site = Repo.preload(site, :owner)

with :ok <- Invitations.ensure_transfer_valid(site, user, :owner),
:ok <- Invitations.ensure_can_take_ownership(site, user) do
membership = get_or_create_owner_membership(site, user)

multi = add_and_transfer_ownership(site, membership, user)

case Repo.transaction(multi) do
{:ok, changes} ->
Plausible.Teams.Invitations.transfer_site_sync(site, user)

membership = Repo.preload(changes.membership, [:site, :user])

{:ok, membership}

{:error, _operation, error, _changes} ->
{:error, error}
@type transfer_error() ::
Billing.Quota.Limits.over_limits_error()
| Ecto.Changeset.t()
| :transfer_to_self
| :no_plan

@type accept_error() ::
:invitation_not_found
| Billing.Quota.Limits.over_limits_error()
| Ecto.Changeset.t()
| :no_plan

@spec bulk_transfer_ownership_direct(Auth.User.t(), [Site.t()], Auth.User.t()) ::
{:ok, [Site.Membership.t()]} | {:error, transfer_error()}
def bulk_transfer_ownership_direct(current_user, sites, new_owner) do
Repo.transaction(fn ->
for site <- sites do
case transfer_ownership(current_user, site, new_owner) do
{:ok, membership} ->
membership

{:error, error} ->
Repo.rollback(error)
end
end
end
end)
end

@spec accept_invitation(String.t(), Auth.User.t()) ::
{:ok, Site.Membership.t()}
| {:error,
:invitation_not_found
| Billing.Quota.Limits.over_limits_error()
| Ecto.Changeset.t()
| :no_plan}
{:ok, Site.Membership.t()} | {:error, accept_error()}
def accept_invitation(invitation_id, user) do
with {:ok, invitation} <- Invitations.find_for_user(invitation_id, user) do
if invitation.role == :owner do
Expand All @@ -70,11 +63,45 @@ defmodule Plausible.Site.Memberships.AcceptInvitation do
end
end

defp transfer_ownership(current_user, site, new_owner) do
with :ok <-
Plausible.Teams.Adapter.Read.Invitations.ensure_transfer_valid(
current_user,
site,
new_owner,
:owner
),
:ok <- Plausible.Teams.Adapter.Read.Ownership.ensure_can_take_ownership(site, new_owner) do
membership = get_or_create_owner_membership(site, new_owner)

multi = add_and_transfer_ownership(site, membership, new_owner)

case Repo.transaction(multi) do
{:ok, changes} ->
Plausible.Teams.Invitations.transfer_site_sync(site, new_owner)

membership = Repo.preload(changes.membership, [:site, :user])

{:ok, membership}

{:error, _operation, error, _changes} ->
{:error, error}
end
end
end

defp do_accept_ownership_transfer(invitation, user) do
membership = get_or_create_membership(invitation, user)
site = Repo.preload(invitation.site, :owner)

with :ok <- Invitations.ensure_can_take_ownership(site, user) do
site = invitation.site

with :ok <-
Plausible.Teams.Adapter.Read.Invitations.ensure_transfer_valid(
user,
site,
user,
:owner
),
:ok <- Plausible.Teams.Adapter.Read.Ownership.ensure_can_take_ownership(site, user) do
site
|> add_and_transfer_ownership(membership, user)
|> Multi.delete(:invitation, invitation)
Expand Down
21 changes: 0 additions & 21 deletions lib/plausible/site/memberships/create_invitation.ex
Original file line number Diff line number Diff line change
Expand Up @@ -37,27 +37,6 @@ defmodule Plausible.Site.Memberships.CreateInvitation do
end)
end

@spec bulk_transfer_ownership_direct([Site.t()], User.t()) ::
{:ok, [Membership.t()]}
| {:error,
invite_error()
| Quota.Limits.over_limits_error()}
def bulk_transfer_ownership_direct(sites, new_owner) do
Plausible.Repo.transaction(fn ->
for site <- sites do
site = Plausible.Repo.preload(site, :owner)

case Site.Memberships.transfer_ownership(site, new_owner) do
{:ok, membership} ->
membership

{:error, error} ->
Plausible.Repo.rollback(error)
end
end
end)
end

@spec bulk_create_invitation([Site.t()], User.t(), String.t(), atom(), Keyword.t()) ::
{:ok, [Invitation.t()]} | {:error, invite_error()}
def bulk_create_invitation(sites, inviter, invitee_email, role, opts \\ []) do
Expand Down
4 changes: 2 additions & 2 deletions lib/plausible/teams/adapter/read/invitations.ex
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ defmodule Plausible.Teams.Adapter.Read.Invitations do
)
end

def ensure_transfer_valid(inviter, site, invitee, role) do
def ensure_transfer_valid(current_user, site, invitee, role) do
switch(
inviter,
current_user,
team_fn: fn _ ->
site_team = Repo.preload(site, :team).team

Expand Down
14 changes: 13 additions & 1 deletion lib/plausible/teams/adapter/read/sites.ex
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,19 @@ defmodule Plausible.Teams.Adapter.Read.Sites do
)
|> Repo.all()

%{memberships: memberships, invitations: invitations}
site_transfers =
from(
st in Teams.SiteTransfer,
where: st.site_id == ^site.id,
select: %Plausible.Auth.Invitation{
invitation_id: st.transfer_id,
email: st.email,
role: :owner
}
)
|> Repo.all()

%{memberships: memberships, invitations: site_transfers ++ invitations}
else
site
|> Repo.preload([:invitations, memberships: :user])
Expand Down
1 change: 1 addition & 0 deletions lib/plausible/teams/sites.ex
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ defmodule Plausible.Teams.Sites do
inner_join: u in assoc(tm, :user),
where: tm.team_id == parent_as(:site).team_id,
where: u.email == parent_as(:site_transfer).email,
where: tm.role == :owner,
select: 1
),
select: %{
Expand Down
35 changes: 13 additions & 22 deletions test/plausible/site/admin_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -75,16 +75,15 @@ defmodule Plausible.Site.AdminTest do
end

test "new owner must be an existing user", %{conn: conn, transfer_direct_action: action} do
site = insert(:site)
site = new_site()

assert action.(conn, [site], %{"email" => "[email protected]"}) ==
{:error, "User could not be found"}
end

test "new owner can't be the same as old owner", %{conn: conn, transfer_direct_action: action} do
current_owner = insert(:user)

site = insert(:site, members: [current_owner])
current_owner = new_user()
site = new_site(owner: current_owner)

assert {:error, "User is already an owner of one of the sites"} =
action.(conn, [site], %{"email" => current_owner.email})
Expand All @@ -96,20 +95,16 @@ defmodule Plausible.Site.AdminTest do
transfer_direct_action: action
} do
today = Date.utc_today()
current_owner = insert(:user)
current_owner = new_user()

new_owner =
insert(:user,
subscription:
build(:growth_subscription,
last_bill_date: Timex.shift(today, days: -5)
)
)
new_user()
|> subscribe_to_growth_plan(last_bill_date: Date.shift(today, day: -5))

# fills the site limit quota
insert_list(10, :site, members: [new_owner])
for _ <- 1..10, do: new_site(owner: new_owner)

site = insert(:site, members: [current_owner])
site = new_site(owner: current_owner)

assert {:error, "Plan limits exceeded" <> _} =
action.(conn, [site], %{"email" => new_owner.email})
Expand All @@ -120,18 +115,14 @@ defmodule Plausible.Site.AdminTest do
transfer_direct_action: action
} do
today = Date.utc_today()
current_owner = insert(:user)
current_owner = new_user()

new_owner =
insert(:user,
subscription: build(:subscription, last_bill_date: Timex.shift(today, days: -5))
)

site1 =
insert(:site, memberships: [build(:site_membership, user: current_owner, role: :owner)])
new_user()
|> subscribe_to_growth_plan(last_bill_date: Date.shift(today, day: -5))

site2 =
insert(:site, memberships: [build(:site_membership, user: current_owner, role: :owner)])
site1 = new_site(owner: current_owner)
site2 = new_site(owner: current_owner)

assert :ok = action.(conn, [site1, site2], %{"email" => new_owner.email})
end
Expand Down
Loading
Loading