feat: Add Google Profile Photo when connecting Google Calendar #13627
Conversation
|
@zeeshanbhati is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
|
|
|
Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link. |
zeeshanbhati
changed the title
zeeshanbhati
changed the title
📦 Next.js Bundle Analysis for @calcom/webThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
keithwillcode
added
Low priority
PeerRich
added
the
⚡ Quick Wins
PeerRich
changed the title
|
@joeauyeung is it worth adding another scope just to get the profile image? |
|
The scope change could add a lot of friction to making this PR possible |
|
Can we fetch the profile picture without using this scope ? This is also a non critical scope Also, what kind of friction can it introduce? Is it something related to compliance or something? |
|
@zeeshanbhati the fear was if we added a new scope would it invalidate access tokens from other users. Testing this by authenticating GCal w/o the profile scope. Then on another account authenticating w/ the scope. When refreshing the token and creating a calendar event with the token w/o the profile scope, it worked as expected. @baileypumfleet @keithwillcode the profile scope is non-sensitive. |
There was a problem hiding this comment.
Reviewed new scope change (see #13627 (comment))
Test cases:
- With no avatar set ✅
- Shouldn't overwrite preset avatar ✅
- Throwing an error in
setProfilePicturedoesn't block creation of calendar credential ✅
| @@ -91,6 +93,10 @@ async function getHandler(req: NextApiRequest, res: NextApiResponse) { | |||
| throw new HttpError({ message: "Internal Error", statusCode: 500 }); | |||
| } | |||
|
|
|||
| // Update the user's profile photo with google profile photo if it's null | |||
| // Since we don't want to block the user from using the app if this fails, we don't await this | |||
There was a problem hiding this comment.
This doesn't really work in serverless environments. If you don't await and the main endpoint responds first the call will be dropped.
There was a problem hiding this comment.
TIL. Can we just wrap it in a try/catch block and if it fails just continue on with the rest of the code?
There was a problem hiding this comment.
@zomars there's already a try/catch in the function it's self. I tested throw an error in the function and it still creates the credential. I'll add the await and one more change in a follow up PR.
|
There's a race condition happening in this PR. |
What does this PR do?
If a user links their Google Calendar and hasn't set an avatar, we'll retrieve their profile photo from their connected Google account and use it as their profile picture.
Fixes : #13492
Requirement/Documentation
https://www.googleapis.com/auth/userinfo.profilewill be added to the calendar scope.Type of change
How should this be tested?
Checklist