From 4d9ea15e9e172afcf0ac918ef4ea41fced5fad3b Mon Sep 17 00:00:00 2001 From: hq1 Date: Thu, 21 Nov 2024 12:12:51 +0100 Subject: [PATCH] =?UTF-8?q?Switch=20reads=20in=20invitation=20creation=20l?= =?UTF-8?q?ogic=20to=20teams=20behind=20FF=20+=20loosely=20related=20fixes?= =?UTF-8?q?=20=F0=9F=8D=8C=20(#4845)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Reduce "choose plan" reliance on the old schema * Fix team members usage computation on teams schema * Switch CreateInvitation to reading from team schemas behind FF (WIP) * Allow test-inviting one guest into multiple sites * Convert another test case where team members count is wrong cc @zoldar * WIP: support site transfer notification e-mails * Even more strict SiteTransfer fetching * Make skipping permissions work * Make CreateInvitation read from team schemas behind FF fully * Fix passing options to `check_invitation_permissions` * Fix allowance check for pageview usage for active or recently ended trial case * Fix `check_invitation_permissions` * Remove no longer relevant invite implementations for Teams --------- Co-authored-by: Adrian Gruntkowski --- .../site/memberships/create_invitation.ex | 53 +++- lib/plausible/teams.ex | 4 +- lib/plausible/teams/adapter/read/billing.ex | 14 + .../teams/adapter/read/invitations.ex | 120 ++++++++ lib/plausible/teams/adapter/read/teams.ex | 27 ++ lib/plausible/teams/billing.ex | 41 ++- lib/plausible/teams/invitations.ex | 123 ++++---- .../components/billing/plan_box.ex | 9 +- .../controllers/site/membership_controller.ex | 4 +- lib/plausible_web/live/choose_plan.ex | 2 +- test/plausible/site/admin_test.exs | 18 +- .../memberships/accept_invitation_test.exs | 58 ---- .../memberships/create_invitation_test.exs | 283 ++++-------------- .../site/membership_controller_test.exs | 3 - test/plausible_web/live/choose_plan_test.exs | 39 +-- test/support/teams/test.ex | 16 +- 16 files changed, 387 insertions(+), 427 deletions(-) create mode 100644 lib/plausible/teams/adapter/read/invitations.ex create mode 100644 lib/plausible/teams/adapter/read/teams.ex diff --git a/lib/plausible/site/memberships/create_invitation.ex b/lib/plausible/site/memberships/create_invitation.ex index 242956b64..7d3f8f3ba 100644 --- a/lib/plausible/site/memberships/create_invitation.ex +++ b/lib/plausible/site/memberships/create_invitation.ex @@ -6,7 +6,6 @@ defmodule Plausible.Site.Memberships.CreateInvitation do alias Plausible.Auth.{User, Invitation} alias Plausible.{Site, Sites, Site.Membership} - alias Plausible.Site.Memberships.Invitations alias Plausible.Billing.Quota import Ecto.Query use Plausible @@ -73,24 +72,49 @@ defmodule Plausible.Site.Memberships.CreateInvitation do attrs = %{email: invitee_email, role: role, site_id: site.id, inviter_id: inviter.id} with site <- Plausible.Repo.preload(site, :owner), - :ok <- check_invitation_permissions(site, inviter, role, opts), - :ok <- check_team_member_limit(site, role, invitee_email), + :ok <- + Plausible.Teams.Adapter.Read.Invitations.check_invitation_permissions( + site, + inviter, + role, + opts + ), + :ok <- + Plausible.Teams.Adapter.Read.Invitations.check_team_member_limit( + inviter, + site, + role, + invitee_email + ), invitee = Plausible.Auth.find_user_by(email: invitee_email), - :ok <- Invitations.ensure_transfer_valid(site, invitee, role), - :ok <- ensure_new_membership(site, invitee, role), + :ok <- + Plausible.Teams.Adapter.Read.Invitations.ensure_transfer_valid( + inviter, + site, + invitee, + role + ), + :ok <- + Plausible.Teams.Adapter.Read.Invitations.ensure_new_membership( + inviter, + site, + invitee, + role + ), %Ecto.Changeset{} = changeset <- Invitation.new(attrs), {:ok, invitation} <- Plausible.Repo.insert(changeset) do - send_invitation_email(invitation, invitee) - Plausible.Teams.Invitations.invite_sync(site, invitation) + Plausible.Teams.Adapter.Read.Invitations.send_invitation_email(inviter, invitation, invitee) + invitation else {:error, cause} -> Plausible.Repo.rollback(cause) end end - defp check_invitation_permissions(site, inviter, requested_role, opts) do + @doc false + def check_invitation_permissions(site, inviter, requested_role, opts) do check_permissions? = Keyword.get(opts, :check_permissions, true) if check_permissions? do @@ -107,7 +131,8 @@ defmodule Plausible.Site.Memberships.CreateInvitation do end end - defp send_invitation_email(invitation, invitee) do + @doc false + def send_invitation_email(invitation, invitee) do invitation = Plausible.Repo.preload(invitation, [:site, :inviter]) email = @@ -140,11 +165,12 @@ defmodule Plausible.Site.Memberships.CreateInvitation do Plausible.Mailer.send(email) end - defp ensure_new_membership(_site, _invitee, :owner) do + @doc false + def ensure_new_membership(_site, _invitee, :owner) do :ok end - defp ensure_new_membership(site, invitee, _role) do + def ensure_new_membership(site, invitee, _role) do if invitee && Sites.is_member?(invitee.id, site) do {:error, :already_a_member} else @@ -152,11 +178,12 @@ defmodule Plausible.Site.Memberships.CreateInvitation do end end - defp check_team_member_limit(_site, :owner, _invitee_email) do + @doc false + def check_team_member_limit(_site, :owner, _invitee_email) do :ok end - defp check_team_member_limit(site, _role, invitee_email) do + def check_team_member_limit(site, _role, invitee_email) do site = Plausible.Repo.preload(site, :owner) limit = Quota.Limits.team_member_limit(site.owner) usage = Quota.Usage.team_member_usage(site.owner, exclude_emails: [invitee_email]) diff --git a/lib/plausible/teams.ex b/lib/plausible/teams.ex index 32894bd88..c575a20ee 100644 --- a/lib/plausible/teams.ex +++ b/lib/plausible/teams.ex @@ -16,7 +16,9 @@ defmodule Plausible.Teams do def on_trial?(team) do team = with_subscription(team) - not Plausible.Billing.Subscriptions.active?(team.subscription) && trial_days_left(team) >= 0 + + not Plausible.Billing.Subscriptions.active?(team.subscription) && + trial_days_left(team) >= 0 end else def on_trial?(_), do: true diff --git a/lib/plausible/teams/adapter/read/billing.ex b/lib/plausible/teams/adapter/read/billing.ex index 1f33a0f70..e93974a3c 100644 --- a/lib/plausible/teams/adapter/read/billing.ex +++ b/lib/plausible/teams/adapter/read/billing.ex @@ -4,6 +4,20 @@ defmodule Plausible.Teams.Adapter.Read.Billing do """ use Plausible.Teams.Adapter + def quota_usage(user, opts \\ []) do + switch(user, + team_fn: &Plausible.Teams.Billing.quota_usage(&1, opts), + user_fn: &Plausible.Billing.Quota.Usage.usage(&1, opts) + ) + end + + def allow_next_upgrade_override?(user) do + switch(user, + team_fn: &(&1 && &1.allow_next_upgrade_override), + user_fn: & &1.allow_next_upgrade_override + ) + end + def change_plan(user, new_plan_id) do switch(user, team_fn: &Plausible.Teams.Billing.change_plan(&1, new_plan_id), diff --git a/lib/plausible/teams/adapter/read/invitations.ex b/lib/plausible/teams/adapter/read/invitations.ex new file mode 100644 index 000000000..369a1e1d0 --- /dev/null +++ b/lib/plausible/teams/adapter/read/invitations.ex @@ -0,0 +1,120 @@ +defmodule Plausible.Teams.Adapter.Read.Invitations do + @moduledoc """ + Transition adapter for new schema reads + """ + use Plausible + use Plausible.Teams.Adapter + + alias Plausible.Repo + + def check_invitation_permissions(site, inviter, role, opts) do + switch( + inviter, + team_fn: fn _ -> + Plausible.Teams.Invitations.check_invitation_permissions( + site, + inviter, + role, + opts + ) + end, + user_fn: fn _ -> + Plausible.Site.Memberships.CreateInvitation.check_invitation_permissions( + site, + inviter, + role, + opts + ) + end + ) + end + + def check_team_member_limit(inviter, site, role, invitee_email) do + switch( + inviter, + team_fn: fn _ -> + site_team = Repo.preload(site, :team).team + + Plausible.Teams.Invitations.check_team_member_limit( + site_team, + role, + invitee_email + ) + end, + user_fn: fn _ -> + Plausible.Site.Memberships.CreateInvitation.check_team_member_limit( + site, + role, + invitee_email + ) + end + ) + end + + def ensure_transfer_valid(inviter, site, invitee, role) do + switch( + inviter, + team_fn: fn _ -> + site_team = Repo.preload(site, :team).team + + Plausible.Teams.Invitations.ensure_transfer_valid( + site_team, + invitee, + role + ) + end, + user_fn: fn _ -> + Plausible.Site.Memberships.Invitations.ensure_transfer_valid( + site, + invitee, + role + ) + end + ) + end + + def ensure_new_membership(inviter, site, invitee, role) do + switch( + inviter, + team_fn: fn _ -> + Plausible.Teams.Invitations.ensure_new_membership( + site, + invitee, + role + ) + end, + user_fn: fn _ -> + Plausible.Site.Memberships.CreateInvitation.ensure_new_membership( + site, + invitee, + role + ) + end + ) + end + + def send_invitation_email(inviter, invitation, invitee) do + switch( + inviter, + team_fn: fn _ -> + if invitation.role == :owner do + Teams.SiteTransfer + |> Repo.get_by!(transfer_id: invitation.invitation_id, initiator_id: inviter.id) + |> Repo.preload([:site, :initiator]) + |> Plausible.Teams.Invitations.send_invitation_email(invitee) + else + Teams.GuestInvitation + |> Repo.get_by!(invitation_id: invitation.invitation_id) + |> Repo.preload([:site, team_invitation: :inviter]) + |> Plausible.Teams.Invitations.send_invitation_email(invitee) + end + end, + user_fn: fn _ -> + Plausible.Site.Memberships.CreateInvitation.send_invitation_email( + invitation, + invitee + ) + end + ) + end +end diff --git a/lib/plausible/teams/adapter/read/teams.ex b/lib/plausible/teams/adapter/read/teams.ex new file mode 100644 index 000000000..bd92db4f0 --- /dev/null +++ b/lib/plausible/teams/adapter/read/teams.ex @@ -0,0 +1,27 @@ +defmodule Plausible.Teams.Adapter.Read.Teams do + @moduledoc """ + Transition adapter for new schema reads + """ + use Plausible.Teams.Adapter + + def trial_expiry_date(user) do + switch(user, + team_fn: &(&1 && &1.trial_expiry_date), + user_fn: & &1.trial_expiry_date + ) + end + + def on_trial?(user) do + switch(user, + team_fn: &Plausible.Teams.on_trial?/1, + user_fn: &Plausible.Users.on_trial?/1 + ) + end + + def trial_days_left(user) do + switch(user, + team_fn: &Plausible.Teams.trial_days_left/1, + user_fn: &Plausible.Users.trial_days_left/1 + ) + end +end diff --git a/lib/plausible/teams/billing.ex b/lib/plausible/teams/billing.ex index f2c773787..8b1182636 100644 --- a/lib/plausible/teams/billing.ex +++ b/lib/plausible/teams/billing.ex @@ -252,7 +252,9 @@ defmodule Plausible.Teams.Billing do end def team_member_usage(team, opts) do - exclude_emails = Keyword.get(opts, :exclude_emails, []) + {:ok, owner} = Teams.Sites.get_owner(team) + exclude_emails = Keyword.get(opts, :exclude_emails, []) ++ [owner.email] + pending_site_ids = Keyword.get(opts, :pending_ownership_site_ids, []) team @@ -320,7 +322,7 @@ defmodule Plausible.Teams.Billing do } end - def features_usage(user, site_ids \\ nil) + def features_usage(team, site_ids \\ nil) def features_usage(%Teams.Team{} = team, nil) do owned_site_ids = team |> Teams.owned_sites() |> Enum.map(& &1.id) @@ -331,8 +333,16 @@ defmodule Plausible.Teams.Billing do site_scoped_feature_usage = features_usage(nil, owned_site_ids) stats_api_used? = - from(a in Plausible.Auth.ApiKey, where: a.team_id == ^team.id) - |> Plausible.Repo.exists?() + Plausible.Repo.exists?( + from tm in Plausible.Teams.Membership, + as: :team_membership, + where: tm.team_id == ^team.id, + where: + exists( + from ak in Plausible.Auth.ApiKey, + where: ak.user_id == parent_as(:team_membership).user_id + ) + ) if stats_api_used? do site_scoped_feature_usage ++ [Feature.StatsAPI] @@ -345,36 +355,47 @@ defmodule Plausible.Teams.Billing do Plausible.Billing.Quota.Usage.features_usage(nil, owned_site_ids) end - defp query_team_member_emails(team, site_ids, exclude_emails) do + defp query_team_member_emails(team, pending_ownership_site_ids, exclude_emails) do + pending_owner_memberships_q = + from s in Plausible.Site, + inner_join: t in assoc(s, :team), + inner_join: tm in assoc(t, :team_memberships), + inner_join: u in assoc(tm, :user), + where: s.id in ^pending_ownership_site_ids, + where: tm.role == :owner, + where: u.email not in ^exclude_emails, + select: %{email: u.email} + pending_memberships_q = from tm in Teams.Membership, inner_join: u in assoc(tm, :user), - inner_join: gm in assoc(tm, :guest_memberships), - where: gm.site_id in ^site_ids and tm.role != :owner, + left_join: gm in assoc(tm, :guest_memberships), + where: gm.site_id in ^pending_ownership_site_ids, where: u.email not in ^exclude_emails, select: %{email: u.email} pending_invitations_q = from ti in Teams.Invitation, inner_join: gi in assoc(ti, :guest_invitations), - where: gi.site_id in ^site_ids and ti.role != :owner, + where: gi.site_id in ^pending_ownership_site_ids, where: ti.email not in ^exclude_emails, select: %{email: ti.email} team_memberships_q = from tm in Teams.Membership, inner_join: u in assoc(tm, :user), - where: tm.team_id == ^team.id and tm.role != :owner, + where: tm.team_id == ^team.id, where: u.email not in ^exclude_emails, select: %{email: u.email} team_invitations_q = from ti in Teams.Invitation, - where: ti.team_id == ^team.id and ti.role != :owner, + where: ti.team_id == ^team.id, where: ti.email not in ^exclude_emails, select: %{email: ti.email} pending_memberships_q + |> union(^pending_owner_memberships_q) |> union(^pending_invitations_q) |> union(^team_memberships_q) |> union(^team_invitations_q) diff --git a/lib/plausible/teams/invitations.ex b/lib/plausible/teams/invitations.ex index 92ce7002b..aad1426be 100644 --- a/lib/plausible/teams/invitations.ex +++ b/lib/plausible/teams/invitations.ex @@ -3,41 +3,10 @@ defmodule Plausible.Teams.Invitations do import Ecto.Query - alias Plausible.Auth alias Plausible.Billing alias Plausible.Repo alias Plausible.Teams - def invite(site, inviter, invitee_email, role, opts \\ []) - - def invite(site, initiator, invitee_email, :owner, opts) do - check_permissions? = opts[:check_permissions] - site = Repo.preload(site, :team) - - with :ok <- check_transfer_permissions(site.team, initiator, check_permissions?), - new_owner = Plausible.Auth.find_user_by(email: invitee_email), - :ok <- ensure_transfer_valid(site.team, new_owner), - {:ok, site_transfer} <- create_site_transfer(site, initiator, invitee_email) do - send_transfer_init_email(site_transfer, new_owner) - {:ok, site_transfer} - end - end - - def invite(site, inviter, invitee_email, role, opts) do - check_permissions? = opts[:check_permissions] - site = Repo.preload(site, :team) - role = translate_role(role) - - with :ok <- check_invitation_permissions(site.team, inviter, check_permissions?), - :ok <- check_team_member_limit(site.team, role, invitee_email), - invitee = Auth.find_user_by(email: invitee_email), - :ok <- ensure_new_membership(site, invitee, role), - {:ok, guest_invitation} <- create_invitation(site, invitee_email, role, inviter) do - send_invitation_email(guest_invitation, invitee) - {:ok, guest_invitation} - end - end - def invite_sync(site, site_invitation) do site = Teams.load_for_site(site) site_invitation = Repo.preload(site_invitation, :inviter) @@ -97,31 +66,6 @@ defmodule Plausible.Teams.Invitations do :ok end - def transfer_site(site, new_owner, now \\ NaiveDateTime.utc_now(:second)) do - site = Repo.preload(site, :team) - - with :ok <- ensure_transfer_valid(site.team, new_owner), - {:ok, team} <- Teams.get_or_create(new_owner), - :ok <- ensure_can_take_ownership(site, team) do - site = - Repo.preload(site, [ - :team, - :owner, - guest_memberships: [team_membership: :user], - guest_invitations: :team_invitation - ]) - - {:ok, _} = - Repo.transaction(fn -> - :ok = transfer_site_ownership(site, team, now) - end) - - {:ok, team_membership} = Teams.Memberships.get(team, new_owner) - - {:ok, team_membership} - end - end - def transfer_site_sync(site, user) do {:ok, team} = Teams.get_or_create(user) site = Teams.load_for_site(site) @@ -219,26 +163,29 @@ defmodule Plausible.Teams.Invitations do end) end - defp check_transfer_permissions(_team, _initiator, false = _check_permissions?) do + def check_transfer_permissions(_team, _initiator, false = _check_permissions?) do :ok end - defp check_transfer_permissions(team, initiator, _) do + def check_transfer_permissions(team, initiator, _) do case Teams.Memberships.team_role(team, initiator) do {:ok, :owner} -> :ok _ -> {:error, :forbidden} end end - defp ensure_transfer_valid(_team, nil), do: :ok + @doc false + def ensure_transfer_valid(_team, nil, :owner), do: :ok - defp ensure_transfer_valid(team, new_owner) do + def ensure_transfer_valid(team, new_owner, :owner) do case Teams.Memberships.team_role(team, new_owner) do {:ok, :owner} -> {:error, :transfer_to_self} _ -> :ok end end + def ensure_transfer_valid(_team, _new_owner, _role), do: :ok + defp create_site_transfer(site, initiator, invitee_email, now \\ NaiveDateTime.utc_now(:second)) do site |> Teams.SiteTransfer.changeset(initiator: initiator, email: invitee_email) @@ -249,7 +196,7 @@ defmodule Plausible.Teams.Invitations do ) end - defp send_transfer_init_email(site_transfer, new_owner) do + def send_transfer_init_email(site_transfer, new_owner) do email = PlausibleWeb.Email.ownership_transfer_request( site_transfer.email, @@ -296,7 +243,7 @@ defmodule Plausible.Teams.Invitations do # - remove old guest memberships site_transfer = Repo.preload(site_transfer, [:initiator, site: :team]) - with :ok <- ensure_transfer_valid(site_transfer.site.team, new_owner), + with :ok <- ensure_transfer_valid(site_transfer.site.team, new_owner, :owner), {:ok, team} <- Teams.get_or_create(new_owner), :ok <- ensure_can_take_ownership(site_transfer.site, team) do site = Repo.preload(site_transfer.site, guest_memberships: [team_membership: :user]) @@ -465,21 +412,34 @@ defmodule Plausible.Teams.Invitations do end end - defp check_invitation_permissions(_team, _inviter, false = _check_permission?) do - :ok - end + @doc false + def check_invitation_permissions(site, inviter, invitation_role, opts) do + check_permissions? = Keyword.get(opts, :check_permissions, true) - defp check_invitation_permissions(team, inviter, _) do - case Teams.Memberships.team_role(team, inviter) do - {:ok, role} when role in [:owner, :admin] -> :ok - _ -> {:error, :forbidden} + if check_permissions? do + case Teams.Memberships.site_role(site, inviter) do + {:ok, :owner} when invitation_role == :owner -> + :ok + + {:ok, inviter_role} + when inviter_role in [:owner, :editor, :admin] and invitation_role != :owner -> + :ok + + _ -> + {:error, :forbidden} + end + else + :ok end end defp translate_role(:admin), do: :editor defp translate_role(role), do: role - defp check_team_member_limit(team, _role, invitee_email) do + @doc false + def check_team_member_limit(_team, :owner, _invitee_email), do: :ok + + def check_team_member_limit(team, _role, invitee_email) do limit = Teams.Billing.team_member_limit(team) usage = Teams.Billing.team_member_usage(team, exclude_emails: [invitee_email]) @@ -490,9 +450,12 @@ defmodule Plausible.Teams.Invitations do end end - defp ensure_new_membership(_site, nil, _role), do: :ok + @doc false + def ensure_new_membership(_site, nil, _role), do: :ok - defp ensure_new_membership(site, invitee, _role) do + def ensure_new_membership(_site, _invitee, :owner), do: :ok + + def ensure_new_membership(site, invitee, _role) do if Teams.Memberships.site_role(site, invitee) == {:error, :not_a_member} do :ok else @@ -535,7 +498,21 @@ defmodule Plausible.Teams.Invitations do ) end - defp send_invitation_email(guest_invitation, invitee) do + @doc false + def send_invitation_email(%Teams.SiteTransfer{} = transfer, invitee) do + email = + PlausibleWeb.Email.ownership_transfer_request( + transfer.email, + transfer.transfer_id, + transfer.site, + transfer.initiator, + invitee + ) + + Plausible.Mailer.send(email) + end + + def send_invitation_email(%Teams.GuestInvitation{} = guest_invitation, invitee) do team_invitation = guest_invitation.team_invitation email = diff --git a/lib/plausible_web/components/billing/plan_box.ex b/lib/plausible_web/components/billing/plan_box.ex index c389f6b1e..d0a668360 100644 --- a/lib/plausible_web/components/billing/plan_box.ex +++ b/lib/plausible_web/components/billing/plan_box.ex @@ -174,7 +174,8 @@ defmodule PlausibleWeb.Components.Billing.PlanBox do paddle_product_id = get_paddle_product_id(assigns.plan_to_render, assigns.selected_interval) change_plan_link_text = change_plan_link_text(assigns) - subscription = assigns.current_user.subscription + subscription = + Plausible.Teams.Adapter.Read.Billing.get_subscription(assigns.current_user) billing_details_expired = Subscription.Status.in?(subscription, [ @@ -270,15 +271,15 @@ defmodule PlausibleWeb.Components.Billing.PlanBox do # because in the past we've let users upgrade without that constraint, as # well as transfer sites to those accounts. to these accounts we won't be # offering an extra pageview limit allowance margin though. - invited_user? = is_nil(current_user.trial_expiry_date) + invited_user? = is_nil(Plausible.Teams.Adapter.Read.Teams.trial_expiry_date(current_user)) trial_active_or_ended_recently? = not invited_user? && - Date.diff(Date.utc_today(), current_user.trial_expiry_date) <= 10 + Plausible.Teams.Adapter.Read.Teams.trial_days_left(current_user) >= -10 limit_checking_opts = cond do - current_user.allow_next_upgrade_override -> + Plausible.Teams.Adapter.Read.Billing.allow_next_upgrade_override?(current_user) -> [ignore_pageview_limit: true] trial_active_or_ended_recently? && plan.volume == "10k" -> diff --git a/lib/plausible_web/controllers/site/membership_controller.ex b/lib/plausible_web/controllers/site/membership_controller.ex index 33aea074b..faed17f77 100644 --- a/lib/plausible_web/controllers/site/membership_controller.ex +++ b/lib/plausible_web/controllers/site/membership_controller.ex @@ -29,8 +29,8 @@ defmodule PlausibleWeb.Site.MembershipController do |> Plausible.Teams.Adapter.Read.Sites.get_for_user!(conn.assigns.site.domain) |> Plausible.Repo.preload(:owner) - limit = Plausible.Billing.Quota.Limits.team_member_limit(site.owner) - usage = Plausible.Billing.Quota.Usage.team_member_usage(site.owner) + limit = Plausible.Teams.Adapter.Read.Billing.team_member_limit(site.owner) + usage = Plausible.Teams.Adapter.Read.Billing.team_member_usage(site.owner) below_limit? = Plausible.Billing.Quota.below_limit?(usage, limit) render( diff --git a/lib/plausible_web/live/choose_plan.ex b/lib/plausible_web/live/choose_plan.ex index 3a8fd9f5f..d5b258a55 100644 --- a/lib/plausible_web/live/choose_plan.ex +++ b/lib/plausible_web/live/choose_plan.ex @@ -25,7 +25,7 @@ defmodule PlausibleWeb.Live.ChoosePlan do current_user: current_user, pending_ownership_site_ids: pending_ownership_site_ids } -> - Quota.Usage.usage(current_user, + Plausible.Teams.Adapter.Read.Billing.quota_usage(current_user, with_features: true, pending_ownership_site_ids: pending_ownership_site_ids ) diff --git a/test/plausible/site/admin_test.exs b/test/plausible/site/admin_test.exs index 45deac9cf..99590dd38 100644 --- a/test/plausible/site/admin_test.exs +++ b/test/plausible/site/admin_test.exs @@ -1,6 +1,7 @@ defmodule Plausible.Site.AdminTest do use Plausible use Plausible.DataCase, async: true + use Plausible.Teams.Test use Bamboo.Test @subject_prefix if ee?(), do: "[Plausible Analytics] ", else: "[Plausible CE] " @@ -38,9 +39,8 @@ defmodule Plausible.Site.AdminTest do end test "new owner can't be the same as old owner", %{conn: conn, transfer_action: action} do - current_owner = insert(:user) - - site = insert(:site, members: [current_owner]) + current_owner = new_user() + site = new_site(owner: current_owner) assert {:error, "User is already an owner of one of the sites"} = action.(conn, [site], %{"email" => current_owner.email}) @@ -50,14 +50,10 @@ defmodule Plausible.Site.AdminTest do conn: conn, transfer_action: action } do - current_owner = insert(:user) - new_owner = insert(:user) - - site1 = - insert(:site, memberships: [build(:site_membership, user: current_owner, role: :owner)]) - - site2 = - insert(:site, memberships: [build(:site_membership, user: current_owner, role: :owner)]) + current_owner = new_user() + new_owner = new_user() + site1 = new_site(owner: current_owner) + site2 = new_site(owner: current_owner) assert :ok = action.(conn, [site1, site2], %{"email" => new_owner.email}) diff --git a/test/plausible/site/memberships/accept_invitation_test.exs b/test/plausible/site/memberships/accept_invitation_test.exs index 43c2f23bb..29a7d7693 100644 --- a/test/plausible/site/memberships/accept_invitation_test.exs +++ b/test/plausible/site/memberships/accept_invitation_test.exs @@ -78,64 +78,6 @@ defmodule Plausible.Site.Memberships.AcceptInvitationTest do assert_guest_membership(team, site, existing_owner, :editor) end - @tag :teams - test "transfers ownership successfully (TEAM)" do - site = insert(:site, memberships: []) - existing_owner = insert(:user) - - _existing_membership = - insert(:site_membership, user: existing_owner, site: site, role: :owner) - - site = Plausible.Teams.load_for_site(site) - old_team = site.team - - another_user = insert(:user) - - another_team_membership = - insert(:team_membership, user: another_user, team: old_team, role: :guest) - - another_guest_membership = - insert(:guest_membership, - team_membership: another_team_membership, - site: site, - role: :viewer - ) - - new_owner = insert(:user) - new_team = insert(:team) - insert(:team_membership, user: new_owner, team: new_team, role: :owner) - insert(:growth_subscription, user: new_owner, team: new_team) - - assert {:ok, new_team_membership} = - Plausible.Teams.Invitations.transfer_site(site, new_owner) - - assert new_team_membership.team_id == new_team.id - assert new_team_membership.user_id == new_owner.id - assert new_team_membership.role == :owner - - assert_team_membership(existing_owner, old_team) - - refute Repo.reload(another_team_membership) - refute Repo.reload(another_guest_membership) - - assert new_another_team_membership = - Plausible.Teams.Membership - |> Repo.get_by( - team_id: new_team.id, - user_id: another_user.id - ) - |> Repo.preload(:guest_memberships) - - assert another_team_membership.id != new_another_team_membership.id - assert [new_another_guest_membership] = new_another_team_membership.guest_memberships - assert new_another_guest_membership.site_id == site.id - assert new_another_guest_membership.role == another_guest_membership.role - - assert new_another_team_membership.role == :guest - - assert_no_emails_delivered() - end - @tag :ee_only test "unlocks the site if it was previously locked" do site = insert(:site, locked: true, memberships: []) diff --git a/test/plausible/site/memberships/create_invitation_test.exs b/test/plausible/site/memberships/create_invitation_test.exs index 795c1bec0..cd0d2fc4e 100644 --- a/test/plausible/site/memberships/create_invitation_test.exs +++ b/test/plausible/site/memberships/create_invitation_test.exs @@ -9,137 +9,38 @@ defmodule Plausible.Site.Memberships.CreateInvitationTest do describe "create_invitation/4" do test "creates an invitation" do - inviter = insert(:user) - invitee = insert(:user) - team = insert(:team) - - site = - insert(:site, - team: team, - memberships: [build(:site_membership, user: inviter, role: :owner)] - ) - - insert(:team_membership, team: team, user: inviter, role: :owner) + inviter = new_user() + invitee = new_user() + site = new_site(owner: inviter) assert {:ok, %Plausible.Auth.Invitation{}} = CreateInvitation.create_invitation(site, inviter, invitee.email, :viewer) - - assert {:ok, %Plausible.Teams.GuestInvitation{}} = - Plausible.Teams.Invitations.invite(site, inviter, invitee.email, :viewer) - end - - @tag :teams - test "[TEAMS] syncs a created invitation" do - inviter = insert(:user) - invitee = insert(:user) - - site = - insert(:site, - team: nil, - memberships: [build(:site_membership, user: inviter, role: :owner)] - ) - - assert {:ok, %Plausible.Auth.Invitation{}} = - CreateInvitation.create_invitation(site, inviter, invitee.email, :viewer) - - team = assert_team_attached(site) - assert_guest_invitation(team, site, invitee.email, :viewer) - end - - @tag :teams - test "[TEAMS] sync a created invitation with team already setup but site not assigned yet" do - inviter = insert(:user) - invitee = insert(:user) - - {:ok, %{id: team_id}} = Plausible.Teams.get_or_create(inviter) - - site = - insert(:site, - team: nil, - memberships: [build(:site_membership, user: inviter, role: :owner)] - ) - - assert {:ok, %Plausible.Auth.Invitation{}} = - CreateInvitation.create_invitation(site, inviter, invitee.email, :viewer) - - team = assert_team_attached(site, team_id) - assert_guest_invitation(team, site, invitee.email, :viewer) - end - - @tag :teams - test "[TEAMS] sync a created invitation with team fully setup" do - inviter = insert(:user) - invitee = insert(:user) - - {:ok, %{id: team_id} = team} = Plausible.Teams.get_or_create(inviter) - - site = - insert(:site, - team: team, - memberships: [build(:site_membership, user: inviter, role: :owner)] - ) - - assert {:ok, %Plausible.Auth.Invitation{}} = - CreateInvitation.create_invitation(site, inviter, invitee.email, :viewer) - - team = assert_team_attached(site, team_id) - assert_guest_invitation(team, site, invitee.email, :viewer) end test "returns validation errors" do - inviter = insert(:user) - team = insert(:team) - - site = - insert(:site, - team: team, - memberships: [build(:site_membership, user: inviter, role: :owner)] - ) - - insert(:team_membership, team: team, user: inviter, role: :owner) + inviter = new_user() + site = new_site(owner: inviter) assert {:error, changeset} = CreateInvitation.create_invitation(site, inviter, "", :viewer) assert {"can't be blank", _} = changeset.errors[:email] - - assert {:error, changeset} = Plausible.Teams.Invitations.invite(site, inviter, "", :viewer) - assert {"can't be blank", _} = changeset.errors[:email] end test "returns error when user is already a member" do - inviter = insert(:user) - invitee = insert(:user) - - team = insert(:team) - - site = - insert(:site, - team: team, - memberships: [ - build(:site_membership, user: inviter, role: :owner), - build(:site_membership, user: invitee, role: :viewer) - ] - ) - - insert(:team_membership, team: team, user: inviter, role: :owner) - team_membership = insert(:team_membership, team: team, user: invitee, role: :guest) - insert(:guest_membership, team_membership: team_membership, site: site, role: :viewer) + inviter = new_user() + invitee = new_user() + site = new_site(owner: inviter) + add_guest(site, user: invitee, role: :viewer) assert {:error, :already_a_member} = CreateInvitation.create_invitation(site, inviter, invitee.email, :viewer) - assert {:error, :already_a_member} = - Plausible.Teams.Invitations.invite(site, inviter, invitee.email, :viewer) - assert {:error, :already_a_member} = CreateInvitation.create_invitation(site, inviter, inviter.email, :viewer) - - assert {:error, :already_a_member} = - Plausible.Teams.Invitations.invite(site, inviter, inviter.email, :viewer) end test "sends invitation email for existing users" do - [inviter, invitee] = insert_list(2, :user) - site = insert(:site, memberships: [build(:site_membership, user: inviter, role: :owner)]) + [inviter, invitee] = for _ <- 1..2, do: new_user() + site = new_site(owner: inviter) assert {:ok, %Plausible.Auth.Invitation{}} = CreateInvitation.create_invitation(site, inviter, invitee.email, :viewer) @@ -151,8 +52,8 @@ defmodule Plausible.Site.Memberships.CreateInvitationTest do end test "sends invitation email for new users" do - inviter = insert(:user) - site = insert(:site, memberships: [build(:site_membership, user: inviter, role: :owner)]) + inviter = new_user() + site = new_site(owner: inviter) assert {:ok, %Plausible.Auth.Invitation{}} = CreateInvitation.create_invitation(site, inviter, "vini@plausible.test", :viewer) @@ -165,15 +66,11 @@ defmodule Plausible.Site.Memberships.CreateInvitationTest do @tag :ee_only test "returns error when owner is over their team member limit" do - [owner, inviter, invitee] = insert_list(3, :user) + [owner, inviter, invitee] = for _ <- 1..3, do: new_user() - memberships = - [ - build(:site_membership, user: owner, role: :owner), - build(:site_membership, user: inviter, role: :admin) - ] ++ build_list(4, :site_membership) - - site = insert(:site, memberships: memberships) + site = new_site(owner: owner) + inviter = add_guest(site, user: inviter, role: :editor) + for _ <- 1..4, do: add_guest(site, role: :viewer) assert {:error, {:over_limit, 3}} = CreateInvitation.create_invitation(site, inviter, invitee.email, :viewer) @@ -181,14 +78,8 @@ defmodule Plausible.Site.Memberships.CreateInvitationTest do @tag :ee_only test "allows inviting users who were already invited to other sites, within the limit" do - owner = insert(:user) - - memberships = - [ - build(:site_membership, user: owner, role: :owner) - ] - - site = insert(:site, memberships: memberships) + owner = new_user() + site = new_site(owner: owner) invite = fn site, email -> CreateInvitation.create_invitation(site, owner, email, :viewer) @@ -199,28 +90,24 @@ defmodule Plausible.Site.Memberships.CreateInvitationTest do assert {:ok, _} = invite.(site, "i3@example.com") assert {:error, {:over_limit, 3}} = invite.(site, "i4@example.com") - site2 = insert(:site, memberships: memberships) + site2 = new_site(owner: owner) assert {:ok, _} = invite.(site2, "i3@example.com") end + import Ecto.Query + @tag :ee_only test "allows inviting users who are already members of other sites, within the limit" do - [u1, u2, u3, u4] = insert_list(4, :user) + [u1, u2, u3, u4] = for _ <- 1..4, do: new_user() + site = new_site(owner: u1) + add_guest(site, user: u2, role: :viewer) + add_guest(site, user: u3, role: :viewer) + add_guest(site, user: u4, role: :viewer) - memberships = - [ - build(:site_membership, user: u1, role: :owner), - build(:site_membership, user: u2, role: :viewer), - build(:site_membership, user: u3, role: :viewer) - ] - - site = - insert(:site, - memberships: memberships ++ [build(:site_membership, user: u4, role: :viewer)] - ) - - site2 = insert(:site, memberships: memberships) + site2 = new_site(owner: u1) + add_guest(site2, user: u2, role: :viewer) + add_guest(site2, user: u3, role: :viewer) invite = fn site, email -> CreateInvitation.create_invitation(site, u1, email, :viewer) @@ -232,8 +119,8 @@ defmodule Plausible.Site.Memberships.CreateInvitationTest do end test "sends ownership transfer email when invitation role is owner" do - inviter = insert(:user) - site = insert(:site, memberships: [build(:site_membership, user: inviter, role: :owner)]) + inviter = new_user() + site = new_site(owner: inviter) assert {:ok, %Plausible.Auth.Invitation{}} = CreateInvitation.create_invitation(site, inviter, "vini@plausible.test", :owner) @@ -245,68 +132,37 @@ defmodule Plausible.Site.Memberships.CreateInvitationTest do end test "only allows owners to transfer ownership" do - inviter = insert(:user) + inviter = new_user() - site = - insert(:site, - memberships: [ - build(:site_membership, user: build(:user), role: :owner), - build(:site_membership, user: inviter, role: :admin) - ] - ) + site = new_site() + add_guest(site, user: inviter, role: :editor) assert {:error, :forbidden} = CreateInvitation.create_invitation(site, inviter, "vini@plausible.test", :owner) end test "allows ownership transfer to existing site members" do - inviter = insert(:user) - invitee = insert(:user) - - site = - insert(:site, - memberships: [ - build(:site_membership, user: inviter, role: :owner), - build(:site_membership, user: invitee, role: :viewer) - ] - ) - |> Plausible.Teams.load_for_site() - - insert(:team_membership, team: site.team, user: invitee, role: :viewer) + inviter = new_user() + invitee = new_user() + site = new_site(owner: inviter) + add_guest(site, user: invitee, role: :viewer) assert {:ok, %Plausible.Auth.Invitation{}} = CreateInvitation.create_invitation(site, inviter, invitee.email, :owner) - - assert {:ok, %Plausible.Teams.SiteTransfer{}} = - Plausible.Teams.Invitations.invite(site, inviter, invitee.email, :owner) end test "does not allow transferring ownership to existing owner" do - inviter = insert(:user, email: "vini@plausible.test") - - site = - insert(:site, - memberships: [ - build(:site_membership, user: inviter, role: :owner) - ] - ) - - site = Plausible.Teams.load_for_site(site) + inviter = new_user(email: "vini@plausible.test") + site = new_site(owner: inviter) assert {:error, :transfer_to_self} = CreateInvitation.create_invitation(site, inviter, "vini@plausible.test", :owner) - - assert {:error, :transfer_to_self} = - Plausible.Teams.Invitations.invite(site, inviter, "vini@plausible.test", :owner) end test "allows creating an ownership transfer even when at team member limit" do - inviter = insert(:user) - - memberships = - [build(:site_membership, user: inviter, role: :owner)] ++ build_list(3, :site_membership) - - site = insert(:site, memberships: memberships) + inviter = new_user() + site = new_site(owner: inviter) + for _ <- 1..3, do: add_guest(site, role: :viewer) assert {:ok, _invitation} = CreateInvitation.create_invitation( @@ -318,37 +174,19 @@ defmodule Plausible.Site.Memberships.CreateInvitationTest do end test "does not allow viewers to invite users" do - inviter = insert(:user) - owner = insert(:user) - - site = - insert(:site, - memberships: [ - build(:site_membership, user: owner, role: :owner), - build(:site_membership, user: inviter, role: :viewer) - ] - ) - |> Plausible.Teams.load_for_site() - - insert(:team_membership, team: site.team, user: inviter, role: :viewer) + inviter = new_user() + owner = new_user() + site = new_site(owner: owner) + add_guest(site, user: inviter, role: :viewer) assert {:error, :forbidden} = CreateInvitation.create_invitation(site, inviter, "vini@plausible.test", :viewer) - - assert {:error, :forbidden} = - Plausible.Teams.Invitations.invite(site, inviter, "vini@plausible.test", :viewer) end test "allows admins to invite other admins" do - inviter = insert(:user) - - site = - insert(:site, - memberships: [ - build(:site_membership, user: build(:user), role: :owner), - build(:site_membership, user: inviter, role: :admin) - ] - ) + inviter = new_user() + site = new_site() + add_guest(site, user: inviter, role: :editor) assert {:ok, %Plausible.Auth.Invitation{}} = CreateInvitation.create_invitation(site, inviter, "vini@plausible.test", :admin) @@ -357,14 +195,11 @@ defmodule Plausible.Site.Memberships.CreateInvitationTest do describe "bulk_create_invitation/5" do test "initiates ownership transfer for multiple sites in one action" do - admin_user = insert(:user) - new_owner = insert(:user) + admin_user = new_user() + new_owner = new_user() - site1 = - insert(:site, memberships: [build(:site_membership, user: admin_user, role: :owner)]) - - site2 = - insert(:site, memberships: [build(:site_membership, user: admin_user, role: :owner)]) + site1 = new_site(owner: admin_user) + site2 = new_site(owner: admin_user) assert {:ok, _} = CreateInvitation.bulk_create_invitation( @@ -397,11 +232,11 @@ defmodule Plausible.Site.Memberships.CreateInvitationTest do end test "initiates ownership transfer for multiple sites in one action skipping permission checks" do - superadmin_user = insert(:user) - new_owner = insert(:user) + superadmin_user = new_user() + new_owner = new_user() - site1 = insert(:site) - site2 = insert(:site) + site1 = new_site() + site2 = new_site() assert {:ok, _} = CreateInvitation.bulk_create_invitation( diff --git a/test/plausible_web/controllers/site/membership_controller_test.exs b/test/plausible_web/controllers/site/membership_controller_test.exs index fa531388e..03e0659a7 100644 --- a/test/plausible_web/controllers/site/membership_controller_test.exs +++ b/test/plausible_web/controllers/site/membership_controller_test.exs @@ -293,9 +293,6 @@ defmodule PlausibleWeb.Site.MembershipControllerTest do put(conn, "/sites/#{site.domain}/memberships/u/#{collaborator.id}/role/viewer") assert_team_membership(collaborator, site.team, :viewer) - - old_model_membership = Repo.get_by(Plausible.Site.Membership, user_id: collaborator.id) - assert old_model_membership.role == :viewer end @tag :teams diff --git a/test/plausible_web/live/choose_plan_test.exs b/test/plausible_web/live/choose_plan_test.exs index 808937578..bd0271966 100644 --- a/test/plausible_web/live/choose_plan_test.exs +++ b/test/plausible_web/live/choose_plan_test.exs @@ -280,8 +280,8 @@ defmodule PlausibleWeb.Live.ChoosePlanTest do conn: conn, user: user } do - insert_list(9, :site, members: [user]) - assert 10 = Plausible.Billing.Quota.Usage.site_usage(user) + for _ <- 1..9, do: new_site(owner: user) + assert 10 = Plausible.Teams.Adapter.Read.Billing.site_usage(user) another_user = new_user() pending_ownership_site = new_site(owner: another_user) @@ -323,6 +323,7 @@ defmodule PlausibleWeb.Live.ChoosePlanTest do user |> Plausible.Auth.User.changeset(%{trial_expiry_date: Timex.shift(Timex.today(), days: -10)}) |> Repo.update!() + |> Plausible.Teams.sync_team() generate_usage_for(site, 13_000) @@ -349,6 +350,7 @@ defmodule PlausibleWeb.Live.ChoosePlanTest do user |> Plausible.Auth.User.changeset(%{trial_expiry_date: Timex.shift(Timex.today(), days: -11)}) |> Repo.update!() + |> Plausible.Teams.sync_team() generate_usage_for(site, 11_000) @@ -613,7 +615,7 @@ defmodule PlausibleWeb.Live.ChoosePlanTest do new_site(owner: user) end - assert 50 = Plausible.Billing.Quota.Usage.site_usage(user) + assert 50 = Plausible.Teams.Adapter.Read.Billing.quota_usage(user).sites another_user = new_user() pending_ownership_site = new_site(owner: another_user) @@ -655,15 +657,8 @@ defmodule PlausibleWeb.Live.ChoosePlanTest do conn: conn, user: user } do - insert(:site, - memberships: [ - build(:site_membership, user: user, role: :owner), - build(:site_membership, user: build(:user)), - build(:site_membership, user: build(:user)), - build(:site_membership, user: build(:user)), - build(:site_membership, user: build(:user)) - ] - ) + site = new_site(owner: user) + for _ <- 1..4, do: add_guest(site, role: :viewer) {:ok, _lv, doc} = get_liveview(conn) @@ -678,7 +673,7 @@ defmodule PlausibleWeb.Live.ChoosePlanTest do conn: conn, user: user } do - for _ <- 1..11, do: insert(:site, members: [user]) + for _ <- 1..11, do: new_site(owner: user) {:ok, _lv, doc} = get_liveview(conn) @@ -693,17 +688,10 @@ defmodule PlausibleWeb.Live.ChoosePlanTest do conn: conn, user: user } do - for _ <- 1..11, do: insert(:site, members: [user]) + for _ <- 1..11, do: new_site(owner: user) - insert(:site, - memberships: [ - build(:site_membership, user: user, role: :owner), - build(:site_membership, user: build(:user)), - build(:site_membership, user: build(:user)), - build(:site_membership, user: build(:user)), - build(:site_membership, user: build(:user)) - ] - ) + site = new_site(owner: user) + for _ <- 1..4, do: add_guest(site, role: :viewer) {:ok, _lv, doc} = get_liveview(conn) @@ -1111,7 +1099,10 @@ defmodule PlausibleWeb.Live.ChoosePlanTest do defp create_subscription_for(user, subscription_opts) do {paddle_plan_id, subscription_opts} = Keyword.pop(subscription_opts, :paddle_plan_id) - user = subscribe_to_plan(user, paddle_plan_id, subscription_opts) + + user = + subscribe_to_plan(user, paddle_plan_id, subscription_opts) + {:ok, user: user} end diff --git a/test/support/teams/test.ex b/test/support/teams/test.ex index bf44e2cab..5e3ff51c6 100644 --- a/test/support/teams/test.ex +++ b/test/support/teams/test.ex @@ -62,8 +62,15 @@ defmodule Plausible.Teams.Test do insert(:site_membership, user: user, role: translate_role_to_old_model(role), site: site) - team_membership = insert(:team_membership, team: team, user: user, role: :guest) - insert(:guest_membership, team_membership: team_membership, site: site, role: role) + team_membership = + build(:team_membership, team: team, user: user, role: :guest) + |> Repo.insert!( + on_conflict: [set: [updated_at: NaiveDateTime.utc_now()]], + conflict_target: [:team_id, :user_id], + returning: true + ) + + insert(:guest_membership, site: site, team_membership: team_membership, role: role) user |> Repo.preload([:site_memberships, :team_memberships]) end @@ -152,7 +159,10 @@ defmodule Plausible.Teams.Test do def subscribe_to_plan(user, paddle_plan_id, attrs \\ []) do {:ok, team} = Teams.get_or_create(user) attrs = Keyword.merge([user: user, team: team, paddle_plan_id: paddle_plan_id], attrs) - subscription = insert(:subscription, attrs) + + subscription = + insert(:subscription, attrs) + %{user | subscription: subscription} end