From 81047ba06de853f2c05c70656b8a2bf58218dce5 Mon Sep 17 00:00:00 2001 From: RobertJoonas <56999674+RobertJoonas@users.noreply.github.com> Date: Thu, 4 Jul 2024 11:57:57 +0300 Subject: [PATCH] Fix recommending a tier on the upgrade page (#4287) * fix recommending a plan * Fix credo issue * Review suggestions * remove dead code --- lib/plausible/billing/plans.ex | 18 +-- lib/plausible/billing/qouta/quota.ex | 35 +++++ .../components/billing/plan_box.ex | 37 +++-- lib/plausible_web/live/choose_plan.ex | 21 ++- test/plausible/billing/plans_test.exs | 33 ---- test/plausible/billing/quota_test.exs | 24 +++ test/plausible_web/live/choose_plan_test.exs | 148 ++++++++++++++++-- 7 files changed, 235 insertions(+), 81 deletions(-) diff --git a/lib/plausible/billing/plans.ex b/lib/plausible/billing/plans.ex index 955bd9947..055c9d55a 100644 --- a/lib/plausible/billing/plans.ex +++ b/lib/plausible/billing/plans.ex @@ -1,8 +1,7 @@ defmodule Plausible.Billing.Plans do alias Plausible.Billing.Subscriptions use Plausible.Repo - alias Plausible.Billing.{Quota, Subscription, Plan, EnterprisePlan} - alias Plausible.Billing.Feature.{StatsAPI, Props} + alias Plausible.Billing.{Subscription, Plan, EnterprisePlan} alias Plausible.Auth.User for f <- [ @@ -235,21 +234,6 @@ defmodule Plausible.Billing.Plans do Enum.find(available_plans, &(usage_during_cycle < &1.monthly_pageview_limit)) end - def suggest_tier(user) do - growth_features = - if Timex.before?(user.inserted_at, @business_tier_launch) do - [StatsAPI, Props] - else - [] - end - - if Enum.any?(Quota.Usage.features_usage(user), &(&1 not in growth_features)) do - :business - else - :growth - end - end - def all() do @legacy_plans ++ @plans_v1 ++ @plans_v2 ++ @plans_v3 ++ @plans_v4 ++ sandbox_plans() end diff --git a/lib/plausible/billing/qouta/quota.ex b/lib/plausible/billing/qouta/quota.ex index cf129fbe8..8d2ae5f4f 100644 --- a/lib/plausible/billing/qouta/quota.ex +++ b/lib/plausible/billing/qouta/quota.ex @@ -58,6 +58,41 @@ defmodule Plausible.Billing.Quota do def eligible_for_upgrade?(usage), do: usage.sites > 0 + def ensure_feature_access(usage, plan) do + case usage.features -- plan.features do + [] -> :ok + features -> {:error, {:unavailable_features, features}} + end + end + + @doc """ + Suggests a suitable tier (Growth or Business) for the given usage map. + + If even the highest Business plan does not accommodate the usage, then + `:custom` is returned. This means that this kind of usage should get on + a custom plan. + + `nil` is returned if the usage is not eligible for upgrade. + """ + def suggest_tier(usage, highest_growth_plan, highest_business_plan) do + if eligible_for_upgrade?(usage) do + cond do + usage_fits_plan?(usage, highest_growth_plan) -> :growth + usage_fits_plan?(usage, highest_business_plan) -> :business + true -> :custom + end + end + end + + defp usage_fits_plan?(usage, plan) do + with :ok <- ensure_within_plan_limits(usage, plan), + :ok <- ensure_feature_access(usage, plan) do + true + else + _ -> false + end + end + defp exceeded_limits(usage, plan, opts) do for {limit, exceeded?} <- [ {:team_member_limit, not within_limit?(usage.team_members, plan.team_member_limit)}, diff --git a/lib/plausible_web/components/billing/plan_box.ex b/lib/plausible_web/components/billing/plan_box.ex index 887b29ae3..9bb7913f6 100644 --- a/lib/plausible_web/components/billing/plan_box.ex +++ b/lib/plausible_web/components/billing/plan_box.ex @@ -10,7 +10,7 @@ defmodule PlausibleWeb.Components.Billing.PlanBox do def standard(assigns) do highlight = cond do - assigns.owned -> "Current" + assigns.owned && assigns.recommended -> "Current" assigns.recommended -> "Recommended" true -> nil end @@ -57,9 +57,28 @@ defmodule PlausibleWeb.Components.Billing.PlanBox do ~H"""
Custom
@@ -193,7 +212,7 @@ defmodule PlausibleWeb.Components.Billing.PlanBox do
[]
end
- features_to_lose = assigns.usage.features -- assigns.plan_to_render.features
+ feature_usage_check = Quota.ensure_feature_access(assigns.usage, assigns.plan_to_render)
assigns =
assigns
@@ -202,7 +221,7 @@ defmodule PlausibleWeb.Components.Billing.PlanBox do
|> assign(:checkout_disabled, checkout_disabled)
|> assign(:disabled_message, disabled_message)
|> assign(:exceeded_plan_limits, exceeded_plan_limits)
- |> assign(:confirm_message, losing_features_message(features_to_lose))
+ |> assign(:confirm_message, losing_features_message(feature_usage_check))
~H"""
<%= if @owned_plan && Plausible.Billing.Subscriptions.resumable?(@user.subscription) do %>
@@ -325,15 +344,15 @@ defmodule PlausibleWeb.Components.Billing.PlanBox do
"""
end
- defp losing_features_message([]), do: nil
+ defp losing_features_message(:ok), do: nil
- defp losing_features_message(features_to_lose) do
+ defp losing_features_message({:error, {:unavailable_features, features}}) do
features_list_str =
- features_to_lose
+ features
|> Enum.map(fn feature_mod -> feature_mod.display_name() end)
|> PlausibleWeb.TextHelpers.pretty_join()
- "This plan does not support #{features_list_str}, which you are currently using. Please note that by subscribing to this plan you will lose access to #{if length(features_to_lose) == 1, do: "this feature", else: "these features"}."
+ "This plan does not support #{features_list_str}, which you are currently using. Please note that by subscribing to this plan you will lose access to #{if length(features) == 1, do: "this feature", else: "these features"}."
end
defp contact_button(assigns) do
diff --git a/lib/plausible_web/live/choose_plan.ex b/lib/plausible_web/live/choose_plan.ex
index 1946b9c39..f4681798d 100644
--- a/lib/plausible_web/live/choose_plan.ex
+++ b/lib/plausible_web/live/choose_plan.ex
@@ -41,23 +41,17 @@ defmodule PlausibleWeb.Live.ChoosePlan do
|> assign_new(:owned_tier, fn %{owned_plan: owned_plan} ->
if owned_plan, do: Map.get(owned_plan, :kind), else: nil
end)
- |> assign_new(:recommended_tier, fn %{
- owned_plan: owned_plan,
- usage: usage,
- user: user
- } ->
- if owned_plan != nil or not Quota.eligible_for_upgrade?(usage) do
- nil
- else
- Plans.suggest_tier(user)
- end
- end)
|> assign_new(:current_interval, fn %{user: user} ->
current_user_subscription_interval(user.subscription)
end)
|> assign_new(:available_plans, fn %{user: user} ->
Plans.available_plans_for(user, with_prices: true, customer_ip: remote_ip)
end)
+ |> assign_new(:recommended_tier, fn %{usage: usage, available_plans: available_plans} ->
+ highest_growth_plan = List.last(available_plans.growth)
+ highest_business_plan = List.last(available_plans.business)
+ Quota.suggest_tier(usage, highest_growth_plan, highest_business_plan)
+ end)
|> assign_new(:available_volumes, fn %{available_plans: available_plans} ->
get_available_volumes(available_plans)
end)
@@ -148,7 +142,10 @@ defmodule PlausibleWeb.Live.ChoosePlan do
available={!!@selected_business_plan}
{assigns}
/>
-
<.render_usage pageview_usage={@usage.monthly_pageviews} /> diff --git a/test/plausible/billing/plans_test.exs b/test/plausible/billing/plans_test.exs index bc8d02428..f3e387d7d 100644 --- a/test/plausible/billing/plans_test.exs +++ b/test/plausible/billing/plans_test.exs @@ -284,39 +284,6 @@ defmodule Plausible.Billing.PlansTest do end end - describe "suggest_tier/1" do - test "suggests Business when user has used a premium feature" do - user = insert(:user) - insert(:api_key, user: user) - - assert Plans.suggest_tier(user) == :business - end - - test "suggests Growth when no premium features used" do - user = insert(:user) - site = insert(:site, members: [user]) - insert(:goal, site: site, event_name: "goals_is_not_premium") - - assert Plans.suggest_tier(user) == :growth - end - - test "suggests Growth tier for a user who used the Stats API, but signed up before it was considered a premium feature" do - user = insert(:user, inserted_at: ~N[2023-10-25 10:00:00]) - insert(:api_key, user: user) - - assert Plans.suggest_tier(user) == :growth - end - - @tag :ee_only - test "suggests Business tier for a user who used the Revenue Goals, even when they signed up before Business tier release" do - user = insert(:user, inserted_at: ~N[2023-10-25 10:00:00]) - site = insert(:site, members: [user]) - insert(:goal, site: site, currency: :USD, event_name: "Purchase") - - assert Plans.suggest_tier(user) == :business - end - end - defp assert_generation(plans_list, generation) do assert List.first(plans_list).generation == generation end diff --git a/test/plausible/billing/quota_test.exs b/test/plausible/billing/quota_test.exs index 3b1c87e52..ceeb861ea 100644 --- a/test/plausible/billing/quota_test.exs +++ b/test/plausible/billing/quota_test.exs @@ -15,6 +15,8 @@ defmodule Plausible.Billing.QuotaTest do @v3_plan_id "749342" @v3_business_plan_id "857481" @v4_1m_plan_id "857101" + @v4_10m_growth_plan_id "857104" + @v4_10m_business_plan_id "857112" describe "site_limit/1" do @describetag :ee_only @@ -930,4 +932,26 @@ defmodule Plausible.Billing.QuotaTest do assert current_cycle == Date.range(~D[2021-01-01], ~D[2021-01-31]) end end + + describe "suggest_tier/2" do + setup do + %{user: insert(:user) |> Plausible.Users.with_subscription()} + end + + test "returns nil if the monthly pageview limit exceeds regular plans", + %{user: user} do + highest_growth_plan = Plausible.Billing.Plans.find(@v4_10m_growth_plan_id) + highest_business_plan = Plausible.Billing.Plans.find(@v4_10m_business_plan_id) + + usage = + Quota.Usage.usage(user) + |> Map.replace!(:monthly_pageviews, %{last_30_days: %{total: 12_000_000}}) + + suggested_tier = + usage + |> Quota.suggest_tier(highest_growth_plan, highest_business_plan) + + assert suggested_tier == nil + end + end end diff --git a/test/plausible_web/live/choose_plan_test.exs b/test/plausible_web/live/choose_plan_test.exs index 3104c7546..300202cc7 100644 --- a/test/plausible_web/live/choose_plan_test.exs +++ b/test/plausible_web/live/choose_plan_test.exs @@ -35,6 +35,7 @@ defmodule PlausibleWeb.Live.ChoosePlanTest do @business_checkout_button "#business-checkout" @enterprise_plan_box "#enterprise-plan-box" + @enterprise_highlight_pill "#enterprise-highlight-pill" @slider_volumes ["10k", "100k", "200k", "500k", "1M", "2M", "5M", "10M", "10M+"] @@ -224,8 +225,8 @@ defmodule PlausibleWeb.Live.ChoosePlanTest do test "recommends Growth tier when no premium features were used", %{conn: conn} do {:ok, _lv, doc} = get_liveview(conn) - assert text_of_element(doc, @growth_plan_box) =~ "Recommended" - refute text_of_element(doc, @business_plan_box) =~ "Recommended" + assert text_of_element(doc, @growth_highlight_pill) == "Recommended" + refute element_exists?(doc, @business_highlight_pill) end test "recommends Business when Revenue Goals used during trial", %{conn: conn, site: site} do @@ -233,8 +234,82 @@ defmodule PlausibleWeb.Live.ChoosePlanTest do {:ok, _lv, doc} = get_liveview(conn) - assert text_of_element(doc, @business_plan_box) =~ "Recommended" - refute text_of_element(doc, @growth_plan_box) =~ "Recommended" + assert text_of_element(doc, @business_highlight_pill) == "Recommended" + refute element_exists?(doc, @growth_highlight_pill) + end + + test "recommends Business when pending ownership site used a premium feature", %{ + conn: conn, + user: user + } do + previous_owner = insert(:user) + site = insert(:site, members: [previous_owner]) + + insert(:goal, site: site, currency: :USD, event_name: "Purchase") + + insert(:invitation, email: user.email, inviter: previous_owner, role: :owner, site: site) + + {:ok, _lv, doc} = get_liveview(conn) + + assert text_of_element(doc, @business_highlight_pill) == "Recommended" + refute element_exists?(doc, @growth_highlight_pill) + end + + test "recommends Business when team member limit for Growth exceeded due to pending ownerships", + %{conn: conn, user: user} do + _owned_site = + insert(:site, + memberships: [ + build(:site_membership, role: :owner, user: user), + build(:site_membership, role: :admin, user: insert(:user)), + build(:site_membership, role: :admin, user: insert(:user)) + ] + ) + + previous_owner = insert(:user) + + pending_ownership_site = + insert(:site, + memberships: [ + build(:site_membership, role: :owner, user: previous_owner), + build(:site_membership, role: :viewer, user: insert(:user)) + ] + ) + + insert(:invitation, + email: user.email, + inviter: previous_owner, + role: :owner, + site: pending_ownership_site + ) + + {:ok, _lv, doc} = get_liveview(conn) + + assert text_of_element(doc, @business_highlight_pill) == "Recommended" + refute element_exists?(doc, @growth_highlight_pill) + end + + test "recommends Business when Growth site limit exceeded due to a pending ownership", %{ + conn: conn, + user: user + } do + insert_list(9, :site, members: [user]) + assert 10 = Plausible.Billing.Quota.Usage.site_usage(user) + + another_user = insert(:user) + pending_ownership_site = insert(:site, members: [another_user]) + + insert(:invitation, + email: user.email, + site: pending_ownership_site, + role: :owner, + inviter: another_user + ) + + {:ok, _lv, doc} = get_liveview(conn) + + assert text_of_element(doc, @business_highlight_pill) == "Recommended" + refute element_exists?(doc, @growth_highlight_pill) end @tag :slow @@ -420,10 +495,11 @@ defmodule PlausibleWeb.Live.ChoosePlanTest do assert doc =~ "billable pageviews in the last billing cycle" end - test "warns about losing access to a feature used by a pending ownership site", %{ - conn: conn, - user: user - } do + test "warns about losing access to a feature used by a pending ownership site and recommends business tier", + %{ + conn: conn, + user: user + } do another_user = insert(:user) pending_site = insert(:site, members: [another_user]) @@ -442,6 +518,9 @@ defmodule PlausibleWeb.Live.ChoosePlanTest do assert text_of_attr(find(doc, @growth_checkout_button), "onclick") =~ "if (confirm(\"This plan does not support Custom Properties, which you are currently using. Please note that by subscribing to this plan you will lose access to this feature.\")) {window.location = " + + assert text_of_element(doc, @business_highlight_pill) == "Recommended" + refute element_exists?(doc, @growth_highlight_pill) end test "gets default selected interval from current subscription plan", %{conn: conn} do @@ -527,7 +606,12 @@ defmodule PlausibleWeb.Live.ChoosePlanTest do assert text_of_element(doc, @slider_value) == "10k" end - test "makes it clear that the user is currently on a business tier", %{conn: conn} do + test "highlights Business box as the 'Current' tier if it's suitable for their usage", %{ + conn: conn, + site: site + } do + insert(:goal, site: site, currency: :USD, event_name: "Purchase") + {:ok, _lv, doc} = get_liveview(conn) class = class_of_element(doc, @business_plan_box) @@ -535,6 +619,47 @@ defmodule PlausibleWeb.Live.ChoosePlanTest do assert class =~ "ring-2" assert class =~ "ring-indigo-600" assert text_of_element(doc, @business_highlight_pill) == "Current" + + refute element_exists?(doc, @growth_highlight_pill) + end + + test "highlights Growth box as the 'Recommended' tier if it would accommodate their usage", %{ + conn: conn + } do + {:ok, _lv, doc} = get_liveview(conn) + + class = class_of_element(doc, @growth_plan_box) + + assert class =~ "ring-2" + assert class =~ "ring-indigo-600" + assert text_of_element(doc, @growth_highlight_pill) == "Recommended" + + refute element_exists?(doc, @business_highlight_pill) + end + + test "recommends Enterprise when site limit exceeds Business tier due to pending ownerships", + %{ + conn: conn, + user: user + } do + insert_list(49, :site, members: [user]) + assert 50 = Plausible.Billing.Quota.Usage.site_usage(user) + + another_user = insert(:user) + pending_ownership_site = insert(:site, members: [another_user]) + + insert(:invitation, + email: user.email, + site: pending_ownership_site, + role: :owner, + inviter: another_user + ) + + {:ok, _lv, doc} = get_liveview(conn) + + assert text_of_element(doc, @enterprise_highlight_pill) == "Recommended" + refute element_exists?(doc, @business_highlight_pill) + refute element_exists?(doc, @growth_highlight_pill) end test "checkout button text and click-disabling CSS classes are dynamic", %{conn: conn} do @@ -970,7 +1095,10 @@ defmodule PlausibleWeb.Live.ChoosePlanTest do describe "for a user with no sites but pending ownership transfer" do setup [:create_user, :log_in] - test "allows to subscribe and does not render a notice", %{conn: conn, user: user} do + test "allows to subscribe and does not render the 'upgrade ineligible' notice", %{ + conn: conn, + user: user + } do old_owner = insert(:user) site = insert(:site, members: [old_owner]) insert(:invitation, site_id: site.id, inviter: old_owner, email: user.email, role: :owner)