From 904be9514894acdc64dbe7b4fb411d666cb61bc7 Mon Sep 17 00:00:00 2001 From: nitin <142569587+ehconitin@users.noreply.github.com> Date: Thu, 25 Sep 2025 13:49:09 +0530 Subject: [PATCH] Fix calendar booking step in onboarding when env var is not configured (#14707) ## The Issue When CALENDAR_BOOKING_PAGE_ID env variable is not configured, the onboarding flow still sets the booking step as pending in the database. This causes users to get stuck on a broken booking page after logout/login, as the Cal.com iframe tries to load with an empty calendar link. ## The Fix Made the booking step handling idempotent across the stack: Backend: - setOnboardingBookOnboardingPending now checks if calendar is actually configured before setting the step as pending - getOnboardingStatus auto-cleans invalid booking states when detected (booking pending but no calendar configured) - Empty strings in env are now treated as undefined in client config Frontend: - Added navigation protection to redirect away from booking pages when calendar isn't configured - Existing defensive logic in useSetNextOnboardingStatus already skips booking when no calendar ID Result - New users won't get invalid booking states - Existing bad data self-heals when users interact with the system - Backend and frontend stay in sync about when booking should be shown Fixes the issue Felix reported where users saw a broken booking page in production. I think we should keep the old CAL_LINK constant for now - while we could remove the booking onboarding step entirely, it would break the plan/pricing modal which uses it as a fallback when no calendar is configured. Open for discussion! -- May be we dont show the `Book a Call` button if the env is not set -- but we should keep it as it is if we want two different behaviors :) closes https://github.com/twentyhq/core-team-issues/issues/1558 --- ...sePageChangeEffectNavigateLocation.test.ts | 3 ++ .../usePageChangeEffectNavigateLocation.ts | 5 ++++ .../services/client-config.service.ts | 11 +++++-- .../onboarding/onboarding.service.ts | 30 ++++++++++++++++++- 4 files changed, 45 insertions(+), 4 deletions(-) diff --git a/packages/twenty-front/src/hooks/__tests__/usePageChangeEffectNavigateLocation.test.ts b/packages/twenty-front/src/hooks/__tests__/usePageChangeEffectNavigateLocation.test.ts index afcf4796e3..e269a0f6f7 100644 --- a/packages/twenty-front/src/hooks/__tests__/usePageChangeEffectNavigateLocation.test.ts +++ b/packages/twenty-front/src/hooks/__tests__/usePageChangeEffectNavigateLocation.test.ts @@ -67,9 +67,11 @@ jest.mock('recoil'); const setupMockRecoil = ( objectNamePlural?: string, verifyEmailRedirectPath?: string, + calendarBookingPageId?: string | null, ) => { jest .mocked(useRecoilValue) + .mockReturnValueOnce(calendarBookingPageId ?? 'mock-calendar-id') .mockReturnValueOnce([{ namePlural: objectNamePlural ?? '' }]) .mockReturnValueOnce(verifyEmailRedirectPath); }; @@ -343,6 +345,7 @@ describe('usePageChangeEffectNavigateLocation', () => { expect(usePageChangeEffectNavigateLocation()).toEqual(res); }, ); + describe('tests should be exhaustive', () => { it('all location, onboarding status and suspended/not suspended workspace activation status should be tested', () => { expect(testCases.length).toEqual( diff --git a/packages/twenty-front/src/hooks/usePageChangeEffectNavigateLocation.ts b/packages/twenty-front/src/hooks/usePageChangeEffectNavigateLocation.ts index c27c48935f..dbd693ea9f 100644 --- a/packages/twenty-front/src/hooks/usePageChangeEffectNavigateLocation.ts +++ b/packages/twenty-front/src/hooks/usePageChangeEffectNavigateLocation.ts @@ -1,5 +1,6 @@ import { verifyEmailRedirectPathState } from '@/app/states/verifyEmailRedirectPathState'; import { useIsLogged } from '@/auth/hooks/useIsLogged'; +import { calendarBookingPageIdState } from '@/client-config/states/calendarBookingPageIdState'; import { useIsCurrentLocationOnAWorkspace } from '@/domain-manager/hooks/useIsCurrentLocationOnAWorkspace'; import { useDefaultHomePagePath } from '@/navigation/hooks/useDefaultHomePagePath'; import { objectMetadataItemsState } from '@/object-metadata/states/objectMetadataItemsState'; @@ -22,6 +23,7 @@ export const usePageChangeEffectNavigateLocation = () => { ); const { defaultHomePagePath } = useDefaultHomePagePath(); const location = useLocation(); + const calendarBookingPageId = useRecoilValue(calendarBookingPageIdState); const someMatchingLocationOf = (appPaths: AppPath[]): boolean => appPaths.some((appPath) => isMatchingLocation(location, appPath)); @@ -123,6 +125,9 @@ export const usePageChangeEffectNavigateLocation = () => { onboardingStatus === OnboardingStatus.BOOK_ONBOARDING && !someMatchingLocationOf([AppPath.BookCallDecision, AppPath.BookCall]) ) { + if (!isDefined(calendarBookingPageId)) { + return defaultHomePagePath; + } return AppPath.BookCallDecision; } diff --git a/packages/twenty-server/src/engine/core-modules/client-config/services/client-config.service.ts b/packages/twenty-server/src/engine/core-modules/client-config/services/client-config.service.ts index c82e9bf7e4..d7eabe096a 100644 --- a/packages/twenty-server/src/engine/core-modules/client-config/services/client-config.service.ts +++ b/packages/twenty-server/src/engine/core-modules/client-config/services/client-config.service.ts @@ -1,5 +1,7 @@ import { Injectable } from '@nestjs/common'; +import { isNonEmptyString } from '@sniptt/guards'; + import { NodeEnvironment } from 'src/engine/core-modules/twenty-config/interfaces/node-environment.interface'; import { SupportDriver } from 'src/engine/core-modules/twenty-config/interfaces/support.interface'; @@ -28,6 +30,9 @@ export class ClientConfigService { async getClientConfig(): Promise { const captchaProvider = this.twentyConfigService.get('CAPTCHA_DRIVER'); const supportDriver = this.twentyConfigService.get('SUPPORT_DRIVER'); + const calendarBookingPageId = this.twentyConfigService.get( + 'CALENDAR_BOOKING_PAGE_ID', + ); const availableModels = this.aiModelRegistryService.getAvailableModels(); @@ -153,9 +158,9 @@ export class ClientConfigService { isImapSmtpCaldavEnabled: this.twentyConfigService.get( 'IS_IMAP_SMTP_CALDAV_ENABLED', ), - calendarBookingPageId: this.twentyConfigService.get( - 'CALENDAR_BOOKING_PAGE_ID', - ), + calendarBookingPageId: isNonEmptyString(calendarBookingPageId) + ? calendarBookingPageId + : undefined, }; return clientConfig; diff --git a/packages/twenty-server/src/engine/core-modules/onboarding/onboarding.service.ts b/packages/twenty-server/src/engine/core-modules/onboarding/onboarding.service.ts index eb13105a7d..89b9184b6f 100644 --- a/packages/twenty-server/src/engine/core-modules/onboarding/onboarding.service.ts +++ b/packages/twenty-server/src/engine/core-modules/onboarding/onboarding.service.ts @@ -1,9 +1,12 @@ import { Injectable } from '@nestjs/common'; +import { isNonEmptyString } from '@sniptt/guards'; +import { isDefined } from 'twenty-shared/utils'; import { WorkspaceActivationStatus } from 'twenty-shared/workspace'; import { BillingService } from 'src/engine/core-modules/billing/services/billing.service'; import { OnboardingStatus } from 'src/engine/core-modules/onboarding/enums/onboarding-status.enum'; +import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; import { UserVarsService } from 'src/engine/core-modules/user/user-vars/services/user-vars.service'; import { type User } from 'src/engine/core-modules/user/user.entity'; import { type Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; @@ -27,6 +30,7 @@ export class OnboardingService { constructor( private readonly billingService: BillingService, private readonly userVarsService: UserVarsService, + private readonly twentyConfigService: TwentyConfigService, ) {} private isWorkspaceActivationPending(workspace: Workspace) { @@ -81,6 +85,22 @@ export class OnboardingService { } if (isBookOnboardingPending) { + const calendarBookingPageId = this.twentyConfigService.get( + 'CALENDAR_BOOKING_PAGE_ID', + ); + const isBookingConfigured = + isDefined(calendarBookingPageId) && + isNonEmptyString(calendarBookingPageId); + + if (!isBookingConfigured) { + await this.userVarsService.delete({ + workspaceId: workspace.id, + key: OnboardingStepKeys.ONBOARDING_BOOK_ONBOARDING_PENDING, + }); + + return OnboardingStatus.COMPLETED; + } + return OnboardingStatus.BOOK_ONBOARDING; } @@ -171,7 +191,15 @@ export class OnboardingService { workspaceId: string; value: boolean; }) { - if (!value) { + const calendarBookingPageId = this.twentyConfigService.get( + 'CALENDAR_BOOKING_PAGE_ID', + ); + + const isBookingConfigured = + isDefined(calendarBookingPageId) && + isNonEmptyString(calendarBookingPageId); + + if (!value || !isBookingConfigured) { await this.userVarsService.delete({ workspaceId, key: OnboardingStepKeys.ONBOARDING_BOOK_ONBOARDING_PENDING,