From e0ef30eb523c0d9e8ece47e364d3c01af0a64dee Mon Sep 17 00:00:00 2001 From: "GitStart-Cal.com" <121884634+gitstart-calcom@users.noreply.github.com> Date: Mon, 10 Jul 2023 10:24:04 -0300 Subject: [PATCH] fix: /routing-form/single - Don't clear edits when moving from one tab to the other (#8125) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: gitstart-calcom Co-authored-by: Omar López Co-authored-by: Nafees Nazik <84864519+G3root@users.noreply.github.com> Co-authored-by: Udit Takkar <53316345+Udit-takkar@users.noreply.github.com> Co-authored-by: Keith Williams Co-authored-by: Joe Au-Yeung <65426560+joeauyeung@users.noreply.github.com> --- apps/web/pages/apps/[slug]/[...pages].tsx | 2 +- .../routing-forms/components/SingleForm.tsx | 32 +++++++++--- .../pages/app-routing.config.tsx | 2 + .../pages/form-edit/[...appPages].tsx | 20 +++++-- .../pages/forms/[...appPages].tsx | 8 +++ .../pages/layout-handler/[...appPages].tsx | 52 +++++++++++++++++++ .../pages/route-builder/[...appPages].tsx | 16 +++--- .../playwright/tests/basic.e2e.ts | 4 +- 8 files changed, 114 insertions(+), 22 deletions(-) create mode 100644 packages/app-store/routing-forms/pages/layout-handler/[...appPages].tsx diff --git a/apps/web/pages/apps/[slug]/[...pages].tsx b/apps/web/pages/apps/[slug]/[...pages].tsx index 5d7acac573..cb67cd2211 100644 --- a/apps/web/pages/apps/[slug]/[...pages].tsx +++ b/apps/web/pages/apps/[slug]/[...pages].tsx @@ -46,7 +46,7 @@ function getRoute(appName: string, pages: string[]) { } as NotFound; } const mainPage = pages[0]; - const appPage = routingConfig[mainPage] as AppPageType; + const appPage = routingConfig.layoutHandler || (routingConfig[mainPage] as AppPageType); if (!appPage) { return { diff --git a/packages/app-store/routing-forms/components/SingleForm.tsx b/packages/app-store/routing-forms/components/SingleForm.tsx index f591575971..f3fb4031f3 100644 --- a/packages/app-store/routing-forms/components/SingleForm.tsx +++ b/packages/app-store/routing-forms/components/SingleForm.tsx @@ -2,7 +2,7 @@ import type { App_RoutingForms_Form, Team } from "@prisma/client"; import Link from "next/link"; import { useEffect, useState } from "react"; import type { UseFormReturn } from "react-hook-form"; -import { Controller, useForm } from "react-hook-form"; +import { Controller, useFormContext } from "react-hook-form"; import { ShellMain } from "@calcom/features/shell/Shell"; import useApp from "@calcom/lib/hooks/useApp"; @@ -247,19 +247,39 @@ function SingleForm({ form, appUrl, Page }: SingleFormComponentProps) { const [isTestPreviewOpen, setIsTestPreviewOpen] = useState(false); const [response, setResponse] = useState({}); const [decidedAction, setDecidedAction] = useState(null); + const [skipFirstUpdate, setSkipFirstUpdate] = useState(true); function testRouting() { const action = processRoute({ form, response }); setDecidedAction(action); } - const hookForm = useForm({ - defaultValues: form, - }); + const hookForm = useFormContext(); useEffect(() => { - hookForm.reset(form); - }, [form, hookForm]); + // The first time a tab is opened, the hookForm copies the form data (saved version, from the backend), + // and then it is considered the source of truth. + + // There are two events we need to overwrite the hookForm data with the form data coming from the server. + + // 1 - When we change the edited form. + + // 2 - When the form is saved elsewhere (such as in another browser tab) + + // In the second case. We skipped the first execution of useEffect to differentiate a tab change from a form change, + // because each time a tab changes, a new component is created and another useEffect is executed. + // An update from the form always occurs after the first useEffect execution. + if (Object.keys(hookForm.getValues()).length === 0 || hookForm.getValues().id !== form.id) { + hookForm.reset(form); + } + + if (skipFirstUpdate) { + setSkipFirstUpdate(false); + } else { + hookForm.reset(form); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [form]); const mutation = trpc.viewer.appRoutingForms.formMutation.useMutation({ onSuccess() { diff --git a/packages/app-store/routing-forms/pages/app-routing.config.tsx b/packages/app-store/routing-forms/pages/app-routing.config.tsx index 7d15c99e27..151352ef3d 100644 --- a/packages/app-store/routing-forms/pages/app-routing.config.tsx +++ b/packages/app-store/routing-forms/pages/app-routing.config.tsx @@ -1,6 +1,7 @@ //TODO: Generate this file automatically so that like in Next.js file based routing can work automatically import * as formEdit from "./form-edit/[...appPages]"; import * as forms from "./forms/[...appPages]"; +import * as LayoutHandler from "./layout-handler/[...appPages]"; import * as Reporting from "./reporting/[...appPages]"; import * as RouteBuilder from "./route-builder/[...appPages]"; import * as Router from "./router/[...appPages]"; @@ -13,6 +14,7 @@ const routingConfig = { "routing-link": RoutingLink, router: Router, reporting: Reporting, + layoutHandler: LayoutHandler, }; export default routingConfig; 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 9e10195404..fdfb51dc71 100644 --- a/packages/app-store/routing-forms/pages/form-edit/[...appPages].tsx +++ b/packages/app-store/routing-forms/pages/form-edit/[...appPages].tsx @@ -1,6 +1,7 @@ import { useAutoAnimate } from "@formkit/auto-animate/react"; import { useEffect, useState } from "react"; import type { UseFormReturn } from "react-hook-form"; +import { useWatch } from "react-hook-form"; import { Controller, useFieldArray } from "react-hook-form"; import { v4 as uuidv4 } from "uuid"; @@ -145,6 +146,17 @@ function Field({ .join("\n") ); }; + + const label = useWatch({ + control: hookForm.control, + name: `${hookFieldNamespace}.label`, + }); + + const identifier = useWatch({ + control: hookForm.control, + name: `${hookFieldNamespace}.identifier`, + }); + return (
@@ -183,10 +196,7 @@ function Field({ //This change has the same effects that already existed in relation to this field, // but written in a different way. // The identifier field will have the same value as the label field until it is changed - defaultValue={ - hookForm.watch(`${hookFieldNamespace}.identifier`) || - hookForm.watch(`${hookFieldNamespace}.label`) - } + value={identifier || routerField?.identifier || label || routerField?.label || ""} onChange={(e) => { hookForm.setValue(`${hookFieldNamespace}.identifier`, e.target.value); }} diff --git a/packages/app-store/routing-forms/pages/forms/[...appPages].tsx b/packages/app-store/routing-forms/pages/forms/[...appPages].tsx index 15744ec82a..bd5c23a0ce 100644 --- a/packages/app-store/routing-forms/pages/forms/[...appPages].tsx +++ b/packages/app-store/routing-forms/pages/forms/[...appPages].tsx @@ -1,5 +1,7 @@ // TODO: i18n import { useRouter } from "next/router"; +import { useEffect } from "react"; +import { useFormContext } from "react-hook-form"; import SkeletonLoaderTeamList from "@calcom/features/ee/teams/components/SkeletonloaderTeamList"; import { FilterResults } from "@calcom/features/filters/components/FilterResults"; @@ -46,6 +48,7 @@ import { FormActionsDropdown, FormActionsProvider, } from "../../components/FormActions"; +import type { RoutingFormWithResponseCount } from "../../components/SingleForm"; import { isFallbackRoute } from "../../lib/isFallbackRoute"; function NewFormButton() { @@ -69,6 +72,11 @@ export default function RoutingForms({ const { hasPaidPlan } = useHasPaidPlan(); const router = useRouter(); + const hookForm = useFormContext(); + useEffect(() => { + hookForm.reset({}); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); const filters = getTeamsFiltersFromQuery(router.query); const queryRes = trpc.viewer.appRoutingForms.forms.useQuery({ diff --git a/packages/app-store/routing-forms/pages/layout-handler/[...appPages].tsx b/packages/app-store/routing-forms/pages/layout-handler/[...appPages].tsx new file mode 100644 index 0000000000..fd078c8b0b --- /dev/null +++ b/packages/app-store/routing-forms/pages/layout-handler/[...appPages].tsx @@ -0,0 +1,52 @@ +import type { GetServerSidePropsContext } from "next"; +import type { NextRouter } from "next/router"; +import { useRouter } from "next/router"; +import React from "react"; +import { useForm, FormProvider } from "react-hook-form"; + +import type { AppPrisma, AppSsrInit, AppUser } from "@calcom/types/AppGetServerSideProps"; + +import type { AppProps } from "@lib/app-providers"; + +import RoutingFormsRoutingConfig from "../app-routing.config"; + +type GetServerSidePropsRestArgs = [AppPrisma, AppUser, AppSsrInit]; +type Component = { + default: React.ComponentType & Pick; + getServerSideProps?: (context: GetServerSidePropsContext, ...rest: GetServerSidePropsRestArgs) => void; +}; +const getComponent = (route: string | NextRouter): Component => { + const defaultRoute = "forms"; + const routeKey = + typeof route === "string" ? route || defaultRoute : route?.query?.pages?.[0] || defaultRoute; + return (RoutingFormsRoutingConfig as unknown as Record)[routeKey]; +}; + +export default function LayoutHandler(props: { [key: string]: unknown }) { + const methods = useForm(); + const router = useRouter(); + const pageKey = router?.query?.pages?.[0] || "forms"; + const PageComponent = getComponent(pageKey).default; + return ( + + + + ); +} + +LayoutHandler.getLayout = (page: React.ReactElement, router: NextRouter) => { + const component = getComponent(router).default; + if (component && "getLayout" in component) { + return component.getLayout?.(page, router); + } else { + return page; + } +}; + +export async function getServerSideProps( + context: GetServerSidePropsContext, + ...rest: GetServerSidePropsRestArgs +) { + const component = getComponent(context.params?.pages?.[0] || ""); + return component.getServerSideProps?.(context, ...rest) || { props: {} }; +} 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 2466b1fe39..07b43aa06d 100644 --- a/packages/app-store/routing-forms/pages/route-builder/[...appPages].tsx +++ b/packages/app-store/routing-forms/pages/route-builder/[...appPages].tsx @@ -4,6 +4,7 @@ import React, { useCallback, useState } from "react"; import { Query, Builder, Utils as QbUtils } from "react-awesome-query-builder"; // types import type { JsonTree, ImmutableTree, BuilderProps } from "react-awesome-query-builder"; +import type { UseFormReturn } from "react-hook-form"; import Shell from "@calcom/features/shell/Shell"; import { areTheySiblingEntitites } from "@calcom/lib/entityPermissionUtils"; @@ -20,6 +21,7 @@ import { Divider, } from "@calcom/ui"; +import type { RoutingFormWithResponseCount } from "../../components/SingleForm"; import SingleForm, { getServerSidePropsForSingleFormView as getServerSideProps, } from "../../components/SingleForm"; @@ -304,15 +306,13 @@ const Routes = ({ appUrl, }: { form: inferSSRProps["form"]; - // Figure out the type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - hookForm: any; + hookForm: UseFormReturn; appUrl: string; }) => { - const { routes: serializedRoutes } = form; + const { routes: serializedRoutes } = hookForm.getValues(); const { t } = useLocale(); - const config = getQueryBuilderConfig(form); + const config = getQueryBuilderConfig(hookForm.getValues()); const [routes, setRoutes] = useState(() => { const transformRoutes = () => { const _routes = serializedRoutes || [getEmptyRoute()]; @@ -343,11 +343,11 @@ const Routes = ({ userId: router.userId, }, entity2: { - teamId: form.teamId ?? null, - userId: form.userId, + teamId: hookForm.getValues().teamId ?? null, + userId: hookForm.getValues().userId, }, }); - return router.id !== form.id && routerValidInContext; + return router.id !== hookForm.getValues().id && routerValidInContext; }) .map(({ form: router }) => { return { 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 b452da3060..3b1675d4fd 100644 --- a/packages/app-store/routing-forms/playwright/tests/basic.e2e.ts +++ b/packages/app-store/routing-forms/playwright/tests/basic.e2e.ts @@ -397,7 +397,7 @@ async function expectCurrentFormToHaveFields( types: string[] ) { for (const [index, field] of Object.entries(fields)) { - expect(await page.inputValue(`[name="fields.${index}.label"]`)).toBe(field.label); + expect(await page.inputValue(`[data-testid="fields.${index}.label"]`)).toBe(field.label); expect(await page.locator(".data-testid-field-type").nth(+index).locator("div").nth(1).innerText()).toBe( types[field.typeIndex] ); @@ -519,7 +519,7 @@ export async function addOneFieldAndDescriptionAndSaveForm( const nextFieldIndex = (await page.locator('[data-testid="field"]').count()) - 1; if (form.field) { - await page.fill(`[name="fields.${nextFieldIndex}.label"]`, form.field.label); + await page.fill(`[data-testid="fields.${nextFieldIndex}.label"]`, form.field.label); await page .locator('[data-testid="field"]') .nth(nextFieldIndex)