Skip to content

Commit 7e167ed

Browse files
icecrasher321claude
andcommitted
fix(invitations): tell an existing member their standing is unchanged
The join preview reported the same no-join shape for two different outcomes: an external collaborator, and an invitee who already belongs to the organization acceptance lands in. `buildMembershipNotice` rendered both as "you'll join as an external collaborator ... everything you own stays yours", which is wrong for an existing member — they stay an internal member and simply gain the granted workspaces. The preview now reports `alreadyMemberOfOrganization` for that case (a membership in a DIFFERENCE organization is still the external path, since acceptance downgrades), and the notice states that standing is unchanged. This is the same conflation behind the accept loop fixed in 4eb7725, now removed from the shape itself rather than worked around per consumer. Also re-verify the removal-impact disclosure at the moment of confirmation. `isFetching` only holds the confirm button while a request is in flight, so an identity-bound credential the member gained after the fetch settled would break on removal without ever being disclosed. Confirm now refetches and, if the set changed, keeps the dialog open on the refreshed warning instead of proceeding. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 4eb7725 commit 7e167ed

5 files changed

Lines changed: 82 additions & 4 deletions

File tree

apps/sim/app/workspace/[workspaceId]/settings/components/team-management/team-management.tsx

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,15 @@ export function TeamManagement({
8787
data: removalImpactCredentials,
8888
isFetching: isRemovalImpactFetching,
8989
isError: isRemovalImpactError,
90+
refetch: refetchRemovalImpact,
9091
} = useMemberRemovalImpact(organizationId, removeMemberDialog.memberId, {
9192
enabled: removeMemberDialog.open,
9293
})
9394

95+
const disclosedBreakingCredentials = [
96+
...new Set(removalImpactCredentials?.map((credential) => credential.displayName) ?? []),
97+
]
98+
9499
const totalSeats = organizationBillingData?.data?.totalSeats ?? 0
95100
const usedSeats = organizationBillingData?.data?.members?.length ?? 0
96101
const reservedSeats = organizationBillingData?.data?.usedSeats ?? 0
@@ -174,6 +179,25 @@ export function TeamManagement({
174179
if (!session?.user || !memberId) return
175180

176181
try {
182+
/**
183+
* Re-verify the disclosure at the moment of confirmation. `isFetching`
184+
* holds the button only while a request is in flight; a credential the
185+
* member gained after the fetch settled would otherwise be removed
186+
* without ever having been disclosed. On a change the dialog stays open
187+
* showing the refreshed warning, so the admin confirms against what is
188+
* actually true — the same consent contract the invite flow uses.
189+
*/
190+
const refreshed = await refetchRemovalImpact()
191+
if (refreshed.data) {
192+
const current = [...new Set(refreshed.data.map((credential) => credential.displayName))]
193+
if (
194+
current.length !== disclosedBreakingCredentials.length ||
195+
current.some((name) => !disclosedBreakingCredentials.includes(name))
196+
) {
197+
return
198+
}
199+
}
200+
177201
await removeMemberMutation.mutateAsync({
178202
memberId,
179203
orgId: organizationId,
@@ -195,6 +219,7 @@ export function TeamManagement({
195219
}, [
196220
removeMemberDialog.memberId,
197221
removeMemberDialog.isSelfRemoval,
222+
disclosedBreakingCredentials,
198223
session?.user?.id,
199224
organizationId,
200225
removeMemberMutation,
@@ -368,9 +393,7 @@ export function TeamManagement({
368393
memberName={removeMemberDialog.memberName}
369394
isSelfRemoval={removeMemberDialog.isSelfRemoval}
370395
isExternalRemoval={removeMemberDialog.isExternalRemoval}
371-
breakingCredentials={[
372-
...new Set(removalImpactCredentials?.map((c) => c.displayName) ?? []),
373-
]}
396+
breakingCredentials={disclosedBreakingCredentials}
374397
credentialImpactPending={isRemovalImpactFetching}
375398
credentialImpactFailed={isRemovalImpactError}
376399
isSubmitting={removeMemberMutation.isPending}

apps/sim/lib/api/contracts/invitations.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,13 @@ export const invitationDetailsSchema = z.object({
140140

141141
export const invitationJoinPreviewSchema = z.object({
142142
willJoinOrganization: z.boolean(),
143+
/**
144+
* The invitee is already a member of the organization acceptance lands in, so
145+
* only workspace access changes. Reported separately because it shares
146+
* `willJoinOrganization: false` with the external case, which means something
147+
* different to the person accepting.
148+
*/
149+
alreadyMemberOfOrganization: z.boolean(),
143150
/** Name of the organization acceptance will actually join. */
144151
organizationName: z.string().nullable(),
145152
workspacesToMove: z.array(z.string()).max(DISCLOSED_WORKSPACE_ID_LIMIT),

apps/sim/lib/invitations/core.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,15 @@ async function hasLiveGrantInStampedOrganization(
218218

219219
export interface InvitationJoinPreviewResult {
220220
willJoinOrganization: boolean
221+
/**
222+
* True when the invitee is ALREADY a member of the organization acceptance
223+
* lands in, so nothing about their standing changes — they simply gain the
224+
* granted workspaces. Distinct from the external case, which also reports
225+
* `willJoinOrganization: false` but means "no seat, and never a member".
226+
* Without this the accept screen cannot tell the two apart and would tell an
227+
* existing member they are joining as an external collaborator.
228+
*/
229+
alreadyMemberOfOrganization: boolean
221230
/**
222231
* Name of the organization acceptance will actually join. For a workspace
223232
* invite this is the granted workspace's LIVE organization, which can differ
@@ -247,6 +256,7 @@ export async function getInvitationJoinPreview(
247256
): Promise<InvitationJoinPreviewResult> {
248257
const noJoin: InvitationJoinPreviewResult = {
249258
willJoinOrganization: false,
259+
alreadyMemberOfOrganization: false,
250260
organizationName: null,
251261
workspacesToMove: [],
252262
workspaceIdsToMove: [],
@@ -277,7 +287,16 @@ export async function getInvitationJoinPreview(
277287
* one (acceptance downgrades to external or rejects).
278288
*/
279289
const existingMembership = await getUserOrganization(inviteeUserId)
280-
if (existingMembership) return noJoin
290+
if (existingMembership) {
291+
/**
292+
* Already in the organization acceptance lands in: nothing about their
293+
* standing changes. A membership in a DIFFERENT organization is the
294+
* external case — acceptance downgrades — so it keeps the plain shape.
295+
*/
296+
const inTargetOrganization =
297+
!!workspaceOrganizationId && existingMembership.organizationId === workspaceOrganizationId
298+
return inTargetOrganization ? { ...noJoin, alreadyMemberOfOrganization: true } : noJoin
299+
}
281300

282301
if (!(await stampedOrganizationAllowsEscalation(inv, workspaceOrganizationId))) return noJoin
283302

@@ -324,6 +343,7 @@ export async function getInvitationJoinPreview(
324343

325344
return {
326345
willJoinOrganization: true,
346+
alreadyMemberOfOrganization: false,
327347
organizationName: targetOrganizationName,
328348
workspacesToMove: ownedWorkspaces.map((row) => row.name),
329349
workspaceIdsToMove: ownedWorkspaces.map((row) => row.id),

apps/sim/lib/invitations/disclosure-copy.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
const preview = (over: Partial<Parameters<typeof buildMembershipNotice>[0]['joinPreview']> = {}) =>
1111
({
1212
willJoinOrganization: true,
13+
alreadyMemberOfOrganization: false,
1314
organizationName: 'Acme',
1415
workspacesToMove: [],
1516
workspaceIdsToMove: [],
@@ -61,6 +62,24 @@ describe('buildMembershipNotice', () => {
6162
expect(notice).not.toContain('uses one of their seats')
6263
})
6364

65+
/**
66+
* An existing member shares `willJoinOrganization: false` with the external
67+
* case but means something different — their standing is unchanged.
68+
*/
69+
it('discloses unchanged standing for an existing member, not external', () => {
70+
const notice = buildMembershipNotice({
71+
joinPreview: preview({ willJoinOrganization: false, alreadyMemberOfOrganization: true }),
72+
membershipIntent: 'internal',
73+
isOrganizationAdminRole: false,
74+
organizationLabel: 'Acme',
75+
isOrganizationScoped: true,
76+
})
77+
78+
expect(notice).toContain('already a member of Acme')
79+
expect(notice).not.toContain('external collaborator')
80+
expect(notice).not.toContain('uses one of their seats')
81+
})
82+
6483
it('falls back to the sent intent when no preview could be computed', () => {
6584
expect(
6685
buildMembershipNotice({

apps/sim/lib/invitations/disclosure-copy.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,15 @@ export function buildMembershipNotice({
7474
}): string {
7575
if (!isOrganizationScoped || !membershipIntent) return ''
7676

77+
/**
78+
* Already a member of the organization acceptance lands in: their standing is
79+
* unchanged, so neither the join copy nor the external copy is true. Checked
80+
* first because this case also reports `willJoinOrganization: false`.
81+
*/
82+
if (joinPreview?.alreadyMemberOfOrganization) {
83+
return ` You're already a member of ${organizationLabel}, so accepting only adds the workspaces above — your membership and seat don't change.`
84+
}
85+
7786
const willJoinOrganization = joinPreview
7887
? joinPreview.willJoinOrganization
7988
: membershipIntent !== 'external'

0 commit comments

Comments
 (0)