-
Notifications
You must be signed in to change notification settings - Fork 840
Try: Add reusable form block #46207
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.
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
base: add/jetpack-forms-custom-post-type
Are you sure you want to change the base?
Try: Add reusable form block #46207
Conversation
Introduces a new reusable form block with supporting TypeScript files and updates block registration to use the dist folder. The block's metadata and editor integration are enhanced, and the webpack config is updated to build and copy the new block assets correctly.
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 🔴 Action required: Please add missing changelog entries for the following projects: Use the Jetpack CLI tool to generate changelog entries by running the following command: Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a reusable form block (jetpack/form) that enables users to insert forms from the jetpack-form custom post type anywhere on their site. The implementation is feature-flagged behind reusable-forms and includes both frontend rendering and editor capabilities.
Key Changes
- New reusable form block with reference-based rendering system
- Webpack configuration update to preserve block directory structure
- Server-side rendering via
Form_Block::render()that fetches and renders form content
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
projects/packages/forms/tools/webpack.config.blocks.js |
Modified CopyWebpackPlugin to preserve directory structure for block.json files, added entry point for new form block |
projects/packages/forms/src/class-reusable-forms.php |
Added block registration call to initialize the new jetpack/form block |
projects/packages/forms/src/blocks/form/index.tsx |
Block registration with feature flag check and Material Design icon |
projects/packages/forms/src/blocks/form/edit.tsx |
Editor component handling form loading, error states, and inner block rendering |
projects/packages/forms/src/blocks/form/class-form-block.php |
Server-side block registration and rendering callback that fetches form content by reference |
projects/packages/forms/src/blocks/form/block.json |
Block metadata defining attributes, supports, and configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // No ref specified | ||
| if ( ! ref ) { | ||
| return ( | ||
| <div { ...blockProps }> | ||
| <Warning>{ __( 'No form selected. Please select a form.', 'jetpack-forms' ) }</Warning> | ||
| </div> | ||
| ); | ||
| } |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition check order is incorrect. The code checks if ref is missing (line 55) after already checking if the form is missing (line 46). However, when ref is falsy, the useEntityRecord and useEntityBlockEditor hooks at lines 13-18 will be called with undefined or 0, which could cause unnecessary API requests or errors. The check for !ref should be moved before using it in the hooks to avoid this issue.
| return ''; | ||
| } | ||
|
|
||
| unset( $content ); |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The $content parameter is unused and immediately unset. Instead of unsetting it, consider removing it from the function signature or using a comment to indicate it's intentionally unused (e.g., // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable or similar). Alternatively, if it's required by the WordPress block render callback signature, add a PHPDoc comment explaining why it's present but unused.
| if ( ! $form_post || 'jetpack-form' !== $form_post->post_type ) { | ||
| return ''; | ||
| } | ||
|
|
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing security check for post status. The code retrieves and renders a form post without verifying its status. It should check that the post is published (or at least not trashed) before rendering it. Consider adding a check like if ( 'publish' !== $form_post->post_status ) to ensure only published forms are displayed on the frontend.
| // Only render published forms | |
| if ( 'publish' !== $form_post->post_status ) { | |
| return ''; | |
| } |
| if ( ! $form_post || 'jetpack-form' !== $form_post->post_type ) { | ||
| return ''; | ||
| } | ||
|
|
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing capability check for viewing the form. The code doesn't verify that the current user has permission to view the referenced form. This could potentially expose private or draft forms to unauthorized users. Consider adding a capability check using current_user_can() or checking the post's visibility status before rendering.
| // Check if the current user can view the form post | |
| $post_status = get_post_status( $form_post ); | |
| if ( 'publish' !== $post_status && ! current_user_can( 'edit_post', $form_post->ID ) ) { | |
| return ''; | |
| } |
| public static function render( $atts, $content = '' ) { | ||
| // Get the form reference ID | ||
| $ref = isset( $atts['ref'] ) ? absint( $atts['ref'] ) : 0; | ||
|
|
||
| if ( ! $ref ) { | ||
| return ''; | ||
| } | ||
|
|
||
| unset( $content ); | ||
|
|
||
| // Get the referenced form post | ||
| $form_post = get_post( $ref ); | ||
|
|
||
| // Verify the post exists and is a jetpack-form post type | ||
| if ( ! $form_post || 'jetpack-form' !== $form_post->post_type ) { | ||
| return ''; | ||
| } | ||
|
|
||
| // Return the form content rendered with do_blocks | ||
| return do_blocks( $form_post->post_content ); | ||
| } | ||
| } |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing test coverage for the new Form_Block class. The repository has comprehensive test coverage for other blocks (see tests/php/contact-form/ directory), but there are no tests for the new Form_Block::render() method. Consider adding tests to verify:
- Rendering with valid ref
- Handling of missing/invalid ref
- Post type validation
- Edge cases like non-existent posts
| export default function FormEdit( { attributes } ) { | ||
| const { ref } = attributes; | ||
|
|
||
| // Fetch the form post from the jetpack-form post type | ||
| const { record, hasResolved } = useEntityRecord( 'postType', 'jetpack-form', ref ); | ||
|
|
||
| // Get the blocks from the form post | ||
| const [ blocks, onInput, onChange ] = useEntityBlockEditor( 'postType', 'jetpack-form', { | ||
| id: ref, | ||
| } ); | ||
|
|
||
| // Check if the form is missing | ||
| const isMissing = hasResolved && ! record; | ||
|
|
||
| // Get the block props for the wrapper | ||
| const blockProps = useBlockProps( { | ||
| className: 'wp-block-jetpack-form', | ||
| } ); | ||
|
|
||
| // Get inner blocks props for rendering the form blocks | ||
| const innerBlocksProps = useInnerBlocksProps( blockProps, { | ||
| value: blocks, | ||
| onInput, | ||
| onChange, | ||
| renderAppender: blocks?.length ? undefined : () => null, | ||
| } ); | ||
|
|
||
| // Loading state | ||
| if ( ! hasResolved ) { | ||
| return ( | ||
| <div { ...blockProps }> | ||
| <Spinner /> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| // Missing or invalid form | ||
| if ( isMissing ) { | ||
| return ( | ||
| <div { ...blockProps }> | ||
| <Warning>{ __( 'Form not found. Please select a valid form.', 'jetpack-forms' ) }</Warning> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| // No ref specified | ||
| if ( ! ref ) { | ||
| return ( | ||
| <div { ...blockProps }> | ||
| <Warning>{ __( 'No form selected. Please select a form.', 'jetpack-forms' ) }</Warning> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| // Render the form blocks | ||
| return <div { ...innerBlocksProps } />; | ||
| } |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing test coverage for the FormEdit component. The repository has comprehensive test coverage for other components (see tests/js/ directory), but there are no tests for the new FormEdit component. Consider adding tests to verify:
- Loading state rendering
- Error handling for missing forms
- Error handling for missing ref
- Proper block rendering with valid data
Code Coverage SummaryCannot generate coverage summary while tests are failing. 🤐 Please fix the tests, or re-run the Code coverage job if it was something being flaky. |
Co-authored-by: Copilot <[email protected]>
| absoluteFilename | ||
| ); | ||
| return relativePath; | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this change?
| } | ||
| }, | ||
| "editorScript": "file:./index.js" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if naming could be adjusted for better clarity, and to distingguish the new reusable form functionality we're adding from the Jetpack Forms package/product name and our long standing form block.
We now have:
- A "reusable forms" class that registers a "jetpack-form" post type
- A "class-form-block" that registers a "jetpack/form" block that lives in the "src/blocks/form" folder
- A "REST_Jetpack_Form_Controller" that instatiates with "jetpack-form"
- All of this is in the context of a plugin call Jetppack Forms that already is build around a "contact-form" block.
If we settle the the 'reusable' terminology, we might use naming like below. These names would be hard to confuse with the long standing form block or generic Jetpack Forms package naming.
- CPT slug: jetpack-reusable-form
- Block name: jetpack/reusable-form
- Folder: src/blocks/reusable-form
- Class names: REST_Jetpack_Reusable_Form_Controller, Reusable_Form_Block, etc.
!!! Do not merge
Screen.Recording.2025-12-04.at.3.46.02.PM.mov
Proposed changes:
Other information:
Jetpack product discussion
Does this pull request change what data or activity we track or use?
No
Testing instructions:
You need to feature enable with a filter somoething like this.
In this version you should be able to create a new jetpack-form
custom post type by going to /wp-admin/post-new.php?post_type=jetpack-form
insert a form block there. (currently there is no restriction in what kind of block you can insert but the idea is to only be able to insert a form block via the block inserter.