fix: block Google Meet installation without Google Calendar dependency (#25513)
* fix: block Google Meet installation without Google Calendar dependency - Prevent users from installing Google Meet when Google Calendar is not connected - Update AppDependencyComponent to show error state for unmet dependencies - Fix InstallAppButtonChild to respect disabled prop from dependencies check - Ensure consistent dependency validation across App Store and app detail pages Fixes #25497 * test: add unit tests for dependency validation - Add tests for AppDependencyComponent (8 tests) - Add tests for InstallAppButtonChild (5 tests) - Cover dependency blocking behavior - Test visual state changes (bg-error/bg-subtle) * test: resolve type-check error in InstallAppButtonChild tests - Add complete MockCredential type definition - Create factory function to generate typed mock credentials - Eliminate code duplication across test cases * fix: remove redundant disabled prop assignment Disabled state is already passed through via props.disableInstall which gets converted to props.disabled in InstallAppButtonWithoutPlanCheck. This change addresses the review comment to avoid unnecessary duplication --------- Co-authored-by: Udit Takkar <53316345+Udit-takkar@users.noreply.github.com>
This commit is contained in:
co-authored by
Udit Takkar
parent
6bdf320640
commit
3ebac0194e
@@ -184,8 +184,7 @@ export const AppPage = ({
|
||||
enabled: !!dependencies,
|
||||
});
|
||||
|
||||
const disableInstall =
|
||||
dependencyData.data && dependencyData.data.some((dependency) => !dependency.installed);
|
||||
const disableInstall = dependencyData.data ? dependencyData.data.some((dependency) => !dependency.installed) : false;
|
||||
|
||||
// const disableInstall = requiresGCal && !gCalInstalled.data;
|
||||
|
||||
|
||||
@@ -0,0 +1,103 @@
|
||||
import { render, screen } from "@testing-library/react";
|
||||
import { vi } from "vitest";
|
||||
|
||||
import { InstallAppButtonChild } from "./InstallAppButtonChild";
|
||||
|
||||
// Mock credential type
|
||||
type MockCredential = {
|
||||
id: number;
|
||||
delegatedToId: string;
|
||||
userId: number;
|
||||
user: { email: string };
|
||||
key: { access_token: string };
|
||||
invalid: boolean;
|
||||
teamId: null;
|
||||
team: null;
|
||||
delegationCredentialId: string;
|
||||
type:
|
||||
| `${string}_calendar`
|
||||
| `${string}_messaging`
|
||||
| `${string}_payment`
|
||||
| `${string}_video`
|
||||
| `${string}_other`
|
||||
| `${string}_automation`
|
||||
| `${string}_analytics`
|
||||
| `${string}_crm`
|
||||
| `${string}_other_calendar`;
|
||||
appId: string;
|
||||
};
|
||||
|
||||
// Factory function to create mock credentials
|
||||
const createMockCredential = (overrides: Partial<MockCredential> = {}): MockCredential => ({
|
||||
id: 1,
|
||||
type: "google_calendar" as const,
|
||||
userId: 1,
|
||||
delegatedToId: "delegation-123",
|
||||
teamId: null,
|
||||
team: null,
|
||||
user: { email: "test@example.com" },
|
||||
key: { access_token: "mock_token_123" },
|
||||
delegationCredentialId: "delegation-123",
|
||||
appId: "google-calendar",
|
||||
invalid: false,
|
||||
...overrides,
|
||||
});
|
||||
|
||||
// Mock the useLocale hook
|
||||
vi.mock("@calcom/lib/hooks/useLocale", () => ({
|
||||
useLocale: () => ({
|
||||
t: (key: string) => {
|
||||
const translations = {
|
||||
install_app: "Install App",
|
||||
install_another: "Install Another",
|
||||
start_paid_trial: "Start Free Trial",
|
||||
subscribe: "Subscribe",
|
||||
};
|
||||
return translations[key as keyof typeof translations] || key;
|
||||
},
|
||||
}),
|
||||
}));
|
||||
|
||||
describe("InstallAppButtonChild", () => {
|
||||
afterEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("is disabled when credentials exist and multiInstall is false", () => {
|
||||
const mockCredentials = [createMockCredential()];
|
||||
render(<InstallAppButtonChild multiInstall={false} credentials={mockCredentials} />);
|
||||
|
||||
const button = screen.getByTestId("install-app-button");
|
||||
expect(button).toBeDisabled();
|
||||
});
|
||||
|
||||
it("is enabled when credentials exist and multiInstall is true", () => {
|
||||
const mockCredentials = [createMockCredential()];
|
||||
render(<InstallAppButtonChild multiInstall={true} credentials={mockCredentials} />);
|
||||
|
||||
const button = screen.getByTestId("install-app-button");
|
||||
expect(button).not.toBeDisabled();
|
||||
});
|
||||
|
||||
it("is disabled when disabled prop is true regardless of credentials", () => {
|
||||
render(<InstallAppButtonChild disabled={true} multiInstall={true} credentials={[]} />);
|
||||
|
||||
const button = screen.getByTestId("install-app-button");
|
||||
expect(button).toBeDisabled();
|
||||
});
|
||||
|
||||
it("combines disabled prop with credential logic correctly", () => {
|
||||
const mockCredentials = [createMockCredential()];
|
||||
render(<InstallAppButtonChild disabled={true} multiInstall={false} credentials={mockCredentials} />);
|
||||
|
||||
const button = screen.getByTestId("install-app-button");
|
||||
expect(button).toBeDisabled();
|
||||
});
|
||||
|
||||
it("is enabled when no blocking conditions exist", () => {
|
||||
render(<InstallAppButtonChild multiInstall={false} credentials={[]} disabled={false} />);
|
||||
|
||||
const button = screen.getByTestId("install-app-button");
|
||||
expect(button).not.toBeDisabled();
|
||||
});
|
||||
});
|
||||
@@ -17,6 +17,7 @@ export const InstallAppButtonChild = ({
|
||||
const { t } = useLocale();
|
||||
|
||||
const shouldDisableInstallation = !multiInstall ? !!(credentials && credentials.length) : false;
|
||||
const isDisabled = shouldDisableInstallation || props.disabled;
|
||||
|
||||
// Paid apps don't support team installs at the moment
|
||||
// Also, cal.ai(the only paid app at the moment) doesn't support team install either
|
||||
@@ -25,7 +26,7 @@ export const InstallAppButtonChild = ({
|
||||
<Button
|
||||
data-testid="install-app-button"
|
||||
{...props}
|
||||
disabled={shouldDisableInstallation}
|
||||
disabled={isDisabled}
|
||||
color="primary"
|
||||
size="base">
|
||||
{paid.trial ? t("start_paid_trial") : t("subscribe")}
|
||||
@@ -37,7 +38,7 @@ export const InstallAppButtonChild = ({
|
||||
<Button
|
||||
data-testid="install-app-button"
|
||||
{...props}
|
||||
disabled={shouldDisableInstallation}
|
||||
disabled={isDisabled}
|
||||
color="primary"
|
||||
size="base">
|
||||
{multiInstall ? t("install_another") : t("install_app")}
|
||||
|
||||
@@ -0,0 +1,177 @@
|
||||
import { render, screen } from "@testing-library/react";
|
||||
import { vi } from "vitest";
|
||||
|
||||
import { AppDependencyComponent } from "./AppDependencyComponent";
|
||||
|
||||
// Type for dependency data
|
||||
type MockDependency = {
|
||||
name: string;
|
||||
slug: string;
|
||||
installed: boolean;
|
||||
};
|
||||
|
||||
// Mock the useLocale hook
|
||||
vi.mock("@calcom/lib/hooks/useLocale", () => ({
|
||||
useLocale: () => ({
|
||||
t: (key: string, values?: Record<string, unknown>) => {
|
||||
if (key === "app_is_connected") {
|
||||
return `${values?.dependencyName} is connected`;
|
||||
}
|
||||
if (key === "this_app_requires_connected_account") {
|
||||
return `${values?.appName} requires a connected ${values?.dependencyName} account`;
|
||||
}
|
||||
if (key === "connect_app") {
|
||||
return `Connect ${values?.dependencyName}`;
|
||||
}
|
||||
return key;
|
||||
},
|
||||
}),
|
||||
}));
|
||||
|
||||
// Mock constants and UI components
|
||||
vi.mock("@calcom/lib/constants", () => ({
|
||||
WEBAPP_URL: "http://localhost:3000",
|
||||
}));
|
||||
|
||||
vi.mock("@calcom/ui/components/icon", () => ({
|
||||
Icon: ({ name, className }: { name: string; className?: string }) => (
|
||||
<svg data-testid={`${name}-icon`} className={className}>
|
||||
{name}
|
||||
</svg>
|
||||
),
|
||||
}));
|
||||
|
||||
describe("AppDependencyComponent", () => {
|
||||
// Factory function to reduce duplication
|
||||
const createMockDependency = (overrides: Partial<MockDependency> = {}): MockDependency => ({
|
||||
name: "Google Calendar",
|
||||
slug: "google-calendar",
|
||||
installed: true,
|
||||
...overrides,
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("shows success indicators when dependencies are met", () => {
|
||||
const dependency = createMockDependency({ installed: true });
|
||||
const { container } = render(
|
||||
<AppDependencyComponent
|
||||
appName="Google Meet"
|
||||
dependencyData={[dependency]}
|
||||
/>
|
||||
);
|
||||
|
||||
expect(container.firstChild).toHaveClass("bg-subtle");
|
||||
expect(screen.getByTestId("check-icon")).toBeInTheDocument();
|
||||
expect(screen.getByText("Google Calendar is connected")).toBeInTheDocument();
|
||||
expect(screen.getByText("Google Meet requires a connected Google Calendar account"))
|
||||
.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("shows error indicators and connect link when dependencies are not met", () => {
|
||||
const dependency = createMockDependency({ installed: false });
|
||||
const { container } = render(
|
||||
<AppDependencyComponent
|
||||
appName="Google Meet"
|
||||
dependencyData={[dependency]}
|
||||
/>
|
||||
);
|
||||
|
||||
expect(container.firstChild).toHaveClass("bg-error");
|
||||
expect(screen.getByTestId("circle-x-icon")).toBeInTheDocument();
|
||||
expect(screen.getByRole("link", { name: /connect google calendar/i }))
|
||||
.toHaveAttribute("href", expect.stringContaining("/apps/google-calendar"));
|
||||
expect(screen.getByText("Google Meet requires a connected Google Calendar account"))
|
||||
.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("shows mixed states when some dependencies are unmet", () => {
|
||||
const dependencies = [
|
||||
createMockDependency({ installed: true }),
|
||||
createMockDependency({ installed: false }),
|
||||
];
|
||||
|
||||
const { container } = render(
|
||||
<AppDependencyComponent
|
||||
appName="Google Meet"
|
||||
dependencyData={dependencies}
|
||||
/>
|
||||
);
|
||||
|
||||
expect(container.firstChild).toHaveClass("bg-error");
|
||||
expect(screen.getByTestId("check-icon")).toBeInTheDocument();
|
||||
expect(screen.getByTestId("circle-x-icon")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("shows only success indicators when all dependencies are met", () => {
|
||||
const dependencies = [
|
||||
createMockDependency({ installed: true }),
|
||||
createMockDependency({ name: "Zoom", slug: "zoom", installed: true }),
|
||||
];
|
||||
|
||||
const { container } = render(
|
||||
<AppDependencyComponent
|
||||
appName="Google Meet"
|
||||
dependencyData={dependencies}
|
||||
/>
|
||||
);
|
||||
|
||||
expect(container.firstChild).toHaveClass("bg-subtle");
|
||||
const checkIcons = screen.getAllByTestId("check-icon");
|
||||
expect(checkIcons).toHaveLength(2);
|
||||
expect(screen.queryByTestId("circle-x-icon")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("handles empty dependency data gracefully", () => {
|
||||
const { container } = render(
|
||||
<AppDependencyComponent appName="Google Meet" dependencyData={undefined} />
|
||||
);
|
||||
|
||||
expect(container.firstChild).toHaveClass("bg-subtle");
|
||||
expect(screen.queryByTestId("check-icon")).not.toBeInTheDocument();
|
||||
expect(screen.queryByTestId("circle-x-icon")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("handles empty dependency array gracefully", () => {
|
||||
const { container } = render(
|
||||
<AppDependencyComponent appName="Google Meet" dependencyData={[]} />
|
||||
);
|
||||
|
||||
expect(container.firstChild).toHaveClass("bg-subtle");
|
||||
expect(screen.queryByTestId("check-icon")).not.toBeInTheDocument();
|
||||
expect(screen.queryByTestId("circle-x-icon")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("treats truthy non-boolean values as installed", () => {
|
||||
const dependency = createMockDependency({
|
||||
installed: "yes" as unknown as boolean
|
||||
});
|
||||
|
||||
const { container } = render(
|
||||
<AppDependencyComponent
|
||||
appName="Google Meet"
|
||||
dependencyData={[dependency]}
|
||||
/>
|
||||
);
|
||||
|
||||
expect(container.firstChild).toHaveClass("bg-subtle");
|
||||
expect(screen.getByTestId("check-icon")).toBeInTheDocument();
|
||||
expect(screen.getByText("Google Calendar is connected")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("displays requirement message consistently", () => {
|
||||
const dependency = createMockDependency({ installed: true });
|
||||
render(
|
||||
<AppDependencyComponent
|
||||
appName="Google Meet"
|
||||
dependencyData={[dependency]}
|
||||
/>
|
||||
);
|
||||
|
||||
expect(
|
||||
screen.getByText("Google Meet requires a connected Google Calendar account")
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
@@ -16,11 +16,13 @@ export const AppDependencyComponent = ({
|
||||
}) => {
|
||||
const { t } = useLocale();
|
||||
|
||||
const hasUnmetDependencies = dependencyData ? dependencyData.some((dep) => !dep.installed) : false;
|
||||
|
||||
return (
|
||||
<div
|
||||
className={classNames(
|
||||
"rounded-md px-4 py-3",
|
||||
dependencyData && dependencyData.some((dependency) => !dependency.installed) ? "bg-cal-info" : "bg-subtle"
|
||||
hasUnmetDependencies ? "bg-error" : "bg-subtle"
|
||||
)}>
|
||||
{dependencyData &&
|
||||
dependencyData.map((dependency) => {
|
||||
@@ -50,9 +52,9 @@ export const AppDependencyComponent = ({
|
||||
</div>
|
||||
) : (
|
||||
<div className="items-start space-x-2.5">
|
||||
<div className="text-info flex items-start">
|
||||
<div className="text-error flex items-start">
|
||||
<div>
|
||||
<Icon name="circle-alert" className="mr-2 mt-1 font-semibold" />
|
||||
<Icon name="circle-x" className="mr-2 mt-1 font-semibold" />
|
||||
</div>
|
||||
<div>
|
||||
<span className="font-semibold">
|
||||
@@ -62,13 +64,12 @@ export const AppDependencyComponent = ({
|
||||
interpolation: { escapeValue: false },
|
||||
})}
|
||||
</span>
|
||||
|
||||
<div>
|
||||
<div>
|
||||
<>
|
||||
<Link
|
||||
href={`${WEBAPP_URL}/apps/${dependency.slug}`}
|
||||
className="text-info flex items-center underline">
|
||||
className="text-error flex items-center underline">
|
||||
<span className="mr-1">
|
||||
{t("connect_app", { dependencyName: dependency.name })}
|
||||
</span>
|
||||
|
||||
Reference in New Issue
Block a user