-
Notifications
You must be signed in to change notification settings - Fork 5.5k
13382 components remote #19418
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?
13382 components remote #19418
Conversation
- Bump version of @pipedream/remote to 0.1.0 - Introduce new actions: Create Expense, List Employments, Show Time Off Balance - Add prop definitions for employmentId and expenseCategorySlug in remote.app.mjs - Implement methods for creating expenses and listing employments - Add new event sources for contract amendments and custom field updates - Enhance webhook handling in common base source
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds a Remote integration: three actions (create expense, list employments, show time-off balance), a reusable webhook source base plus three instant webhook sources and fixtures, expanded app methods/propDefinitions for Remote API calls, and a package bump to 0.1.0 with a new dependency. Changes
Sequence Diagram(s)sequenceDiagram
participant Platform
participant RemoteApp as Remote App (component)
participant RemoteAPI as Remote API
participant DB as Persistent DB
Platform->>RemoteApp: activate() hook
RemoteApp->>RemoteApp: getEvent() → events
RemoteApp->>RemoteAPI: POST /hooks (endpoint, events) via _makeRequest
RemoteAPI-->>RemoteApp: { id, ... }
RemoteApp->>DB: _setWebhookId(id)
Note over RemoteAPI,Platform: Remote sends webhook when event occurs
RemoteAPI->>Platform: POST /webhook (body)
Platform->>RemoteApp: run({ body })
RemoteApp->>RemoteApp: generateMeta(body, ts)
RemoteApp-->>Platform: emit({ body, meta })
Platform->>RemoteApp: deactivate() hook
RemoteApp->>DB: _getWebhookId()
DB-->>RemoteApp: id
RemoteApp->>RemoteAPI: DELETE /hooks/{id}
RemoteAPI-->>RemoteApp: { deleted }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Suggested labels ai-assisted Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (3 warnings, 1 inconclusive)
✅ Passed checks (1 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 |
…pdates - Change event types to reflect new actions: 'employment.onboarding.started' and 'custom_field.value_updated'. - Adjust employment IDs and custom field IDs in the respective test event files.
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: 8
📜 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 (12)
components/remote/actions/create-expense/create-expense.mjs(1 hunks)components/remote/actions/list-employments/list-employments.mjs(1 hunks)components/remote/actions/show-timeoff-balance/show-timeoff-balance.mjs(1 hunks)components/remote/package.json(2 hunks)components/remote/remote.app.mjs(1 hunks)components/remote/sources/common/base.mjs(1 hunks)components/remote/sources/new-contract-amendment-done-instant/new-contract-amendment-done-instant.mjs(1 hunks)components/remote/sources/new-contract-amendment-done-instant/test-event.mjs(1 hunks)components/remote/sources/new-custom-field-value-updated-instant/new-custom-field-value-updated-instant.mjs(1 hunks)components/remote/sources/new-custom-field-value-updated-instant/test-event.mjs(1 hunks)components/remote/sources/new-employment-onboarding-started-instant/new-employment-onboarding-started-instant.mjs(1 hunks)components/remote/sources/new-employment-onboarding-started-instant/test-event.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📚 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/remote/sources/new-contract-amendment-done-instant/test-event.mjs
📚 Learning: 2024-07-24T02:06:47.016Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-07-24T02:06:47.016Z
Learning: The `common-webhook-methods.mjs` object is designed to be extended, similar to an abstract class, and intentionally does not implement certain methods like `generateWebhookMeta` and `getEventType` to enforce implementation in subclasses.
Applied to files:
components/remote/sources/common/base.mjs
📚 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/remote/package.json
📚 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/remote/sources/new-employment-onboarding-started-instant/new-employment-onboarding-started-instant.mjscomponents/remote/sources/new-custom-field-value-updated-instant/new-custom-field-value-updated-instant.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/remote/remote.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/remote/remote.app.mjs
📚 Learning: 2025-01-23T03:55:15.166Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 15376
File: components/monday/sources/name-updated/name-updated.mjs:6-6
Timestamp: 2025-01-23T03:55:15.166Z
Learning: Source names in Monday.com components don't need to start with "New" if they emit events for updated items (e.g., "Name Updated", "Column Value Updated") rather than new items. This follows the component guidelines exception where the "New" prefix is only required when emits are limited to new items.
Applied to files:
components/remote/sources/new-custom-field-value-updated-instant/new-custom-field-value-updated-instant.mjs
🧬 Code graph analysis (3)
components/remote/actions/list-employments/list-employments.mjs (2)
components/remote/actions/create-expense/create-expense.mjs (1)
response(116-133)components/remote/actions/show-timeoff-balance/show-timeoff-balance.mjs (1)
response(24-27)
components/remote/actions/show-timeoff-balance/show-timeoff-balance.mjs (2)
components/remote/actions/create-expense/create-expense.mjs (1)
response(116-133)components/remote/actions/list-employments/list-employments.mjs (1)
response(52-61)
components/remote/actions/create-expense/create-expense.mjs (2)
components/remote/actions/list-employments/list-employments.mjs (1)
response(52-61)components/remote/actions/show-timeoff-balance/show-timeoff-balance.mjs (1)
response(24-27)
⏰ 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: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (18)
components/remote/package.json (2)
3-3: Version bump looks appropriate.The minor version increment (0.0.2 → 0.1.0) aligns with semantic versioning for feature additions.
15-17: Dependency @pipedream/platform ^3.1.1 is valid and properly used.The version 3.1.1 is available on npm and is actively imported in
remote.app.mjsandcreate-expense.mjs, confirming legitimate usage across the component modules.components/remote/actions/show-timeoff-balance/show-timeoff-balance.mjs (1)
23-31: LGTM!The implementation correctly calls the API method and provides a clear summary message.
components/remote/sources/new-custom-field-value-updated-instant/new-custom-field-value-updated-instant.mjs (1)
12-30: LGTM!The implementation correctly defines the event type and generates appropriate metadata using the custom field ID.
components/remote/sources/common/base.mjs (3)
1-18: LGTM!The base structure correctly defines props and helper methods for webhook management. The abstract
generateMetamethod properly enforces implementation in subclasses.
20-33: LGTM!The webhook lifecycle hooks are correctly implemented, managing creation and cleanup of webhooks appropriately.
35-41: LGTM!The run method correctly handles incoming webhook events and emits them with proper metadata.
components/remote/sources/new-contract-amendment-done-instant/test-event.mjs (1)
1-5: LGTM!The test event structure is correct for the contract amendment done event, with appropriate fields and event type.
components/remote/actions/list-employments/list-employments.mjs (1)
51-70: LGTM!The implementation correctly uses pagination to fetch employments, accumulates results, and provides a clear summary message.
components/remote/actions/create-expense/create-expense.mjs (4)
32-89: LGTM!The props are well-defined with appropriate types and descriptions. The dependent prop pattern for
expenseCategorySlugbased onemploymentIdis correctly implemented.
90-108: LGTM!The
streamToBase64method correctly converts a readable stream to a base64 string using promises and proper stream event handling.
110-137: LGTM!The run method correctly:
- Reads and converts the receipt file to base64
- Constructs a proper data URL with content type
- Maps all props to the appropriate API fields
- Provides a clear summary message
22-31: The currency field should remain required. The Remote API documentation explicitly requires currency when creating an expense (using a 3-letter ISO code like USD or EUR). The current implementation is correct, and the PR objective stating "Currency is optional" does not align with the API requirements.Likely an incorrect or invalid review comment.
components/remote/sources/new-employment-onboarding-started-instant/new-employment-onboarding-started-instant.mjs (1)
12-30: LGTM!The methods implementation follows the established pattern for instant sources. Using
employment_idas the dedupe ID is appropriate since each employment onboarding starts once.components/remote/remote.app.mjs (3)
6-44: LGTM!The prop definitions correctly handle pagination (converting Pipedream's 0-indexed pages to 1-indexed API pages) and properly format the options with meaningful labels.
46-65: LGTM!The internal request helpers follow Pipedream's standard patterns. The
$ = thisdefault parameter correctly provides axios context.
66-105: LGTM!The API wrapper methods are clean, consistent, and follow the established pattern for Pipedream app modules.
components/remote/sources/new-contract-amendment-done-instant/new-contract-amendment-done-instant.mjs (1)
4-18: LGTM!The source metadata and
getEventimplementation follow the established pattern correctly.
...te/sources/new-custom-field-value-updated-instant/new-custom-field-value-updated-instant.mjs
Outdated
Show resolved
Hide resolved
components/remote/sources/new-custom-field-value-updated-instant/test-event.mjs
Show resolved
Hide resolved
...rces/new-employment-onboarding-started-instant/new-employment-onboarding-started-instant.mjs
Outdated
Show resolved
Hide resolved
components/remote/sources/new-employment-onboarding-started-instant/test-event.mjs
Show resolved
Hide resolved
- Added a dataKey parameter to the paginate function for flexible data extraction. - Updated references in list-employments and other components to utilize the new dataKey. - Corrected employment ID references in new contract amendment and onboarding sources. - Improved descriptions in custom field value updated source for clarity.
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
♻️ Duplicate comments (1)
components/remote/sources/new-custom-field-value-updated-instant/new-custom-field-value-updated-instant.mjs (1)
8-8: Documentation URL appears invalid.The URL
https://developer.remote.com/reference/custom_fieldvalue_updateddoesn't follow Remote's standard documentation structure. Based on Remote's API docs, custom field webhooks are documented at/docs/custom-fields.- description: "Emit new event when a custom field is updated. [See the documentation](https://developer.remote.com/reference/custom_fieldvalue_updated)", + description: "Emit new event when a custom field is updated. [See the documentation](https://developer.remote.com/docs/custom-fields)",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
components/remote/actions/list-employments/list-employments.mjs(1 hunks)components/remote/actions/show-timeoff-balance/show-timeoff-balance.mjs(1 hunks)components/remote/remote.app.mjs(1 hunks)components/remote/sources/new-contract-amendment-done-instant/new-contract-amendment-done-instant.mjs(1 hunks)components/remote/sources/new-custom-field-value-updated-instant/new-custom-field-value-updated-instant.mjs(1 hunks)components/remote/sources/new-employment-onboarding-started-instant/new-employment-onboarding-started-instant.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 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/remote/sources/new-custom-field-value-updated-instant/new-custom-field-value-updated-instant.mjscomponents/remote/sources/new-contract-amendment-done-instant/new-contract-amendment-done-instant.mjs
📚 Learning: 2025-01-23T03:55:15.166Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 15376
File: components/monday/sources/name-updated/name-updated.mjs:6-6
Timestamp: 2025-01-23T03:55:15.166Z
Learning: Source names in Monday.com components don't need to start with "New" if they emit events for updated items (e.g., "Name Updated", "Column Value Updated") rather than new items. This follows the component guidelines exception where the "New" prefix is only required when emits are limited to new items.
Applied to files:
components/remote/sources/new-custom-field-value-updated-instant/new-custom-field-value-updated-instant.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/remote/remote.app.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/remote/remote.app.mjs
🧬 Code graph analysis (2)
components/remote/actions/list-employments/list-employments.mjs (2)
components/remote/actions/show-timeoff-balance/show-timeoff-balance.mjs (1)
response(24-27)components/remote/actions/create-expense/create-expense.mjs (1)
response(116-133)
components/remote/actions/show-timeoff-balance/show-timeoff-balance.mjs (2)
components/remote/actions/list-employments/list-employments.mjs (1)
response(52-62)components/remote/actions/create-expense/create-expense.mjs (1)
response(116-133)
⏰ 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/remote/actions/list-employments/list-employments.mjs (1)
3-72: LGTM!The action is well-structured with appropriate annotations (readOnlyHint correctly set to true for a read-only operation), clear prop definitions with sensible filter options, and proper use of the paginate helper to collect results. The async iteration pattern correctly handles the generator output.
components/remote/actions/show-timeoff-balance/show-timeoff-balance.mjs (1)
3-32: LGTM!Clean implementation with correct
readOnlyHint: trueannotation, proper use of the sharedemploymentIdpropDefinition, and appropriate summary export.components/remote/remote.app.mjs (1)
6-44: LGTM on propDefinitions.The dynamic option resolvers for
employmentIdandexpenseCategorySlugare well-implemented with proper label formatting and value mapping.components/remote/sources/new-employment-onboarding-started-instant/new-employment-onboarding-started-instant.mjs (1)
8-8: > Likely an incorrect or invalid review comment.components/remote/sources/new-contract-amendment-done-instant/new-contract-amendment-done-instant.mjs (4)
1-3: Imports and base wiring look consistent with other Remote sourcesImporting
../common/base.mjsand a localtest-event.mjsfollows the usual Pipedream source pattern; nothing to change here.
4-11: Source metadata (key/name/description/type/dedupe) is coherentKey, name, description, version, type, and
dedupe: "unique"align with the intended “contract_amendment.done” instant source and look good as-is.
12-18: Event filter definition is straightforwardReturning
["contract_amendment.done"]fromgetEvent()clearly scopes this source to the correct webhook event; no issues spotted here.
30-31: Sample event wiring is correctExposing
sampleEmitfrom./test-event.mjsmatches the pattern used by other sources and helps with UI discoverability; no change needed.
...s/remote/sources/new-contract-amendment-done-instant/new-contract-amendment-done-instant.mjs
Show resolved
Hide resolved
...te/sources/new-custom-field-value-updated-instant/new-custom-field-value-updated-instant.mjs
Show resolved
Hide resolved
- Modified ID generation in new contract amendment, custom field value updated, and employment onboarding sources to append timestamps for uniqueness. - Enhanced clarity in summary messages for better context.
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
♻️ Duplicate comments (2)
components/remote/sources/new-contract-amendment-done-instant/new-contract-amendment-done-instant.mjs (1)
19-28: Remove timestamp from ID to preserve deduplication; clarify summary textTwo issues in
generateMeta:
Line 24 (Critical): Including
tsin theiddefeats thededupe: "unique"setting. If Remote retries the webhook (network issues, timeouts, etc.), each retry will generate a separate event instead of being deduplicated. Theidshould uniquely identify the contract amendment request itself, not each webhook delivery.Line 25 (Minor): The summary still references
employment_request_idas if it were the employment ID, which remains confusing per the previous review comment. The payload contains bothemployment_request_id(the amendment request) andemployment_id(the actual employment), so the summary should distinguish them.Apply this diff:
generateMeta({ body, ts, }) { return { - id: `${body.employment_request_id}-${ts}`, - summary: `Contract amendment for employment with ID ${body.employment_request_id} has been done`, + id: body.employment_request_id, + summary: `Contract amendment request ${body.employment_request_id} for employment ${body.employment_id} has been done`, ts, }; },This ensures duplicate webhook deliveries are properly deduplicated and the summary clearly identifies both the request and the employment.
components/remote/sources/new-custom-field-value-updated-instant/new-custom-field-value-updated-instant.mjs (1)
8-8: Verify the documentation URL is correct.The documentation URL should point to Remote's webhook documentation. Based on a previous review that included web searches, the custom field value updated webhook is documented at
https://developer.remote.com/docs/custom-fields, not in the/reference/section.Please verify the correct documentation URL:
site:developer.remote.com custom_field.value_updated webhook documentation
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
components/remote/sources/new-contract-amendment-done-instant/new-contract-amendment-done-instant.mjs(1 hunks)components/remote/sources/new-custom-field-value-updated-instant/new-custom-field-value-updated-instant.mjs(1 hunks)components/remote/sources/new-employment-onboarding-started-instant/new-employment-onboarding-started-instant.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 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/remote/sources/new-custom-field-value-updated-instant/new-custom-field-value-updated-instant.mjscomponents/remote/sources/new-employment-onboarding-started-instant/new-employment-onboarding-started-instant.mjs
📚 Learning: 2025-01-23T03:55:15.166Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 15376
File: components/monday/sources/name-updated/name-updated.mjs:6-6
Timestamp: 2025-01-23T03:55:15.166Z
Learning: Source names in Monday.com components don't need to start with "New" if they emit events for updated items (e.g., "Name Updated", "Column Value Updated") rather than new items. This follows the component guidelines exception where the "New" prefix is only required when emits are limited to new items.
Applied to files:
components/remote/sources/new-custom-field-value-updated-instant/new-custom-field-value-updated-instant.mjs
🧬 Code graph analysis (3)
components/remote/sources/new-custom-field-value-updated-instant/new-custom-field-value-updated-instant.mjs (1)
components/remote/sources/common/base.mjs (1)
ts(36-36)
components/remote/sources/new-contract-amendment-done-instant/new-contract-amendment-done-instant.mjs (1)
components/remote/sources/common/base.mjs (1)
ts(36-36)
components/remote/sources/new-employment-onboarding-started-instant/new-employment-onboarding-started-instant.mjs (1)
components/remote/sources/common/base.mjs (1)
ts(36-36)
⏰ 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: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (7)
components/remote/sources/new-custom-field-value-updated-instant/new-custom-field-value-updated-instant.mjs (2)
14-18: LGTM!The event name
"custom_field.value_updated"follows Remote's webhook naming convention, and returning it as an array is the correct pattern for the base implementation.
19-28: LGTM! Deduplication fix is correctly implemented.The
generateMetamethod now includes the timestamp in the ID (${body.custom_field_id}-${ts}), which ensures that each update to the same custom field will emit a unique event. This correctly addresses the previous deduplication concern.components/remote/sources/new-employment-onboarding-started-instant/new-employment-onboarding-started-instant.mjs (5)
1-2: LGTM!The imports are correct and follow standard component patterns.
4-11: LGTM!The metadata configuration is correct. The grammatical error from the previous review has been fixed.
14-18: LGTM!The
getEvent()method correctly returns the webhook event type for Remote's onboarding started event.
30-30: LGTM!The
sampleEmitis correctly assigned from the test event fixture.
1-31: Verify alignment with PR objectives.This source implements
new-employment-onboarding-started-instant, but the linked issue #13382 lists different webhook sources:
new-custom-field-value-updated-instantnew-contract-amendment-done-instantPlease confirm whether:
- The PR objectives are incomplete, or
- This source was added in addition to the originally planned ones, or
- The requirements have changed
This helps ensure the PR delivers what was planned in the linked issue.
...rces/new-employment-onboarding-started-instant/new-employment-onboarding-started-instant.mjs
Show resolved
Hide resolved
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
♻️ Duplicate comments (1)
components/remote/sources/new-contract-amendment-done-instant/new-contract-amendment-done-instant.mjs (1)
19-28: Clarify summary text to distinguish employment vs amendment request ID.The summary text is still confusing as it refers to
employment_request_idas if it were the employment ID. Since the payload includes bothemployment_idandemployment_request_id, consider updating the summary to make this distinction clear.Apply this diff:
generateMeta({ body, ts, }) { return { id: `${body.employment_request_id}-${ts}`, - summary: `Contract amendment for employment with ID ${body.employment_request_id} is done`, + summary: `Contract amendment request ${body.employment_request_id} for employment ${body.employment_id} is done`, ts, }; },
📜 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 (6)
components/remote/actions/create-expense/create-expense.mjs(1 hunks)components/remote/actions/show-timeoff-balance/show-timeoff-balance.mjs(1 hunks)components/remote/sources/common/base.mjs(1 hunks)components/remote/sources/new-contract-amendment-done-instant/new-contract-amendment-done-instant.mjs(1 hunks)components/upsales/upsales.app.mjs(1 hunks)components/xero_payroll/xero_payroll.app.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-07-24T02:06:47.016Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-07-24T02:06:47.016Z
Learning: The `common-webhook-methods.mjs` object is designed to be extended, similar to an abstract class, and intentionally does not implement certain methods like `generateWebhookMeta` and `getEventType` to enforce implementation in subclasses.
Applied to files:
components/remote/sources/common/base.mjs
🧬 Code graph analysis (2)
components/remote/actions/show-timeoff-balance/show-timeoff-balance.mjs (2)
components/remote/actions/create-expense/create-expense.mjs (1)
response(116-133)components/remote/actions/list-employments/list-employments.mjs (1)
response(52-62)
components/remote/sources/new-contract-amendment-done-instant/new-contract-amendment-done-instant.mjs (1)
components/remote/sources/common/base.mjs (1)
ts(37-37)
⏰ 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: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (7)
components/upsales/upsales.app.mjs (1)
11-11: LGTM! Formatting improvement.The trailing newline addition is a pure formatting change with no functional impact.
components/xero_payroll/xero_payroll.app.mjs (1)
11-11: LGTM! Formatting improvement.The trailing newline addition is a pure formatting change with no functional impact.
components/remote/actions/create-expense/create-expense.mjs (1)
110-137: LGTM! Proper use of async/await and error handling.The run method correctly handles file streaming and API calls. The optional chaining at line 135 safely handles the response structure.
components/remote/sources/common/base.mjs (2)
17-19: LGTM! Abstract method pattern correctly implemented.The
generateMetamethod intentionally throws an error to enforce implementation in subclasses, following the established pattern for base/abstract classes in this codebase.Based on learnings, this is the expected design pattern.
36-42: LGTM! Clean event emission logic.The
runmethod correctly generates a timestamp and emits events with the generated metadata. The implementation is simple and effective.components/remote/actions/show-timeoff-balance/show-timeoff-balance.mjs (2)
8-12: LGTM! Annotations correctly set.The
readOnlyHintis now correctly set totrue, reflecting that this action performs a read-only GET operation. The previous review concern has been addressed.
23-31: LGTM! Clean implementation.The action correctly retrieves time-off balance data and provides a clear summary. The implementation is straightforward and appropriate for this read-only operation.
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! Ready for QA!
For Integration QA:
|
Resolves #13382
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.