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.

Sign up for GitHub

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

Apply correct error pattern in Xero integration (return to previous value on error) #46898

Merged
merged 7 commits into from
Aug 12, 2024
Merged

Apply correct error pattern in Xero integration (return to previous value on error) #46898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
876f2a9
update advanced page
war-in Aug 6, 2024
f8074f3
update export page
war-in Aug 6, 2024
dc881c6
update import page
war-in Aug 6, 2024
44dd858
update organization selection page
war-in Aug 6, 2024
a24753a
do not mark first account as selected
war-in Aug 6, 2024
190de87
Merge branch 'refs/heads/main' into war-in/return-to-previous-value-xero
war-in Aug 7, 2024
a881d6a
add nullable
war-in Aug 12, 2024
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
2 changes: 1 addition & 1 deletion src/components/SelectionScreen.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ function SelectionScreen<T = string>({
initiallyFocusedOptionKey={initiallyFocusedOptionKey}
listEmptyContent={listEmptyContent}
listFooterContent={listFooterContent}
sectionListStyle={[styles.flexGrow0]}
sectionListStyle={!!sections.length && [styles.flexGrow0]}
shouldSingleExecuteRowSelect={shouldSingleExecuteRowSelect}
>
<ErrorMessageRow
Expand Down
1 change: 1 addition & 0 deletions src/languages/en.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2244,6 +2244,7 @@ export default {
customersDescription: 'Choose whether to re-bill customers in Expensify. Your Xero customer contacts can be tagged to expenses, and will export to Xero as a sales invoice.',
taxesDescription: 'Choose how to handle Xero taxes in Expensify.',
notImported: 'Not imported',
notConfigured: 'Not configured',
trackingCategoriesOptions: {
default: 'Xero contact default',
tag: 'Tags',
Expand Down
1 change: 1 addition & 0 deletions src/languages/es.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2286,6 +2286,7 @@ export default {
'Elige si quieres volver a facturar a los clientes en Expensify. Tus contactos de clientes de Xero se pueden etiquetar como gastos, y se exportarán a Xero como una factura de venta.',
taxesDescription: 'Elige cómo gestionar los impuestos de Xero en Expensify.',
notImported: 'No importado',
notConfigured: 'No configurado',
trackingCategoriesOptions: {
default: 'Contacto de Xero por defecto',
tag: 'Etiquetas',
Expand Down
9 changes: 4 additions & 5 deletions src/libs/PolicyUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,15 +536,14 @@ function getCurrentXeroOrganizationName(policy: Policy | undefined): string | un
return findCurrentXeroOrganization(getXeroTenants(policy), policy?.connections?.xero?.config?.tenantID)?.name;
}

function getXeroBankAccountsWithDefaultSelect(policy: Policy | undefined, selectedBankAccountId: string | undefined): SelectorType[] {
function getXeroBankAccounts(policy: Policy | undefined, selectedBankAccountId: string | undefined): SelectorType[] {
const bankAccounts = policy?.connections?.xero?.data?.bankAccounts ?? [];
const isMatchFound = bankAccounts?.some(({id}) => id === selectedBankAccountId);

return (bankAccounts ?? []).map(({id, name}, index) => ({
return (bankAccounts ?? []).map(({id, name}) => ({
value: id,
text: name,
keyForList: id,
isSelected: isMatchFound ? selectedBankAccountId === id : index === 0,
isSelected: selectedBankAccountId === id,
}));
}

Expand Down Expand Up @@ -954,7 +953,7 @@ export {
getXeroTenants,
findCurrentXeroOrganization,
getCurrentXeroOrganizationName,
getXeroBankAccountsWithDefaultSelect,
getXeroBankAccounts,
findSelectedVendorWithDefaultSelect,
findSelectedBankAccountWithDefaultSelect,
findSelectedInvoiceItemWithDefaultSelect,
Expand Down
8 changes: 6 additions & 2 deletions src/libs/actions/connections/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ import type Policy from '@src/types/onyx/Policy';

type ConnectionNameExceptNetSuite = Exclude<ConnectionName, typeof CONST.POLICY.CONNECTIONS.NAME.NETSUITE>;

type Nullable<T> = {
[P in keyof T]: T[P] | null;
};

function removePolicyConnection(policyID: string, connectionName: PolicyConnectionName) {
const optimisticData: OnyxUpdate[] = [
{
Expand Down Expand Up @@ -73,6 +77,7 @@ function updatePolicyXeroConnectionConfig<TConnectionName extends ConnectionName
connectionName: TConnectionName,
settingName: TSettingName,
settingValue: Partial<Connections[TConnectionName]['config'][TSettingName]>,
oldSettingValue?: Nullable<Partial<Connections[TConnectionName]['config'][TSettingName]>>,
) {
const optimisticData: OnyxUpdate[] = [
{
Expand Down Expand Up @@ -100,7 +105,7 @@ function updatePolicyXeroConnectionConfig<TConnectionName extends ConnectionName
connections: {
[connectionName]: {
config: {
[settingName]: settingValue ?? null,
[settingName]: oldSettingValue ?? null,
pendingFields: createPendingFields(settingName, settingValue, null),
errorFields: createErrorFields(settingName, settingValue, ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('common.genericErrorMessage')),
},
Expand All @@ -118,7 +123,6 @@ function updatePolicyXeroConnectionConfig<TConnectionName extends ConnectionName
connections: {
[connectionName]: {
config: {
[settingName]: settingValue ?? null,
pendingFields: createPendingFields(settingName, settingValue, null),
errorFields: createErrorFields(settingName, settingValue, null),
},
Expand Down
7 changes: 3 additions & 4 deletions src/pages/workspace/accounting/xero/XeroMapTrackingCategoryConfigurationPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ type RouteParams = {
categoryName?: string;
};

const TRACKING_CATEGORIES_KEY = 'trackingCategory_';

function XeroMapTrackingCategoryConfigurationPage({policy}: WithPolicyProps) {
const {translate} = useLocalize();
const route = useRoute();
Expand All @@ -37,7 +35,7 @@ function XeroMapTrackingCategoryConfigurationPage({policy}: WithPolicyProps) {
const {mappings} = policy?.connections?.xero?.config ?? {};

const currentTrackingCategory = trackingCategories?.find((category) => category.id === categoryId);
const currentTrackingCategoryValue = currentTrackingCategory ? mappings?.[`${TRACKING_CATEGORIES_KEY}${currentTrackingCategory.id}`] ?? '' : '';
const currentTrackingCategoryValue = currentTrackingCategory ? mappings?.[`${CONST.XERO_CONFIG.TRACKING_CATEGORY_PREFIX}${currentTrackingCategory.id}`] ?? '' : '';

const optionsList = useMemo(
() =>
Expand Down Expand Up @@ -67,11 +65,12 @@ function XeroMapTrackingCategoryConfigurationPage({policy}: WithPolicyProps) {
CONST.POLICY.CONNECTIONS.NAME.XERO,
CONST.XERO_CONFIG.MAPPINGS,
categoryId ? {[`${CONST.XERO_CONFIG.TRACKING_CATEGORY_PREFIX}${categoryId}`]: option.value} : {},
categoryId ? {[`${CONST.XERO_CONFIG.TRACKING_CATEGORY_PREFIX}${categoryId}`]: currentTrackingCategoryValue} : {},
);
}
Navigation.goBack(ROUTES.POLICY_ACCOUNTING_XERO_TRACKING_CATEGORIES.getRoute(policyID));
},
[categoryId, categoryName, policyID],
[categoryId, categoryName, currentTrackingCategoryValue, policyID],
);

return (
Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/accounting/xero/XeroOrganizationConfigurationPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ function XeroOrganizationConfigurationPage({
return;
}

updatePolicyXeroConnectionConfig(policyID, CONST.POLICY.CONNECTIONS.NAME.XERO, CONST.XERO_CONFIG.TENANT_ID, keyForList);
updatePolicyXeroConnectionConfig(policyID, CONST.POLICY.CONNECTIONS.NAME.XERO, CONST.XERO_CONFIG.TENANT_ID, keyForList, xeroConfig?.tenantID);
Navigation.goBack();
};

Expand Down
10 changes: 9 additions & 1 deletion src/pages/workspace/accounting/xero/XeroTaxesConfigurationPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,15 @@ function XeroTaxesConfigurationPage({policy}: WithPolicyProps) {
title={translate('workspace.accounting.import')}
switchAccessibilityLabel={translate('workspace.xero.customers')}
isActive={isSwitchOn}
onToggle={() => Connections.updatePolicyXeroConnectionConfig(policyID, CONST.POLICY.CONNECTIONS.NAME.XERO, CONST.XERO_CONFIG.IMPORT_TAX_RATES, !xeroConfig?.importTaxRates)}
onToggle={() =>
Connections.updatePolicyXeroConnectionConfig(
policyID,
CONST.POLICY.CONNECTIONS.NAME.XERO,
CONST.XERO_CONFIG.IMPORT_TAX_RATES,
!xeroConfig?.importTaxRates,
xeroConfig?.importTaxRates,
)
}
errors={ErrorUtils.getLatestErrorField(xeroConfig ?? {}, CONST.XERO_CONFIG.IMPORT_TAX_RATES)}
onCloseError={() => Policy.clearXeroErrorField(policyID, CONST.XERO_CONFIG.IMPORT_TAX_RATES)}
pendingAction={PolicyUtils.settingsPendingAction([CONST.XERO_CONFIG.IMPORT_TAX_RATES], xeroConfig?.pendingFields)}
Expand Down
1 change: 1 addition & 0 deletions src/pages/workspace/accounting/xero/XeroTrackingCategoryConfigurationPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ function XeroTrackingCategoryConfigurationPage({policy}: WithPolicyProps) {
CONST.POLICY.CONNECTIONS.NAME.XERO,
CONST.XERO_CONFIG.IMPORT_TRACKING_CATEGORIES,
!xeroConfig?.importTrackingCategories,
xeroConfig?.importTrackingCategories,
)
}
pendingAction={settingsPendingAction([CONST.XERO_CONFIG.IMPORT_TRACKING_CATEGORIES], xeroConfig?.pendingFields)}
Expand Down
28 changes: 20 additions & 8 deletions src/pages/workspace/accounting/xero/advanced/XeroAdvancedPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ function XeroAdvancedPage({policy}: WithPolicyConnectionsProps) {
const getSelectedAccountName = useMemo(
() => (accountID: string) => {
const selectedAccount = (bankAccounts ?? []).find((bank) => bank.id === accountID);
return selectedAccount?.name ?? bankAccounts?.[0]?.name ?? '';
return selectedAccount?.name ?? translate('workspace.xero.notConfigured');
},
[bankAccounts],
[bankAccounts, translate],
);

const selectedBankAccountName = getSelectedAccountName(invoiceCollectionsAccountID ?? '-1');
Expand Down Expand Up @@ -59,9 +59,15 @@ function XeroAdvancedPage({policy}: WithPolicyConnectionsProps) {
wrapperStyle={styles.mv3}
isActive={!!autoSync?.enabled}
onToggle={() =>
Connections.updatePolicyXeroConnectionConfig(policyID, CONST.POLICY.CONNECTIONS.NAME.XERO, CONST.XERO_CONFIG.AUTO_SYNC, {
enabled: !autoSync?.enabled,
})
Connections.updatePolicyXeroConnectionConfig(
policyID,
CONST.POLICY.CONNECTIONS.NAME.XERO,
CONST.XERO_CONFIG.AUTO_SYNC,
{
enabled: !autoSync?.enabled,
},
{enabled: autoSync?.enabled ?? null},
)
}
pendingAction={settingsPendingAction([CONST.XERO_CONFIG.ENABLED], pendingFields)}
errors={ErrorUtils.getLatestErrorField(xeroConfig ?? {}, CONST.XERO_CONFIG.ENABLED)}
Expand All @@ -76,9 +82,15 @@ function XeroAdvancedPage({policy}: WithPolicyConnectionsProps) {
wrapperStyle={styles.mv3}
isActive={!!sync?.syncReimbursedReports}
onToggle={() =>
Connections.updatePolicyXeroConnectionConfig(policyID, CONST.POLICY.CONNECTIONS.NAME.XERO, CONST.XERO_CONFIG.SYNC, {
syncReimbursedReports: !sync?.syncReimbursedReports,
})
Connections.updatePolicyXeroConnectionConfig(
policyID,
CONST.POLICY.CONNECTIONS.NAME.XERO,
CONST.XERO_CONFIG.SYNC,
{
syncReimbursedReports: !sync?.syncReimbursedReports,
},
{syncReimbursedReports: sync?.syncReimbursedReports ?? null},
)
}
pendingAction={settingsPendingAction([CONST.XERO_CONFIG.SYNC_REIMBURSED_REPORTS], pendingFields)}
errors={ErrorUtils.getLatestErrorField(xeroConfig ?? {}, CONST.XERO_CONFIG.SYNC_REIMBURSED_REPORTS)}
Expand Down
18 changes: 12 additions & 6 deletions src/pages/workspace/accounting/xero/advanced/XeroBillPaymentAccountSelectorPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import useThemeStyles from '@hooks/useThemeStyles';
import * as Connections from '@libs/actions/connections';
import * as ErrorUtils from '@libs/ErrorUtils';
import Navigation from '@libs/Navigation/Navigation';
import {getXeroBankAccountsWithDefaultSelect, settingsPendingAction} from '@libs/PolicyUtils';
import {getXeroBankAccounts, settingsPendingAction} from '@libs/PolicyUtils';
import type {WithPolicyConnectionsProps} from '@pages/workspace/withPolicyConnections';
import withPolicyConnections from '@pages/workspace/withPolicyConnections';
import variables from '@styles/variables';
Expand All @@ -27,7 +27,7 @@ function XeroBillPaymentAccountSelectorPage({policy}: WithPolicyConnectionsProps

const {config} = policy?.connections?.xero ?? {};
const {reimbursementAccountID, syncReimbursedReports} = policy?.connections?.xero?.config.sync ?? {};
const xeroSelectorOptions = useMemo<SelectorType[]>(() => getXeroBankAccountsWithDefaultSelect(policy ?? undefined, reimbursementAccountID), [reimbursementAccountID, policy]);
const xeroSelectorOptions = useMemo<SelectorType[]>(() => getXeroBankAccounts(policy ?? undefined, reimbursementAccountID), [reimbursementAccountID, policy]);

const listHeaderComponent = useMemo(
() => (
Expand All @@ -42,12 +42,18 @@ function XeroBillPaymentAccountSelectorPage({policy}: WithPolicyConnectionsProps

const updateAccount = useCallback(
({value}: SelectorType) => {
Connections.updatePolicyXeroConnectionConfig(policyID, CONST.POLICY.CONNECTIONS.NAME.XERO, CONST.XERO_CONFIG.SYNC, {
reimbursementAccountID: value,
});
Connections.updatePolicyXeroConnectionConfig(
policyID,
CONST.POLICY.CONNECTIONS.NAME.XERO,
CONST.XERO_CONFIG.SYNC,
{
reimbursementAccountID: value,
},
{reimbursementAccountID: reimbursementAccountID ?? null},
);
Navigation.goBack(ROUTES.POLICY_ACCOUNTING_XERO_ADVANCED.getRoute(policyID));
},
[policyID],
[policyID, reimbursementAccountID],
);

const listEmptyContent = useMemo(
Expand Down
18 changes: 12 additions & 6 deletions src/pages/workspace/accounting/xero/advanced/XeroInvoiceAccountSelectorPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import useThemeStyles from '@hooks/useThemeStyles';
import * as Connections from '@libs/actions/connections';
import * as ErrorUtils from '@libs/ErrorUtils';
import Navigation from '@libs/Navigation/Navigation';
import {getXeroBankAccountsWithDefaultSelect, settingsPendingAction} from '@libs/PolicyUtils';
import {getXeroBankAccounts, settingsPendingAction} from '@libs/PolicyUtils';
import type {WithPolicyConnectionsProps} from '@pages/workspace/withPolicyConnections';
import withPolicyConnections from '@pages/workspace/withPolicyConnections';
import variables from '@styles/variables';
Expand All @@ -27,7 +27,7 @@ function XeroInvoiceAccountSelectorPage({policy}: WithPolicyConnectionsProps) {

const {config} = policy?.connections?.xero ?? {};
const {invoiceCollectionsAccountID, syncReimbursedReports} = policy?.connections?.xero?.config.sync ?? {};
const xeroSelectorOptions = useMemo<SelectorType[]>(() => getXeroBankAccountsWithDefaultSelect(policy ?? undefined, invoiceCollectionsAccountID), [invoiceCollectionsAccountID, policy]);
const xeroSelectorOptions = useMemo<SelectorType[]>(() => getXeroBankAccounts(policy ?? undefined, invoiceCollectionsAccountID), [invoiceCollectionsAccountID, policy]);

const listHeaderComponent = useMemo(
() => (
Expand All @@ -42,12 +42,18 @@ function XeroInvoiceAccountSelectorPage({policy}: WithPolicyConnectionsProps) {

const updateAccount = useCallback(
({value}: SelectorType) => {
Connections.updatePolicyXeroConnectionConfig(policyID, CONST.POLICY.CONNECTIONS.NAME.XERO, CONST.XERO_CONFIG.SYNC, {
invoiceCollectionsAccountID: value,
});
Connections.updatePolicyXeroConnectionConfig(
policyID,
CONST.POLICY.CONNECTIONS.NAME.XERO,
CONST.XERO_CONFIG.SYNC,
{
invoiceCollectionsAccountID: value,
},
{invoiceCollectionsAccountID: invoiceCollectionsAccountID ?? null},
);
Navigation.goBack(ROUTES.POLICY_ACCOUNTING_XERO_ADVANCED.getRoute(policyID));
},
[policyID],
[policyID, invoiceCollectionsAccountID],
);

const listEmptyContent = useMemo(
Expand Down
18 changes: 12 additions & 6 deletions src/pages/workspace/accounting/xero/export/XeroBankAccountSelectPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import useThemeStyles from '@hooks/useThemeStyles';
import * as Connections from '@libs/actions/connections';
import * as ErrorUtils from '@libs/ErrorUtils';
import Navigation from '@libs/Navigation/Navigation';
import {getXeroBankAccountsWithDefaultSelect} from '@libs/PolicyUtils';
import {getXeroBankAccounts} from '@libs/PolicyUtils';
import * as PolicyUtils from '@libs/PolicyUtils';
import type {WithPolicyConnectionsProps} from '@pages/workspace/withPolicyConnections';
import withPolicyConnections from '@pages/workspace/withPolicyConnections';
Expand All @@ -28,7 +28,7 @@ function XeroBankAccountSelectPage({policy}: WithPolicyConnectionsProps) {

const {config} = policy?.connections?.xero ?? {};
const xeroSelectorOptions = useMemo<SelectorType[]>(
() => getXeroBankAccountsWithDefaultSelect(policy ?? undefined, config?.export?.nonReimbursableAccount),
() => getXeroBankAccounts(policy ?? undefined, config?.export?.nonReimbursableAccount),
Copy link
Contributor

Choose a reason for hiding this comment

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

Coming from this issue #52699, removing default value here cause inconsistent between "Export" page and "Xero bank account" page, and we handled it here #52974

[config?.export?.nonReimbursableAccount, policy],
);

Expand All @@ -46,13 +46,19 @@ function XeroBankAccountSelectPage({policy}: WithPolicyConnectionsProps) {
const updateBankAccount = useCallback(
({value}: SelectorType) => {
if (initiallyFocusedOptionKey !== value) {
Connections.updatePolicyXeroConnectionConfig(policyID, CONST.POLICY.CONNECTIONS.NAME.XERO, CONST.XERO_CONFIG.EXPORT, {
nonReimbursableAccount: value,
});
Connections.updatePolicyXeroConnectionConfig(
policyID,
CONST.POLICY.CONNECTIONS.NAME.XERO,
CONST.XERO_CONFIG.EXPORT,
{
nonReimbursableAccount: value,
},
{nonReimbursableAccount: config?.export?.nonReimbursableAccount ?? null},
);
}
Navigation.goBack(ROUTES.POLICY_ACCOUNTING_XERO_EXPORT.getRoute(policyID));
},
[policyID, initiallyFocusedOptionKey],
[initiallyFocusedOptionKey, policyID, config?.export?.nonReimbursableAccount],
);

const listEmptyContent = useMemo(
Expand Down
8 changes: 7 additions & 1 deletion src/pages/workspace/accounting/xero/export/XeroPreferredExporterSelectPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,13 @@ function XeroPreferredExporterSelectPage({policy}: WithPolicyConnectionsProps) {
const selectExporter = useCallback(
(row: CardListItem) => {
if (row.value !== config?.export?.exporter) {
Connections.updatePolicyXeroConnectionConfig(policyID, CONST.POLICY.CONNECTIONS.NAME.XERO, CONST.XERO_CONFIG.EXPORT, {exporter: row.value});
Connections.updatePolicyXeroConnectionConfig(
policyID,
CONST.POLICY.CONNECTIONS.NAME.XERO,
CONST.XERO_CONFIG.EXPORT,
{exporter: row.value},
{exporter: config?.export?.exporter ?? null},
);
}
Navigation.goBack(ROUTES.POLICY_ACCOUNTING_XERO_EXPORT.getRoute(policyID));
},
Expand Down
8 changes: 7 additions & 1 deletion src/pages/workspace/accounting/xero/export/XeroPurchaseBillDateSelectPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,13 @@ function XeroPurchaseBillDateSelectPage({policy}: WithPolicyConnectionsProps) {
const selectExportDate = useCallback(
(row: MenuListItem) => {
if (row.value !== config?.export?.billDate) {
Connections.updatePolicyXeroConnectionConfig(policyID, CONST.POLICY.CONNECTIONS.NAME.XERO, CONST.XERO_CONFIG.EXPORT, {billDate: row.value});
Connections.updatePolicyXeroConnectionConfig(
policyID,
CONST.POLICY.CONNECTIONS.NAME.XERO,
CONST.XERO_CONFIG.EXPORT,
{billDate: row.value},
{billDate: config?.export?.billDate ?? null},
);
}
Navigation.goBack(ROUTES.POLICY_ACCOUNTING_XERO_EXPORT_PURCHASE_BILL_DATE_SELECT.getRoute(policyID));
},
Expand Down
Loading
Loading