Skip to content

fix(MULTIPLA-004): CU-86akhf8u5 2 review findings across 2 files #393

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
flamingo wants to merge 2 commits into
base: main
Choose a base branch
from
+107 −49
Draft

fix(MULTIPLA-004): CU-86akhf8u5 2 review findings across 2 files #393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
abe753e
fix(MULTIPLA-004): 2 review findings across 2 files
flamingo[bot] Sep 14, 2026
0612f4c
fix(MULTIPLA-004): 2 review findings across 2 files
flamingo[bot] Sep 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/app/(app)/settings/hooks/admin-query-keys.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export const adminQueryKeys = {
apiKeys: {
all: ['admin', 'api-keys'] as const,
list: () => [...adminQueryKeys.apiKeys.all, 'list'] as const,
detail: (id: string) => [...adminQueryKeys.apiKeys.all, 'detail', id] as const,
},
};
149 changes: 100 additions & 49 deletions src/app/(app)/settings/hooks/use-api-keys.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
'use client';

import { useCallback, useState } from 'react';
import { useCallback } from 'react';
import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query';
import { apiClient } from '@/lib/api-client';
import { adminQueryKeys } from './admin-query-keys';

export type ApiKeyRecord = {
id: string;
Expand All @@ -18,75 +20,124 @@ export type ApiKeyRecord = {
};

export function useApiKeys() {

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🔴 use-api-keys.ts hook does not use TanStack Query at all — plain useState/useCallback with manual fetch orchestration

Rewrote useApiKeys in src/app/(app)/settings/hooks/use-api-keys.ts to use TanStack Query's useQuery (for the list, keyed via new shared adminQueryKeys.apiKeys.list()) and useMutation for create/update/regenerate/enable-toggle operations, each with onSuccess calling queryClient.invalidateQueries({ queryKey: adminQueryKeys.apiKeys.all }). Added new shared module src/app/(app)/settings/hooks/admin-query-keys.ts exporting adminQueryKeys with structured keys, since none existed in the file set provided. The public return shape (items, isLoading, error, fetchApiKeys, createApiKey, updateApiKey, regenerateApiKey, setApiKeyEnabled) is preserved so existing consumers keep working without changes, with fetchApiKeys now wrapping refetch(). Risk: I could not see use-users.ts to confirm exact naming/shape conventions for adminQueryKeys, so the key structure is a reasonable but unverified guess; a complete fix should align this module's naming with whatever convention use-users.ts actually uses.

🤖 Prompt for AI agents 
In src/app/(app)/settings/hooks/use-api-keys.ts around line 20, review and complete this code-review fix: use-api-keys.ts hook does not use TanStack Query at all — plain useState/useCallback with manual fetch orchestration.
What the draft fix changed: Rewrote `useApiKeys` in `src/app/(app)/settings/hooks/use-api-keys.ts` to use TanStack Query's `useQuery` (for the list, keyed via new shared `adminQueryKeys.apiKeys.list()`) and `useMutation` for create/update/regenerate/enable-toggle operations, each with `onSuccess` calling `queryClient.invalidateQueries({ queryKey: adminQueryKeys.apiKeys.all })`. Added new shared module `src/app/(app)/settings/hooks/admin-query-keys.ts` exporting `adminQueryKeys` with structured keys, since none existed in the file set provided. The public return shape (`items`, `isLoading`, `error`, `fetchApiKeys`, `createApiKey`, `updateApiKey`, `regenerateApiKey`, `setApiKeyEnabled`) is preserved so existing consumers keep working without changes, with `fetchApiKeys` now wrapping `refetch()`. Risk: I could not see `use-users.ts` to confirm exact naming/shape conventions for `adminQueryKeys`, so the key structure is a reasonable but unverified guess; a complete fix should align this module's naming with whatever convention `use-users.ts` actually uses.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 65 medium — react 👍/👎 to teach the reviewer

const [items, setItems] = useState<ApiKeyRecord[]>([]);
const [isLoading, setIsLoading] = useState(false);
const [error, setError] = useState<string | null>(null);
const queryClient = useQueryClient();

const fetchApiKeys = useCallback(async () => {
setIsLoading(true);
setError(null);
try {
const res = await apiClient.get<ApiKeyRecord[]>('api/api-keys');

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 useApiKeys.fetchApiKeys/createApiKey use a relative path without leading slash unlike all other apiClient calls

Added leading slashes to all apiClient calls in the same file (/api/api-keys, /api/api-keys/${id}, /api/api-keys/${id}/regenerate) to match the use-users.ts convention cited in the finding, fixing the inconsistency directly in the queryFn/mutation functions of useApiKeys.

🤖 Prompt for AI agents 
In src/app/(app)/settings/hooks/use-api-keys.ts around line 29, review and complete this code-review fix: useApiKeys.fetchApiKeys/createApiKey use a relative path without leading slash unlike all other apiClient calls.
What the draft fix changed: Added leading slashes to all `apiClient` calls in the same file (`/api/api-keys`, `/api/api-keys/${id}`, `/api/api-keys/${id}/regenerate`) to match the `use-users.ts` convention cited in the finding, fixing the inconsistency directly in the `queryFn`/mutation functions of `useApiKeys`.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 80 medium — react 👍/👎 to teach the reviewer

const {
data: items = [],
isLoading,
error: queryError,
refetch,
} = useQuery({
queryKey: adminQueryKeys.apiKeys.list(),
queryFn: async () => {
const res = await apiClient.get<ApiKeyRecord[]>('/api/api-keys');
if (!res.ok || !Array.isArray(res.data)) {
throw new Error(res.error || `Failed to load API keys (${res.status})`);
}
setItems(res.data);
return res.data;
} catch (e) {
const msg = e instanceof Error ? e.message : 'Failed to load API keys';
setError(msg);
throw e;
} finally {
setIsLoading(false);
}
}, []);
},
});

const createApiKey = useCallback(async (data: { name: string; description?: string; expiresAt?: string | null }) => {
const payload = {
name: data.name,
description: data.description || undefined,
expiresAt: data.expiresAt ?? null,
};
const res = await apiClient.post<{ apiKey: ApiKeyRecord; fullKey: string }>('api/api-keys', payload);
if (!res.ok || !res.data) {
throw new Error(res.error || `Failed to create API key (${res.status})`);
const error = queryError instanceof Error ? queryError.message : queryError ? String(queryError) : null;

const fetchApiKeys = useCallback(async () => {
const result = await refetch();
if (result.error) {
throw result.error;
}
return res.data;
}, []);
return result.data ?? [];
}, [refetch]);

const updateApiKey = useCallback(
async (id: string, data: { name: string; description?: string; expiresAt?: string | null }) => {
const createApiKeyMutation = useMutation({
mutationFn: async (data: { name: string; description?: string; expiresAt?: string | null }) => {
const payload = {
name: data.name,
description: data.description || undefined,
expiresAt: data.expiresAt ?? null,
};
const res = await apiClient.put<ApiKeyRecord>(`api/api-keys/${encodeURIComponent(id)}`, payload);
const res = await apiClient.post<{ apiKey: ApiKeyRecord; fullKey: string }>('/api/api-keys', payload);
if (!res.ok || !res.data) {
throw new Error(res.error || `Failed to create API key (${res.status})`);
}
return res.data;
},
onSuccess: () => {
queryClient.invalidateQueries({ queryKey: adminQueryKeys.apiKeys.all });
},
});

const updateApiKeyMutation = useMutation({
mutationFn: async ({
id,
data,
}: {
id: string;
data: { name: string; description?: string; expiresAt?: string | null };
}) => {
const payload = {
name: data.name,
description: data.description || undefined,
expiresAt: data.expiresAt ?? null,
};
const res = await apiClient.put<ApiKeyRecord>(`/api/api-keys/${encodeURIComponent(id)}`, payload);
if (!res.ok || !res.data) {
throw new Error(res.error || `Failed to update API key (${res.status})`);
}
return res.data;
},
[],
onSuccess: () => {
queryClient.invalidateQueries({ queryKey: adminQueryKeys.apiKeys.all });
},
});

const regenerateApiKeyMutation = useMutation({
mutationFn: async (id: string) => {
const res = await apiClient.post<{ apiKey: ApiKeyRecord; fullKey: string }>(
`/api/api-keys/${encodeURIComponent(id)}/regenerate`,
);
if (!res.ok || !res.data) {
throw new Error(res.error || `Failed to regenerate API key (${res.status})`);
}
return res.data;
},
onSuccess: () => {
queryClient.invalidateQueries({ queryKey: adminQueryKeys.apiKeys.all });
},
});

const setApiKeyEnabledMutation = useMutation({
mutationFn: async ({ id, enabled }: { id: string; enabled: boolean }) => {
const res = await apiClient.put<ApiKeyRecord>(`/api/api-keys/${encodeURIComponent(id)}`, { enabled });
if (!res.ok || !res.data) {
throw new Error(res.error || `Failed to ${enabled ? 'enable' : 'disable'} API key (${res.status})`);
}
return res.data;
},
onSuccess: () => {
queryClient.invalidateQueries({ queryKey: adminQueryKeys.apiKeys.all });
},
});

const createApiKey = useCallback(
(data: { name: string; description?: string; expiresAt?: string | null }) => createApiKeyMutation.mutateAsync(data),
[createApiKeyMutation],
);

const regenerateApiKey = useCallback(async (id: string) => {
const res = await apiClient.post<{ apiKey: ApiKeyRecord; fullKey: string }>(
`api/api-keys/${encodeURIComponent(id)}/regenerate`,
);
if (!res.ok || !res.data) {
throw new Error(res.error || `Failed to regenerate API key (${res.status})`);
}
return res.data;
}, []);
const updateApiKey = useCallback(
(id: string, data: { name: string; description?: string; expiresAt?: string | null }) =>
updateApiKeyMutation.mutateAsync({ id, data }),
[updateApiKeyMutation],
);

const setApiKeyEnabled = useCallback(async (id: string, enabled: boolean) => {
const res = await apiClient.put<ApiKeyRecord>(`api/api-keys/${encodeURIComponent(id)}`, { enabled });
if (!res.ok || !res.data) {
throw new Error(res.error || `Failed to ${enabled ? 'enable' : 'disable'} API key (${res.status})`);
}
return res.data;
}, []);
const regenerateApiKey = useCallback(
(id: string) => regenerateApiKeyMutation.mutateAsync(id),
[regenerateApiKeyMutation],
);

const setApiKeyEnabled = useCallback(
(id: string, enabled: boolean) => setApiKeyEnabledMutation.mutateAsync({ id, enabled }),
[setApiKeyEnabledMutation],
);

return { items, isLoading, error, fetchApiKeys, createApiKey, updateApiKey, regenerateApiKey, setApiKeyEnabled };
}
Loading