Skip to content

Commit

Permalink
Rework team creation constraints (#5003)
Browse files Browse the repository at this point in the history
* Move `GracePeriod` under `Teams` and clean it up a bit

* Switch to relying on new team membership constraint when creating my team

* Remove unused unique_constraint from Teams.Membership schema changeset
  • Loading branch information
zoldar authored Jan 29, 2025
1 parent 57e942a commit 339dd89
Show file tree
Hide file tree
Showing 10 changed files with 69 additions and 61 deletions.
32 changes: 20 additions & 12 deletions lib/plausible/teams.ex
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ defmodule Plausible.Teams do
import Ecto.Query

alias __MODULE__
alias Plausible.Auth.GracePeriod
alias Plausible.Auth
alias Plausible.Repo
use Plausible

Expand All @@ -26,7 +26,7 @@ defmodule Plausible.Teams do
end

@spec get_owner(Teams.Team.t()) ::
{:ok, Plausible.Auth.User.t()} | {:error, :no_owner | :multiple_owners}
{:ok, Auth.User.t()} | {:error, :no_owner | :multiple_owners}
def get_owner(team) do
case Repo.preload(team, :owner).owner do
nil -> {:error, :no_owner}
Expand Down Expand Up @@ -143,6 +143,7 @@ defmodule Plausible.Teams do
If the user has a non-guest membership other than owner, `:no_team` error
is returned.
"""
@spec get_or_create(Auth.User.t()) :: {:ok, Teams.Team.t()} | {:error, :multiple_teams}
def get_or_create(user) do
with {:error, :no_team} <- get_by_owner(user) do
case create_my_team(user) do
Expand All @@ -155,6 +156,8 @@ defmodule Plausible.Teams do
end
end

@spec get_by_owner(Auth.User.t() | pos_integer()) ::
{:ok, Teams.Team.t()} | {:error, :no_team | :multiple_teams}
def get_by_owner(user_id) when is_integer(user_id) do
result =
from(tm in Teams.Membership,
Expand All @@ -163,18 +166,21 @@ defmodule Plausible.Teams do
select: t,
order_by: t.id
)
|> Repo.one()
|> Repo.all()

case result do
nil ->
[] ->
{:error, :no_team}

team ->
[team] ->
{:ok, team}

_teams ->
{:error, :multiple_teams}
end
end

def get_by_owner(%Plausible.Auth.User{} = user) do
def get_by_owner(%Auth.User{} = user) do
get_by_owner(user.id)
end

Expand All @@ -193,25 +199,25 @@ defmodule Plausible.Teams do

def start_grace_period(team) do
team
|> GracePeriod.start_changeset()
|> Teams.GracePeriod.start_changeset()
|> Repo.update!()
end

def start_manual_lock_grace_period(team) do
team
|> GracePeriod.start_manual_lock_changeset()
|> Teams.GracePeriod.start_manual_lock_changeset()
|> Repo.update!()
end

def end_grace_period(team) do
team
|> GracePeriod.end_changeset()
|> Teams.GracePeriod.end_changeset()
|> Repo.update!()
end

def remove_grace_period(team) do
team
|> GracePeriod.remove_changeset()
|> Teams.GracePeriod.remove_changeset()
|> Repo.update!()
end

Expand Down Expand Up @@ -289,7 +295,7 @@ defmodule Plausible.Teams do
case result do
{:ok, invitations} ->
Enum.each(invitations, fn invitation ->
invitee = Plausible.Auth.find_user_by(email: invitation.email)
invitee = Auth.find_user_by(email: invitation.email)
Teams.Invitations.InviteToTeam.send_invitation_email(invitation, invitee)
end)

Expand All @@ -311,11 +317,13 @@ defmodule Plausible.Teams do
team_membership =
team
|> Teams.Membership.changeset(user, :owner)
|> Ecto.Changeset.put_change(:is_autocreated, true)
|> Ecto.Changeset.put_change(:inserted_at, user.inserted_at)
|> Ecto.Changeset.put_change(:updated_at, user.updated_at)
|> Repo.insert!(
on_conflict: :nothing,
conflict_target: {:unsafe_fragment, "(user_id) WHERE role != 'guest'"}
conflict_target:
{:unsafe_fragment, "(user_id) WHERE role = 'owner' and is_autocreated = true"}
)

if team_membership.id do
Expand Down
2 changes: 1 addition & 1 deletion lib/plausible/teams/billing.ex
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ defmodule Plausible.Teams.Billing do
trial_over? and not subscription_active? ->
{:needs_to_upgrade, :no_active_subscription}

Plausible.Auth.GracePeriod.expired?(team) ->
Teams.GracePeriod.expired?(team) ->
{:needs_to_upgrade, :grace_period_ended}

true ->
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
defmodule Plausible.Auth.GracePeriod do
defmodule Plausible.Teams.GracePeriod do
@moduledoc """
This embedded schema stores information about the account locking grace
period.
Users are given this 7-day grace period to upgrade their account after
Teams are given this 7-day grace period to upgrade their account after
outgrowing their subscriptions. The actual account locking happens in
background with `Plausible.Workers.LockSites`.
Expand All @@ -29,7 +29,7 @@ defmodule Plausible.Auth.GracePeriod do

@spec start_changeset(Teams.Team.t()) :: Ecto.Changeset.t()
@doc """
Starts a account locking grace period of 7 days by changing the User struct.
Starts a account locking grace period of 7 days by changing the Team struct.
"""
def start_changeset(%Teams.Team{} = team) do
grace_period = %__MODULE__{
Expand All @@ -43,7 +43,7 @@ defmodule Plausible.Auth.GracePeriod do

@spec start_manual_lock_changeset(Teams.Team.t()) :: Ecto.Changeset.t()
@doc """
Starts a manual account locking grace period by changing the User struct.
Starts a manual account locking grace period by changing the Team struct.
Manual locking means the grace period can only be removed manually from the
CRM.
"""
Expand All @@ -59,7 +59,7 @@ defmodule Plausible.Auth.GracePeriod do

@spec end_changeset(Teams.Team.t()) :: Ecto.Changeset.t()
@doc """
Ends an existing grace period by `setting users.grace_period.is_over` to true.
Ends an existing grace period by setting `teams.grace_period.is_over` to true.
This means the grace period has expired.
"""
def end_changeset(%Teams.Team{} = team) do
Expand All @@ -68,16 +68,16 @@ defmodule Plausible.Auth.GracePeriod do

@spec remove_changeset(Teams.Team.t()) :: Ecto.Changeset.t()
@doc """
Removes the grace period from the User completely.
Removes the grace period from the Team completely.
"""
def remove_changeset(%Teams.Team{} = team) do
Ecto.Changeset.change(team, grace_period: nil)
end

@spec active?(Teams.Team.t() | nil) :: 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.
Returns whether the grace period is still active for a Team. Defaults to
false if the team is nil or there is no grace period.
"""
def active?(team)

Expand All @@ -93,8 +93,8 @@ defmodule Plausible.Auth.GracePeriod do

@spec expired?(Teams.Team.t() | nil) :: boolean()
@doc """
Returns whether the grace period has already expired for a User. Defaults to
false if the user is nil or there is no grace period.
Returns whether the grace period has already expired for a Team. Defaults to
false if the team is nil or there is no grace period.
"""
def expired?(team) do
if team && team.grace_period, do: !active?(team), else: false
Expand Down
2 changes: 1 addition & 1 deletion lib/plausible/teams/membership.ex
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ defmodule Plausible.Teams.Membership do

schema "team_memberships" do
field :role, Ecto.Enum, values: @roles
field :is_autocreated, :boolean, default: false

belongs_to :user, Plausible.Auth.User
belongs_to :team, Plausible.Teams.Team
Expand All @@ -30,6 +31,5 @@ defmodule Plausible.Teams.Membership do
|> put_change(:role, role)
|> put_assoc(:team, team)
|> put_assoc(:user, user)
|> unique_constraint(:user_id, name: :one_team_per_user)
end
end
2 changes: 1 addition & 1 deletion lib/plausible/teams/team.ex
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ defmodule Plausible.Teams.Team do
field :setup_complete, :boolean, default: false
field :setup_at, :naive_datetime

embeds_one :grace_period, Plausible.Auth.GracePeriod, on_replace: :update
embeds_one :grace_period, Plausible.Teams.GracePeriod, on_replace: :update

has_many :sites, Plausible.Site
has_many :team_memberships, Plausible.Teams.Membership
Expand Down
4 changes: 2 additions & 2 deletions lib/plausible_web/templates/layout/_notice.html.heex
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@

<div :if={assigns[:my_team]} class="flex flex-col gap-y-2">
<Notice.active_grace_period
:if={Plausible.Auth.GracePeriod.active?(@my_team)}
:if={Plausible.Teams.GracePeriod.active?(@my_team)}
enterprise?={Plausible.Teams.Billing.enterprise_configured?(@my_team)}
grace_period_end={grace_period_end(@my_team)}
/>

<Notice.dashboard_locked :if={Plausible.Auth.GracePeriod.expired?(@my_team)} />
<Notice.dashboard_locked :if={Plausible.Teams.GracePeriod.expired?(@my_team)} />

<Notice.subscription_cancelled subscription={@my_team.subscription} />

Expand Down
4 changes: 2 additions & 2 deletions test/plausible/billing/billing_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -412,8 +412,8 @@ defmodule Plausible.BillingTest do
assert Repo.reload!(api_key).hourly_request_limit == 10_000
end

test "if user's grace period has ended, upgrading will unlock sites and remove grace period" do
grace_period = %Plausible.Auth.GracePeriod{end_date: Timex.shift(Timex.today(), days: -1)}
test "if teams's grace period has ended, upgrading will unlock sites and remove grace period" do
grace_period = %Plausible.Teams.GracePeriod{end_date: Timex.shift(Timex.today(), days: -1)}
user = new_user(team: [grace_period: grace_period])

subscribe_to_growth_plan(user)
Expand Down
12 changes: 6 additions & 6 deletions test/plausible/billing/site_locker_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ defmodule Plausible.Billing.SiteLockerTest do
refute Repo.reload!(site).locked
end

test "does not lock user who has an active subscription and is on grace period" do
grace_period = %Plausible.Auth.GracePeriod{end_date: Timex.shift(Timex.today(), days: 1)}
test "does not lock team which has an active subscription and is on grace period" do
grace_period = %Plausible.Teams.GracePeriod{end_date: Timex.shift(Timex.today(), days: 1)}

user =
new_user(team: [grace_period: grace_period])
Expand Down Expand Up @@ -77,8 +77,8 @@ defmodule Plausible.Billing.SiteLockerTest do
assert Repo.reload!(site).locked
end

test "locks all sites if user has active subscription but grace period has ended" do
grace_period = %Plausible.Auth.GracePeriod{end_date: Timex.shift(Timex.today(), days: -1)}
test "locks all sites if team has active subscription but grace period has ended" do
grace_period = %Plausible.Teams.GracePeriod{end_date: Timex.shift(Timex.today(), days: -1)}
user = new_user(team: [grace_period: grace_period])
subscribe_to_plan(user, "123")
site = new_site(owner: user)
Expand All @@ -90,7 +90,7 @@ defmodule Plausible.Billing.SiteLockerTest do
end

test "sends email if grace period has ended" do
grace_period = %Plausible.Auth.GracePeriod{end_date: Timex.shift(Timex.today(), days: -1)}
grace_period = %Plausible.Teams.GracePeriod{end_date: Timex.shift(Timex.today(), days: -1)}
user = new_user(team: [grace_period: grace_period])
subscribe_to_plan(user, "123")
new_site(owner: user)
Expand All @@ -105,7 +105,7 @@ defmodule Plausible.Billing.SiteLockerTest do
end

test "does not send grace period email if site is already locked" do
grace_period = %Plausible.Auth.GracePeriod{
grace_period = %Plausible.Teams.GracePeriod{
end_date: Timex.shift(Timex.today(), days: -1),
is_over: false
}
Expand Down
Loading

0 comments on commit 339dd89

Please sign in to comment.