Skip to content

fix(keap): share contact field schema across mapping refresh#162

Merged
RishadAlam merged 2 commits into
mainfrom
fix/plugin-review-team-issues
May 13, 2026
Merged

fix(keap): share contact field schema across mapping refresh#162
RishadAlam merged 2 commits into
mainfrom
fix/plugin-review-team-issues

Conversation

@RishadAlam

Copy link
Copy Markdown
Member

Description

This PR fixes Keap field mapping consistency by centralizing contact field definitions and reusing them during custom field refresh. It prevents mapping drift when users reload Keap custom fields in integration setup.

Motivation & Context

Keap contact field metadata was defined inside the component only, so refresh flows could miss or desync base contact fields. Exporting and reusing one shared source keeps mapping stable and avoids configuration confusion.

Related Links: (if applicable)

Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • 💥 Breaking change
  • 📚 Documentation update
  • ⚡ Improvement
  • 🔄 Code refactor

Key Changes

Integrations

  • Fixed Keap mapping setup by moving base contact field schema to a shared exported constant.
  • Fixed Keap custom-field refresh flow to re-attach base contact fields before regenerating field mappings.

Frontend

  • Updated Keap component to consume shared contactFields definition instead of local inline duplication.
  • Improved mapping reliability after refresh so required/default Keap contact fields stay available.

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Tests added/updated
  • Documentation updated if needed
  • README updated if needed

Changelog

  • Fix: Keap contact field mapping now stays consistent after refreshing custom fields.
  • Improvement: Keap configuration now uses one shared contact field source for more reliable setup.

Copilot AI review requested due to automatic review settings May 13, 2026 08:04

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the Keap integration by moving the contactFields array to an exported constant and integrating it into the configuration refresh logic. Feedback indicates that importing this constant into KeapCommonFunc.js creates a circular dependency that should be resolved by moving shared constants to a separate file. Additionally, the assignment of contactFields should be moved outside the conditional block to ensure the configuration is updated regardless of whether custom fields are present.

@@ -1,5 +1,6 @@
import { __, sprintf } from '../../../Utils/i18nwrap'
import bitsFetch from '../../../Utils/bitsFetch'
import { contactFields } from './Keap'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This import introduces a circular dependency because Keap.jsx already imports from KeapCommonFunc.js. Since contactFields is defined at the bottom of Keap.jsx, it may be undefined when this module is evaluated due to the way ESM handles circular bindings with const. It is highly recommended to move contactFields to a separate constants file or directly into KeapCommonFunc.js to break the cycle and ensure the variable is available at runtime.

Comment on lines 199 to 202
if (result.data) {
newConf.contactFields = contactFields
newConf.customFields = result.data
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The contactFields (base fields) should be updated regardless of whether custom fields are returned. This ensures that the integration always uses the latest base field schema defined in the code, even if no custom fields are present or if the API returns an empty response. Moving this assignment outside the conditional block also ensures that generateMappedField (called on line 203) always has access to the updated field definitions.

Suggested change
if (result.data) {
newConf.contactFields = contactFields
newConf.customFields = result.data
}
newConf.contactFields = contactFields
if (result.data) {
newConf.customFields = result.data
}

@github-actions

github-actions Bot commented May 13, 2026

Copy link
Copy Markdown

✅ WordPress Plugin Check Report

✅ Status: Passed

📊 Report

All checks passed! No errors or warnings found.


🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check

Copilot AI left a comment

Copy link
Copy Markdown

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 aims to keep Keap field mappings consistent by centralizing the base contact field schema and reusing it when refreshing Keap custom fields, preventing mapping drift during integration setup.

Changes:

  • Moved the base Keap contactFields schema to a shared exported constant.
  • Updated the custom-field refresh flow to re-apply base contactFields before regenerating field_map.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
frontend/src/components/AllIntegrations/Keap/KeapCommonFunc.js Reuses a shared contactFields source during custom field refresh before regenerating mappings.
frontend/src/components/AllIntegrations/Keap/Keap.jsx Removes inline contactFields definition and exports it for reuse elsewhere.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@@ -1,5 +1,6 @@
import { __, sprintf } from '../../../Utils/i18nwrap'
import bitsFetch from '../../../Utils/bitsFetch'
import { contactFields } from './Keap'
Comment on lines +115 to +121
export default Keap


export const contactFields = [
{ key: 'given_name', label: __('First Name', 'bit-integrations'), required: true },
{ key: 'middle_name', label: __('Middle Name', 'bit-integrations'), required: false },
{ key: 'family_name', label: __('Last Name', 'bit-integrations'), required: false },
@RishadAlam RishadAlam merged commit 893d0af into main May 13, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants