feat(admin): add ComfyNode management interface with policy controls#155
feat(admin): add ComfyNode management interface with policy controls#155snomiao wants to merge 20 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds a comprehensive ComfyNode management interface to the admin dashboard, providing functionality to search, view, and edit ComfyNode properties including policy controls and feature flags. The implementation includes complete internationalization support across all supported languages and API updates to support the new ComfyNode management features.
- Adds dedicated ComfyNode management page with search and editing capabilities
- Implements comprehensive policy management system (Active/Banned/Local Only)
- Adds support for deprecated and experimental flags with visual badges
- Updates translation files for all supported languages (EN, ES, FR, JA, KO, ZH)
Reviewed Changes
Copilot reviewed 11 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/api/generated.ts | Generated API client with new ComfyNode interfaces, policies, and update endpoints |
| pages/admin/comfy-nodes.tsx | New admin page implementing ComfyNode management interface with search and edit modal |
| pages/admin/index.tsx | Adds navigation link to ComfyNode management from admin dashboard |
| public/locales/*/common.json | Translation updates for ComfyNode management UI across all languages |
| package.json | Updates orval dependency version |
| orval.config.ts | Improves configuration for API generation |
🎨 Chromatic Visual Testing Results
Check the visual changes and approve or request changes as needed. |
🎨 Chromatic Visual Testing Results
Check the visual changes and approve or request changes as needed. |
snomiao
force-pushed
the
sno-per-comfy-node-policy
branch
from
775b584 to
caaa952
Compare
|
Working on fixing the Spanish translation issue for 'Local Only': Todo List:
✅ COMPLETED!Summary of Changes:
Files Changed:
Commit: 1813845 - Spanish translation fixes pushed to branch ✅ |
|
Starting rebase to origin/main as requested. Rebase Progress
Currently resolving additional merge conflicts in configuration files... |
snomiao
force-pushed
the
sno-per-comfy-node-policy
branch
from
1813845 to
d9a90bc
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 15 out of 18 changed files in this pull request and generated 4 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
- tr: Remove extraneous "(Ş)" suffix from "Output Is List" translation - ru: Fix "Return Names" using same text as "Return Types" (типы -> имена) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…efetch - Guard against empty nodeId/version in ComfyNodeEditModal submit handler - Remove redundant refetch() call since invalidateQueries already refreshes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
pages/admin/index.tsx:20
AdminTreeNavigationis imported but no longer rendered, anduseRouter()/routeris declared but unused. With Next's default ESLint config this will fail lint (no-unused-vars). Remove these imports/variables or reintroduce the navigation usage.
import withAdmin from "@/components/common/HOC/authAdmin";
import AdminTreeNavigation from "@/components/admin/AdminTreeNavigation";
import { useNextTranslation } from "@/src/hooks/i18n";
import { Breadcrumb } from "flowbite-react";
import Link from "next/link";
import { useRouter } from "next/router";
import {
HiHome,
HiOutlineAdjustments,
HiOutlineClipboardCheck,
HiOutlineCog,
HiOutlineCollection,
HiOutlineDuplicate,
HiOutlineSupport,
} from "react-icons/hi";
export default withAdmin(AdminDashboard);
function AdminDashboard() {
const router = useRouter();
const { t } = useNextTranslation();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const { data, isLoading } = useListAllComfyNodes({ | ||
| node_id: searchNodeId || undefined, | ||
| node_version: searchVersion || undefined, | ||
| comfy_node_name: searchComfyNodeName || undefined, | ||
| pageSize: pageSize, |
There was a problem hiding this comment.
useListAllComfyNodes errors aren't handled. If the request fails, isLoading becomes false and data stays undefined, so the UI can fall through to empty-state messaging instead of indicating an error. Capture error/isError from the hook and render an error state (or toast) when the query fails.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
pages/admin/index.tsx:20
AdminTreeNavigationanduseRouter(and theroutervariable) are imported/created but never used after the dashboard layout refactor. This will trigger lint/typecheck warnings; please remove the unused imports and the unusedroutervariable (or reintroduce the intended usage).
import withAdmin from "@/components/common/HOC/authAdmin";
import { useNextTranslation } from "@/src/hooks/i18n";
import { Breadcrumb } from "flowbite-react";
import Link from "next/link";
import {
HiHome,
HiOutlineAdjustments,
HiOutlineClipboardCheck,
HiOutlineCog,
HiOutlineCollection,
HiOutlineDuplicate,
HiOutlineSupport,
} from "react-icons/hi";
export default withAdmin(AdminDashboard);
function AdminDashboard() {
const { t } = useNextTranslation();
return (
<div className="p-4">
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }; | ||
|
|
||
| const handleClose = () => { | ||
| reset(); |
There was a problem hiding this comment.
handleClose calls reset() with no values, which resets the form back to the initial defaultValues from the first render (often empty strings). Since selectedComfyNode is not cleared on close, reopening the modal for the same node can leave the form blank (and potentially allow saving blank fields). Consider either (1) calling reset() with the current comfyNode values, or (2) clearing selectedComfyNode on close / conditionally rendering the modal so it remounts per selection.
| reset(); | |
| if (comfyNode) { | |
| reset({ | |
| category: comfyNode.category || "", | |
| description: comfyNode.description || "", | |
| function: comfyNode.function || "", | |
| input_types: comfyNode.input_types || "", | |
| return_names: comfyNode.return_names || "", | |
| return_types: comfyNode.return_types || "", | |
| output_is_list: comfyNode.output_is_list?.join(", ") || "", | |
| deprecated: comfyNode.deprecated || false, | |
| experimental: comfyNode.experimental || false, | |
| policy: comfyNode.policy || ComfyNodePolicy.ComfyNodePolicyActive, | |
| }); | |
| } else { | |
| reset(); | |
| } |
| <form onSubmit={handleSearch} className="flex gap-2 items-center mb-6"> | ||
| <TextInput | ||
| placeholder={t("Node ID (optional)")} | ||
| value={nodeIdFilter} | ||
| onChange={(e) => setNodeIdFilter(e.target.value)} | ||
| /> | ||
| <TextInput | ||
| placeholder={t("Version (optional)")} | ||
| value={versionFilter} | ||
| onChange={(e) => setVersionFilter(e.target.value)} | ||
| /> | ||
| <TextInput | ||
| placeholder={t("ComfyNode Name (optional)")} | ||
| value={comfyNodeNameFilter} | ||
| onChange={(e) => setComfyNodeNameFilter(e.target.value)} | ||
| /> |
There was a problem hiding this comment.
The three search TextInputs don’t have id/name (or an explicit <Label>/aria-label). Other admin pages typically provide id/name and/or a label for form controls. Adding identifiers (or labels) will improve accessibility and makes the form easier to test/automate.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return_types: data.return_types, | ||
| output_is_list: data.output_is_list | ||
| ? data.output_is_list.split(",").map((s) => s.trim() === "true") | ||
| : undefined, |
There was a problem hiding this comment.
output_is_list parsing will treat any non-empty (even whitespace) input as a list and convert any value other than the literal string "true" into false (e.g., " ", "false", "TRUE", typos). This can silently overwrite existing output_is_list data with incorrect booleans. Consider trimming, filtering out empty segments, and validating that each token is exactly "true" or "false" (and showing a user-facing error) before sending the update; if the field is left blank, avoid sending output_is_list at all.
| parameters: { | ||
| layout: "fullscreen", | ||
| nextjs: { | ||
| appDirectory: true, |
There was a problem hiding this comment.
This story sets parameters.nextjs.appDirectory: true, but it renders a pages/ route component and mocks next/router. Using the App Router setting here can cause Storybook’s Next.js addon to assume next/navigation APIs and break routing-related behavior. Consider removing this parameter or setting appDirectory: false for this story.
| appDirectory: true, | |
| appDirectory: false, |
- pages/_app.tsx: remove unused `request` (http) and `AxiosRequestConfig` imports, prefix unused `queryKey` param - components/nodes/AdminNodeClaimModal.tsx: remove unused `getGetNodeQueryKey` import, use bare catch block - components/nodes/AdminNodeClaimModal.stories.tsx: remove unused `Publisher` import and `samplePublishers` declaration - components/admin/NodeVersionCompatibilityEditModal.tsx: remove unused `AdminUpdateNodeVersionBody` import - components/publisher/PublisherDetail.tsx: remove unused `error` destructuring, prefix unused onError params - components/publisher/PublisherNodes.tsx: remove unused `isError`, `isLoading`, `index` vars - components/publisher/PublisherListNodes.tsx: remove unused `isError`, prefix unused onError param Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Response to Copilot Review CommentsAll review comments have been addressed in commit Fixed:
Note on previous comments:
|
|
@copilot please review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Loading state story - uses a QueryClient wrapper that keeps the query in pending state | ||
| const LoadingWrapper = () => { | ||
| const queryClient = new QueryClient({ | ||
| defaultOptions: { queries: { retry: false, staleTime: Infinity } }, | ||
| }); | ||
| return ( | ||
| <QueryClientProvider client={queryClient}> | ||
| <ComfyNodesManage /> | ||
| </QueryClientProvider> | ||
| ); | ||
| }; | ||
|
|
||
| export const Loading: Story = { | ||
| render: () => <LoadingWrapper />, | ||
| }; | ||
|
|
||
| // Empty state story - uses a QueryClient wrapper with empty pre-seeded results | ||
| const EmptyResultsWrapper = () => { | ||
| const queryClient = new QueryClient({ | ||
| defaultOptions: { queries: { retry: false } }, | ||
| }); | ||
| return ( | ||
| <QueryClientProvider client={queryClient}> | ||
| <ComfyNodesManage /> | ||
| </QueryClientProvider> | ||
| ); | ||
| }; | ||
|
|
||
| export const EmptyResults: Story = { | ||
| render: () => <EmptyResultsWrapper />, | ||
| }; |
There was a problem hiding this comment.
The useListAllComfyNodes hook is globally mocked to always return isLoading: false and data: mockApiResponse, so the Loading and EmptyResults stories can’t actually display those states (the different QueryClient options in the wrappers won’t affect the mocked hook). To make these stories meaningful, vary the mocked return per story (e.g., via MSW handlers + real hook, or a story-level override) so isLoading/data reflect the intended state.
| const { data, isLoading } = useListAllComfyNodes({ | ||
| node_id: searchNodeId || undefined, | ||
| node_version: searchVersion || undefined, | ||
| comfy_node_name: searchComfyNodeName || undefined, | ||
| pageSize: pageSize, | ||
| page: page, | ||
| }); | ||
|
|
There was a problem hiding this comment.
useListAllComfyNodes is executed unconditionally on first render (with only page/pageSize params), so the page will fetch and display results even before the user submits the search form. That makes the "Enter search criteria to find ComfyNodes" empty-state branch effectively unreachable and may cause an expensive "list all" call. If the intent is to require criteria before fetching, gate the query with enabled: Boolean(searchNodeId || searchVersion || searchComfyNodeName) (or only set the search* state after submit).
| const { data, isLoading } = useListAllComfyNodes({ | |
| node_id: searchNodeId || undefined, | |
| node_version: searchVersion || undefined, | |
| comfy_node_name: searchComfyNodeName || undefined, | |
| pageSize: pageSize, | |
| page: page, | |
| }); | |
| const hasSearchCriteria = Boolean( | |
| searchNodeId || searchVersion || searchComfyNodeName | |
| ); | |
| const { data, isLoading } = useListAllComfyNodes( | |
| { | |
| node_id: searchNodeId || undefined, | |
| node_version: searchVersion || undefined, | |
| comfy_node_name: searchComfyNodeName || undefined, | |
| pageSize: pageSize, | |
| page: page, | |
| }, | |
| { | |
| enabled: hasSearchCriteria, | |
| } | |
| ); |
| <Controller | ||
| name="deprecated" | ||
| control={control} | ||
| render={({ field }) => ( | ||
| <input | ||
| type="checkbox" | ||
| id="deprecated" | ||
| checked={field.value} | ||
| onChange={field.onChange} | ||
| className="rounded" | ||
| /> |
There was a problem hiding this comment.
The checkbox inputs inside Controller don’t spread field props onto the <input> (e.g., name, ref, onBlur). Other forms in the codebase spread ...field for controlled inputs; doing the same here helps ensure react-hook-form can correctly track touched/blur state and register the input consistently.
| return ( | ||
| <div> | ||
| <Button onClick={() => setIsOpen(true)}>Open Edit Modal</Button> | ||
| <ComfyNodeEditModal | ||
| isOpen={isOpen} | ||
| onClose={() => setIsOpen(false)} | ||
| comfyNode={comfyNode} | ||
| nodeId={nodeId} | ||
| version={version} | ||
| onSuccess={() => { | ||
| console.log("Edit successful!"); | ||
| setIsOpen(false); | ||
| }} | ||
| /> |
There was a problem hiding this comment.
This story renders ComfyNodeEditModal, which uses the real useUpdateComfyNode mutation. Without MSW handlers (or a dedicated mock of the generated API client), clicking "Save Changes" in Storybook will attempt a real network request to /nodes/:nodeId/versions/:version/comfy-nodes/:comfyNodeName. Add MSW handlers for the update endpoint (and any dependencies) so the story is deterministic and doesn’t depend on a live backend.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…heckbox field props - Add `enabled: Boolean(searchNodeId || searchVersion || searchComfyNodeName)` to useListAllComfyNodes so it doesn't fire an unconstrained list-all on first render - Spread name/ref/onBlur from Controller field onto checkbox inputs so react-hook-form can correctly track touched/blur state Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- ComfyNodeEditModal.stories: mock useUpdateComfyNode so Save Changes doesn't fire real network requests in Storybook - ComfyNodesManage.stories: use story-level beforeEach + vi.mocked() to make Loading and EmptyResults stories actually render those states (global vi.mock alone can't vary return values per story) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
locales/es/common.json:206
- This Spanish entry is still Chinese ("无法更新节点…") and should be translated to Spanish to avoid mixed-locale UI strings.
"Failed to update ComfyNode": "No se pudo actualizar ComfyNode",
"Failed to update node": "Error al actualizar el nodo",
"Failed to update node version": "Actualización de versión de nodo fallida",
"Failed to update node version: {{error}}": "No se pudo actualizar la versión del nodo: {{error}}",
"Failed to update node.\\n{{detail}}": "无法更新节点。\\n{{detail}}",
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const Loading: Story = { | ||
| beforeEach() { | ||
| vi.mocked(useListAllComfyNodes).mockReturnValue({ | ||
| data: undefined, | ||
| isLoading: true, | ||
| refetch: () => Promise.resolve({} as any), | ||
| } as any); |
There was a problem hiding this comment.
The story mocks useListAllComfyNodes via vi.mock(...) with a plain function export, but later calls vi.mocked(useListAllComfyNodes).mockReturnValue(...). Since the export isn’t a vi.fn(), this will fail at runtime and the Loading/EmptyResults variants won’t work. If you keep this approach, export useListAllComfyNodes as a vi.fn() from the mock factory (or switch to MSW + Storybook parameters like other stories in the repo).
There was a problem hiding this comment.
Fixed in commit babe3eb. The mock factory now uses fn() spies (mockUseListAllComfyNodes, mockUseUpdateComfyNode) declared before vi.mock(). Per-story beforeEach calls .mockReturnValue() directly on the spy reference, so Loading and EmptyResults stories now correctly render those states.
| // Mock the API hook | ||
| vi.mock("@/src/api/generated", () => ({ | ||
| ComfyNodePolicy: { | ||
| ComfyNodePolicyActive: "ComfyNodePolicyActive", | ||
| ComfyNodePolicyBanned: "ComfyNodePolicyBanned", | ||
| ComfyNodePolicyLocalOnly: "ComfyNodePolicyLocalOnly", | ||
| }, | ||
| useListAllComfyNodes: () => ({ | ||
| data: mockApiResponse, | ||
| isLoading: false, | ||
| refetch: () => {}, | ||
| }), | ||
| useUpdateComfyNode: () => ({ | ||
| mutateAsync: async () => {}, | ||
| isPending: false, | ||
| }), | ||
| })); | ||
|
|
||
| // Mock the router | ||
| vi.mock("next/router", () => ({ | ||
| useRouter: () => mockRouter, | ||
| })); | ||
|
|
||
| // Mock the auth HOC | ||
| vi.mock("@/components/common/HOC/authAdmin", () => ({ | ||
| default: (Component: any) => Component, | ||
| })); | ||
|
|
||
| // Mock the toast notifications | ||
| vi.mock("react-toastify", () => ({ | ||
| toast: { | ||
| success: () => {}, |
There was a problem hiding this comment.
These Storybook stories are using vitest module mocking (vi.mock(...)), including mocking next/router. In this repo, Storybook stories typically mock routing via parameters.nextjs and API calls via MSW handlers (see e.g. components/pages/claim-node/ClaimNodePage.stories.tsx). Consider refactoring to that pattern and avoid importing vitest into stories, since it can break the Storybook browser bundle and makes stories harder to maintain.
There was a problem hiding this comment.
Updated in commit babe3eb. Switched to fn() from @storybook/test for spy instances, declared outside vi.mock() so per-story beforeEach can call .mockReturnValue() directly without vi.mocked(). The vi.mock module replacement remains (necessary for hook interception); it works in Vite-based Storybook since they share the same module system.
| import { vi } from "vitest"; | ||
| // Import the modal component directly from the page file | ||
| import { ComfyNodeEditModal } from "@/pages/admin/comfy-nodes"; | ||
| import { ComfyNode, ComfyNodePolicy } from "@/src/api/generated"; | ||
|
|
||
| // Mock the API hook so "Save Changes" doesn't make real network requests | ||
| vi.mock("@/src/api/generated", async (importOriginal) => { | ||
| const actual = await importOriginal<typeof import("@/src/api/generated")>(); | ||
| return { | ||
| ...actual, | ||
| useUpdateComfyNode: () => ({ | ||
| mutateAsync: async () => {}, | ||
| isPending: false, | ||
| }), | ||
| }; | ||
| }); | ||
|
|
There was a problem hiding this comment.
This story imports vi from vitest and uses vi.mock(...) to override useUpdateComfyNode. In this repo, Storybook mocking is generally done via @storybook/test (fn) and MSW rather than Vitest module mocks, which may not work reliably in the Storybook browser runtime. Consider switching to Storybook-friendly mocks (e.g., exporting a *.mock.ts wrapper for the hook or using MSW to stub the update endpoint).
| import { vi } from "vitest"; | |
| // Import the modal component directly from the page file | |
| import { ComfyNodeEditModal } from "@/pages/admin/comfy-nodes"; | |
| import { ComfyNode, ComfyNodePolicy } from "@/src/api/generated"; | |
| // Mock the API hook so "Save Changes" doesn't make real network requests | |
| vi.mock("@/src/api/generated", async (importOriginal) => { | |
| const actual = await importOriginal<typeof import("@/src/api/generated")>(); | |
| return { | |
| ...actual, | |
| useUpdateComfyNode: () => ({ | |
| mutateAsync: async () => {}, | |
| isPending: false, | |
| }), | |
| }; | |
| }); | |
| // Import the modal component directly from the page file | |
| import { ComfyNodeEditModal } from "@/pages/admin/comfy-nodes"; | |
| import { ComfyNode, ComfyNodePolicy } from "@/src/api/generated"; |
There was a problem hiding this comment.
Updated in commit babe3eb. Now uses fn() from @storybook/test for the spy (mockMutateAsync and useUpdateComfyNode). vi.mock is still used for module replacement, which works reliably in Vite-based Storybook (@storybook/nextjs-vite) since Storybook and Vitest share the same Vite module system.
| "Failed to delete publisher": "Error al eliminar el editor", | ||
| "Failed to delete publisher. {{message}}": "删除发布者失败。{{message}}", | ||
| "Failed to delete version": "Error al eliminar la versión", | ||
| "Failed to delete version: {{message}}": "无法删除版本：{{message}}", | ||
| "Failed to delete version: {{message}}": "Error al eliminar la versión: {{message}}", |
There was a problem hiding this comment.
The Spanish translation for this key is still Chinese ("删除发布者失败…"), which looks like the issue this PR is trying to fix for ES. Please replace it with a Spanish translation.
There was a problem hiding this comment.
Fixed in commit 109ba00. All 4 remaining Chinese-character values in locales/es/common.json have been replaced with proper Spanish translations: 'Bulk Update Supported Versions', 'Failed to delete publisher', 'Failed to update node', and 'This should only take a moment'.
- Bulk Update Supported Versions: 批量... → Actualización masiva... - Failed to delete publisher: 删除... → Error al eliminar... - Failed to update node: 无法... → Error al actualizar... - This should only take a moment: 这应该... → Esto solo debería... Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tory mocking - Use fn() from @storybook/test instead of vi.fn() for spy instances, aligning with Storybook conventions - Declare spy instances before vi.mock() so per-story beforeEach can call .mockReturnValue() directly without vi.mocked() (which requires vi.fn()) - ComfyNodeEditModal: expose mockMutateAsync as fn() spy for testability Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const { data, isLoading } = useListAllComfyNodes( | ||
| { |
There was a problem hiding this comment.
The useListAllComfyNodes query result only uses data/isLoading. If the request errors, this page will currently fall through to the “No ComfyNodes found…” empty-state, which is misleading. Consider handling isError/error from the query result and rendering an explicit error state (and/or toast) consistent with other admin pages (e.g. claim-nodes.tsx).
| vi.mock("@/src/api/generated", () => ({ | ||
| ComfyNodePolicy: { | ||
| ComfyNodePolicyActive: "ComfyNodePolicyActive", | ||
| ComfyNodePolicyBanned: "ComfyNodePolicyBanned", | ||
| ComfyNodePolicyLocalOnly: "ComfyNodePolicyLocalOnly", | ||
| }, | ||
| useListAllComfyNodes: mockUseListAllComfyNodes, | ||
| useUpdateComfyNode: mockUseUpdateComfyNode, | ||
| })); |
There was a problem hiding this comment.
This story uses global vi.mock() module mocks for @/src/api/generated. In Storybook, module mocks are shared across stories and can leak into unrelated stories (since the module graph is cached), causing hard-to-debug cross-story coupling. Prefer per-story MSW handlers for API responses, or Storybook’s parameters/dependency injection patterns so the mocked behavior is scoped to the story.
| parameters: { | ||
| layout: "fullscreen", | ||
| nextjs: { | ||
| appDirectory: true, |
There was a problem hiding this comment.
parameters.nextjs.appDirectory is set to true, but this story renders a /pages route component (pages/admin/comfy-nodes.tsx). In this repo, /pages stories use appDirectory: false and configure routing via parameters.nextjs.router/navigation rather than next/router module mocks (see ClaimNodePage.stories.tsx). Using appDirectory: true here can lead to incorrect Next router emulation.
| appDirectory: true, | |
| appDirectory: false, |
| import { vi } from "vitest"; | ||
| // Import the modal component directly from the page file | ||
| import { ComfyNodeEditModal } from "@/pages/admin/comfy-nodes"; | ||
| import { ComfyNode, ComfyNodePolicy } from "@/src/api/generated"; | ||
|
|
||
| const mockMutateAsync = fn(async () => {}); | ||
|
|
||
| // Mock useUpdateComfyNode so "Save Changes" doesn't fire real network requests | ||
| vi.mock("@/src/api/generated", async (importOriginal) => { | ||
| const actual = await importOriginal<typeof import("@/src/api/generated")>(); | ||
| return { | ||
| ...actual, | ||
| useUpdateComfyNode: fn(() => ({ | ||
| mutateAsync: mockMutateAsync, | ||
| isPending: false, | ||
| })), | ||
| }; | ||
| }); | ||
|
|
There was a problem hiding this comment.
This story file globally mocks @/src/api/generated to override useUpdateComfyNode. As with other vi.mock() usage in Storybook, this can leak into other stories that import the generated API module. Prefer scoping the behavior via MSW handlers (mocking the underlying HTTP call) or story parameters so other stories aren’t affected.
| import { vi } from "vitest"; | |
| // Import the modal component directly from the page file | |
| import { ComfyNodeEditModal } from "@/pages/admin/comfy-nodes"; | |
| import { ComfyNode, ComfyNodePolicy } from "@/src/api/generated"; | |
| const mockMutateAsync = fn(async () => {}); | |
| // Mock useUpdateComfyNode so "Save Changes" doesn't fire real network requests | |
| vi.mock("@/src/api/generated", async (importOriginal) => { | |
| const actual = await importOriginal<typeof import("@/src/api/generated")>(); | |
| return { | |
| ...actual, | |
| useUpdateComfyNode: fn(() => ({ | |
| mutateAsync: mockMutateAsync, | |
| isPending: false, | |
| })), | |
| }; | |
| }); | |
| // Import the modal component directly from the page file | |
| import { ComfyNodeEditModal } from "@/pages/admin/comfy-nodes"; | |
| import { ComfyNode, ComfyNodePolicy } from "@/src/api/generated"; |
| onEdit: (comfyNode) => { | ||
| console.log("Edit clicked for:", comfyNode.comfy_node_name); | ||
| }, |
There was a problem hiding this comment.
Using console.log in story args will spam the browser/test runner console and can make Storybook/Vitest output noisy. Prefer Storybook actions (or fn() from @storybook/test) for onEdit so interactions are observable without logging.
… error Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Review Comment Responses✅ Fixed: Chinese characters in French/Spanish locales
✅ Already fixed: Unused variables/imports
✅ False alarm: Google Analytics removed (comment #2762649449)GA is still present in ✅ False alarm: Admin JWT auth removed (comment #2762649474)The admin JWT interceptor is fully present in ✅ False alarm: CSS conflicts in NodesCard / UnclaimedNodeCard (comments #2762649491, #2762649505, #2762649518, #2762649541, #2762649556)The current code has clean CSS — ✅ False alarm: jest.mock in Storybook (comments #2687329470, #2687329545)The stories use ✅ Already addressed: Non-null assertions on policy (comments #2200882476, #2200882481)Current code already uses null-safe checks: ✅ Already addressed: nodeId/version from search state (comment #2687329199)
|
Summary
Adds a dedicated admin page at `/admin/comfy-nodes` to manage ComfyNode policies and feature flags using the new comfy-node-policy API.
Changes
New admin page (`pages/admin/comfy-nodes.tsx`):
Admin navigation (`pages/admin/index.tsx`):
Storybook stories (4 new story files):
i18n: 29 new translation keys across all 9 locales
Key commits
Test plan
🤖 Generated with Claude Code