-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Update new-payment.mjs #15266
Update new-payment.mjs #15266
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
@samss084 is attempting to deploy a commit to the Pipedreamers Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe pull request updates the Changes
Suggested Reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Thank you so much for submitting this! We've added it to our backlog to review, and our team has been notified. |
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
🔭 Outside diff range comments (1)
components/stripe/sources/new-payment/new-payment.mjs (1)
Line range hint
1-44
: Remove duplicate code blockThe file contains duplicate declarations of imports and the default export. This is a syntax error that needs to be fixed immediately.
Remove one of the duplicate blocks. Keep only one copy of the code:
import common from "../common/webhook-base.mjs"; export default { ...common, key: "stripe-new-payment", name: "New Payment", type: "source", version: "0.1.1", description: "Emit new event for each new payment", methods: { ...common.methods, getEvents() { return [ "payment_intent.created", ]; }, async onEvent(event) { const amount = 5500.00; this.$emit(event, { summary: `New payment of $${amount} received`, id: event.id, ts: event.created, }); }, }, }; -import common from "../common/webhook-base.mjs"; - -export default { - ...common, - key: "stripe-new-payment", - name: "New Payment", - type: "source", - version: "0.1.1", - description: "Emit new event for each new payment", - methods: { - ...common.methods, - getEvents() { - return [ - "payment_intent.created", - ]; - }, - async onEvent(event) { - const amount = 5500.00; - this.$emit(event, { - summary: `New payment of $${amount} received`, - id: event.id, - ts: event.created, - }); - }, - }, -};🧰 Tools
🪛 Biome (1.9.4)
[error] 21-44: Illegal duplicate default export declarations
multiple default exports are erroneous
the module's default export is first defined here
(parse)
[error] 19-19: Shouldn't redeclare 'common'. Consider to delete it or rename it.
'common' is defined here:
(lint/suspicious/noRedeclare)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/stripe/sources/new-payment/new-payment.mjs
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
components/stripe/sources/new-payment/new-payment.mjs
[error] 21-44: Illegal duplicate default export declarations
multiple default exports are erroneous
the module's default export is first defined here
(parse)
[error] 19-19: Shouldn't redeclare 'common'. Consider to delete it or rename it.
'common' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (2)
components/stripe/sources/new-payment/new-payment.mjs (2)
21-44
: Please provide PR descriptionThe PR lacks a description explaining the rationale behind these changes. Given the significance of the changes (adding event handling and payment processing), please update the PR description to include:
- The purpose of these changes
- Any related issues or tickets
- Testing performed to validate the changes
🧰 Tools
🪛 Biome (1.9.4)
[error] 21-44: Illegal duplicate default export declarations
multiple default exports are erroneous
the module's default export is first defined here
(parse)
36-36
:⚠️ Potential issueReplace hardcoded amount with actual payment amount
The amount is hardcoded to
5500.00
, which will incorrectly report the same amount for all payments instead of using the actual payment amount from the event.The amount should be extracted from the payment event. Based on Stripe's API, the amount should be accessed from the event object:
-const amount = 5500.00; +const amount = (event.data?.object?.amount ?? 0) / 100; // Stripe amounts are in centsLet's verify the Stripe payment intent event structure:
✅ Verification successful
Review comment is accurate and critical
The review comment correctly identifies a critical issue. The file
components/stripe/sources/common/webhook-base.mjs
confirms thatevent.data.object
is the correct pattern for accessing Stripe webhook event data, and hardcoding the amount defeats the purpose of a payment webhook integration.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other Stripe payment handlers to confirm the event structure rg -l 'payment_intent.*amount' --type jsLength of output: 42
Script:
#!/bin/bash # Search for Stripe payment handling with broader patterns echo "=== Searching for Stripe payment handling ===" rg -l "stripe.*payment" --type js echo -e "\n=== Searching for amount handling in payment context ===" rg "amount.*event" --type js -A 2 -B 2 echo -e "\n=== Looking for Stripe event object usage ===" rg "event\.data\.object" --type js -A 2 -B 2Length of output: 460
Script:
#!/bin/bash # Examine the specific file and its context echo "=== Checking the specific file content ===" cat components/stripe/sources/new-payment/new-payment.mjs echo -e "\n=== Looking for other .mjs files with Stripe handling ===" fd -e mjs | xargs rg "stripe" -l echo -e "\n=== Looking for event.data.object pattern in .mjs files ===" fd -e mjs | xargs rg "event\.data\.object" -A 2 -B 2Length of output: 6643
Hi everyone, all test cases are passed! Ready for release! Test report |
WHY
Summary by CodeRabbit
@pipedream/stripe
to 0.6.2.