Add block document reference support to schema form#4
Conversation
📝 WalkthroughWalkthroughThis PR introduces two new React components—BlockDocumentCombobox for selecting block documents and BlockDocumentCreateDialog for creating them—along with SchemaFormInputBlockDocument to integrate both into form schema contexts. Comprehensive tests and Storybook stories are included for each component. The changes integrate into the schema form system, enabling dynamic block document management. Changes
Sequence DiagramssequenceDiagram
actor User
participant SchemaForm as SchemaFormInputBlockDocument
participant Combobox as BlockDocumentCombobox
participant API as API Server
User->>SchemaForm: Interact with block input
SchemaForm->>Combobox: Render with blockTypeSlug
Combobox->>API: Fetch block documents (query)
API-->>Combobox: Return filtered results
Combobox-->>SchemaForm: Display dropdown options
User->>Combobox: Select block document
Combobox->>SchemaForm: Call onSelect(blockId)
SchemaForm->>SchemaForm: Update value { $ref: blockId }
SchemaForm-->>User: Display selected block name
sequenceDiagram
actor User
participant SchemaForm as SchemaFormInputBlockDocument
participant Dialog as BlockDocumentCreateDialog
participant API as API Server
User->>SchemaForm: Click create new button
SchemaForm->>Dialog: Open dialog (createDialogOpen=true)
Dialog->>API: Fetch block type by slug
Dialog->>API: Fetch block schemas for type
API-->>Dialog: Return schemas
Dialog-->>User: Display create form
User->>Dialog: Fill form & submit
Dialog->>API: POST create block document
API-->>Dialog: Return new block document
Dialog->>SchemaForm: Call onCreated(blockId)
SchemaForm->>SchemaForm: Update value { $ref: blockId }
SchemaForm->>Dialog: Close dialog
SchemaForm-->>User: Display newly created block
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 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
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 2
🤖 Fix all issues with AI agents
In
`@ui-v2/src/components/blocks/block-document-combobox/block-document-combobox.tsx`:
- Around line 73-77: selectedBlockDocument is currently derived from
filteredData so when the user searches and the selected document is excluded it
shows "Select a block..."; change the lookup to use the unfiltered data array
instead of filteredData by finding the item in data by selectedBlockDocumentId
(keep the same memoization hook/identifier selectedBlockDocument and dependency
on data and selectedBlockDocumentId) so the selected item remains shown even
when it's not matched by the current search filter.
In
`@ui-v2/src/components/blocks/block-document-create-dialog/block-document-create-dialog.tsx`:
- Around line 151-173: The onSave flow is passing user values to validateForm
and then immediately checking the async-updated errors array; change
validateForm call to pass the actual schema definition (blockSchemaFields)
instead of values, and update validateForm usage to return a synchronous
validation result (e.g., { valid, errors }) or await that result before
proceeding; then check the returned valid flag or returned errors (instead of
reading the component-level errors immediately) before calling
createBlockDocument. Update references in useSchemaForm/validateForm, and adjust
callers (onSave in block-document-create-dialog's onSave, and the similar
handlers in block-document-create-page and block-document-edit-page) to rely on
the returned validation outcome rather than the asynchronously setErrors state.
🧹 Nitpick comments (3)
ui-v2/src/components/schemas/stories/properties.stories.tsx (1)
645-653: Consider extracting duplicated MSW handlers.The MSW handler for
/block_documents/filteris identical in bothblockTypeSlugEmptyandblockTypeSlugWithValuestories. You could define it once and reuse it.♻️ Optional: Extract common handler
+const blockDocumentsFilterHandler = http.post( + buildApiUrl("/block_documents/filter"), + () => HttpResponse.json(MOCK_BLOCK_DOCUMENTS), +); + export const blockTypeSlugEmpty: Story = { // ... parameters: { msw: { - handlers: [ - http.post(buildApiUrl("/block_documents/filter"), () => { - return HttpResponse.json(MOCK_BLOCK_DOCUMENTS); - }), - ], + handlers: [blockDocumentsFilterHandler], }, }, };Also applies to: 675-683
ui-v2/src/components/blocks/block-document-combobox/block-document-combobox.tsx (2)
30-30: Provide a fallback for the Suspense boundary.The
<Suspense>wrapper has no fallback, which means nothing is rendered while the query is loading. Consider adding a loading indicator or skeleton.♻️ Add a fallback
- <Suspense> + <Suspense fallback={<ComboboxTrigger disabled>Loading...</ComboboxTrigger>}>
50-71: Redundant client-side filtering.The query already includes a server-side name filter (line 61:
name: { like_: deferredSearch }), butfilteredData(lines 67-71) applies the same filter client-side. This is unnecessary and could be removed.♻️ Remove redundant filtering
- const filteredData = useMemo(() => { - return data.filter((blockDocument) => - blockDocument.name?.toLowerCase().includes(deferredSearch.toLowerCase()), - ); - }, [data, deferredSearch]); + const filteredData = data;Or simply use
datadirectly in the component.
| const selectedBlockDocument = useMemo(() => { | ||
| return filteredData.find( | ||
| (blockDocument) => blockDocument.id === selectedBlockDocumentId, | ||
| ); | ||
| }, [filteredData, selectedBlockDocumentId]); |
There was a problem hiding this comment.
Selected document may not be found when searching.
selectedBlockDocument is derived from filteredData, which is filtered by the search term. If the user searches for something that doesn't match the currently selected document's name, the selected document won't be found, and the trigger will show "Select a block..." instead of the actual selection.
Use the unfiltered data array to find the selected document:
🐛 Proposed fix
const selectedBlockDocument = useMemo(() => {
- return filteredData.find(
+ return data.find(
(blockDocument) => blockDocument.id === selectedBlockDocumentId,
);
- }, [filteredData, 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 selectedBlockDocument = useMemo(() => { | |
| return filteredData.find( | |
| (blockDocument) => blockDocument.id === selectedBlockDocumentId, | |
| ); | |
| }, [filteredData, selectedBlockDocumentId]); | |
| 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 73 - 77, selectedBlockDocument is currently derived from
filteredData so when the user searches and the selected document is excluded it
shows "Select a block..."; change the lookup to use the unfiltered data array
instead of filteredData by finding the item in data by selectedBlockDocumentId
(keep the same memoization hook/identifier selectedBlockDocument and dependency
on data and selectedBlockDocumentId) so the selected item remains shown even
when it's not matched by the current search filter.
| const { values, setValues, errors, validateForm } = useSchemaForm(); | ||
| const { createBlockDocument, isPending } = useCreateBlockDocument(); | ||
|
|
||
| const form = useForm({ | ||
| resolver: zodResolver(BlockNameFormSchema), | ||
| defaultValues: DEFAULT_VALUES, | ||
| }); | ||
|
|
||
| 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, | ||
| }, | ||
| { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for useSchemaForm hook definition
echo "=== Searching for useSchemaForm hook definition ==="
rg -n "useSchemaForm" --type=ts --type=tsx -A 3 | head -50
echo -e "\n=== Searching for validateForm definition/usage ==="
rg -n "validateForm" --type=ts --type=tsx -B 2 -A 2 | head -100
echo -e "\n=== Checking the hook implementation file ==="
fd "useSchemaForm" --type=ts --type=tsxRepository: agentic-review-benchmarks/prefect-combined-coderabbit
Length of output: 491
🏁 Script executed:
#!/bin/bash
# Search for useSchemaForm hook definition without type flags
echo "=== Searching for useSchemaForm hook ==="
rg -n "useSchemaForm" -A 10 | head -80
echo -e "\n=== Searching for validateForm function ==="
rg -n "validateForm" -B 2 -A 3 | head -100
echo -e "\n=== Listing TypeScript/TSX files in components ==="
fd -e ts -e tsx | grep -i schema | head -20
echo -e "\n=== Reading the specific file mentioned in the review ==="
head -200 "ui-v2/src/components/blocks/block-document-create-dialog/block-document-create-dialog.tsx" | tail -100Repository: agentic-review-benchmarks/prefect-combined-coderabbit
Length of output: 21450
🏁 Script executed:
#!/bin/bash
# Read the full useSchemaForm.ts implementation
echo "=== Full useSchemaForm.ts implementation ==="
cat "ui-v2/src/components/schemas/hooks/useSchemaForm.ts"Repository: agentic-review-benchmarks/prefect-combined-coderabbit
Length of output: 1676
🏁 Script executed:
#!/bin/bash
# Find and read the validateSchemaValues function
echo "=== Searching for validateSchemaValues ==="
fd -e ts -e tsx | xargs grep -l "validateSchemaValues" | head -10
echo -e "\n=== Reading validate utility ==="
cat "ui-v2/src/components/schemas/utilities/validate.ts"Repository: agentic-review-benchmarks/prefect-combined-coderabbit
Length of output: 688
🏁 Script executed:
#!/bin/bash
# Verify blockSchemaFields usage in the file to confirm it's never passed to validation
echo "=== Check blockSchemaFields usage in block-document-create-dialog ==="
grep -n "blockSchemaFields" "ui-v2/src/components/blocks/block-document-create-dialog/block-document-create-dialog.tsx"
echo -e "\n=== Check if similar issue exists in other files ==="
grep -n "await validateForm" ui-v2/src/components/blocks/block-document-*.tsxRepository: agentic-review-benchmarks/prefect-combined-coderabbit
Length of output: 504
🏁 Script executed:
#!/bin/bash
# Check the full context of SchemaForm rendering and validation flow
echo "=== Check SchemaForm usage in block-document-create-dialog ==="
sed -n '200,250p' "ui-v2/src/components/blocks/block-document-create-dialog/block-document-create-dialog.tsx"
echo -e "\n=== Check how other components use validateForm correctly ==="
grep -B 2 -A 5 "await validateForm" ui-v2/src/components/blocks/block-document-create-page/block-document-create-page.tsx | head -20Repository: agentic-review-benchmarks/prefect-combined-coderabbit
Length of output: 1186
Pass blockSchemaFields to validateForm and handle the asynchronous error state correctly.
Line 161 passes values (user data) as the schema parameter to validateForm, but it should pass blockSchemaFields (the actual schema definition). Additionally, errors is updated asynchronously via setErrors(), so checking errors.length > 0 immediately after await validateForm will not reflect the validation result. Either await the state update separately or refactor to return the validation status from validateForm for synchronous checking.
This same issue appears in block-document-create-page.tsx (line 62) and block-document-edit-page.tsx (line 39).
🤖 Prompt for AI Agents
In
`@ui-v2/src/components/blocks/block-document-create-dialog/block-document-create-dialog.tsx`
around lines 151 - 173, The onSave flow is passing user values to validateForm
and then immediately checking the async-updated errors array; change
validateForm call to pass the actual schema definition (blockSchemaFields)
instead of values, and update validateForm usage to return a synchronous
validation result (e.g., { valid, errors }) or await that result before
proceeding; then check the returned valid flag or returned errors (instead of
reading the component-level errors immediately) before calling
createBlockDocument. Update references in useSchemaForm/validateForm, and adjust
callers (onSave in block-document-create-dialog's onSave, and the similar
handlers in block-document-create-page and block-document-edit-page) to rely on
the returned validation outcome rather than the asynchronously setErrors state.
1 participant
Benchmark PR from agentic-review-benchmarks#4
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.