Preserve GravityForms field mapping on connection change; add feed JSON export/import - #281
Preserve GravityForms field mapping on connection change; add feed JSON export/import#281davidperezgar wants to merge 1 commit into
Conversation
…ON export/import - GFCRM::save_feed_settings() now reconciles the field mapping against the feed's previous values whenever the CRM connection or module changes, matching by target field key first and falling back to a label match (via a stored listFields_labels snapshot), instead of losing the mapping. - Added Export/Import JSON controls on the feed edit page: a feed's connection type, module and field mapping can be downloaded as JSON and re-applied later, reconciled against the current connection the same way. Credentials are intentionally excluded from the export. - New pure helpers in helpers-functions.php: formscrm_gf_build_field_map_labels(), formscrm_gf_merge_field_map(), formscrm_gf_build_feed_export(), formscrm_gf_feed_meta_from_import(), with unit tests. Closes #265.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.
Once credits are available, push a new commit or reopen this pull request to trigger a review.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac80cc678c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( ! empty( $old_values[ $key ] ) ) { | ||
| $merged[ $key ] = $old_values[ $key ]; |
There was a problem hiding this comment.
When an existing feed mapping is deliberately changed to the blank option, the submitted value is empty (or omitted), so this branch restores the previous non-empty value. Because save_feed_settings() invokes this merge for every save rather than only during a connection/module transition, administrators cannot unmap a field and submissions continue sending data they explicitly attempted to remove.
Useful? React with 👍 / 👎.
| } | ||
| } | ||
|
|
||
| $merged = $new_values; |
There was a problem hiding this comment.
When the connection or module selector auto-submits, new_values contains inputs rendered for the previous selection, but this initializes the result with every one of those keys and never removes keys absent from new_field_map. The obsolete CRM field names are therefore saved alongside reconciled fields, and process_feed() later iterates every stored listFields entry at class-gravityforms.php:1098-1105, potentially sending invalid or unintended fields to the newly selected module.
Useful? React with 👍 / 👎.
| if ( $new_crmlib ) { | ||
| $old_feed = GFAPI::get_feed( $feed_id ); | ||
| $old_values = ( ! is_wp_error( $old_feed ) && ! empty( $old_feed['meta']['listFields'] ) ) ? $old_feed['meta']['listFields'] : array(); | ||
| $old_labels = ( ! is_wp_error( $old_feed ) && ! empty( $old_feed['meta']['listFields_labels'] ) ) ? $old_feed['meta']['listFields_labels'] : array(); |
There was a problem hiding this comment.
Feeds created before this release—and newly created feeds, because the initial save has no feed ID—do not yet contain listFields_labels. On their first connection/module switch, $old_labels is consequently empty, so mappings whose target keys changed cannot use the advertised label fallback; storing labels for the new map later in this save is too late. The old field definitions need to be derived or backfilled before performing the first reconciliation.
Useful? React with 👍 / 👎.
|
|
||
| $current_feed = GFAPI::get_feed( $feed_id ); | ||
|
|
||
| if ( false === $meta || is_wp_error( $current_feed ) || empty( $current_feed ) ) { |
There was a problem hiding this comment.
The import nonce is scoped only to the client-supplied form_id, while feed_id can be replaced independently, and this validation accepts any feed returned by the globally keyed GFAPI::get_feed(). A user with the FormsCRM capability and a valid import nonce can therefore substitute another form's or another add-on's feed ID, after which update_feed() overwrites that feed and supplies the attacker-selected form ID; verify both the feed's form_id and addon_slug before updating it.
Useful? React with 👍 / 👎.
| $merged_meta = array_merge( $current_meta, $meta ); | ||
| $updated = GFAPI::update_feed( $feed_id, $merged_meta, $form_id ); | ||
|
|
||
| wp_safe_redirect( add_query_arg( 'formscrm_import_status', $updated ? 'success' : 'error', $redirect_url ) ); |
There was a problem hiding this comment.
When GFAPI::update_feed() returns a WP_Error—which the newly added PHPStan stub explicitly permits—the object is truthy, so the redirect reports success even though the feed was not updated. Check is_wp_error( $updated ) before converting the result to an import status so database/API failures do not produce a false success notice.
Useful? React with 👍 / 👎.
| 'formscrm_feed_export' => true, | ||
| 'schema_version' => 1, | ||
| 'feed_name' => isset( $meta['feedName'] ) ? $meta['feedName'] : '', | ||
| 'fc_crm_custom_type' => isset( $meta['fc_crm_custom_type'] ) ? $meta['fc_crm_custom_type'] : 'no', |
There was a problem hiding this comment.
For the common case where a feed uses the plugin-wide CRM connection, this field contains only the sentinel no; the actual connector type lives in global settings and is absent from the JSON. Importing that advertised cross-site backup on a site with a different default CRM consequently selects the target site's connector while retaining the source module and mappings, so the feed can submit an incompatible payload. Include the resolved CRM type separately while continuing to exclude its credentials.
Useful? React with 👍 / 👎.
| 'fc_crm_module' => isset( $meta['fc_crm_module'] ) ? $meta['fc_crm_module'] : '', | ||
| 'fc_crm_merge_entry' => isset( $meta['fc_crm_merge_entry'] ) ? $meta['fc_crm_merge_entry'] : '', | ||
| 'fc_crm_webhook' => isset( $meta['fc_crm_webhook'] ) ? $meta['fc_crm_webhook'] : '', | ||
| 'listFields' => isset( $meta['listFields'] ) && is_array( $meta['listFields'] ) ? $meta['listFields'] : array(), |
There was a problem hiding this comment.
The exported mapping values are Gravity Forms field IDs from the source form, but the import logic reconciles only the CRM keys and stores these values unchanged. In the advertised move-to-another-form/site workflow, a mapping such as email => 3 therefore reads field 3 from the target form even when its email field has a different ID (and field 3 may contain unrelated data), because get_value_from_field() uses the stored ID directly. Export source-field identity metadata and resolve it against the target form before applying the mapping.
Useful? React with 👍 / 👎.
| if ( isset( $import_data['fc_crm_custom_type'] ) && is_string( $import_data['fc_crm_custom_type'] ) ) { | ||
| $meta['fc_crm_custom_type'] = sanitize_text_field( $import_data['fc_crm_custom_type'] ); |
There was a problem hiding this comment.
This accepts any sanitized string as fc_crm_custom_type, including a valid export naming an external CRM connector that is not installed on the target site. The import handler resolves fields using the feed's pre-import connector and therefore still reports success, but subsequent process_feed() calls make formscrm_get_api_class() return null and then dereference it at class-gravityforms.php:1170, fatally failing every submission handled by that feed. Validate that the imported connector resolves before updating the feed.
Useful? React with 👍 / 👎.
2 participants
Summary
Closes #265.
GFCRM::save_feed_settings()now reconciles a feed's field mapping instead of letting it be silently wiped when the CRM connection or module changes. It matches by target field key first, then falls back to matching by label against a storedlistFields_labelssnapshot, so standard fields (email, phone, etc.) keep their mapping across a connection switch.GFAPI::update_feed(). CRM credentials are intentionally excluded from the export for security.helpers-functions.php:formscrm_gf_build_field_map_labels(),formscrm_gf_merge_field_map(),formscrm_gf_build_feed_export(),formscrm_gf_feed_meta_from_import().GFAPI/GFFeedAddOnmethod stubs used by PHPStan.Test plan
composer lint— PHPCS (WordPress coding standards)composer phpstancomposer test— new suitetests/Unit/test-gravityforms-feed-mapping.phpcovers the merge/export/import helpers (key match, label match/case-insensitivity, unmatched fields, credential exclusion, round trip)Note: this sandbox could not run
composer installto completion (a GitHub API zipball request forphpstan/phpstanwas blocked by the environment's egress policy), so the automated lint/phpstan/test commands above could not be executed here. Syntax was verified withphp -lon every changed file, and the new pure helper functions were exercised against hand-written stubs outside PHPUnit; the checklist above should be run before merging.🤖 Generated with Claude Code
Generated by Claude Code