Skip to content

fix: Make routing-form a preinstalled app#9836

Merged
zomars merged 11 commits into
mainfrom
hariom/cal-1869-make-routing-form-a-core-component
Jul 26, 2023
Merged

fix: Make routing-form a preinstalled app#9836
zomars merged 11 commits into
mainfrom
hariom/cal-1869-make-routing-form-a-core-component

Conversation

@hariombalhara

@hariombalhara hariombalhara commented Jun 28, 2023

Copy link
Copy Markdown
Member

What does this PR do?

Fixes #10372
Fixes #9351

Routing Forms is a Default app now, globally installed.
image

It will remain accessible at/apps/routing-formsbut will be referred to as /routing-forms throughout the app.

Demo Loom

Type of change

  • Improvement

How should this be tested?

  • See the loom
  • You simply need to create a new account and you would see Routing Forms App preinstalled.

Mandatory Tasks

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

Checklist

  • I haven't checked if my changes generate no new warnings

@linear

linear Bot commented Jun 28, 2023

Copy link
Copy Markdown
CAL-1869 Make Routing Form a core component.

We are going to support creating a Routing Form for a team.

After that, it would require all team members to be able to access a Routing Form(even if the Routing Form App isn't installed by that user).

So, it makes even more sense to move it to core now.

@vercel

vercel Bot commented Jun 28, 2023

Copy link
Copy Markdown

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

@github-actions

github-actions Bot commented Jun 28, 2023

Copy link
Copy Markdown
Contributor

Thank you for following the naming conventions! 🙏

@hariombalhara hariombalhara changed the title WIP fix: Make routing Forms a system app Jun 28, 2023
@github-actions

github-actions Bot commented Jun 28, 2023

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

@deploysentinel

deploysentinel Bot commented Jun 28, 2023

Copy link
Copy Markdown

Current Playwright Test Results Summary

✅ 105 Passing - ⚠️ 4 Flaky

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

(Last updated on 07/26/2023 04:43:34pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: 3307ac9

Started: 07/26/2023 04:41:26pm UTC

⚠️ Flakes

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

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Stripe integration Can book a paid booking
Retry 1Initial Attempt
1.63% (5) 5 / 307 runs
failed over last 7 days
1.63% (5) 5 / 307 runs
flaked over last 7 days

📄   packages/embeds/embed-core/playwright/tests/action-based.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
Popup Tests should be able to reschedule
Retry 1Initial Attempt
1.38% (3) 3 / 217 runs
failed over last 7 days
98.16% (213) 213 / 217 runs
flaked over last 7 days
Popup Tests should open embed iframe on click - Configured with light theme
Retry 1Initial Attempt
0% (0) 0 / 216 runs
failed over last 7 days
6.48% (14) 14 / 216 runs
flaked over last 7 days

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

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
FORM_SUBMITTED can submit a form and get a submission event
Retry 1Initial Attempt
1.94% (6) 6 / 309 runs
failed over last 7 days
7.44% (23) 23 / 309 runs
flaked over last 7 days

View Detailed Build Results


@github-actions

Copy link
Copy Markdown
Contributor

This PR is being marked as stale due to inactivity.

@github-actions github-actions Bot added the Stale label Jul 13, 2023
@PeerRich PeerRich added the core area: core, team members only label Jul 21, 2023
};

function useShouldDisplayNavigationItem(item: NavigationItemType) {
const { status } = useSession();

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.

No condition needed now.

const path = router.asPath.split("?")[0];
// During Server rendering path is /v2/apps but on client it becomes /apps(weird..)
return (
(path.startsWith(item.href) || path.startsWith("/v2" + item.href)) && !path.includes("routing-forms/")

@hariombalhara hariombalhara Jul 26, 2023

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.

We would be accessing routing-forms as /routing-forms now instead of /apps/routing-forms. So, Apps navigation item doesn't need to detect if it's Routing Form or App that's selected.

Comment thread apps/web/next.config.js Outdated
destination: "/booking/:path*",
},
{
source: "/routing-forms/:slug*",

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.

Routing Forms is now system app. So, allow accessing it directly. This also allows us to never have to move Routing Forms app outside AppStore unless needed for some other reason.

"/*": "Don't modify slug - If required, do it using cli edit command",
"name": "Routing Forms",
"title": "Routing Forms",
"isGlobal": true,

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.

Make Routing Forms app preinstalled.

Comment thread apps/web/middleware.ts
return NextResponse.rewrite(url);
}

// Don't 404 old routing_forms links

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.

/apps/routing_forms does 404 for me, should that work?
Screenshot 2023-07-26 at 12 30 24

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.

Fixed it here. #10412. Should have worked

Comment thread packages/types/App.d.ts Outdated

@zomars zomars left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good enough to ship. We might depart from all [...pages] and [...appPages] in the future and give it a proper location in their respective directories.

Ship it

@zomars zomars enabled auto-merge (squash) July 26, 2023 16:34
@zomars zomars merged commit f1acedf into main Jul 26, 2023
@zomars zomars deleted the hariom/cal-1869-make-routing-form-a-core-component branch July 26, 2023 16:44
joeauyeung pushed a commit that referenced this pull request Jul 26, 2023
Co-authored-by: zomars <zomars@me.com>
sean-brydon pushed a commit that referenced this pull request Jul 31, 2023
Co-authored-by: zomars <zomars@me.com>
sean-brydon pushed a commit that referenced this pull request Aug 1, 2023
Co-authored-by: zomars <zomars@me.com>
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 High priority Created by Linear-GitHub Sync organizations area: organizations, orgs

Projects

No open projects
Status: No status

Development

Successfully merging this pull request may close these issues.

[CAL-2257] Orgs: Routing forms can't be installed for the Org. [CAL-1869] Make Routing Form a core component.

4 participants