Allow inviting users who are members already (#3797)

* Allow e-mail exclusion in team members quota

* Exclude invitee from quota on invitation create

* Enable invitation submission but report errors on quota violation

* Use a single interface for team members quota

* Check the `Keyword.validate/2` result

* Update test/plausible_web/controllers/site/membership_controller_test.exs

Co-authored-by: Uku Taht <Uku.taht@gmail.com>

---------

Co-authored-by: Uku Taht <Uku.taht@gmail.com>

lib/plausible/billing/quota.ex

@@ -266,15 +266,28 @@ defmodule Plausible.Billing.Quota do
 
   * Users are counted uniquely - i.e. even if an account is associated with
     many sites owned by the given user, they still count as one team member.
+
+  * Specific e-mails can be excluded from the count, so that where necessary,
+    we can ensure inviting the same person(s) to more than 1 sites is allowed
   """
-  def team_member_usage(user) do
-    Plausible.Repo.aggregate(team_member_usage_query(user), :count)
+  def team_member_usage(user, opts \\ []) do
+    {:ok, opts} = Keyword.validate(opts, site: nil, exclude_emails: [])
+
+    user
+    |> team_member_usage_query(opts)
+    |> Plausible.Repo.aggregate(:count)
   end
 
-  @doc false
-  def team_member_usage_query(user, site \\ nil) do
+  defp team_member_usage_query(user, opts) do
     owned_sites_query = owned_sites_query(user)
 
+    excluded_emails =
+      opts
+      |> Keyword.get(:exclude_emails, [])
+      |> List.wrap()
+
+    site = opts[:site]
+
     owned_sites_query =
       if site do
         where(owned_sites_query, [os], os.site_id == ^site.id)
@@ -290,12 +303,27 @@ defmodule Plausible.Billing.Quota do
         where: sm.role != :owner,
         select: u.email
 
-    from i in Plausible.Auth.Invitation,
-      inner_join: os in subquery(owned_sites_query),
-      on: i.site_id == os.site_id,
-      where: i.role != :owner,
-      select: i.email,
-      union: ^team_members_query
+    team_members_query =
+      if excluded_emails != [] do
+        team_members_query |> where([..., u], u.email not in ^excluded_emails)
+      else
+        team_members_query
+      end
+
+    query =
+      from i in Plausible.Auth.Invitation,
+        inner_join: os in subquery(owned_sites_query),
+        on: i.site_id == os.site_id,
+        where: i.role != :owner,
+        select: i.email,
+        union: ^team_members_query
+
+    if excluded_emails != [] do
+      query
+      |> where([i], i.email not in ^excluded_emails)
+    else
+      query
+    end
   end
 
   @spec features_usage(User.t() | Site.t()) :: [atom()]

lib/plausible/site/memberships/create_invitation.ex

@@ -73,7 +73,7 @@ defmodule Plausible.Site.Memberships.CreateInvitation do
 
     with site <- Plausible.Repo.preload(site, :owner),
          :ok <- check_invitation_permissions(site, inviter, role, opts),
-         :ok <- check_team_member_limit(site, role),
+         :ok <- check_team_member_limit(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),
@@ -128,14 +128,14 @@ defmodule Plausible.Site.Memberships.CreateInvitation do
     end
   end
 
-  defp check_team_member_limit(_site, :owner) do
+  defp check_team_member_limit(_site, :owner, _invitee_email) do
     :ok
   end
 
-  defp check_team_member_limit(site, _role) do
+  defp check_team_member_limit(site, _role, invitee_email) do
     site = Plausible.Repo.preload(site, :owner)
     limit = Quota.team_member_limit(site.owner)
-    usage = Quota.team_member_usage(site.owner)
+    usage = Quota.team_member_usage(site.owner, exclude_emails: invitee_email)
 
     if Quota.below_limit?(usage, limit),
       do: :ok,

lib/plausible/site/memberships/invitations.ex

@@ -89,7 +89,7 @@ defmodule Plausible.Site.Memberships.Invitations do
 
     defp team_member_usage_after_transfer(site, new_owner) do
       current_usage = Quota.team_member_usage(new_owner)
-      site_usage = Repo.aggregate(Quota.team_member_usage_query(site.owner, site), :count)
+      site_usage = Quota.team_member_usage(site.owner, site: site)
 
       extra_usage =
         if Plausible.Sites.is_member?(new_owner.id, site), do: 0, else: 1

lib/plausible_web/controllers/site/membership_controller.ex

@@ -46,7 +46,10 @@ defmodule PlausibleWeb.Site.MembershipController do
 
   def invite_member(conn, %{"email" => email, "role" => role}) do
     site_domain = conn.assigns[:site].domain
-    site = Sites.get_for_user!(conn.assigns[:current_user].id, site_domain)
+
+    site =
+      Sites.get_for_user!(conn.assigns[:current_user].id, site_domain)
+      |> Plausible.Repo.preload(:owner)
 
     case Memberships.create_invitation(site, conn.assigns.current_user, email, role) do
       {:ok, invitation} ->
@@ -71,7 +74,9 @@ defmodule PlausibleWeb.Site.MembershipController do
             "Your account is limited to #{limit} team members. You can upgrade your plan to increase this limit.",
           site: site,
           layout: {PlausibleWeb.LayoutView, "focus.html"},
-          skip_plausible_tracking: true
+          skip_plausible_tracking: true,
+          is_at_limit: true,
+          team_member_limit: limit
         )
 
       {:error, %Ecto.Changeset{} = changeset} ->

lib/plausible_web/templates/site/membership/invite_member_form.html.heex

@@ -17,10 +17,6 @@
     The invitation will expire in 48 hours
   </p>
 
-  <%= if @conn.assigns[:error] do %>
-    <div class="text-red-500 text-xs italic mt-4"><%= @conn.assigns[:error] %></div>
-  <% end %>
-
   <div class="my-6">
     <%= label(f, :email, "Email address",
       class: "block text-sm font-medium text-gray-700 dark:text-gray-300"
@@ -45,6 +41,10 @@
       ) %>
     </div>
     <%= error_tag(f, :email) %>
+
+    <%= if @conn.assigns[:error] do %>
+      <div class="text-red-500 text-xs mb-4"><%= @conn.assigns[:error] %></div>
+    <% end %>
   </div>
 
   <fieldset x-data="{selectedOption: null}">
@@ -105,7 +105,7 @@
   </fieldset>
 
   <div class="mt-6">
-    <%= submit(class: "button w-full disabled:cursor-not-allowed", disabled: Map.get(assigns, :is_at_limit, false)) do %>
+    <%= submit(class: "button w-full") do %>
       <svg
         class="w-5 h-5 mr-1"
         fill="currentColor"

test/plausible/billing/quota_test.exs

@@ -360,6 +360,29 @@ defmodule Plausible.Billing.QuotaTest do
 
       assert Quota.team_member_usage(me) == 0
     end
+
+    test "excludes specific emails from limit calculation" do
+      me = insert(:user)
+      member = insert(:user)
+
+      site_i_own =
+        insert(:site,
+          memberships: [
+            build(:site_membership, user: me, role: :owner),
+            build(:site_membership, user: member, role: :admin)
+          ]
+        )
+
+      insert(:invitation, site: site_i_own, inviter: me)
+      insert(:invitation, site: site_i_own, inviter: member)
+      invitation = insert(:invitation, site: site_i_own, inviter: me, email: "foo@example.com")
+
+      assert Quota.team_member_usage(me) == 4
+      assert Quota.team_member_usage(me, exclude_emails: "arbitrary@example.com") == 4
+      assert Quota.team_member_usage(me, exclude_emails: member.email) == 3
+      assert Quota.team_member_usage(me, exclude_emails: invitation.email) == 3
+      assert Quota.team_member_usage(me, exclude_emails: [member.email, invitation.email]) == 2
+    end
   end
 
   describe "team_member_limit/1" do

test/plausible/site/memberships/create_invitation_test.exs

@@ -82,6 +82,58 @@ defmodule Plausible.Site.Memberships.CreateInvitationTest do
                CreateInvitation.create_invitation(site, inviter, invitee.email, :viewer)
     end
 
+    @tag :full_build_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)
+
+      invite = fn site, email ->
+        CreateInvitation.create_invitation(site, owner, email, :viewer)
+      end
+
+      assert {:ok, _} = invite.(site, "i1@example.com")
+      assert {:ok, _} = invite.(site, "i2@example.com")
+      assert {:ok, _} = invite.(site, "i3@example.com")
+      assert {:error, {:over_limit, 3}} = invite.(site, "i4@example.com")
+
+      site2 = insert(:site, memberships: memberships)
+
+      assert {:ok, _} = invite.(site2, "i3@example.com")
+    end
+
+    @tag :full_build_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)
+
+      invite = fn site, email ->
+        CreateInvitation.create_invitation(site, u1, email, :viewer)
+      end
+
+      assert {:error, {:over_limit, 3}} = invite.(site, "another@example.com")
+      assert {:error, :already_a_member} = invite.(site, u4.email)
+      assert {:ok, _} = invite.(site2, u4.email)
+    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)])

test/plausible_web/controllers/site/membership_controller_test.exs

@@ -22,7 +22,7 @@ defmodule PlausibleWeb.Site.MembershipControllerTest do
     end
 
     @tag :full_build_only
-    test "disables invite form when is over limit", %{conn: conn, user: user} do
+    test "display a notice when is over limit", %{conn: conn, user: user} do
       memberships = [
         build(:site_membership, user: user, role: :owner) | build_list(5, :site_membership)
       ]
@@ -34,7 +34,7 @@ defmodule PlausibleWeb.Site.MembershipControllerTest do
         |> get("/sites/#{site.domain}/memberships/invite")
         |> html_response(200)
 
-      assert element_exists?(html, ~s/button[type=submit][disabled]/)
+      assert html =~ "Your account is limited to 3 team members"
     end
   end