Bugfix site transfers (#3531)

* Bugfix: allow ownership transfers when premium features enabled but not used

Fields like `props_enabled` and `funnels_enabled` are true by default,
and these fields do not indicate whether the user/site is actually using
these features or not.

* allow site transfers if they will be at limit after transfer

* small refactor

lib/plausible/billing/quota.ex

@@ -4,6 +4,8 @@ defmodule Plausible.Billing.Quota do
   """
 
   import Ecto.Query
+  alias Plausible.Auth.User
+  alias Plausible.Site
   alias Plausible.Billing
   alias Plausible.Billing.{Plan, Plans, Subscription, EnterprisePlan, Feature}
   alias Plausible.Billing.Feature.{Goals, RevenueGoals, Funnels, Props, StatsAPI}
@@ -24,7 +26,7 @@ defmodule Plausible.Billing.Quota do
   end
 
   @limit_sites_since ~D[2021-05-05]
-  @spec site_limit(Plausible.Auth.User.t()) :: non_neg_integer() | :unlimited
+  @spec site_limit(User.t()) :: non_neg_integer() | :unlimited
   @doc """
   Returns the limit of sites a user can have.
 
@@ -64,7 +66,7 @@ defmodule Plausible.Billing.Quota do
     end
   end
 
-  @spec site_usage(Plausible.Auth.User.t()) :: non_neg_integer()
+  @spec site_usage(User.t()) :: non_neg_integer()
   @doc """
   Returns the number of sites the given user owns.
   """
@@ -102,7 +104,7 @@ defmodule Plausible.Billing.Quota do
     end
   end
 
-  @spec monthly_pageview_usage(Plausible.Auth.User.t()) :: non_neg_integer()
+  @spec monthly_pageview_usage(User.t()) :: non_neg_integer()
   @doc """
   Returns the amount of pageviews and custom events
   sent by the sites the user owns in last 30 days.
@@ -115,7 +117,7 @@ defmodule Plausible.Billing.Quota do
 
   @team_member_limit_for_trials 3
   @team_member_limit_for_legacy_trials :unlimited
-  @spec team_member_limit(Plausible.Auth.User.t()) :: non_neg_integer()
+  @spec team_member_limit(User.t()) :: non_neg_integer()
   @doc """
   Returns the limit of team members a user can have in their sites.
   """
@@ -141,7 +143,7 @@ defmodule Plausible.Billing.Quota do
     end
   end
 
-  @spec team_member_usage(Plausible.Auth.User.t()) :: integer()
+  @spec team_member_usage(User.t()) :: integer()
   @doc """
   Returns the total count of team members and pending invitations associated
   with the user's sites.
@@ -163,7 +165,7 @@ defmodule Plausible.Billing.Quota do
 
     team_members_query =
       from os in subquery(owned_sites_query),
-        inner_join: sm in Plausible.Site.Membership,
+        inner_join: sm in Site.Membership,
         on: sm.site_id == os.site_id,
         inner_join: u in assoc(sm, :user),
         where: sm.role != :owner,
@@ -177,15 +179,16 @@ defmodule Plausible.Billing.Quota do
       union: ^team_members_query
   end
 
-  @spec features_usage(Plausible.Auth.User.t()) :: [atom()]
+  @spec features_usage(User.t() | Site.t()) :: [atom()]
   @doc """
-  Returns a list of features the given user is using. At the
-  current stage, the only features that we need to know the
-  usage for are `Props`, `Funnels`, and `RevenueGoals`
+  Given a user, this function returns the features used across all the sites
+  this user owns + StatsAPI if the user has a configured Stats API key.
+
+  Given a site, returns the features used by the site.
   """
-  def features_usage(user) do
+  def features_usage(%User{} = user) do
     props_usage_query =
-      from s in Plausible.Site,
+      from s in Site,
         inner_join: os in subquery(owned_sites_query(user)),
         on: s.id == os.site_id,
         where: fragment("cardinality(?) > 0", s.allowed_event_props)
@@ -215,6 +218,27 @@ defmodule Plausible.Billing.Quota do
     end)
   end
 
+  def features_usage(%Site{} = site) do
+    props_exist = is_list(site.allowed_event_props) && site.allowed_event_props != []
+
+    funnels_exist =
+      Plausible.Repo.exists?(from f in Plausible.Funnel, where: f.site_id == ^site.id)
+
+    revenue_goals_exist =
+      Plausible.Repo.exists?(
+        from g in Plausible.Goal, where: g.site_id == ^site.id and not is_nil(g.currency)
+      )
+
+    used_features =
+      [
+        {Props, props_exist},
+        {Funnels, funnels_exist},
+        {RevenueGoals, revenue_goals_exist}
+      ]
+
+    for {f_mod, used?} <- used_features, used?, f_mod.enabled?(site), do: f_mod
+  end
+
   def ensure_can_subscribe_to_plan(user, %Plan{} = plan) do
     case exceeded_limits(usage(user), plan) do
       [] ->
@@ -260,7 +284,7 @@ defmodule Plausible.Billing.Quota do
   end
 
   defp owned_sites_query(user) do
-    from sm in Plausible.Site.Membership,
+    from sm in Site.Membership,
       where: sm.role == :owner and sm.user_id == ^user.id,
       select: %{site_id: sm.site_id}
   end

lib/plausible/site/memberships/create_invitation.ex

@@ -115,28 +115,22 @@ defmodule Plausible.Site.Memberships.CreateInvitation do
 
     current_usage = Quota.team_member_usage(new_owner)
     site_usage = Plausible.Repo.aggregate(Quota.team_member_usage_query(site.owner, site), :count)
-    usage_after_transfer = current_usage + site_usage
+    usage_after_transfer = current_usage + site_usage + 1
 
-    Quota.below_limit?(usage_after_transfer, limit)
+    Quota.within_limit?(usage_after_transfer, limit)
   end
 
   defp within_site_limit_after_transfer?(new_owner) do
     limit = Quota.site_limit(new_owner)
     usage_after_transfer = Quota.site_usage(new_owner) + 1
 
-    Quota.below_limit?(usage_after_transfer, limit)
+    Quota.within_limit?(usage_after_transfer, limit)
   end
 
   defp has_access_to_site_features?(site, new_owner) do
-    features_to_check = [
-      Plausible.Billing.Feature.Props,
-      Plausible.Billing.Feature.RevenueGoals,
-      Plausible.Billing.Feature.Funnels
-    ]
-
-    Enum.all?(features_to_check, fn feature ->
-      if feature.enabled?(site), do: feature.check_availability(new_owner) == :ok, else: true
-    end)
+    site
+    |> Plausible.Billing.Quota.features_usage()
+    |> Enum.all?(&(&1.check_availability(new_owner) == :ok))
   end
 
   defp ensure_transfer_valid(%Site{} = site, %User{} = new_owner, :owner) do

test/plausible/billing/quota_test.exs

@@ -425,23 +425,25 @@ defmodule Plausible.Billing.QuotaTest do
   end
 
   describe "features_usage/1" do
-    test "returns an empty list" do
-      user = insert(:user)
-      assert [] == Quota.features_usage(user)
+    test "returns an empty list for a user/site who does not use any feature" do
+      assert [] == Quota.features_usage(insert(:user))
+      assert [] == Quota.features_usage(insert(:site))
     end
 
-    test "returns [Props] when user uses custom props" do
+    test "returns [Props] when user/site uses custom props" do
       user = insert(:user)
 
-      insert(:site,
-        allowed_event_props: ["dummy"],
-        memberships: [build(:site_membership, user: user, role: :owner)]
-      )
+      site =
+        insert(:site,
+          allowed_event_props: ["dummy"],
+          memberships: [build(:site_membership, user: user, role: :owner)]
+        )
 
+      assert [Props] == Quota.features_usage(site)
       assert [Props] == Quota.features_usage(user)
     end
 
-    test "returns [Funnels] when user uses funnels" do
+    test "returns [Funnels] when user/site uses funnels" do
       user = insert(:user)
       site = insert(:site, memberships: [build(:site_membership, user: user, role: :owner)])
 
@@ -449,14 +451,16 @@ defmodule Plausible.Billing.QuotaTest do
       steps = Enum.map(goals, &%{"goal_id" => &1.id})
       Plausible.Funnels.create(site, "dummy", steps)
 
+      assert [Funnels] == Quota.features_usage(site)
       assert [Funnels] == Quota.features_usage(user)
     end
 
-    test "returns [RevenueGoals] when user uses revenue goals" do
+    test "returns [RevenueGoals] when user/site uses revenue goals" do
       user = insert(:user)
       site = insert(:site, memberships: [build(:site_membership, user: user, role: :owner)])
       insert(:goal, currency: :USD, site: site, event_name: "Purchase")
 
+      assert [RevenueGoals] == Quota.features_usage(site)
       assert [RevenueGoals] == Quota.features_usage(user)
     end
 
@@ -482,6 +486,7 @@ defmodule Plausible.Billing.QuotaTest do
       steps = Enum.map(goals, &%{"goal_id" => &1.id})
       Plausible.Funnels.create(site, "dummy", steps)
 
+      assert [Props, Funnels, RevenueGoals] == Quota.features_usage(site)
       assert [Props, Funnels, RevenueGoals] == Quota.features_usage(user)
     end
 

test/plausible/site/memberships/create_invitation_test.exs

@@ -201,7 +201,7 @@ defmodule Plausible.Site.Memberships.CreateInvitationTest do
         insert(:site,
           memberships: [build(:site_membership, user: old_owner, role: :owner)],
           props_enabled: true,
-          funnels_enabled: true
+          allowed_event_props: ["author"]
         )
 
       assert {:error, :upgrade_required} =
@@ -220,6 +220,58 @@ defmodule Plausible.Site.Memberships.CreateInvitationTest do
                  :owner
                )
     end
+
+    test "allows transferring ownership to growth plan when premium feature enabled but not used" do
+      old_owner = insert(:user)
+      site = insert(:site, members: [old_owner], props_enabled: true)
+
+      new_owner = insert(:user, subscription: build(:growth_subscription))
+
+      assert {:ok, _invitation} =
+               CreateInvitation.create_invitation(
+                 site,
+                 old_owner,
+                 new_owner.email,
+                 :owner
+               )
+    end
+
+    test "allows transferring ownership when invitee reaches (but does not exceed) site limit" do
+      old_owner = insert(:user)
+      site = insert(:site, members: [old_owner])
+
+      new_owner = insert(:user, subscription: build(:growth_subscription))
+      for _ <- 1..9, do: insert(:site, members: [new_owner])
+
+      assert {:ok, _invitation} =
+               CreateInvitation.create_invitation(
+                 site,
+                 old_owner,
+                 new_owner.email,
+                 :owner
+               )
+    end
+
+    test "allows transferring ownership when invitee reaches (but does not exceed) team member limit" do
+      old_owner = insert(:user)
+
+      site =
+        insert(:site,
+          memberships:
+            [build(:site_membership, user: old_owner, role: :owner)] ++
+              build_list(2, :site_membership, role: :admin)
+        )
+
+      new_owner = insert(:user, subscription: build(:growth_subscription))
+
+      assert {:ok, _invitation} =
+               CreateInvitation.create_invitation(
+                 site,
+                 old_owner,
+                 new_owner.email,
+                 :owner
+               )
+    end
   end
 
   describe "bulk_create_invitation/5" do