Skip to content

Commit 0ab7225

Browse files
committed
fix: review findings -- clipboard error handling, React Query, revoke confirm, platform paths
1. CopyButton: wrapped clipboard.writeText in try/catch, shows toast.error on failure (matches DocsCodeBlock.tsx pattern) 2. React Query: replaced manual fetchKeys/useEffect/useState with useQuery + queryClient.invalidateQueries. Matches the established useCachedQuery.ts pattern used everywhere else in the dashboard. Generate and revoke both invalidate ['api-keys'] query. 3. Revoke confirmation: added a Dialog that shows key name and warns 'applications using this key will stop working immediately'. Matches SettingsPage delete confirmation pattern. 4. Platform-aware config paths: detects OS from userAgent, shows macOS/Windows/Linux paths accordingly instead of hardcoded macOS. Build passes, TS clean.
1 parent 289e0f1 commit 0ab7225

1 file changed

Lines changed: 81 additions & 37 deletions

File tree

frontend/src/pages/APIKeysPage.tsx

Lines changed: 81 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
import { useEffect, useState, useCallback } from 'react'
1+
import { useState } from 'react'
22
import { KeyRound, Plus, Copy, Check, Loader2, Trash2, Clock } from 'lucide-react'
3+
import { useQuery, useQueryClient } from '@tanstack/react-query'
34
import { useAuth } from '@/contexts/AuthContext'
45
import { Button } from '@/components/ui/button'
56
import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card'
@@ -64,9 +65,13 @@ function CopyButton({ text }: { text: string }) {
6465
const [copied, setCopied] = useState(false)
6566

6667
const handleCopy = async () => {
67-
await navigator.clipboard.writeText(text)
68-
setCopied(true)
69-
setTimeout(() => setCopied(false), 2000)
68+
try {
69+
await navigator.clipboard.writeText(text)
70+
setCopied(true)
71+
setTimeout(() => setCopied(false), 2000)
72+
} catch {
73+
toast.error('Failed to copy. Try selecting the text manually.')
74+
}
7075
}
7176

7277
return (
@@ -76,37 +81,49 @@ function CopyButton({ text }: { text: string }) {
7681
)
7782
}
7883

84+
function getConfigPaths(): { label: string; path: string }[] {
85+
const ua = navigator.userAgent.toLowerCase()
86+
if (ua.includes('win')) {
87+
return [
88+
{ label: 'Windows', path: '%APPDATA%\\Claude\\claude_desktop_config.json' },
89+
]
90+
}
91+
if (ua.includes('linux')) {
92+
return [
93+
{ label: 'Linux', path: '~/.config/Claude/claude_desktop_config.json' },
94+
]
95+
}
96+
return [
97+
{ label: 'macOS', path: '~/Library/Application Support/Claude/claude_desktop_config.json' },
98+
]
99+
}
100+
101+
async function fetchKeys(token: string): Promise<APIKey[]> {
102+
const res = await fetch(`${API_URL}/keys`, {
103+
headers: { Authorization: `Bearer ${token}` },
104+
})
105+
if (!res.ok) throw new Error('Failed to load keys')
106+
const data = await res.json()
107+
return data.keys || []
108+
}
109+
79110
export function APIKeysPage() {
80111
const { session } = useAuth()
81-
const [keys, setKeys] = useState<APIKey[]>([])
82-
const [loading, setLoading] = useState(true)
112+
const queryClient = useQueryClient()
83113
const [generateOpen, setGenerateOpen] = useState(false)
84114
const [keyName, setKeyName] = useState('')
85115
const [generating, setGenerating] = useState(false)
86116
const [generatedKey, setGeneratedKey] = useState<string | null>(null)
87117
const [revoking, setRevoking] = useState<string | null>(null)
118+
const [revokeConfirm, setRevokeConfirm] = useState<APIKey | null>(null)
88119

89-
const token = session?.access_token
90-
91-
const fetchKeys = useCallback(async () => {
92-
if (!token) return
93-
try {
94-
const res = await fetch(`${API_URL}/keys`, {
95-
headers: { Authorization: `Bearer ${token}` },
96-
})
97-
if (!res.ok) throw new Error('Failed to load keys')
98-
const data = await res.json()
99-
setKeys(data.keys || [])
100-
} catch {
101-
toast.error('Failed to load API keys')
102-
} finally {
103-
setLoading(false)
104-
}
105-
}, [token])
120+
const token = session?.access_token || ''
106121

107-
useEffect(() => {
108-
fetchKeys()
109-
}, [fetchKeys])
122+
const { data: keys = [], isLoading } = useQuery({
123+
queryKey: ['api-keys'],
124+
queryFn: () => fetchKeys(token),
125+
enabled: !!token,
126+
})
110127

111128
const handleGenerate = async () => {
112129
if (!token || !keyName.trim()) return
@@ -127,25 +144,26 @@ export function APIKeysPage() {
127144
const data = await res.json()
128145
setGeneratedKey(data.api_key)
129146
setKeyName('')
130-
fetchKeys()
147+
queryClient.invalidateQueries({ queryKey: ['api-keys'] })
131148
} catch (e) {
132149
toast.error(e instanceof Error ? e.message : 'Failed to generate key')
133150
} finally {
134151
setGenerating(false)
135152
}
136153
}
137154

138-
const handleRevoke = async (keyId: string) => {
155+
const handleRevoke = async (key: APIKey) => {
139156
if (!token) return
140-
setRevoking(keyId)
157+
setRevoking(key.id)
158+
setRevokeConfirm(null)
141159
try {
142-
const res = await fetch(`${API_URL}/keys/${keyId}`, {
160+
const res = await fetch(`${API_URL}/keys/${key.id}`, {
143161
method: 'DELETE',
144162
headers: { Authorization: `Bearer ${token}` },
145163
})
146164
if (!res.ok) throw new Error('Failed to revoke key')
147165
toast.success('API key revoked')
148-
fetchKeys()
166+
queryClient.invalidateQueries({ queryKey: ['api-keys'] })
149167
} catch {
150168
toast.error('Failed to revoke key')
151169
} finally {
@@ -160,9 +178,9 @@ export function APIKeysPage() {
160178
}
161179

162180
const activeKeys = keys.filter((k) => k.active)
163-
const revokedKeys = keys.filter((k) => !k.active)
181+
const configPaths = getConfigPaths()
164182

165-
if (loading) {
183+
if (isLoading) {
166184
return (
167185
<div className="flex items-center justify-center min-h-[300px] text-muted-foreground">
168186
<Loader2 className="w-5 h-5 animate-spin mr-2" />
@@ -252,7 +270,7 @@ export function APIKeysPage() {
252270
<Button
253271
variant="ghost"
254272
size="sm"
255-
onClick={() => handleRevoke(key.id)}
273+
onClick={() => setRevokeConfirm(key)}
256274
disabled={revoking === key.id}
257275
className="text-destructive hover:text-destructive hover:bg-destructive/10"
258276
>
@@ -277,9 +295,11 @@ export function APIKeysPage() {
277295
<p className="text-sm text-muted-foreground">
278296
<span className="font-medium text-foreground">Quick setup:</span>{' '}
279297
Copy your key and add it to your Claude Desktop config at{' '}
280-
<code className="text-xs bg-muted px-1 py-0.5 rounded">
281-
~/Library/Application Support/Claude/claude_desktop_config.json
282-
</code>
298+
{configPaths.map((cp) => (
299+
<span key={cp.label}>
300+
<code className="text-xs bg-muted px-1 py-0.5 rounded">{cp.path}</code>
301+
</span>
302+
))}
283303
</p>
284304
</CardContent>
285305
</Card>
@@ -345,6 +365,30 @@ export function APIKeysPage() {
345365
)}
346366
</DialogContent>
347367
</Dialog>
368+
369+
{/* Revoke confirmation dialog */}
370+
<Dialog open={!!revokeConfirm} onOpenChange={() => setRevokeConfirm(null)}>
371+
<DialogContent className="sm:max-w-sm">
372+
<DialogHeader>
373+
<DialogTitle>Revoke API Key</DialogTitle>
374+
<DialogDescription>
375+
Are you sure you want to revoke <span className="font-medium text-foreground">{revokeConfirm?.name}</span>?
376+
Any applications using this key will stop working immediately.
377+
</DialogDescription>
378+
</DialogHeader>
379+
<DialogFooter>
380+
<Button variant="outline" onClick={() => setRevokeConfirm(null)}>
381+
Cancel
382+
</Button>
383+
<Button
384+
variant="destructive"
385+
onClick={() => revokeConfirm && handleRevoke(revokeConfirm)}
386+
>
387+
Revoke Key
388+
</Button>
389+
</DialogFooter>
390+
</DialogContent>
391+
</Dialog>
348392
</div>
349393
)
350394
}

0 commit comments

Comments
 (0)