Return all custom fields in Pipedrive trigger updated-lead-instant #19003
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughVersion bumps across multiple Pipedrive actions and sources, addition of a new utility Changes
Sequence DiagramsequenceDiagram
participant Source as updated-lead-instant
participant Util as formatLeadDataFromSource
participant PD as Pipedrive API
rect rgb(240,248,255)
Source->>Util: formatLeadDataFromSource({ body, customFieldFn, resourceFn })
end
rect rgb(245,245,245)
Util->>PD: customFieldFn() (e.g. getDealCustomFields)
PD-->>Util: custom field metadata
Util->>PD: resourceFn(body.id) (e.g. getLead)
PD-->>Util: lead data
Util->>Util: map lead fields → custom field names (populate data.custom_fields)
Util-->>Source: return body with data.custom_fields (and previous.custom_fields) populated
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
components/pipedrive/actions/add-activity/add-activity.mjs(1 hunks)components/pipedrive/actions/add-deal/add-deal.mjs(1 hunks)components/pipedrive/actions/add-lead/add-lead.mjs(1 hunks)components/pipedrive/actions/add-person/add-person.mjs(1 hunks)components/pipedrive/actions/search-persons/search-persons.mjs(1 hunks)components/pipedrive/actions/update-person/update-person.mjs(1 hunks)components/pipedrive/common/utils.mjs(1 hunks)components/pipedrive/package.json(1 hunks)components/pipedrive/sources/updated-deal-instant/updated-deal-instant.mjs(1 hunks)components/pipedrive/sources/updated-lead-instant/updated-lead-instant.mjs(3 hunks)components/pipedrive/sources/updated-person-instant/updated-person-instant.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-12-12T19:23:09.039Z
Learnt from: jcortes
Repo: PipedreamHQ/pipedream PR: 14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like `fs` to `package.json` dependencies, as they are native modules provided by the Node.js runtime.
Applied to files:
components/pipedrive/package.json
📚 Learning: 2024-10-10T19:18:27.998Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 14265
File: components/the_magic_drip/sources/common.mjs:35-43
Timestamp: 2024-10-10T19:18:27.998Z
Learning: In `components/the_magic_drip/sources/common.mjs`, when processing items in `getAndProcessData`, `savedIds` is intentionally updated with IDs of both emitted and non-emitted items to avoid emitting retroactive events upon first deployment and ensure only new events are emitted as they occur.
Applied to files:
components/pipedrive/sources/updated-lead-instant/updated-lead-instant.mjs
🧬 Code graph analysis (1)
components/pipedrive/sources/updated-lead-instant/updated-lead-instant.mjs (1)
components/pipedrive/common/utils.mjs (2)
formatLeadDataFromSource(94-115)formatLeadDataFromSource(94-115)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (12)
components/pipedrive/package.json (1)
3-3: Version bump aligns with the component updates.
Matches the action/source metadata increments in this PR; no issues spotted.components/pipedrive/actions/add-lead/add-lead.mjs (1)
9-9: Action version metadata looks good.
Increment to 0.0.12 stays in sync with the package release.components/pipedrive/actions/add-activity/add-activity.mjs (1)
10-10: Version increment confirmed.
Metadata now reflects 0.1.18; consistent with the rollout.components/pipedrive/actions/add-person/add-person.mjs (1)
9-9: Metadata version bump looks correct.
0.1.18 tracks with the package release numbers.components/pipedrive/sources/updated-deal-instant/updated-deal-instant.mjs (1)
10-10: Source version update acknowledged.
Setting the descriptor to 0.1.7 keeps the source in step with this release.components/pipedrive/actions/update-person/update-person.mjs (1)
9-9: Action version bump confirmed.
0.0.10 is consistent with the other component updates.components/pipedrive/actions/search-persons/search-persons.mjs (1)
11-11: Version metadata matches the release plan.
Setting 0.1.18 keeps this action aligned with the package bump.components/pipedrive/actions/add-deal/add-deal.mjs (1)
9-9: Version increment looks good.
Updating to 0.1.19 keeps the action’s metadata aligned with this release.components/pipedrive/sources/updated-person-instant/updated-person-instant.mjs (1)
10-10: LGTM: Version bump aligns with package update.This version bump is coordinated with the package-level version update to 0.10.5.
components/pipedrive/sources/updated-lead-instant/updated-lead-instant.mjs (3)
1-1: LGTM: Import updated for new utility function.The import correctly switches to the new
formatLeadDataFromSourcefunction that provides more comprehensive custom field handling.
10-10: LGTM: Version bump reflects functional changes.
24-30: No changes required—the code is correct.Leads do not have separate custom fields; they share the same custom field definitions as Deals. The Pipedrive API has no distinct "lead custom fields" endpoint. Using
this.pipedrive.getDealCustomFieldsis the correct and intended approach for retrieving custom field definitions for leads. The original concern was based on a false assumption about the API structure.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/pipedrive/common/utils.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
jcortes
left a comment
jcortes
left a comment
There was a problem hiding this comment.
Hi @michelle0927 lgtm! Ready for QA!
For Integration QA: |
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check test reports below for more information:
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/pipedrive/common/utils.mjs (1)
109-119: Critical: Extract.valuefrom webhook custom field structure.Lines 115-116 read from
body.previous.custom_fields[key], which comes from the webhook payload. According toparseCustomFields(line 32), webhook custom fields have a.valueproperty. Line 116 stores the entire object instead of extracting.value, creating a structure mismatch with the current custom fields at line 105 (which stores direct values from the API).Additionally, line 115's check will exclude valid falsy values.
Apply this diff to extract
.valueand handle falsy values:const formattedPreviousCustomFields = {}; if (body?.previous?.custom_fields) { for (const [ key, value, ] of Object.entries(customFieldNames)) { - if (body.previous.custom_fields[key]) { - formattedPreviousCustomFields[value] = body.previous.custom_fields[key]; + if (body.previous.custom_fields[key]?.value !== undefined) { + formattedPreviousCustomFields[value] = body.previous.custom_fields[key].value; } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/pipedrive/common/utils.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (1)
components/pipedrive/common/utils.mjs (1)
120-132: Verify the conditional logic for includingpreviousin the return value.Line 126 only includes
previousin the return value ifbody.previous?.custom_fieldsexists. Ifbody.previousexists but doesn't havecustom_fields, other fields fromprevious(if any) will be omitted from the output.If
body.previousis always present for update events (as suggested by line 45 inparseData), consider simplifying to:- ...(body.previous?.custom_fields && { + ...(body.previous && { previous: { ...body.previous, custom_fields: formattedPreviousCustomFields, }, }),This ensures all
previousfields are preserved even ifcustom_fieldsis absent. Please verify whetherbody.previouscan exist withoutcustom_fieldsin your webhook payloads.
| export const formatLeadDataFromSource = async ({ | ||
| body, customFieldFn, resourceFn, | ||
| }) => { | ||
| const customFieldNames = await getCustomFieldNames(customFieldFn); | ||
| const { data: lead } = await resourceFn(body.data.id); | ||
| const formattedCustomFields = {}; | ||
| for (const [ | ||
| key, | ||
| value, | ||
| ] of Object.entries(customFieldNames)) { | ||
| if (lead[key]) { | ||
| formattedCustomFields[value] = lead[key]; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider handling falsy custom field values.
Line 104 checks if (lead[key]), which will exclude valid falsy values like 0, false, or empty strings. If custom fields can legitimately have these values, use a more precise check.
Apply this diff to handle falsy values correctly:
for (const [
key,
value,
] of Object.entries(customFieldNames)) {
- if (lead[key]) {
+ if (lead[key] !== undefined) {
formattedCustomFields[value] = lead[key];
}
}📝 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.
| export const formatLeadDataFromSource = async ({ | |
| body, customFieldFn, resourceFn, | |
| }) => { | |
| const customFieldNames = await getCustomFieldNames(customFieldFn); | |
| const { data: lead } = await resourceFn(body.data.id); | |
| const formattedCustomFields = {}; | |
| for (const [ | |
| key, | |
| value, | |
| ] of Object.entries(customFieldNames)) { | |
| if (lead[key]) { | |
| formattedCustomFields[value] = lead[key]; | |
| } | |
| } | |
| export const formatLeadDataFromSource = async ({ | |
| body, customFieldFn, resourceFn, | |
| }) => { | |
| const customFieldNames = await getCustomFieldNames(customFieldFn); | |
| const { data: lead } = await resourceFn(body.data.id); | |
| const formattedCustomFields = {}; | |
| for (const [ | |
| key, | |
| value, | |
| ] of Object.entries(customFieldNames)) { | |
| if (lead[key] !== undefined) { | |
| formattedCustomFields[value] = lead[key]; | |
| } | |
| } |
🤖 Prompt for AI Agents
In components/pipedrive/common/utils.mjs around lines 94 to 107, the check `if
(lead[key])` wrongly filters out valid falsy custom field values (0, false, ""),
so change it to test presence rather than truthiness; replace that condition
with a presence check such as `if (Object.prototype.hasOwnProperty.call(lead,
key) && lead[key] !== null && lead[key] !== undefined)` so falsy-but-valid
values are included while still excluding missing or null/undefined fields.
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check test reports below for more information:
|
Resolves #18999
Summary by CodeRabbit