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: rescheduling daily #13736

Merged
merged 36 commits into from Apr 16, 2024
Merged

fix: rescheduling daily #13736

merged 36 commits into from Apr 16, 2024

Conversation

Udit-takkar
Copy link
Contributor

@Udit-takkar Udit-takkar commented Feb 16, 2024

What does this PR do?

Fixes #13563
Fixes #14502
Fixes CAL-3389
Rescheduling works fine for daily video but doesn't work when your location is empty string. It could be possible when there is no location selected on event type settings.
Screenshot 2024-02-17 at 1 11 30 AM

updateMeeting(in dailyvideo/lib/VideoApiAdapter) was never called because location is empty string.

How to reproduce the bug?

  1. Remove all location from event type settings
  2. Book a meeting
  3. Reschedule the meeting

Then check the location url is daily video url and check sidebar if it is updated to new date and time

Type of change

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

@Udit-takkar Udit-takkar requested a review from a team February 16, 2024 19:45
@github-actions github-actions bot added bookings area: bookings, availability, timezones, double booking linear Sync Github Issue from community members to Linear.app Medium priority Created by Linear-GitHub Sync labels Feb 16, 2024
Copy link
Contributor

github-actions bot commented Feb 16, 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 Feb 16, 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 Feb 16, 2024

Current Playwright Test Results Summary

✅ 302 Passing - ⚠️ 11 Flaky

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

(Last updated on 04/13/2024 01:21:16pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: f22d6e3

Started: 04/13/2024 01:17:27pm UTC

⚠️ Flakes

📄   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 single organizer address location without display location public option
Retry 2Retry 1Initial Attempt
0.37% (1) 1 / 270 run
failed over last 7 days
3.33% (9) 9 / 270 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 2Retry 1Initial Attempt
4.56% (12) 12 / 263 runs
failed over last 7 days
31.94% (84) 84 / 263 runs
flaked over last 7 days

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

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Availablity Can manage single schedule
Retry 1Initial Attempt
0.36% (1) 1 / 280 run
failed over last 7 days
25.36% (71) 71 / 280 runs
flaked over last 7 days

📄   packages/app-store/routing-forms/playwright/tests/basic.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
Routing Forms Seeded Routing Form Router URL should work
Retry 1Initial Attempt
0% (0) 0 / 270 runs
failed over last 7 days
11.85% (32) 32 / 270 runs
flaked over last 7 days
Routing Forms Seeded Routing Form Test preview should return correct route
Retry 2Retry 1Initial Attempt
0.37% (1) 1 / 269 run
failed over last 7 days
34.57% (93) 93 / 269 runs
flaked over last 7 days

📄   apps/web/playwright/profile.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
Update Profile Can update a users email (verification enabled)
Retry 1Initial Attempt
6.69% (18) 18 / 269 runs
failed over last 7 days
24.91% (67) 67 / 269 runs
flaked over last 7 days
Update Profile Can verify the newly added secondary email
Retry 1Initial Attempt
0% (0) 0 / 267 runs
failed over last 7 days
6.37% (17) 17 / 267 runs
flaked over last 7 days

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

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Teams - NonOrg -- future Team Onboarding Invite Members
Retry 1Initial Attempt
9.22% (26) 26 / 282 runs
failed over last 7 days
35.11% (99) 99 / 282 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 matching orgAutoAcceptEmail and a Verified Organization with DNS Setup Done existing user migrated to an organization
Retry 1Initial Attempt
0% (0) 0 / 269 runs
failed over last 7 days
4.46% (12) 12 / 269 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 / 270 runs
failed over last 7 days
41.85% (113) 113 / 270 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.74% (2) 2 / 270 runs
failed over last 7 days
54.07% (146) 146 / 270 runs
flaked over last 7 days

View Detailed Build Results


sean-brydon
sean-brydon previously approved these changes Feb 19, 2024
Copy link
Member

@sean-brydon sean-brydon left a comment

Choose a reason for hiding this comment

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

Tested locally works fine

@PeerRich PeerRich enabled auto-merge (squash) February 19, 2024 10:43
Copy link
Member

@CarinaWolli CarinaWolli left a comment

Choose a reason for hiding this comment

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

Webhook e2e tests are failing because of the changes

Copy link

vercel bot commented Feb 19, 2024

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

5 Ignored Deployments
Name Status Preview Comments Updated (UTC)
ai ⬜️ Ignored (Inspect) Visit Preview Apr 13, 2024 1:05pm
cal ⬜️ Ignored (Inspect) Visit Preview Apr 13, 2024 1:05pm
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Apr 13, 2024 1:05pm
qa ⬜️ Ignored (Inspect) Visit Preview Apr 13, 2024 1:05pm
qa-not-in-use ⬜️ Ignored (Inspect) Visit Preview Apr 13, 2024 1:05pm

CarinaWolli
CarinaWolli previously approved these changes Feb 19, 2024
Comment on lines -83 to -88
await expectEmailsToHaveSubject({
emails,
organizer: user,
booker: bookerObj,
eventTitle,
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this works fine locally but fails on CI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a new unit test instead "cannot book same slot multiple times" in fresh-booking to make sure which would check the email has been delivered to both booker and organizer

@dosubot dosubot bot modified the milestones: v4.0, v4.1 Apr 15, 2024
@keithwillcode keithwillcode modified the milestones: v4.0, v4.1 Apr 15, 2024
Copy link

linear bot commented Apr 16, 2024

@CarinaWolli
Copy link
Member

  1. Remove all location from event type settings
  2. Book a meeting
  3. Reschedule the meeting
    Then check the location url is daily video url and check sidebar if it is updated to new date and time

I tried that in production and the daily link did show the updated date and time. How do I reproduce this bug?

Copy link
Member

@CarinaWolli CarinaWolli left a comment

Choose a reason for hiding this comment

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

Looks good 🙏

@CarinaWolli CarinaWolli enabled auto-merge (squash) April 16, 2024 16:56
@CarinaWolli CarinaWolli merged commit 268438b into main Apr 16, 2024
42 checks passed
@CarinaWolli CarinaWolli deleted the fix/reschedule branch April 16, 2024 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bookings area: bookings, availability, timezones, double booking 🐛 bug Something isn't working cal video consumer core area: core, team members only linear Sync Github Issue from community members to Linear.app Medium priority Created by Linear-GitHub Sync
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CAL-3389] Rescheduling bug [CAL-3084] self-rescheduled meeting didn't have cal.com meeting
5 participants