fix: Create new Daily room if original expired during past booking reschedule #22591
Conversation
|
@anikdhabal is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes update the Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/lib/EventManager.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (07/17/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add ready-for-e2e label" took an action on this PR • (07/17/25)1 label was added to this PR based on Keith Williams's automation. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/lib/EventManager.ts (2)
589-596: Well-implemented expiration logic with good practices.The Daily video room expiration check is correctly implemented with proper Date handling and clear variable naming. The 14-day expiration period appears reasonable for video room lifecycles.
Consider making the expiration period configurable through an environment variable or constant for better maintainability:
+const DAILY_VIDEO_ROOM_EXPIRY_DAYS = 14; + if (evt.location === "integrations:daily") { const originalBookingEndTime = new Date(booking.endTime); - const roomExpiryTime = new Date(originalBookingEndTime.getTime() + 14 * 24 * 60 * 60 * 1000); + const roomExpiryTime = new Date(originalBookingEndTime.getTime() + DAILY_VIDEO_ROOM_EXPIRY_DAYS * 24 * 60 * 60 * 1000); const now = new Date(); isDailyVideoRoomExpired = now > roomExpiryTime; }
589-596: Consider adding logging for better observability.The expiration logic implementation is solid. Consider adding a log statement when a Daily video room is detected as expired for better debugging and monitoring:
if (evt.location === "integrations:daily") { const originalBookingEndTime = new Date(booking.endTime); const roomExpiryTime = new Date(originalBookingEndTime.getTime() + 14 * 24 * 60 * 60 * 1000); const now = new Date(); isDailyVideoRoomExpired = now > roomExpiryTime; + + if (isDailyVideoRoomExpired) { + log.info("Daily video room expired, will create new room", { + bookingId: booking.id, + originalEndTime: originalBookingEndTime, + roomExpiryTime: roomExpiryTime + }); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/lib/EventManager.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: alishaz-polymath
PR: calcom/cal.com#22304
File: packages/prisma/schema.prisma:1068-1071
Timestamp: 2025-07-16T05:10:22.863Z
Learning: In PR #22304 for Cal.com private link expiration features, the `maxUsageCount` field was intentionally set to default to 1 (non-nullable) as a breaking change, making all existing private links single-use after migration. This was a deliberate design decision by alishaz-polymath.
⏰ 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). (3)
- GitHub Check: Check for E2E label
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Security Check
🔇 Additional comments (3)
packages/lib/EventManager.ts (3)
552-552: LGTM! Necessary field addition for expiration logic.The
endTimefield addition is required for the Daily video room expiration check and follows the existing query pattern correctly.
599-599: Correct integration of expiration logic into booking reference updates.The addition of
isDailyVideoRoomExpiredto theshouldUpdateBookingReferencescondition is logical and consistent with the existing pattern. Expired Daily rooms should indeed trigger booking reference updates similar to location changes.
623-623: Proper integration of expiration logic into location update flow.The inclusion of
isDailyVideoRoomExpiredin the location update condition correctly ensures that expired Daily video rooms trigger theupdateLocationmethod, which will create a new room. This completes the logical flow for handling expired rooms.
E2E results are ready! |
| @@ -609,7 +620,7 @@ export default class EventManager { | |||
| updatedBookingReferences.push(...createdEvent.referencesToCreate); | |||
| } else { | |||
| // If the reschedule doesn't require confirmation, we can "update" the events and meetings to new time. | |||
| if (isLocationChanged || isBookingRequestedReschedule) { | |||
| if (isLocationChanged || isBookingRequestedReschedule || isDailyVideoRoomExpired) { | |||
There was a problem hiding this comment.
@anikdhabal you have mentioned creating new room when old one expires but you can also just update the meeting expiry time
There was a problem hiding this comment.
@Udit-takkar But how can we update a room's expiry if the room is already expired? To update the expiry, we need to pass that room's id in the api — but it's already expired
There was a problem hiding this comment.
Aslo as per their docs:-
Rooms that have an exp in the past are not returned by the [list rooms endpoint](https://docs.daily.co/reference/rest-api/rooms/list-rooms); they are zombies of a sort. An existing meeting can continue in the room, but from the perspective of most of the API, the room does not exist!
There was a problem hiding this comment.
Oh okay so the meeting was rescheduled very late
What does this PR do?
A booking was rescheduled from a past booking. The original booking's end time was June 23rd, and it was rescheduled on July 10th — resulting in a gap of more than 14 days. Since we set the room expiry to 14 days+endtime, it’s throwing an error when trying to update using an expired room in the API