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

)

* Move `bulk_transfer_ownership_direct` under `AcceptInvitation`

* [WIP] Switch ownership transfer operations to read from team schemas behind FF

* Fix usage test regression

* Semantics - current user; ownership is not necessarily involved

* Perform remaining read via adapter; remove obsolete test

* Properly list site with pending site transfer while being guest on a team

* Account for pending site transfers in Settings > People list

---------

Co-authored-by: Adam Rutkowski <[email protected]>

lib/plausible/site/admin.ex

@@ -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} ->

lib/plausible/site/memberships.ex

@@ -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
@@ -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

lib/plausible/site/memberships/accept_invitation.ex

@@ -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
@@ -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)

lib/plausible/site/memberships/create_invitation.ex

@@ -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

lib/plausible/teams/adapter/read/invitations.ex

@@ -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
 

lib/plausible/teams/adapter/read/sites.ex

@@ -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])

lib/plausible/teams/sites.ex

@@ -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: %{

test/plausible/site/admin_test.exs

@@ -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})
@@ -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})
@@ -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