Activecampaign contact added to segment #18896
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Thank you so much for submitting this! We've added it to our backlog to review, and our team has been notified. |
WalkthroughAdds segment selection props and option resolution to the ActiveCampaign app, three API helper methods (segments/audiences/options), a new polling source "New Contact Added to Segment" with per-segment state, pagination, and a test-event, plus many version bumps across package/actions/sources. Changes
Sequence Diagram(s)sequenceDiagram
participant Timer as Timer
participant Source as New Contact<br/>Added to Segment
participant API as ActiveCampaign API
participant Store as Persistent Storage
Timer->>Source: Trigger (interval)
Source->>Source: Validate `segmentId`
Source->>API: listAudiences(segmentId) (resolve segment name)
API-->>Source: Segment info
Source->>API: getSegmentContacts(segmentId) [paginated]
API-->>Source: Contacts pages
Source->>Store: _getStoredContactIds(segmentKey)
Store-->>Source: storedIds
alt First run (no storedIds)
Source->>Store: _setStoredContactIds(currentIds)
else Subsequent runs
Source->>Source: Compute newIds = currentIds - storedIds
loop for each newId
Source->>API: getContactFieldValues(contactId)
API-->>Source: field values
Source->>Timer: Emit event (contact + metadata + addedAt)
end
Source->>Store: _setStoredContactIds(currentIds)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 3
🧹 Nitpick comments (4)
components/activecampaign/activecampaign.app.mjs (1)
441-446: /segments helper includedOK to keep for legacy/hidden segments. Consider adding a short JSDoc noting it returns legacy segments vs. /audiences to avoid confusion later.
components/activecampaign/sources/new-contact-added-to-segment/new-contact-added-to-segment.mjs (3)
47-72: Pagination logic OK; consider minor resilienceLoop and offset/limit are fine. Optionally guard against transient failures with a bounded retry/backoff on 429/5xx to improve reliability.
39-46: Event id includes a timestamp; consider a stable idUsing ts in id makes replays non-idempotent. Since you already track membership in db, a stable id like
${segmentId}-${contact.id}simplifies dedupe.Apply this diff:
- id: `${segmentInfo.segmentId}-${contact.id}-${ts}`, + id: `${segmentInfo.segmentId}-${contact.id}`,
127-137: addedAt is “now”; call this out in summary or include segment infoIf “addedAt” is the poll time (not true join time), consider reflecting that in summary or field name (e.g., detectedAt) to avoid confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
components/activecampaign/activecampaign.app.mjs(3 hunks)components/activecampaign/package.json(1 hunks)components/activecampaign/sources/campaign-bounce/campaign-bounce.mjs(1 hunks)components/activecampaign/sources/campaign-link-clicked/campaign-link-clicked.mjs(1 hunks)components/activecampaign/sources/campaign-opened/campaign-opened.mjs(1 hunks)components/activecampaign/sources/campaign-starts-sending/campaign-starts-sending.mjs(1 hunks)components/activecampaign/sources/contact-added-to-list/contact-added-to-list.mjs(1 hunks)components/activecampaign/sources/new-automation-webhook/new-automation-webhook.mjs(1 hunks)components/activecampaign/sources/new-campaign-unsubscribe/new-campaign-unsubscribe.mjs(1 hunks)components/activecampaign/sources/new-contact-added-to-segment/new-contact-added-to-segment.mjs(1 hunks)components/activecampaign/sources/new-contact-added-to-segment/test-event.mjs(1 hunks)components/activecampaign/sources/new-contact-note/new-contact-note.mjs(1 hunks)components/activecampaign/sources/new-contact-task/new-contact-task.mjs(1 hunks)components/activecampaign/sources/new-deal-note/new-deal-note.mjs(1 hunks)components/activecampaign/sources/new-deal-task/new-deal-task.mjs(1 hunks)components/activecampaign/sources/new-event/new-event.mjs(1 hunks)components/activecampaign/sources/new-or-updated-account/new-or-updated-account.mjs(1 hunks)components/activecampaign/sources/new-or-updated-calendar-feed/new-or-updated-calendar-feed.mjs(1 hunks)components/activecampaign/sources/new-or-updated-contact/new-or-updated-contact.mjs(1 hunks)components/activecampaign/sources/new-or-updated-deal/new-or-updated-deal.mjs(1 hunks)components/activecampaign/sources/updated-contact/updated-contact.mjs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
components/activecampaign/sources/new-contact-added-to-segment/new-contact-added-to-segment.mjs (1)
components/activecampaign/activecampaign.app.mjs (4)
limit(602-602)offset(603-603)response(496-501)response(503-505)
components/activecampaign/activecampaign.app.mjs (1)
components/activecampaign/sources/new-contact-added-to-segment/new-contact-added-to-segment.mjs (4)
response(54-60)response(84-88)segment(89-89)meta(135-135)
⏰ 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: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (22)
components/activecampaign/package.json (1)
3-3: LGTM! Standard version bump for package release.The version increment from 0.4.5 to 0.4.6 aligns with the addition of the new "Contact Added to Segment" polling source and related API enhancements.
components/activecampaign/sources/new-automation-webhook/new-automation-webhook.mjs (1)
10-10: LGTM! Metadata version update.Version bumped to 0.0.7 as part of the package release cycle. No functional changes.
components/activecampaign/sources/new-deal-note/new-deal-note.mjs (1)
9-9: LGTM! Routine version update.Metadata version bumped to 0.0.6. No functional changes.
components/activecampaign/sources/updated-contact/updated-contact.mjs (1)
8-8: LGTM! Metadata version update.Version bumped to 0.0.6 with no functional changes.
components/activecampaign/sources/new-or-updated-account/new-or-updated-account.mjs (1)
10-10: LGTM! Metadata version update.Version bumped to 0.0.5 as part of the release cycle. No functional changes.
components/activecampaign/sources/campaign-bounce/campaign-bounce.mjs (1)
9-9: LGTM! Routine version increment.Metadata version updated to 0.0.6. No behavioral changes.
components/activecampaign/sources/new-deal-task/new-deal-task.mjs (1)
9-9: LGTM! Metadata version update.Version bumped to 0.0.6 with no functional changes.
components/activecampaign/sources/new-contact-note/new-contact-note.mjs (1)
9-9: LGTM! Metadata version update.Version bumped to 0.0.6 as part of the release cycle. No functional changes.
components/activecampaign/sources/campaign-starts-sending/campaign-starts-sending.mjs (1)
8-8: Version bump looks good.The patch version increment from 0.0.5 to 0.0.6 is appropriate for this metadata-only update with no functional changes.
components/activecampaign/sources/new-or-updated-deal/new-or-updated-deal.mjs (1)
8-8: LGTM.Consistent version increment aligned with the package update.
components/activecampaign/sources/new-event/new-event.mjs (1)
10-10: Version update confirmed.No functional changes, version increment is appropriate.
components/activecampaign/sources/contact-added-to-list/contact-added-to-list.mjs (1)
10-10: Version increment approved.The bump to 0.0.7 (vs. 0.0.6 for most other sources) is noted, likely reflecting an earlier independent update to this file.
components/activecampaign/sources/new-campaign-unsubscribe/new-campaign-unsubscribe.mjs (1)
9-9: LGTM.Routine version increment with no functional impact.
components/activecampaign/sources/new-or-updated-calendar-feed/new-or-updated-calendar-feed.mjs (1)
10-10: Version update approved.The patch increment from 0.0.4 to 0.0.5 is consistent with routine maintenance.
components/activecampaign/sources/campaign-link-clicked/campaign-link-clicked.mjs (1)
9-9: LGTM.Routine metadata update consistent with the broader package version bump.
components/activecampaign/sources/campaign-opened/campaign-opened.mjs (1)
9-9: Version increment confirmed.Metadata-only change with no functional impact.
components/activecampaign/sources/new-contact-task/new-contact-task.mjs (1)
9-9: Version bump LGTMNo behavioral change. Keep changelog/package version in sync.
components/activecampaign/activecampaign.app.mjs (2)
80-89: Segments prop: good async options wiringLooks correct and follows existing options pattern.
90-97: Single Segment prop: good additionWorks with the new resolver for dropdowns.
components/activecampaign/sources/new-or-updated-contact/new-or-updated-contact.mjs (1)
8-8: Version bump verified and consistentFile correctly contains version 0.0.6 at line 8. Verified that 11 other AC source components share the same version, confirming consistency across the majority of sources. Version variations in other sources (0.0.7, 0.0.5, 0.0.1) appear intentional.
components/activecampaign/sources/new-contact-added-to-segment/test-event.mjs (1)
1-15: Sample payload is correctly formattedThe test event payload correctly uses camelCase (
firstName,lastName) which aligns with the ActiveCampaign v3 Contacts API specification. The Contact object uses firstName / lastName in requests/responses for the standard v3 endpoints. The reviewer's conditional concern about snake_case misalignment does not apply to this use case.components/activecampaign/sources/new-contact-added-to-segment/new-contact-added-to-segment.mjs (1)
82-95: Verify API endpoint availability and pagination structure before implementing fixThe review comment raises a valid concern—fetching only page 1 of
/audiencescould miss segments—but the proposed solution cannot be verified. Web searches do not provide access to the ActiveCampaign API v3 complete reference for the audiences endpoint. The fix assumes: (1) GET/audiences/{id}exists, (2)makeRequest()is available and correct, and (3) pagination metadata includesresp.meta.page.current_pageandresp.meta.page.last_page, none of which could be confirmed from public documentation.Before implementing, verify against the official ActiveCampaign API v3 reference that:
- The direct audience lookup endpoint exists and is the correct path
- The
listAudiencespagination response includes those specific metadata fields- The SDK's
makeRequest()method is the appropriate fallback approach
components/activecampaign/sources/new-contact-added-to-segment/new-contact-added-to-segment.mjs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Hi @choeqq. Thank you so much for your contribution! Once the suggested changes are made, this should be ready for QA.
Note: I tried making the changes, but was denied access by your repo.
components/activecampaign/sources/new-contact-added-to-segment/new-contact-added-to-segment.mjs
Show resolved
Hide resolved
components/activecampaign/sources/new-contact-added-to-segment/new-contact-added-to-segment.mjs
Show resolved
Hide resolved
components/activecampaign/sources/new-contact-added-to-segment/new-contact-added-to-segment.mjs
Outdated
Show resolved
Hide resolved
components/activecampaign/sources/new-contact-added-to-segment/new-contact-added-to-segment.mjs
Show resolved
Hide resolved
michelle0927
moved this from In Review
to Changes Required
in Component (Source and Action) Backlog
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
components/activecampaign/activecampaign.app.mjs (2)
447-452: Do not merge:/audiencesendpoint remains undocumented.The GET /api/3/audiences endpoint is not officially documented by ActiveCampaign. As noted in the previous review, this creates maintenance and stability risks. Before merging:
- Verify endpoint support and stability with ActiveCampaign
- Confirm the actual response schema matches expectations (standard
data+meta.pagevs LIME-style collection)- Document why this undocumented endpoint is required
493-523: Fix unsafe property access and error handling structure.Several critical issues that will cause crashes:
- Line 494:
prevContext.pagewill throw TypeError whenprevContextis undefined on first invocation- Line 505: Error handler returns
[]but should return{options: [], context: {page: null}}- Lines 508-510: Destructuring
responsewithout guards—iflistAudiencesreturnsundefinedor an unexpected structure, this will crash- Lines 513, 518: Assumes
datais an array andmeta.pageexists with the expected propertiesApply this diff to add proper guards:
- async listSegmentOptions(prevContext) { - const page = prevContext.page || 1; + async listSegmentOptions(prevContext = {}) { + const page = prevContext?.page ?? 1; let response; try { response = await this.listAudiences({ params: { sort: "-recent", page, }, }); } catch { - return []; + return { + options: [], + context: { + page: null, + }, + }; } - const { - data, meta, - } = response; + const { data = [], meta = {} } = response || {}; return { options: data.map((segment) => ({ label: segment.attributes.name, value: segment.id, })), context: { - page: meta.page.current_page < meta.page.last_page + page: meta?.page?.current_page < meta?.page?.last_page ? page + 1 : null, }, }; },
🧹 Nitpick comments (2)
components/activecampaign/sources/new-contact-added-to-segment/new-contact-added-to-segment.mjs (2)
92-105: Consider fetching segment by ID or document the limitation.The code only fetches the first page of audiences to find the segment name. If the segment appears on page 2 or later, it will use the fallback "Segment {id}" format. While the fallback prevents errors, this could be confusing for users.
Consider either:
- Fetching the specific segment by ID (if a GET
/audiences/{id}endpoint exists)- Paginating through all pages until the segment is found
- Adding a comment documenting this limitation
143-143: Consider timestamp format consistency.Line 143 uses
new Date().toISOString()foraddedAt, while thegenerateMetamethod (line 49) usesDate.now()for the metadata timestamp. While both serve different purposes, using consistent formats throughout (either both ISO strings or both numeric timestamps) would improve code clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/activecampaign/activecampaign.app.mjs(3 hunks)components/activecampaign/sources/new-contact-added-to-segment/new-contact-added-to-segment.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-10T19:18:27.998Z
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#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/activecampaign/sources/new-contact-added-to-segment/new-contact-added-to-segment.mjs
🧬 Code graph analysis (2)
components/activecampaign/activecampaign.app.mjs (1)
components/activecampaign/sources/new-contact-added-to-segment/new-contact-added-to-segment.mjs (4)
response(63-70)response(94-98)segment(99-99)meta(146-146)
components/activecampaign/sources/new-contact-added-to-segment/new-contact-added-to-segment.mjs (1)
components/activecampaign/activecampaign.app.mjs (4)
limit(607-607)offset(608-608)response(496-496)response(508-510)
⏰ 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: Publish TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
🔇 Additional comments (8)
components/activecampaign/activecampaign.app.mjs (2)
80-97: LGTM: PropDefinitions follow established patterns.The
segmentsandsegmentpropDefinitions are well-structured and consistent with other prop definitions in this file.
441-446: LGTM: Follows standard pattern.The
listSegmentsmethod follows the same pattern used by other list methods in this file.components/activecampaign/sources/new-contact-added-to-segment/new-contact-added-to-segment.mjs (6)
1-28: LGTM: Component structure follows guidelines.The component metadata, imports, and props are properly structured and follow Pipedream's component development guidelines.
30-38: LGTM: Proper first-run detection.The initialization tracking methods correctly address the past review concern about segments that initially have zero contacts.
39-47: LGTM: State management methods.The contact ID storage methods are correctly implemented with per-segment keys.
48-55: LGTM: Proper event metadata generation.The
generateMetamethod creates appropriate unique IDs and summaries for event deduplication.
56-82: LGTM: Proper pagination handling.The
getSegmentContactsmethod correctly paginates through all contacts in a segment.
112-155: LGTM: Polling logic correctly implemented.The main run logic properly handles:
- First-run initialization without emitting retroactive events (per learnings)
- Detecting newly added contacts by comparing current vs stored IDs
- Emitting events with full contact data and metadata
- Updating stored state
Based on learnings
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
components/activecampaign/activecampaign.app.mjs (2)
449-460: The undocumented/audiencesendpoint has already been flagged as a critical issue.A previous review identified that the GET
/api/3/audiencesendpoint is not officially documented by ActiveCampaign and may use a non-standard LIME-style collection format rather than the expecteddata+meta.pagestructure.
501-531: Critical null-safety issues remain unresolved.While the try-catch block (lines 504-514) handles API failures, the previously flagged critical issues with null safety are still present:
- Line 502:
prevContext.pageaccess whenprevContextmay beundefined- Lines 516-518: Destructuring
dataandmetawithout defaults- Lines 526-528: Direct access to
meta.page.current_pageandmeta.page.last_pagewithout optional chainingApply this diff to add the missing null guards:
- async listSegmentOptions(prevContext) { - const page = prevContext.page || 1; + async listSegmentOptions(prevContext = {}) { + const page = prevContext?.page ?? 1; let response; try { ... } - const { - data, meta, - } = response; + const { data = [], meta = {} } = response || {}; return { options: data.map((segment) => ({ label: segment.attributes.name, value: segment.id, })), context: { - page: meta.page.current_page < meta.page.last_page + page: meta?.page?.current_page < meta?.page?.last_page ? page + 1 : null, }, };
🧹 Nitpick comments (1)
components/activecampaign/sources/new-contact-added-to-segment/new-contact-added-to-segment.mjs (1)
174-216: Potential property collision when enriching contact with custom fields.Line 191 assigns custom field values directly to the contact object using the field title as the property name:
contact[fieldTitle] = fieldValue.value;If a custom field is named "email", "id", "firstName", etc., it will overwrite the standard contact property. While custom field names are unlikely to collide with standard properties in practice, consider:
- Using a nested object (e.g.,
contact.customFields = { ... })- Or prefixing custom field properties (e.g.,
contact[custom_${fieldTitle}])Otherwise, the contact enrichment and event emission logic is well-structured and properly handles errors (line 195-197).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/activecampaign/activecampaign.app.mjs(4 hunks)components/activecampaign/sources/new-contact-added-to-segment/new-contact-added-to-segment.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-10T19:18:27.998Z
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#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/activecampaign/sources/new-contact-added-to-segment/new-contact-added-to-segment.mjs
📚 Learning: 2025-06-04T17:52:05.780Z
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#16954
File: components/salesloft/salesloft.app.mjs:14-23
Timestamp: 2025-06-04T17:52:05.780Z
Learning: In the Salesloft API integration (components/salesloft/salesloft.app.mjs), the _makeRequest method returns response.data which directly contains arrays for list endpoints like listPeople, listCadences, listUsers, and listAccounts. The propDefinitions correctly call .map() directly on these responses without needing to destructure a nested data property.
Applied to files:
components/activecampaign/activecampaign.app.mjs
🧬 Code graph analysis (2)
components/activecampaign/sources/new-contact-added-to-segment/new-contact-added-to-segment.mjs (1)
components/activecampaign/activecampaign.app.mjs (4)
limit(615-615)offset(616-616)response(504-504)response(516-518)
components/activecampaign/activecampaign.app.mjs (1)
components/activecampaign/sources/new-contact-added-to-segment/new-contact-added-to-segment.mjs (4)
response(69-74)response(105-112)response(136-140)segment(141-141)
⏰ 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: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (7)
components/activecampaign/activecampaign.app.mjs (2)
80-97: LGTM: Segment prop definitions follow established patterns.Both propDefinitions are well-structured and consistent with existing patterns in the codebase for campaigns, contacts, and lists.
331-338: LGTM: Contact field values method follows the established pattern.The implementation is consistent with similar methods like
getContactTags.components/activecampaign/sources/new-contact-added-to-segment/new-contact-added-to-segment.mjs (5)
1-28: LGTM: Component structure and props are well-defined.The component follows Pipedream guidelines with proper naming conventions, dedupe strategy, and polling interval configuration.
29-56: LGTM: State management methods properly track initialization and data.The explicit initialization flag (lines 30-38) correctly addresses the previously identified issue where an initially empty segment would be treated as uninitialized on subsequent runs. Based on learnings.
57-89: LGTM: Field mapping caching is efficient and correct.The cache-first approach (lines 58-61) avoids redundant API calls, and pagination properly handles large field lists.
98-124: Clarify the purpose of theforceQueryparameter.Line 110 passes
forceQuery: trueto thelistContactsAPI call, but this parameter is not documented in the code. Please verify:
- Is this parameter required for segment-based contact queries?
- What behavior does it control?
- Should this be documented inline?
Otherwise, the pagination logic and contact fetching implementation look correct.
126-172: LGTM: First-run detection correctly implemented.Lines 165-172 properly use the initialization flag to prevent emitting events for existing contacts on the first run, addressing the critical issue from previous reviews. Based on learnings.
michelle0927
moved this from Changes Required
to Ready for QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from Ready for QA
to In QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from In QA
to In Review
in Component (Source and Action) Backlog
vunguyenhung
moved this from In Review
to Ready for QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from Ready for QA
to In QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from In QA
to Ready for Release
in Component (Source and Action) Backlog
|
Hi everyone, all test cases are passed! Ready for release! Test report |
github-project-automation
bot
moved this from Ready for Release
to Done
in Component (Source and Action) Backlog
5 participants
WHY
New Contact Added to Segment (ActiveCampaign)
Description
Adds a new polling source that emits events when contacts are added to an ActiveCampaign segment.
How it works
/audiencesAPI to list and fetch segment details (the V1/segmentsAPI only returns legacy/hidden segments)Key features
{segmentId}-{contactId}-{timestamp}Use case example
Monitor when contacts match specific criteria like "Status = Active" or "First Name contains 'kacper'" by creating a segment with those filters and watching for new additions.
Changes to
activecampaign.app.mjslistAudiences()method to fetch segments from/api/3/audiencessegmentpropDefinition for single segment selectionlistSegmentOptions()to populate segment dropdown with user-created segmentsSummary by CodeRabbit
New Features
Chores