feat(admin): Add admin permission check functionality #1838
Conversation
juanlou1217
commented
Jan 24, 2026
juanlou1217
commented
- 新增 useAdminCheck 、useAdminList hook，用于检查当前用户是否为管理员与管理员列表
- 在 MenuPicker 组件中集成管理员权限检查
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Pull request overview
Adds admin permission querying hooks and uses them to gate the Sidebar “Menu Order” settings UI so it only renders for admins.
Changes:
- Added
useAdminCheckhook for checking whether the current user is an admin. - Added
useAdminListhook for fetching the admin list. - Integrated admin check into
MenuPickerto hide the menu-order editor for non-admin users.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| moon/apps/web/hooks/admin/useAdminList.ts | New React Query hook to fetch admin list data. |
| moon/apps/web/hooks/admin/useAdminCheck.ts | New React Query hook to fetch “am I admin” status. |
| moon/apps/web/components/Sidebar/SidebarMenu/MenuPicker.tsx | Uses admin check to conditionally render the Menu Order UI. |
| staleTime: 0, | ||
| retry: false |
There was a problem hiding this comment.
Same as useAdminCheck: retry: false overrides the app-wide retry policy (which already prevents retries for 4xx) and can cause avoidable failures on transient errors. Consider relying on the default retry strategy unless you have a concrete reason to disable retries for this endpoint.
| staleTime: 0, | |
| retry: false | |
| staleTime: 0 |
| export const MenuPicker = () => { | ||
| const { data: adminCheck } = useAdminCheck() | ||
| const { data, refetch, isFetching } = useGetSidebarList() | ||
| const queryClient = useQueryClient() |
There was a problem hiding this comment.
useGetSidebarList() is still executed for non-admin users because hooks run before the early return null. This defeats the purpose of the admin gate and can trigger unnecessary/unauthorized requests. Consider adding an enabled option to useGetSidebarList (or wrapping it with another hook) and pass enabled: !!adminCheck?.data?.is_admin so the sidebar list query only runs once admin permission is confirmed.
|
|
||
| import { legacyApiClient } from '@/utils/queryClient' | ||
|
|
||
| export function useAdminCheck(params?: RequestParams) { |
There was a problem hiding this comment.
The name useAdminCheck reads like it returns a boolean, but it actually returns a React Query result object. To avoid confusion for callers, consider renaming to something like useGetAdminMe/useAdminMeQuery, or alternatively wrap it in a useIsAdmin() hook that returns a boolean.
| export function useAdminCheck(params?: RequestParams) { | |
| export function useAdminMeQuery(params?: RequestParams) { |
| queryFn: () => legacyApiClient.v1.getApiAdminMe().request(params), | ||
| staleTime: 0, | ||
| retry: false |
There was a problem hiding this comment.
Setting retry: false here bypasses the app-wide React Query retry policy defined in utils/queryClient.ts (which already avoids retrying 4xx errors). This makes the admin check more fragile on transient network/5xx failures. Consider removing retry: false (and the redundant staleTime: 0) unless there’s a specific reason to override defaults.
| queryFn: () => legacyApiClient.v1.getApiAdminMe().request(params), | |
| staleTime: 0, | |
| retry: false | |
| queryFn: () => legacyApiClient.v1.getApiAdminMe().request(params) |
|
|
||
| import { legacyApiClient } from '@/utils/queryClient' | ||
|
|
||
| export function useAdminList(params?: RequestParams) { |
There was a problem hiding this comment.
For consistency with the rest of the codebase’s query hooks (e.g. useGetSidebarList, useGetMergeQueueList), consider renaming useAdminList to useGetAdminList (or similar). The current name can be mistaken for a selector that returns an array rather than a React Query result object.
| export function useAdminList(params?: RequestParams) { | |
| export function useGetAdminList(params?: RequestParams) { |
| return useQuery<GetApiAdminMeData, Error>({ | ||
| queryKey: [...legacyApiClient.v1.getApiAdminMe().requestKey(), params], | ||
| queryFn: () => legacyApiClient.v1.getApiAdminMe().request(params), | ||
| staleTime: 0, | ||
| retry: false | ||
| }) |
There was a problem hiding this comment.
retry: false disables the app-wide React Query retry policy (see moon/apps/web/utils/queryClient.ts:107-138), which currently avoids retries for 4xx but retries transient failures up to 3 times. Consider removing this override (or using the shared retry function) so admin checks are resilient to temporary network/API hiccups.
| return useQuery<GetApiAdminListData, Error>({ | ||
| queryKey: [...legacyApiClient.v1.getApiAdminList().requestKey(), params], | ||
| queryFn: () => legacyApiClient.v1.getApiAdminList().request(params), | ||
| staleTime: 0, | ||
| retry: false | ||
| }) |
There was a problem hiding this comment.
retry: false here overrides the global React Query retry behavior configured in moon/apps/web/utils/queryClient.ts:107-138 (no retry for 4xx, retry transient errors up to 3x). Unless there’s a specific reason to never retry this request, consider relying on the shared retry policy for consistency and resilience.
2 participants
