Skip to content

fix: name value can be undefined for existing name type fields#10298

Merged
emrysal merged 2 commits into
mainfrom
fix/crash-on-booking-page
Jul 21, 2023
Merged

fix: name value can be undefined for existing name type fields#10298
emrysal merged 2 commits into
mainfrom
fix/crash-on-booking-page

Conversation

@hariombalhara
Copy link
Copy Markdown
Member

@hariombalhara hariombalhara commented Jul 21, 2023

What does this PR do?

Fixes booking page crash when a user has a legacy name type of field which was simply a text field. Now, name field is much more than that.

Type of change

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

How should this be tested?

  • Add a "short text" user field. Go to DB and change it's type to 'name'(because name type can't be added from UI. User had it already as it was allowed earlier but wasn't useful)
Screenshot 2023-07-21 at 8 23 37 PM
  • Now try booking the page and observe the crash.

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

Couple of findings

- I should really not have assumed that the value can't be `undefined` [here](https://github.com/calcom/cal.com/blob/5bb39dda3a81070b26c43665d6ed20d123638317/packages/features/form-builder/Components.tsx#L84).  TypeScript didn't catch it because we [do type assertions here.](https://github.com/calcom/cal.com/blob/5bb39dda3a81070b26c43665d6ed20d123638317/packages/features/form-builder/FormBuilderField.tsx#L211)
- I thought about the possibility of someone having a `name` type of field already but missed testing it out in the final PR. It could have been caught
- This bug doesn't affect anyone else that doesn't have a 'name' type of field added already.

@hariombalhara hariombalhara added the 📉 regressing This used to work. Now it doesn't anymore. label Jul 21, 2023
@hariombalhara hariombalhara self-assigned this Jul 21, 2023
@vercel
Copy link
Copy Markdown

vercel Bot commented Jul 21, 2023

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

Name Status Preview Comments Updated (UTC)
api ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 21, 2023 3:10pm
cal-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 21, 2023 3:10pm
dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 21, 2023 3:10pm
qa 🔄 Building (Inspect) Jul 21, 2023 3:10pm
ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 21, 2023 3:10pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Jul 21, 2023 3:10pm

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 21, 2023

Thank you for following the naming conventions! 🙏

@zomars zomars added the core area: core, team members only label Jul 21, 2023
@hariombalhara hariombalhara changed the title name can be undefined for existing name type fields name value can be undefined for existing name type fields Jul 21, 2023
@hariombalhara hariombalhara changed the title name value can be undefined for existing name type fields fix: name value can be undefined for existing name type fields Jul 21, 2023
@alwaysmeticulous
Copy link
Copy Markdown

alwaysmeticulous Bot commented Jul 21, 2023

🤖 Meticulous spotted visual differences in 82 of 207 screens tested: view and approve differences detected.

Last updated for commit ed8a4ae. This comment will update as new commits are pushed.

Copy link
Copy Markdown
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.

Tested and works 👍

@emrysal emrysal enabled auto-merge (squash) July 21, 2023 15:11
@github-actions
Copy link
Copy Markdown
Contributor

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

@emrysal emrysal merged commit 299f74c into main Jul 21, 2023
@emrysal emrysal deleted the fix/crash-on-booking-page branch July 21, 2023 15:14
fritterhoff pushed a commit to hm-edu/cal.com that referenced this pull request Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only 📉 regressing This used to work. Now it doesn't anymore.

Projects

No open projects
Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants