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.

Sign up for GitHub

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

Jump to bottom

fix: booking error in case of no calendar credential but stray destinationCalendar #12680

Merged
merged 1 commit into from
Dec 5, 2023
Merged

fix: booking error in case of no calendar credential but stray destinationCalendar #12680

merged 1 commit into from
Dec 5, 2023
+12 −11

Conversation

hariombalhara
Copy link
Member

@hariombalhara hariombalhara commented Dec 5, 2023 •
edited

What does this PR do?

Fixes #12464

Type of change

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

How should this be tested?

  1. Install Google Calendar
  2. Ensure that it is selected as destinationCalendar for an event
  3. Set credentialId to null for the destinationCalendar
  4. Delete the Credential - Ensure that app isn't opened in any of the tabs(going to app pages would cleanup the invalid destinationCalendar)
  5. Try and do a booking and you would be able to replicate the error.

Mandatory Tasks

  • Make sure you have self-reviewed the code. A decent size PR without self-review might be rejected.

Checklist

  • I haven't added tests that prove my fix is effective or that my feature works

@vercel Vercel
Copy link

vercel bot commented Dec 5, 2023 •
edited

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 Dec 5, 2023 3:41pm
api ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 5, 2023 3:41pm
dev ❌ Failed (Inspect) Dec 5, 2023 3:41pm
4 Ignored Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Dec 5, 2023 3:41pm
cal-demo ⬜️ Ignored (Inspect) Dec 5, 2023 3:41pm
qa ⬜️ Ignored (Inspect) Visit Preview Dec 5, 2023 3:41pm
ui ⬜️ Ignored (Inspect) Visit Preview Dec 5, 2023 3:41pm

@hariombalhara
Copy link
Member Author

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 5, 2023 •
edited

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

@zomars zomars added the core area: core, team members only label Dec 5, 2023
hariombalhara
hariombalhara commented Dec 5, 2023
View reviewed changes
packages/core/EventManager.ts
@@ -574,24 +569,16 @@ export default class EventManager {
(c) => c.type === destination.integration
);
// It might not be the first connected calendar as it seems that the order is not guaranteed to be ascending of credentialId.
const firstCalendarCredential = destinationCalendarCredentials[0];
const firstCalendarCredential = destinationCalendarCredentials[0] as
Copy link
Member Author

Choose a reason for hiding this comment

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

It could be undefined as well. So, explicitly assert it as TypeScript by default can't do that.

hariombalhara
hariombalhara commented Dec 5, 2023
View reviewed changes
packages/core/EventManager.ts
})
);

createdEvents.push(await createEvent(firstCalendarCredential, event));
Copy link
Member Author

Choose a reason for hiding this comment

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

We can't use firstCalendarCredential if it's undefined. So, this code was supposed to be in else

@hariombalhara hariombalhara self-assigned this Dec 5, 2023
@hariombalhara hariombalhara marked this pull request as ready for review December 5, 2023 14:28
@deploysentinel DeploySentinel
Copy link

deploysentinel bot commented Dec 5, 2023 •
edited

Current Playwright Test Results Summary

✅ 7 Passing - ⚠️ 1 Flaky

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

(Last updated on 12/05/2023 04:16:02pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: 3616036

Started: 12/05/2023 04:15:25pm UTC

⚠️ Flakes

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

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Organization should be able to create an organization and complete onboarding
Retry 1 • Initial Attempt
18.94% (57) 57 / 301 runs
failed over last 7 days
60.13% (181) 181 / 301 runs
flaked over last 7 days

View Detailed Build Results

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 5, 2023 •
edited

📦 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! 🙌

@hariombalhara hariombalhara force-pushed the 12-05-Fix_booking_error_in_case_of_no_calendar_credential_but_stray_destinationCalendar branch from 6f2b40f to 3616036 Compare December 5, 2023 14:38
CarinaWolli
CarinaWolli approved these changes Dec 5, 2023
View reviewed changes
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.

didn't test but code looks good to me

@emrysal emrysal enabled auto-merge (squash) December 5, 2023 16:16
@emrysal emrysal merged commit f2a59fe into main Dec 5, 2023
40 of 41 checks passed
@emrysal emrysal deleted the 12-05-Fix_booking_error_in_case_of_no_calendar_credential_but_stray_destinationCalendar branch December 5, 2023 16:16
@hariombalhara hariombalhara mentioned this pull request Dec 5, 2023
Merged
hbjORbj pushed a commit to codemod-com/cal.com-demo that referenced this pull request Dec 21, 2023
@hariombalhara @hbjORbj
Fix booking error in case of no calendar credential but stray destina…
5b7fc44 
…tionCalendar (calcom#12680)
haranrk pushed a commit to haranrk/cal.com that referenced this pull request Jan 4, 2024
@hariombalhara @haranrk
Fix booking error in case of no calendar credential but stray destina…
16a1b64 
…tionCalendar (calcom#12680)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CarinaWolli CarinaWolli CarinaWolli approved these changes

@joeauyeung joeauyeung Awaiting requested review from joeauyeung

Assignees

@hariombalhara hariombalhara

Labels
core area: core, team members only
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[CAL-2749] Could not book the meeting. Cannot read properties of undefined (reading 'appId')
4 participants
@hariombalhara @CarinaWolli @emrysal @zomars