Ensure team is present before use in sync logic (#4709)

* Ensure team is present before use in sync logic

* Ensure teams backfill works against partially assigned sites

* Associate site with team on creation

* Associate site with team on sync

* Reuse alias

* Add tests for invitation creation sync

* Move team assertions to a helper module

* Format

* Test team creation on site creation via Sites context module

* Add tests for teams sync on subscription changes

* Tag tests

* Test grace period start syncing up with teams

* Test grace period manual lock sycning w/ teams

* Test grace period end sycing up w/ teams

* Test clearing grace period sync with teams

* Update moduledoc

* Fix missing preloads and wrong result pattern matching in sync logic

* Test sync on accepting invites and site transfers

* Test sync on membership role update and member removal

* transfer async fix WIP

* Stop privisioning team in site factory

* Remove unused relationship from Site schema

* Ensure consistent parsing of `passthrough` from Paddle webhook

* Update team passthrough notification tests & logic

---------

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

lib/plausible/auth/grace_period.ex

@@ -74,18 +74,18 @@ defmodule Plausible.Auth.GracePeriod do
     Ecto.Changeset.change(user, grace_period: nil)
   end
 
-  @spec active?(User.t()) :: boolean()
+  @spec active?(Plausible.Teams.Team.t() | User.t()) :: boolean()
   @doc """
   Returns whether the grace period is still active for a User. Defaults to
   false if the user is nil or there is no grace period.
   """
-  def active?(user)
+  def active?(user_or_team)
 
-  def active?(%User{grace_period: %__MODULE__{end_date: %Date{} = end_date}}) do
+  def active?(%{grace_period: %__MODULE__{end_date: %Date{} = end_date}}) do
     Timex.diff(end_date, Date.utc_today(), :days) >= 0
   end
 
-  def active?(%User{grace_period: %__MODULE__{manual_lock: true}}) do
+  def active?(%{grace_period: %__MODULE__{manual_lock: true}}) do
     true
   end
 

lib/plausible/billing/billing.ex

@@ -120,25 +120,7 @@ defmodule Plausible.Billing do
   defp handle_subscription_created(params) do
     params =
       if present?(params["passthrough"]) do
-        case String.split(to_string(params["passthrough"]), ";") do
-          [user_id] ->
-            user = Repo.get!(User, user_id)
-            {:ok, team} = Plausible.Teams.get_or_create(user)
-            Map.put(params, "team_id", team.id)
-
-          [user_id, ""] ->
-            user = Repo.get!(User, user_id)
-            {:ok, team} = Plausible.Teams.get_or_create(user)
-
-            params
-            |> Map.put("passthrough", user_id)
-            |> Map.put("team_id", team.id)
-
-          [user_id, team_id] ->
-            params
-            |> Map.put("passthrough", user_id)
-            |> Map.put("team_id", team_id)
-        end
+        format_params(params)
       else
         user = Repo.get_by!(User, email: params["email"])
         {:ok, team} = Plausible.Teams.get_or_create(user)
@@ -148,7 +130,10 @@ defmodule Plausible.Billing do
         |> Map.put("team_id", team.id)
       end
 
-    subscription_params = format_subscription(params) |> add_last_bill_date(params)
+    subscription_params =
+      params
+      |> format_subscription()
+      |> add_last_bill_date(params)
 
     %Subscription{}
     |> Subscription.changeset(subscription_params)
@@ -175,8 +160,13 @@ defmodule Plausible.Billing do
     irrelevant? = params["old_status"] == "paused" && params["status"] == "past_due"
 
     if subscription && not irrelevant? do
+      params =
+        params
+        |> format_params()
+        |> format_subscription()
+
       subscription
-      |> Subscription.changeset(format_subscription(params))
+      |> Subscription.changeset(params)
       |> Repo.update!()
       |> after_subscription_update()
     end
@@ -230,8 +220,26 @@ defmodule Plausible.Billing do
     end
   end
 
+  defp format_params(%{"passthrough" => passthrough} = params) do
+    case String.split(to_string(passthrough), ";") do
+      [user_id] ->
+        user = Repo.get!(User, user_id)
+        {:ok, team} = Plausible.Teams.get_or_create(user)
+        Map.put(params, "team_id", team.id)
+
+      ["user:" <> user_id, "team:" <> team_id] ->
+        params
+        |> Map.put("passthrough", user_id)
+        |> Map.put("team_id", team_id)
+    end
+  end
+
+  defp format_params(params) do
+    params
+  end
+
   defp format_subscription(params) do
-    params = %{
+    subscription_params = %{
       paddle_subscription_id: params["subscription_id"],
       paddle_plan_id: params["subscription_plan_id"],
       cancel_url: params["cancel_url"],
@@ -244,9 +252,9 @@ defmodule Plausible.Billing do
     }
 
     if team_id = params["team_id"] do
-      Map.put(params, :team_id, team_id)
+      Map.put(subscription_params, :team_id, team_id)
     else
-      params
+      subscription_params
     end
   end
 

lib/plausible/data_migration/backfill_teams.ex

@@ -402,25 +402,20 @@ defmodule Plausible.DataMigration.BackfillTeams do
       fn {{owner, site_ids}, idx} ->
         @repo.transaction(
           fn ->
+            {:ok, team} = Teams.get_or_create(owner)
+
             team =
-              "My Team"
-              |> Teams.Team.changeset()
+              team
+              |> Ecto.Changeset.change()
               |> Ecto.Changeset.put_change(:trial_expiry_date, owner.trial_expiry_date)
               |> Ecto.Changeset.put_change(:accept_traffic_until, owner.accept_traffic_until)
               |> Ecto.Changeset.put_change(
                 :allow_next_upgrade_override,
                 owner.allow_next_upgrade_override
               )
               |> Ecto.Changeset.put_embed(:grace_period, owner.grace_period)
-              |> Ecto.Changeset.put_change(:inserted_at, owner.inserted_at)
-              |> Ecto.Changeset.put_change(:updated_at, owner.updated_at)
-              |> @repo.insert!()
-
-            team
-            |> Teams.Membership.changeset(owner, :owner)
-            |> Ecto.Changeset.put_change(:inserted_at, owner.inserted_at)
-            |> Ecto.Changeset.put_change(:updated_at, owner.updated_at)
-            |> @repo.insert!()
+              |> Ecto.Changeset.force_change(:updated_at, owner.updated_at)
+              |> @repo.update!()
 
             @repo.update_all(from(s in Plausible.Site, where: s.id in ^site_ids),
               set: [team_id: team.id]

lib/plausible/site/memberships/accept_invitation.ex

@@ -42,12 +42,7 @@ defmodule Plausible.Site.Memberships.AcceptInvitation do
       case Repo.transaction(multi) do
         {:ok, changes} ->
           with_teams do
-            sync_job =
-              Task.async(fn ->
-                Plausible.Teams.Invitations.transfer_site_sync(site, user)
-              end)
-
-            Task.await(sync_job)
+            Plausible.Teams.Invitations.transfer_site_sync(site, user)
           end
 
           membership = Repo.preload(changes.membership, [:site, :user])

lib/plausible/sites.ex

@@ -179,6 +179,9 @@ defmodule Plausible.Sites do
     with :ok <- Quota.ensure_can_add_new_site(user) do
       Ecto.Multi.new()
       |> Ecto.Multi.put(:site_changeset, Site.new(params))
+      |> Ecto.Multi.run(:create_team, fn _repo, _context ->
+        Plausible.Teams.get_or_create(user)
+      end)
       |> Ecto.Multi.run(:clear_changed_from, fn
         _repo, %{site_changeset: %{changes: %{domain: domain}}} ->
           case get_for_user(user.id, domain, [:owner]) do
@@ -196,7 +199,9 @@ defmodule Plausible.Sites do
         _repo, _context ->
           {:ok, :ignore}
       end)
-      |> Ecto.Multi.insert(:site, fn %{site_changeset: site} -> site end)
+      |> Ecto.Multi.insert(:site, fn %{site_changeset: site, create_team: team} ->
+        Ecto.Changeset.put_assoc(site, :team, team)
+      end)
       |> Ecto.Multi.insert(:site_membership, fn %{site: site} ->
         Site.Membership.new(site, user)
       end)

lib/plausible/teams.ex

@@ -16,6 +16,27 @@ defmodule Plausible.Teams do
     Repo.preload(team, :sites).sites
   end
 
+  @doc """
+  Create (when necessary) and load team relation for provided site.
+
+  Used for sync logic to work smoothly during transitional period.
+  """
+  def load_for_site(site) do
+    site = Repo.preload(site, [:team, :owner])
+
+    if site.team do
+      site
+    else
+      {:ok, team} = get_or_create(site.owner)
+
+      site
+      |> Ecto.Changeset.change()
+      |> Ecto.Changeset.put_assoc(:team, team)
+      |> Ecto.Changeset.force_change(:updated_at, site.updated_at)
+      |> Repo.update!()
+    end
+  end
+
   @doc """
   Get or create user's team.
 
@@ -31,24 +52,21 @@ defmodule Plausible.Teams do
   def get_or_create(user) do
     with {:error, :no_team} <- get_owned_by_user(user) do
       case create_my_team(user) do
-        {:ok, team} -> {:ok, team}
-        {:error, :exists_already} -> get_owned_by_user(user)
+        {:ok, team} ->
+          {:ok, team}
+
+        {:error, :exists_already} ->
+          get_owned_by_user(user)
       end
     end
   end
 
   def sync_team(user) do
-    case get_owned_by_user(user) do
-      {:ok, team} ->
-        team
-        |> Teams.Team.sync_changeset(user)
-        |> Repo.update!()
-
-      _ ->
-        :skip
-    end
+    {:ok, team} = get_or_create(user)
 
-    :ok
+    team
+    |> Teams.Team.sync_changeset(user)
+    |> Repo.update!()
   end
 
   defp create_my_team(user) do
@@ -88,8 +106,11 @@ defmodule Plausible.Teams do
       |> Repo.one()
 
     case result do
-      nil -> {:error, :no_team}
-      team -> {:ok, team}
+      nil ->
+        {:error, :no_team}
+
+      team ->
+        {:ok, team}
     end
   end