Skip to content

Commit a54549e

Browse files
committed
fix(settings): address pr review — atomic autoAddNewMembers, extract query hook, fix types and signal forwarding
1 parent 755bce6 commit a54549e

File tree

7 files changed

+101
-67
lines changed

7 files changed

+101
-67
lines changed

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

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -155,31 +155,34 @@ export async function PUT(req: NextRequest, { params }: { params: Promise<{ id:
155155
? { ...currentConfig, ...updates.config }
156156
: currentConfig
157157

158-
// If setting autoAddNewMembers to true, unset it on other groups in the org first
159-
if (updates.autoAddNewMembers === true) {
160-
await db
161-
.update(permissionGroup)
162-
.set({ autoAddNewMembers: false, updatedAt: new Date() })
163-
.where(
164-
and(
165-
eq(permissionGroup.organizationId, result.group.organizationId),
166-
eq(permissionGroup.autoAddNewMembers, true)
158+
const now = new Date()
159+
160+
await db.transaction(async (tx) => {
161+
if (updates.autoAddNewMembers === true) {
162+
await tx
163+
.update(permissionGroup)
164+
.set({ autoAddNewMembers: false, updatedAt: now })
165+
.where(
166+
and(
167+
eq(permissionGroup.organizationId, result.group.organizationId),
168+
eq(permissionGroup.autoAddNewMembers, true)
169+
)
167170
)
168-
)
169-
}
171+
}
170172

171-
await db
172-
.update(permissionGroup)
173-
.set({
174-
...(updates.name !== undefined && { name: updates.name }),
175-
...(updates.description !== undefined && { description: updates.description }),
176-
...(updates.autoAddNewMembers !== undefined && {
177-
autoAddNewMembers: updates.autoAddNewMembers,
178-
}),
179-
config: newConfig,
180-
updatedAt: new Date(),
181-
})
182-
.where(eq(permissionGroup.id, id))
173+
await tx
174+
.update(permissionGroup)
175+
.set({
176+
...(updates.name !== undefined && { name: updates.name }),
177+
...(updates.description !== undefined && { description: updates.description }),
178+
...(updates.autoAddNewMembers !== undefined && {
179+
autoAddNewMembers: updates.autoAddNewMembers,
180+
}),
181+
config: newConfig,
182+
updatedAt: now,
183+
})
184+
.where(eq(permissionGroup.id, id))
185+
})
183186

184187
const [updated] = await db
185188
.select()

apps/sim/app/api/permission-groups/route.ts

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -171,19 +171,6 @@ export async function POST(req: Request) {
171171
...config,
172172
}
173173

174-
// If autoAddNewMembers is true, unset it on any existing groups first
175-
if (autoAddNewMembers) {
176-
await db
177-
.update(permissionGroup)
178-
.set({ autoAddNewMembers: false, updatedAt: new Date() })
179-
.where(
180-
and(
181-
eq(permissionGroup.organizationId, organizationId),
182-
eq(permissionGroup.autoAddNewMembers, true)
183-
)
184-
)
185-
}
186-
187174
const now = new Date()
188175
const newGroup = {
189176
id: generateId(),
@@ -197,7 +184,20 @@ export async function POST(req: Request) {
197184
autoAddNewMembers: autoAddNewMembers || false,
198185
}
199186

200-
await db.insert(permissionGroup).values(newGroup)
187+
await db.transaction(async (tx) => {
188+
if (autoAddNewMembers) {
189+
await tx
190+
.update(permissionGroup)
191+
.set({ autoAddNewMembers: false, updatedAt: now })
192+
.where(
193+
and(
194+
eq(permissionGroup.organizationId, organizationId),
195+
eq(permissionGroup.autoAddNewMembers, true)
196+
)
197+
)
198+
}
199+
await tx.insert(permissionGroup).values(newGroup)
200+
})
201201

202202
logger.info('Created permission group', {
203203
permissionGroupId: newGroup.id,

apps/sim/app/workspace/[workspaceId]/settings/[section]/page.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { dehydrate, HydrationBoundary } from '@tanstack/react-query'
22
import type { Metadata } from 'next'
3+
import { isBillingEnabled } from '@/lib/core/config/feature-flags'
34
import { getQueryClient } from '@/app/_shell/providers/get-query-client'
45
import type { SettingsSection } from '@/app/workspace/[workspaceId]/settings/navigation'
56
import { prefetchGeneralSettings, prefetchSubscriptionData, prefetchUserProfile } from './prefetch'
@@ -11,6 +12,7 @@ const SECTION_TITLES: Record<string, string> = {
1112
secrets: 'Secrets',
1213
'template-profile': 'Template Profile',
1314
'access-control': 'Access Control',
15+
'audit-logs': 'Audit Logs',
1416
apikeys: 'Sim Keys',
1517
byok: 'BYOK',
1618
subscription: 'Subscription',
@@ -46,7 +48,7 @@ export default async function SettingsSectionPage({
4648

4749
void prefetchGeneralSettings(queryClient)
4850
void prefetchUserProfile(queryClient)
49-
void prefetchSubscriptionData(queryClient)
51+
if (isBillingEnabled) void prefetchSubscriptionData(queryClient)
5052

5153
return (
5254
<HydrationBoundary state={dehydrate(queryClient)}>

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

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import { useCallback, useMemo, useState } from 'react'
44
import { createLogger } from '@sim/logger'
5-
import { useQuery } from '@tanstack/react-query'
65
import { Plus, Search } from 'lucide-react'
76
import {
87
Avatar,
@@ -42,17 +41,27 @@ import {
4241
useRemovePermissionGroupMember,
4342
useUpdatePermissionGroup,
4443
} from '@/ee/access-control/hooks/permission-groups'
44+
import { useBlacklistedProviders } from '@/hooks/queries/allowed-providers'
4545
import { useOrganization, useOrganizations } from '@/hooks/queries/organization'
4646
import { useSubscriptionData } from '@/hooks/queries/subscription'
4747
import { PROVIDER_DEFINITIONS } from '@/providers/models'
4848
import { getAllProviderIds } from '@/providers/utils'
4949

5050
const logger = createLogger('AccessControl')
5151

52+
interface OrgMember {
53+
userId: string
54+
user: {
55+
name: string | null
56+
email: string
57+
image?: string | null
58+
}
59+
}
60+
5261
interface AddMembersModalProps {
5362
open: boolean
5463
onOpenChange: (open: boolean) => void
55-
availableMembers: any[]
64+
availableMembers: OrgMember[]
5665
selectedMemberIds: Set<string>
5766
setSelectedMemberIds: React.Dispatch<React.SetStateAction<Set<string>>>
5867
onAddMembers: () => void
@@ -73,7 +82,7 @@ function AddMembersModal({
7382
const filteredMembers = useMemo(() => {
7483
if (!searchTerm.trim()) return availableMembers
7584
const query = searchTerm.toLowerCase()
76-
return availableMembers.filter((m: any) => {
85+
return availableMembers.filter((m) => {
7786
const name = m.user?.name || ''
7887
const email = m.user?.email || ''
7988
return name.toLowerCase().includes(query) || email.toLowerCase().includes(query)
@@ -82,12 +91,12 @@ function AddMembersModal({
8291

8392
const allFilteredSelected = useMemo(() => {
8493
if (filteredMembers.length === 0) return false
85-
return filteredMembers.every((m: any) => selectedMemberIds.has(m.userId))
94+
return filteredMembers.every((m) => selectedMemberIds.has(m.userId))
8695
}, [filteredMembers, selectedMemberIds])
8796

8897
const handleToggleAll = () => {
8998
if (allFilteredSelected) {
90-
const filteredIds = new Set(filteredMembers.map((m: any) => m.userId))
99+
const filteredIds = new Set(filteredMembers.map((m) => m.userId))
91100
setSelectedMemberIds((prev) => {
92101
const next = new Set(prev)
93102
filteredIds.forEach((id) => next.delete(id))
@@ -96,7 +105,7 @@ function AddMembersModal({
96105
} else {
97106
setSelectedMemberIds((prev) => {
98107
const next = new Set(prev)
99-
filteredMembers.forEach((m: any) => next.add(m.userId))
108+
filteredMembers.forEach((m) => next.add(m.userId))
100109
return next
101110
})
102111
}
@@ -153,7 +162,7 @@ function AddMembersModal({
153162
</p>
154163
) : (
155164
<div className='flex flex-col'>
156-
{filteredMembers.map((member: any) => {
165+
{filteredMembers.map((member) => {
157166
const name = member.user?.name || 'Unknown'
158167
const email = member.user?.email || ''
159168
const avatarInitial = name.charAt(0).toUpperCase()
@@ -466,16 +475,7 @@ export function AccessControl() {
466475
return a.name.localeCompare(b.name)
467476
})
468477
}, [])
469-
const { data: blacklistedProvidersData } = useQuery({
470-
queryKey: ['blacklistedProviders'],
471-
queryFn: async ({ signal }) => {
472-
const res = await fetch('/api/settings/allowed-providers', { signal })
473-
if (!res.ok) return { blacklistedProviders: [] as string[] }
474-
return res.json() as Promise<{ blacklistedProviders: string[] }>
475-
},
476-
staleTime: 5 * 60 * 1000,
477-
enabled: showConfigModal,
478-
})
478+
const { data: blacklistedProvidersData } = useBlacklistedProviders({ enabled: showConfigModal })
479479

480480
const allProviderIds = useMemo(() => {
481481
const allIds = getAllProviderIds()
@@ -733,7 +733,7 @@ export function AccessControl() {
733733

734734
const availableMembersToAdd = useMemo(() => {
735735
const existingMemberUserIds = new Set(members.map((m) => m.userId))
736-
return orgMembers.filter((m: any) => !existingMemberUserIds.has(m.userId))
736+
return orgMembers.filter((m) => !existingMemberUserIds.has(m.userId))
737737
}, [orgMembers, members])
738738

739739
if (isLoading) {

apps/sim/ee/sso/hooks/sso.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ export const ssoKeys = {
1414
/**
1515
* Fetch SSO providers
1616
*/
17-
async function fetchSSOProviders() {
18-
const response = await fetch('/api/auth/sso/providers')
17+
async function fetchSSOProviders(signal: AbortSignal) {
18+
const response = await fetch('/api/auth/sso/providers', { signal })
1919
if (!response.ok) {
2020
throw new Error('Failed to fetch SSO providers')
2121
}
@@ -32,7 +32,7 @@ interface UseSSOProvidersOptions {
3232
export function useSSOProviders({ enabled = true }: UseSSOProvidersOptions = {}) {
3333
return useQuery({
3434
queryKey: ssoKeys.providers(),
35-
queryFn: fetchSSOProviders,
35+
queryFn: ({ signal }) => fetchSSOProviders(signal),
3636
staleTime: 5 * 60 * 1000,
3737
placeholderData: keepPreviousData,
3838
enabled,
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
'use client'
2+
3+
import { useQuery } from '@tanstack/react-query'
4+
5+
/**
6+
* Query key factory for allowed providers queries
7+
*/
8+
export const allowedProvidersKeys = {
9+
all: ['allowedProviders'] as const,
10+
blacklisted: () => [...allowedProvidersKeys.all, 'blacklisted'] as const,
11+
}
12+
13+
interface BlacklistedProvidersResponse {
14+
blacklistedProviders: string[]
15+
}
16+
17+
async function fetchBlacklistedProviders(
18+
signal: AbortSignal
19+
): Promise<BlacklistedProvidersResponse> {
20+
const res = await fetch('/api/settings/allowed-providers', { signal })
21+
if (!res.ok) return { blacklistedProviders: [] }
22+
return res.json()
23+
}
24+
25+
/**
26+
* Hook to fetch the list of blacklisted provider IDs from the server.
27+
*/
28+
export function useBlacklistedProviders({ enabled = true }: { enabled?: boolean } = {}) {
29+
return useQuery({
30+
queryKey: allowedProvidersKeys.blacklisted(),
31+
queryFn: ({ signal }) => fetchBlacklistedProviders(signal),
32+
staleTime: 5 * 60 * 1000,
33+
enabled,
34+
})
35+
}

apps/sim/providers/utils.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import type { ChatCompletionChunk } from 'openai/resources/chat/completions'
44
import type { CompletionUsage } from 'openai/resources/completions'
55
import { dollarsToCredits } from '@/lib/billing/credits/conversion'
66
import { env } from '@/lib/core/config/env'
7-
import { isHosted } from '@/lib/core/config/feature-flags'
7+
import { getBlacklistedProvidersFromEnv, isHosted } from '@/lib/core/config/feature-flags'
88
import {
99
buildCanonicalIndex,
1010
type CanonicalGroup,
@@ -281,14 +281,8 @@ export function getProviderModels(providerId: ProviderId): string[] {
281281
return getProviderModelsFromDefinitions(providerId)
282282
}
283283

284-
function getBlacklistedProviders(): string[] {
285-
if (!env.BLACKLISTED_PROVIDERS) return []
286-
return env.BLACKLISTED_PROVIDERS.split(',').map((p) => p.trim().toLowerCase())
287-
}
288-
289284
export function isProviderBlacklisted(providerId: string): boolean {
290-
const blacklist = getBlacklistedProviders()
291-
return blacklist.includes(providerId.toLowerCase())
285+
return getBlacklistedProvidersFromEnv().includes(providerId.toLowerCase())
292286
}
293287

294288
/**

0 commit comments

Comments
 (0)