From f1acedff77b39b57c4c20e46c930ed4e975877d3 Mon Sep 17 00:00:00 2001 From: Hariom Balhara Date: Wed, 26 Jul 2023 22:14:19 +0530 Subject: [PATCH] fix: Make routing-form a preinstalled app (#9836) Co-authored-by: zomars --- .github/workflows/e2e-app-store.yml | 2 +- .github/workflows/e2e-embed-react.yml | 2 +- .github/workflows/e2e-embed.yml | 2 +- apps/web/middleware.ts | 28 +++++++++--- apps/web/pages/apps/[slug]/[...pages].tsx | 9 +++- apps/web/playwright/webhook.e2e.ts | 9 +--- packages/app-store/routing-forms/config.json | 4 +- .../emails/components/ResponseEmail.tsx | 2 +- .../pages/form-edit/[...appPages].tsx | 2 +- .../pages/reporting/[...appPages].tsx | 2 +- .../pages/route-builder/[...appPages].tsx | 2 +- .../playwright/tests/basic.e2e.ts | 43 ++++++------------- packages/app-store/typeform/config.json | 3 +- .../typeform/playwright/tests/basic.e2e.ts | 10 ++--- packages/features/shell/Shell.tsx | 22 +++------- packages/types/App.d.ts | 5 +++ 16 files changed, 70 insertions(+), 77 deletions(-) diff --git a/.github/workflows/e2e-app-store.yml b/.github/workflows/e2e-app-store.yml index f98dc1d6e2..1d8ef28bcb 100644 --- a/.github/workflows/e2e-app-store.yml +++ b/.github/workflows/e2e-app-store.yml @@ -39,4 +39,4 @@ jobs: uses: actions/upload-artifact@v2 with: name: app-store-results-${{ matrix.shard }}_${{ strategy.job-total }} - path: app-store-results + path: test-results diff --git a/.github/workflows/e2e-embed-react.yml b/.github/workflows/e2e-embed-react.yml index 95c26e3781..d95efc64ec 100644 --- a/.github/workflows/e2e-embed-react.yml +++ b/.github/workflows/e2e-embed-react.yml @@ -41,4 +41,4 @@ jobs: uses: actions/upload-artifact@v2 with: name: embed-react-results-${{ matrix.shard }}_${{ strategy.job-total }} - path: embed-react-results + path: test-results diff --git a/.github/workflows/e2e-embed.yml b/.github/workflows/e2e-embed.yml index 96ec8e0251..a909e3afc3 100644 --- a/.github/workflows/e2e-embed.yml +++ b/.github/workflows/e2e-embed.yml @@ -39,4 +39,4 @@ jobs: uses: actions/upload-artifact@v2 with: name: embed-core-results-${{ matrix.shard }}_${{ strategy.job-total }} - path: embed-core-results + path: test-results diff --git a/apps/web/middleware.ts b/apps/web/middleware.ts index 418938220e..d877adde6d 100644 --- a/apps/web/middleware.ts +++ b/apps/web/middleware.ts @@ -54,10 +54,9 @@ const middleware: NextMiddleware = async (req) => { } } - // Don't 404 old routing_forms links - if (url.pathname.startsWith("/apps/routing_forms")) { - url.pathname = url.pathname.replace("/apps/routing_forms", "/apps/routing-forms"); - return NextResponse.rewrite(url); + const res = routingForms.handle(url); + if (res) { + return res; } if (url.pathname.startsWith("/api/trpc/")) { @@ -76,15 +75,34 @@ const middleware: NextMiddleware = async (req) => { }); }; +const routingForms = { + handle: (url: URL) => { + // Next.config.js Redirects don't handle Client Side navigations and we need that. + // So, we add the rewrite here instead. + if (url.pathname.startsWith("/routing-forms/")) { + url.pathname = url.pathname.replace(/^\/routing-forms\//, "/apps/routing-forms/"); + return NextResponse.rewrite(url); + } + + // Don't 404 old routing_forms links + if (url.pathname.startsWith("/apps/routing_forms")) { + url.pathname = url.pathname.replace(/^\/apps\/routing_forms\//, "/apps/routing-forms/"); + return NextResponse.rewrite(url); + } + }, + // Matcher paths that are required by the handle function to work + paths: ["/apps/routing_forms/:path*", "/routing-forms/:path*"], +}; + export const config = { matcher: [ "/((?!_next|.*avatar.png$|favicon.ico$).*)", "/api/collect-events/:path*", "/api/auth/:path*", - "/apps/routing_forms/:path*", "/:path*/embed", "/api/trpc/:path*", "/auth/login", + ...routingForms.paths, ], }; diff --git a/apps/web/pages/apps/[slug]/[...pages].tsx b/apps/web/pages/apps/[slug]/[...pages].tsx index cb67cd2211..e770f894c3 100644 --- a/apps/web/pages/apps/[slug]/[...pages].tsx +++ b/apps/web/pages/apps/[slug]/[...pages].tsx @@ -1,6 +1,7 @@ import type { GetServerSidePropsContext } from "next"; import { useRouter } from "next/router"; +import { getAppWithMetadata } from "@calcom/app-store/_appRegistry"; import RoutingFormsRoutingConfig from "@calcom/app-store/routing-forms/pages/app-routing.config"; import TypeformRoutingConfig from "@calcom/app-store/typeform/pages/app-routing.config"; import { getServerSession } from "@calcom/features/auth/lib/getServerSession"; @@ -127,6 +128,12 @@ export async function getServerSideProps( params.appPages = pages.slice(1); const session = await getServerSession({ req, res }); const user = session?.user; + const app = await getAppWithMetadata({ slug: appName }); + if (!app) { + return { + notFound: true, + }; + } const result = await route.getServerSideProps( context as GetServerSidePropsContext<{ @@ -153,7 +160,7 @@ export async function getServerSideProps( return { props: { appName, - appUrl: `/apps/${appName}`, + appUrl: app.simplePath || `/apps/${appName}`, ...result.props, }, }; diff --git a/apps/web/playwright/webhook.e2e.ts b/apps/web/playwright/webhook.e2e.ts index 24ed897f29..8f371cf520 100644 --- a/apps/web/playwright/webhook.e2e.ts +++ b/apps/web/playwright/webhook.e2e.ts @@ -409,13 +409,6 @@ test.describe("FORM_SUBMITTED", async () => { await page.waitForLoadState("networkidle"); - // Install Routing Forms App - await page.goto(`/apps/routing-forms`); - // eslint-disable-next-line playwright/no-conditional-in-test - - await page.click('[data-testid="install-app-button"]'); - await page.click('[data-testid="install-app-button-personal"]'); - await page.waitForLoadState("networkidle"); await page.goto(`/settings/developer/webhooks/new`); @@ -428,7 +421,7 @@ test.describe("FORM_SUBMITTED", async () => { expect(page.locator(`text='${webhookReceiver.url}'`)).toBeDefined(); await page.waitForLoadState("networkidle"); - await page.goto("/apps/routing-forms/forms"); + await page.goto("/routing-forms/forms"); await page.click('[data-testid="new-routing-form"]'); // Choose to create the Form for the user(which is the first option) and not the team await page.click('[data-testid="option-0"]'); diff --git a/packages/app-store/routing-forms/config.json b/packages/app-store/routing-forms/config.json index f6bf29d3d9..914b394a14 100644 --- a/packages/app-store/routing-forms/config.json +++ b/packages/app-store/routing-forms/config.json @@ -2,6 +2,7 @@ "/*": "Don't modify slug - If required, do it using cli edit command", "name": "Routing Forms", "title": "Routing Forms", + "isGlobal": true, "slug": "routing-forms", "type": "routing-forms_other", "logo": "icon-dark.svg", @@ -9,10 +10,11 @@ "variant": "other", "categories": ["automation"], "publisher": "Cal.com, Inc.", + "simplePath": "/routing-forms", "email": "help@cal.com", "licenseRequired": true, "teamsPlanRequired": { - "upgradeUrl": "/apps/routing-forms/forms" + "upgradeUrl": "/routing-forms/forms" }, "description": "It would allow a booker to connect with the right person or choose the right event, faster. It would work by taking inputs from the booker and using that data to route to the correct booker/event as configured by Cal user", "__createdUsingCli": true diff --git a/packages/app-store/routing-forms/emails/components/ResponseEmail.tsx b/packages/app-store/routing-forms/emails/components/ResponseEmail.tsx index 77ae541933..7fde2e765c 100644 --- a/packages/app-store/routing-forms/emails/components/ResponseEmail.tsx +++ b/packages/app-store/routing-forms/emails/components/ResponseEmail.tsx @@ -27,7 +27,7 @@ export const ResponseEmail = ({ color: "#3e3e3e", }}>

- + <>Manage this form

diff --git a/packages/app-store/routing-forms/pages/form-edit/[...appPages].tsx b/packages/app-store/routing-forms/pages/form-edit/[...appPages].tsx index fdfb51dc71..802758f56c 100644 --- a/packages/app-store/routing-forms/pages/form-edit/[...appPages].tsx +++ b/packages/app-store/routing-forms/pages/form-edit/[...appPages].tsx @@ -425,7 +425,7 @@ export default function FormEditPage({ FormEditPage.getLayout = (page: React.ReactElement) => { return ( - + {page} ); diff --git a/packages/app-store/routing-forms/pages/reporting/[...appPages].tsx b/packages/app-store/routing-forms/pages/reporting/[...appPages].tsx index b92ab481d6..9d8522697e 100644 --- a/packages/app-store/routing-forms/pages/reporting/[...appPages].tsx +++ b/packages/app-store/routing-forms/pages/reporting/[...appPages].tsx @@ -199,7 +199,7 @@ export default function ReporterWrapper({ ReporterWrapper.getLayout = (page: React.ReactElement) => { return ( - + {page} ); diff --git a/packages/app-store/routing-forms/pages/route-builder/[...appPages].tsx b/packages/app-store/routing-forms/pages/route-builder/[...appPages].tsx index 07b43aa06d..efb0431557 100644 --- a/packages/app-store/routing-forms/pages/route-builder/[...appPages].tsx +++ b/packages/app-store/routing-forms/pages/route-builder/[...appPages].tsx @@ -562,7 +562,7 @@ export default function RouteBuilder({ RouteBuilder.getLayout = (page: React.ReactElement) => { return ( - + {page} ); diff --git a/packages/app-store/routing-forms/playwright/tests/basic.e2e.ts b/packages/app-store/routing-forms/playwright/tests/basic.e2e.ts index ec6d4fb070..aad31dc55b 100644 --- a/packages/app-store/routing-forms/playwright/tests/basic.e2e.ts +++ b/packages/app-store/routing-forms/playwright/tests/basic.e2e.ts @@ -17,7 +17,7 @@ test.describe("Routing Forms", () => { const formId = await addForm(page); - await page.click('[href="/apps/routing-forms/forms"]'); + await page.click('[href*="/forms"]'); await page.waitForSelector('[data-testid="routing-forms-list"]'); // Ensure that it's visible in forms list @@ -54,7 +54,7 @@ test.describe("Routing Forms", () => { await expectCurrentFormToHaveFields(page, createdFields, types); - await page.click('[href*="/apps/routing-forms/route-builder/"]'); + await page.click('[href*="/route-builder/"]'); await selectNewRoute(page); await page.click('[data-testid="add-rule"]'); @@ -86,7 +86,7 @@ test.describe("Routing Forms", () => { }); // Add F1 as Router to F2 - await page.goto(`/apps/routing-forms/route-builder/${form2Id}`); + await page.goto(`/routing-forms/route-builder/${form2Id}`); await selectNewRoute(page, { // It should be F1. TODO: Verify that it's F1 routeSelectNumber: 2, @@ -94,7 +94,7 @@ test.describe("Routing Forms", () => { await saveCurrentForm(page); // Expect F1 fields to be available in F2 - await page.goto(`/apps/routing-forms/form-edit/${form2Id}`); + await page.goto(`/routing-forms/form-edit/${form2Id}`); //FIXME: Figure out why this delay is required. Without it field count comes out to be 1 only await new Promise((resolve) => setTimeout(resolve, 1000)); @@ -108,7 +108,7 @@ test.describe("Routing Forms", () => { }, }); - await page.goto(`/apps/routing-forms/form-edit/${form2Id}`); + await page.goto(`/routing-forms/form-edit/${form2Id}`); //FIXME: Figure out why this delay is required. Without it field count comes out to be 1 only await new Promise((resolve) => setTimeout(resolve, 1000)); expect(await page.locator('[data-testid="field"]').count()).toBe(3); @@ -119,7 +119,7 @@ test.describe("Routing Forms", () => { test("should be able to submit a prefilled form with all types of fields", async ({ page }) => { const formId = await addForm(page); - await page.click('[href*="/apps/routing-forms/route-builder/"]'); + await page.click('[href*="/route-builder/"]'); await selectNewRoute(page); await selectOption({ selector: { @@ -183,11 +183,7 @@ test.describe("Routing Forms", () => { } ); await user.apiLogin(); - // Install app - await page.goto(`/apps/routing-forms`); - await page.click('[data-testid="install-app-button"]'); - (await page.waitForSelector('[data-testid="install-app-button-personal"]')).click(); - await page.waitForURL((url) => url.pathname === `/apps/routing-forms/forms`); + await page.goto(`/routing-forms/forms`); }); test.afterEach(async ({ users }) => { @@ -203,28 +199,17 @@ test.describe("Routing Forms", () => { // This also delete forms on cascade await users.deleteAll(); }); - const createUserAndLoginAndInstallApp = async function ({ - users, - page, - }: { - users: Fixtures["users"]; - page: Page; - }) { + const createUserAndLogin = async function ({ users, page }: { users: Fixtures["users"]; page: Page }) { const user = await users.create( { username: "routing-forms" }, { seedRoutingForms: true, hasTeam: true } ); await user.apiLogin(); - // Install app - await page.goto(`/apps/routing-forms`); - await page.click('[data-testid="install-app-button"]'); - (await page.waitForSelector('[data-testid="install-app-button-personal"]')).click(); - await page.waitForURL((url) => url.pathname === `/apps/routing-forms/forms`); return user; }; test("Routing Link - Reporting and CSV Download ", async ({ page, users }) => { - const user = await createUserAndLoginAndInstallApp({ users, page }); + const user = await createUserAndLogin({ users, page }); const routingForm = user.routingForms[0]; test.setTimeout(120000); // Fill form when you are logged out @@ -235,7 +220,7 @@ test.describe("Routing Forms", () => { // Log back in to view form responses. await user.apiLogin(); - await page.goto(`/apps/routing-forms/reporting/${routingForm.id}`); + await page.goto(`/routing-forms/reporting/${routingForm.id}`); const headerEls = page.locator("[data-testid='reporting-header'] th"); // Once the response is there, React would soon render it, so 500ms is enough @@ -305,7 +290,7 @@ test.describe("Routing Forms", () => { }); test("Router URL should work", async ({ page, users }) => { - const user = await createUserAndLoginAndInstallApp({ users, page }); + const user = await createUserAndLogin({ users, page }); const routingForm = user.routingForms[0]; // Router should be publicly accessible @@ -328,7 +313,7 @@ test.describe("Routing Forms", () => { }); test("Routing Link should validate fields", async ({ page, users }) => { - const user = await createUserAndLoginAndInstallApp({ users, page }); + const user = await createUserAndLogin({ users, page }); const routingForm = user.routingForms[0]; await gotoRoutingLink({ page, formId: routingForm.id }); page.click('button[type="submit"]'); @@ -340,7 +325,7 @@ test.describe("Routing Forms", () => { }); test("Test preview should return correct route", async ({ page, users }) => { - const user = await createUserAndLoginAndInstallApp({ users, page }); + const user = await createUserAndLogin({ users, page }); const routingForm = user.routingForms[0]; page.goto(`apps/routing-forms/form-edit/${routingForm.id}`); await page.click('[data-testid="test-preview"]'); @@ -423,7 +408,7 @@ async function fillSeededForm(page: Page, routingFormId: string) { } export async function addForm(page: Page, { name = "Test Form Name" } = {}) { - await page.goto("/apps/routing-forms/forms"); + await page.goto("/routing-forms/forms"); await page.click('[data-testid="new-routing-form"]'); // Choose to create the Form for the user(which is the first option) and not the team await page.click('[data-testid="option-0"]'); diff --git a/packages/app-store/typeform/config.json b/packages/app-store/typeform/config.json index 2ff0d414e7..daf11d5a7d 100644 --- a/packages/app-store/typeform/config.json +++ b/packages/app-store/typeform/config.json @@ -10,6 +10,5 @@ "publisher": "Cal.com, Inc.", "email": "help@cal.com", "description": "Adds a link to copy Typeform Redirect URL", - "__createdUsingCli": true, - "dependencies": ["routing-forms"] + "__createdUsingCli": true } diff --git a/packages/app-store/typeform/playwright/tests/basic.e2e.ts b/packages/app-store/typeform/playwright/tests/basic.e2e.ts index 9ddc1ee603..406d58e19d 100644 --- a/packages/app-store/typeform/playwright/tests/basic.e2e.ts +++ b/packages/app-store/typeform/playwright/tests/basic.e2e.ts @@ -17,10 +17,6 @@ const installApps = async (page: Page, users: Fixtures["users"]) => { } ); await user.login(); - await page.goto(`/apps/routing-forms`); - await page.click('[data-testid="install-app-button"]'); - (await page.waitForSelector('[data-testid="install-app-button-personal"]')).click(); - await page.waitForURL((url) => url.pathname === `/apps/routing-forms/forms`); await page.goto(`/apps/typeform`); await page.click('[data-testid="install-app-button"]'); (await page.waitForSelector('[data-testid="install-app-button-personal"]')).click(); @@ -38,7 +34,7 @@ test.describe("Typeform App", () => { await installApps(page, users); context.grantPermissions(["clipboard-read", "clipboard-write"]); - await page.goto(`/apps/routing-forms/forms`); + await page.goto(`/routing-forms/forms`); const formId = await addRoutingForm(page); await addOneFieldAndDescriptionAndSaveForm(formId, page, { description: "", @@ -57,14 +53,14 @@ test.describe("Typeform App", () => { await installApps(page, users); context.grantPermissions(["clipboard-read", "clipboard-write"]); - await page.goto("/apps/routing-forms/forms"); + await page.goto("/routing-forms/forms"); const formId = await addRoutingForm(page); await addOneFieldAndDescriptionAndSaveForm(formId, page, { description: "", field: { label: "test", typeIndex: 1 }, }); - await page.goto("/apps/routing-forms/forms"); + await page.goto("/routing-forms/forms"); await page.click('[data-testid="form-dropdown"]'); await page.click('[data-testid="copy-redirect-url"]'); const text = await page.evaluate(async () => { diff --git a/packages/features/shell/Shell.tsx b/packages/features/shell/Shell.tsx index e238aa830f..a2fbf62d88 100644 --- a/packages/features/shell/Shell.tsx +++ b/packages/features/shell/Shell.tsx @@ -539,9 +539,7 @@ const navigation: NavigationItemType[] = [ isCurrent: ({ router, item }) => { 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/") - ); + return path.startsWith(item.href) || path.startsWith("/v2" + item.href); }, child: [ { @@ -551,9 +549,7 @@ const navigation: NavigationItemType[] = [ 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/") && - !path.includes("/installed") + (path.startsWith(item.href) || path.startsWith("/v2" + item.href)) && !path.includes("/installed") ); }, }, @@ -574,10 +570,10 @@ const navigation: NavigationItemType[] = [ }, { name: "Routing Forms", - href: "/apps/routing-forms/forms", + href: "/routing-forms/forms", icon: FileText, isCurrent: ({ router }) => { - return router.asPath.startsWith("/apps/routing-forms/"); + return router.asPath.startsWith("/routing-forms/"); }, }, { @@ -626,17 +622,9 @@ const Navigation = () => { }; function useShouldDisplayNavigationItem(item: NavigationItemType) { - const { status } = useSession(); - const { data: routingForms } = trpc.viewer.appById.useQuery( - { appId: "routing-forms" }, - { - enabled: status === "authenticated" && requiredCredentialNavigationItems.includes(item.name), - trpc: {}, - } - ); const flags = useFlagMap(); if (isKeyInObject(item.name, flags)) return flags[item.name]; - return !requiredCredentialNavigationItems.includes(item.name) || routingForms?.isInstalled; + return true; } const defaultIsCurrent: NavigationItemType["isCurrent"] = ({ isChild, item, router }) => { diff --git a/packages/types/App.d.ts b/packages/types/App.d.ts index 31c39385b3..61c4755301 100644 --- a/packages/types/App.d.ts +++ b/packages/types/App.d.ts @@ -115,6 +115,11 @@ export interface App { * Used to show Connect/Disconnect buttons in App Store * */ isGlobal?: boolean; + /** + * For apps that are accessible on an alternate URL(which is simpler and shorter), this can be set. + * e.g. Routing Forms App is available as /routing-forms in addition to regular /apps/routing-forms. + */ + simplePath?: string; /** A contact email, mainly to ask for support */ email: string; /** Needed API Keys (usually for global apps) */