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
This commit is contained in:
@@ -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(
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
+8
-3
@@ -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<ClientConfig> {
|
||||
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;
|
||||
|
||||
@@ -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<OnboardingKeyValueTypeMap>,
|
||||
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,
|
||||
|
||||
Reference in New Issue
Block a user