Extend team consistency checks and improve syncing team and membershi…

…ps (#4735)

* Extend team consistency checks

* Sync team against user right after creating the site

* Prune orphaned team guest memberships and invitations on site removal

extra/lib/plausible_web/controllers/api/external_sites_controller.ex

@@ -97,7 +97,7 @@ defmodule PlausibleWeb.Api.ExternalSitesController do
   def delete_site(conn, %{"site_id" => site_id}) do
     case get_site(conn.assigns.current_user, site_id, [:owner]) do
       {:ok, site} ->
-        {:ok, _} = Plausible.Site.Removal.run(site.domain)
+        {:ok, _} = Plausible.Site.Removal.run(site)
         json(conn, %{"deleted" => true})
 
       {:error, :site_not_found} ->

lib/plausible/auth/auth.ex

@@ -95,16 +95,19 @@ defmodule Plausible.Auth do
 
   def delete_user(user) do
     Repo.transaction(fn ->
-      user =
-        user
-        |> Repo.preload(site_memberships: :site)
+      user = Repo.preload(user, site_memberships: :site)
 
       for membership <- user.site_memberships do
-        Repo.delete!(membership)
-
         if membership.role == :owner do
-          Plausible.Site.Removal.run(membership.site.domain)
+          Plausible.Site.Removal.run(membership.site)
         end
+
+        Repo.delete_all(
+          from(
+            sm in Plausible.Site.Membership,
+            where: sm.id == ^membership.id
+          )
+        )
       end
 
       {:ok, team} = Plausible.Teams.get_or_create(user)

lib/plausible/data_migration/teams_consistency_check.ex

@@ -228,6 +228,104 @@ defmodule Plausible.DataMigration.TeamsConsitencyCheck do
       |> @repo.aggregate(:count, timeout: :infinity)
 
     log("#{out_of_sync_site_transfers_count} out of sync site transfers")
+
+    # Guest memberships out of sync
+
+    respective_site_memberships_query =
+      from(
+        sm in Plausible.Site.Membership,
+        where: sm.site_id == parent_as(:guest_membership).site_id,
+        where: sm.user_id == parent_as(:team_membership).user_id,
+        where:
+          (sm.role == :viewer and parent_as(:guest_membership).role == :viewer) or
+            (sm.role == :admin and parent_as(:guest_membership).role == :editor),
+        select: 1
+      )
+
+    out_of_sync_guest_memberships_count =
+      from(
+        gm in Plausible.Teams.GuestMembership,
+        as: :guest_membership,
+        inner_join: tm in assoc(gm, :team_membership),
+        as: :team_membership,
+        where: tm.role != :owner,
+        where: not exists(respective_site_memberships_query)
+      )
+      |> @repo.aggregate(:count, timeout: :infinity)
+
+    log("#{out_of_sync_guest_memberships_count} out of sync guest memberships")
+
+    # Owner memberships out of sync
+
+    respective_site_memberships_query =
+      from(
+        sm in Plausible.Site.Membership,
+        where: sm.site_id == parent_as(:site).id,
+        where: sm.user_id == parent_as(:team_membership).user_id,
+        where: sm.role == :owner,
+        select: 1
+      )
+
+    out_of_sync_owner_memberships_count =
+      from(
+        tm in Plausible.Teams.Membership,
+        as: :team_membership,
+        inner_join: t in assoc(tm, :team),
+        inner_join: s in assoc(t, :sites),
+        as: :site,
+        where: tm.role == :owner,
+        where: not exists(respective_site_memberships_query)
+      )
+      |> @repo.aggregate(:count, timeout: :infinity)
+
+    log("#{out_of_sync_owner_memberships_count} out of sync owner team memberships")
+
+    # Guest invitations out of sync
+
+    respective_site_invitations_query =
+      from(
+        i in Plausible.Auth.Invitation,
+        where: i.site_id == parent_as(:guest_invitation).site_id,
+        where: i.email == parent_as(:team_invitation).email,
+        where:
+          (i.role == :viewer and parent_as(:guest_invitation).role == :viewer) or
+            (i.role == :admin and parent_as(:guest_invitation).role == :editor),
+        select: 1
+      )
+
+    out_of_sync_guest_invitations_count =
+      from(
+        gi in Plausible.Teams.GuestInvitation,
+        as: :guest_invitation,
+        inner_join: ti in assoc(gi, :team_invitation),
+        as: :team_invitation,
+        where: ti.role != :owner,
+        where: not exists(respective_site_invitations_query)
+      )
+      |> @repo.aggregate(:count, timeout: :infinity)
+
+    log("#{out_of_sync_guest_invitations_count} out of sync guest invitations")
+
+    # Team site transfers out of sync
+
+    respective_site_transfers_query =
+      from(
+        i in Plausible.Auth.Invitation,
+        where: i.site_id == parent_as(:site_transfer).site_id,
+        where: i.email == parent_as(:site_transfer).email,
+        where: i.role == :owner,
+        select: 1
+      )
+
+    out_of_sync_site_transfers_count =
+      from(
+        st in Plausible.Teams.SiteTransfer,
+        as: :site_transfer,
+        where: not exists(respective_site_transfers_query)
+      )
+      |> @repo.aggregate(:count, timeout: :infinity)
+
+    log("#{out_of_sync_site_transfers_count} out of sync team site transfers")
   end
 
   defp log(msg) do

lib/plausible/site/removal.ex

@@ -6,9 +6,17 @@ defmodule Plausible.Site.Removal do
 
   import Ecto.Query
 
-  @spec run(String.t()) :: {:ok, map()}
-  def run(domain) do
-    result = Repo.delete_all(from(s in Plausible.Site, where: s.domain == ^domain))
-    {:ok, %{delete_all: result}}
+  @spec run(Plausible.Site.t()) :: {:ok, map()}
+  def run(site) do
+    Repo.transaction(fn ->
+      site = Plausible.Teams.load_for_site(site)
+
+      result = Repo.delete_all(from(s in Plausible.Site, where: s.domain == ^site.domain))
+
+      Plausible.Teams.Memberships.prune_guests(site.team)
+      Plausible.Teams.Invitations.prune_guest_invitations(site.team)
+
+      %{delete_all: result}
+    end)
   end
 end

lib/plausible/sites.ex

@@ -206,6 +206,14 @@ defmodule Plausible.Sites do
         Site.Membership.new(site, user)
       end)
       |> maybe_start_trial(user)
+      |> Ecto.Multi.run(:sync_team, fn
+        _repo, %{user: user} ->
+          Plausible.Teams.sync_team(user)
+          {:ok, nil}
+
+        _repo, _context ->
+          {:ok, nil}
+      end)
       |> Repo.transaction()
     end
   end

lib/plausible_web/controllers/site_controller.ex

@@ -325,7 +325,7 @@ defmodule PlausibleWeb.SiteController do
   def delete_site(conn, _params) do
     site = conn.assigns[:site]
 
-    Plausible.Site.Removal.run(site.domain)
+    Plausible.Site.Removal.run(site)
 
     conn
     |> put_flash(:success, "Your site and page views deletion process has started.")

test/plausible/goals_test.exs

@@ -167,7 +167,7 @@ defmodule Plausible.GoalsTest do
     insert(:goal, %{site: site, event_name: " Signup "})
     insert(:goal, %{site: site, page_path: " /Signup "})
 
-    Plausible.Site.Removal.run(site.domain)
+    Plausible.Site.Removal.run(site)
 
     assert [] = Goals.for_site(site)
   end

test/plausible/site/site_removal_test.exs

@@ -7,13 +7,34 @@ defmodule Plausible.Site.SiteRemovalTest do
 
   test "site from postgres is immediately deleted" do
     site = insert(:site)
-    assert {:ok, context} = Removal.run(site.domain)
+    assert {:ok, context} = Removal.run(site)
     assert context.delete_all == {1, nil}
     refute Sites.get_by_domain(site.domain)
   end
 
-  test "deletion is idempotent" do
-    assert {:ok, context} = Removal.run("some.example.com")
-    assert context.delete_all == {0, nil}
+  @tag :teams
+  test "site deletion prunes team guest memberships" do
+    site = insert(:site) |> Plausible.Teams.load_for_site() |> Repo.preload(:owner)
+
+    team_membership =
+      insert(:team_membership, user: build(:user), team: site.team, role: :guest)
+
+    insert(:guest_membership, team_membership: team_membership, site: site, role: :viewer)
+
+    team_invitation =
+      insert(:team_invitation,
+        email: "sitedeletion@example.test",
+        team: site.team,
+        inviter: site.owner,
+        role: :guest
+      )
+
+    insert(:guest_invitation, team_invitation: team_invitation, site: site, role: :viewer)
+
+    assert {:ok, context} = Removal.run(site)
+    assert context.delete_all == {1, nil}
+
+    refute Repo.reload(team_membership)
+    refute Repo.reload(team_invitation)
   end
 end