From bfe4bac233563598b225812cbdedfffdbdb1fb6d Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Wed, 20 Nov 2024 11:59:21 +0100 Subject: [PATCH 01/13] Reduce "choose plan" reliance on the old schema --- lib/plausible/teams.ex | 5 ++- lib/plausible/teams/adapter/read/billing.ex | 14 +++++++ lib/plausible/teams/adapter/read/teams.ex | 27 +++++++++++++ lib/plausible/teams/billing.ex | 28 ++++++++++--- .../components/billing/plan_box.ex | 9 +++-- lib/plausible_web/live/choose_plan.ex | 2 +- test/plausible_web/live/choose_plan_test.exs | 40 +++++++------------ test/support/teams/test.ex | 22 +++++----- 8 files changed, 100 insertions(+), 47 deletions(-) create mode 100644 lib/plausible/teams/adapter/read/teams.ex diff --git a/lib/plausible/teams.ex b/lib/plausible/teams.ex index 32894bd88c92..b1ada49069d8 100644 --- a/lib/plausible/teams.ex +++ b/lib/plausible/teams.ex @@ -16,7 +16,10 @@ 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 1f33a0f70908..89d43b9e049a 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/teams.ex b/lib/plausible/teams/adapter/read/teams.ex new file mode 100644 index 000000000000..bd92db4f0cd9 --- /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 f2c773787c9e..8f1af4abcc4a 100644 --- a/lib/plausible/teams/billing.ex +++ b/lib/plausible/teams/billing.ex @@ -253,6 +253,7 @@ defmodule Plausible.Teams.Billing do def team_member_usage(team, opts) do exclude_emails = Keyword.get(opts, :exclude_emails, []) + pending_site_ids = Keyword.get(opts, :pending_ownership_site_ids, []) team @@ -320,7 +321,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 +332,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,19 +354,19 @@ 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_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, + where: gm.site_id in ^pending_ownership_site_ids and tm.role != :owner, 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 and ti.role != :owner, where: ti.email not in ^exclude_emails, select: %{email: ti.email} @@ -374,7 +383,14 @@ defmodule Plausible.Teams.Billing do where: ti.email not in ^exclude_emails, select: %{email: ti.email} + site_transfers_q = + from st in Teams.SiteTransfer, + where: st.site_id in ^pending_ownership_site_ids, + where: st.email not in ^exclude_emails, + select: %{email: st.email} + pending_memberships_q + |> union(^site_transfers_q) |> union(^pending_invitations_q) |> union(^team_memberships_q) |> union(^team_invitations_q) diff --git a/lib/plausible_web/components/billing/plan_box.ex b/lib/plausible_web/components/billing/plan_box.ex index c389f6b1eda3..10d1094f2ce0 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/live/choose_plan.ex b/lib/plausible_web/live/choose_plan.ex index 3a8fd9f5faa6..d5b258a555e6 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_web/live/choose_plan_test.exs b/test/plausible_web/live/choose_plan_test.exs index 808937578e1d..ba1be2894bb2 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) @@ -613,7 +614,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 +656,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 +672,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 +687,10 @@ defmodule PlausibleWeb.Live.ChoosePlanTest do conn: conn, user: user } do - for _ <- 1..11, do: insert(:site, members: [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)) - ] - ) + for _ <- 1..11, do: new_site(owner: user) + + site = new_site(owner: user) + for _ <- 1..4, do: add_guest(site, role: :viewer) {:ok, _lv, doc} = get_liveview(conn) @@ -1111,7 +1098,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 bf44e2cabf9f..9c26c316bfe3 100644 --- a/test/support/teams/test.ex +++ b/test/support/teams/test.ex @@ -60,7 +60,7 @@ defmodule Plausible.Teams.Test do role = Keyword.fetch!(args, :role) team = Repo.preload(site, :team).team - insert(:site_membership, user: user, role: translate_role_to_old_model(role), site: site) + # 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) @@ -79,15 +79,14 @@ defmodule Plausible.Teams.Test do email when is_binary(email) -> email end + # insert(:invitation, + # email: email, + # inviter: inviter, + # role: translate_role_to_old_model(role), + # site: site + # ) old_model_invitation = - insert(:invitation, - email: email, - inviter: inviter, - role: translate_role_to_old_model(role), - site: site - ) - - team_invitation = + team_invitation = insert(:team_invitation, team: team, email: email, @@ -152,7 +151,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 From 8f0449147cd24fdc4b88dd149393f336b88e42ac Mon Sep 17 00:00:00 2001 From: Adrian Gruntkowski Date: Wed, 20 Nov 2024 14:10:37 +0100 Subject: [PATCH 02/13] Fix team members usage computation on teams schema --- lib/plausible/teams/adapter/read/billing.ex | 2 +- lib/plausible/teams/billing.ex | 20 +++++++------------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/lib/plausible/teams/adapter/read/billing.ex b/lib/plausible/teams/adapter/read/billing.ex index 89d43b9e049a..e93974a3c88d 100644 --- a/lib/plausible/teams/adapter/read/billing.ex +++ b/lib/plausible/teams/adapter/read/billing.ex @@ -13,7 +13,7 @@ defmodule Plausible.Teams.Adapter.Read.Billing do def allow_next_upgrade_override?(user) do switch(user, - team_fn: & &1 && &1.allow_next_upgrade_override, + team_fn: &(&1 && &1.allow_next_upgrade_override), user_fn: & &1.allow_next_upgrade_override ) end diff --git a/lib/plausible/teams/billing.ex b/lib/plausible/teams/billing.ex index 8f1af4abcc4a..0262b5cb3ea2 100644 --- a/lib/plausible/teams/billing.ex +++ b/lib/plausible/teams/billing.ex @@ -252,7 +252,8 @@ 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, []) @@ -358,39 +359,32 @@ defmodule Plausible.Teams.Billing do 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 ^pending_ownership_site_ids and tm.role != :owner, + left_join: gm in assoc(tm, :guest_memberships), + where: gm.site_id in ^pending_ownership_site_ids or tm.role == :owner, 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 ^pending_ownership_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} - site_transfers_q = - from st in Teams.SiteTransfer, - where: st.site_id in ^pending_ownership_site_ids, - where: st.email not in ^exclude_emails, - select: %{email: st.email} - pending_memberships_q - |> union(^site_transfers_q) |> union(^pending_invitations_q) |> union(^team_memberships_q) |> union(^team_invitations_q) From b1d86e3dd78cf9e283e35bd6d47491667dc7fe4e Mon Sep 17 00:00:00 2001 From: Adrian Gruntkowski Date: Wed, 20 Nov 2024 15:33:58 +0100 Subject: [PATCH 03/13] Switch CreateInvitation to reading from team schemas behind FF (WIP) --- .../site/memberships/create_invitation.ex | 53 ++++-- .../teams/adapter/read/invitations.ex | 112 ++++++++++++ lib/plausible/teams/invitations.ex | 35 ++-- .../memberships/create_invitation_test.exs | 173 +++--------------- test/support/teams/test.ex | 2 + 5 files changed, 205 insertions(+), 170 deletions(-) create mode 100644 lib/plausible/teams/adapter/read/invitations.ex diff --git a/lib/plausible/site/memberships/create_invitation.ex b/lib/plausible/site/memberships/create_invitation.ex index 242956b64f8c..7d3f8f3ba7f2 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/adapter/read/invitations.ex b/lib/plausible/teams/adapter/read/invitations.ex new file mode 100644 index 000000000000..b6e0f62bc94b --- /dev/null +++ b/lib/plausible/teams/adapter/read/invitations.ex @@ -0,0 +1,112 @@ +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, + 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 _ -> + Teams.GuestInvitation + |> Repo.get_by!(invitation_id: invitation.invitation_id) + |> Repo.preload([:site, team_invitation: :inviter]) + |> Plausible.Teams.Invitations.send_invitation_email(invitee) + end, + user_fn: fn _ -> + Plausible.Site.Memberships.CreateInvitation.send_invitation_email( + invitation, + invitee + ) + end + ) + end +end diff --git a/lib/plausible/teams/invitations.ex b/lib/plausible/teams/invitations.ex index 92ce7002b00a..195914b07d80 100644 --- a/lib/plausible/teams/invitations.ex +++ b/lib/plausible/teams/invitations.ex @@ -16,7 +16,7 @@ defmodule Plausible.Teams.Invitations do 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 <- ensure_transfer_valid(site.team, new_owner, :owner), {:ok, site_transfer} <- create_site_transfer(site, initiator, invitee_email) do send_transfer_init_email(site_transfer, new_owner) {:ok, site_transfer} @@ -100,7 +100,7 @@ defmodule Plausible.Teams.Invitations do 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), + with :ok <- ensure_transfer_valid(site.team, new_owner, :owner), {:ok, team} <- Teams.get_or_create(new_owner), :ok <- ensure_can_take_ownership(site, team) do site = @@ -230,15 +230,18 @@ defmodule Plausible.Teams.Invitations do 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) @@ -296,7 +299,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,13 +468,14 @@ defmodule Plausible.Teams.Invitations do end end - defp check_invitation_permissions(_team, _inviter, false = _check_permission?) do + @doc false + def check_invitation_permissions(_site, _inviter, false = _check_permission?) do :ok end - defp check_invitation_permissions(team, inviter, _) do - case Teams.Memberships.team_role(team, inviter) do - {:ok, role} when role in [:owner, :admin] -> :ok + def check_invitation_permissions(site, inviter, _) do + case Teams.Memberships.site_role(site, inviter) do + {:ok, role} when role in [:owner, :editor, :admin] -> :ok _ -> {:error, :forbidden} end end @@ -479,7 +483,8 @@ defmodule Plausible.Teams.Invitations do 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, _role, invitee_email) do limit = Teams.Billing.team_member_limit(team) usage = Teams.Billing.team_member_usage(team, exclude_emails: [invitee_email]) @@ -490,9 +495,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 + + def ensure_new_membership(_site, _invitee, :owner), do: :ok - defp ensure_new_membership(site, invitee, _role) do + def ensure_new_membership(site, invitee, _role) do if Teams.Memberships.site_role(site, invitee) == {:error, :not_a_member} do :ok else @@ -535,7 +543,8 @@ defmodule Plausible.Teams.Invitations do ) end - defp send_invitation_email(guest_invitation, invitee) do + @doc false + def send_invitation_email(guest_invitation, invitee) do team_invitation = guest_invitation.team_invitation email = diff --git a/test/plausible/site/memberships/create_invitation_test.exs b/test/plausible/site/memberships/create_invitation_test.exs index 795c1bec070b..64b5d44707bf 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) - - 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)] - ) + inviter = new_user() + invitee = new_user() + site = new_site(owner: inviter) 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,22 @@ 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 @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) - - 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) + [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) + + 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) diff --git a/test/support/teams/test.ex b/test/support/teams/test.ex index 9c26c316bfe3..b4868476c26b 100644 --- a/test/support/teams/test.ex +++ b/test/support/teams/test.ex @@ -62,6 +62,8 @@ defmodule Plausible.Teams.Test do # insert(:site_membership, user: user, role: translate_role_to_old_model(role), site: site) + # FIXME: turn this into an upsert so that we can add the same guest + # to more than 1 site within a single team team_membership = insert(:team_membership, team: team, user: user, role: :guest) insert(:guest_membership, team_membership: team_membership, site: site, role: role) From 617bf6d9db8fbbed2cd71e23da739e7312b966bd Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Thu, 21 Nov 2024 06:11:24 +0100 Subject: [PATCH 04/13] Allow test-inviting one guest into multiple sites --- test/support/teams/test.ex | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/support/teams/test.ex b/test/support/teams/test.ex index b4868476c26b..e6dfa747bd69 100644 --- a/test/support/teams/test.ex +++ b/test/support/teams/test.ex @@ -62,10 +62,8 @@ defmodule Plausible.Teams.Test do # insert(:site_membership, user: user, role: translate_role_to_old_model(role), site: site) - # FIXME: turn this into an upsert so that we can add the same guest - # to more than 1 site within a single team - 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!(team_membership, on_conflict: :nothing, conflict_target: [:team_id, :user_id]) user |> Repo.preload([:site_memberships, :team_memberships]) end From 33cce98f2330b1b5cd8c3407f9c601b04deca28d Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Thu, 21 Nov 2024 06:12:08 +0100 Subject: [PATCH 05/13] Convert another test case where team members count is wrong cc @zoldar --- .../site/memberships/create_invitation_test.exs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/test/plausible/site/memberships/create_invitation_test.exs b/test/plausible/site/memberships/create_invitation_test.exs index 64b5d44707bf..1b96271b3114 100644 --- a/test/plausible/site/memberships/create_invitation_test.exs +++ b/test/plausible/site/memberships/create_invitation_test.exs @@ -186,12 +186,9 @@ defmodule Plausible.Site.Memberships.CreateInvitationTest do 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( From 79c2bd7a157f22512753aa56ef8640e730c36d3c Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Thu, 21 Nov 2024 06:51:02 +0100 Subject: [PATCH 06/13] WIP: support site transfer notification e-mails --- .../site/memberships/create_invitation.ex | 1 + .../teams/adapter/read/invitations.ex | 15 ++++++--- lib/plausible/teams/invitations.ex | 12 ++++++- .../memberships/create_invitation_test.exs | 32 ++++++------------- 4 files changed, 32 insertions(+), 28 deletions(-) diff --git a/lib/plausible/site/memberships/create_invitation.ex b/lib/plausible/site/memberships/create_invitation.ex index 7d3f8f3ba7f2..8c8bf0a386c7 100644 --- a/lib/plausible/site/memberships/create_invitation.ex +++ b/lib/plausible/site/memberships/create_invitation.ex @@ -104,6 +104,7 @@ defmodule Plausible.Site.Memberships.CreateInvitation do %Ecto.Changeset{} = changeset <- Invitation.new(attrs), {:ok, invitation} <- Plausible.Repo.insert(changeset) do Plausible.Teams.Invitations.invite_sync(site, invitation) + invitation Plausible.Teams.Adapter.Read.Invitations.send_invitation_email(inviter, invitation, invitee) diff --git a/lib/plausible/teams/adapter/read/invitations.ex b/lib/plausible/teams/adapter/read/invitations.ex index b6e0f62bc94b..f9a51bd57bbd 100644 --- a/lib/plausible/teams/adapter/read/invitations.ex +++ b/lib/plausible/teams/adapter/read/invitations.ex @@ -96,10 +96,17 @@ defmodule Plausible.Teams.Adapter.Read.Invitations do switch( inviter, team_fn: fn _ -> - Teams.GuestInvitation - |> Repo.get_by!(invitation_id: invitation.invitation_id) - |> Repo.preload([:site, team_invitation: :inviter]) - |> Plausible.Teams.Invitations.send_invitation_email(invitee) + if invitation.role == :owner do + Teams.SiteTransfer + |> Repo.get_by!(transfer_id: invitation.invitation_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( diff --git a/lib/plausible/teams/invitations.ex b/lib/plausible/teams/invitations.ex index 195914b07d80..5ee712f5e939 100644 --- a/lib/plausible/teams/invitations.ex +++ b/lib/plausible/teams/invitations.ex @@ -544,7 +544,17 @@ defmodule Plausible.Teams.Invitations do end @doc false - def send_invitation_email(guest_invitation, invitee) do + 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/test/plausible/site/memberships/create_invitation_test.exs b/test/plausible/site/memberships/create_invitation_test.exs index 1b96271b3114..6cc2fbd3ba94 100644 --- a/test/plausible/site/memberships/create_invitation_test.exs +++ b/test/plausible/site/memberships/create_invitation_test.exs @@ -117,8 +117,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) @@ -130,34 +130,20 @@ 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, 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) From 1438a0ad68ad76de2e08d2a884936fbe1d1a7b48 Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Thu, 21 Nov 2024 06:53:02 +0100 Subject: [PATCH 07/13] Even more strict SiteTransfer fetching --- lib/plausible/teams/adapter/read/invitations.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/plausible/teams/adapter/read/invitations.ex b/lib/plausible/teams/adapter/read/invitations.ex index f9a51bd57bbd..69701c95a1ed 100644 --- a/lib/plausible/teams/adapter/read/invitations.ex +++ b/lib/plausible/teams/adapter/read/invitations.ex @@ -98,7 +98,7 @@ defmodule Plausible.Teams.Adapter.Read.Invitations do team_fn: fn _ -> if invitation.role == :owner do Teams.SiteTransfer - |> Repo.get_by!(transfer_id: invitation.invitation_id) + |> Repo.get_by!(transfer_id: invitation.invitation_id, initiator_id: inviter.id) |> Repo.preload([:site, :initiator]) |> Plausible.Teams.Invitations.send_invitation_email(invitee) else From 2d92fd16a9395d900509832843e9314075b4ef67 Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Thu, 21 Nov 2024 07:06:27 +0100 Subject: [PATCH 08/13] Make skipping permissions work --- .../site/memberships/create_invitation.ex | 3 +-- .../memberships/create_invitation_test.exs | 19 ++++++++----------- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/lib/plausible/site/memberships/create_invitation.ex b/lib/plausible/site/memberships/create_invitation.ex index 8c8bf0a386c7..9b2e1355bb8f 100644 --- a/lib/plausible/site/memberships/create_invitation.ex +++ b/lib/plausible/site/memberships/create_invitation.ex @@ -77,7 +77,7 @@ defmodule Plausible.Site.Memberships.CreateInvitation do site, inviter, role, - opts + Keyword.get(opts, :check_permissions, true) ), :ok <- Plausible.Teams.Adapter.Read.Invitations.check_team_member_limit( @@ -104,7 +104,6 @@ defmodule Plausible.Site.Memberships.CreateInvitation do %Ecto.Changeset{} = changeset <- Invitation.new(attrs), {:ok, invitation} <- Plausible.Repo.insert(changeset) do Plausible.Teams.Invitations.invite_sync(site, invitation) - invitation Plausible.Teams.Adapter.Read.Invitations.send_invitation_email(inviter, invitation, invitee) diff --git a/test/plausible/site/memberships/create_invitation_test.exs b/test/plausible/site/memberships/create_invitation_test.exs index 6cc2fbd3ba94..c6a7a70d7ea6 100644 --- a/test/plausible/site/memberships/create_invitation_test.exs +++ b/test/plausible/site/memberships/create_invitation_test.exs @@ -225,14 +225,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( @@ -265,11 +262,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( From add62febda9c4aaabb21e88922764fc6980b2969 Mon Sep 17 00:00:00 2001 From: Adrian Gruntkowski Date: Thu, 21 Nov 2024 10:31:56 +0100 Subject: [PATCH 09/13] Make CreateInvitation read from team schemas behind FF fully --- lib/plausible/teams/billing.ex | 13 ++++- lib/plausible/teams/invitations.ex | 19 ++++--- .../controllers/site/membership_controller.ex | 4 +- test/plausible/site/admin_test.exs | 18 +++---- .../memberships/create_invitation_test.exs | 52 ++++--------------- .../site/membership_controller_test.exs | 3 -- test/support/teams/test.ex | 28 ++++++---- 7 files changed, 62 insertions(+), 75 deletions(-) diff --git a/lib/plausible/teams/billing.ex b/lib/plausible/teams/billing.ex index 0262b5cb3ea2..8b1182636d61 100644 --- a/lib/plausible/teams/billing.ex +++ b/lib/plausible/teams/billing.ex @@ -356,11 +356,21 @@ defmodule Plausible.Teams.Billing do end 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), left_join: gm in assoc(tm, :guest_memberships), - where: gm.site_id in ^pending_ownership_site_ids or tm.role == :owner, + where: gm.site_id in ^pending_ownership_site_ids, where: u.email not in ^exclude_emails, select: %{email: u.email} @@ -385,6 +395,7 @@ defmodule Plausible.Teams.Billing do 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 5ee712f5e939..4ff23ddc82e4 100644 --- a/lib/plausible/teams/invitations.ex +++ b/lib/plausible/teams/invitations.ex @@ -484,6 +484,8 @@ defmodule Plausible.Teams.Invitations do defp translate_role(role), do: role @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]) @@ -545,15 +547,18 @@ defmodule Plausible.Teams.Invitations 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 - ) + 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 diff --git a/lib/plausible_web/controllers/site/membership_controller.ex b/lib/plausible_web/controllers/site/membership_controller.ex index 33aea074bca5..faed17f774e1 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/test/plausible/site/admin_test.exs b/test/plausible/site/admin_test.exs index 45deac9cfc72..99590dd3882b 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/create_invitation_test.exs b/test/plausible/site/memberships/create_invitation_test.exs index c6a7a70d7ea6..e04cbde54b60 100644 --- a/test/plausible/site/memberships/create_invitation_test.exs +++ b/test/plausible/site/memberships/create_invitation_test.exs @@ -95,6 +95,8 @@ defmodule Plausible.Site.Memberships.CreateInvitationTest do 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] = for _ <- 1..4, do: new_user() @@ -147,28 +149,14 @@ defmodule Plausible.Site.Memberships.CreateInvitationTest do 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 @@ -186,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) diff --git a/test/plausible_web/controllers/site/membership_controller_test.exs b/test/plausible_web/controllers/site/membership_controller_test.exs index fa531388ecd7..03e0659a74f4 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/support/teams/test.ex b/test/support/teams/test.ex index e6dfa747bd69..5e3ff51c652a 100644 --- a/test/support/teams/test.ex +++ b/test/support/teams/test.ex @@ -60,10 +60,17 @@ defmodule Plausible.Teams.Test do role = Keyword.fetch!(args, :role) team = Repo.preload(site, :team).team - # insert(:site_membership, user: user, role: translate_role_to_old_model(role), site: site) + insert(:site_membership, user: user, role: translate_role_to_old_model(role), site: site) + + 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 + ) - team_membership = build(:team_membership, team: team, user: user, role: :guest) - Repo.insert!(team_membership, on_conflict: :nothing, conflict_target: [:team_id, :user_id]) + insert(:guest_membership, site: site, team_membership: team_membership, role: role) user |> Repo.preload([:site_memberships, :team_memberships]) end @@ -79,14 +86,15 @@ defmodule Plausible.Teams.Test do email when is_binary(email) -> email end - # insert(:invitation, - # email: email, - # inviter: inviter, - # role: translate_role_to_old_model(role), - # site: site - # ) old_model_invitation = - team_invitation = + insert(:invitation, + email: email, + inviter: inviter, + role: translate_role_to_old_model(role), + site: site + ) + + team_invitation = insert(:team_invitation, team: team, email: email, From 004cf177043964888fb5b9ee3f75b842e3eaf4b6 Mon Sep 17 00:00:00 2001 From: Adrian Gruntkowski Date: Thu, 21 Nov 2024 10:40:44 +0100 Subject: [PATCH 10/13] Fix passing options to `check_invitation_permissions` --- .../site/memberships/create_invitation.ex | 2 +- lib/plausible/teams/invitations.ex | 19 ++++++++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/lib/plausible/site/memberships/create_invitation.ex b/lib/plausible/site/memberships/create_invitation.ex index 9b2e1355bb8f..7d3f8f3ba7f2 100644 --- a/lib/plausible/site/memberships/create_invitation.ex +++ b/lib/plausible/site/memberships/create_invitation.ex @@ -77,7 +77,7 @@ defmodule Plausible.Site.Memberships.CreateInvitation do site, inviter, role, - Keyword.get(opts, :check_permissions, true) + opts ), :ok <- Plausible.Teams.Adapter.Read.Invitations.check_team_member_limit( diff --git a/lib/plausible/teams/invitations.ex b/lib/plausible/teams/invitations.ex index 4ff23ddc82e4..e5fab2bd9d3b 100644 --- a/lib/plausible/teams/invitations.ex +++ b/lib/plausible/teams/invitations.ex @@ -24,11 +24,10 @@ defmodule Plausible.Teams.Invitations do 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?), + with :ok <- check_invitation_permissions(site.team, inviter, opts), :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), @@ -469,14 +468,16 @@ defmodule Plausible.Teams.Invitations do end @doc false - def check_invitation_permissions(_site, _inviter, false = _check_permission?) do - :ok - end + def check_invitation_permissions(site, inviter, opts) do + check_permissions? = Keyword.get(opts, :check_permissions, true) - def check_invitation_permissions(site, inviter, _) do - case Teams.Memberships.site_role(site, inviter) do - {:ok, role} when role in [:owner, :editor, :admin] -> :ok - _ -> {:error, :forbidden} + if check_permissions? do + case Teams.Memberships.site_role(site, inviter) do + {:ok, role} when role in [:owner, :editor, :admin] -> :ok + _ -> {:error, :forbidden} + end + else + :ok end end From faac8f42feeacbfbb90760876ab112312a5a7b35 Mon Sep 17 00:00:00 2001 From: Adrian Gruntkowski Date: Thu, 21 Nov 2024 11:17:39 +0100 Subject: [PATCH 11/13] Fix allowance check for pageview usage for active or recently ended trial case --- lib/plausible_web/components/billing/plan_box.ex | 2 +- test/plausible_web/live/choose_plan_test.exs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/plausible_web/components/billing/plan_box.ex b/lib/plausible_web/components/billing/plan_box.ex index 10d1094f2ce0..d0a668360e33 100644 --- a/lib/plausible_web/components/billing/plan_box.ex +++ b/lib/plausible_web/components/billing/plan_box.ex @@ -275,7 +275,7 @@ defmodule PlausibleWeb.Components.Billing.PlanBox do trial_active_or_ended_recently? = not invited_user? && - Plausible.Teams.Adapter.Read.Teams.trial_days_left(current_user) <= 10 + Plausible.Teams.Adapter.Read.Teams.trial_days_left(current_user) >= -10 limit_checking_opts = cond do diff --git a/test/plausible_web/live/choose_plan_test.exs b/test/plausible_web/live/choose_plan_test.exs index ba1be2894bb2..bd0271966d52 100644 --- a/test/plausible_web/live/choose_plan_test.exs +++ b/test/plausible_web/live/choose_plan_test.exs @@ -350,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) From b46ea6bb04b32ab1285514cac1c150de78b99493 Mon Sep 17 00:00:00 2001 From: Adrian Gruntkowski Date: Thu, 21 Nov 2024 11:54:55 +0100 Subject: [PATCH 12/13] Fix `check_invitation_permissions` --- lib/plausible/teams.ex | 3 +-- lib/plausible/teams/adapter/read/invitations.ex | 1 + lib/plausible/teams/invitations.ex | 15 +++++++++++---- .../site/memberships/create_invitation_test.exs | 2 +- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/lib/plausible/teams.ex b/lib/plausible/teams.ex index b1ada49069d8..c575a20ee7e0 100644 --- a/lib/plausible/teams.ex +++ b/lib/plausible/teams.ex @@ -18,8 +18,7 @@ defmodule Plausible.Teams do team = with_subscription(team) not Plausible.Billing.Subscriptions.active?(team.subscription) && - trial_days_left(team) >= - 0 + trial_days_left(team) >= 0 end else def on_trial?(_), do: true diff --git a/lib/plausible/teams/adapter/read/invitations.ex b/lib/plausible/teams/adapter/read/invitations.ex index 69701c95a1ed..369a1e1d05a9 100644 --- a/lib/plausible/teams/adapter/read/invitations.ex +++ b/lib/plausible/teams/adapter/read/invitations.ex @@ -14,6 +14,7 @@ defmodule Plausible.Teams.Adapter.Read.Invitations do Plausible.Teams.Invitations.check_invitation_permissions( site, inviter, + role, opts ) end, diff --git a/lib/plausible/teams/invitations.ex b/lib/plausible/teams/invitations.ex index e5fab2bd9d3b..3a22991d4e42 100644 --- a/lib/plausible/teams/invitations.ex +++ b/lib/plausible/teams/invitations.ex @@ -27,7 +27,7 @@ defmodule Plausible.Teams.Invitations do site = Repo.preload(site, :team) role = translate_role(role) - with :ok <- check_invitation_permissions(site.team, inviter, opts), + with :ok <- check_invitation_permissions(site.team, inviter, role, opts), :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), @@ -468,13 +468,20 @@ defmodule Plausible.Teams.Invitations do end @doc false - def check_invitation_permissions(site, inviter, opts) do + def check_invitation_permissions(site, inviter, invitation_role, opts) do check_permissions? = Keyword.get(opts, :check_permissions, true) if check_permissions? do case Teams.Memberships.site_role(site, inviter) do - {:ok, role} when role in [:owner, :editor, :admin] -> :ok - _ -> {:error, :forbidden} + {: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 diff --git a/test/plausible/site/memberships/create_invitation_test.exs b/test/plausible/site/memberships/create_invitation_test.exs index e04cbde54b60..cd0d2fc4e6ac 100644 --- a/test/plausible/site/memberships/create_invitation_test.exs +++ b/test/plausible/site/memberships/create_invitation_test.exs @@ -135,7 +135,7 @@ defmodule Plausible.Site.Memberships.CreateInvitationTest do inviter = new_user() site = new_site() - add_guest(site, role: :editor) + add_guest(site, user: inviter, role: :editor) assert {:error, :forbidden} = CreateInvitation.create_invitation(site, inviter, "vini@plausible.test", :owner) From a2c473406403047aea1ecbb70b7bb0fb4f8fb845 Mon Sep 17 00:00:00 2001 From: Adrian Gruntkowski Date: Thu, 21 Nov 2024 12:02:04 +0100 Subject: [PATCH 13/13] Remove no longer relevant invite implementations for Teams --- lib/plausible/teams/invitations.ex | 61 +------------------ .../memberships/accept_invitation_test.exs | 58 ------------------ 2 files changed, 3 insertions(+), 116 deletions(-) diff --git a/lib/plausible/teams/invitations.ex b/lib/plausible/teams/invitations.ex index 3a22991d4e42..aad1426be456 100644 --- a/lib/plausible/teams/invitations.ex +++ b/lib/plausible/teams/invitations.ex @@ -3,40 +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, :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 - site = Repo.preload(site, :team) - role = translate_role(role) - - with :ok <- check_invitation_permissions(site.team, inviter, role, opts), - :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) @@ -96,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, :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) @@ -218,11 +163,11 @@ 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} @@ -251,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, diff --git a/test/plausible/site/memberships/accept_invitation_test.exs b/test/plausible/site/memberships/accept_invitation_test.exs index 43c2f23bb73e..29a7d7693db7 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: [])