fix(invitations): allow re-inviting after an unaccepted invite expires (#2543)

* fix(invitations): allow re-inviting after an unaccepted invite expires

An expired, never-accepted invitation still occupies the partial unique
index `index_invitations_on_email_and_family_id_pending` (predicate
`accepted_at IS NULL`), but the `pending` scope excludes it via the
`expires_at > now` clause. The duplicate-guard validation therefore
passes and the INSERT collides with the index, raising
ActiveRecord::RecordNotUnique — surfaced to the admin as a 500 with no
way to re-invite that address through the UI.

Remove the stale expired row inside the create transaction
(before_create) so the unique slot is freed and the re-invite succeeds.
It runs only after validations pass, so a still-pending (non-expired)
invitation can never be removed here. Add a controller-level rescue of
ActiveRecord::RecordNotUnique as a safety net against a concurrent
double-submit race.

Closes #2535

* refactor(invitations): scope RecordNotUnique rescue to the save

The method-level `rescue ActiveRecord::RecordNotUnique` wrapped the whole
create action, so any future write after @invitation.save (e.g.
accept_for) would silently be reported as a generic invite failure.
Extract save_invitation to scope the rescue to the save alone, and add a
regression test that drives the raced unique-index path directly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

app/controllers/invitations_controller.rb

@@ -14,7 +14,7 @@ class InvitationsController < ApplicationController
     @invitation = Current.family.invitations.build(invitation_params)
     @invitation.inviter = Current.user
 
-    if @invitation.save
+    if save_invitation
       normalized_email = @invitation.email.to_s.strip.downcase
       existing_user = User.find_by(email: normalized_email)
       if existing_user && @invitation.would_orphan_owned_accounts?(existing_user)
@@ -67,4 +67,15 @@ class InvitationsController < ApplicationController
     def invitation_params
       params.require(:invitation).permit(:email, :role)
     end
+
+    # Persist the invitation, treating a raced partial-unique-index violation as
+    # an ordinary save failure. A concurrent double-submit can still slip between
+    # the `no_duplicate_pending_invitation_in_family` validation and the INSERT.
+    # Scoping the rescue to the save alone keeps later writes in #create (e.g.
+    # accept_for) from silently swallowing a genuine RecordNotUnique.
+    def save_invitation
+      @invitation.save
+    rescue ActiveRecord::RecordNotUnique
+      false
+    end
 end

app/models/invitation.rb

@@ -19,6 +19,7 @@ class Invitation < ApplicationRecord
 
   before_validation :normalize_email
   before_validation :generate_token, on: :create
+  before_create :remove_expired_duplicates_in_family
   before_create :set_expiration
 
   scope :pending, -> { where(accepted_at: nil).where("expires_at > ?", Time.current) }
@@ -69,6 +70,29 @@ class Invitation < ApplicationRecord
       self.expires_at = 3.days.from_now
     end
 
+    # An expired, never-accepted invitation still occupies the partial unique
+    # index `index_invitations_on_email_and_family_id_pending` (predicate
+    # `accepted_at IS NULL`), but the `pending` scope treats it as gone because
+    # of the `expires_at > now` clause. The duplicate-guard validation therefore
+    # passes, and the INSERT collides with the index, raising RecordNotUnique.
+    # Removing the stale row inside the create transaction frees the slot so the
+    # re-invite can proceed. It runs only after validations pass, so a still
+    # pending (non-expired) invitation can never be removed here.
+    def remove_expired_duplicates_in_family
+      return if email.blank?
+
+      scope = self.class.where(family_id: family_id, accepted_at: nil)
+                        .where("expires_at <= ?", Time.current)
+
+      scope = if self.class.encryption_ready?
+        scope.where(email: email)
+      else
+        scope.where("LOWER(email) = ?", email.to_s.strip.downcase)
+      end
+
+      scope.delete_all
+    end
+
     def normalize_email
       self.email = email.to_s.strip.downcase if email.present?
     end

test/controllers/invitations_controller_test.rb

@@ -134,6 +134,45 @@ class InvitationsControllerTest < ActionDispatch::IntegrationTest
     assert existing_user.ai_enabled?
   end
 
+  test "re-inviting an email whose prior invitation expired succeeds instead of 500ing" do
+    email = "expired-reinvite-ctrl@example.com"
+    expired = @admin.family.invitations.create!(email: email, role: "member", inviter: @admin)
+    expired.update_column(:expires_at, 1.day.ago)
+
+    post invitations_url, params: {
+      invitation: {
+        email: email,
+        role: "member"
+      }
+    }
+
+    assert_redirected_to settings_profile_path
+    assert_equal I18n.t("invitations.create.success"), flash[:notice]
+    assert_not Invitation.exists?(expired.id), "stale expired invitation should be replaced"
+
+    new_invitation = @admin.family.invitations.find_by(email: email)
+    assert new_invitation.present?
+    assert new_invitation.pending?
+  end
+
+  test "raced unique-index violation on save is handled as a failure, not a 500" do
+    # Simulate a concurrent double-submit that slips past the pending-invite
+    # validation and hits the partial unique index at INSERT time.
+    Invitation.any_instance.stubs(:save).raises(ActiveRecord::RecordNotUnique)
+
+    assert_no_difference("Invitation.count") do
+      post invitations_url, params: {
+        invitation: {
+          email: "raced@example.com",
+          role: "member"
+        }
+      }
+    end
+
+    assert_redirected_to settings_profile_path
+    assert_equal I18n.t("invitations.create.failure"), flash[:alert]
+  end
+
   test "should handle invalid invitation creation" do
     assert_no_difference("Invitation.count") do
       post invitations_url, params: {

test/models/invitation_test.rb

@@ -110,6 +110,32 @@ class InvitationTest < ActiveSupport::TestCase
     assert invitation.valid?
   end
 
+  test "re-inviting an email with an expired unaccepted invitation replaces it without raising" do
+    email = "expired-reinvite@example.com"
+
+    expired = @family.invitations.create!(email: email, role: "member", inviter: @inviter)
+    expired.update_column(:expires_at, 1.day.ago)
+
+    invitation = nil
+    assert_nothing_raised do
+      invitation = @family.invitations.create!(email: email, role: "admin", inviter: @inviter)
+    end
+
+    assert invitation.pending?
+    assert_not Invitation.exists?(expired.id), "stale expired invitation should be removed"
+    assert_equal 1, @family.invitations.where(email: email).count
+  end
+
+  test "re-invite does not remove a still-pending duplicate (validation blocks first)" do
+    email = "still-pending@example.com"
+    pending = @family.invitations.create!(email: email, role: "member", inviter: @inviter)
+
+    duplicate = @family.invitations.build(email: email, role: "admin", inviter: @inviter)
+    assert_not duplicate.valid?
+
+    assert Invitation.exists?(pending.id), "an unexpired pending invitation must be preserved"
+  end
+
   test "can create invitation in same family (uniqueness scoped to family)" do
     email = "same-family-test@example.com"