From 5a78106fe7f216f29b03b5877bb8ec6d31605f53 Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Wed, 5 Feb 2025 10:14:45 +0100 Subject: [PATCH] Update tests --- lib/plausible/teams/management/layout.ex | 9 +++- lib/plausible_web/live/components/team.ex | 5 +- lib/plausible_web/live/team_management.ex | 48 +++++++++++-------- .../templates/settings/team_general.html.heex | 2 +- .../teams/management/layout_test.exs | 30 +++++++++++- .../live/team_management_test.exs | 39 ++++++++++++++- test/plausible_web/live/team_setup_test.exs | 30 ++++++++++++ 7 files changed, 137 insertions(+), 26 deletions(-) diff --git a/lib/plausible/teams/management/layout.ex b/lib/plausible/teams/management/layout.ex index 7011adc638c5..2f77dad917e8 100644 --- a/lib/plausible/teams/management/layout.ex +++ b/lib/plausible/teams/management/layout.ex @@ -39,11 +39,16 @@ defmodule Plausible.Teams.Management.Layout do end) end - @spec active_size(t()) :: non_neg_integer() - def active_size(layout) do + @spec active_count(t()) :: non_neg_integer() + def active_count(layout) do Enum.count(layout, fn {_, entry} -> entry.queued_op != :delete end) end + @spec owners_count(t()) :: non_neg_integer() + def owners_count(layout) do + Enum.count(layout, fn {_, entry} -> entry.queued_op != :delete and entry.role == :owner end) + end + @spec has_guests?(t()) :: boolean() def has_guests?(layout) do not is_nil( diff --git a/lib/plausible_web/live/components/team.ex b/lib/plausible_web/live/components/team.ex index e357218270cd..4c8d89616cb0 100644 --- a/lib/plausible_web/live/components/team.ex +++ b/lib/plausible_web/live/components/team.ex @@ -133,10 +133,11 @@ defmodule PlausibleWeb.Live.Components.Team do attr :role, :atom, required: true attr :disabled, :boolean, default: false attr :dispatch_animation?, :boolean, default: false - slot :inner_block, required: true attr :rest, :global attr :user, :map, default: %{email: nil} - attr(:id, :string, default: nil) + attr :id, :string, default: nil + + slot :inner_block, required: true def role_item(assigns) do click = diff --git a/lib/plausible_web/live/team_management.ex b/lib/plausible_web/live/team_management.ex index b0c6ccb35007..3e64ab9f0520 100644 --- a/lib/plausible_web/live/team_management.ex +++ b/lib/plausible_web/live/team_management.ex @@ -23,22 +23,24 @@ defmodule PlausibleWeb.Live.TeamManagement do defp reset(%{assigns: %{current_user: current_user, my_team: my_team}} = socket) do {:ok, my_role} = Teams.Memberships.team_role(my_team, current_user) - # XXX handle redirect here - true = my_role in [:owner, :admin] - - layout = Layout.init(my_team) - team_members_limit = Plausible.Teams.Billing.team_member_limit(my_team) - - assign(socket, - attempted_save?: false, - team_members_limit: team_members_limit, - guests_limit: 10, - layout: layout, - my_role: my_role, - team_layout_changed?: false, - input_role: :viewer, - input_email: "" - ) + + if my_role not in [:owner, :admin] do + redirect(socket, to: Routes.settings_path(socket, :team_general)) + else + layout = Layout.init(my_team) + team_members_limit = Plausible.Teams.Billing.team_member_limit(my_team) + + assign(socket, + attempted_save?: false, + team_members_limit: team_members_limit, + guests_limit: 10, + layout: layout, + my_role: my_role, + team_layout_changed?: false, + input_role: :viewer, + input_email: "" + ) + end end def render(assigns) do @@ -49,7 +51,7 @@ defmodule PlausibleWeb.Live.TeamManagement do :if={ (not @team_layout_changed? or @attempted_save?) and not Plausible.Billing.Quota.below_limit?( - Layout.active_size(@layout) - 1, + Layout.active_count(@layout) - 1, @team_members_limit ) } @@ -129,6 +131,7 @@ defmodule PlausibleWeb.Live.TeamManagement do label={entry_label(entry, @current_user)} my_role={@my_role} remove_disabled={not Layout.removable?(@layout, email)} + disabled={entry.role == :owner && Layout.owners_count(@layout) == 1} /> @@ -298,6 +301,13 @@ defmodule PlausibleWeb.Live.TeamManagement do |> reset() |> put_live_flash(:success, "Team layout updated successfully") + {{:error, :only_one_owner}, _} -> + socket + |> put_live_flash( + :error, + "The team has to have at least one owner" + ) + {{:error, {:over_limit, limit}}, _} -> socket |> put_live_flash( @@ -305,9 +315,9 @@ defmodule PlausibleWeb.Live.TeamManagement do "Your account is limited to #{limit} team members. You can upgrade your plan to increase this limit" ) - {{:error, error}, _} -> + {{:error, :permission_denied}, _} -> socket - |> put_live_flash(:error, inspect(error)) + |> put_live_flash(:error, "This operation is not supported") end end diff --git a/lib/plausible_web/templates/settings/team_general.html.heex b/lib/plausible_web/templates/settings/team_general.html.heex index f2f3fdb0a0ea..eb5e4c742697 100644 --- a/lib/plausible_web/templates/settings/team_general.html.heex +++ b/lib/plausible_web/templates/settings/team_general.html.heex @@ -24,7 +24,7 @@ Team Members <:subtitle> - Add, remove or change your team memberships. Take your time, changes apply on save. + Add, remove or change your team memberships {live_render(@conn, PlausibleWeb.Live.TeamManagement, id: "team-setup", diff --git a/test/plausible/teams/management/layout_test.exs b/test/plausible/teams/management/layout_test.exs index cec5e57dc2fd..5ab383519ca9 100644 --- a/test/plausible/teams/management/layout_test.exs +++ b/test/plausible/teams/management/layout_test.exs @@ -71,7 +71,7 @@ defmodule Plausible.Teams.Management.LayoutTest do ] = Layout.sorted_for_display(layout) end - test "removable?/2" do + test "removable?/2 + counters" do layout = sample_layout() assert Layout.removable?(layout, "invitation-pending@example.com") assert Layout.removable?(layout, "current@example.com") @@ -97,8 +97,14 @@ defmodule Plausible.Teams.Management.LayoutTest do assert Layout.removable?(layout, "owner@example.com") assert Layout.removable?(layout, "secondary-owner@example.com") + assert Layout.owners_count(layout) == 3 + assert Layout.active_count(layout) == 9 + layout = Layout.schedule_delete(layout, "owner@example.com") + assert Layout.owners_count(layout) == 2 + assert Layout.active_count(layout) == 8 + refute Layout.removable?(layout, "secondary-owner@example.com") end @@ -312,6 +318,28 @@ defmodule Plausible.Teams.Management.LayoutTest do |> Layout.schedule_send("test4@example.com", :admin) |> Layout.persist(%{current_user: user, my_team: team}) + assert {:error, :only_one_owner} = + team + |> Layout.init() + |> Layout.schedule_delete(user.email) + |> Layout.put( + invitation_pending("00-invitation-pending@example.com", role: :owner) + ) + |> Layout.put(invitation_sent("00-invitation-sent@example.com", role: :owner)) + |> Layout.persist(%{current_user: user, my_team: team}) + + assert {:error, :only_one_owner} = + team + |> Layout.init() + |> Layout.update_role(user.email, :viewer) + |> Layout.persist(%{current_user: user, my_team: team}) + + assert {:error, :already_a_member} = + team + |> Layout.init() + |> Layout.schedule_send(user.email, :admin) + |> Layout.persist(%{current_user: user, my_team: team}) + assert_no_emails_delivered() end diff --git a/test/plausible_web/live/team_management_test.exs b/test/plausible_web/live/team_management_test.exs index da9ec2eb3b58..9d53e5b2bb49 100644 --- a/test/plausible_web/live/team_management_test.exs +++ b/test/plausible_web/live/team_management_test.exs @@ -19,7 +19,7 @@ defmodule PlausibleWeb.Live.TeamMangementTest do |> html_response(200) |> text() - assert resp =~ "Add, remove or change your team memberships." + assert resp =~ "Add, remove or change your team memberships" refute element_exists?(resp, ~s|button[phx-click="save-team-layout"]|) end @@ -203,6 +203,43 @@ defmodule PlausibleWeb.Live.TeamMangementTest do end end + describe "to be revisited" do + setup [:create_user, :log_in, :create_team, :setup_team] + + test "guest->owner promotion is currently not supported by the underlying services", + %{ + conn: conn, + user: user + } do + site = new_site(owner: user) + add_guest(site, role: :viewer, user: new_user(name: "Mr Guest", email: "guest@example.com")) + + lv = get_liveview(conn) + + change_role(lv, 1, "owner", "#guest-list .guest") + html = render(lv) + + assert html =~ "This operation is not supported" + end + + @tag :capture_log + test "billing role is currently not supported by the underlying servies", + %{ + conn: conn, + team: team + } do + Process.flag(:trap_exit, true) + _member2 = add_member(team, role: :admin) + + lv = get_liveview(conn) + + assert :unsupported == change_role(lv, 2, "billing") + catch + _, _ -> + :unsupported + end + end + defp change_role(lv, index, role, main_selector \\ "#member-list .member") do lv |> element(~s|#{main_selector}:nth-of-type(#{index}) a[phx-value-role="#{role}"]|) diff --git a/test/plausible_web/live/team_setup_test.exs b/test/plausible_web/live/team_setup_test.exs index e4d4cd31ee44..e9aa153371e6 100644 --- a/test/plausible_web/live/team_setup_test.exs +++ b/test/plausible_web/live/team_setup_test.exs @@ -163,6 +163,36 @@ defmodule PlausibleWeb.Live.TeamSetupTest do assert lv |> render() |> text() =~ "Your account is limited to 3 team members" end + test "all options are disabled for the sole owner", %{conn: conn} do + lv = get_child_lv(conn) + + options = + lv + |> render() + |> find(~s|#member-list .member a|) + + assert Enum.empty?(options) + end + + test "in case of >1 owner, the one owner limit is still enforced", %{conn: conn, team: team} do + _other_owner = add_member(team, role: :owner) + lv = get_child_lv(conn) + + options = + lv + |> render() + |> find(~s|#member-list .member a|) + + refute Enum.empty?(options) + + change_role(lv, 1, "viewer") + + html = lv |> render() + + assert [_ | _] = find(html, "#member-list .member:nth-of-type(1) a") + assert find(html, "#member-list .member:nth-of-type(2) a") == [] + end + test "allows removing any type of entry", %{ conn: conn, user: user,