Skip to content

Commit 91efd36

Browse files
icecrasher321claude
andcommitted
refactor(invitations): drop dead code left by the revocation extraction
`revokeInvitationWorkspaceGrant` lost its only caller when the DELETE route moved to `revokeInvitationAsAdmin`, leaving a locked wrapper nothing invoked. Remove it and fold its documentation into `revokeInvitationWorkspaceGrantTx`, which direct grants and scoped revocation still call. The grant-revocation test now drives the transactional form directly, so the sibling-grant and final-grant-cancels behaviour it covers stays under test. `isSameOrgMember` has had no caller since before this branch — direct grant resolves membership through `getUserOrganization` inside its own transaction — so it and its tests go too. `getWorkspaceMembership` is no longer imported outside its module now that credential creation reads `getCredentialCreationWorkspaceContext`; make it module-private rather than leave it on the public surface. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 244bff6 commit 91efd36

5 files changed

Lines changed: 10 additions & 77 deletions

File tree

apps/sim/lib/credentials/environment.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export interface WorkspaceMembership {
2424
* Credential-admin status is derived from workspace role at access time, so
2525
* members are seeded only for use access (the owner plus permission holders).
2626
*/
27-
export async function getWorkspaceMembership(workspaceId: string): Promise<WorkspaceMembership> {
27+
async function getWorkspaceMembership(workspaceId: string): Promise<WorkspaceMembership> {
2828
const [workspaceRows, permissionRows] = await Promise.all([
2929
db
3030
.select({ ownerId: workspace.ownerId })

apps/sim/lib/invitations/core.ts

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1746,25 +1746,8 @@ export async function revokeInvitationAsAdmin(input: {
17461746
* admin of one workspace has no authority over the others. Removing the final
17471747
* grant would otherwise strand a pending invitation that grants nothing, so
17481748
* that case cancels it instead.
1749-
*/
1750-
export async function revokeInvitationWorkspaceGrant({
1751-
invitationId,
1752-
workspaceId,
1753-
}: {
1754-
invitationId: string
1755-
workspaceId: string
1756-
}): Promise<{ revoked: boolean; invitationCancelled: boolean }> {
1757-
return db.transaction(async (tx) => {
1758-
await acquireInvitationMutationLocks(tx, {
1759-
invitationIds: [invitationId],
1760-
workspaceIds: [workspaceId],
1761-
})
1762-
return revokeInvitationWorkspaceGrantTx(tx, { invitationId, workspaceId })
1763-
})
1764-
}
1765-
1766-
/**
1767-
* Transaction-local form used by callers that already hold the canonical
1749+
*
1750+
* Transaction-local: callers must already hold the canonical
17681751
* invitation/workspace advisory lock set. Keeping the grant deletion and
17691752
* final-grant cancellation in one implementation prevents direct grants,
17701753
* scoped revocation, and future callers from drifting on multi-workspace

apps/sim/lib/invitations/direct-grant.test.ts

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ vi.mock('@/lib/posthog/server', () => ({
7474
import {
7575
DirectGrantContextChangedError,
7676
grantWorkspaceAccessDirectly,
77-
isSameOrgMember,
7877
} from '@/lib/invitations/direct-grant'
7978

8079
const baseInput = {
@@ -246,30 +245,3 @@ describe('grantWorkspaceAccessDirectly', () => {
246245
expect(dbChainMockFns.insert).not.toHaveBeenCalled()
247246
})
248247
})
249-
250-
describe('isSameOrgMember', () => {
251-
beforeEach(() => {
252-
vi.clearAllMocks()
253-
resetDbChainMock()
254-
})
255-
256-
it('returns false when the workspace has no organization', async () => {
257-
expect(await isSameOrgMember('user-2', null)).toBe(false)
258-
expect(mockGetUserOrganization).not.toHaveBeenCalled()
259-
})
260-
261-
it('returns false when the user belongs to no organization', async () => {
262-
mockGetUserOrganization.mockResolvedValueOnce(null)
263-
expect(await isSameOrgMember('user-2', 'org-1')).toBe(false)
264-
})
265-
266-
it('returns true when the user belongs to the workspace organization', async () => {
267-
mockGetUserOrganization.mockResolvedValueOnce({ organizationId: 'org-1', role: 'member' })
268-
expect(await isSameOrgMember('user-2', 'org-1')).toBe(true)
269-
})
270-
271-
it('returns false when the user belongs to a different organization', async () => {
272-
mockGetUserOrganization.mockResolvedValueOnce({ organizationId: 'org-2', role: 'member' })
273-
expect(await isSameOrgMember('user-2', 'org-1')).toBe(false)
274-
})
275-
})

apps/sim/lib/invitations/direct-grant.ts

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -67,20 +67,6 @@ export interface GrantWorkspaceAccessDirectlyInput {
6767
notify?: boolean
6868
}
6969

70-
/**
71-
* Returns whether the given user is already a member of the workspace's
72-
* organization. Only same-org members are eligible for direct (no-acceptance)
73-
* workspace access.
74-
*/
75-
export async function isSameOrgMember(
76-
userId: string,
77-
workspaceOrganizationId: string | null
78-
): Promise<boolean> {
79-
if (!workspaceOrganizationId) return false
80-
const membership = await getUserOrganization(userId)
81-
return !!membership && membership.organizationId === workspaceOrganizationId
82-
}
83-
8470
async function getPendingWorkspaceInvitationIds(
8571
executor: DbOrTx,
8672
workspaceId: string,

apps/sim/lib/invitations/grant-revocation.test.ts

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,37 @@
11
/**
22
* @vitest-environment node
33
*/
4+
import { db } from '@sim/db'
45
import { invitation, invitationWorkspaceGrant } from '@sim/db/schema'
56
import { auditMock, dbChainMockFns, queueTableRows, resetDbChainMock } from '@sim/testing'
67
import { beforeEach, describe, expect, it, vi } from 'vitest'
78

8-
const { mockAcquireInvitationMutationLocks } = vi.hoisted(() => ({
9-
mockAcquireInvitationMutationLocks: vi.fn(),
10-
}))
11-
129
vi.mock('@sim/audit', () => auditMock)
1310

1411
vi.mock('@/lib/invitations/locks', () => ({
15-
acquireInvitationMutationLocks: mockAcquireInvitationMutationLocks,
12+
acquireInvitationMutationLocks: vi.fn(),
1613
}))
1714

18-
import { revokeInvitationWorkspaceGrant } from '@/lib/invitations/core'
15+
import { revokeInvitationWorkspaceGrantTx } from '@/lib/invitations/core'
1916

20-
describe('revokeInvitationWorkspaceGrant', () => {
17+
describe('revokeInvitationWorkspaceGrantTx', () => {
2118
beforeEach(() => {
2219
vi.clearAllMocks()
2320
resetDbChainMock()
24-
mockAcquireInvitationMutationLocks.mockResolvedValue(undefined)
2521
})
2622

27-
it('uses the shared lock fence and preserves sibling workspace grants', async () => {
23+
it('preserves sibling workspace grants', async () => {
2824
queueTableRows(invitation, [{ id: 'inv-1' }])
2925
queueTableRows(invitationWorkspaceGrant, [{ value: 1 }])
3026
dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'grant-1' }])
3127

3228
await expect(
33-
revokeInvitationWorkspaceGrant({
29+
revokeInvitationWorkspaceGrantTx(db, {
3430
invitationId: 'inv-1',
3531
workspaceId: 'ws-1',
3632
})
3733
).resolves.toEqual({ revoked: true, invitationCancelled: false })
3834

39-
expect(mockAcquireInvitationMutationLocks).toHaveBeenCalledWith(expect.anything(), {
40-
invitationIds: ['inv-1'],
41-
workspaceIds: ['ws-1'],
42-
})
4335
expect(dbChainMockFns.set).toHaveBeenCalledWith(
4436
expect.not.objectContaining({ status: 'cancelled' })
4537
)
@@ -51,7 +43,7 @@ describe('revokeInvitationWorkspaceGrant', () => {
5143
dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'grant-1' }])
5244

5345
await expect(
54-
revokeInvitationWorkspaceGrant({
46+
revokeInvitationWorkspaceGrantTx(db, {
5547
invitationId: 'inv-1',
5648
workspaceId: 'ws-1',
5749
})

0 commit comments

Comments
 (0)