Skip to content

Commit

Permalink
Bugfix site transfers (#3531)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
RobertJoonas authored Nov 17, 2023
1 parent 0175158 commit 02a1271
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 36 deletions.
50 changes: 37 additions & 13 deletions lib/plausible/billing/quota.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand All @@ -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.
Expand Down Expand Up @@ -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.
"""
Expand Down Expand Up @@ -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.
Expand All @@ -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.
"""
Expand All @@ -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.
Expand All @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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
[] ->
Expand Down Expand Up @@ -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
Expand Down
18 changes: 6 additions & 12 deletions lib/plausible/site/memberships/create_invitation.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 15 additions & 10 deletions test/plausible/billing/quota_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -425,38 +425,42 @@ 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)])

goals = insert_list(3, :goal, site: site, event_name: fn -> Ecto.UUID.generate() end)
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

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

Expand Down
54 changes: 53 additions & 1 deletion test/plausible/site/memberships/create_invitation_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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} =
Expand All @@ -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
Expand Down

0 comments on commit 02a1271

Please sign in to comment.