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

[QBD] Handle errors gracefully if the setup link cannot be obtained #51850

Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
6 changes: 6 additions & 0 deletions src/languages/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2477,6 +2477,10 @@ const translations = {
setupPage: {
title: 'Open this link to connect',
body: 'To complete setup, open the following link on the computer where QuickBooks Desktop is running.',
setupErrorTitle: 'Something went wrong',
setupErrorBody1: "The QuickBooks Desktop connection isn't working at the moment. Please try again later or",
setupErrorBody2: 'if the problem persists.',
setupErrorBodyContactConcierge: 'reach out to Concierge',
},
importDescription: 'Choose which coding configurations to import from QuickBooks Desktop to Expensify.',
classes: 'Classes',
Expand Down Expand Up @@ -3655,6 +3659,8 @@ const translations = {
return "Can't connect to Xero.";
case CONST.POLICY.CONNECTIONS.NAME.NETSUITE:
return "Can't connect to NetSuite.";
case CONST.POLICY.CONNECTIONS.NAME.QBD:
return "Can't connect to QuickBooks Desktop.";
default: {
return "Can't connect to integration.";
}
Expand Down
6 changes: 6 additions & 0 deletions src/languages/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2502,6 +2502,10 @@ const translations = {
setupPage: {
title: 'Abre este enlace para conectar',
body: 'Para completar la configuración, abre el siguiente enlace en la computadora donde se está ejecutando QuickBooks Desktop.',
setupErrorTitle: '¡Ups! Ha ocurrido un error',
setupErrorBody1: 'La conexión con QuickBooks Desktop no está funcionando en este momento. Por favor, inténtalo de nuevo más tarde o',
setupErrorBody2: 'si el problema persiste.',
setupErrorBodyContactConcierge: 'contacta a Concierge',
},
importDescription: 'Elige que configuraciónes de codificación son importadas desde QuickBooks Desktop a Expensify.',
classes: 'Clases',
Expand Down Expand Up @@ -3660,6 +3664,8 @@ const translations = {
return 'No se puede conectar a Xero.';
case CONST.POLICY.CONNECTIONS.NAME.NETSUITE:
return 'No se puede conectar a NetSuite.';
case CONST.POLICY.CONNECTIONS.NAME.QBD:
return 'No se puede conectar a QuickBooks Desktop.';
default: {
return 'No se ha podido conectar a la integración.';
}
Expand Down
15 changes: 15 additions & 0 deletions src/libs/actions/connections/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,20 @@ function isConnectionUnverified(policy: OnyxEntry<Policy>, connectionName: Polic
return !(policy?.connections?.[connectionName]?.lastSync?.isConnected ?? true);
}

function setConnectionError(policyID: string, connectionName: PolicyConnectionName, errorMessage?: string) {
Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {
connections: {
[connectionName]: {
lastSync: {
isSuccessful: false,
Copy link
Contributor

@ZhenjaHorbach ZhenjaHorbach Nov 5, 2024

Choose a reason for hiding this comment

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

I think we can add isConnected: false or changeisSuccessful toisConnected here to not show options

function isConnectionUnverified(policy: OnyxEntry<Policy>, connectionName: PolicyConnectionName): boolean {
// A verified connection is one that has been successfully synced at least once
// We'll always err on the side of considering a connection as verified connected even if we can't find a lastSync property saying as such
// i.e. this is a property that is explicitly set to false, not just missing
if (connectionName === CONST.POLICY.CONNECTIONS.NAME.NETSUITE) {
return !(policy?.connections?.[CONST.POLICY.CONNECTIONS.NAME.NETSUITE]?.verified ?? true);
}
// If the connection has no lastSync property, we'll consider it unverified
if (isEmptyObject(policy?.connections?.[connectionName]?.lastSync)) {
return true;
}
return !(policy?.connections?.[connectionName]?.lastSync?.isConnected ?? true);
}

const shouldHideConfigurationOptions = isConnectionUnverified(policy, connectedIntegration);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ZhenjaHorbach do you mean BE (Pusher event) sent isConnected=true in your case?

Copy link
Contributor

@ZhenjaHorbach ZhenjaHorbach Nov 6, 2024

Choose a reason for hiding this comment

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

No
When we have an error
We don't receive anything

But in case an error we save in Onyx this using setConnectionError

lastSync: {
    "isSuccessful": false,
    "errorDate": "2024-11-06T06:20:52.757Z",
    "errorMessage": "Something went wrong"
}

As result we return false here

function isConnectionUnverified(policy: OnyxEntry<Policy>, connectionName: PolicyConnectionName): boolean {
// A verified connection is one that has been successfully synced at least once
// We'll always err on the side of considering a connection as verified connected even if we can't find a lastSync property saying as such
// i.e. this is a property that is explicitly set to false, not just missing
if (connectionName === CONST.POLICY.CONNECTIONS.NAME.NETSUITE) {
return !(policy?.connections?.[CONST.POLICY.CONNECTIONS.NAME.NETSUITE]?.verified ?? true);
}
// If the connection has no lastSync property, we'll consider it unverified
if (isEmptyObject(policy?.connections?.[connectionName]?.lastSync)) {
return true;
}
return !(policy?.connections?.[connectionName]?.lastSync?.isConnected ?? true);
}

and show options

const shouldHideConfigurationOptions = isConnectionUnverified(policy, connectedIntegration);

...(isEmptyObject(policy?.connections) || shouldHideConfigurationOptions ? [] : configurationOptions),

For example when we have correct response with setupLink we save "isConnected": false in Onyx and don't show options because shouldHideConfigurationOptions === true

lastSync: {
    "errorDate": "",
    "errorMessage": "",
    "isAuthenticationError": false,
    "isConnected": false,
    "isSuccessful": false,
    "source": "",
    "successfulDate": ""
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah I see. Good catch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated here 1dc585e

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks !

errorDate: new Date().toISOString(),
errorMessage,
},
},
},
});
}

function copyExistingPolicyConnection(connectedPolicyID: string, targetPolicyID: string, connectionName: ConnectionName) {
let stageInProgress;
switch (connectionName) {
Expand Down Expand Up @@ -389,4 +403,5 @@ export {
isConnectionUnverified,
isConnectionInProgress,
hasSynchronizationErrorMessage,
setConnectionError,
};
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,22 @@ import CopyTextToClipboard from '@components/CopyTextToClipboard';
import FixedFooter from '@components/FixedFooter';
import FullScreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import Icon from '@components/Icon';
import * as Illustrations from '@components/Icon/Illustrations';
import ImageSVG from '@components/ImageSVG';
import ScreenWrapper from '@components/ScreenWrapper';
import Text from '@components/Text';
import TextLink from '@components/TextLink';
import useEnvironment from '@hooks/useEnvironment';
import useLocalize from '@hooks/useLocalize';
import useNetwork from '@hooks/useNetwork';
import useThemeStyles from '@hooks/useThemeStyles';
import {setConnectionError} from '@libs/actions/connections';
import * as QuickbooksDesktop from '@libs/actions/connections/QuickbooksDesktop';
import Navigation from '@libs/Navigation/Navigation';
import type {SettingsNavigatorParamList} from '@libs/Navigation/types';
import * as PolicyAction from '@userActions/Policy/Policy';
import CONST from '@src/CONST';
import ROUTES from '@src/ROUTES';
import type SCREENS from '@src/SCREENS';

Expand All @@ -26,20 +32,31 @@ type RequireQuickBooksDesktopModalProps = StackScreenProps<SettingsNavigatorPara
function RequireQuickBooksDesktopModal({route}: RequireQuickBooksDesktopModalProps) {
const {translate} = useLocalize();
const styles = useThemeStyles();
const {environmentURL} = useEnvironment();
const policyID: string = route.params.policyID;
const [isLoading, setIsLoading] = useState(true);
const [hasError, setHasError] = useState(false);
const [codatSetupLink, setCodatSetupLink] = useState<string>('');

const ContentWrapper = codatSetupLink ? ({children}: React.PropsWithChildren) => children : FullPageOfflineBlockingView;

const fetchSetupLink = useCallback(() => {
setIsLoading(true);
setHasError(false);
// eslint-disable-next-line rulesdir/no-thenable-actions-in-views
QuickbooksDesktop.getQuickbooksDesktopCodatSetupLink(policyID).then((response) => {
setCodatSetupLink(String(response?.setupUrl ?? ''));
if (response?.jsonCode) {
if (response.jsonCode === CONST.JSON_CODE.SUCCESS) {
setCodatSetupLink(String(response?.setupUrl ?? ''));
} else {
setConnectionError(policyID, CONST.POLICY.CONNECTIONS.NAME.QBD, translate('workspace.qbd.setupPage.setupErrorTitle'));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @lakchote I'm expecting that BE also sends connection error via Pusher event in those cases. So we can display error message like this (same thing we used to work here)

Screenshot 2024-11-04 at 17 01 20

Can you confirm if it's correct? IF yes, I will remove those optimistic set error message here (avoid flicker)

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @hoangzinh, we do not send via Pusher this error, it is returned instead in the response's body on the PHP side.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then, I guess I have to set optimistic error here

setHasError(true);
}
}

setIsLoading(false);
});
}, [policyID]);
}, [policyID, translate]);

useEffect(() => {
// Since QBD doesn't support Taxes, we should disable them from the LHN when connecting to QBD
Expand All @@ -59,6 +76,9 @@ function RequireQuickBooksDesktopModal({route}: RequireQuickBooksDesktopModalPro
},
});

const shouldShowLoading = isLoading || (!codatSetupLink && !hasError);
const shouldShowError = hasError;

return (
<ScreenWrapper
shouldEnablePickerAvoiding={false}
Expand All @@ -71,9 +91,28 @@ function RequireQuickBooksDesktopModal({route}: RequireQuickBooksDesktopModalPro
onBackButtonPress={() => Navigation.dismissModal()}
/>
<ContentWrapper>
{isLoading || !codatSetupLink ? (
<FullScreenLoadingIndicator style={[styles.flex1, styles.pRelative]} />
) : (
{shouldShowLoading && <FullScreenLoadingIndicator style={[styles.flex1, styles.pRelative]} />}
{shouldShowError && (
<View style={[styles.flex1, styles.justifyContentCenter, styles.alignItemsCenter, styles.ph5, styles.mb9]}>
<Icon
src={Illustrations.BrokenMagnifyingGlass}
width={116}
height={168}
/>
<Text style={[styles.textHeadlineLineHeightXXL, styles.mt3]}>{translate('workspace.qbd.setupPage.setupErrorTitle')}</Text>
<Text style={[styles.textSupporting, styles.ph5, styles.mv3, styles.textAlignCenter]}>
{translate('workspace.qbd.setupPage.setupErrorBody1')}{' '}
<TextLink
href={`${environmentURL}/${ROUTES.CONCIERGE}`}
style={styles.link}
>
{translate('workspace.qbd.setupPage.setupErrorBodyContactConcierge')}
</TextLink>{' '}
{translate('workspace.qbd.setupPage.setupErrorBody2')}
</Text>
</View>
)}
{!shouldShowLoading && !shouldShowError && (
<View style={[styles.flex1, styles.ph5]}>
<View style={[styles.alignSelfCenter, styles.computerIllustrationContainer, styles.pv6]}>
<ImageSVG src={Computer} />
Expand Down
Loading