Skip to content
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.

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

Display a generic fallback component when initial config load fails #8588

Merged
merged 7 commits into from
Nov 22, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import styled from '@emotion/styled';
import { isNonEmptyString } from '@sniptt/guards';
import { ChangeEvent, useRef } from 'react';

import { AppThemeProvider } from '@/ui/theme/components/AppThemeProvider';
import { Button } from 'twenty-ui';
import { isDefined } from '~/utils/isDefined';
import { isUndefinedOrNull } from '~/utils/isUndefinedOrNull';
Expand Down Expand Up @@ -90,33 +89,26 @@ export const FileBlock = createReactBlockSpec(

if (isNonEmptyString(block.props.url)) {
return (
<AppThemeProvider>
<StyledFileLine>
<AttachmentIcon
attachmentType={block.props.fileType as AttachmentType}
></AttachmentIcon>
<StyledLink href={block.props.url} target="__blank">
{block.props.name}
</StyledLink>
</StyledFileLine>
</AppThemeProvider>
<StyledFileLine>
<AttachmentIcon
attachmentType={block.props.fileType as AttachmentType}
></AttachmentIcon>
<StyledLink href={block.props.url} target="__blank">
{block.props.name}
</StyledLink>
</StyledFileLine>
);
}

return (
<AppThemeProvider>
<StyledUploadFileContainer>
<StyledFileInput
ref={inputFileRef}
onChange={handleFileChange}
type="file"
/>
<Button
onClick={handleUploadFileClick}
title="Upload File"
></Button>
</StyledUploadFileContainer>
</AppThemeProvider>
<StyledUploadFileContainer>
<StyledFileInput
ref={inputFileRef}
onChange={handleFileChange}
type="file"
/>
<Button onClick={handleUploadFileClick} title="Upload File"></Button>
</StyledUploadFileContainer>
);
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import { PrefetchDataProvider } from '@/prefetch/components/PrefetchDataProvider
import { DialogManager } from '@/ui/feedback/dialog-manager/components/DialogManager';
import { DialogManagerScope } from '@/ui/feedback/dialog-manager/scopes/DialogManagerScope';
import { SnackBarProvider } from '@/ui/feedback/snack-bar-manager/components/SnackBarProvider';
import { AppThemeProvider } from '@/ui/theme/components/AppThemeProvider';
import { UserThemeProviderEffect } from '@/ui/theme/components/AppThemeProvider';
import { BaseThemeProvider } from '@/ui/theme/components/BaseThemeProvider';
import { PageTitle } from '@/ui/utilities/page-title/components/PageTitle';
import { UserProvider } from '@/users/components/UserProvider';
import { UserProviderEffect } from '@/users/components/UserProviderEffect';
Expand All @@ -27,17 +28,18 @@ export const AppRouterProviders = () => {

return (
<ApolloProvider>
<ClientConfigProviderEffect />
<ClientConfigProvider>
<ChromeExtensionSidecarEffect />
<ChromeExtensionSidecarProvider>
<UserProviderEffect />
<UserProvider>
<AuthProvider>
<ApolloMetadataClientProvider>
<ObjectMetadataItemsProvider>
<PrefetchDataProvider>
<AppThemeProvider>
<BaseThemeProvider>
<ClientConfigProviderEffect />
<ClientConfigProvider>
<ChromeExtensionSidecarEffect />
<ChromeExtensionSidecarProvider>
<UserProviderEffect />
<UserProvider>
<AuthProvider>
<ApolloMetadataClientProvider>
<ObjectMetadataItemsProvider>
<PrefetchDataProvider>
<UserThemeProviderEffect />
<SnackBarProvider>
<DialogManagerScope dialogManagerScopeId="dialog-manager">
<DialogManager>
Expand All @@ -50,15 +52,15 @@ export const AppRouterProviders = () => {
</DialogManager>
</DialogManagerScope>
</SnackBarProvider>
</AppThemeProvider>
</PrefetchDataProvider>
<PageChangeEffect />
</ObjectMetadataItemsProvider>
</ApolloMetadataClientProvider>
</AuthProvider>
</UserProvider>
</ChromeExtensionSidecarProvider>
</ClientConfigProvider>
</PrefetchDataProvider>
<PageChangeEffect />
</ObjectMetadataItemsProvider>
</ApolloMetadataClientProvider>
</AuthProvider>
</UserProvider>
</ChromeExtensionSidecarProvider>
</ClientConfigProvider>
</BaseThemeProvider>
</ApolloProvider>
);
};
8 changes: 4 additions & 4 deletions packages/twenty-front/src/modules/auth/hooks/useAuth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { workspacesState } from '@/auth/states/workspaces';
import { authProvidersState } from '@/client-config/states/authProvidersState';
import { billingState } from '@/client-config/states/billingState';
import { captchaProviderState } from '@/client-config/states/captchaProviderState';
import { isClientConfigLoadedState } from '@/client-config/states/isClientConfigLoadedState';
import { clientConfigApiStatusState } from '@/client-config/states/clientConfigApiStatusState';
import { isDebugModeState } from '@/client-config/states/isDebugModeState';
import { isSignInPrefilledState } from '@/client-config/states/isSignInPrefilledState';
import { supportChatState } from '@/client-config/states/supportChatState';
Expand Down Expand Up @@ -229,8 +229,8 @@ export const useAuth = () => {
const captchaProvider = snapshot
.getLoadable(captchaProviderState)
.getValue();
const isClientConfigLoaded = snapshot
.getLoadable(isClientConfigLoadedState)
const clientConfigApiStatus = snapshot
.getLoadable(clientConfigApiStatusState)
.getValue();
const isCurrentUserLoaded = snapshot
.getLoadable(isCurrentUserLoadedState)
Expand All @@ -244,7 +244,7 @@ export const useAuth = () => {
set(supportChatState, supportChat);
set(isDebugModeState, isDebugMode);
set(captchaProviderState, captchaProvider);
set(isClientConfigLoadedState, isClientConfigLoaded);
set(clientConfigApiStatusState, clientConfigApiStatus);
set(isCurrentUserLoadedState, isCurrentUserLoaded);
return undefined;
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,21 @@
import { useRecoilValue } from 'recoil';

import { isClientConfigLoadedState } from '@/client-config/states/isClientConfigLoadedState';
import { clientConfigApiStatusState } from '@/client-config/states/clientConfigApiStatusState';
import { ClientConfigError } from '@/error-handler/components/ClientConfigError';

export const ClientConfigProvider: React.FC<React.PropsWithChildren> = ({
children,
}) => {
const isClientConfigLoaded = useRecoilValue(isClientConfigLoadedState);
const { isLoaded, isErrored, error } = useRecoilValue(
clientConfigApiStatusState,
);

return isClientConfigLoaded ? <>{children}</> : <></>;
// TODO: Implement a better loading strategy
if (!isLoaded) return null;

return isErrored && error instanceof Error ? (
<ClientConfigError error={error} />
) : (
children
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import { authProvidersState } from '@/client-config/states/authProvidersState';
import { billingState } from '@/client-config/states/billingState';
import { captchaProviderState } from '@/client-config/states/captchaProviderState';
import { chromeExtensionIdState } from '@/client-config/states/chromeExtensionIdState';
import { clientConfigApiStatusState } from '@/client-config/states/clientConfigApiStatusState';
import { isAnalyticsEnabledState } from '@/client-config/states/isAnalyticsEnabledState';
import { isClientConfigLoadedState } from '@/client-config/states/isClientConfigLoadedState';
import { isDebugModeState } from '@/client-config/states/isDebugModeState';
import { isSignInPrefilledState } from '@/client-config/states/isSignInPrefilledState';
import { isSignUpDisabledState } from '@/client-config/states/isSignUpDisabledState';
Expand All @@ -27,8 +27,8 @@ export const ClientConfigProviderEffect = () => {
const setSupportChat = useSetRecoilState(supportChatState);

const setSentryConfig = useSetRecoilState(sentryConfigState);
const [isClientConfigLoaded, setIsClientConfigLoaded] = useRecoilState(
isClientConfigLoadedState,
const [clientConfigApiStatus, setClientConfigApiStatus] = useRecoilState(
clientConfigApiStatusState,
);

const setCaptchaProvider = useSetRecoilState(captchaProviderState);
Expand All @@ -37,42 +37,64 @@ export const ClientConfigProviderEffect = () => {

const setApiConfig = useSetRecoilState(apiConfigState);

const { data, loading } = useGetClientConfigQuery({
skip: isClientConfigLoaded,
const { data, loading, error } = useGetClientConfigQuery({
skip: clientConfigApiStatus.isLoaded,
});

useEffect(() => {
if (!loading && isDefined(data?.clientConfig)) {
setIsClientConfigLoaded(true);
setAuthProviders({
google: data?.clientConfig.authProviders.google,
microsoft: data?.clientConfig.authProviders.microsoft,
password: data?.clientConfig.authProviders.password,
magicLink: false,
sso: data?.clientConfig.authProviders.sso,
});
setIsDebugMode(data?.clientConfig.debugMode);
setIsAnalyticsEnabled(data?.clientConfig.analyticsEnabled);
setIsSignInPrefilled(data?.clientConfig.signInPrefilled);
setIsSignUpDisabled(data?.clientConfig.signUpDisabled);

setBilling(data?.clientConfig.billing);
setSupportChat(data?.clientConfig.support);

setSentryConfig({
dsn: data?.clientConfig?.sentry?.dsn,
release: data?.clientConfig?.sentry?.release,
environment: data?.clientConfig?.sentry?.environment,
});

setCaptchaProvider({
provider: data?.clientConfig?.captcha?.provider,
siteKey: data?.clientConfig?.captcha?.siteKey,
});

setChromeExtensionId(data?.clientConfig?.chromeExtensionId);
setApiConfig(data?.clientConfig?.api);
if (loading) return;
setClientConfigApiStatus((currentStatus) => ({
...currentStatus,
isLoaded: true,
}));
Comment on lines +46 to +49
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Setting isLoaded:true before checking for errors could cause race conditions. Move this after error checks.


if (error instanceof Error) {
setClientConfigApiStatus((currentStatus) => ({
...currentStatus,
isErrored: true,
error,
}));
return;
}

if (!isDefined(data?.clientConfig)) {
return;
}
Comment on lines +60 to 62
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Should set isErrored:true here since missing clientConfig is an error state


setClientConfigApiStatus((currentStatus) => ({
...currentStatus,
isErrored: false,
error: undefined,
}));

setAuthProviders({
google: data?.clientConfig.authProviders.google,
microsoft: data?.clientConfig.authProviders.microsoft,
password: data?.clientConfig.authProviders.password,
magicLink: false,
sso: data?.clientConfig.authProviders.sso,
});
Comment on lines +70 to +76
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Optional chaining is inconsistent - using ?. for clientConfig but not for nested authProviders properties

setIsDebugMode(data?.clientConfig.debugMode);
setIsAnalyticsEnabled(data?.clientConfig.analyticsEnabled);
setIsSignInPrefilled(data?.clientConfig.signInPrefilled);
setIsSignUpDisabled(data?.clientConfig.signUpDisabled);

setBilling(data?.clientConfig.billing);
setSupportChat(data?.clientConfig.support);

setSentryConfig({
dsn: data?.clientConfig?.sentry?.dsn,
release: data?.clientConfig?.sentry?.release,
environment: data?.clientConfig?.sentry?.environment,
});

setCaptchaProvider({
provider: data?.clientConfig?.captcha?.provider,
siteKey: data?.clientConfig?.captcha?.siteKey,
});

setChromeExtensionId(data?.clientConfig?.chromeExtensionId);
setApiConfig(data?.clientConfig?.api);
}, [
data,
setAuthProviders,
Expand All @@ -83,11 +105,12 @@ export const ClientConfigProviderEffect = () => {
setBilling,
setSentryConfig,
loading,
setIsClientConfigLoaded,
setClientConfigApiStatus,
setCaptchaProvider,
setChromeExtensionId,
setApiConfig,
setIsAnalyticsEnabled,
error,
]);

return <></>;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { createState } from 'twenty-ui';

type ClientConfigApiStatus = {
isLoaded: boolean;
isErrored: boolean;
error?: Error;
};

export const clientConfigApiStatusState = createState<ClientConfigApiStatus>({
key: 'clientConfigApiStatus',
defaultValue: { isLoaded: false, isErrored: false, error: undefined },
});

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,16 @@ export const AppErrorBoundary = ({ children }: { children: ReactNode }) => {
});
};

// TODO: Implement a better reset strategy, hard reload for now
const handleReset = () => {
window.location.reload();
};

return (
<ErrorBoundary
FallbackComponent={GenericErrorFallback}
onError={handleError}
onReset={handleReset}
>
{children}
</ErrorBoundary>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import styled from '@emotion/styled';
import { MOBILE_VIEWPORT } from 'twenty-ui';
import { GenericErrorFallback } from './GenericErrorFallback';

const StyledContainer = styled.div`
background: ${({ theme }) => theme.background.noisy};
box-sizing: border-box;
display: flex;
height: 100dvh;
width: 100%;
padding-top: ${({ theme }) => theme.spacing(3)};
padding-left: ${({ theme }) => theme.spacing(3)};
padding-bottom: 0;
@media (max-width: ${MOBILE_VIEWPORT}px) {
padding-left: 0;
padding-bottom: ${({ theme }) => theme.spacing(3)};
}
`;

type ClientConfigErrorProps = {
error: Error;
};

export const ClientConfigError = ({ error }: ClientConfigErrorProps) => {
// TODO: Implement a better loading strategy
const handleReset = () => {
window.location.reload();
};

return (
<StyledContainer>
<GenericErrorFallback
error={error}
resetErrorBoundary={handleReset}
title="Unable to Reach Back-end"
hidePageHeader
/>
</StyledContainer>
);
};
Loading
Loading