Segments end of life (#5095)

* Deal with segments when guest member is removed

* Handle segments when user removed from team

* Deal with user deletion

* Fix spelling

* Fix postgres client make task

* Remove migration
This commit is contained in:
Uku Taht
2025-02-26 15:54:07 +02:00
committed by GitHub
parent e252c75164
commit 6d3e6a76da
8 changed files with 236 additions and 4 deletions

View File

@ -1,4 +1,4 @@
.PHONY: help install server clickhouse clickhouse-prod clickhouse-stop postgres postgres-prod postgres-stop
.PHONY: help install server clickhouse clickhouse-prod clickhouse-stop postgres postgres-client postgres-prod postgres-stop
help:
@perl -nle'print $& if m{^[a-zA-Z_-]+:.*?## .*$$}' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}'
@ -34,6 +34,9 @@ PG_FLAGS ?= --detach -e POSTGRES_PASSWORD="postgres" -p 5432:5432 --name plausib
postgres: ## Start a container with a recent version of postgres
docker run $(PG_FLAGS) --volume=plausible_db:/var/lib/postgresql/data postgres:latest
postgres-client: ## Connect to postgres
docker exec -it plausible_db psql -U postgres -d plausible_dev
postgres-prod: ## Start a container with the same version of postgres as the one in prod
docker run $(PG_FLAGS) --volume=plausible_db_prod:/var/lib/postgresql/data postgres:15

View File

@ -84,6 +84,7 @@ defmodule Plausible.Auth do
from ep in Plausible.Billing.EnterprisePlan, where: ep.team_id == ^team.id
)
Plausible.Segments.user_removed(user)
Repo.delete!(team)
Repo.delete!(user)

View File

@ -185,6 +185,64 @@ defmodule Plausible.Segments do
:ok
end
def after_user_removed_from_site(site, user) do
Repo.delete_all(
from segment in Segment,
where: segment.site_id == ^site.id,
where: segment.owner_id == ^user.id,
where: segment.type == :personal
)
Repo.update_all(
from(segment in Segment,
where: segment.site_id == ^site.id,
where: segment.owner_id == ^user.id,
where: segment.type == :site,
update: [set: [owner_id: nil]]
),
[]
)
end
def after_user_removed_from_team(team, user) do
team_sites_q =
from(
site in Plausible.Site,
where: site.team_id == ^team.id,
where: parent_as(:segment).site_id == site.id
)
Repo.delete_all(
from segment in Segment,
as: :segment,
where: segment.owner_id == ^user.id,
where: segment.type == :personal,
where: exists(team_sites_q)
)
Repo.update_all(
from(segment in Segment,
as: :segment,
where: segment.owner_id == ^user.id,
where: segment.type == :site,
where: exists(team_sites_q),
update: [set: [owner_id: nil]]
),
[]
)
end
def user_removed(user) do
Repo.delete_all(
from segment in Segment,
as: :segment,
where: segment.owner_id == ^user.id,
where: segment.type == :personal
)
# Site segments are set to owner=null via ON DELETE SET NULL
end
def delete_one(user_id, %Plausible.Site{} = site, site_role, segment_id) do
with {:ok, segment} <- get_one(user_id, site, site_role, segment_id) do
cond do

View File

@ -138,8 +138,13 @@ defmodule Plausible.Teams.Memberships do
guest_membership =
Repo.preload(guest_membership, [:site, team_membership: [:team, :user]])
Repo.delete!(guest_membership)
prune_guests(guest_membership.team_membership.team)
{:ok, _} =
Repo.transaction(fn ->
Repo.delete!(guest_membership)
prune_guests(guest_membership.team_membership.team)
Plausible.Segments.after_user_removed_from_site(site, user)
end)
send_site_member_removed_email(guest_membership)
{:error, _} ->

View File

@ -14,7 +14,16 @@ defmodule Plausible.Teams.Memberships.Remove do
:ok <- check_can_remove_membership(current_user_role, team_membership.role),
:ok <- check_owner_can_get_removed(team, team_membership.role) do
team_membership = Repo.preload(team_membership, [:team, :user])
Repo.delete!(team_membership)
{:ok, _} =
Repo.transaction(fn ->
Repo.delete!(team_membership)
Plausible.Segments.after_user_removed_from_team(
team_membership.team,
team_membership.user
)
end)
if Keyword.get(opts, :send_email?, true) do
send_team_member_removed_email(team_membership)

View File

@ -26,6 +26,44 @@ defmodule Plausible.Teams.Memberships.RemoveTest do
)
end
test "when member is removed, associated personal segment is deleted" do
user = new_user()
site = new_site(owner: user)
team = team_of(user)
collaborator = add_member(team, role: :editor)
segment =
insert(:segment,
type: :personal,
owner: collaborator,
site: site,
name: "personal segment"
)
assert {:ok, _} = Remove.remove(team, collaborator.id, user)
refute Repo.reload(segment)
end
test "when member is removed, associated site segment will be owner-less" do
user = new_user()
site = new_site(owner: user)
team = team_of(user)
collaborator = add_member(team, role: :editor)
segment =
insert(:segment,
type: :site,
owner: collaborator,
site: site,
name: "site segment"
)
assert {:ok, _} = Remove.remove(team, collaborator.id, user)
assert Repo.reload(segment).owner_id == nil
end
test "can't remove the only owner" do
user = new_user()
_site = new_site(owner: user)

View File

@ -654,6 +654,84 @@ defmodule PlausibleWeb.AuthControllerTest do
assert Repo.reload(user)
end
test "context > team is autodeleted - personal segment is also deleted", %{
conn: conn,
user: user,
site: owner_site
} do
segment =
insert(:segment,
type: :personal,
owner: user,
site: owner_site,
name: "personal segment"
)
delete(conn, "/me")
refute Repo.reload(segment)
end
test "context > team is autodeleted - site segment is also deleted", %{
conn: conn,
user: user,
site: owner_site
} do
segment =
insert(:segment,
type: :site,
owner: user,
site: owner_site,
name: "site segment"
)
delete(conn, "/me")
refute Repo.reload(segment)
end
test "context > team is not autodeleted - personal segment is deleted", %{
conn: conn,
user: user
} do
another_owner = new_user()
another_site = new_site(owner: another_owner)
add_member(another_site.team, user: user, role: :admin)
segment =
insert(:segment,
type: :personal,
owner: user,
site: another_site,
name: "personal segment"
)
delete(conn, "/me")
refute Repo.reload(segment)
end
test "context > team is not autodeleted - site segment is kept with owner=null", %{
conn: conn,
user: user
} do
another_owner = new_user()
another_site = new_site(owner: another_owner)
add_member(another_site.team, user: user, role: :admin)
segment =
insert(:segment,
type: :site,
owner: user,
site: another_site,
name: "site segment"
)
delete(conn, "/me")
assert Repo.reload(segment).owner_id == nil
end
test "allows to delete user when not the only owner of a setup team", %{
conn: conn,
user: user

View File

@ -384,6 +384,46 @@ defmodule PlausibleWeb.Site.MembershipControllerTest do
refute Repo.exists?(from tm in Plausible.Teams.Membership, where: tm.user_id == ^admin.id)
end
test "when members is removed, associated personal segment is deleted", %{
conn: conn,
user: user
} do
site = new_site(owner: user)
admin = add_guest(site, role: :editor)
segment =
insert(:segment,
type: :personal,
owner: admin,
site: site,
name: "personal segment"
)
delete(conn, "/sites/#{site.domain}/memberships/u/#{admin.id}")
refute Repo.reload(segment)
end
test "when members is removed, associated site segment will be owner-less", %{
conn: conn,
user: user
} do
site = new_site(owner: user)
admin = add_guest(site, role: :editor)
segment =
insert(:segment,
type: :site,
owner: admin,
site: site,
name: "site segment"
)
delete(conn, "/sites/#{site.domain}/memberships/u/#{admin.id}")
assert Repo.reload(segment).owner_id == nil
end
test "fails to remove a member from a foreign site (silently)", %{conn: conn, user: user} do
foreign_member = new_user()
foreign_site = new_site()