fix: og images avatars #13790
Merged
fix: og images avatars #13790
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link. |
keithwillcode
added
api
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 4 Ignored Deployments
|
zomars
commented
Feb 20, 2024
Comment on lines
+210
to
+213
| image: getUserAvatarUrl({ | ||
| ...user, | ||
| profile: user.profile, | ||
| }), |
There was a problem hiding this comment.
This was returning "image": "/peer/avatar.png" which is wrong.
Comment on lines
+37
to
+39
| export const CAL_URL = new URL(WEBAPP_URL).hostname.endsWith(".vercel.app") | ||
| ? WEBAPP_URL | ||
| : process.env.NEXT_PUBLIC_WEBSITE_URL || WEBAPP_URL; |
There was a problem hiding this comment.
Avoid hard-coding cal.com as much as possible. In our hosted instance NEXT_PUBLIC_WEBSITE_URL is always set as https://cal.com so there should not be any problems here. Also this won't disturb self-hosts.
| import type { UserProfile } from "@calcom/types/UserProfile"; | ||
|
|
||
| /** | ||
| * Gives an organization aware avatar url for a user | ||
| * It ensures that the wrong avatar isn't fetched by ensuring that organizationId is always passed | ||
| * It should always return a fully formed url | ||
| */ | ||
| export const getUserAvatarUrl = ( |
There was a problem hiding this comment.
Refactored so we always return fully formed URLs
| @@ -62,7 +62,7 @@ export const constructMeetingImage = ( | |||
| `?type=meeting`, | |||
| `&title=${encodeURIComponent(title)}`, | |||
| `&meetingProfileName=${encodeURIComponent(profile.name)}`, | |||
| profile.image && `&meetingImage=${encodeURIComponent(WEBAPP_URL + profile.image)}`, | |||
There was a problem hiding this comment.
image should return fully formed always.
| @@ -125,7 +125,7 @@ export const Meeting = ({ title, users = [], profile }: MeetingImageProps) => { | |||
| const avatars = attendees | |||
| .map((user) => { | |||
| if ("image" in user && user?.image) return user.image; | |||
| if ("username" in user && user?.username) return `${WEBAPP_URL}/${user.username}/avatar.png`; | |||
| if ("username" in user && user?.username) return `${CAL_URL}/${user.username}/avatar.png`; | |||
There was a problem hiding this comment.
This is a last resort fallback.
PeerRich
previously approved these changes
Feb 20, 2024
📦 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! 🙌 |
Graphite AutomationsA Graphite automation took an action on this PR • (02/20/24)1 reviewer was added based on Keith Williams's automation, 'Add foundation team as reviewer' |
Current Playwright Test Results Summary✅ 435 Passing - Run may still be in progress, this comment will be updated as current testing workflow or job completes... (Last updated on 02/20/2024 10:47:28pm UTC) Run DetailsRunning Workflow PR Update on Github Actions Commit: 38a08f9 Started: 02/20/2024 10:36:53pm UTC
|
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Booking With Multiple Email Question and Each Other Question Booking With Multiple Email Question and Radio group Question Booking With Multiple Email Question and Short text question Multiple Email and Short text not required
Retry 1 • Initial Attempt |
0% (0)0 / 242 runsfailed over last 7 days |
4.13% (10)10 / 242 runsflaked over last 7 days |
📄 apps/web/playwright/locale.e2e.ts • 13 Flakes
Top 1 Common Error Messages
|
|
13 Test Cases Affected |
Test Case Results
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
unauthorized user sees correct translations (de) should use correct translations and html attributes
Retry 1 • Initial Attempt |
-7.45% (-19)-19 / 255 runsfailed over last 7 days |
7.45% (19)19 / 255 runsflaked over last 7 days |
|
unauthorized user sees correct translations (ar) should use correct translations and html attributes
Retry 1 • Initial Attempt |
-7.45% (-19)-19 / 255 runsfailed over last 7 days |
7.45% (19)19 / 255 runsflaked over last 7 days |
|
unauthorized user sees correct translations (zh) should use correct translations and html attributes
Retry 1 • Initial Attempt |
-7.45% (-19)-19 / 255 runsfailed over last 7 days |
7.45% (19)19 / 255 runsflaked over last 7 days |
|
unauthorized user sees correct translations (zh-CN) should use correct translations and html attributes
Retry 1 • Initial Attempt |
-7.45% (-19)-19 / 255 runsfailed over last 7 days |
7.45% (19)19 / 255 runsflaked over last 7 days |
|
unauthorized user sees correct translations (zh-TW) should use correct translations and html attributes
Retry 1 • Initial Attempt |
-7.45% (-19)-19 / 255 runsfailed over last 7 days |
7.45% (19)19 / 255 runsflaked over last 7 days |
|
unauthorized user sees correct translations (pt) should use correct translations and html attributes
Retry 1 • Initial Attempt |
-7.45% (-19)-19 / 255 runsfailed over last 7 days |
7.45% (19)19 / 255 runsflaked over last 7 days |
|
unauthorized user sees correct translations (pt-br) should use correct translations and html attributes
Retry 1 • Initial Attempt |
-7.45% (-19)-19 / 255 runsfailed over last 7 days |
7.45% (19)19 / 255 runsflaked over last 7 days |
|
unauthorized user sees correct translations (es-419) should use correct translations and html attributes
Retry 1 • Initial Attempt |
-7.45% (-19)-19 / 255 runsfailed over last 7 days |
7.45% (19)19 / 255 runsflaked over last 7 days |
|
authorized user sees correct translations (de) should return correct translations and html attributes
Retry 1 • Initial Attempt |
-7.48% (-19)-19 / 254 runsfailed over last 7 days |
7.48% (19)19 / 254 runsflaked over last 7 days |
|
authorized user sees correct translations (pt-br) should return correct translations and html attributes
Retry 1 • Initial Attempt |
-7.48% (-19)-19 / 254 runsfailed over last 7 days |
7.48% (19)19 / 254 runsflaked over last 7 days |
|
authorized user sees correct translations (ar) should return correct translations and html attributes
Retry 1 • Initial Attempt |
-7.48% (-19)-19 / 254 runsfailed over last 7 days |
7.48% (19)19 / 254 runsflaked over last 7 days |
|
authorized user sees changed translations (de->ar) should return correct translations and html attributes
Retry 1 • Initial Attempt |
-2.36% (-6)-6 / 254 runsfailed over last 7 days |
5.51% (14)14 / 254 runsflaked over last 7 days |
|
authorized user sees changed translations (de->pt-BR) [locale1] should return correct translations and html attributes
Retry 1 • Initial Attempt |
-2.04% (-5)-5 / 245 runsfailed over last 7 days |
5.31% (13)13 / 245 runsflaked over last 7 days |
📄 apps/web/playwright/booking/radioGroupQuestion.e2e.ts • 1 Flake
Test Case Results
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Booking With Radio Question and Each Other Question Booking With Radio Question and Address Question Booking With Radio Question and select Question Radio and select not required
Retry 2 • Retry 1 • Initial Attempt |
0.38% (1)1 / 261 runfailed over last 7 days |
4.60% (12)12 / 261 runsflaked over last 7 days |
📄 apps/web/playwright/booking/addressQuestione2e/addressQuestion.e2e.ts • 2 Flakes
Top 1 Common Error Messages
|
|
2 Test Cases Affected |
Test Case Results
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Booking With Address Question and Each Other Question Booking With Address Question and Checkbox Question Addres and checkbox not required
Retry 1 • Initial Attempt |
1.54% (4)4 / 259 runsfailed over last 7 days |
5.02% (13)13 / 259 runsflaked over last 7 days |
|
Booking With Address Question and Each Other Question Booking With Address Question and Radio group Question Address required and Radio group required
Retry 1 • Initial Attempt |
0% (0)0 / 252 runsfailed over last 7 days |
5.16% (13)13 / 252 runsflaked over last 7 days |
📄 apps/web/playwright/integrations-stripe.e2e.ts • 1 Flake
Test Case Results
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Stripe integration Pending payment booking should not be confirmed by default
Retry 1 • Initial Attempt |
1.54% (4)4 / 260 runsfailed over last 7 days |
20.77% (54)54 / 260 runsflaked over last 7 days |
📄 apps/web/playwright/booking/phoneQuestion.e2e.ts • 2 Flakes
Top 1 Common Error Messages
|
|
2 Test Cases Affected |
Test Case Results
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Booking With Phone Question and Each Other Question Booking With Phone Question and Address Question Booking With Phone Question and checkbox Question Phone and checkbox required
Retry 1 • Initial Attempt |
1.49% (4)4 / 268 runsfailed over last 7 days |
6.72% (18)18 / 268 runsflaked over last 7 days |
|
Booking With Phone Question and Each Other Question Booking With Phone Question and Address Question Booking With Phone Question and checkbox Question Phone required and checkbox not required
Retry 1 • Initial Attempt |
0.38% (1)1 / 265 runfailed over last 7 days |
4.91% (13)13 / 265 runsflaked over last 7 days |
📄 apps/web/playwright/profile.e2e.ts • 2 Flakes
Top 1 Common Error Messages
|
|
2 Test Cases Affected |
Test Case Results
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Update Profile Cannot update a users email when existing user has same email (verification enabled)
Retry 1 • Initial Attempt |
1.47% (4)4 / 272 runsfailed over last 7 days |
50.74% (138)138 / 272 runsflaked over last 7 days |
|
Update Profile Can update a users email (verification enabled)
Retry 1 • Initial Attempt |
13.55% (37)37 / 273 runsfailed over last 7 days |
50.92% (139)139 / 273 runsflaked over last 7 days |
📄 apps/web/playwright/event-types.e2e.ts • 1 Flake
Test Case Results
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Event Types tests -- future user Different Locations Tests Can add Organzer Phone Number location and book with it
Retry 1 • Initial Attempt |
0% (0)0 / 261 runsfailed over last 7 days |
2.30% (6)6 / 261 runsflaked over last 7 days |
keithwillcode
previously approved these changes
Feb 20, 2024
|
Code looks good. Need to test |
emrysal
reviewed
Feb 20, 2024
emrysal
approved these changes
Feb 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes some edge cases for OG avatars not displaying properly
Type of change
How should this be tested?
Mandatory Tasks