-
Notifications
You must be signed in to change notification settings - Fork 5.5k
[Deprecation] Xola - Deprecated order.* sources and introduced purchase.* sources #19062
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?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughThis PR refactors Xola webhook sources by introducing three new purchase-based sources (purchase created, canceled, and updated) with event handling methods and test event payloads, while deprecating three corresponding order-based sources. The package version is bumped to 0.2.0. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
components/xola/package.json(1 hunks)components/xola/sources/new-order-created-instant/new-order-created-instant.mjs(1 hunks)components/xola/sources/new-purchase-created-instant/new-purchase-created-instant.mjs(1 hunks)components/xola/sources/new-purchase-created-instant/test-event.mjs(1 hunks)components/xola/sources/order-deleted-instant/order-deleted-instant.mjs(1 hunks)components/xola/sources/order-updated-instant/order-updated-instant.mjs(1 hunks)components/xola/sources/purchase-canceled-instant/purchase-canceled-instant.mjs(1 hunks)components/xola/sources/purchase-canceled-instant/test-event.mjs(1 hunks)components/xola/sources/purchase-updated-instant/purchase-updated-instant.mjs(1 hunks)components/xola/sources/purchase-updated-instant/test-event.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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/xola/sources/purchase-updated-instant/test-event.mjscomponents/xola/sources/purchase-updated-instant/purchase-updated-instant.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/xola/package.json
📚 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/xola/sources/purchase-updated-instant/purchase-updated-instant.mjscomponents/xola/sources/purchase-canceled-instant/purchase-canceled-instant.mjscomponents/xola/sources/new-purchase-created-instant/new-purchase-created-instant.mjs
🧬 Code graph analysis (3)
components/xola/sources/purchase-updated-instant/purchase-updated-instant.mjs (5)
components/xola/sources/new-order-created-instant/new-order-created-instant.mjs (1)
body(18-18)components/xola/sources/new-purchase-created-instant/new-purchase-created-instant.mjs (1)
body(18-18)components/xola/sources/order-deleted-instant/order-deleted-instant.mjs (2)
body(18-18)ts(19-19)components/xola/sources/order-updated-instant/order-updated-instant.mjs (2)
body(18-18)ts(19-19)components/xola/sources/purchase-canceled-instant/purchase-canceled-instant.mjs (1)
body(18-18)
components/xola/sources/purchase-canceled-instant/purchase-canceled-instant.mjs (5)
components/xola/sources/new-order-created-instant/new-order-created-instant.mjs (1)
body(18-18)components/xola/sources/new-purchase-created-instant/new-purchase-created-instant.mjs (1)
body(18-18)components/xola/sources/order-deleted-instant/order-deleted-instant.mjs (1)
body(18-18)components/xola/sources/order-updated-instant/order-updated-instant.mjs (1)
body(18-18)components/xola/sources/purchase-updated-instant/purchase-updated-instant.mjs (1)
body(18-18)
components/xola/sources/new-purchase-created-instant/new-purchase-created-instant.mjs (5)
components/xola/sources/new-order-created-instant/new-order-created-instant.mjs (1)
body(18-18)components/xola/sources/order-deleted-instant/order-deleted-instant.mjs (1)
body(18-18)components/xola/sources/order-updated-instant/order-updated-instant.mjs (1)
body(18-18)components/xola/sources/purchase-canceled-instant/purchase-canceled-instant.mjs (1)
body(18-18)components/xola/sources/purchase-updated-instant/purchase-updated-instant.mjs (1)
body(18-18)
⏰ 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/xola/package.json (1)
3-3: Version bump is consistent with the Xola source changesMoving to
0.2.0for the package aligns with adding new purchase.* sources and deprecating order.* ones. No issues from this change.components/xola/sources/order-updated-instant/order-updated-instant.mjs (1)
8-9: Deprecation messaging and version bump look goodDescription clearly directs users to the new “Purchase Updated (Instant)” source, and the minor source version bump to
0.0.2is appropriate for this metadata change.components/xola/sources/new-order-created-instant/new-order-created-instant.mjs (1)
8-9: Clear deprecation notice and appropriate version incrementThe description unambiguously deprecates this source and points to “New Purchase Created (Instant)”, and bumping to
0.0.2matches the metadata-only change pattern.components/xola/sources/order-deleted-instant/order-deleted-instant.mjs (1)
8-9: Deprecation text and source version are consistentThe updated description correctly routes users to “Purchase Canceled (Instant)”, and the version bump to
0.0.2matches the deprecation-only change.components/xola/sources/purchase-updated-instant/test-event.mjs (1)
1-542: LGTM! Comprehensive test fixture.The test event provides a complete purchase object with nested line items, travelers, and audit information that accurately represents a purchase update scenario.
components/xola/sources/new-purchase-created-instant/test-event.mjs (1)
1-533: LGTM! Comprehensive test fixture.The test event provides a complete purchase object suitable for testing purchase creation scenarios. The empty audit object (line 532) is appropriate since this is a creation event with no prior state.
components/xola/sources/new-purchase-created-instant/new-purchase-created-instant.mjs (1)
1-27: LGTM! Clean and consistent implementation.The webhook source follows the established pattern and correctly implements purchase creation event handling.
components/xola/sources/purchase-updated-instant/purchase-updated-instant.mjs (1)
21-21: The review comment identifies a valid inconsistency, but resolution depends on business logic clarification.The verification confirms the concern: Purchase Updated uses a compound ID (
${data.id}-${ts}) while Purchase Canceled uses a simple ID (data.id). However, there's a broader pattern in the codebase to consider:
- Create events: simple ID (Purchase Created, Experience Created, Order Created)
- Update/Delete events: compound ID (Purchase Updated, Experience Updated/Deleted, Order Updated/Deleted)
- Cancel event: simple ID (inconsistent with update/delete pattern)
Additionally, Purchase Updated uses
Date.parse(data.updatedAt)(stable across webhook retries), while Experience/Order Deletions useDate.now()(creates new IDs on retries). This affects deduplication behavior.The question hinges on: Can a purchase be canceled multiple times? If cancellations can be retried/repeated, a compound ID is warranted for uniqueness. If a purchase can only be canceled once, the simple ID is appropriate but should align with the chosen pattern across all entity types.
components/xola/sources/purchase-canceled-instant/purchase-canceled-instant.mjs
Show resolved
Hide resolved
components/xola/sources/purchase-updated-instant/purchase-updated-instant.mjs
Show resolved
Hide resolved
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: |
|
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:
|
WHY
Fixes of this PR #18963
Summary by CodeRabbit
New Features
Chores