Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds webhook-based Shortcut sources with a shared base for webhook lifecycle, a new "New Event Received" trigger with filtering and sample event, introduces async propDefinitions (workflowStateId, labelIds, storyIds), refactors create-story action to use propDefinitions and simplify props/payload, and bumps package versions/dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Pipedream
participant DB as Database
participant Shortcut as Shortcut API
rect rgba(200,220,240,0.5)
Note over Pipedream,DB: Source Activation
User->>Pipedream: Activate source
Pipedream->>Shortcut: POST /integrations/webhook (webhook_url)
Shortcut-->>Pipedream: { hookId }
Pipedream->>DB: Store hookId
end
rect rgba(240,240,200,0.5)
Note over Shortcut,Pipedream: Event Delivery & Processing
Shortcut->>Pipedream: POST webhook (event body)
activate Pipedream
Pipedream->>Pipedream: Validate body → respond 200
Pipedream->>Pipedream: processEvent(body) — filter by issueType/name
Pipedream->>Shortcut: GET searchStories or searchEpics (resolve issue)
Shortcut-->>Pipedream: Return issue
Pipedream->>Pipedream: Apply workflow/label filters
Pipedream->>Pipedream: Emit event (body + resolved issue)
deactivate Pipedream
end
rect rgba(240,200,220,0.5)
Note over Pipedream,DB: Source Deactivation
User->>Pipedream: Deactivate source
Pipedream->>DB: Retrieve hookId
Pipedream->>Shortcut: DELETE /integrations/webhook/{hookId}
Shortcut-->>Pipedream: Success
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ 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
📜 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 (7)
components/shortcut/actions/create-story/create-story.mjs(11 hunks)components/shortcut/actions/search-stories/search-stories.mjs(1 hunks)components/shortcut/package.json(2 hunks)components/shortcut/shortcut.app.mjs(1 hunks)components/shortcut/sources/common/base.mjs(1 hunks)components/shortcut/sources/new-event-received/new-event-received.mjs(1 hunks)components/shortcut/sources/new-event-received/test-event.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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/shortcut/sources/new-event-received/test-event.mjs
📚 Learning: 2024-10-08T15:33:38.240Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-10-08T15:33:38.240Z
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/shortcut/sources/common/base.mjscomponents/shortcut/sources/new-event-received/new-event-received.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/shortcut/sources/new-event-received/new-event-received.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: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (7)
components/shortcut/package.json (1)
3-18: Version bump and new platform dependency look consistentThe 1.1.0 version and the addition of
@pipedream/platformalign with the new shared base source that relies on platform utilities, so this metadata change looks correct.Please confirm
^3.1.1matches the version used by other newer components in this repo to avoid subtle runtime differences.components/shortcut/sources/new-event-received/test-event.mjs (1)
1-99: Sample event payload aligns with source expectationsThe exported
{ body, issue }shape and fields (e.g.,body.actions[0].entity_type,body.actions[0].id,body.changed_at,issue.workflow_state_id,issue.label_ids) line up with how the source processes and emits events, so this is a good representative test payload.components/shortcut/sources/common/base.mjs (1)
6-79: Webhook lifecycle + run pattern look solid for an abstract baseThe base source cleanly encapsulates webhook create/delete using the stored
hookId, andruncorrectly responds 200 before delegating toprocessEvent, which child sources are expected to implement. This aligns well with the abstract “common webhook methods” pattern used in other integrations.Based on learnings
components/shortcut/actions/search-stories/search-stories.mjs (1)
7-7: Version bump is consistent with surrounding Shortcut updatesUpdating the action version to
1.0.2to accompany the broader Shortcut integration changes looks appropriate; behavior is unchanged and metadata stays in sync with the package.components/shortcut/shortcut.app.mjs (1)
44-52: LGTM!The labelIds implementation is clean and handles empty data gracefully with destructuring defaults.
components/shortcut/actions/create-story/create-story.mjs (2)
20-24: Excellent refactor to eliminate duplication.Moving workflowStateId to a shared propDefinition improves maintainability and consistency across actions.
29-244: LGTM! Text improvements enhance consistency.The label and description updates improve consistency (e.g., "Id" → "ID", "Ids" → "IDs") and polish the user-facing text.
For Integration QA: |
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check test reports below for more information:
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/shortcut/actions/create-story/create-story.mjs (1)
245-337: Fix validation typo solinkedFileIdsis actually validatedIn the validation constraints you have:
const constraints = { // ... externalLinks: utils.validateArrayWhenPresent, fileIds: utils.validateArrayWhenPresent, followerIds: utils.validateArrayWhenPresent, inkedFileIds: utils.validateArrayWhenPresent, ownerIds: utils.validateArrayWhenPresent, };but the attributes you validate are:
{ // ... linkedFileIds: this.linkedFileIds, ownerIds: this.ownerIds, }Because of the
inkedFileIdstypo,linkedFileIdsis not checked at all. Suggest aligning the key:- followerIds: utils.validateArrayWhenPresent, - inkedFileIds: utils.validateArrayWhenPresent, - ownerIds: utils.validateArrayWhenPresent, + followerIds: utils.validateArrayWhenPresent, + linkedFileIds: utils.validateArrayWhenPresent, + ownerIds: utils.validateArrayWhenPresent,This restores array validation for
linkedFileIdswhile keeping the rest of the refactor intact.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
components/shortcut/actions/create-story/create-story.mjs(10 hunks)components/shortcut/shortcut.app.mjs(1 hunks)components/shortcut/sources/new-event-received/new-event-received.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-09-12T07:49:36.125Z
Learnt from: matyascimbulka
Repo: PipedreamHQ/pipedream PR: 18308
File: components/apify/actions/run-task-synchronously/run-task-synchronously.mjs:70-0
Timestamp: 2025-09-12T07:49:36.125Z
Learning: The Apify Task object always contains the `options` field according to the official API documentation, making nested destructuring like `options: { build }` safe to use without additional checks.
Applied to files:
components/shortcut/shortcut.app.mjs
📚 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/shortcut/sources/new-event-received/new-event-received.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/shortcut/sources/new-event-received/new-event-received.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/shortcut/sources/new-event-received/new-event-received.mjs
📚 Learning: 2024-10-08T15:33:38.240Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-10-08T15:33:38.240Z
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/shortcut/sources/new-event-received/new-event-received.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). (5)
- GitHub Check: pnpm publish
- GitHub Check: Ensure component commits modify component versions
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check test reports below for more information:
|
|
@vunguyenhung The screenshot in the report shows "nov21" entered in the |
|
Hi everyone, all test cases are passed! Ready for release! Test reports
|
|
/approve |
|
Hi @michelle0927, my bad on this. Thank you for clarifying! |
3 participants
Resolves #18593
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.