Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: usage of CAL_URL #13326

Merged
merged 7 commits into from
Feb 13, 2024
Merged

fix: usage of CAL_URL #13326

merged 7 commits into from
Feb 13, 2024

Conversation

Udit-takkar
Copy link
Contributor

What does this PR do?

Fixes reverts this PR #12973

https://threads.com/thread/34561538856/34561538859?s=dUJjVzSUcZCeQtprQ7CdUc

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Copy link

vercel bot commented Jan 19, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ai ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 7, 2024 5:08pm
api ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 7, 2024 5:08pm
dev ❌ Failed (Inspect) Feb 7, 2024 5:08pm
5 Ignored Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Feb 7, 2024 5:08pm
cal-demo ⬜️ Ignored (Inspect) Visit Preview Feb 7, 2024 5:08pm
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Feb 7, 2024 5:08pm
qa ⬜️ Ignored (Inspect) Visit Preview Feb 7, 2024 5:08pm
ui ⬜️ Ignored (Inspect) Visit Preview Feb 7, 2024 5:08pm

Copy link
Contributor

github-actions bot commented Jan 19, 2024

Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link.

Copy link
Contributor

github-actions bot commented Jan 19, 2024

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

Copy link

deploysentinel bot commented Jan 19, 2024

Current Playwright Test Results Summary

✅ 442 Passing - ⚠️ 16 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 02/07/2024 05:21:20pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: 593f23a

Started: 02/07/2024 05:11:16pm UTC

⚠️ Flakes

📄   apps/web/playwright/booking/multipleEmailQuestion.e2e.ts • 2 Flakes

Top 1 Common Error Messages

null

2 Test Cases Affected

Test Case Results

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 Address Question Multiple Email and Address not required
Retry 1Initial Attempt
2.14% (7) 7 / 327 runs
failed over last 7 days
3.98% (13) 13 / 327 runs
flaked over last 7 days
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 required and Short text required
Retry 1Initial Attempt
0.32% (1) 1 / 315 run
failed over last 7 days
4.76% (15) 15 / 315 runs
flaked over last 7 days

📄   apps/web/playwright/booking/longTextQuestion.e2e.ts • 3 Flakes

Top 1 Common Error Messages

null

3 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Booking With Long Text Question and Each Other Question Booking With Long Text Question and checkbox Question Long Text required and checkbox not required
Retry 1Initial Attempt
0% (0) 0 / 311 runs
failed over last 7 days
3.54% (11) 11 / 311 runs
flaked over last 7 days
Booking With Long Text Question and Each Other Question Booking With Long Text Question and multiselect Question Long Text and multiselect text required
Retry 1Initial Attempt
0.65% (2) 2 / 310 runs
failed over last 7 days
5.81% (18) 18 / 310 runs
flaked over last 7 days
Booking With Long Text Question and Each Other Question Long Text required and Number not required
Retry 1Initial Attempt
0.32% (1) 1 / 310 run
failed over last 7 days
5.81% (18) 18 / 310 runs
flaked over last 7 days

📄   apps/web/playwright/booking/responsiveBooking.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Booking page with no questions Booking page with 640x480 resolution
Retry 1Initial Attempt
0% (0) 0 / 300 runs
failed over last 7 days
4.67% (14) 14 / 300 runs
flaked over last 7 days

📄   apps/web/playwright/booking/selectQuestion.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Booking With Phone Question and Each Other Question Booking With Select Question and checkbox group Question Select required and checkbox group required
Retry 1Initial Attempt
0% (0) 0 / 299 runs
failed over last 7 days
3.68% (11) 11 / 299 runs
flaked over last 7 days

📄   apps/web/playwright/booking/checkboxGroupQuestion.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Booking With Checkbox Group Question and Each Other Question Booking With Checkbox Group Question and Address Question Booking With Checkbox Group Question and Number Question Checkbox Group and Number not required
Retry 2Retry 1Initial Attempt
0.95% (3) 3 / 317 runs
failed over last 7 days
5.36% (17) 17 / 317 runs
flaked over last 7 days

📄   apps/web/playwright/login.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
user can login & logout succesfully -- future login flow user & logout using dashboard
Retry 1Initial Attempt
5.11% (16) 16 / 313 runs
failed over last 7 days
37.38% (117) 117 / 313 runs
flaked over last 7 days

📄   apps/web/playwright/booking/addressQuestione2e/addressQuestion.e2e.ts • 1 Flake

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 multiselect Question Address and multiselect text not required
Retry 1Initial Attempt
0% (0) 0 / 329 runs
failed over last 7 days
4.86% (16) 16 / 329 runs
flaked over last 7 days

📄   apps/web/playwright/profile.e2e.ts • 1 Flake

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 1Initial Attempt
0% (0) 0 / 16 runs
failed over last 7 days
56.25% (9) 9 / 16 runs
flaked over last 7 days

📄   apps/web/playwright/organization/organization-invitation.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Organization Email not matching orgAutoAcceptEmail Team invitation
Retry 1Initial Attempt
14.33% (47) 47 / 328 runs
failed over last 7 days
12.80% (42) 42 / 328 runs
flaked 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 Cal video location and book with it
Retry 1Initial Attempt
2.29% (8) 8 / 350 runs
failed over last 7 days
3.71% (13) 13 / 350 runs
flaked over last 7 days

📄   packages/embeds/embed-core/playwright/tests/preview.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Preview Preview - embed-core should load
Retry 1Initial Attempt
0% (0) 0 / 333 runs
failed over last 7 days
30.03% (100) 100 / 333 runs
flaked over last 7 days

📄   apps/web/playwright/booking/phoneQuestion.e2e.ts • 1 Flake

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 Phone and Address required
Retry 1Initial Attempt
0.90% (3) 3 / 332 runs
failed over last 7 days
4.22% (14) 14 / 332 runs
flaked over last 7 days

📄   packages/embeds/embed-core/playwright/tests/inline.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Inline Iframe Inline Iframe - Configured with Dark Theme
Retry 1Initial Attempt
0.60% (2) 2 / 331 runs
failed over last 7 days
41.09% (136) 136 / 331 runs
flaked over last 7 days

View Detailed Build Results


PeerRich
PeerRich previously approved these changes Jan 19, 2024
Copy link
Member

@alishaz-polymath alishaz-polymath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Holding off the revert until further discussions conclude. Please wait 🙏

@Udit-takkar Udit-takkar added this to the v3.8 milestone Jan 19, 2024
@keithwillcode keithwillcode requested a review from a team January 20, 2024 13:17
@keithwillcode keithwillcode added the High priority Created by Linear-GitHub Sync label Jan 20, 2024
@keithwillcode keithwillcode marked this pull request as draft January 27, 2024 12:06
@keithwillcode
Copy link
Contributor

Converted back to draft while until decision is made

Copy link
Contributor

@keithwillcode keithwillcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge conflicts, failing type checks

@keithwillcode keithwillcode marked this pull request as draft January 29, 2024 08:23
@Udit-takkar Udit-takkar marked this pull request as ready for review January 29, 2024 09:14
@Udit-takkar Udit-takkar changed the title revert: 'fix: preview url for booking page' fix: usage of CAL_URL Jan 29, 2024
keithwillcode
keithwillcode previously approved these changes Jan 29, 2024
@keithwillcode keithwillcode dismissed alishaz-polymath’s stale review January 29, 2024 09:25

Was decided to revert this

@Udit-takkar
Copy link
Contributor Author

@keithwillcode we should wait for @zomars or @hariombalhara approval before merging

@hariombalhara
Copy link
Member

hariombalhara commented Jan 29, 2024

I still don't think we should revert this.

Also, if we need to revert this, it should just be the changes you made in the PR #12973. Right now it seem to be doing a complete replacement of CAL_URL.

Also, CAL_URL handles the case of our previews, where website isn't setup and then we use WEBAPP_URL only. We need to consider that in any new approach that we use.

Are there any more reports of broken links except the one case reported on threads?

I have an expectation that WEBSITE_URL is set same as WEBAPP_URL by default(as set in .env.example), unless someone changes it, so WEBSITE_URL is always set and CAL_URL is same as WEBSITE_URL with some special handling

Copy link
Member

@hariombalhara hariombalhara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a comment

@Udit-takkar
Copy link
Contributor Author

I still don't think we should revert this.

Also, if we need to revert this, it should just be the changes you made in the PR #12973. Right now it seem to be doing a complete replacement of CAL_URL.

Reverting #12973 would display preview urls as 'app.cal.com/[]' and not 'cal.com/[]'

@keithwillcode
Copy link
Contributor

@hariombalhara @Udit-takkar @zomars What do we need to do to move forward on this one?

Copy link
Member

@zomars zomars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed conflicts. LGTM

@keithwillcode
Copy link
Contributor

@zomars So merging this as is addresses @hariombalhara concerns?

@zomars zomars merged commit e6cd016 into main Feb 13, 2024
37 checks passed
@zomars zomars deleted the revert/12973 branch February 13, 2024 06:03
@zomars
Copy link
Member

zomars commented Feb 14, 2024

Seems like we have regression of this fix #10172

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consumer core area: core, team members only High priority Created by Linear-GitHub Sync
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants