-
Notifications
You must be signed in to change notification settings - Fork 5.4k
[Components] ContactOut - new components #17026
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
""" WalkthroughA comprehensive ContactOut integration was introduced, implementing multiple action modules for searching, enriching, and verifying contacts, companies, and emails. The core app module now includes extensive property definitions and API method implementations for all ContactOut endpoints, enabling a wide range of search and enrichment workflows. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Action
participant ContactOutApp
participant ContactOutAPI
User->>Action: Provide input parameters
Action->>ContactOutApp: Call corresponding API method with parameters
ContactOutApp->>ContactOutAPI: Make HTTP request to endpoint
ContactOutAPI-->>ContactOutApp: Return response
ContactOutApp-->>Action: Return processed response
Action-->>User: Output result/summary
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/contactout/actions/decision-makers-search/decision-makers-search.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/contactout/actions/company-search/company-search.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/contactout/actions/email-to-linkedin/email-to-linkedin.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (10)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (4)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ad658ed
to
463d728
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 6
🧹 Nitpick comments (8)
components/contactout/actions/verify-email/verify-email.mjs (1)
1-35
: LGTM - Clean implementation with opportunity for error handling enhancement.The action is well-structured and follows expected patterns. Consider adding error handling for API failures to provide better user experience.
Consider wrapping the API call in try-catch for better error handling:
async run({ $ }) { const { app, email, } = this; + try { const response = await app.verifyEmail({ $, params: { email, }, }); $.export("$summary", "Successfully verified email"); return response; + } catch (error) { + $.export("$summary", `Failed to verify email: ${error.message}`); + throw error; + } },components/contactout/actions/company-search/company-search.mjs (1)
94-112
: LGTM - Good parameter mapping with opportunity for error handling.The camelCase to snake_case parameter mapping is implemented correctly. Consider adding error handling for API failures.
Consider adding try-catch for better error handling:
+ try { const response = await this.app.searchCompanies({ $, data: { name, domain, size, hq_only: hqOnly, location, industries, min_revenue: minRevenue, max_revenue: maxRevenue, year_founded_from: yearFoundedFrom, year_founded_to: yearFoundedTo, }, }); $.export("$summary", "Successfully searched for companies"); return response; + } catch (error) { + $.export("$summary", `Failed to search companies: ${error.message}`); + throw error; + }components/contactout/actions/get-contact-info-by-member-id/get-contact-info-by-member-id.mjs (1)
25-42
: LGTM - Clean conditional parameter handling.The action properly handles optional parameters and implements correct camelCase to snake_case mapping. Consider adding error handling for consistency with other actions.
Consider adding try-catch for better error handling:
async run({ $ }) { const params = { member_id: this.memberId, }; if (this.includePhone) { params.include_phone = this.includePhone; } + try { const response = await this.app.getContactInfoByMemberId({ $, params, }); $.export("$summary", `Successfully retrieved contact info for LinkedIn Member ID: ${this.memberId}`); return response; + } catch (error) { + $.export("$summary", `Failed to retrieve contact info: ${error.message}`); + throw error; + } },components/contactout/actions/get-contact-info/get-contact-info.mjs (2)
16-16
: Consider adding URL validation for LinkedIn profiles.The description specifies that the URL "must begin with http and must contain linkedin.com/in/ or linkedin.com/pub/" but there's no validation to enforce this constraint.
Add URL validation to the profile prop definition or in the run method:
profile: { propDefinition: [ app, "profile", ], description: "The LinkedIn profile URL to get contact information for. URL must begin with http and must contain linkedin.com/in/ or linkedin.com/pub/", + validate: (value) => { + if (!value.startsWith('http')) { + return 'URL must begin with http'; + } + if (!value.includes('linkedin.com/in/') && !value.includes('linkedin.com/pub/')) { + return 'URL must contain linkedin.com/in/ or linkedin.com/pub/'; + } + return true; + }, },
31-52
: LGTM - Excellent conditional parameter handling.The action properly handles multiple optional parameters with clean conditional logic and correct parameter mapping.
Consider adding try-catch for better error handling:
+ try { const response = await this.app.getContactInfo({ $, params, }); $.export("$summary", `Successfully retrieved contact info for LinkedIn profile: ${this.profile}`); return response; + } catch (error) { + $.export("$summary", `Failed to retrieve contact info: ${error.message}`); + throw error; + }components/contactout/actions/people-search/people-search.mjs (1)
77-82
: Consider using propDefinition for consistency.The
domain
prop is defined locally while all other props usepropDefinition
from the app. For consistency and maintainability, consider defining this in the app's prop definitions.- domain: { - type: "string[]", - label: "Domains", - description: "Array of company domains to search for (max 50)", - optional: true, - }, + domain: { + propDefinition: [ + app, + "domain", + ], + type: "string[]", + description: "Array of company domains to search for (max 50)", + },components/contactout/actions/get-company-info/get-company-info.mjs (1)
25-26
: Make the success message more informative.The current success message is generic. Consider including the number of companies retrieved to provide more context.
- $.export("$summary", "Successfully retrieved company information"); + $.export("$summary", `Successfully retrieved information for ${this.domains.length} domain${this.domains.length === 1 ? "" : "s"}`);components/contactout/actions/verify-email-bulk/verify-email-bulk.mjs (1)
42-43
: Consider making the polling delay configurable.The polling delay is hardcoded to 15 seconds, which might not be optimal for all use cases. Consider adding it as an optional prop.
Add a new prop after
waitForCompletion
:pollingInterval: { type: "integer", label: "Polling Interval (seconds)", description: "How often to check the job status when waiting for completion", default: 15, min: 5, max: 300, optional: true, },Then update the delay calculation:
-const DELAY = 1000 * 15; // 15 seconds +const DELAY = 1000 * (this.pollingInterval || 15);
🛑 Comments failed to post (6)
components/contactout/actions/company-search/company-search.mjs (1)
66-78: 🛠️ Refactor suggestion
Add validation for year range consistency.
The
yearFoundedTo
description mentions it "requires Year Founded From" but there's no validation to enforce this constraint.Add validation in the run method:
async run({ $ }) { const { name, domain, size, hqOnly, location, industries, minRevenue, maxRevenue, yearFoundedFrom, yearFoundedTo, } = this; + if (yearFoundedTo && !yearFoundedFrom) { + throw new Error("Year Founded From is required when Year Founded To is specified"); + } + + if (yearFoundedFrom && yearFoundedTo && yearFoundedFrom > yearFoundedTo) { + throw new Error("Year Founded From cannot be greater than Year Founded To"); + }📝 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.async run({ $ }) { const { name, domain, size, hqOnly, location, industries, minRevenue, maxRevenue, yearFoundedFrom, yearFoundedTo, } = this; if (yearFoundedTo && !yearFoundedFrom) { throw new Error( "Year Founded From is required when Year Founded To is specified" ); } if (yearFoundedFrom && yearFoundedTo && yearFoundedFrom > yearFoundedTo) { throw new Error( "Year Founded From cannot be greater than Year Founded To" ); } // …rest of existing logic… }
🤖 Prompt for AI Agents
In components/contactout/actions/company-search/company-search.mjs around lines 66 to 78, the yearFoundedTo field states it requires yearFoundedFrom, but no validation enforces this. Add a validation check in the run method to ensure that if yearFoundedTo is provided, yearFoundedFrom must also be present. If yearFoundedTo is given without yearFoundedFrom, throw an error or return a validation failure indicating the dependency requirement.
components/contactout/actions/get-company-info/get-company-info.mjs (1)
11-16: 💡 Verification agent
🧩 Analysis chain
Add validation for domain count and format constraints.
The description mentions a maximum of 30 domains and valid domain format requirements, but there's no validation to enforce these constraints. This could lead to API errors if invalid data is passed.
Add validation before the API call:
async run({ $ }) { + // Validate domain count + if (this.domains.length > 30) { + throw new ConfigurationError("Maximum 30 domains allowed"); + } + + // Validate domain format + const domainRegex = /^([a-z0-9]+(-[a-z0-9]+)*\.)+[a-z]{2,}$/i; + const invalidDomains = this.domains.filter(domain => !domainRegex.test(domain)); + if (invalidDomains.length > 0) { + throw new ConfigurationError(`Invalid domain format: ${invalidDomains.join(", ")}`); + } + const response = await this.app.getCompanyInfo({Also, import
ConfigurationError
at the top:import { ConfigurationError } from "@pipedream/platform";Run the following script to check if other ContactOut actions have similar validation:
🏁 Script executed:
#!/bin/bash # Check for validation patterns in other ContactOut actions fd -e mjs . components/contactout/actions/ --exec grep -l "ConfigurationError\|throw.*Error" {} \;Length of output: 255
Add domain count and format validation in get-company-info
Please enforce the constraints declared in the description before calling the ContactOut API:
• File:
components/contactout/actions/get-company-info/get-company-info.mjs
- Import the
ConfigurationError
helper:import { ConfigurationError } from "@pipedream/platform";- In the
async run({ $ })
method, beforethis.app.getCompanyInfo
, insert:async run({ $ }) { + // Validate maximum number of domains + if (this.domains.length > 30) { + throw new ConfigurationError("Maximum 30 domains allowed"); + } + + // Validate each domain’s format + const domainRegex = /^([a-z0-9]+(-[a-z0-9]+)*\.)+[a-z]{2,}$/i; + const invalid = this.domains.filter(d => !domainRegex.test(d)); + if (invalid.length) { + throw new ConfigurationError( + `Invalid domain format: ${invalid.join(", ")}` + ); + } const response = await this.app.getCompanyInfo({ domains: this.domains, });• Note: Other ContactOut actions (e.g.
verify-email-bulk
,linkedin-profile-enrich
) already import and throwConfigurationError
but don’t enforce array‐length or format checks. Consider applying similar input validations there for consistency.🤖 Prompt for AI Agents
In components/contactout/actions/get-company-info/get-company-info.mjs around lines 11 to 16, add validation to enforce the maximum of 30 domains and ensure each domain is in a valid format before calling this.app.getCompanyInfo. First, import ConfigurationError from "@pipedream/platform" at the top of the file. Then, inside the async run({ $ }) method, before the API call, check if the domains array length exceeds 30 or if any domain is invalid, and throw ConfigurationError with a descriptive message if validation fails. This will prevent invalid input from causing API errors.
components/contactout/actions/verify-email-bulk/verify-email-bulk.mjs (1)
84-87:
⚠️ Potential issueAdd null-safe handling for result count.
The code assumes
statusResponse.data.result
is an object with keys, but this might not always be the case, potentially causing a runtime error.if (statusResponse.data?.status === "DONE") { - $.export("$summary", `Bulk email verification job \`${jobId}\` completed successfully. Verified ${Object.keys(statusResponse.data.result || {}).length} emails.`); + const resultCount = statusResponse.data?.result + ? (Array.isArray(statusResponse.data.result) + ? statusResponse.data.result.length + : Object.keys(statusResponse.data.result).length) + : 0; + $.export("$summary", `Bulk email verification job \`${jobId}\` completed successfully. Verified ${resultCount} emails.`); return statusResponse; }📝 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.if (statusResponse.data?.status === "DONE") { const resultCount = statusResponse.data?.result ? (Array.isArray(statusResponse.data.result) ? statusResponse.data.result.length : Object.keys(statusResponse.data.result).length) : 0; $.export("$summary", `Bulk email verification job \`${jobId}\` completed successfully. Verified ${resultCount} emails.`); return statusResponse; }
🤖 Prompt for AI Agents
In components/contactout/actions/verify-email-bulk/verify-email-bulk.mjs around lines 84 to 87, the code assumes statusResponse.data.result is always an object and uses Object.keys on it, which can cause runtime errors if result is null or not an object. To fix this, add a null-safe check to ensure result is an object before calling Object.keys, for example by verifying its type or using a default empty object, so the length calculation does not throw an error.
components/contactout/contactout.app.mjs (3)
28-31: 🛠️ Refactor suggestion
Add validation for array size limits in prop definitions.
Multiple array props mention size limits (emails: max 100, jobTitle: max 50, etc.) but don't enforce them. This could lead to API errors when limits are exceeded.
Consider adding validation using the
validate
property for array props. Example for the emails prop:emails: { type: "string[]", label: "Email Addresses", description: "Array of email addresses (max 100 for bulk operations)", + validate: (value) => { + if (!Array.isArray(value)) return true; + if (value.length > 100) { + return "Maximum 100 email addresses allowed"; + } + return true; + }, },Run the following script to identify all array props with mentioned limits:
#!/bin/bash # Find all array prop definitions that mention "max" in their descriptions ast-grep --pattern 'type: "string[]",$$$description: $_,' components/contactout/contactout.app.mjs | grep -B2 -A2 "max"🤖 Prompt for AI Agents
In components/contactout/contactout.app.mjs around lines 28 to 31, the array props such as "Email Addresses" mention maximum size limits in their descriptions but lack validation to enforce these limits. Add a validate function to these prop definitions that checks the array length does not exceed the specified maximum (e.g., 100 for emails). This validation should return true if valid or an error message if the limit is exceeded. Repeat this for all array props with size limits mentioned in their descriptions.
345-353: 🛠️ Refactor suggestion
Add error handling to the base request method.
The
_makeRequest
method lacks error handling, which could make debugging API errors difficult._makeRequest({ $ = this, path, headers, ...args }) { - return axios($, { - ...args, - url: this.getUrl(path), - headers: this.getHeaders(headers), - }); + return axios($, { + ...args, + url: this.getUrl(path), + headers: this.getHeaders(headers), + }).catch((error) => { + const message = error.response?.data?.message || error.message; + const status = error.response?.status; + throw new Error(`ContactOut API error (${status}): ${message}`); + }); },📝 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._makeRequest({ $ = this, path, headers, ...args }) { return axios($, { ...args, url: this.getUrl(path), headers: this.getHeaders(headers), }).catch((error) => { const message = error.response?.data?.message || error.message; const status = error.response?.status; throw new Error(`ContactOut API error (${status}): ${message}`); }); },
🤖 Prompt for AI Agents
In components/contactout/contactout.app.mjs around lines 345 to 353, the _makeRequest method currently does not handle errors, which can obscure API failure causes. Modify this method to include a try-catch block around the axios call, catching any errors thrown during the request. In the catch block, log or rethrow the error with additional context to improve debugging and error traceability.
338-344: 💡 Verification agent
🧩 Analysis chain
Verify the authorization header structure.
The authorization header structure appears unusual - it sets
authorization: "basic"
but uses a separatetoken
header instead of the standardAuthorization: Basic <credentials>
format.Please verify if this is the correct header format for the ContactOut API:
If this is incorrect, the standard Basic auth format would be:
getHeaders(headers) { return { ...headers, - authorization: "basic", - token: this.$auth.api_key, + Authorization: `Bearer ${this.$auth.api_key}`, }; },
🌐 Web query:
What is the correct authorization header format for ContactOut API v1?
💡 Result:
To authenticate requests to the ContactOut API v1, include your API token in the request headers using the following format:
token: <YOUR_API_TOKEN>
Replace
<YOUR_API_TOKEN>
with your actual API token. For example, using cURL:curl "https://api.contactout.com/v1/people/search" \ -H "token: <YOUR_API_TOKEN>"
You can obtain your API token by booking a meeting with ContactOut. (api.contactout.com)
Citations:
Remove non-standard authorization header for ContactOut API v1
ContactOut v1 expects only a
token: <YOUR_API_TOKEN>
header. Theauthorization: "basic"
entry is not used by their API and should be removed.• Update in components/contactout/contactout.app.mjs:
getHeaders(headers) { return { ...headers, - authorization: "basic", token: this.$auth.api_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.getHeaders(headers) { return { ...headers, token: this.$auth.api_key, }; },
🤖 Prompt for AI Agents
In components/contactout/contactout.app.mjs around lines 338 to 344, the getHeaders method incorrectly sets an authorization header with the value "basic" and a separate token header. The ContactOut API v1 expects only a token header with the API token and does not use the authorization header. Remove the authorization: "basic" entry from the returned headers object and keep only the token header with this.$auth.api_key as its value.
WHY
Resolves #16976
Summary by CodeRabbit