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

Improvements: Prefill Routing Forms and connect prefilling with Booking Form #8780

Merged
merged 7 commits into from May 17, 2023

Conversation

hariombalhara
Copy link
Member

@hariombalhara hariombalhara commented May 9, 2023

What does this PR do?

  • Routing Form can now be prefilled with Query Params. Similar to Booking Form - Requested here(Private Link)
  • Any query params that are provided to Routing Form aren't lost now in navigating to Booking Form or external redirect
  • Any responses provided in Routing Form are also forwarded to Booking Form for prefilling. So that a question if responsed in Routing Form can be prefilled in booking form if it's there as well.

Also, Fixes #8839

Demo

Environment: Production

Type of change

  • New feature (non-breaking change which adds functionality)

How should this be tested?

  • Test A
  • Test B

Checklist

  • I haven't added tests that prove my fix is effective or that my feature works
  • I haven't checked if new and existing unit tests pass locally with my changes

@vercel
Copy link

vercel bot commented May 9, 2023

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

Name Status Preview Comments Updated (UTC)
cal ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 17, 2023 4:53am
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
ui ⬜️ Ignored (Inspect) Visit Preview May 17, 2023 4:53am

@hariombalhara hariombalhara force-pushed the improvements/prefill-support-routing-forms branch from 93164a9 to 45fb94c Compare May 9, 2023 14:54
@github-actions
Copy link
Contributor

github-actions bot commented May 9, 2023

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

@deploysentinel
Copy link

deploysentinel bot commented May 9, 2023

Current Playwright Test Results Summary

✅ 109 Passing - ❌ 3 Failing - ⚠️ 2 Flaky

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

(Last updated on 05/17/2023 05:11:13am UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: 39c83d9

Started: 05/17/2023 04:59:28am UTC

❌ Failures

📄   packages/embeds/embed-core/playwright/tests/action-based.test.ts • 2 Failures

Top 1 Common Error Messages

Expected to provide an iframe, got null

2 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Popup Tests should open embed iframe on click - Configured with light theme
Retry 2Retry 1Initial Attempt
Error: Expected to provide an iframe, got null
Expected to provide an iframe, got null
5.52% (8) 8 / 145 runs
failed over last 7 days
0% (0) 0 / 145 runs
flaked over last 7 days
Popup Tests should be able to reschedule
Retry 2Retry 1Initial Attempt
Error: Expected to provide an iframe, got null
Expected to provide an iframe, got null
10.20% (15) 15 / 147 runs
failed over last 7 days
45.58% (67) 67 / 147 runs
flaked over last 7 days

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

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Inline Iframe - Configured with Dark Theme
Retry 1Initial Attempt
Error: Embed iframe not found
Embed iframe not found
5.52% (8) 8 / 145 runs
failed over last 7 days
5.52% (8) 8 / 145 runs
flaked over last 7 days

⚠️ Flakes

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

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Reschedule Tests -- new-booker Should display request reschedule send on bookings/cancelled
Retry 1Initial Attempt
0% (0) 0 / 281 runs
failed over last 7 days
2.49% (7) 7 / 281 runs
flaked over last 7 days

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

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Booking with Seats -- new-booker Reschedule for booking with seats -- old-booker Should reschedule booking with seats and if everyone rescheduled it should be deleted
Retry 1Initial Attempt
0% (0) 0 / 290 runs
failed over last 7 days
82.07% (238) 238 / 290 runs
flaked over last 7 days

View Detailed Build Results


@@ -201,7 +201,7 @@ export const FormBuilder = function FormBuilder({
// It has the same drawback that if the label is changed, the value of the option will change. It is not a big deal for now.
value.splice(index, 1, {
label: e.target.value,
value: e.target.value.toLowerCase().trim(),
Copy link
Member Author

Choose a reason for hiding this comment

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

Routing Form uses the strategy of not lowercasing the option label and using it as is.
Changed this logic here to match that, otherwise an option value forwarded from Routing Form won't be prefillable in Booking Form.
It also fixes #8839


return (
<Select
className="mb-2"
onChange={(items) => {
setValue(items?.map((item) => item.value));
}}
defaultValue={defaultValue}
value={validValue}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a controlled input so it should use value field.

@@ -276,6 +276,9 @@ export function FormActionsProvider({ appUrl, children }: { appUrl: string; chil
}
return { previousValue };
},
onSuccess: () => {
showToast(t("form_updated_successfully"), "success");
Copy link
Member Author

Choose a reason for hiding this comment

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

On toggling the form there was no toast coming up.


return (
<Select
className="mb-2"
onChange={(items) => {
setValue(items?.map((item) => item.value));
}}
defaultValue={defaultValue}
value={optionsFromList}
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 is supposed to be a controlled input

@hariombalhara hariombalhara marked this pull request as ready for review May 16, 2023 11:30
@hariombalhara hariombalhara requested a review from a team May 16, 2023 11:30
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.

Nice looks good 🙏

@hariombalhara hariombalhara added this pull request to the merge queue May 17, 2023
Merged via the queue into main with commit b8b6c48 May 17, 2023
20 of 21 checks passed
@hariombalhara hariombalhara deleted the improvements/prefill-support-routing-forms branch May 17, 2023 09:19
sean-brydon pushed a commit that referenced this pull request May 18, 2023
…ng Form (#8780)

* Support prefilling routing form and prefilling Booking form through routing form

* Use Option Value as is instead of lowercasing

* Fix prefill validation issue

* Add prefill tests

* Fix Routing Form tests

* Small fix
@PeerRich PeerRich added the core area: core, team members only label Jul 24, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CAL-1508] Response for fields with options is shown as lowercase, even when options are in uppercase
3 participants