[Component] Pipedrive - add note prop to add deal action #18321
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughMost files in Pipedrive components received patch version bumps. The Add Deal action switched to a generic app instance, added multiple new optional deal fields, and introduced optional post-create note attachment. The Pipedrive app added new propDefinitions for those fields, including async label options. Package version bumped. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant AddDeal Action as AddDeal Action
participant Pipedrive App as Pipedrive App
participant Pipedrive API as Pipedrive API
User->>AddDeal Action: Provide deal data (+ optional note)
AddDeal Action->>Pipedrive App: addDeal(payload with new fields)
Pipedrive App->>Pipedrive API: POST /deals
Pipedrive API-->>Pipedrive App: 201 Created (deal_id)
Pipedrive App-->>AddDeal Action: deal response
alt Note provided
AddDeal Action->>Pipedrive App: addNote({ deal_id, content })
Pipedrive App->>Pipedrive API: POST /notes
Pipedrive API-->>Pipedrive App: 201 Created (note_id)
Pipedrive App-->>AddDeal Action: note response
else No note
Note over AddDeal Action: Skip note creation
end
AddDeal Action-->>User: Return deal (and note if created)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks (3 passed, 2 warnings)❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/pipedrive/actions/add-lead/add-lead.mjs (1)
94-120: Shield addNote errors and align error handling with other actionsadd-lead currently lacks the try/catch pattern used elsewhere and will fail the whole step if note creation throws after a successful lead create. Wrap run in try/catch and isolate addNote errors so the lead creation still succeeds.
async run({ $ }) { - if (!this.organizationId && !this.personId) { - throw new ConfigurationError("Either organizationId or personId is required"); - } - - const response = await this.pipedrive.addLead({ - title: this.title, - person_id: this.personId, - organization_id: this.organizationId, - owner_id: this.ownerId, - label_ids: this.leadLabelIds, - value: parseObject(this.value), - expected_close_date: this.expectedCloseDate, - visible_to: this.visibleTo, - was_seen: this.wasSeen, - }); - - if (this.note) { - await this.pipedrive.addNote({ - content: this.note, - lead_id: response.data?.id, - }); - } - - $.export("$summary", `Successfully created lead: ${response.data?.title || response.data?.id}`); - return response; + try { + if (!this.organizationId && !this.personId) { + throw new ConfigurationError("Either organizationId or personId is required"); + } + + const response = await this.pipedrive.addLead({ + title: this.title, + person_id: this.personId, + organization_id: this.organizationId, + owner_id: this.ownerId, + label_ids: this.leadLabelIds, + value: parseObject(this.value), + expected_close_date: this.expectedCloseDate, + visible_to: this.visibleTo, + was_seen: this.wasSeen, + }); + + const leadId = response?.data?.id; + if (this.note && leadId) { + try { + await this.pipedrive.addNote({ + content: this.note, + lead_id: leadId, + }); + } catch (err) { + $.export("noteError", err?.message ?? "Failed to add note"); + } + } + + $.export("$summary", `Successfully created lead: ${response.data?.title || response.data?.id}`); + return response; + } catch ({ error }) { + throw new ConfigurationError(error); + } },
🧹 Nitpick comments (5)
components/pipedrive/actions/get-person-details/get-person-details.mjs (1)
21-31: Return shape consistency: return data instead of raw respOther actions typically return the response data (e.g., get-lead-by-id). Consider returning
resp.datafor consistency.- return resp; + return resp?.data ?? resp;components/pipedrive/actions/update-deal/update-deal.mjs (1)
119-124: Isolate note failures so deal updates don’t fail post-updateMirror the add-lead pattern by catching addNote errors; avoid failing after a successful update.
- if (this.note) { - await this.pipedriveApp.addNote({ - content: this.note, - deal_id: this.dealId, - }); - } + if (this.note) { + try { + await this.pipedriveApp.addNote({ + content: this.note, + deal_id: this.dealId, + }); + } catch (err) { + $.export("noteError", err?.message ?? "Failed to add note"); + } + }components/pipedrive/actions/add-deal/add-deal.mjs (2)
2-4: Avoid shadowing the imported app and align naming with other actions
import app ...is shadowed byconst { app } = this;which is easy to confuse with the imported module. Other actions usepipedriveApp. Recommend renaming the import topipedriveAppand reference it in propDefinitions; keep the prop namedappif you want, but avoid same identifier for two meanings.Minimal illustration (apply similarly to all propDefinition entries referencing
app):-import app from "../../pipedrive.app.mjs"; +import pipedriveApp from "../../pipedrive.app.mjs"; props: { - app, + app, title: { propDefinition: [ - app, + pipedriveApp, "dealTitle", ], }, ownerId: { propDefinition: [ - app, + pipedriveApp, "userId", ], }, // ...repeat for every propDefinition currently using `app` }, async run({ $ }) { - const { - app, + const { + app, title, // ... } = this;Also applies to: 12-18, 20-24, 26-30, 32-36, 38-43, 45-49, 51-55, 57-61, 63-67, 69-73, 75-79, 81-85, 86-91, 92-97, 98-103, 104-109, 110-115, 116-121, 122-127, 128-133, 134-139, 148-153
86-127: Enforce status constraints for lifecycle props on addDeal
- Pipedrive API v2 supports
isDeleted,isArchived,archiveTime, andexpectedCloseDateon create, butcloseTimeonly whenstatusis"won"or"lost",wonTimeonly whenstatusis"won", andlostTimeonly whenstatusis"lost".- Gate or strip these fields in components/pipedrive/actions/add-deal/add-deal.mjs (lines 86–127) to prevent API rejections.
components/pipedrive/pipedrive.app.mjs (1)
321-350: Timestamp format guidance is inconsistent with other propsThese props document
YYYY-MM-DD HH:MM:SSwhileaddTimeuses ISO8601 withT/Z. Unify guidance (ideally ISO8601) to reduce user input errors.- description: "The optional date and time of archiving the deal in UTC. Format: `YYYY-MM-DD HH:MM:SS`.... + description: "The optional date and time of archiving the deal in UTC. Recommended format: `YYYY-MM-DDTHH:MM:SSZ` (ISO 8601). ...(Apply similarly to Close/Won/Lost Time.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
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-note/add-note.mjs(1 hunks)components/pipedrive/actions/add-organization/add-organization.mjs(1 hunks)components/pipedrive/actions/add-person/add-person.mjs(1 hunks)components/pipedrive/actions/get-lead-by-id/get-lead-by-id.mjs(1 hunks)components/pipedrive/actions/get-person-details/get-person-details.mjs(1 hunks)components/pipedrive/actions/merge-deals/merge-deals.mjs(1 hunks)components/pipedrive/actions/merge-persons/merge-persons.mjs(1 hunks)components/pipedrive/actions/remove-duplicate-notes/remove-duplicate-notes.mjs(1 hunks)components/pipedrive/actions/search-leads/search-leads.mjs(1 hunks)components/pipedrive/actions/search-notes/search-notes.mjs(1 hunks)components/pipedrive/actions/search-persons/search-persons.mjs(1 hunks)components/pipedrive/actions/update-deal/update-deal.mjs(1 hunks)components/pipedrive/actions/update-person/update-person.mjs(1 hunks)components/pipedrive/package.json(1 hunks)components/pipedrive/pipedrive.app.mjs(1 hunks)components/pipedrive/sources/new-deal-instant/new-deal-instant.mjs(1 hunks)components/pipedrive/sources/new-event-instant/new-event-instant.mjs(1 hunks)components/pipedrive/sources/new-person-instant/new-person-instant.mjs(1 hunks)components/pipedrive/sources/updated-deal-instant/updated-deal-instant.mjs(1 hunks)components/pipedrive/sources/updated-lead-instant/updated-lead-instant.mjs(1 hunks)components/pipedrive/sources/updated-person-instant/updated-person-instant.mjs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
components/pipedrive/pipedrive.app.mjs (1)
components/pipedrive/actions/add-activity/add-activity.mjs (1)
data(156-170)
components/pipedrive/actions/add-deal/add-deal.mjs (2)
components/pipedrive/actions/add-note/add-note.mjs (1)
resp(89-102)components/pipedrive/common/utils.mjs (2)
parseObject(1-24)parseObject(1-24)
⏰ 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: Lint Code Base
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (23)
components/pipedrive/actions/add-lead/add-lead.mjs (1)
111-116: Confirmed: Pipedrive Notes API supports lead_id
No changes needed—thelead_idparameter (string, UUID) is valid on POST/v1/notes.components/pipedrive/package.json (1)
3-3: Version bump looks good0.10.1 aligns with action/source version bumps in this release.
components/pipedrive/sources/updated-person-instant/updated-person-instant.mjs (1)
10-10: LGTM on version bumpNo functional changes; metadata update only.
components/pipedrive/actions/get-lead-by-id/get-lead-by-id.mjs (1)
7-7: LGTM on version bumpAction remains unchanged functionally.
components/pipedrive/actions/get-person-details/get-person-details.mjs (1)
8-8: LGTM on version bumpcomponents/pipedrive/actions/add-activity/add-activity.mjs (1)
10-10: LGTM on version bumpcomponents/pipedrive/actions/merge-deals/merge-deals.mjs (1)
7-7: LGTM on version bumpcomponents/pipedrive/actions/search-leads/search-leads.mjs (1)
8-8: Version bump only — OKNo functional changes detected. Safe to publish.
components/pipedrive/actions/merge-persons/merge-persons.mjs (1)
7-7: Version bump only — OKNo behavior changes in merge logic. Good to release.
components/pipedrive/actions/remove-duplicate-notes/remove-duplicate-notes.mjs (1)
8-8: Version bump only — OKLogic unchanged. Proceed.
components/pipedrive/actions/add-person/add-person.mjs (1)
9-9: Version bump only — OKNo runtime changes; existing error handling remains intact.
components/pipedrive/actions/search-notes/search-notes.mjs (1)
7-7: Version bump only — OKFilter/sort behavior unchanged.
components/pipedrive/actions/add-note/add-note.mjs (1)
8-8: Version bump only — OKNo changes to add-note behavior. Compatible with upstream usage.
components/pipedrive/sources/new-event-instant/new-event-instant.mjs (1)
8-8: Version bump only — OKSource interface and webhook handling unchanged.
components/pipedrive/actions/add-organization/add-organization.mjs (1)
8-8: Version bump only — OKNo functional differences; safe increment.
components/pipedrive/actions/add-deal/add-deal.mjs (1)
128-139: Ignore case conversion comment
The code correctly uses camelCase inputs (labelIds,customFields), and the Pipedrive Node SDK automatically serializes these to the required snake_case (label_ids,custom_fields) in the HTTP payload, so no changes are needed.Likely an incorrect or invalid review comment.
components/pipedrive/pipedrive.app.mjs (1)
357-372: Async label options resolution looks solidFetching Deal fields and mapping the
labelfield options to{label, value}is correct and defensive with a safe fallback.components/pipedrive/sources/updated-deal-instant/updated-deal-instant.mjs (1)
10-10: Version bump only — OKNo functional changes detected.
components/pipedrive/sources/updated-lead-instant/updated-lead-instant.mjs (1)
10-10: Version bump only — OKNo functional changes detected.
components/pipedrive/sources/new-deal-instant/new-deal-instant.mjs (1)
9-9: Version bump only — OKNo functional changes detected.
components/pipedrive/sources/new-person-instant/new-person-instant.mjs (1)
9-9: Version bump only — OKNo functional changes detected.
components/pipedrive/actions/search-persons/search-persons.mjs (1)
10-10: Version bump only — OKNo functional changes detected.
components/pipedrive/actions/update-person/update-person.mjs (1)
9-9: Version bump only — OKNo functional changes detected.
jcortes
deleted the
pipedrive-improve-add-deal-with-optional-note-prop
branch
WHY
Resolves #18235
Summary by CodeRabbit
New Features
Chores