-
Notifications
You must be signed in to change notification settings - Fork 843
Forms: Add synced forms support with convert toolbar #46297
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: trunk
Are you sure you want to change the base?
Changes from all commits
ecc94d6
ed88888
7d2e9ae
1beaa39
fd79ad6
782c3eb
2002a46
fd9c530
7c1624f
97151fa
4bde14b
281b663
f277ed8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| Significance: minor | ||
| Type: added | ||
|
|
||
| Add ConvertFormToolbar component for managing synced form conversions in the block editor | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -728,9 +728,62 @@ public static function gutenblock_render_form( $atts, $content ) { | |||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| self::load_view_scripts(); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // Handle ref attribute - load form from jetpack-form post | ||||||||||||||||||||||||||||||||||||||||||
| if ( isset( $atts['ref'] ) && is_numeric( $atts['ref'] ) ) { | ||||||||||||||||||||||||||||||||||||||||||
| return self::render_synced_form( $atts['ref'] ); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+731
to
+734
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| return Contact_Form::parse( $atts, do_blocks( $content ) ); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||
| * Render a synced form by reference ID. | ||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||
| * @param int $ref_id The jetpack_form post ID. | ||||||||||||||||||||||||||||||||||||||||||
| * @return string Rendered form HTML. | ||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||
| private static function render_synced_form( $ref_id ) { | ||||||||||||||||||||||||||||||||||||||||||
| // Circular reference prevention. | ||||||||||||||||||||||||||||||||||||||||||
| static $seen_refs = array(); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| if ( isset( $seen_refs[ $ref_id ] ) ) { | ||||||||||||||||||||||||||||||||||||||||||
| return sprintf( | ||||||||||||||||||||||||||||||||||||||||||
| '<div class="wp-block-jetpack-contact-form">%s</div>', | ||||||||||||||||||||||||||||||||||||||||||
| esc_html__( 'Circular reference detected in form.', 'jetpack-forms' ) | ||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // Load the jetpack-form post. | ||||||||||||||||||||||||||||||||||||||||||
| $synced_form = get_post( $ref_id ); | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+732
to
+757
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // Validate post. | ||||||||||||||||||||||||||||||||||||||||||
| if ( ! $synced_form || 'jetpack_form' !== $synced_form->post_type ) { | ||||||||||||||||||||||||||||||||||||||||||
| return ''; | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // Only render published and draft post statuses. | ||||||||||||||||||||||||||||||||||||||||||
| // todo: add a "active" status so that we can disable forms without deleting them. | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
| // todo: add a "active" status so that we can disable forms without deleting them. |
Copilot
AI
Dec 12, 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 check allowing jetpack_form posts with post_status of draft to be rendered means draft forms can be displayed on public pages if a block references their ref ID. This bypasses the usual WordPress expectation that drafts are not publicly visible and could unintentionally expose in-progress or disabled forms and their configuration to unauthenticated visitors. Restrict rendering to publish status for front-end requests (and/or gate draft rendering behind appropriate capability checks or editor-only contexts) so that non-public form statuses cannot be surfaced to general users.
| // Only render published and draft post statuses. | |
| // todo: add a "active" status so that we can disable forms without deleting them. | |
| if ( ! in_array( $synced_form->post_status, array( 'publish', 'draft' ), true ) ) { | |
| // Only render published forms on the front end. | |
| // Allow drafts only in admin/editor contexts or for users with edit capability. | |
| if ( | |
| 'publish' !== $synced_form->post_status && | |
| ( | |
| ! is_admin() && | |
| ! ( defined( 'REST_REQUEST' ) && REST_REQUEST ) && | |
| ! ( defined( 'DOING_AJAX' ) && DOING_AJAX ) && | |
| ! current_user_can( 'edit_post', $synced_form->ID ) | |
| ) | |
| ) { |
Copilot
AI
Dec 12, 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.
When a synced form post is not found or has an invalid post type, the function returns an empty string, giving no feedback to users. Consider returning a user-friendly error message (similar to the circular reference case) so users understand why the form isn't displaying. This would improve the debugging experience.
| return ''; | |
| } | |
| // Only render published and draft post statuses. | |
| // todo: add a "active" status so that we can disable forms without deleting them. | |
| if ( ! in_array( $synced_form->post_status, array( 'publish', 'draft' ), true ) ) { | |
| return ''; | |
| return sprintf( | |
| '<div class="wp-block-jetpack-contact-form">%s</div>', | |
| esc_html__( 'Referenced form not found or invalid.', 'jetpack-forms' ) | |
| ); | |
| } | |
| // Only render published and draft post statuses. | |
| // todo: add a "active" status so that we can disable forms without deleting them. | |
| if ( ! in_array( $synced_form->post_status, array( 'publish', 'draft' ), true ) ) { | |
| return sprintf( | |
| '<div class="wp-block-jetpack-contact-form">%s</div>', | |
| esc_html__( 'Referenced form is not published or is inactive.', 'jetpack-forms' ) | |
| ); |
Copilot
AI
Dec 12, 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 static $seen_refs array for circular reference prevention will persist across multiple requests in persistent PHP environments (like PHP-FPM). If an error occurs between marking a ref as seen and unsetting it, subsequent requests could incorrectly detect circular references. Consider adding cleanup logic or using instance-level tracking instead of static variables.
Copilot
AI
Dec 12, 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 new render_synced_form method lacks test coverage. Consider adding tests for: 1) successful rendering of synced forms, 2) circular reference detection, 3) validation of post type, 4) handling of different post statuses (publish, draft, trash), and 5) handling of non-existent post IDs.
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,141 @@ | ||||||||
| /** | ||||||||
| * Convert Form Toolbar Component | ||||||||
| * Provides toolbar buttons to convert forms to synced mode and edit synced forms | ||||||||
| */ | ||||||||
|
|
||||||||
| import { store as blockEditorStore } from '@wordpress/block-editor'; | ||||||||
| import { ToolbarGroup, ToolbarButton } from '@wordpress/components'; | ||||||||
| import { useSelect, useDispatch } from '@wordpress/data'; | ||||||||
| import { store as editorStore } from '@wordpress/editor'; | ||||||||
| import { useState } from '@wordpress/element'; | ||||||||
| import { __ } from '@wordpress/i18n'; | ||||||||
| import { store as noticesStore } from '@wordpress/notices'; | ||||||||
| import { FORM_POST_TYPE } from '../../shared/util/constants.js'; | ||||||||
| import { createSyncedForm } from '../utils/form-sync-manager'; | ||||||||
|
|
||||||||
| interface ConvertFormToolbarProps { | ||||||||
| clientId: string; | ||||||||
| attributes: Record< string, unknown >; | ||||||||
| setAttributes: ( attrs: Record< string, unknown > ) => void; | ||||||||
|
||||||||
| setAttributes: ( attrs: Record< string, unknown > ) => void; |
Copilot
AI
Dec 12, 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 useSelect hook on line 25 has an empty dependency array, which means it only runs once on mount. If the post title changes during editing, the postTitle value won't update, potentially causing the synced form to be created with an outdated title. Add postTitle or relevant dependencies to the array to ensure the value stays current.
| }, [] ); | |
| } ); |
Copilot
AI
Dec 12, 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 destructuring syntax here doesn't actually remove the 'ref' property from attributes. The spread operator ...cleanAttributes creates a shallow copy of all properties in attributes. To properly exclude the 'ref' property, you should use: const { ref, ...cleanAttributes } = attributes;
| const { ...cleanAttributes } = attributes; | |
| const { ref, ...cleanAttributes } = attributes; |
Copilot
AI
Dec 12, 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 error variable is caught but not logged or used for debugging. While the eslint-disable comment acknowledges this, consider logging the error to the console for debugging purposes, especially since this involves network operations and post creation that could fail in various ways.
| } catch ( error ) { | |
| } catch ( error ) { | |
| console.error( 'Error converting form to synced form:', error ); |
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.

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 changelog entry only mentions adding the ConvertFormToolbar component, but this PR introduces several other significant changes including the useSyncedForm hook, form-sync-manager utilities, PHP rendering logic for synced forms, and the ref attribute. Consider updating the changelog to provide a more comprehensive summary of all the changes.