-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
Functions: Handle "charge.updated" events to process updated balance transactions from Stripe #1007
Conversation
…transactions from Stripe
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
📝 WalkthroughWalkthroughThe pull request introduces modifications to the Stripe webhook handler and event processing logic. The changes focus on expanding event handling capabilities, specifically for 'charge.updated' events, and refining how charge information is processed and stored. The updates improve the robustness of charge event management by adding more flexible event processing and ensuring comprehensive data handling. Changes
Sequence DiagramsequenceDiagram
participant Stripe
participant WebhookHandler
participant StripeEventHandler
participant Firestore
Stripe->>WebhookHandler: Send webhook event
WebhookHandler->>StripeEventHandler: Process event
alt Charge Event
StripeEventHandler->>Firestore: Retrieve User
StripeEventHandler->>Firestore: Store Contribution (Merged)
end
Finishing Touches
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 (
|
Visit the preview URL for this PR (updated for commit 148849d): https://si-admin-staging--pr1007-michael-stripe-charg-ak0zjc5n.web.app (expires Sun, 19 Jan 2025 11:15:12 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: b7b0969384059dce6ea8fad1ee1d1737e54e6676 |
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: 0
🧹 Nitpick comments (3)
shared/tests/utils.ts (1)
3-11
: Add error handling and logging for deletion failures.The function should catch and log any errors during deletion to help with debugging test failures.
export async function clearFirestoreData(firestoreAdmin: FirestoreAdmin) { + try { const collections = await firestoreAdmin.firestore.listCollections(); const deletePromises = collections.map(async (collection) => { const documents = await collection.listDocuments(); const deleteDocPromises = documents.map((doc) => firestoreAdmin.firestore.recursiveDelete(doc)); await Promise.all(deleteDocPromises); }); await Promise.all(deletePromises); + } catch (error) { + console.error('Failed to clear Firestore data:', error); + throw error; + } }shared/src/stripe/StripeEventHandler.test.ts (1)
35-35
: Consider adding error case tests.The tests only cover successful scenarios. Add tests for:
- Invalid charge data
- Firestore write failures
- Missing customer data
Also applies to: 56-56, 66-66, 78-78
shared/.env.sample (1)
1-4
: Add validation instructions for environment setup.Consider adding:
- Instructions for verifying emulator connectivity
- Required Firebase emulator versions
- Commands to start emulators with these ports
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
shared/.env.sample
(1 hunks)shared/src/stripe/StripeEventHandler.test.ts
(6 hunks)shared/src/stripe/StripeEventHandler.ts
(2 hunks)shared/tests/utils.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- shared/src/stripe/StripeEventHandler.ts
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Test shared code
- GitHub Check: Test functions
- GitHub Check: Test admin
- GitHub Check: Test website
- GitHub Check: Prettify
- GitHub Check: Security checks (typescript)
🔇 Additional comments (2)
shared/src/stripe/StripeEventHandler.test.ts (2)
18-20
: LGTM! Good test isolation practice.Adding clearFirestoreData in beforeEach ensures clean state for each test.
Line range hint
83-85
: Implement the TODO for checkout session test.The missing test for handleCheckoutSessionCompletedEvent should be implemented to ensure complete coverage.
Would you like me to help implement this test case?
Summary by CodeRabbit
New Features
Bug Fixes
Refactor