fix(daily-webhook): use BookingRepository instead of direct prisma call#29541
fix(daily-webhook): use BookingRepository instead of direct prisma call#29541JustSidus wants to merge 2 commits into
Conversation
|
Welcome to Cal.diy, @JustSidus! Thanks for opening this pull request. A few things to keep in mind:
A maintainer will review your PR soon. Thanks for contributing! |
📝 WalkthroughWalkthroughThis PR contains three targeted fixes and refactoring changes. A new 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/lib/daily-webhook/getBooking.ts`:
- Around line 10-11: The repository method used by getBooking
(BookingRepository.findByIdForDailyWebhook) currently calls findUniqueOrThrow
which raises a PrismaClientKnownRequestError for missing rows and breaks the
null-check in getBooking; change that repository implementation in
packages/features/bookings/repositories/BookingRepository.ts to use
prisma.findUnique (not findUniqueOrThrow) so it returns null when no booking is
found, preserving the existing null-check in getBooking.ts and allowing
getBooking to throw the intended HttpError 404 with the descriptive message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 54291b07-beeb-4113-9b9c-1e59cb71af4f
📒 Files selected for processing (3)
apps/api/v2/src/modules/verified-resources/services/verified-resources.service.tsapps/web/lib/daily-webhook/getBooking.tspackages/features/bookings/repositories/BookingRepository.ts
| const bookingRepository = new BookingRepository(prisma); | ||
| const booking = await bookingRepository.findByIdForDailyWebhook(bookingId); |
There was a problem hiding this comment.
Change findUniqueOrThrow to findUnique to preserve error handling behavior.
The new repository method uses findUniqueOrThrow, which throws a PrismaClientKnownRequestError when the booking is not found. This bypasses the null check below (lines 13-25) that throws a specific HttpError with status 404 and a descriptive message.
Impact:
- Before: Missing booking →
HttpError404 with message "Booking of id X does not exist or does not contain daily video as location" - After: Missing booking →
PrismaClientKnownRequestError→ caught by downstream handler as generic 500 "something went wrong"
This degrades the error response quality for API consumers.
🔧 Proposed fix
In packages/features/bookings/repositories/BookingRepository.ts, change the repository method to use findUnique:
async findByIdForDailyWebhook(bookingId: number) {
- return await this.prismaClient.booking.findUniqueOrThrow({
+ return await this.prismaClient.booking.findUnique({
where: {
id: bookingId,
},This preserves the existing error handling contract where getBooking returns null when not found, then throws the appropriate HttpError.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/lib/daily-webhook/getBooking.ts` around lines 10 - 11, The
repository method used by getBooking (BookingRepository.findByIdForDailyWebhook)
currently calls findUniqueOrThrow which raises a PrismaClientKnownRequestError
for missing rows and breaks the null-check in getBooking; change that repository
implementation in packages/features/bookings/repositories/BookingRepository.ts
to use prisma.findUnique (not findUniqueOrThrow) so it returns null when no
booking is found, preserving the existing null-check in getBooking.ts and
allowing getBooking to throw the intended HttpError 404 with the descriptive
message.
|
|
||
| async getUserVerifiedPhoneNumber(userId: number, phoneNumber: string) { | ||
| return this.usersVerifiedResourcesRepository.getUserVerifiedEmail(userId, phoneNumber); | ||
| return this.usersVerifiedResourcesRepository.getUserVerifiedPhoneNumber(userId, phoneNumber); |
There was a problem hiding this comment.
@JustSidus can you undo this file changes? it's out of scope.
There was a problem hiding this comment.
Done - removed the out-of-scope file change. The commit now only touches getBooking.ts and BookingRepository.ts.
| }); | ||
| } | ||
|
|
||
| async findByIdForDailyWebhook(bookingId: number) { |
There was a problem hiding this comment.
this name is too specific. We might want to reuse somewhere too. Either rename it something that can be reused in future, or use some existing functions!
There was a problem hiding this comment.
Renamed to findByIdWithUserAndEventType - more generic and reusable. Also changed findUniqueOrThrow to findUnique to preserve the null-check error handling.
269db08 to
c16e130
Compare
c16e130 to
dd41cdd
Compare
The getBooking function in daily-webhook/getBooking.ts had a // TODO: use BookingRepository comment and was calling prisma.booking.findUniqueOrThrow directly. This replaces it with a new indByIdForDailyWebhook method on BookingRepository, keeping the same select shape so the return type is unchanged.
Before:
ypescript const booking = await prisma.booking.findUniqueOrThrow({ where: { id: bookingId }, select: { ...bookingMinimalSelect, uid: true, location: true, isRecorded: true, eventTypeId: true, eventType: { ... }, user: { ... }, }, });After:
ypescript const bookingRepository = new BookingRepository(prisma); const booking = await bookingRepository.findByIdForDailyWebhook(bookingId);No behavior change - the select is identical, just routed through the repository layer as the TODO requested.
Fixes #28835