🌐 AI搜索 & 代理 主页
Skip to content

Conversation

@luancazarine
Copy link
Collaborator

@luancazarine luancazarine commented Dec 10, 2025

Resolves #13382

Summary by CodeRabbit

  • New Features
    • Create Expense action with receipt upload (base64 handling)
    • List Employments action with pagination
    • Show Time Off Balance action
    • New instant webhook events: Contract Amendment Done, Custom Field Value Updated, Employment Onboarding Started
    • Webhook source base with activation/deactivation and sample test events
    • Dynamic selection fields for Employment and Expense Category in the Remote integration
  • Chores
    • Package version bumped and platform dependency added

✏️ Tip: You can customize this high-level summary in your review settings.

- 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
@luancazarine luancazarine linked an issue Dec 10, 2025 that may be closed by this pull request
@vercel
Copy link

vercel bot commented Dec 10, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
pipedream-docs Ignored Ignored Dec 11, 2025 3:55pm
pipedream-docs-redirect-do-not-edit Ignored Ignored Dec 11, 2025 3:55pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 10, 2025

Walkthrough

Adds 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

Cohort / File(s) Summary
Core app module
components/remote/remote.app.mjs
Adds dynamic propDefinitions (employmentId, expenseCategorySlug), internal HTTP helpers (_baseUrl, _getHeaders, _makeRequest), public API wrappers (createExpense, listEmployments, listExpenseCategories, showTimeoffBalance, createHook, deleteHook), and an async paginate() generator; removes authKeys().
Action modules
components/remote/actions/create-expense/create-expense.mjs, components/remote/actions/list-employments/list-employments.mjs, components/remote/actions/show-timeoff-balance/show-timeoff-balance.mjs
Adds three actions: create-expense (reads receipt stream, converts to base64 via streamToBase64, calls createExpense with data URL), list-employments (uses paginate to collect employments with filters and maxResults), and show-timeoff-balance (calls showTimeoffBalance for an employment and exports a summary).
Webhook source base infrastructure
components/remote/sources/common/base.mjs
New reusable webhook source base with props (remote, db, http), methods to persist webhook ID (_setWebhookId, _getWebhookId), lifecycle hooks (activate creates hook, deactivate deletes hook), and run() that emits events with generated metadata (expects subclass to implement generateMeta and getEvent).
Webhook event sources
components/remote/sources/new-contract-amendment-done-instant/..., components/remote/sources/new-custom-field-value-updated-instant/..., components/remote/sources/new-employment-onboarding-started-instant/...
Adds three instant webhook sources extending the base; each defines getEvent() (event type) and generateMeta({ body, ts }) (id, summary, ts) and includes sampleEmit and a test-event fixture.
Test event fixtures
components/remote/sources/new-contract-amendment-done-instant/test-event.mjs, components/remote/sources/new-custom-field-value-updated-instant/test-event.mjs, components/remote/sources/new-employment-onboarding-started-instant/test-event.mjs
Adds sample event payloads for the new webhook sources.
Package configuration
components/remote/package.json
Bumps package version to 0.1.0 and adds dependency @pipedream/platform@^3.1.1.
Minor formatting
components/upsales/upsales.app.mjs, components/xero_payroll/xero_payroll.app.mjs
Adds trailing newlines; no functional 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 }
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Focus areas:
    • components/remote/remote.app.mjs — HTTP request helpers, header/token handling, pagination behavior, and API wrapper parameter mapping.
    • components/remote/sources/common/base.mjs — activate/deactivate webhook lifecycle, DB persistence of webhook ID, and event emission semantics.
    • components/remote/actions/create-expense/create-expense.mjs — stream-to-base64 conversion, content-type and filename handling for receipt data URL, and error propagation from stream handling.

Suggested labels

ai-assisted

Suggested reviewers

  • michelle0927
  • GTFalcao

Pre-merge checks and finishing touches

❌ Failed checks (3 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description only contains 'Resolves #13382' with no additional context, failing to follow the required template structure that includes a 'WHY' section. Complete the PR description using the template by adding a 'WHY' section explaining the motivation, context, and benefits of implementing the Remote component integration.
Linked Issues check ⚠️ Warning The PR implements most requirements but is missing the polling source 'new-employee' and does not fully satisfy all requirements from issue #13382. Implement the missing 'new-employee' polling source and verify all required actions are complete: 'create-approved-expense', 'get-time-off-information' (show-timeoff-balance exists but may need verification for completeness), and 'list-employees' (list-employments exists but verify it matches requirements).
Out of Scope Changes check ⚠️ Warning The PR includes minor out-of-scope changes: trailing newline additions to upsales.app.mjs and xero_payroll.app.mjs, and version bump in package.json not mentioned in linked issues. Remove the trailing newline additions to upsales and xero_payroll files, and either document the package version bump rationale or revert it to focus on Remote component implementation only.
Title check ❓ Inconclusive The PR title '13382 components remote' is vague and generic, using only an issue reference and generic terms without conveying meaningful information about the specific changeset. Replace with a descriptive title that summarizes the main changes, such as 'Add Remote component actions and webhook sources' or 'Implement Remote API integration with expense, timeoff, and employment features'.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 13382-components-remote

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

…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.
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between b144fc9 and ded2fe9.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is 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.mjs
  • components/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.mjs and create-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 generateMeta method 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 expenseCategorySlug based on employmentId is correctly implemented.


90-108: LGTM!

The streamToBase64 method 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_id as 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 $ = this default 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 getEvent implementation follow the established pattern correctly.

- 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.
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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_updated doesn'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

📥 Commits

Reviewing files that changed from the base of the PR and between dabd201 and a11fc13.

📒 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.mjs
  • components/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: true annotation, proper use of the shared employmentId propDefinition, and appropriate summary export.

components/remote/remote.app.mjs (1)

6-44: LGTM on propDefinitions.

The dynamic option resolvers for employmentId and expenseCategorySlug are 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 sources

Importing ../common/base.mjs and a local test-event.mjs follows the usual Pipedream source pattern; nothing to change here.


4-11: Source metadata (key/name/description/type/dedupe) is coherent

Key, 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 straightforward

Returning ["contract_amendment.done"] from getEvent() clearly scopes this source to the correct webhook event; no issues spotted here.


30-31: Sample event wiring is correct

Exposing sampleEmit from ./test-event.mjs matches the pattern used by other sources and helps with UI discoverability; no change needed.

- 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.
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 text

Two issues in generateMeta:

  1. Line 24 (Critical): Including ts in the id defeats the dedupe: "unique" setting. If Remote retries the webhook (network issues, timeouts, etc.), each retry will generate a separate event instead of being deduplicated. The id should uniquely identify the contract amendment request itself, not each webhook delivery.

  2. Line 25 (Minor): The summary still references employment_request_id as if it were the employment ID, which remains confusing per the previous review comment. The payload contains both employment_request_id (the amendment request) and employment_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

📥 Commits

Reviewing files that changed from the base of the PR and between a11fc13 and a98307b.

📒 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.mjs
  • components/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 generateMeta method 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 sampleEmit is 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-instant
  • new-contract-amendment-done-instant

Please confirm whether:

  1. The PR objectives are incomplete, or
  2. This source was added in addition to the originally planned ones, or
  3. The requirements have changed

This helps ensure the PR delivers what was planned in the linked issue.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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_id as if it were the employment ID. Since the payload includes both employment_id and employment_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

📥 Commits

Reviewing files that changed from the base of the PR and between a98307b and 9e22ada.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is 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 generateMeta method 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 run method 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 readOnlyHint is now correctly set to true, 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.

Copy link
Collaborator

@michelle0927 michelle0927 left a 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!

@vunguyenhung
Copy link
Collaborator

For Integration QA:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Components] remote

4 participants