- Notifications
You must be signed in to change notification settings - Fork11.2k
fix: Grab booking organizer credentials when team admins request reschedule#24645
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
require params that are required
…entials- Add test for team admin requesting reschedule with proper permissions- Add test verifying organizer's credentials are used (not requester's)- Add test for team member without permissions (should fail)These tests cover the fix in PR#24645 which ensures that when a team adminrequests a reschedule, the booking organizer's credentials are used to deletecalendar events instead of the requester's credentials.Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
…entials- Add test for team admin requesting reschedule with proper permissions- Add test verifying organizer's credentials are used (not requester's)- Add test for team member without permissions (should fail)These tests cover the fix in PR#24645 which ensures that when a team adminrequests a reschedule, the booking organizer's credentials are used to deletecalendar events instead of the requester's credentials.Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
vercelbot commentedOct 23, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
The latest updates on your projects. Learn more aboutVercel for GitHub. |
github-actionsbot commentedOct 28, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
E2E results are ready! |
| isOpenDialog={isOpenRescheduleDialog} | ||
| setIsOpenDialog={setIsOpenRescheduleDialog} | ||
| bookingUId={booking.uid} | ||
| bookingUid={booking.uid} |
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.
Throughout the codebase we usebookingUid so cleaning this up to be more consistent.
| onClick={()=>{ | ||
| rescheduleApi({ | ||
| bookingId, | ||
| bookingUid, |
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.
We're actually passing thebookingUid instead of the id here so cleaning this up to be more clear.
| * It includes in-memory DelegationCredential credentials as well. | ||
| */ | ||
| exportasyncfunctiongetUsersCredentialsIncludeServiceAccountKey(user:User){ | ||
| exportasyncfunctiongetUsersCredentialsIncludeServiceAccountKey(user:{id:number;email:string}){ |
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.
Changed the type here to only require the params that are used in the function rather than the broaderUser type
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.
Moved these two prisma calls fromrequestReschedule.handler to theBookingRepository
| constbookingToReschedule=awaitbookingRepository.findBookingForRequestReschedule({ bookingUid}); | ||
| if(!bookingToReschedule.userId){ | ||
| if(!bookingToReschedule)return; |
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.
Moved this early return right after when the query is made.
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.
findBookingForRequestReschedule already throws when it can't find the booking, so this is probably dead code
| if(!isBookingOrganizer&&bookingBelongsToTeam&&bookingToReschedule.eventType?.teamId){ | ||
| constpermissionCheckService=newPermissionCheckService(); | ||
| consthasPermission=awaitpermissionCheckService.checkPermission({ | ||
| userId:user.id, | ||
| teamId:bookingToReschedule.eventType.teamId, | ||
| permission:"booking.update", | ||
| fallbackRoles:["ADMIN","OWNER"], | ||
| }); |
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.
Replaced this manual check with thePermissionCheckService. Shoutout@sean-brydon
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.
No issues found across 6 files
| } | ||
| constbookingBelongsToTeam=!!bookingToReschedule.eventType?.teamId; | ||
| constisBookingOrganizer=bookingToReschedule.userId===user.id; |
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.
In case of multiple hosts (collective or round robin event type) thenisBookingOrganizer would be false in case the second host clicks on request reschedule
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.
It should only be the single organizer because we use their credentials for the calendar event.
Uh oh!
There was an error while loading.Please reload this page.
Udit-takkar left a comment
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.
left some comments
Uh oh!
There was an error while loading.Please reload this page.
| if(!bookingToReschedule.userId){ | ||
| if(!bookingToReschedule)return; | ||
| if(!bookingToReschedule.userId||!bookingToReschedule.user){ |
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.
Isn't user the relation foruserId ? By that logic both will either be set together or null together. So, one of the checks should suffice
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.
We need to keep this in here or we'll throw a type error
Uh oh!
There was an error while loading.Please reload this page.
hariombalhara left a comment
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.
Nice edge case handling !!
Have added some comments. Feel free to implement what makes sense
joeauyeung commentedNov 20, 2025
@hariombalhara@Udit-takkar removed business logic references from the repository methods. Decided to keep the business logic in the handler for now. |
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.
No issues found across 6 files
Udit-takkar left a comment
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.
@joeauyeung type checks are failing
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.
No issues found across 6 files
2b2bf36 intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
What does this PR do?
bookingUIdtobookingUidinline with other places in the codebaseMandatory Tasks (DO NOT REMOVE)
How should this be tested?
Summary by cubic
Fixes reschedule requests for team bookings by using the organizer’s credentials to cancel calendar/video events and enforcing permissions. Also switches the API input to bookingUid.
Bug Fixes
Migration
Written for commit78d994e. Summary will update automatically on new commits.