feat: Write same metadata to Stripe for booking payment and no show fee payment#23634
feat: Write same metadata to Stripe for booking payment and no show fee payment#23634hariombalhara merged 12 commits intomainfrom
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough
Possibly related PRs
✨ Finishing touches🧪 Generate unit tests
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 |
keithwillcode
added
core
| throw new Error("Stripe credentials not found"); | ||
| } | ||
|
|
||
| const stripeAppKeys = await prisma.app.findFirst({ |
There was a problem hiding this comment.
This variable is unused
| slug: "stripe", | ||
| }, | ||
| }); | ||
| const bookingRepository = new BookingRepository(prisma); |
There was a problem hiding this comment.
We now need to get the booking to generate the metadata to send to Stripe
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/app-store/stripepayment/lib/PaymentService.ts (1)
226-231: Good: fetch booking via repositoryUsing BookingRepository centralizes selection and avoids ad-hoc Prisma in the service. If this path is hot or tested often, consider injecting the repository (constructor param) for easier mocking.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/app-store/stripepayment/lib/PaymentService.ts(7 hunks)packages/lib/server/repository/booking.ts(1 hunks)packages/trpc/server/routers/viewer/payments.tsx(0 hunks)
💤 Files with no reviewable changes (1)
- packages/trpc/server/routers/viewer/payments.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/lib/server/repository/booking.tspackages/app-store/stripepayment/lib/PaymentService.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/lib/server/repository/booking.tspackages/app-store/stripepayment/lib/PaymentService.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/lib/server/repository/booking.tspackages/app-store/stripepayment/lib/PaymentService.ts
**/*Service.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Service files must include
Servicesuffix, use PascalCase matching exported class, and avoid generic names (e.g.,MembershipService.ts)
Files:
packages/app-store/stripepayment/lib/PaymentService.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: alishaz-polymath
PR: calcom/cal.com#23247
File: packages/features/webhooks/lib/factory/WebhookPayloadFactory.ts:274-282
Timestamp: 2025-08-21T12:28:42.018Z
Learning: In BookingPaymentInitiatedDTO and other webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking field is a restricted structure containing only specific fields (id, eventTypeId, userId) rather than the full database booking object, so there are no security or PII leakage concerns when passing the booking object to buildEventPayload.
📚 Learning: 2025-08-21T13:44:06.805Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.805Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.
Applied to files:
packages/lib/server/repository/booking.ts
📚 Learning: 2025-08-19T09:47:49.478Z
Learnt from: eunjae-lee
PR: calcom/cal.com#23166
File: packages/prisma/migrations/20250818151914_routing_form_response_denormalized_backfill2/migration.sql:65-66
Timestamp: 2025-08-19T09:47:49.478Z
Learning: The Booking table has a unique constraint and index on the uid column (defined as `uid String unique` in schema.prisma), so JOINs on Booking.uid have optimal performance and don't require additional indexing.
Applied to files:
packages/lib/server/repository/booking.ts
🧬 Code graph analysis (2)
packages/lib/server/repository/booking.ts (1)
packages/prisma/selects/booking.ts (1)
bookingMinimalSelect(3-13)
packages/app-store/stripepayment/lib/PaymentService.ts (1)
apps/api/v2/src/lib/logger.bridge.ts (1)
error(77-79)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (2)
packages/app-store/stripepayment/lib/PaymentService.ts (2)
90-100: Good: unified metadata construction via helperSwitching create() to use a shared generator keeps booking and no-show flows consistent.
369-371: LGTM: clearer attendee filteringExplicit arrow function improves readability without changing behavior.
| } | ||
|
|
||
| async chargeCard(payment: Payment, _bookingId?: Booking["id"]): Promise<Payment> { | ||
| async chargeCard(payment: Payment, bookingId: Booking["id"]): Promise<Payment> { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Signature change: ensure all call sites updated to required bookingId
chargeCard now requires bookingId (no optional param). Verify interface/type and usages are updated accordingly to avoid compile/runtime breaks.
Run:
🏁 Script executed:
#!/bin/bash
# 1) Check IAbstractPaymentService signature
rg -nP -C3 'IAbstractPaymentService' --type=ts --type=tsx
# 2) Find all call sites of chargeCard
rg -nP -C2 '\bchargeCard\s*\(' --type=ts --type=tsx
# 3) Sanity: ensure two args are passed (payment, bookingId)
rg -nP -C2 '\bchargeCard\s*\(\s*[^,]+,\s*[^)]+\)' --type=ts --type=tsxLength of output: 268
🏁 Script executed:
#!/bin/bash
# 1) Check IAbstractPaymentService signature in TS files
rg -nP -C3 'interface\s+IAbstractPaymentService' -g '*.ts'
# 2) Find all call sites of chargeCard in TS files
rg -nP -C2 '\bchargeCard\s*\(' -g '*.ts'
# 3) Sanity: ensure two args are passed (payment, bookingId)
rg -nP -C2 '\bchargeCard\s*\(\s*[^,]+,\s*[^)]+\)' -g '*.ts'Length of output: 5205
Require bookingId parameter across interface and all implementations
- In packages/types/PaymentService.d.ts, change
bookingId?: Booking["id"]tobookingId: Booking["id"] - Update stub in setupVitest.ts to
async chargeCard(payment, bookingId) - In packages/app-store/paypal/lib/PaymentService.ts and packages/app-store/mock-payment-app/lib/PaymentService.ts, add
(payment, bookingId)to chargeCard signature - Verify other store providers (hitpay, btcpayserver, alby) conform to the updated signature
🤖 Prompt for AI Agents
In packages/app-store/stripepayment/lib/PaymentService.ts around line 220, the
chargeCard method currently declares async chargeCard(payment: Payment,
bookingId is missing) — update the signature to async chargeCard(payment:
Payment, bookingId: Booking["id"]): Promise<Payment> to match the new required
bookingId in the interface; then update all call sites in this file to pass
bookingId. Also modify packages/types/PaymentService.d.ts to make bookingId
non-optional, change the stub in setupVitest.ts to async chargeCard(payment,
bookingId), and update chargeCard signatures in
packages/app-store/paypal/lib/PaymentService.ts and
packages/app-store/mock-payment-app/lib/PaymentService.ts; finally scan and
adjust other store providers (hitpay, btcpayserver, alby) and any tests or
usages to accept and forward the bookingId parameter.
| metadata: this.generateMetadata({ | ||
| bookingId, | ||
| userId: booking.user?.id, | ||
| username: booking.user?.username, | ||
| bookerName: booking.attendees[0].name, | ||
| bookerEmail: booking.attendees[0].email, | ||
| bookerPhoneNumber: booking.attendees[0].phoneNumber ?? null, | ||
| eventTitle: booking.eventType?.title || null, | ||
| bookingTitle: booking.title, | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard against empty/missing attendees to avoid runtime crash
booking.attendees[0] can be undefined (e.g., malformed data, edge imports). Add optional chaining and safe fallbacks.
Apply this diff:
- metadata: this.generateMetadata({
- bookingId,
- userId: booking.user?.id,
- username: booking.user?.username,
- bookerName: booking.attendees[0].name,
- bookerEmail: booking.attendees[0].email,
- bookerPhoneNumber: booking.attendees[0].phoneNumber ?? null,
- eventTitle: booking.eventType?.title || null,
- bookingTitle: booking.title,
- }),
+ metadata: this.generateMetadata({
+ bookingId,
+ userId: booking.user?.id ?? null,
+ username: booking.user?.username ?? null,
+ bookerName: booking.attendees?.[0]?.name ?? "",
+ bookerEmail: booking.attendees?.[0]?.email ?? "",
+ bookerPhoneNumber: booking.attendees?.[0]?.phoneNumber ?? null,
+ eventTitle: booking.eventType?.title ?? null,
+ bookingTitle: booking.title ?? "",
+ }),🤖 Prompt for AI Agents
In packages/app-store/stripepayment/lib/PaymentService.ts around lines 260 to
269, metadata accesses booking.attendees[0] directly which can be undefined and
cause a runtime crash; change those accesses to use optional chaining and safe
fallbacks (e.g., booking.attendees?.[0]?.name ?? null,
booking.attendees?.[0]?.email ?? null, booking.attendees?.[0]?.phoneNumber ??
null) so bookerName, bookerEmail and bookerPhoneNumber always have defined safe
values before calling this.generateMetadata.
| async findBookingById(bookingId: number) { | ||
| return await this.prismaClient.booking.findUnique({ | ||
| where: { | ||
| id: bookingId, | ||
| }, | ||
| select: { | ||
| ...bookingMinimalSelect, | ||
| eventType: { | ||
| select: { | ||
| title: true, | ||
| }, | ||
| }, | ||
| user: { | ||
| select: { | ||
| id: true, | ||
| username: true, | ||
| }, | ||
| }, | ||
| attendees: { | ||
| select: { | ||
| name: true, | ||
| email: true, | ||
| phoneNumber: true, | ||
| }, | ||
| }, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Select only fields needed for Stripe metadata; drop bookingMinimalSelect here
This repository method is used to build Stripe metadata (event title, booking title, user id/username, and primary attendee info). Spreading bookingMinimalSelect pulls extra fields (customInputs, description, metadata, start/endTime, etc.) that aren’t needed. Tighten the select to minimize data surface and match our Prisma guideline.
Apply this diff:
- async findBookingById(bookingId: number) {
- return await this.prismaClient.booking.findUnique({
- where: {
- id: bookingId,
- },
- select: {
- ...bookingMinimalSelect,
- eventType: {
- select: {
- title: true,
- },
- },
- user: {
- select: {
- id: true,
- username: true,
- },
- },
- attendees: {
- select: {
- name: true,
- email: true,
- phoneNumber: true,
- },
- },
- },
- });
- }
+ async findBookingById(bookingId: number) {
+ return await this.prismaClient.booking.findUnique({
+ where: { id: bookingId },
+ select: {
+ id: true,
+ title: true,
+ eventType: { select: { title: true } },
+ user: { select: { id: true, username: true } },
+ attendees: { select: { name: true, email: true, phoneNumber: true } },
+ },
+ });
+ }Optional: align with repo style by accepting a param object, e.g., findBookingById({ bookingId }: { bookingId: number }).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async findBookingById(bookingId: number) { | |
| return await this.prismaClient.booking.findUnique({ | |
| where: { | |
| id: bookingId, | |
| }, | |
| select: { | |
| ...bookingMinimalSelect, | |
| eventType: { | |
| select: { | |
| title: true, | |
| }, | |
| }, | |
| user: { | |
| select: { | |
| id: true, | |
| username: true, | |
| }, | |
| }, | |
| attendees: { | |
| select: { | |
| name: true, | |
| email: true, | |
| phoneNumber: true, | |
| }, | |
| }, | |
| }, | |
| }); | |
| } | |
| async findBookingById(bookingId: number) { | |
| return await this.prismaClient.booking.findUnique({ | |
| where: { id: bookingId }, | |
| select: { | |
| id: true, | |
| title: true, | |
| eventType: { select: { title: true } }, | |
| user: { select: { id: true, username: true } }, | |
| attendees: { select: { name: true, email: true, phoneNumber: true } }, | |
| }, | |
| }); | |
| } |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
| // Ascending order ensures that the first attendee in the list is the booker and others are guests | ||
| // See why it is important https://github.com/calcom/cal.com/pull/20935 | ||
| // TODO: Ideally we should return `booker` property directly from the booking |
E2E results are ready! |
3 participants
What does this PR do?
This PR ensures the same booking metadata is written to the Stripe payment for paid bookings and when charging a no show fee
this.generateMetadatain the StripePaymentServiceBooking payment metadata
No show fee payment metadata
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist