Skip to content

fix: Crash of onboarding connected-video page #23047

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.

Sign up for GitHub

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

Merged
hbjORbj merged 1 commit into from
Aug 13, 2025
Merged

fix: Crash of onboarding connected-video page #23047

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 60 additions & 34 deletions apps/web/components/getting-started/steps-views/ConnectedVideoStep.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
import { type TFunction } from "i18next";
import { useState } from "react";
import type { Dispatch, SetStateAction } from "react";

import { useLocale } from "@calcom/lib/hooks/useLocale";
import { userMetadata } from "@calcom/prisma/zod-utils";
import { trpc } from "@calcom/trpc/react";
Expand All @@ -14,8 +18,13 @@ interface ConnectedAppStepProps {
isPageLoading: boolean;
}

const ConnectedVideoStep = (props: ConnectedAppStepProps) => {
const { nextStep, isPageLoading } = props;
const ConnectedVideoStepInner = ({
Copy link
Contributor Author

@emrysal emrysal Aug 13, 2025

Choose a reason for hiding this comment

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

This nested component allows for refactoring in the early return pattern, making it more readable and more predictable.

t,
setAnyInstalledVideoApps,
}: {
t: TFunction;
setAnyInstalledVideoApps: Dispatch<SetStateAction<boolean>>;
}) => {
const { data: queryConnectedVideoApps, isPending } = trpc.viewer.apps.integrations.useQuery({
variant: "conferencing",
onlyInstalled: false,
Expand All @@ -30,47 +39,64 @@ const ConnectedVideoStep = (props: ConnectedAppStepProps) => {
sortByMostPopular: true,
sortByInstalledFirst: true,
});
const { data } = useMeQuery();
const { t } = useLocale();
// we want to start loading immediately, after all this is a hook.
const { data, status } = useMeQuery();

const metadata = userMetadata.parse(data?.metadata);
if (isPending) {
return <StepConnectionLoader />;
}

const hasAnyInstalledVideoApps = queryConnectedVideoApps?.items.some(
(item) => item.userCredentialIds.length > 0
);
if (hasAnyInstalledVideoApps) {
setAnyInstalledVideoApps(true);
}
Comment on lines 49 to +54
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 13, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Potential state update issue with conditional logic

The state update only occurs when hasAnyInstalledVideoApps is true, but there's no handling for when it becomes false (e.g., if a user uninstalls all video apps). This could lead to stale state.

   const hasAnyInstalledVideoApps = queryConnectedVideoApps?.items.some(
     (item) => item.userCredentialIds.length > 0
   );
-  if (hasAnyInstalledVideoApps) {
-    setAnyInstalledVideoApps(true);
-  }
+  setAnyInstalledVideoApps(hasAnyInstalledVideoApps ?? false);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const hasAnyInstalledVideoApps = queryConnectedVideoApps?.items.some(
(item) => item.userCredentialIds.length > 0
);
if (hasAnyInstalledVideoApps) {
setAnyInstalledVideoApps(true);
}
const hasAnyInstalledVideoApps = queryConnectedVideoApps?.items.some(
(item) => item.userCredentialIds.length > 0
);
setAnyInstalledVideoApps(hasAnyInstalledVideoApps ?? false);
🤖 Prompt for AI Agents 
In apps/web/components/getting-started/steps-views/ConnectedVideoStep.tsx around
lines 49 to 54, the code only calls setAnyInstalledVideoApps(true) when
hasAnyInstalledVideoApps is true, which leaves state stale when it becomes
false; change the logic to always update state with the boolean result (e.g.,
compute const hasAny = !!queryConnectedVideoApps?.items?.some(...) and call
setAnyInstalledVideoApps(hasAny)) and ensure this runs in the appropriate effect
with the query result in the dependency array so the state is reset to false
when no apps are installed.

Comment on lines 49 to +54
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 13, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Potential issue with state updates during render.

Setting state during render (lines 53-54) can cause unnecessary re-renders and potential infinite loops. State updates should be performed in a useEffect hook.

+import { useEffect } from "react";

 const hasAnyInstalledVideoApps = queryConnectedVideoApps?.items.some(
   (item) => item.userCredentialIds.length > 0
 );
-if (hasAnyInstalledVideoApps) {
-  setAnyInstalledVideoApps(true);
-}
+
+useEffect(() => {
+  if (hasAnyInstalledVideoApps) {
+    setAnyInstalledVideoApps(true);
+  }
+}, [hasAnyInstalledVideoApps, setAnyInstalledVideoApps]);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const hasAnyInstalledVideoApps = queryConnectedVideoApps?.items.some(
(item) => item.userCredentialIds.length > 0
);
if (hasAnyInstalledVideoApps) {
setAnyInstalledVideoApps(true);
}
import { useEffect } from "react";
const hasAnyInstalledVideoApps = queryConnectedVideoApps?.items.some(
(item) => item.userCredentialIds.length > 0
);
useEffect(() => {
if (hasAnyInstalledVideoApps) {
setAnyInstalledVideoApps(true);
}
}, [hasAnyInstalledVideoApps, setAnyInstalledVideoApps]);
🤖 Prompt for AI Agents 
In apps/web/components/getting-started/steps-views/ConnectedVideoStep.tsx around
lines 49 to 54, the code calls setAnyInstalledVideoApps during render which can
cause extra renders or loops; move this logic into a useEffect: compute whether
any installed video apps exist from queryConnectedVideoApps (guarding for
undefined), then inside a useEffect with queryConnectedVideoApps (or its items)
in the dependency array call setAnyInstalledVideoApps only when the computed
boolean differs from current state to avoid redundant updates.


if (status !== "success") {
return <StepConnectionLoader />;
}

const result = userMetadata.safeParse(data?.metadata);
if (!result.success) {
return <StepConnectionLoader />;
}
const { data: metadata } = result;
Comment on lines +43 to +64
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 13, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for failed data fetches

The component returns StepConnectionLoader for both loading and error states without distinguishing between them. Consider adding proper error handling to provide better user feedback when the API call fails.

-  if (status !== "success") {
-    return <StepConnectionLoader />;
-  }
+  if (status === "error") {
+    // Consider showing an error message to the user
+    return <div className="text-center text-red-500">{t("error_loading_video_apps")}</div>;
+  }
+  
+  if (status !== "success") {
+    return <StepConnectionLoader />;
+  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const { data, status } = useMeQuery();
const metadata = userMetadata.parse(data?.metadata);
if (isPending) {
return <StepConnectionLoader />;
}
const hasAnyInstalledVideoApps = queryConnectedVideoApps?.items.some(
(item) => item.userCredentialIds.length > 0
);
if (hasAnyInstalledVideoApps) {
setAnyInstalledVideoApps(true);
}
if (status !== "success") {
return <StepConnectionLoader />;
}
const result = userMetadata.safeParse(data?.metadata);
if (!result.success) {
return <StepConnectionLoader />;
}
const { data: metadata } = result;
const { data, status } = useMeQuery();
if (isPending) {
return <StepConnectionLoader />;
}
const hasAnyInstalledVideoApps = queryConnectedVideoApps?.items.some(
(item) => item.userCredentialIds.length > 0
);
if (hasAnyInstalledVideoApps) {
setAnyInstalledVideoApps(true);
}
if (status === "error") {
// Consider showing an error message to the user
return <div className="text-center text-red-500">{t("error_loading_video_apps")}</div>;
}
if (status !== "success") {
return <StepConnectionLoader />;
}
const result = userMetadata.safeParse(data?.metadata);
if (!result.success) {
return <StepConnectionLoader />;
}
const { data: metadata } = result;
🤖 Prompt for AI Agents 
In apps/web/components/getting-started/steps-views/ConnectedVideoStep.tsx around
lines 43 to 64, the component currently treats loading and error the same by
returning StepConnectionLoader; update the control flow to explicitly handle the
query status: render a loading state when status/loading/isPending, render a
dedicated error UI (or message with optional retry) when status === "error" or
the fetch failed, and only proceed to parse metadata when status === "success";
also move any setAnyInstalledVideoApps state updates into a useEffect that runs
when queryConnectedVideoApps changes to avoid triggering state updates during
render.

const defaultConferencingApp = metadata?.defaultConferencingApp?.appSlug;
return (
<>
{!isPending && (
<List className="bg-default border-subtle divide-subtle scroll-bar mx-1 max-h-[45vh] divide-y !overflow-y-scroll rounded-md border p-0 sm:mx-0">
{queryConnectedVideoApps?.items &&
queryConnectedVideoApps?.items.map((item) => {
if (item.slug === "daily-video") return null; // we dont want to show daily here as it is installed by default
return (
<li key={item.name}>
{item.name && item.logo && (
<AppConnectionItem
type={item.type}
title={item.name}
isDefault={item.slug === defaultConferencingApp}
description={item.description}
dependencyData={item.dependencyData}
logo={item.logo}
slug={item.slug}
installed={item.userCredentialIds.length > 0}
defaultInstall={
!defaultConferencingApp && item.appData?.location?.linkType === "dynamic"
}
/>
)}
</li>
);
})}
</List>
)}
<List className="bg-default border-subtle divide-subtle scroll-bar mx-1 max-h-[45vh] divide-y !overflow-y-scroll rounded-md border p-0 sm:mx-0">
{queryConnectedVideoApps?.items &&
queryConnectedVideoApps?.items.map((item) => {
if (item.slug === "daily-video") return null; // we dont want to show daily here as it is installed by default
return (
<li key={item.name}>
{item.name && item.logo && (
<AppConnectionItem
type={item.type}
title={item.name}
isDefault={item.slug === defaultConferencingApp}
description={item.description}
dependencyData={item.dependencyData}
logo={item.logo}
slug={item.slug}
installed={item.userCredentialIds.length > 0}
defaultInstall={!defaultConferencingApp && item.appData?.location?.linkType === "dynamic"}
/>
)}
</li>
);
})}
</List>
);
};

{isPending && <StepConnectionLoader />}
const ConnectedVideoStep = (props: ConnectedAppStepProps) => {
const { nextStep, isPageLoading } = props;
const { t } = useLocale();
const [hasAnyInstalledVideoApps, setAnyInstalledVideoApps] = useState(false);
return (
<>
<ConnectedVideoStepInner setAnyInstalledVideoApps={setAnyInstalledVideoApps} t={t} />
<Button
EndIcon="arrow-right"
data-testid="save-video-button"
Expand Down
Loading