Handle duplicate invitations gracefully (#2323)

lib/plausible/auth/invitation.ex

@@ -19,5 +19,10 @@ defmodule Plausible.Auth.Invitation do
     %__MODULE__{invitation_id: Nanoid.generate()}
     |> cast(attrs, @required)
     |> validate_required(@required)
+    |> unique_constraint([:email, :site_id],
+      name: :invitations_site_id_email_index,
+      error_key: :invitation,
+      message: "already sent"
+    )
   end
 end

lib/plausible_web/controllers/site/membership_controller.ex

@@ -50,31 +50,50 @@ defmodule PlausibleWeb.Site.MembershipController do
         skip_plausible_tracking: true
       )
     else
-      invitation =
-        Invitation.new(%{
-          email: email,
-          role: role,
-          site_id: site.id,
-          inviter_id: conn.assigns[:current_user].id
-        })
-        |> Repo.insert!()
-        |> Repo.preload([:site, :inviter])
+      case Repo.insert(
+             Invitation.new(%{
+               email: email,
+               role: role,
+               site_id: site.id,
+               inviter_id: conn.assigns[:current_user].id
+             })
+           ) do
+        {:ok, invitation} ->
+          invitation = Repo.preload(invitation, [:site, :inviter])
 
-      email_template =
-        if user do
-          PlausibleWeb.Email.existing_user_invitation(invitation)
-        else
-          PlausibleWeb.Email.new_user_invitation(invitation)
-        end
+          email_template =
+            if user do
+              PlausibleWeb.Email.existing_user_invitation(invitation)
+            else
+              PlausibleWeb.Email.new_user_invitation(invitation)
+            end
 
-      Plausible.Mailer.send_email(email_template)
+          Plausible.Mailer.send_email(email_template)
 
-      conn
-      |> put_flash(
-        :success,
-        "#{email} has been invited to #{site_domain} as #{PlausibleWeb.SiteView.with_indefinite_article(role)}"
-      )
-      |> redirect(to: Routes.site_path(conn, :settings_people, site.domain))
+          conn
+          |> put_flash(
+            :success,
+            "#{email} has been invited to #{site_domain} as #{PlausibleWeb.SiteView.with_indefinite_article(role)}"
+          )
+          |> redirect(to: Routes.site_path(conn, :settings_people, site.domain))
+
+        {:error, changeset} ->
+          error_msg =
+            case changeset.errors[:invitation] do
+              {"already sent", _} ->
+                "This invitation has been already sent. To send again, remove it from pending invitations first."
+
+              _ ->
+                "Something went wrong."
+            end
+
+          conn
+          |> put_flash(
+            :error,
+            error_msg
+          )
+          |> redirect(to: Routes.site_path(conn, :settings_people, site.domain))
+      end
     end
   end
 

test/plausible_web/controllers/site/membership_controller_test.exs

@@ -74,6 +74,46 @@ defmodule PlausibleWeb.Site.MembershipControllerTest do
       assert html_response(conn, 200) =~
                "#{second_member.email} is already a member of #{site.domain}"
     end
+
+    test "redirects with an error flash when the invitation already exists", %{
+      conn: conn,
+      user: user
+    } do
+      site = insert(:site, members: [user])
+
+      _req1 =
+        post(conn, "/sites/#{site.domain}/memberships/invite", %{
+          email: "joe@example.com",
+          role: "admin"
+        })
+
+      assert_email_delivered_with(
+        to: [nil: "joe@example.com"],
+        subject: "[Plausible Analytics] You've been invited to #{site.domain}"
+      )
+
+      req2 =
+        post(conn, "/sites/#{site.domain}/memberships/invite", %{
+          email: "joe@example.com",
+          role: "admin"
+        })
+
+      refute_email_delivered_with(
+        to: [nil: "joe@example.com"],
+        subject: "[Plausible Analytics] You've been invited to #{site.domain}"
+      )
+
+      assert people_settings = redirected_to(req2, 302)
+
+      assert ^people_settings =
+               PlausibleWeb.Router.Helpers.site_path(
+                 PlausibleWeb.Endpoint,
+                 :settings_people,
+                 site.domain
+               )
+
+      assert get_flash(req2, :error) =~ "This invitation has been already sent."
+    end
   end
 
   describe "GET /sites/:website/transfer-ownership" do