Gong - Add contentSelector to extensive call data #18090
Conversation
Co-authored-by: Jorge Cortes <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
@jocarino is attempting to deploy a commit to the Pipedreamers Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdded new optional props to Get Extensive Data, introduced utils.parseObject to normalize inputs, constructed and sent a contentSelector in POST /v2/calls/extensive, bumped action/package versions, and updated several modules' version metadata. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Action as GetExtensiveData
participant Utils as utils.parseObject
participant API as Gong API (/v2/calls/extensive)
Client->>Action: invoke with filter + new props
Action->>Utils: parseObject(exposedFieldsContent / exposedFieldsInteraction / others)
Utils-->>Action: normalized objects & booleans
Action->>Action: build contentSelector (context, [contextTiming], parties, exposedFields, publicComments, media)
Action->>API: POST /v2/calls/extensive { filter, contentSelector }
API-->>Action: paginated results
Action-->>Client: aggregated results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Thank you so much for submitting this! We've added it to our backlog to review, and our team has been notified. |
|
Thanks for submitting this PR! When we review PRs, we follow the Pipedream component guidelines. If you're not familiar, here's a quick checklist:
|
andrewjschuang
moved this to Ready for PR Review
in Component (Source and Action) Backlog
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
components/gong/actions/get-extensive-data/get-extensive-data.mjs (1)
69-75: AligncontextTimingdefaults with description and constrain input choicesYour description says the default is ['Now'], but the code defaults to an empty array. Also, adding explicit options helps prevent invalid values in the UI.
Apply this diff:
contextTiming: { type: "string[]", label: "Context Timing", description: "Allowed: Now, TimeOfCall. Timing for the context data. The field is optional and can contain either 'Now' or 'TimeOfCall' or both. The default value is ['Now']. Can be provided only when the context field is set to 'Extended'", - default: [], + options: [ + "Now", + "TimeOfCall", + ], + default: [ + "Now", + ], optional: true, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
components/gong/actions/get-extensive-data/get-extensive-data.mjs(3 hunks)components/gong/package.json(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). (3)
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
🔇 Additional comments (3)
components/gong/package.json (1)
3-3: Semver bump looks goodMinor version bump to 0.3.0 aligns with added functionality in the action. No concerns.
components/gong/actions/get-extensive-data/get-extensive-data.mjs (2)
8-9: Docs link update and version bump: LGTMDescription URL and action version updated appropriately.
180-184: Confirm the “media” field path underexposedFieldsDouble-check Gong’s schema for the extensive endpoint: is
mediaa property underexposedFields, or a sibling to it/top-level undercontentSelector? Same question forcollaboration.publicComments. If the path is different, the API may ignore these flags.Would you confirm against the latest Gong API docs for POST /v2/calls/extensive? If needed, I can update the mapping once we confirm the correct shape.
components/gong/actions/get-extensive-data/get-extensive-data.mjs
Outdated
Show resolved
Hide resolved
luancazarine
moved this from Ready for PR Review
to In Review
in Component (Source and Action) Backlog
There was a problem hiding this comment.
Hello @jocarino. Thanks for your contribution! I just added some suggestions. Please let me know if you need any help.
components/gong/actions/get-extensive-data/get-extensive-data.mjs
Outdated
Show resolved
Hide resolved
components/gong/actions/get-extensive-data/get-extensive-data.mjs
Outdated
Show resolved
Hide resolved
components/gong/actions/get-extensive-data/get-extensive-data.mjs
Outdated
Show resolved
Hide resolved
luancazarine
moved this from In Review
to Changes Required
in Component (Source and Action) Backlog
|
@luancazarine Thanks for taking a look this! I've made the suggested changes. Lmk if these make sense |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
components/gong/actions/get-extensive-data/get-extensive-data.mjs (2)
158-188: Crash risk on contextTiming.length, boolean coercion issues, and backward-compat for contentSelector
- contextTiming is optional; accessing length can throw.
- Using "|| false" with string values (e.g. "false") yields truthy strings, not booleans.
- Always sending an all-false contentSelector can change API defaults; include it only when the user opts in.
Apply this diff to guard usage, coerce booleans, and only include contentSelector when it deviates from defaults:
- const contentSelector = { - "context": context || "None", - ...(contextTiming.length > 0 && { - "contextTiming": contextTiming, - }), - "exposedFields": { - "parties": includeParties || false, - "content": { - "structure": exposedFieldsContentObj?.structure || false, - "topics": exposedFieldsContentObj?.topics || false, - "trackers": exposedFieldsContentObj?.trackers || false, - "trackerOccurrences": exposedFieldsContentObj?.trackerOccurrences || false, - "pointsOfInterest": exposedFieldsContentObj?.pointsOfInterest || false, - "brief": exposedFieldsContentObj?.brief || false, - "outline": exposedFieldsContentObj?.outline || false, - "highlights": exposedFieldsContentObj?.highlights || false, - "callOutcome": exposedFieldsContentObj?.callOutcome || false, - "keyPoints": exposedFieldsContentObj?.keyPoints || false, - }, - "interaction": { - "speakers": exposedFieldsInteractionObj?.speakers || false, - "video": exposedFieldsInteractionObj?.video || false, - "personInteractionStats": exposedFieldsInteractionObj?.personInteractionStats || false, - "questions": exposedFieldsInteractionObj?.questions || false, - }, - "collaboration": { - "publicComments": includePublicComments || false, - }, - "media": includeMedia || false, - }, - }; + // Validate prop relationships + if (context !== "Extended" && (contextTiming?.length ?? 0) > 0) { + throw new ConfigurationError("`contextTiming` can only be provided when `context` is set to 'Extended'"); + } + + // Determine if any exposed fields are requested (coerce to booleans) + const hasContent = + !!(exposedFieldsContentObj.structure || exposedFieldsContentObj.topics || + exposedFieldsContentObj.trackers || exposedFieldsContentObj.trackerOccurrences || + exposedFieldsContentObj.pointsOfInterest || exposedFieldsContentObj.brief || + exposedFieldsContentObj.outline || exposedFieldsContentObj.highlights || + exposedFieldsContentObj.callOutcome || exposedFieldsContentObj.keyPoints); + const hasInteraction = + !!(exposedFieldsInteractionObj.speakers || exposedFieldsInteractionObj.video || + exposedFieldsInteractionObj.personInteractionStats || exposedFieldsInteractionObj.questions); + const hasExposedFields = + !!(includeParties || hasContent || hasInteraction || includePublicComments || includeMedia); + + // Only include contentSelector when it deviates from API defaults + const contentSelector = ( + (context && context !== "None") || + (context === "Extended" && (contextTiming?.length ?? 0) > 0) || + hasExposedFields + ) ? { + ...(context && context !== "None" && { context }), + ...(context === "Extended" && (contextTiming?.length ?? 0) > 0 && { contextTiming }), + ...(hasExposedFields && { + exposedFields: { + ...(includeParties && { parties: true }), + ...(hasContent && { + content: { + structure: !!exposedFieldsContentObj.structure, + topics: !!exposedFieldsContentObj.topics, + trackers: !!exposedFieldsContentObj.trackers, + trackerOccurrences: !!exposedFieldsContentObj.trackerOccurrences, + pointsOfInterest: !!exposedFieldsContentObj.pointsOfInterest, + brief: !!exposedFieldsContentObj.brief, + outline: !!exposedFieldsContentObj.outline, + highlights: !!exposedFieldsContentObj.highlights, + callOutcome: !!exposedFieldsContentObj.callOutcome, + keyPoints: !!exposedFieldsContentObj.keyPoints, + }, + }), + ...(hasInteraction && { + interaction: { + speakers: !!exposedFieldsInteractionObj.speakers, + video: !!exposedFieldsInteractionObj.video, + personInteractionStats: !!exposedFieldsInteractionObj.personInteractionStats, + questions: !!exposedFieldsInteractionObj.questions, + }, + }), + ...(includePublicComments && { + collaboration: { + publicComments: true, + }, + }), + ...(includeMedia && { media: true }), + }, + }), + } : null;
195-198: Send contentSelector conditionally to preserve prior behaviorAvoid including an empty selector; let the API default behavior apply when nothing is explicitly requested.
data: { filter, - contentSelector, + ...(contentSelector ? { contentSelector } : {}), },
🧹 Nitpick comments (4)
components/gong/actions/get-extensive-data/get-extensive-data.mjs (4)
70-75: Clarify default and guard usage of contextTiming
- The description states default ['Now'], but no default is set; either set a default or clarify it’s the API default when omitted.
- Since this prop is optional, later usage must not assume it’s defined.
Would you like me to update the prop description to “When omitted, API defaults to ['Now']” and/or set a default of [] to avoid undefined?
83-100: Use boolean defaults instead of string "false" for exposedFieldsContentThese defaults are strings, which are truthy and can leak into the payload as "false" (string). Prefer real booleans. This also reduces reliance on parsing.
Apply this diff:
default: { - "structure": "false", - "topics": "false", - "trackers": "false", - "trackerOccurrences": "false", - "pointsOfInterest": "false", - "brief": "false", - "outline": "false", - "highlights": "false", - "callOutcome": "false", - "keyPoints": "false", + "structure": false, + "topics": false, + "trackers": false, + "trackerOccurrences": false, + "pointsOfInterest": false, + "brief": false, + "outline": false, + "highlights": false, + "callOutcome": false, + "keyPoints": false, },
101-112: Same here: prefer boolean defaults for exposedFieldsInteractionAvoid string "false" defaults for the same reason.
default: { - "speakers": "false", - "video": "false", - "personInteractionStats": "false", - "questions": "false", + "speakers": false, + "video": false, + "personInteractionStats": false, + "questions": false, },
155-157: Normalize parsed objects and provide safe fallbacksEven with parsing, keep safe defaults to avoid repetitive optional chaining and potential undefined reads.
- const exposedFieldsContentObj = utils.parseObject(exposedFieldsContent); - const exposedFieldsInteractionObj = utils.parseObject(exposedFieldsInteraction); + const exposedFieldsContentObj = utils.parseObject(exposedFieldsContent) ?? {}; + const exposedFieldsInteractionObj = utils.parseObject(exposedFieldsInteraction) ?? {};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
components/gong/actions/get-extensive-data/get-extensive-data.mjs(3 hunks)components/gong/common/utils.mjs(3 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). (3)
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (2)
components/gong/common/utils.mjs (1)
80-83: Exporting parseObject looks goodIncluding parseObject in the default export is correct and matches usage elsewhere.
components/gong/actions/get-extensive-data/get-extensive-data.mjs (1)
9-11: Metadata update LGTMDescription link and version bump look good.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
components/gong/actions/get-extensive-data/get-extensive-data.mjs (3)
155-188: Coerce string booleans, validatecontextTimingrelationship, and only buildcontentSelectorwhen non-default
- Coerce all boolean-like inputs (including "true"/"false" strings) to actual booleans.
- Enforce:
contextTimingis only allowed withcontext === "Extended".- Preserve backward compatibility: omit
contentSelectorentirely unless the user opted-in (context != None, has timing, or any exposed field selection). Avoid sending an all-false selector.Apply this diff:
- const exposedFieldsContentObj = utils.parseObject(exposedFieldsContent); - const exposedFieldsInteractionObj = utils.parseObject(exposedFieldsInteraction); - - const contentSelector = { - "context": context || "None", - ...(contextTiming.length > 0 && { - "contextTiming": contextTiming, - }), - "exposedFields": { - "parties": includeParties || false, - "content": { - "structure": exposedFieldsContentObj?.structure || false, - "topics": exposedFieldsContentObj?.topics || false, - "trackers": exposedFieldsContentObj?.trackers || false, - "trackerOccurrences": exposedFieldsContentObj?.trackerOccurrences || false, - "pointsOfInterest": exposedFieldsContentObj?.pointsOfInterest || false, - "brief": exposedFieldsContentObj?.brief || false, - "outline": exposedFieldsContentObj?.outline || false, - "highlights": exposedFieldsContentObj?.highlights || false, - "callOutcome": exposedFieldsContentObj?.callOutcome || false, - "keyPoints": exposedFieldsContentObj?.keyPoints || false, - }, - "interaction": { - "speakers": exposedFieldsInteractionObj?.speakers || false, - "video": exposedFieldsInteractionObj?.video || false, - "personInteractionStats": exposedFieldsInteractionObj?.personInteractionStats || false, - "questions": exposedFieldsInteractionObj?.questions || false, - }, - "collaboration": { - "publicComments": includePublicComments || false, - }, - "media": includeMedia || false, - }, - }; + // Validate prop relationships + if (context !== "Extended" && (contextTiming?.length ?? 0) > 0) { + throw new ConfigurationError("`contextTiming` can only be provided when `context` is set to 'Extended'"); + } + + // Coerce string booleans ("true"/"false") and other truthy/falsy values to boolean + const toBool = (v) => typeof v === "string" ? v.toLowerCase() === "true" : !!v; + + // Defensive parsing and defaults + const efc = utils.parseObject(exposedFieldsContent) ?? {}; + const efi = utils.parseObject(exposedFieldsInteraction) ?? {}; + + const hasContent = + toBool(efc.structure) || + toBool(efc.topics) || + toBool(efc.trackers) || + toBool(efc.trackerOccurrences) || + toBool(efc.pointsOfInterest) || + toBool(efc.brief) || + toBool(efc.outline) || + toBool(efc.highlights) || + toBool(efc.callOutcome) || + toBool(efc.keyPoints); + + const hasInteraction = + toBool(efi.speakers) || + toBool(efi.video) || + toBool(efi.personInteractionStats) || + toBool(efi.questions); + + const hasExposedFields = + toBool(includeParties) || + hasContent || + hasInteraction || + toBool(includePublicComments) || + toBool(includeMedia); + + const contentSelector = ( + (context && context !== "None") || + (context === "Extended" && (contextTiming?.length ?? 0) > 0) || + hasExposedFields + ) ? { + ...(context && context !== "None" && { context }), + ...(context === "Extended" && (contextTiming?.length ?? 0) > 0 && { contextTiming }), + ...(hasExposedFields && { + exposedFields: { + ...(toBool(includeParties) && { parties: true }), + ...(hasContent && { + content: { + structure: toBool(efc.structure), + topics: toBool(efc.topics), + trackers: toBool(efc.trackers), + trackerOccurrences: toBool(efc.trackerOccurrences), + pointsOfInterest: toBool(efc.pointsOfInterest), + brief: toBool(efc.brief), + outline: toBool(efc.outline), + highlights: toBool(efc.highlights), + callOutcome: toBool(efc.callOutcome), + keyPoints: toBool(efc.keyPoints), + }, + }), + ...(hasInteraction && { + interaction: { + speakers: toBool(efi.speakers), + video: toBool(efi.video), + personInteractionStats: toBool(efi.personInteractionStats), + questions: toBool(efi.questions), + }, + }), + ...(toBool(includePublicComments) && { + collaboration: { + publicComments: true, + }, + }), + ...(toBool(includeMedia) && { media: true }), + }, + }), + } : null;
196-198: SendcontentSelectorconditionally to preserve previous behaviorAvoid sending an empty selector; let the API apply its defaults unless the user explicitly opts in.
Apply this diff:
data: { filter, - contentSelector, + ...(contentSelector ? { contentSelector } : {}), },
160-162: Potential runtime error: unsafe access tocontextTiming.length
contextTimingis optional. Accessing.lengthwithout a guard will throw when undefined.If you keep the current structure, minimally guard with optional chaining:
- ...(contextTiming.length > 0 && { + ...(contextTiming?.length > 0 && { "contextTiming": contextTiming, }),Note:
contextTimingshould only be sent whencontext === "Extended". See comprehensive refactor below.
🧹 Nitpick comments (1)
components/gong/actions/get-extensive-data/get-extensive-data.mjs (1)
70-75: Constrain and clarifycontextTiminginput
- Enforce allowed values to prevent invalid payloads.
- Optional: set default to ["Now"] only when
context === "Extended"to match the API docs without forcing a value whencontextis not Extended.Apply this diff to restrict values at the prop level:
contextTiming: { type: "string[]", label: "Context Timing", description: "Allowed: Now, TimeOfCall. Timing for the context data. The field is optional and can contain either 'Now' or 'TimeOfCall' or both. The default value is ['Now']. Can be provided only when the context field is set to 'Extended'", optional: true, + options: [ + "Now", + "TimeOfCall", + ], },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
components/gong/actions/get-extensive-data/get-extensive-data.mjs(3 hunks)components/gong/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/gong/package.json
⏰ 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). (5)
- GitHub Check: Lint Code Base
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Ensure component commits modify component versions
🔇 Additional comments (1)
components/gong/actions/get-extensive-data/get-extensive-data.mjs (1)
9-11: Docs link + version bump look goodThe updated description URL and version bump are appropriate for the new props and payload changes.
components/gong/actions/get-extensive-data/get-extensive-data.mjs
Outdated
Show resolved
Hide resolved
|
@luancazarine lmk if you need anything else from me |
luancazarine
moved this from Changes Required
to Ready for QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from Ready for QA
to Changes Required
in Component (Source and Action) Backlog
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check the test report below for more information |
|
@vunguyenhung Good spots, I've pushed a fix to address the length issue (missed a null check), and the description |
vunguyenhung
moved this from Changes Required
to Ready for PR Review
in Component (Source and Action) Backlog
WHY
Summary by CodeRabbit
New Features
Documentation
Chores