Skip to content

Commit 2bc256d

Browse files
committed
address comments
1 parent 8614a4b commit 2bc256d

3 files changed

Lines changed: 23 additions & 12 deletions

File tree

apps/sim/app/api/organizations/[id]/permission-groups/[groupId]/route.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,16 +115,28 @@ export const PUT = withRouteHandler(
115115
? { ...currentConfig, ...updates.config }
116116
: currentConfig
117117

118+
// Demoting the org default with no new scope: it becomes a non-default
119+
// group with no workspaces (inert) until an admin re-scopes it. The client
120+
// sends only `isDefault: false`, so this never forwards a workspace list
121+
// (which a non-default group otherwise requires) against the per-group cap.
122+
const demotingDefaultToInert =
123+
group.isDefault &&
124+
updates.isDefault === false &&
125+
updates.appliesToAllWorkspaces === undefined &&
126+
updates.workspaceIds === undefined
127+
118128
// Resolve the target workspace scope. Setting the group as default forces
119129
// all-workspaces; otherwise an explicit `appliesToAllWorkspaces` wins, and
120130
// supplying `workspaceIds` alone implies a specific scope.
121131
const scopeProvided =
132+
demotingDefaultToInert ||
122133
updates.appliesToAllWorkspaces !== undefined ||
123134
updates.workspaceIds !== undefined ||
124135
updates.isDefault === true
125136

126-
const resolvedAppliesToAll =
127-
updates.isDefault === true
137+
const resolvedAppliesToAll = demotingDefaultToInert
138+
? false
139+
: updates.isDefault === true
128140
? true
129141
: updates.appliesToAllWorkspaces !== undefined
130142
? updates.appliesToAllWorkspaces
@@ -188,7 +200,7 @@ export const PUT = withRouteHandler(
188200
if (!resolvedAppliesToAll) {
189201
resolvedWorkspaceIds =
190202
providedWorkspaceIds ?? (await getGroupWorkspaces(id, tx)).map((ws) => ws.id)
191-
if (resolvedWorkspaceIds.length === 0) {
203+
if (resolvedWorkspaceIds.length === 0 && !demotingDefaultToInert) {
192204
throw new Error('NO_WORKSPACES')
193205
}
194206
}

apps/sim/ee/access-control/components/access-control.tsx

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,19 +1027,14 @@ export function AccessControl() {
10271027
if (!viewingGroup || !organizationId) return
10281028
const seq = ++scopeWriteSeqRef.current
10291029
try {
1030+
// Promoting forces all-workspaces; demoting leaves the group non-default
1031+
// with no workspaces (inert) until it is re-scoped from the selector — the
1032+
// route handles this from `isDefault: false` alone, so no workspace list
1033+
// (bounded by the per-group cap) is sent.
10301034
const result = await updatePermissionGroup.mutateAsync({
10311035
id: viewingGroup.id,
10321036
organizationId,
10331037
isDefault: enabled,
1034-
// Demoting the org default to a workspace group targets every current
1035-
// workspace so it keeps governing the same scope; the admin can narrow
1036-
// it afterward. Promoting clears workspaces (the route forces all-ws).
1037-
...(enabled
1038-
? {}
1039-
: {
1040-
appliesToAllWorkspaces: false,
1041-
workspaceIds: organizationWorkspaces.map((ws) => ws.id),
1042-
}),
10431038
})
10441039

10451040
if (seq !== scopeWriteSeqRef.current) return

apps/sim/lib/api/contracts/permission-groups.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,10 @@ describe('updatePermissionGroupBodySchema', () => {
9090
expect(updatePermissionGroupBodySchema.safeParse({}).success).toBe(true)
9191
})
9292

93+
it('accepts demoting the default via isDefault:false alone (the route re-scopes it)', () => {
94+
expect(updatePermissionGroupBodySchema.safeParse({ isDefault: false }).success).toBe(true)
95+
})
96+
9397
it('rejects switching to specific scope with no workspaces', () => {
9498
const result = updatePermissionGroupBodySchema.safeParse({
9599
appliesToAllWorkspaces: false,

0 commit comments

Comments
 (0)