Add block document reference support to schema form#30
Conversation
WalkthroughThree new components are introduced for block document management: BlockDocumentCombobox for filtering and selecting block documents, BlockDocumentCreateDialog for creating new documents, and SchemaFormInputBlockDocument as an integration layer. Comprehensive tests and Storybook stories accompany these additions. SchemaFormInput is updated to utilize the new block document input component. Changes
Sequence DiagramsequenceDiagram
actor User
participant SchemaForm as SchemaForm<br/>(Parent)
participant SchemaFormInput as SchemaFormInput
participant BlockCombobox as BlockDocumentCombobox
participant BlockDialog as BlockDocumentCreateDialog
participant API as API
rect rgba(100, 150, 200, 0.5)
Note over User,API: Document Selection Flow
User->>BlockCombobox: Type to search
BlockCombobox->>API: Filter block documents
API-->>BlockCombobox: Filtered results
BlockCombobox->>BlockCombobox: Debounce & display
User->>BlockCombobox: Select document
BlockCombobox->>SchemaFormInput: Call onSelect
SchemaFormInput->>SchemaForm: Update value ($ref)
end
rect rgba(200, 150, 100, 0.5)
Note over User,API: Document Creation Flow
User->>BlockCombobox: Click "Create New"
BlockCombobox->>BlockDialog: Trigger open
BlockDialog->>API: Fetch block type & schemas
API-->>BlockDialog: Block type & schemas
BlockDialog->>BlockDialog: Render form
User->>BlockDialog: Fill form & submit
BlockDialog->>API: POST create document
API-->>BlockDialog: Created document
BlockDialog->>SchemaFormInput: Call onCreated
SchemaFormInput->>SchemaForm: Update value ($ref)
BlockDialog->>BlockDialog: Close dialog
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
`@ui-v2/src/components/blocks/block-document-combobox/block-document-combobox.tsx`:
- Around line 67-77: filteredData duplicates server-side filtering and
selectedBlockDocument is incorrectly derived from filteredData which makes a
previously selected item vanish when the search excludes it; change
selectedBlockDocument to search the full data array (use data.find by
selectedBlockDocumentId) and either remove the client-side filtering
(filteredData) or keep it only for listing results (but never use it to resolve
the selected item), ensuring references to filteredData, data, deferredSearch,
and selectedBlockDocumentId are updated accordingly.
- Around line 29-38: The Suspense wrapper in BlockDocumentCombobox currently has
no fallback, so add a meaningful fallback prop (e.g., a skeleton or spinner) to
the Suspense that renders while BlockDocumentComboboxImplementation is loading;
update the <Suspense> around BlockDocumentComboboxImplementation to include a
fallback component (a small loader/skeleton component you create or an existing
Spinner) to provide visual loading feedback.
In
`@ui-v2/src/components/blocks/block-document-create-dialog/block-document-create-dialog.tsx`:
- Around line 159-191: The onSave handler uses the component-level errors state
after awaiting validateForm, which causes a race because validateForm calls
setErrors asynchronously; update validateForm to return a validation result
(e.g., an array or boolean) or otherwise return the validated values/errors,
then modify onSave to await validateForm and use that returned result (instead
of checking the errors variable) to decide whether to abort; keep the existing
createBlockDocument call and success/error callbacks (symbols: onSave,
validateForm, setErrors, errors, createBlockDocument,
zodFormValues/BlockNameFormSchema) but branch on the returned validation result
to prevent submitting stale/invalid data.
In `@ui-v2/src/components/schemas/schema-form-input.tsx`:
- Around line 78-88: The code unsafely casts the unknown value to { $ref: string
} | undefined before passing it into SchemaFormInputBlockDocument; add a runtime
guard that checks typeof value === "object" && value !== null && typeof (value
as any).$ref === "string" and only then pass a typed object, otherwise pass
undefined (or an appropriate fallback) to
onValueChange/SchemaFormInputBlockDocument; update the block where blockTypeSlug
is handled (referencing property.blockTypeSlug, value,
SchemaFormInputBlockDocument, onValueChange, id) to compute a safeValue
validated at runtime instead of using the direct type assertion.
🧹 Nitpick comments (2)
ui-v2/src/components/schemas/schema-form-input-block-document.tsx (1)
28-34: Consider simplifyinghandleSelectwith a ternary expression.The conditional logic can be condensed for improved readability.
♻️ Optional simplification
const handleSelect = (blockDocumentId: string | undefined) => { - if (blockDocumentId) { - onValueChange({ $ref: blockDocumentId }); - } else { - onValueChange(undefined); - } + onValueChange(blockDocumentId ? { $ref: blockDocumentId } : undefined); };ui-v2/src/components/blocks/block-document-create-dialog/block-document-create-dialog.tsx (1)
179-183: Consider including error details in the toast message.The generic "Unknown error while creating block" message doesn't help users understand what went wrong. Consider extracting a meaningful message from the error object.
♻️ Suggested improvement
onError: (err) => { - const message = "Unknown error while creating block."; + const message = + err instanceof Error + ? `Failed to create block: ${err.message}` + : "Unknown error while creating block."; toast.error(message); console.error(message, err); },
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
ui-v2/src/components/blocks/block-document-combobox/block-document-combobox.stories.tsxui-v2/src/components/blocks/block-document-combobox/block-document-combobox.test.tsxui-v2/src/components/blocks/block-document-combobox/block-document-combobox.tsxui-v2/src/components/blocks/block-document-combobox/index.tsui-v2/src/components/blocks/block-document-create-dialog/block-document-create-dialog.stories.tsxui-v2/src/components/blocks/block-document-create-dialog/block-document-create-dialog.test.tsxui-v2/src/components/blocks/block-document-create-dialog/block-document-create-dialog.tsxui-v2/src/components/blocks/block-document-create-dialog/index.tsui-v2/src/components/schemas/schema-form-input-block-document.test.tsxui-v2/src/components/schemas/schema-form-input-block-document.tsxui-v2/src/components/schemas/schema-form-input.tsxui-v2/src/components/schemas/stories/properties.stories.tsx
🧰 Additional context used
🧬 Code graph analysis (8)
ui-v2/src/components/schemas/stories/properties.stories.tsx (1)
ui-v2/src/mocks/index.ts (1)
createFakeBlockDocument(3-3)
ui-v2/src/components/blocks/block-document-combobox/block-document-combobox.test.tsx (4)
ui-v2/tests/utils/browser.ts (1)
mockPointerEvents(3-22)ui-v2/tests/utils/index.ts (1)
createWrapper(26-33)ui-v2/src/components/blocks/block-document-combobox/block-document-combobox.tsx (1)
BlockDocumentCombobox(23-39)ui-v2/src/components/blocks/block-document-combobox/index.ts (1)
BlockDocumentCombobox(1-1)
ui-v2/src/components/blocks/block-document-combobox/block-document-combobox.tsx (2)
ui-v2/src/components/blocks/block-document-combobox/index.ts (1)
BlockDocumentCombobox(1-1)ui-v2/src/components/ui/icons/icon.tsx (1)
Icon(8-11)
ui-v2/src/components/blocks/block-document-create-dialog/block-document-create-dialog.test.tsx (3)
ui-v2/tests/utils/index.ts (2)
server(5-5)createWrapper(26-33)ui-v2/src/components/blocks/block-document-create-dialog/block-document-create-dialog.tsx (1)
BlockDocumentCreateDialog(55-77)ui-v2/src/components/blocks/block-document-create-dialog/index.ts (1)
BlockDocumentCreateDialog(1-1)
ui-v2/src/components/schemas/schema-form-input.tsx (1)
ui-v2/src/components/schemas/schema-form-input-block-document.tsx (1)
SchemaFormInputBlockDocument(18-56)
ui-v2/src/components/blocks/block-document-combobox/block-document-combobox.stories.tsx (1)
ui-v2/src/components/blocks/block-document-combobox/block-document-combobox.tsx (1)
BlockDocumentCombobox(23-39)
ui-v2/src/components/blocks/block-document-create-dialog/block-document-create-dialog.stories.tsx (1)
ui-v2/src/components/blocks/block-document-create-dialog/block-document-create-dialog.tsx (1)
BlockDocumentCreateDialog(55-77)
ui-v2/src/components/schemas/schema-form-input-block-document.test.tsx (3)
ui-v2/tests/utils/browser.ts (1)
mockPointerEvents(3-22)ui-v2/tests/utils/index.ts (1)
createWrapper(26-33)ui-v2/src/components/schemas/schema-form-input-block-document.tsx (1)
SchemaFormInputBlockDocument(18-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test (4)
- GitHub Check: Test (1)
- GitHub Check: Test (3)
- GitHub Check: Test (2)
- GitHub Check: Pre-commit checks
🔇 Additional comments (13)
ui-v2/src/components/blocks/block-document-create-dialog/block-document-create-dialog.test.tsx (1)
1-109: Well-structured test suite with good coverage of dialog states.The tests appropriately cover the open/closed rendering, cancel behavior, and form field presence. The shared
setupMockshelper keeps tests DRY.Consider adding a test for the complete creation flow—verifying that submitting the form triggers the POST request and calls
onCreatedwith the new block document ID. This would ensure the happy path is covered end-to-end.ui-v2/src/components/blocks/block-document-combobox/index.ts (1)
1-1: LGTM!Standard barrel export pattern enabling clean imports.
ui-v2/src/components/blocks/block-document-create-dialog/index.ts (1)
1-1: LGTM!Standard barrel export consistent with other components in this PR.
ui-v2/src/components/schemas/schema-form-input-block-document.test.tsx (1)
10-83: Good coverage of core selection functionality.The tests appropriately verify rendering, selection, and display of selected values. The
mockPointerEventssetup inbeforeAllcorrectly handles the pointer event requirements for combobox interactions.Consider adding tests for:
- Clearing a selection (verifying
onValueChange(undefined)is called)- The "Create New Block" flow that opens the dialog
ui-v2/src/components/blocks/block-document-combobox/block-document-combobox.test.tsx (1)
11-113: Comprehensive test suite covering key combobox behaviors.The tests thoroughly cover:
- Selection and callback verification
- Pre-selected value display
- Empty state placeholder
- Conditional "Create New Block" action
The typed
mockListBlockDocumentsAPIhelper improves maintainability by ensuring mock data matches the expected schema.ui-v2/src/components/schemas/schema-form-input-block-document.tsx (1)
1-56: Well-structured component with clean separation of concerns.The component correctly manages the selection and creation flow for block documents, delegating to
BlockDocumentComboboxfor selection andBlockDocumentCreateDialogfor creation. The state management is straightforward and the prop drilling is minimal.ui-v2/src/components/blocks/block-document-combobox/block-document-combobox.stories.tsx (1)
1-76: Clean Storybook setup with appropriate MSW mocking.The stories cover the key states: default with data, create new button enabled, and empty state. The wrapper component properly manages local selection state for interactive testing.
ui-v2/src/components/blocks/block-document-create-dialog/block-document-create-dialog.tsx (1)
115-123: Good defensive handling for missing schema.The component gracefully handles the edge case where no schema is found for the block type, providing clear feedback to the user.
ui-v2/src/components/blocks/block-document-create-dialog/block-document-create-dialog.stories.tsx (1)
1-71: Comprehensive Storybook setup with all required API mocks.The story properly mocks all three endpoints needed for the create flow: block type fetch, schema filter, and document creation. The wrapper component provides interactive dialog toggling.
ui-v2/src/components/schemas/stories/properties.stories.tsx (2)
9-11: Mock data follows established patterns in the codebase.The
MOCK_BLOCK_DOCUMENTSarray is created consistently with other mock data patterns in the PR, usingcreateFakeBlockDocumentwith explicit IDs and names.
632-685: Good story coverage for the newblockTypeSlugproperty.The two stories effectively demonstrate both the empty state and pre-populated state with a
$refvalue. The@ts-expect-errorcomments appropriately document thatblockTypeSlugis a custom schema extension.ui-v2/src/components/blocks/block-document-combobox/block-document-combobox.tsx (2)
50-65: Query parameters are well-structured for block document filtering.The query includes appropriate sorting, excludes secrets, filters by block type slug, and limits anonymous blocks. The conditional search filter is cleanly implemented.
110-124: Clean implementation of the "Create new" option.The create new functionality is properly guarded by the
onCreateNewprop, uses a reserved value (__create_new__) to avoid conflicts with real IDs, and correctly resets the search state.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| return ( | ||
| <Suspense> | ||
| <BlockDocumentComboboxImplementation | ||
| blockTypeSlug={blockTypeSlug} | ||
| selectedBlockDocumentId={selectedBlockDocumentId} | ||
| onSelect={onSelect} | ||
| onCreateNew={onCreateNew} | ||
| /> | ||
| </Suspense> | ||
| ); |
There was a problem hiding this comment.
Missing Suspense fallback leaves users with no loading indication.
The <Suspense> component has no fallback prop, so users won't see any visual feedback while the block documents are loading. Consider adding a skeleton or loading indicator.
🛡️ Suggested fix
return (
- <Suspense>
+ <Suspense fallback={<ComboboxTrigger disabled>Loading...</ComboboxTrigger>}>
<BlockDocumentComboboxImplementation🤖 Prompt for AI Agents
In
`@ui-v2/src/components/blocks/block-document-combobox/block-document-combobox.tsx`
around lines 29 - 38, The Suspense wrapper in BlockDocumentCombobox currently
has no fallback, so add a meaningful fallback prop (e.g., a skeleton or spinner)
to the Suspense that renders while BlockDocumentComboboxImplementation is
loading; update the <Suspense> around BlockDocumentComboboxImplementation to
include a fallback component (a small loader/skeleton component you create or an
existing Spinner) to provide visual loading feedback.
| const filteredData = useMemo(() => { | ||
| return data.filter((blockDocument) => | ||
| blockDocument.name?.toLowerCase().includes(deferredSearch.toLowerCase()), | ||
| ); | ||
| }, [data, deferredSearch]); | ||
|
|
||
| const selectedBlockDocument = useMemo(() => { | ||
| return filteredData.find( | ||
| (blockDocument) => blockDocument.id === selectedBlockDocumentId, | ||
| ); | ||
| }, [filteredData, selectedBlockDocumentId]); |
There was a problem hiding this comment.
Client-side filtering may be redundant and selectedBlockDocument lookup could fail.
Two concerns:
-
Redundant filtering: The server-side query already filters by
name: { like_: deferredSearch }(Line 61), so the client-sidefilteredDatafilter appears redundant unless the API filter behaves differently than expected. -
Selected document may disappear:
selectedBlockDocumentis looked up infilteredDatarather thandata. If the user has selected a document and then types a search term that doesn't match,selectedBlockDocumentbecomesundefined, and the trigger will show "Select a block..." even though a block is actually selected.
🐛 Suggested fix for the selected document lookup
-const selectedBlockDocument = useMemo(() => {
- return filteredData.find(
- (blockDocument) => blockDocument.id === selectedBlockDocumentId,
- );
-}, [filteredData, selectedBlockDocumentId]);
+const selectedBlockDocument = useMemo(() => {
+ return data.find(
+ (blockDocument) => blockDocument.id === selectedBlockDocumentId,
+ );
+}, [data, selectedBlockDocumentId]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const filteredData = useMemo(() => { | |
| return data.filter((blockDocument) => | |
| blockDocument.name?.toLowerCase().includes(deferredSearch.toLowerCase()), | |
| ); | |
| }, [data, deferredSearch]); | |
| const selectedBlockDocument = useMemo(() => { | |
| return filteredData.find( | |
| (blockDocument) => blockDocument.id === selectedBlockDocumentId, | |
| ); | |
| }, [filteredData, selectedBlockDocumentId]); | |
| const filteredData = useMemo(() => { | |
| return data.filter((blockDocument) => | |
| blockDocument.name?.toLowerCase().includes(deferredSearch.toLowerCase()), | |
| ); | |
| }, [data, deferredSearch]); | |
| const selectedBlockDocument = useMemo(() => { | |
| return data.find( | |
| (blockDocument) => blockDocument.id === selectedBlockDocumentId, | |
| ); | |
| }, [data, selectedBlockDocumentId]); |
🤖 Prompt for AI Agents
In
`@ui-v2/src/components/blocks/block-document-combobox/block-document-combobox.tsx`
around lines 67 - 77, filteredData duplicates server-side filtering and
selectedBlockDocument is incorrectly derived from filteredData which makes a
previously selected item vanish when the search excludes it; change
selectedBlockDocument to search the full data array (use data.find by
selectedBlockDocumentId) and either remove the client-side filtering
(filteredData) or keep it only for listing results (but never use it to resolve
the selected item), ensuring references to filteredData, data, deferredSearch,
and selectedBlockDocumentId are updated accordingly.
| const onSave = async (zodFormValues: BlockNameFormSchema) => { | ||
| try { | ||
| await validateForm({ schema: values }); | ||
| if (errors.length > 0) { | ||
| return; | ||
| } | ||
| createBlockDocument( | ||
| { | ||
| block_schema_id: blockSchemaId, | ||
| block_type_id: blockTypeId, | ||
| is_anonymous: false, | ||
| data: values, | ||
| name: zodFormValues.blockName, | ||
| }, | ||
| { | ||
| onSuccess: (res) => { | ||
| toast.success("Block created successfully"); | ||
| onOpenChange(false); | ||
| onCreated(res.id); | ||
| }, | ||
| onError: (err) => { | ||
| const message = "Unknown error while creating block."; | ||
| toast.error(message); | ||
| console.error(message, err); | ||
| }, | ||
| }, | ||
| ); | ||
| } catch (err) { | ||
| const message = "Unknown error while validating block data."; | ||
| toast.error(message); | ||
| console.error(message, err); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the file we're reviewing
find . -name "block-document-create-dialog.tsx" -type fRepository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 177
🏁 Script executed:
# Search for validateForm function implementation
rg -n "const validateForm|function validateForm" --type=ts --type=tsx -A 15 -g '!*.test.*'Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 113
🏁 Script executed:
# Search for useSchemaForm hook
rg -n "const useSchemaForm|function useSchemaForm|useSchemaForm\(\)" --type=ts --type=tsx -A 15 -g '!*.test.*'Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 113
🏁 Script executed:
# Look at the complete onSave function and surrounding context
rg -n "const onSave" ui-v2/src/components/blocks/block-document-create-dialog/ --type=tsx -B 5 -A 35Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 113
🏁 Script executed:
# Read the complete file to understand the full context
cat -n ./ui-v2/src/components/blocks/block-document-create-dialog/block-document-create-dialog.tsxRepository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 7350
🏁 Script executed:
# Search for validateForm without tsx type
rg -n "validateForm" --type ts -A 10 -g '!*.test.*'Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 12098
🏁 Script executed:
# Search for useSchemaForm without tsx type
rg -n "useSchemaForm" --type ts -A 10 -g '!*.test.*'Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 37301
🏁 Script executed:
cat -n ui-v2/src/components/schemas/hooks/useSchemaForm.tsRepository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 2040
🏁 Script executed:
# Also check how validateForm result is used in other places to understand the pattern
rg -n "const result.*validateForm|validateForm.*=" --type ts -A 3 -B 1 -g '!*.test.*'Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 2966
Race condition: errors state is stale after validateForm() call.
The validateForm function calls setErrors() internally but does not await the React state update. After await validateForm() completes, the errors variable on the next line still references the previous state, causing the validation check to fail. Invalid form data could proceed to submission.
Either return validation results directly from validateForm or restructure to handle the async state update properly.
🤖 Prompt for AI Agents
In
`@ui-v2/src/components/blocks/block-document-create-dialog/block-document-create-dialog.tsx`
around lines 159 - 191, The onSave handler uses the component-level errors state
after awaiting validateForm, which causes a race because validateForm calls
setErrors asynchronously; update validateForm to return a validation result
(e.g., an array or boolean) or otherwise return the validated values/errors,
then modify onSave to await validateForm and use that returned result (instead
of checking the errors variable) to decide whether to abort; keep the existing
createBlockDocument call and success/error callbacks (symbols: onSave,
validateForm, setErrors, errors, createBlockDocument,
zodFormValues/BlockNameFormSchema) but branch on the returned validation result
to prevent submitting stale/invalid data.
| const blockTypeSlug = property.blockTypeSlug; | ||
| if (typeof blockTypeSlug === "string") { | ||
| return ( | ||
| <SchemaFormInputBlockDocument | ||
| value={value as { $ref: string } | undefined} | ||
| onValueChange={onValueChange} | ||
| blockTypeSlug={blockTypeSlug} | ||
| id={id} | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Unsafe type assertion on value.
The cast value as { $ref: string } | undefined assumes value conforms to this shape, but value is typed as unknown. If value is a non-object primitive or has a different structure, accessing value.$ref in SchemaFormInputBlockDocument could cause unexpected behavior.
Consider adding a runtime guard:
🛡️ Proposed fix with runtime validation
if ("blockTypeSlug" in property) {
const blockTypeSlug = property.blockTypeSlug;
if (typeof blockTypeSlug === "string") {
+ const blockDocValue =
+ value === undefined || (typeof value === "object" && value !== null && "$ref" in value)
+ ? (value as { $ref: string } | undefined)
+ : undefined;
return (
<SchemaFormInputBlockDocument
- value={value as { $ref: string } | undefined}
+ value={blockDocValue}
onValueChange={onValueChange}
blockTypeSlug={blockTypeSlug}
id={id}
/>
);
}
}🤖 Prompt for AI Agents
In `@ui-v2/src/components/schemas/schema-form-input.tsx` around lines 78 - 88, The
code unsafely casts the unknown value to { $ref: string } | undefined before
passing it into SchemaFormInputBlockDocument; add a runtime guard that checks
typeof value === "object" && value !== null && typeof (value as any).$ref ===
"string" and only then pass a typed object, otherwise pass undefined (or an
appropriate fallback) to onValueChange/SchemaFormInputBlockDocument; update the
block where blockTypeSlug is handled (referencing property.blockTypeSlug, value,
SchemaFormInputBlockDocument, onValueChange, id) to compute a safeValue
validated at runtime instead of using the direct type assertion.
1 participant
Benchmark PR from agentic-review-benchmarks#4
Summary by CodeRabbit
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.