-
Notifications
You must be signed in to change notification settings - Fork 5.5k
13286 components aevent #19030
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
13286 components aevent #19030
Conversation
… source - Added methods for API interaction including listing registrants and pagination. - Introduced a new source for emitting events when a new registrant is added. - Updated package version to 0.1.0 and added dependency on @pipedream/platform.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds an axios-based internal API client and a paginate generator to the aevent app, bumps package version and dependency, and introduces a new polling source that emits newly registered users using persisted lastDate plus a test registrant payload. Changes
Sequence Diagram(s)sequenceDiagram
rect rgb(245,250,255)
participant Timer
participant Source as new-registrant source
participant App as aevent.app
participant API as External API
participant DB as Local DB
end
Timer->>Source: scheduled trigger / run()
Source->>DB: _getLastDate()
DB-->>Source: lastDate
Source->>App: call paginate(fn: listRegistrants, params, maxResults)
loop pages
App->>API: _makeRequest(path: "registrants", headers) (axios)
API-->>App: page { items, ... }
App-->>Source: yield items
end
Source->>Source: filter items with registrationTime > lastDate
Source->>Source: collect and reverse (newest first)
Source->>DB: _setLastDate(newLastDate)
Source->>Source: emitEvent(item) => emit
Note over Source: deploy hook may preload an initial batch
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2024-10-10T19:18:27.998ZApplied to files:
⏰ 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)
🔇 Additional comments (7)
Comment |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
components/aevent/aevent.app.mjs(1 hunks)components/aevent/package.json(2 hunks)components/aevent/sources/new-registrant/new-registrant.mjs(1 hunks)components/aevent/sources/new-registrant/test-event.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 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/aevent/package.json
📚 Learning: 2025-10-20T01:01:02.970Z
Learnt from: js07
Repo: PipedreamHQ/pipedream PR: 18744
File: components/slack_v2/actions/send-large-message/send-large-message.mjs:49-64
Timestamp: 2025-10-20T01:01:02.970Z
Learning: In components/slack_v2/actions/send-large-message/send-large-message.mjs, the metadata_event_payload prop is typed as string, so the code only needs to handle string-to-JSON parsing and does not need to handle object inputs.
Applied to files:
components/aevent/sources/new-registrant/test-event.mjs
📚 Learning: 2025-06-04T17:52:05.780Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 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/aevent/aevent.app.mjs
📚 Learning: 2025-09-15T22:01:11.472Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 18362
File: components/leonardo_ai/actions/generate-image/generate-image.mjs:103-105
Timestamp: 2025-09-15T22:01:11.472Z
Learning: In Pipedream components, pipedream/platform's axios implementation automatically excludes undefined values from HTTP requests, so there's no need to manually check for truthiness before including properties in request payloads.
Applied to files:
components/aevent/aevent.app.mjs
📚 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/aevent/sources/new-registrant/new-registrant.mjs
⏰ 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: Lint Code Base
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
🔇 Additional comments (13)
components/aevent/package.json (1)
3-3: LGTM!The version bump to 0.1.0 is appropriate for this new feature addition, and the @pipedream/platform dependency is correctly added to support the axios-based API client introduced in the app module.
Also applies to: 15-16
components/aevent/aevent.app.mjs (6)
1-1: LGTM!The axios import from @pipedream/platform is correctly used for HTTP requests throughout the API client methods.
7-9: LGTM!The base API URL is correctly defined.
10-16: LGTM!The header construction correctly includes Bearer token authorization and appropriate content-type headers.
17-25: LGTM!The request wrapper correctly constructs the URL, includes authentication headers, and allows additional axios options to be passed through.
26-31: LGTM!The listRegistrants method correctly delegates to the internal request handler with the appropriate endpoint path.
32-56: Verify the AEvent API response structure matches the expected format.The paginate method destructures
{ success: data }from the API response (line 44), assuming the AEvent API returns data wrapped in asuccessproperty. Without access to AEvent API documentation in the codebase, this assumption should be verified against the actual API to ensure compatibility.Additionally, the pagination logic makes one unnecessary API call when retrieving all registrants without a
maxResultslimit. The loop continues based ondata.length(line 53), so after retrieving the final page of results, it will make one more request that returns an empty array to determine there are no additional records. Consider optimizing this by checking for an empty response before incrementing the page counter.components/aevent/sources/new-registrant/test-event.mjs (1)
1-86: LGTM!The sample event structure is comprehensive and includes all expected registrant fields, nested timeline data, and metadata. This provides a good reference for the event payload shape.
components/aevent/sources/new-registrant/new-registrant.mjs (5)
1-3: LGTM!The imports are appropriate for a polling source component.
5-21: LGTM!The component definition is well-structured with appropriate metadata, dedupe strategy, and standard polling source props.
23-28: LGTM!The state management methods correctly persist and retrieve the lastDate timestamp for deduplication.
61-69: LGTM!The deploy hook correctly emits a limited set of historical events (25), and the run method processes all new registrants. The sampleEmit reference is properly included for testing.
29-43: Verify API sort order and parameter support with AEvent.The original review comment raises valid concerns that cannot be resolved via public documentation. I've confirmed:
AEvent API documentation is not publicly available. The API requires registration and is not documented in public sources.
The code makes unverified assumptions:
- Line 34 passes
lastDateas a parameter, but there's no documentation confirming the API supports filtering by this parameter- Line 41 assumes registrants are returned in descending order by
registrationTime(breaks onitem.registrationTime <= lastDate)- Line 52 reverses the array, reinforcing the descending-order assumption
These assumptions are critical. If the API doesn't support
lastDatefiltering or doesn't return results in descending order, the pagination logic will fail silently or fetch unnecessary records.Contact AEvent support or test against the live API to confirm:
- Whether the registrants endpoint accepts a
lastDateor similar filtering parameter- Whether results are guaranteed to be in descending order by
registrationTime- Document these assumptions in code comments for future maintainers
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/aevent/sources/new-registrant/new-registrant.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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/aevent/sources/new-registrant/new-registrant.mjs
🪛 GitHub Actions: Pull Request Checks
components/aevent/sources/new-registrant/new-registrant.mjs
[error] 46-46: ESLint: Expected indentation of 8 spaces but found 5 (indent).
🪛 GitHub Check: Lint Code Base
components/aevent/sources/new-registrant/new-registrant.mjs
[failure] 48-48:
Expected indentation of 8 spaces but found 5
[failure] 47-47:
Expected indentation of 10 spaces but found 7
[failure] 46-46:
Expected indentation of 8 spaces but found 5
⏰ 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: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (7)
components/aevent/sources/new-registrant/new-registrant.mjs (7)
1-4: LGTM!The imports are appropriate for a Pipedream polling source with sample data support.
5-21: LGTM!The component metadata and props are correctly structured. The dedupe strategy "unique" paired with
uuidas the event ID ensures proper deduplication of registrants.
22-28: LGTM!State management using
_getLastDateand_setLastDatefollows the standard pattern for Pipedream polling sources. The default value of0ensures all registrants are fetched on the first run.
52-58: LGTM!Event emission logic is correct:
- Reversing the array ensures chronological order (oldest registrants emitted first)
- Using
uuidas the event ID with dedupe "unique" prevents duplicate emissions- The summary provides a user-friendly display with email fallback
61-68: LGTM!The
deployhook correctly limits initial emissions to 25 events to avoid overwhelming workflows on first deployment, while therunmethod processes all new registrants during regular polling. This follows Pipedream best practices.
69-69: LGTM!Providing
sampleEmitenables proper testing and preview functionality for the source.
29-43: The code assumes the API returns registrants in descending order (newest first), but this assumption is not documented or verified in the codebase.The logic reveals this dependency:
- Line 41 breaks when
item.registrationTime <= lastDate, assuming older items appear later- Line 47/49 saves
responseArray[0].registrationTimeas the new checkpoint (assuming it's the newest)- Line 52 reverses the array before emitting (converting descending to ascending order)
However, the
paginatemethod merely yields items as returned by the API without enforcing any ordering guarantee. The external aevent API's actual ordering behavior cannot be verified from the codebase. This creates a silent dependency on undocumented API behavior that could break if the API changes.Recommendation: Verify with the aevent API documentation or add explicit documentation/test case confirming descending order by
registrationTime.
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.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/aevent/sources/new-registrant/new-registrant.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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/aevent/sources/new-registrant/new-registrant.mjs
⏰ 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: Lint Code Base
- GitHub Check: pnpm publish
🔇 Additional comments (8)
components/aevent/sources/new-registrant/new-registrant.mjs (8)
1-3: LGTM!The imports are appropriate for a polling source component.
5-21: LGTM!The source metadata and props follow Pipedream conventions correctly. The
dedupe: "unique"strategy will prevent duplicate emissions based on theuuidfield.
23-28: LGTM!The state management methods correctly persist and retrieve the last seen registration timestamp.
45-47: LGTM with caveat.The state update correctly sets
lastDateto the newest item's timestamp (assuming descending order). This approach is appropriate for timestamp-based polling.
49-55: LGTM!Events are correctly emitted in chronological order (oldest to newest) with appropriate deduplication keys and metadata.
58-62: LGTM!The deploy hook appropriately limits historical events to 25 to avoid overwhelming users on first deployment.
63-66: LGTM!The run method and sampleEmit follow Pipedream conventions correctly.
40-43: Verify that the API returns registrants in descending order byregistrationTime.The code assumes items are returned newest-first (descending order): it stores
responseArray[0].registrationTimeas the new checkpoint (line 46), then reverses the array before emitting (line 48). If the API returns items in a different order, this logic will fail to collect new items correctly. No public API documentation confirms this sort behavior, so add a comment documenting this assumption or verify it against your API.Additionally, note the edge case on line 41: the
<=condition intentionally skips items with a timestamp equal tolastDate. If multiple items share the sameregistrationTime, some may be missed on subsequent polls.
michelle0927
left a comment
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.
LGTM!
For Integration QA:
|
|
Hi everyone, all test cases are passed! Ready for release! Test reports
|
Resolves #13286
Summary by CodeRabbit
New Features
Tests
Chores