From 7d6f10f0cbcd86f9f799f67b76dc8502be781a50 Mon Sep 17 00:00:00 2001 From: Adrian Gruntkowski Date: Wed, 23 Oct 2024 13:29:15 +0200 Subject: [PATCH] Ensure team is present before use in sync logic (#4709) * Ensure team is present before use in sync logic * Ensure teams backfill works against partially assigned sites * Associate site with team on creation * Associate site with team on sync * Reuse alias * Add tests for invitation creation sync * Move team assertions to a helper module * Format * Test team creation on site creation via Sites context module * Add tests for teams sync on subscription changes * Tag tests * Test grace period start syncing up with teams * Test grace period manual lock sycning w/ teams * Test grace period end sycing up w/ teams * Test clearing grace period sync with teams * Update moduledoc * Fix missing preloads and wrong result pattern matching in sync logic * Test sync on accepting invites and site transfers * Test sync on membership role update and member removal * transfer async fix WIP * Stop privisioning team in site factory * Remove unused relationship from Site schema * Ensure consistent parsing of `passthrough` from Paddle webhook * Update team passthrough notification tests & logic --------- Co-authored-by: Adam Rutkowski --- lib/plausible/auth/grace_period.ex | 8 +- lib/plausible/billing/billing.ex | 56 +++++---- .../data_migration/backfill_teams.ex | 17 +-- .../site/memberships/accept_invitation.ex | 7 +- lib/plausible/sites.ex | 7 +- lib/plausible/teams.ex | 49 +++++--- lib/plausible/teams/invitations.ex | 68 ++++------ test/plausible/billing/billing_test.exs | 94 ++++++++++++++ test/plausible/billing/site_locker_test.exs | 22 ++++ .../memberships/accept_invitation_test.exs | 118 ++++++++++++++++-- .../memberships/create_invitation_test.exs | 65 +++++++++- test/plausible/site/sites_test.exs | 23 +++- .../site/membership_controller_test.exs | 103 +++++++++++++++ test/support/factory.ex | 6 +- test/support/teams/test.ex | 88 +++++++++++++ test/workers/check_usage_test.exs | 96 ++++++++++++++ 16 files changed, 712 insertions(+), 115 deletions(-) create mode 100644 test/support/teams/test.ex diff --git a/lib/plausible/auth/grace_period.ex b/lib/plausible/auth/grace_period.ex index 94b6d3560..226736144 100644 --- a/lib/plausible/auth/grace_period.ex +++ b/lib/plausible/auth/grace_period.ex @@ -74,18 +74,18 @@ defmodule Plausible.Auth.GracePeriod do Ecto.Changeset.change(user, grace_period: nil) end - @spec active?(User.t()) :: boolean() + @spec active?(Plausible.Teams.Team.t() | User.t()) :: boolean() @doc """ Returns whether the grace period is still active for a User. Defaults to false if the user is nil or there is no grace period. """ - def active?(user) + def active?(user_or_team) - def active?(%User{grace_period: %__MODULE__{end_date: %Date{} = end_date}}) do + def active?(%{grace_period: %__MODULE__{end_date: %Date{} = end_date}}) do Timex.diff(end_date, Date.utc_today(), :days) >= 0 end - def active?(%User{grace_period: %__MODULE__{manual_lock: true}}) do + def active?(%{grace_period: %__MODULE__{manual_lock: true}}) do true end diff --git a/lib/plausible/billing/billing.ex b/lib/plausible/billing/billing.ex index c8a3b4c5f..9f5071e51 100644 --- a/lib/plausible/billing/billing.ex +++ b/lib/plausible/billing/billing.ex @@ -120,25 +120,7 @@ defmodule Plausible.Billing do defp handle_subscription_created(params) do params = if present?(params["passthrough"]) do - case String.split(to_string(params["passthrough"]), ";") do - [user_id] -> - user = Repo.get!(User, user_id) - {:ok, team} = Plausible.Teams.get_or_create(user) - Map.put(params, "team_id", team.id) - - [user_id, ""] -> - user = Repo.get!(User, user_id) - {:ok, team} = Plausible.Teams.get_or_create(user) - - params - |> Map.put("passthrough", user_id) - |> Map.put("team_id", team.id) - - [user_id, team_id] -> - params - |> Map.put("passthrough", user_id) - |> Map.put("team_id", team_id) - end + format_params(params) else user = Repo.get_by!(User, email: params["email"]) {:ok, team} = Plausible.Teams.get_or_create(user) @@ -148,7 +130,10 @@ defmodule Plausible.Billing do |> Map.put("team_id", team.id) end - subscription_params = format_subscription(params) |> add_last_bill_date(params) + subscription_params = + params + |> format_subscription() + |> add_last_bill_date(params) %Subscription{} |> Subscription.changeset(subscription_params) @@ -175,8 +160,13 @@ defmodule Plausible.Billing do irrelevant? = params["old_status"] == "paused" && params["status"] == "past_due" if subscription && not irrelevant? do + params = + params + |> format_params() + |> format_subscription() + subscription - |> Subscription.changeset(format_subscription(params)) + |> Subscription.changeset(params) |> Repo.update!() |> after_subscription_update() end @@ -230,8 +220,26 @@ defmodule Plausible.Billing do end end + defp format_params(%{"passthrough" => passthrough} = params) do + case String.split(to_string(passthrough), ";") do + [user_id] -> + user = Repo.get!(User, user_id) + {:ok, team} = Plausible.Teams.get_or_create(user) + Map.put(params, "team_id", team.id) + + ["user:" <> user_id, "team:" <> team_id] -> + params + |> Map.put("passthrough", user_id) + |> Map.put("team_id", team_id) + end + end + + defp format_params(params) do + params + end + defp format_subscription(params) do - params = %{ + subscription_params = %{ paddle_subscription_id: params["subscription_id"], paddle_plan_id: params["subscription_plan_id"], cancel_url: params["cancel_url"], @@ -244,9 +252,9 @@ defmodule Plausible.Billing do } if team_id = params["team_id"] do - Map.put(params, :team_id, team_id) + Map.put(subscription_params, :team_id, team_id) else - params + subscription_params end end diff --git a/lib/plausible/data_migration/backfill_teams.ex b/lib/plausible/data_migration/backfill_teams.ex index db03c7b0d..dd7dfffb8 100644 --- a/lib/plausible/data_migration/backfill_teams.ex +++ b/lib/plausible/data_migration/backfill_teams.ex @@ -402,9 +402,11 @@ defmodule Plausible.DataMigration.BackfillTeams do fn {{owner, site_ids}, idx} -> @repo.transaction( fn -> + {:ok, team} = Teams.get_or_create(owner) + team = - "My Team" - |> Teams.Team.changeset() + team + |> Ecto.Changeset.change() |> Ecto.Changeset.put_change(:trial_expiry_date, owner.trial_expiry_date) |> Ecto.Changeset.put_change(:accept_traffic_until, owner.accept_traffic_until) |> Ecto.Changeset.put_change( @@ -412,15 +414,8 @@ defmodule Plausible.DataMigration.BackfillTeams do owner.allow_next_upgrade_override ) |> Ecto.Changeset.put_embed(:grace_period, owner.grace_period) - |> Ecto.Changeset.put_change(:inserted_at, owner.inserted_at) - |> Ecto.Changeset.put_change(:updated_at, owner.updated_at) - |> @repo.insert!() - - team - |> Teams.Membership.changeset(owner, :owner) - |> Ecto.Changeset.put_change(:inserted_at, owner.inserted_at) - |> Ecto.Changeset.put_change(:updated_at, owner.updated_at) - |> @repo.insert!() + |> Ecto.Changeset.force_change(:updated_at, owner.updated_at) + |> @repo.update!() @repo.update_all(from(s in Plausible.Site, where: s.id in ^site_ids), set: [team_id: team.id] diff --git a/lib/plausible/site/memberships/accept_invitation.ex b/lib/plausible/site/memberships/accept_invitation.ex index b1cc23031..838338eb8 100644 --- a/lib/plausible/site/memberships/accept_invitation.ex +++ b/lib/plausible/site/memberships/accept_invitation.ex @@ -42,12 +42,7 @@ defmodule Plausible.Site.Memberships.AcceptInvitation do case Repo.transaction(multi) do {:ok, changes} -> with_teams do - sync_job = - Task.async(fn -> - Plausible.Teams.Invitations.transfer_site_sync(site, user) - end) - - Task.await(sync_job) + Plausible.Teams.Invitations.transfer_site_sync(site, user) end membership = Repo.preload(changes.membership, [:site, :user]) diff --git a/lib/plausible/sites.ex b/lib/plausible/sites.ex index a492ec093..e61b70e68 100644 --- a/lib/plausible/sites.ex +++ b/lib/plausible/sites.ex @@ -179,6 +179,9 @@ defmodule Plausible.Sites do with :ok <- Quota.ensure_can_add_new_site(user) do Ecto.Multi.new() |> Ecto.Multi.put(:site_changeset, Site.new(params)) + |> Ecto.Multi.run(:create_team, fn _repo, _context -> + Plausible.Teams.get_or_create(user) + end) |> Ecto.Multi.run(:clear_changed_from, fn _repo, %{site_changeset: %{changes: %{domain: domain}}} -> case get_for_user(user.id, domain, [:owner]) do @@ -196,7 +199,9 @@ defmodule Plausible.Sites do _repo, _context -> {:ok, :ignore} end) - |> Ecto.Multi.insert(:site, fn %{site_changeset: site} -> site end) + |> Ecto.Multi.insert(:site, fn %{site_changeset: site, create_team: team} -> + Ecto.Changeset.put_assoc(site, :team, team) + end) |> Ecto.Multi.insert(:site_membership, fn %{site: site} -> Site.Membership.new(site, user) end) diff --git a/lib/plausible/teams.ex b/lib/plausible/teams.ex index b953de26e..253fc6e72 100644 --- a/lib/plausible/teams.ex +++ b/lib/plausible/teams.ex @@ -16,6 +16,27 @@ defmodule Plausible.Teams do Repo.preload(team, :sites).sites end + @doc """ + Create (when necessary) and load team relation for provided site. + + Used for sync logic to work smoothly during transitional period. + """ + def load_for_site(site) do + site = Repo.preload(site, [:team, :owner]) + + if site.team do + site + else + {:ok, team} = get_or_create(site.owner) + + site + |> Ecto.Changeset.change() + |> Ecto.Changeset.put_assoc(:team, team) + |> Ecto.Changeset.force_change(:updated_at, site.updated_at) + |> Repo.update!() + end + end + @doc """ Get or create user's team. @@ -31,24 +52,21 @@ defmodule Plausible.Teams do def get_or_create(user) do with {:error, :no_team} <- get_owned_by_user(user) do case create_my_team(user) do - {:ok, team} -> {:ok, team} - {:error, :exists_already} -> get_owned_by_user(user) + {:ok, team} -> + {:ok, team} + + {:error, :exists_already} -> + get_owned_by_user(user) end end end def sync_team(user) do - case get_owned_by_user(user) do - {:ok, team} -> - team - |> Teams.Team.sync_changeset(user) - |> Repo.update!() + {:ok, team} = get_or_create(user) - _ -> - :skip - end - - :ok + team + |> Teams.Team.sync_changeset(user) + |> Repo.update!() end defp create_my_team(user) do @@ -88,8 +106,11 @@ defmodule Plausible.Teams do |> Repo.one() case result do - nil -> {:error, :no_team} - team -> {:ok, team} + nil -> + {:error, :no_team} + + team -> + {:ok, team} end end diff --git a/lib/plausible/teams/invitations.ex b/lib/plausible/teams/invitations.ex index e443901b2..8e5eaf7c3 100644 --- a/lib/plausible/teams/invitations.ex +++ b/lib/plausible/teams/invitations.ex @@ -39,7 +39,7 @@ defmodule Plausible.Teams.Invitations do end def invite_sync(site, site_invitation) do - site = Repo.preload(site, :team) + site = Teams.load_for_site(site) site_invitation = Repo.preload(site_invitation, :inviter) role = translate_role(site_invitation.role) @@ -57,14 +57,6 @@ defmodule Plausible.Teams.Invitations do site_invitation.inviter ) end - catch - _, thrown -> - Sentry.capture_message( - "Failed to sync invitation for site ##{site.id} and email ##{site_invitation.email}", - extra: %{ - error: inspect(thrown) - } - ) end def transfer_site(site, new_owner, now \\ NaiveDateTime.utc_now(:second)) do @@ -75,6 +67,7 @@ defmodule Plausible.Teams.Invitations do :ok <- ensure_can_take_ownership(site, team) do site = Repo.preload(site, [ + :team, :owner, guest_memberships: [team_membership: :user], guest_invitations: [team_invitation: :user] @@ -92,10 +85,12 @@ defmodule Plausible.Teams.Invitations do end def transfer_site_sync(site, user) do - {:ok, team} = Plausible.Teams.get_or_create(user) + {:ok, team} = Teams.get_or_create(user) + site = Teams.load_for_site(site) site = Repo.preload(site, [ + :team, :owner, guest_memberships: [team_membership: :user], guest_invitations: [team_invitation: :user] @@ -105,14 +100,6 @@ defmodule Plausible.Teams.Invitations do Repo.transaction(fn -> :ok = transfer_site_ownership(site, team, NaiveDateTime.utc_now(:second)) end) - catch - _, thrown -> - Sentry.capture_message( - "Failed to sync transfer site for site ##{site.id} and user ##{user.id}", - extra: %{ - error: inspect(thrown) - } - ) end def accept(invitation_id, user, now \\ NaiveDateTime.utc_now(:second)) do @@ -135,55 +122,52 @@ defmodule Plausible.Teams.Invitations do site: :team ) + site = Teams.load_for_site(site_invitation.site) + site_invitation = %{site_invitation | site: site} + role = case site_invitation.role do :viewer -> :viewer :admin -> :editor end - guest_invitation = + {:ok, guest_invitation} = create_invitation( site_invitation.site, - site_invitation.invitee_email, + site_invitation.email, role, site_invitation.inviter ) + team_invitation = + guest_invitation.team_invitation + |> Repo.preload([:team, :inviter, guest_invitations: :site]) + {:ok, _} = - do_accept(guest_invitation.team_invitation, user, NaiveDateTime.utc_now(:second), - send_email?: false - ) - catch - _, thrown -> - Sentry.capture_message( - "Failed to sync accept invitation for site ##{site_invitation.site_id} and user ##{user.id}", - extra: %{ - error: inspect(thrown) - } - ) + do_accept(team_invitation, user, NaiveDateTime.utc_now(:second), send_email?: false) end def accept_transfer_sync(site_invitation, user) do {:ok, team} = Teams.get_or_create(user) site = - Repo.preload(site_invitation.site, [:owner, guest_memberships: [team_membership: :user]]) + site_invitation.site + |> Teams.load_for_site() + |> Repo.preload([ + :team, + :owner, + guest_memberships: [team_membership: :user], + guest_invitations: [team_invitation: :user] + ]) - site_transfer = create_site_transfer(site, site_invitation.inviter, site_invitation.email) + {:ok, site_transfer} = + create_site_transfer(site, site_invitation.inviter, site_invitation.email) {:ok, _} = Repo.transaction(fn -> :ok = transfer_site_ownership(site, team, NaiveDateTime.utc_now(:second)) Repo.delete!(site_transfer) end) - catch - _, thrown -> - Sentry.capture_message( - "Failed to sync accept transfer for site ##{site_invitation.site_id} and user ##{user.id}", - extra: %{ - error: inspect(thrown) - } - ) end defp check_transfer_permissions(_team, _initiator, false = _check_permissions?) do @@ -536,7 +520,7 @@ defmodule Plausible.Teams.Invitations do team_membership |> Teams.GuestMembership.changeset(guest_invitation.site, guest_invitation.role) |> Repo.insert( - on_conflict: [set: [updated_at: now, role: guest_invitation.role]], + on_conflict: [set: [updated_at: now]], conflict_target: [:team_membership_id, :site_id] ) diff --git a/test/plausible/billing/billing_test.exs b/test/plausible/billing/billing_test.exs index 788c53bd2..206a4f33e 100644 --- a/test/plausible/billing/billing_test.exs +++ b/test/plausible/billing/billing_test.exs @@ -1,5 +1,6 @@ defmodule Plausible.BillingTest do use Plausible.DataCase + use Plausible.Teams.Test use Bamboo.Test, shared: true require Plausible.Billing.Subscription.Status alias Plausible.Billing @@ -131,6 +132,42 @@ defmodule Plausible.BillingTest do assert subscription.currency_code == "EUR" end + @tag :teams + test "creates a subscription with teams passthrough" do + user = insert(:user) + {:ok, team} = Plausible.Teams.get_or_create(user) + + assert_team_exists(user) + + %{@subscription_created_params | "passthrough" => "user:#{user.id};team:#{team.id}"} + |> Billing.subscription_created() + + assert Repo.get_by(Plausible.Billing.Subscription, user_id: user.id, team_id: team.id) + end + + @tag :teams + test "creates a team on create subscription" do + user = insert(:user) + + %{@subscription_created_params | "passthrough" => user.id} + |> Billing.subscription_created() + + team = assert_team_exists(user) + assert Repo.get_by(Plausible.Billing.Subscription, user_id: user.id, team_id: team.id) + end + + @tag :teams + test "doesn't create additional teams on create subscription" do + user = insert(:user) + {:ok, team} = Plausible.Teams.get_or_create(user) + + %{@subscription_created_params | "passthrough" => user.id} + |> Billing.subscription_created() + + team = assert_team_exists(user, team.id) + assert Repo.get_by(Plausible.Billing.Subscription, user_id: user.id, team_id: team.id) + end + test "create with email address" do user = insert(:user) @@ -212,6 +249,63 @@ defmodule Plausible.BillingTest do assert subscription.next_bill_amount == "12.00" end + @tag :teams + test "creates a team on subscription update" do + user = insert(:user) + subscription = insert(:subscription, user: user) + + @subscription_updated_params + |> Map.merge(%{ + "subscription_id" => subscription.paddle_subscription_id, + "passthrough" => user.id + }) + |> Billing.subscription_updated() + + team = assert_team_exists(user) + assert Repo.get_by(Plausible.Billing.Subscription, user_id: user.id, team_id: team.id) + end + + @tag :teams + test "updates subscription with user/team passthrough" do + user = insert(:user) + subscription = insert(:subscription, user: user) + {:ok, team} = Plausible.Teams.get_or_create(user) + + @subscription_updated_params + |> Map.merge(%{ + "subscription_id" => subscription.paddle_subscription_id, + "passthrough" => "user:#{user.id};team:#{team.id}" + }) + |> Billing.subscription_updated() + + team = assert_team_exists(user) + assert Repo.get_by(Plausible.Billing.Subscription, user_id: user.id, team_id: team.id) + end + + @tag :teams + test "syncs team properties with user on subscription update" do + user = + insert(:user, accept_traffic_until: ~D[2001-01-01], allow_next_upgrade_override: true) + + user = Plausible.Users.start_grace_period(user) + + subscription = insert(:subscription, user: user) + + @subscription_updated_params + |> Map.merge(%{ + "subscription_id" => subscription.paddle_subscription_id, + "passthrough" => user.id + }) + |> Billing.subscription_updated() + + team = assert_team_exists(user) + user = Repo.reload!(user) + assert team.grace_period == user.grace_period + assert team.trial_expiry_date == user.trial_expiry_date + assert team.accept_traffic_until == user.accept_traffic_until + assert team.allow_next_upgrade_override == user.allow_next_upgrade_override + end + test "status update from 'paused' to 'past_due' is ignored" do user = insert(:user) subscription = insert(:subscription, user: user, status: Subscription.Status.paused()) diff --git a/test/plausible/billing/site_locker_test.exs b/test/plausible/billing/site_locker_test.exs index 1e3bdbae4..7f98b4c14 100644 --- a/test/plausible/billing/site_locker_test.exs +++ b/test/plausible/billing/site_locker_test.exs @@ -1,6 +1,7 @@ defmodule Plausible.Billing.SiteLockerTest do use Plausible.DataCase use Bamboo.Test, shared: true + use Plausible.Teams.Test require Plausible.Billing.Subscription.Status alias Plausible.Billing.{SiteLocker, Subscription} @@ -127,6 +128,27 @@ defmodule Plausible.Billing.SiteLockerTest do assert Repo.reload!(site).locked end + @tag :teams + test "syncs grace period end with teams" do + grace_period = %Plausible.Auth.GracePeriod{end_date: Timex.shift(Timex.today(), days: -1)} + user = insert(:user, grace_period: grace_period) + + insert(:subscription, status: Subscription.Status.active(), user: user) + + insert(:site, + memberships: [ + build(:site_membership, user: user, role: :owner) + ] + ) + + assert SiteLocker.update_sites_for(user) == {:locked, :grace_period_ended_now} + + assert user = Repo.reload!(user) + team = assert_team_exists(user) + assert user.grace_period.is_over + assert team.grace_period.is_over + end + test "sends email if grace period has ended" do grace_period = %Plausible.Auth.GracePeriod{end_date: Timex.shift(Timex.today(), days: -1)} user = insert(:user, grace_period: grace_period) diff --git a/test/plausible/site/memberships/accept_invitation_test.exs b/test/plausible/site/memberships/accept_invitation_test.exs index 7b2aacc88..485f2d822 100644 --- a/test/plausible/site/memberships/accept_invitation_test.exs +++ b/test/plausible/site/memberships/accept_invitation_test.exs @@ -3,6 +3,7 @@ defmodule Plausible.Site.Memberships.AcceptInvitationTest do require Plausible.Billing.Subscription.Status use Plausible.DataCase, async: true use Bamboo.Test + use Plausible.Teams.Test alias Plausible.Site.Memberships.AcceptInvitation @@ -11,6 +12,7 @@ defmodule Plausible.Site.Memberships.AcceptInvitationTest do describe "transfer_ownership/3" do test "transfers ownership successfully" do site = insert(:site, memberships: []) + existing_owner = insert(:user) existing_membership = @@ -34,6 +36,49 @@ defmodule Plausible.Site.Memberships.AcceptInvitationTest do assert_no_emails_delivered() end + @tag :teams + test "syncs ownership transfers provided memberships exist already" do + site = insert(:site, memberships: []) + existing_owner = insert(:user) + + _existing_membership = + insert(:site_membership, user: existing_owner, site: site, role: :owner) + + {:ok, old_team} = Plausible.Teams.get_or_create(existing_owner) + + another_user = insert(:user) + + insert(:site_membership, user: another_user, site: site, role: :viewer) + + 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) + insert(:growth_subscription, user: new_owner) + + assert {:ok, new_membership} = + AcceptInvitation.transfer_ownership(site, new_owner) + + assert new_membership.site_id == site.id + assert new_membership.user_id == new_owner.id + assert new_membership.role == :owner + + team = assert_team_exists(new_owner) + assert team.id != old_team.id + assert_team_attached(site, team.id) + + assert_guest_membership(team, site, another_user, :viewer) + 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) @@ -41,11 +86,9 @@ defmodule Plausible.Site.Memberships.AcceptInvitationTest do _existing_membership = insert(:site_membership, user: existing_owner, site: site, role: :owner) + site = Plausible.Teams.load_for_site(site) old_team = site.team - existing_team_membership = - insert(:team_membership, user: existing_owner, team: old_team, role: :owner) - another_user = insert(:user) another_team_membership = @@ -70,10 +113,7 @@ defmodule Plausible.Site.Memberships.AcceptInvitationTest do assert new_team_membership.user_id == new_owner.id assert new_team_membership.role == :owner - existing_team_membership = Repo.reload!(existing_team_membership) - assert existing_team_membership.user_id == existing_owner.id - assert existing_team_membership.team_id == old_team.id - assert existing_team_membership.role == :owner + assert_team_membership(existing_owner, old_team) refute Repo.reload(another_team_membership) refute Repo.reload(another_guest_membership) @@ -278,6 +318,34 @@ defmodule Plausible.Site.Memberships.AcceptInvitationTest do ) end + @tag :teams + test "sync newly converted membership with team" do + inviter = insert(:user) + invitee = insert(:user) + site = insert(:site, members: [inviter]) + + invitation = + insert(:invitation, + site_id: site.id, + inviter: inviter, + email: invitee.email, + role: :admin + ) + + assert {:ok, membership} = + AcceptInvitation.accept_invitation(invitation.invitation_id, invitee) + + assert membership.site_id == site.id + assert membership.user_id == invitee.id + assert membership.role == :admin + + team = assert_team_exists(inviter) + assert_team_attached(site, team.id) + + assert_guest_membership(team, site, invitee, :editor) + end + + @tag :teams test "converts an invitation into a membership (TEAMS)" do inviter = insert(:user) invitee = insert(:user) @@ -424,6 +492,42 @@ defmodule Plausible.Site.Memberships.AcceptInvitationTest do ) end + @tag :teams + test "syncs accepted ownership transfer to teams" 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 + # site = Repo.reload!(site) + + new_owner = insert(:user) + insert(:growth_subscription, user: new_owner) + + invitation = + insert(:invitation, + site_id: site.id, + inviter: existing_owner, + email: new_owner.email, + role: :owner + ) + + assert {:ok, _new_membership} = + AcceptInvitation.accept_invitation( + invitation.invitation_id, + new_owner + ) + + team = assert_team_exists(new_owner) + assert team.id != old_team.id + assert_team_attached(site, team.id) + + assert_guest_membership(team, site, existing_owner, :editor) + end + @tag :ee_only test "unlocks a previously locked site after transfer" 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 21ab46ae3..795c1bec0 100644 --- a/test/plausible/site/memberships/create_invitation_test.exs +++ b/test/plausible/site/memberships/create_invitation_test.exs @@ -3,6 +3,7 @@ defmodule Plausible.Site.Memberships.CreateInvitationTest do use Plausible use Plausible.DataCase use Bamboo.Test + use Plausible.Teams.Test @subject_prefix if ee?(), do: "[Plausible Analytics] ", else: "[Plausible CE] " @@ -27,6 +28,64 @@ defmodule Plausible.Site.Memberships.CreateInvitationTest do 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) @@ -211,8 +270,8 @@ defmodule Plausible.Site.Memberships.CreateInvitationTest do build(:site_membership, user: invitee, role: :viewer) ] ) + |> Plausible.Teams.load_for_site() - insert(:team_membership, team: site.team, user: inviter, role: :owner) insert(:team_membership, team: site.team, user: invitee, role: :viewer) assert {:ok, %Plausible.Auth.Invitation{}} = @@ -232,7 +291,7 @@ defmodule Plausible.Site.Memberships.CreateInvitationTest do ] ) - insert(:team_membership, team: site.team, user: inviter, role: :owner) + site = Plausible.Teams.load_for_site(site) assert {:error, :transfer_to_self} = CreateInvitation.create_invitation(site, inviter, "vini@plausible.test", :owner) @@ -269,8 +328,8 @@ defmodule Plausible.Site.Memberships.CreateInvitationTest do build(:site_membership, user: inviter, role: :viewer) ] ) + |> Plausible.Teams.load_for_site() - insert(:team_membership, team: site.team, user: owner, role: :owner) insert(:team_membership, team: site.team, user: inviter, role: :viewer) assert {:error, :forbidden} = diff --git a/test/plausible/site/sites_test.exs b/test/plausible/site/sites_test.exs index 36cc24786..07fea67c1 100644 --- a/test/plausible/site/sites_test.exs +++ b/test/plausible/site/sites_test.exs @@ -1,18 +1,38 @@ defmodule Plausible.SitesTest do use Plausible.DataCase + use Plausible.Teams.Test alias Plausible.Sites describe "create a site" do + @tag :teams test "creates a site" do user = insert(:user) params = %{"domain" => "example.com", "timezone" => "Europe/London"} - assert {:ok, %{site: %{domain: "example.com", timezone: "Europe/London"}}} = + assert {:ok, %{site: %{domain: "example.com", timezone: "Europe/London"} = site}} = Sites.create(user, params) + + assert_team_attached(site) end + @tag :teams + test "creates a site and syncs the team properties" do + user = insert(:user, trial_expiry_date: nil) + + params = %{"domain" => "example.com", "timezone" => "Europe/London"} + + assert {:ok, %{site: %{domain: "example.com", timezone: "Europe/London"} = site}} = + Sites.create(user, params) + + team = assert_team_attached(site) + user = Repo.reload!(user) + assert not is_nil(user.trial_expiry_date) + assert user.trial_expiry_date == team.trial_expiry_date + end + + @tag :teams test "creates a site (TEAM)" do user = insert(:user) {:ok, team} = Plausible.Teams.get_or_create(user) @@ -32,6 +52,7 @@ defmodule Plausible.SitesTest do Sites.create(user, params) end + @tag :teams test "fails on invalid timezone (TEAM)" do user = insert(:user) {:ok, team} = Plausible.Teams.get_or_create(user) diff --git a/test/plausible_web/controllers/site/membership_controller_test.exs b/test/plausible_web/controllers/site/membership_controller_test.exs index 0bb4b76f8..bb7edd250 100644 --- a/test/plausible_web/controllers/site/membership_controller_test.exs +++ b/test/plausible_web/controllers/site/membership_controller_test.exs @@ -308,6 +308,32 @@ defmodule PlausibleWeb.Site.MembershipControllerTest do assert membership.role == :viewer end + @tag :teams + test "syncs role update to team", %{conn: conn, user: user} do + admin = insert(:user) + + site = + insert(:site, + memberships: [ + build(:site_membership, user: user, role: :owner), + build(:site_membership, user: admin, role: :admin) + ] + ) + |> Plausible.Teams.load_for_site() + + team_membership = + insert(:team_membership, user: admin, team: site.team, role: :guest) + + guest_membership = + insert(:guest_membership, team_membership: team_membership, site: site, role: :editor) + + membership = Repo.get_by(Plausible.Site.Membership, user_id: admin.id) + + put(conn, "/sites/#{site.domain}/memberships/#{membership.id}/role/viewer") + + assert Repo.reload!(guest_membership).role == :viewer + end + test "can downgrade yourself from admin to viewer, redirects to stats instead", %{ conn: conn, user: user @@ -436,6 +462,83 @@ defmodule PlausibleWeb.Site.MembershipControllerTest do refute Repo.exists?(from sm in Plausible.Site.Membership, where: sm.user_id == ^admin.id) end + @tag :teams + test "syncs member removal to team", %{conn: conn, user: user} do + admin = insert(:user) + + site = + insert(:site, + memberships: [ + build(:site_membership, user: user, role: :owner), + build(:site_membership, user: admin, role: :admin) + ] + ) + |> Plausible.Teams.load_for_site() + + team_membership = + insert(:team_membership, user: admin, team: site.team, role: :guest) + + guest_membership = + insert(:guest_membership, team_membership: team_membership, site: site, role: :editor) + + membership = Enum.find(site.memberships, &(&1.role == :admin)) + + conn = delete(conn, "/sites/#{site.domain}/memberships/#{membership.id}") + assert Phoenix.Flash.get(conn.assigns.flash, :success) =~ "has been removed" + + refute Repo.reload(guest_membership) + refute Repo.reload(team_membership) + end + + @tag :teams + test "sync retains team guest membership when there's another guest membership on it", %{ + conn: conn, + user: user + } do + admin = insert(:user) + + site = + insert(:site, + memberships: [ + build(:site_membership, user: user, role: :owner), + build(:site_membership, user: admin, role: :admin) + ] + ) + |> Plausible.Teams.load_for_site() + + another_site = + insert(:site, + team: site.team, + memberships: [ + build(:site_membership, user: user, role: :owner), + build(:site_membership, user: admin, role: :admin) + ] + ) + |> Plausible.Teams.load_for_site() + + team_membership = + insert(:team_membership, user: admin, team: site.team, role: :guest) + + guest_membership = + insert(:guest_membership, team_membership: team_membership, site: site, role: :editor) + + another_guest_membership = + insert(:guest_membership, + team_membership: team_membership, + site: another_site, + role: :editor + ) + + membership = Enum.find(site.memberships, &(&1.role == :admin)) + + conn = delete(conn, "/sites/#{site.domain}/memberships/#{membership.id}") + assert Phoenix.Flash.get(conn.assigns.flash, :success) =~ "has been removed" + + refute Repo.reload(guest_membership) + assert Repo.reload(another_guest_membership) + assert Repo.reload(team_membership) + end + test "fails to remove a member from a foreign site", %{conn: conn, user: user} do foreign_site = insert(:site, diff --git a/test/support/factory.ex b/test/support/factory.ex index c6c6b6843..53bf4f357 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -75,10 +75,12 @@ defmodule Plausible.Factory do Map.has_key?(attrs, :members) || Map.has_key?(attrs, :owner) - attrs = if defined_memberships?, do: attrs, else: Map.put_new(attrs, :members, [build(:user)]) + attrs = + if defined_memberships?, + do: attrs, + else: Map.put_new(attrs, :members, [build(:user)]) site = %Plausible.Site{ - team: build(:team), native_stats_start_at: ~N[2000-01-01 00:00:00], domain: domain, timezone: "UTC" diff --git a/test/support/teams/test.ex b/test/support/teams/test.ex new file mode 100644 index 000000000..983ea40df --- /dev/null +++ b/test/support/teams/test.ex @@ -0,0 +1,88 @@ +defmodule Plausible.Teams.Test do + @moduledoc """ + Convenience assertions for teams schema transition + """ + alias Plausible.Repo + + use ExUnit.CaseTemplate + + defmacro __using__(_) do + quote do + import Plausible.Teams.Test + end + end + + def assert_team_exists(user, team_id \\ nil) do + assert %{team_memberships: memberships} = Repo.preload(user, team_memberships: :team) + + tm = + case memberships do + [tm] -> tm + _ -> raise "Team doesn't exist for user #{user.id}" + end + + assert tm.role == :owner + assert tm.team.id + + if team_id do + assert tm.team.id == team_id + end + + tm.team + end + + def assert_team_membership(user, team, role \\ :owner) do + assert membership = + Repo.get_by(Plausible.Teams.Membership, + team_id: team.id, + user_id: user.id, + role: role + ) + + membership + end + + def assert_team_attached(site, team_id \\ nil) do + assert site = %{team: team} = site |> Repo.reload!() |> Repo.preload([:team, :owner]) + + assert membership = assert_team_membership(site.owner, team) + + assert membership.team_id == team.id + + if team_id do + assert team.id == team_id + end + + team + end + + def assert_guest_invitation(team, site, email, role) do + assert team_invitation = + Repo.get_by(Plausible.Teams.Invitation, + email: email, + team_id: team.id, + role: :guest + ) + + assert Repo.get_by(Plausible.Teams.GuestInvitation, + team_invitation_id: team_invitation.id, + site_id: site.id, + role: role + ) + end + + def assert_guest_membership(team, site, user, role) do + assert team_membership = + Repo.get_by(Plausible.Teams.Membership, + user_id: user.id, + team_id: team.id, + role: :guest + ) + + assert Repo.get_by(Plausible.Teams.GuestMembership, + team_membership_id: team_membership.id, + site_id: site.id, + role: role + ) + end +end diff --git a/test/workers/check_usage_test.exs b/test/workers/check_usage_test.exs index 7779717a4..4c8a5f489 100644 --- a/test/workers/check_usage_test.exs +++ b/test/workers/check_usage_test.exs @@ -1,6 +1,7 @@ defmodule Plausible.Workers.CheckUsageTest do use Plausible.DataCase, async: true use Bamboo.Test + use Plausible.Teams.Test import Double alias Plausible.Workers.CheckUsage @@ -204,6 +205,37 @@ defmodule Plausible.Workers.CheckUsageTest do assert Repo.reload(user).grace_period.end_date == Timex.shift(Timex.today(), days: 7) end + @tag :teams + test "syncs grace period start with teams", + %{ + user: user + } do + usage_stub = + Plausible.Billing.Quota.Usage + |> stub(:monthly_pageview_usage, fn _user -> + %{ + penultimate_cycle: %{date_range: @date_range, total: 11_000}, + last_cycle: %{date_range: @date_range, total: 11_000} + } + end) + + insert(:subscription, + user: user, + paddle_plan_id: @paddle_id_10k, + last_bill_date: Timex.shift(Timex.today(), days: -1), + status: unquote(status) + ) + + CheckUsage.perform(nil, usage_stub) + + assert user = Repo.reload!(user) + assert user.grace_period.end_date == Timex.shift(Timex.today(), days: 7) + team = assert_team_exists(user) + assert team.grace_period.end_date == user.grace_period.end_date + + assert team.grace_period.id == user.grace_period.id + end + test "sends an email suggesting enterprise plan when usage is greater than 10M ", %{ user: user } do @@ -316,6 +348,42 @@ defmodule Plausible.Workers.CheckUsageTest do CheckUsage.perform(nil, usage_stub) refute user |> Repo.reload() |> Plausible.Auth.GracePeriod.active?() end + + @tag :teams + test "syncs clearing grace period with teams", %{user: user} do + usage_stub = + Plausible.Billing.Quota.Usage + |> stub(:monthly_pageview_usage, fn _user -> + %{ + penultimate_cycle: %{date_range: @date_range, total: 11_000}, + last_cycle: %{date_range: @date_range, total: 11_000} + } + end) + + insert(:subscription, + user: user, + paddle_plan_id: @paddle_id_10k, + last_bill_date: Timex.shift(Timex.today(), days: -1), + status: unquote(status) + ) + + CheckUsage.perform(nil, usage_stub) + assert user |> Repo.reload() |> Plausible.Auth.GracePeriod.active?() + team = assert_team_exists(user) + assert Plausible.Auth.GracePeriod.active?(team) + + usage_stub = + Plausible.Billing.Quota.Usage + |> stub(:monthly_pageview_usage, fn _user -> + %{ + penultimate_cycle: %{date_range: @date_range, total: 11_000}, + last_cycle: %{date_range: @date_range, total: 9_000} + } + end) + + CheckUsage.perform(nil, usage_stub) + refute team |> Repo.reload!() |> Plausible.Auth.GracePeriod.active?() + end end end @@ -435,6 +503,34 @@ defmodule Plausible.Workers.CheckUsageTest do CheckUsage.perform(nil, usage_stub) assert user |> Repo.reload() |> Plausible.Auth.GracePeriod.active?() end + + @tag :teams + test "manual lock grace period is synced with teams", %{user: user} do + usage_stub = + Plausible.Billing.Quota.Usage + |> stub(:monthly_pageview_usage, fn _user -> + %{ + penultimate_cycle: %{date_range: @date_range, total: 1_100_000}, + last_cycle: %{date_range: @date_range, total: 1_100_000} + } + end) + + enterprise_plan = insert(:enterprise_plan, user: user, monthly_pageview_limit: 1_000_000) + + insert(:subscription, + user: user, + paddle_plan_id: enterprise_plan.paddle_plan_id, + last_bill_date: Timex.shift(Timex.today(), days: -1), + status: unquote(status) + ) + + CheckUsage.perform(nil, usage_stub) + + assert user = Repo.reload!(user) + team = assert_team_exists(user) + assert team.grace_period.manual_lock == user.grace_period.manual_lock + assert team.grace_period.manual_lock == true + end end end