fix(webapp): report skipped invites instead of failing, and heal a member with no role

The invite form crashed when every address in a batch was skipped, and
reported the submitted count rather than the created one. It now names what
it skipped and why.

An invitation still leaves an established role alone, but an existing member
with no role assigned gets the invitation role, matching how ensureOrgMember
completes an interrupted assignment.

The invites API reports skipped members separately from skipped pending
invites.

Author: matt-aitken

.server-changes/fix-invite-role-change-for-existing-members.md

@@ -3,4 +3,4 @@ area: webapp
 type: fix
 ---
 
-Accepting an old invitation could change the role of someone who was already in the organization. An invitation now leaves an existing member's role untouched, and people who are already in an organization are no longer sent invitations to it.
+Accepting an old invitation could change the role of someone who was already in the organization. An invitation now leaves an existing member's role untouched, people who are already in an organization are no longer sent invitations to it, and the invite form now says which addresses it skipped instead of failing with an unhelpful error.

apps/webapp/app/models/member.server.ts

@@ -119,9 +119,12 @@ export async function inviteMembers({
   const created: Prisma.OrgMemberInviteGetPayload<{
     include: { organization: true; inviter: true };
   }>[] = [];
+  const alreadyMembers: string[] = [];
+  const alreadyInvited: string[] = [];
 
   for (const email of uniqueEmails) {
     if (existingMemberEmails.has(email)) {
+      alreadyMembers.push(email);
       continue;
     }
 
@@ -146,13 +149,14 @@ export async function inviteMembers({
         error instanceof PrismaNamespace.PrismaClientKnownRequestError &&
         error.code === "P2002"
       ) {
+        alreadyInvited.push(email);
         continue;
       }
       throw error;
     }
   }
 
-  return created;
+  return { created, alreadyMembers, alreadyInvited };
 }
 
 export async function getInviteFromToken({ token }: { token: string }) {
@@ -279,12 +283,21 @@ async function assignInviteRbacRole({
   userId,
   organizationId,
   rbacRoleId,
+  onlyWhenUnassigned,
 }: {
   userId: string;
   organizationId: string;
   rbacRoleId: string;
+  onlyWhenUnassigned: boolean;
 }) {
   try {
+    if (onlyWhenUnassigned) {
+      const currentRole = await rbac.getUserRole({ userId, organizationId });
+      if (currentRole !== null) {
+        return;
+      }
+    }
+
     const roleResult = await rbac.setUserRole({
       userId,
       organizationId,
@@ -499,11 +512,12 @@ export async function acceptInvite({
 
   const remainingInvites = await getUsersInvites({ email: user.email });
 
-  if (invite.rbacRoleId && membershipCreated) {
+  if (invite.rbacRoleId) {
     await assignInviteRbacRole({
       userId: user.id,
       organizationId: invite.organization.id,
       rbacRoleId: invite.rbacRoleId,
+      onlyWhenUnassigned: !membershipCreated,
     });
   }
 

apps/webapp/app/routes/_app.orgs.$organizationSlug.invite/route.tsx

@@ -28,7 +28,7 @@ import { $replica } from "~/db.server";
 import { env } from "~/env.server";
 import { useOrganization } from "~/hooks/useOrganizations";
 import { inviteMembers } from "~/models/member.server";
-import { redirectWithSuccessMessage } from "~/models/message.server";
+import { redirectWithErrorMessage, redirectWithSuccessMessage } from "~/models/message.server";
 import { resolveOrgIdFromSlug } from "~/models/organization.server";
 import { TeamPresenter } from "~/presenters/TeamPresenter.server";
 import { scheduleEmail } from "~/services/scheduleEmail.server";
@@ -127,6 +127,20 @@ const schema = z.object({
   rbacRoleId: z.string().optional(),
 });
 
+function describeSkippedInvites(alreadyMembers: string[], alreadyInvited: string[]) {
+  const parts: string[] = [];
+
+  if (alreadyMembers.length > 0) {
+    parts.push(simplur`${alreadyMembers.length} already [a member|members] of this organization`);
+  }
+
+  if (alreadyInvited.length > 0) {
+    parts.push(simplur`${alreadyInvited.length} already invited`);
+  }
+
+  return parts.join(" and ");
+}
+
 export const action = dashboardAction(
   {
     params: Params,
@@ -201,7 +215,11 @@ export const action = dashboardAction(
     }
 
     try {
-      const invites = await inviteMembers({
+      const {
+        created: invites,
+        alreadyMembers,
+        alreadyInvited,
+      } = await inviteMembers({
         slug: organizationSlug,
         emails: submission.value.emails,
         userId,
@@ -224,10 +242,19 @@ export const action = dashboardAction(
         }
       }
 
+      const teamPath = organizationTeamPath({ slug: organizationSlug });
+      const skipped = describeSkippedInvites(alreadyMembers, alreadyInvited);
+
+      if (invites.length === 0) {
+        return redirectWithErrorMessage(teamPath, request, `No invitations sent: ${skipped}.`);
+      }
+
       return redirectWithSuccessMessage(
-        organizationTeamPath(invites[0].organization),
+        teamPath,
         request,
-        simplur`${submission.value.emails.length} member[|s] invited`
+        skipped
+          ? simplur`${invites.length} member[|s] invited. Skipped ${skipped}.`
+          : simplur`${invites.length} member[|s] invited`
       );
     } catch (error: any) {
       return json({ errors: { body: error.message } }, { status: 400 });

apps/webapp/app/routes/api.v1.orgs.$orgParam.invites.ts

@@ -57,9 +57,7 @@ export const action = createActionPATApiRoute(
       return json({ error: "Membership is managed by Directory Sync" }, { status: 403 });
     }
 
-    // Returns only the invites created by this call; already-invited emails are
-    // skipped (re-sending is the dashboard's dedicated resend flow, not this).
-    const created = await inviteMembers({
+    const { created, alreadyMembers, alreadyInvited } = await inviteMembers({
       slug: organization.slug,
       emails: body.emails,
       userId: authentication.userId,
@@ -85,13 +83,11 @@ export const action = createActionPATApiRoute(
     // Report per-email outcome so callers aren't misled by an empty list on
     // re-invite. 201 when something was created, 200 when everything already
     // existed.
-    const createdEmails = new Set(created.map((invite) => invite.email));
-    const alreadyInvited = [...new Set(body.emails)].filter((email) => !createdEmails.has(email));
-
     return json(
       {
         invited: created.map((invite) => ({ id: invite.id, email: invite.email })),
         alreadyInvited,
+        alreadyMembers,
       },
       { status: created.length > 0 ? 201 : 200 }
     );

apps/webapp/test/member.server.test.ts

@@ -1,5 +1,5 @@
 import { randomBytes } from "node:crypto";
-import { describe, expect, vi } from "vitest";
+import { beforeEach, describe, expect, vi } from "vitest";
 import type { PrismaClient } from "@trigger.dev/database";
 
 const prismaHolder = vi.hoisted(() => ({
@@ -8,40 +8,53 @@ const prismaHolder = vi.hoisted(() => ({
 
 type SetUserRoleParams = { userId: string; organizationId: string; roleId: string };
 type SetUserRoleResult = { ok: true } | { ok: false; error: string; code?: "last_owner" };
+type CurrentRole = { id: string } | null;
 
 const rbacHolder = vi.hoisted(() => ({
   setUserRoleCalls: [] as SetUserRoleParams[],
   setUserRoleResult: { ok: true } as SetUserRoleResult,
+  currentRole: null as CurrentRole,
 }));
 
 vi.mock("~/services/rbac.server", () => ({
   rbac: {
+    getUserRole: async () => rbacHolder.currentRole,
     setUserRole: async (params: SetUserRoleParams) => {
       rbacHolder.setUserRoleCalls.push(params);
       return rbacHolder.setUserRoleResult;
     },
   },
 }));
 
-function resetRbac(result: SetUserRoleResult = { ok: true }) {
+function resetRbac(result: SetUserRoleResult = { ok: true }, currentRole: CurrentRole = null) {
   rbacHolder.setUserRoleCalls.length = 0;
   rbacHolder.setUserRoleResult = result;
+  rbacHolder.currentRole = currentRole;
 }
 
-vi.mock("~/db.server", () => ({
-  get prisma() {
-    if (!prismaHolder.client) {
-      throw new Error("test prisma not set");
-    }
-    return prismaHolder.client;
-  },
-  get $replica() {
-    if (!prismaHolder.client) {
-      throw new Error("test prisma not set");
-    }
-    return prismaHolder.client;
-  },
-}));
+beforeEach(() => {
+  resetRbac();
+});
+
+vi.mock("~/db.server", async () => {
+  const { Prisma } = await import("@trigger.dev/database");
+
+  return {
+    Prisma,
+    get prisma() {
+      if (!prismaHolder.client) {
+        throw new Error("test prisma not set");
+      }
+      return prismaHolder.client;
+    },
+    get $replica() {
+      if (!prismaHolder.client) {
+        throw new Error("test prisma not set");
+      }
+      return prismaHolder.client;
+    },
+  };
+});
 
 import { postgresTest } from "@internal/testcontainers";
 
@@ -276,7 +289,6 @@ describe("acceptInvite", () => {
     { timeout: 60_000 },
     async ({ prisma }) => {
       prismaHolder.client = prisma;
-      resetRbac();
       const { acceptInvite } = await import("../app/models/member.server");
 
       const { invitee, organization, invite } = await seedInviteFixture(prisma, {
@@ -297,11 +309,11 @@ describe("acceptInvite", () => {
   );
 
   postgresTest(
-    "does not apply the invite RBAC role to a user who is already a member",
+    "does not apply the invite RBAC role to a member who already has a role",
     { timeout: 60_000 },
     async ({ prisma }) => {
       prismaHolder.client = prisma;
-      resetRbac();
+      resetRbac({ ok: true }, { id: "role_owner" });
       const { acceptInvite } = await import("../app/models/member.server");
 
       const { invitee, organization, invite } = await seedInviteFixture(prisma, {
@@ -337,6 +349,39 @@ describe("acceptInvite", () => {
     }
   );
 
+  postgresTest(
+    "applies the invite RBAC role to an existing member who has no role assigned",
+    { timeout: 60_000 },
+    async ({ prisma }) => {
+      prismaHolder.client = prisma;
+      resetRbac({ ok: true }, null);
+      const { acceptInvite } = await import("../app/models/member.server");
+
+      const { invitee, organization, invite } = await seedInviteFixture(prisma, {
+        activeProjectCount: 1,
+        rbacRoleId: "role_admin",
+      });
+
+      await prisma.orgMember.create({
+        data: {
+          organizationId: organization.id,
+          userId: invitee.id,
+          role: "MEMBER",
+        },
+      });
+
+      await acceptInvite({
+        inviteId: invite.id,
+        organizationId: organization.id,
+        user: { id: invitee.id, email: invitee.email },
+      });
+
+      expect(rbacHolder.setUserRoleCalls).toEqual([
+        { userId: invitee.id, organizationId: organization.id, roleId: "role_admin" },
+      ]);
+    }
+  );
+
   postgresTest(
     "still joins the org when the RBAC role assignment is refused",
     { timeout: 60_000 },
@@ -367,8 +412,6 @@ describe("acceptInvite", () => {
         where: { userId: invitee.id, organizationId: organization.id },
       });
       expect(member).not.toBeNull();
-
-      resetRbac();
     }
   );
 });
@@ -397,13 +440,15 @@ describe("inviteMembers", () => {
 
       const newcomerEmail = `newcomer-${randomHex(8)}@test.local`;
 
-      const created = await inviteMembers({
+      const { created, alreadyMembers, alreadyInvited } = await inviteMembers({
         slug: organization.slug,
         emails: [invitee.email, newcomerEmail],
         userId: inviter.id,
       });
 
       expect(created.map((row) => row.email)).toEqual([newcomerEmail]);
+      expect(alreadyMembers).toEqual([invitee.email]);
+      expect(alreadyInvited).toEqual([]);
 
       const invites = await prisma.orgMemberInvite.findMany({
         where: { organizationId: organization.id },
@@ -412,6 +457,31 @@ describe("inviteMembers", () => {
       expect(invites.map((pending) => pending.email)).toEqual([newcomerEmail]);
     }
   );
+
+  postgresTest(
+    "reports an email with a pending invitation as already invited",
+    { timeout: 60_000 },
+    async ({ prisma }) => {
+      prismaHolder.client = prisma;
+      const { inviteMembers } = await import("../app/models/member.server");
+
+      const { inviter, organization, invite } = await seedInviteFixture(prisma, {
+        activeProjectCount: 0,
+      });
+
+      const newcomerEmail = `newcomer-${randomHex(8)}@test.local`;
+
+      const { created, alreadyMembers, alreadyInvited } = await inviteMembers({
+        slug: organization.slug,
+        emails: [invite.email, newcomerEmail],
+        userId: inviter.id,
+      });
+
+      expect(created.map((row) => row.email)).toEqual([newcomerEmail]);
+      expect(alreadyInvited).toEqual([invite.email]);
+      expect(alreadyMembers).toEqual([]);
+    }
+  );
 });
 
 describe("provisionMemberDevelopmentEnvironments", () => {