Skip to content

feat: routing form fields as variables in route#13519

Merged
keithwillcode merged 13 commits intomainfrom
feat/routing-form-variable
Feb 2, 2024
Merged

feat: routing form fields as variables in route#13519
keithwillcode merged 13 commits intomainfrom
feat/routing-form-variable

Conversation

@CarinaWolli
Copy link
Copy Markdown
Member

@CarinaWolli CarinaWolli commented Feb 2, 2024

What does this PR do?

Allows using a field identifier as a variable for event type redirects.

Loom: https://www.loom.com/share/2bd8b552028740fc9c4663e0cff08eb9

Type of change

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

How should this be tested?

  • See loom

Checklist

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

@vercel
Copy link
Copy Markdown

vercel Bot commented Feb 2, 2024

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 Feb 2, 2024 8:24pm
6 Ignored Deployments
Name Status Preview Comments Updated (UTC)
ai ⬜️ Ignored (Inspect) Visit Preview Feb 2, 2024 8:24pm
cal ⬜️ Ignored (Inspect) Visit Preview Feb 2, 2024 8:24pm
cal-demo ⬜️ Ignored (Inspect) Visit Preview Feb 2, 2024 8:24pm
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Feb 2, 2024 8:24pm
qa ⬜️ Ignored (Inspect) Visit Preview Feb 2, 2024 8:24pm
ui ⬜️ Ignored (Inspect) Visit Preview Feb 2, 2024 8:24pm

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 2, 2024

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 2, 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! 🙌

@deploysentinel
Copy link
Copy Markdown

deploysentinel Bot commented Feb 2, 2024

Current Playwright Test Results Summary

✅ 75 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/02/2024 10:10:09pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: 442fc67

Started: 02/02/2024 10:05:23pm UTC

⚠️ Flakes

📄   apps/web/playwright/locale.e2e.ts • 13 Flakes

Top 1 Common Error Messages

null

13 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
unauthorized user sees correct translations (de) should use correct translations and html attributes
Retry 1Initial Attempt
-28.46% (-70) -70 / 246 runs
failed over last 7 days
28.46% (70) 70 / 246 runs
flaked over last 7 days
unauthorized user sees correct translations (ar) should use correct translations and html attributes
Retry 1Initial Attempt
-28.46% (-70) -70 / 246 runs
failed over last 7 days
28.46% (70) 70 / 246 runs
flaked over last 7 days
unauthorized user sees correct translations (zh) should use correct translations and html attributes
Retry 1Initial Attempt
-28.46% (-70) -70 / 246 runs
failed over last 7 days
28.46% (70) 70 / 246 runs
flaked over last 7 days
unauthorized user sees correct translations (zh-CN) should use correct translations and html attributes
Retry 1Initial Attempt
-28.46% (-70) -70 / 246 runs
failed over last 7 days
28.46% (70) 70 / 246 runs
flaked over last 7 days
unauthorized user sees correct translations (zh-TW) should use correct translations and html attributes
Retry 1Initial Attempt
-28.05% (-69) -69 / 246 runs
failed over last 7 days
28.46% (70) 70 / 246 runs
flaked over last 7 days
unauthorized user sees correct translations (pt) should use correct translations and html attributes
Retry 1Initial Attempt
-28.57% (-70) -70 / 245 runs
failed over last 7 days
28.57% (70) 70 / 245 runs
flaked over last 7 days
unauthorized user sees correct translations (pt-br) should use correct translations and html attributes
Retry 1Initial Attempt
-28.57% (-70) -70 / 245 runs
failed over last 7 days
28.57% (70) 70 / 245 runs
flaked over last 7 days
unauthorized user sees correct translations (es-419) should use correct translations and html attributes
Retry 1Initial Attempt
-28.57% (-70) -70 / 245 runs
failed over last 7 days
28.57% (70) 70 / 245 runs
flaked over last 7 days
authorized user sees correct translations (de) should return correct translations and html attributes
Retry 1Initial Attempt
-28.57% (-70) -70 / 245 runs
failed over last 7 days
28.57% (70) 70 / 245 runs
flaked over last 7 days
authorized user sees correct translations (pt-br) should return correct translations and html attributes
Retry 1Initial Attempt
-14.29% (-35) -35 / 245 runs
failed over last 7 days
18.78% (46) 46 / 245 runs
flaked over last 7 days
authorized user sees correct translations (ar) should return correct translations and html attributes
Retry 1Initial Attempt
-18.80% (-44) -44 / 234 runs
failed over last 7 days
19.66% (46) 46 / 234 runs
flaked over last 7 days
authorized user sees changed translations (de->ar) should return correct translations and html attributes
Retry 1Initial Attempt
-9.48% (-22) -22 / 232 runs
failed over last 7 days
18.53% (43) 43 / 232 runs
flaked over last 7 days
authorized user sees changed translations (de->pt-BR) [locale1] should return correct translations and html attributes
Retry 1Initial Attempt
-9.48% (-20) -20 / 211 runs
failed over last 7 days
18.96% (40) 40 / 211 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
4.51% (11) 11 / 244 runs
failed over last 7 days
42.21% (103) 103 / 244 runs
flaked over last 7 days

📄   apps/web/playwright/integrations-stripe.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Stripe integration When event is paid and confirmed Payment should confirm pending payment booking
Retry 1Initial Attempt
0% (0) 0 / 246 runs
failed over last 7 days
3.25% (8) 8 / 246 runs
flaked over last 7 days

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

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Insights should have all option in team-and-self filter as admin
Retry 1Initial Attempt
0.39% (1) 1 / 258 run
failed over last 7 days
0.39% (1) 1 / 258 run
flaked over last 7 days

View Detailed Build Results


@CarinaWolli CarinaWolli marked this pull request as ready for review February 2, 2024 16:52
@CarinaWolli CarinaWolli requested a review from a team February 2, 2024 16:54
@CarinaWolli CarinaWolli added the routing-forms area: routing forms, routing, forms label Feb 2, 2024
@PeerRich PeerRich added this to the v3.8 milestone Feb 2, 2024
@PeerRich PeerRich added High priority Created by Linear-GitHub Sync ✨ feature New feature or request labels Feb 2, 2024
Comment on lines +105 to +123
const regex = /\{([^\}]+)\}/g;

const variables: string[] =
decidedAction.value.match(regex)?.map((match: string) => match.slice(1, -1)) || [];

let eventTypeUrl = decidedAction.value;

variables.forEach((variable) => {
for (const key in response) {
const identifier = getFieldIdentifier(fields.find((field) => field.id === key));
if (identifier === variable) {
eventTypeUrl = eventTypeUrl.replace(
`{${variable}}`,
slugify(response[key].value.toString() || "")
);
}
}
});
await router.push(`/${eventTypeUrl}?${allURLSearchParams}`);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We happened to change the same piece of code for 2 different reasons 😅

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yeah I saw that too 😅 we can merge yours first and I'll handle the conflicts

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Mine is not that urgent. Happy to merge it first. Could you extract out this logic in a separate function

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

extracted it into a separate function, @hariombalhara can you re-review?

@hariombalhara
Copy link
Copy Markdown
Member

Looks like the perfect solution.

We can add some improvements later like

  • User isn't able to add variables that don't exist in the Form
  • Whenever user changes an identifier and it's used in a route, we update it there as well.

Comment thread packages/app-store/routing-forms/pages/routing-link/[...appPages].tsx Outdated
hariombalhara
hariombalhara previously approved these changes Feb 2, 2024
Copy link
Copy Markdown
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.

LGTM !!

…ges].tsx

Co-authored-by: Hariom Balhara <hariombalhara@gmail.com>
@keithwillcode keithwillcode merged commit 4aa040d into main Feb 2, 2024
@keithwillcode keithwillcode deleted the feat/routing-form-variable branch February 2, 2024 22:25
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 ✨ feature New feature or request High priority Created by Linear-GitHub Sync routing-forms area: routing forms, routing, forms

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants