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

[HOLD Expensify/issues/390731] Fix account menu item visibility + reset account values #41079

Closed
wants to merge 11 commits into from
Closed

[HOLD Expensify/issues/390731] Fix account menu item visibility + reset account values #41079

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
2c1cdf5
Fix account menu item visibility + reset account values
narefyev91 Apr 26, 2024
27e1fac
correct naming for accountPayable
narefyev91 Apr 26, 2024
3378491
reset values to defaults
narefyev91 Apr 26, 2024
36bff79
clean up
narefyev91 Apr 29, 2024
eaf68b0
Update src/pages/workspace/accounting/qbo/export/QuickbooksCompanyCar…
narefyev91 Apr 29, 2024
ecd9631
Update src/pages/workspace/accounting/qbo/export/QuickbooksOutOfPocke…
narefyev91 Apr 29, 2024
d23b303
update lint
narefyev91 Apr 29, 2024
2e22a9e
Merge branch 'refs/heads/main' into fix-account-visibility
narefyev91 Apr 29, 2024
0b029a3
conflicts
narefyev91 Apr 29, 2024
c32473b
update after review
narefyev91 Apr 30, 2024
7b073df
update callbacks
narefyev91 Apr 30, 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
55 changes: 42 additions & 13 deletions src/pages/workspace/accounting/qbo/export/QuickbooksCompanyCardExpenseAccountPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import AccessOrNotFoundWrapper from '@pages/workspace/AccessOrNotFoundWrapper';
import type {WithPolicyConnectionsProps} from '@pages/workspace/withPolicyConnections';
import withPolicyConnections from '@pages/workspace/withPolicyConnections';
import ToggleSettingOptionRow from '@pages/workspace/workflows/ToggleSettingsOptionRow';
import * as Policy from '@userActions/Policy';
import CONST from '@src/CONST';
import ROUTES from '@src/ROUTES';

Expand All @@ -22,6 +23,31 @@ function QuickbooksCompanyCardExpenseAccountPage({policy}: WithPolicyConnections
const policyID = policy?.id ?? '';
const {exportCompanyCardAccount, exportAccountPayable, autoCreateVendor, errorFields, pendingFields, exportCompanyCard} = policy?.connections?.quickbooksOnline?.config ?? {};
const isVendorSelected = exportCompanyCard === CONST.QUICKBOOKS_EXPORT_COMPANY_CARD.VENDOR_BILL;
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const showAccountSelection = Boolean(autoCreateVendor || (!isVendorSelected && exportCompanyCard));
const {vendors} = policy?.connections?.quickbooksOnline?.data ?? {};

const updateAutoCreateVendor = (value: boolean) => {
Connections.updatePolicyConnectionConfig(policyID, CONST.POLICY.CONNECTIONS.NAME.QBO, CONST.QUICK_BOOKS_CONFIG.EXPORT_COMPANY_CARD_ACCOUNT, value ? vendors?.[0]?.name : undefined);
Connections.updatePolicyConnectionConfig(policyID, CONST.POLICY.CONNECTIONS.NAME.QBO, CONST.QUICK_BOOKS_CONFIG.AUTO_CREATE_VENDOR, value);
};

const navigateToSelectCompanyCardExpense = () => {
Navigation.navigate(ROUTES.POLICY_ACCOUNTING_QUICKBOOKS_ONLINE_COMPANY_CARD_EXPENSE_SELECT.getRoute(policyID));
};

const navigateToSelectCompanyCardExpenseAccount = () => {
Navigation.navigate(ROUTES.POLICY_ACCOUNTING_QUICKBOOKS_ONLINE_COMPANY_CARD_EXPENSE_ACCOUNT_SELECT.getRoute(policyID));
};

const navigateToSelectCompanyCardExpenseAccountPayable = () => {
Navigation.navigate(ROUTES.POLICY_ACCOUNTING_QUICKBOOKS_ONLINE_COMPANY_CARD_EXPENSE_ACCOUNT_PAYABLE_SELECT.getRoute(policyID));
};

const closeQBOErrors = () => {
Policy.clearQBOErrorField(policyID, CONST.QUICK_BOOKS_CONFIG.AUTO_CREATE_VENDOR);
};

return (
<AccessOrNotFoundWrapper
policyID={policyID}
Expand All @@ -40,7 +66,7 @@ function QuickbooksCompanyCardExpenseAccountPage({policy}: WithPolicyConnections
title={exportCompanyCard ? translate(`workspace.qbo.${exportCompanyCard}`) : undefined}
description={translate('workspace.qbo.exportCompany')}
error={errorFields?.exportCompanyCard ? translate('common.genericErrorMessage') : undefined}
onPress={() => Navigation.navigate(ROUTES.POLICY_ACCOUNTING_QUICKBOOKS_ONLINE_COMPANY_CARD_EXPENSE_SELECT.getRoute(policyID))}
onPress={navigateToSelectCompanyCardExpense}
brickRoadIndicator={errorFields?.exportCompanyCard ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
shouldShowRightIcon
/>
Expand All @@ -55,7 +81,7 @@ function QuickbooksCompanyCardExpenseAccountPage({policy}: WithPolicyConnections
title={exportAccountPayable}
description={translate('workspace.qbo.accountsPayable')}
error={errorFields?.exportAccountPayable ? translate('common.genericErrorMessage') : undefined}
onPress={() => Navigation.navigate(ROUTES.POLICY_ACCOUNTING_QUICKBOOKS_ONLINE_COMPANY_CARD_EXPENSE_ACCOUNT_PAYABLE_SELECT.getRoute(policyID))}
onPress={navigateToSelectCompanyCardExpenseAccountPayable}
brickRoadIndicator={errorFields?.exportAccountPayable ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
shouldShowRightIcon
/>
Expand All @@ -66,21 +92,24 @@ function QuickbooksCompanyCardExpenseAccountPage({policy}: WithPolicyConnections
title={translate('workspace.qbo.defaultVendor')}
wrapperStyle={[styles.ph5, styles.mb3, styles.mt1]}
isActive={Boolean(autoCreateVendor)}
onToggle={(isOn) => Connections.updatePolicyConnectionConfig(policyID, CONST.POLICY.CONNECTIONS.NAME.QBO, CONST.QUICK_BOOKS_CONFIG.AUTO_CREATE_VENDOR, isOn)}
onCloseError={closeQBOErrors}
onToggle={updateAutoCreateVendor}
pendingAction={pendingFields?.autoCreateVendor}
/>
</>
)}
<OfflineWithFeedback pendingAction={pendingFields?.exportCompanyCardAccount}>
<MenuItemWithTopDescription
title={exportCompanyCardAccount}
description={isVendorSelected ? translate('workspace.qbo.vendor') : translate('workspace.qbo.account')}
onPress={() => Navigation.navigate(ROUTES.POLICY_ACCOUNTING_QUICKBOOKS_ONLINE_COMPANY_CARD_EXPENSE_ACCOUNT_SELECT.getRoute(policyID))}
brickRoadIndicator={errorFields?.exportCompanyCardAccount ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
shouldShowRightIcon
error={errorFields?.exportCompanyCardAccount ? translate('common.genericErrorMessage') : undefined}
/>
</OfflineWithFeedback>
{showAccountSelection && (
<OfflineWithFeedback pendingAction={pendingFields?.exportCompanyCardAccount}>
<MenuItemWithTopDescription
title={exportCompanyCardAccount}
description={isVendorSelected ? translate('workspace.qbo.vendor') : translate('workspace.qbo.account')}
onPress={navigateToSelectCompanyCardExpenseAccount}
brickRoadIndicator={errorFields?.exportCompanyCardAccount ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
shouldShowRightIcon
error={errorFields?.exportCompanyCardAccount ? translate('common.genericErrorMessage') : undefined}
/>
</OfflineWithFeedback>
)}
</ScrollView>
</ScreenWrapper>
</AccessOrNotFoundWrapper>
Expand Down
6 changes: 3 additions & 3 deletions .../workspace/accounting/qbo/export/QuickbooksCompanyCardExpenseAccountPayableSelectPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,19 @@ type CardListItem = ListItem & {
function QuickbooksCompanyCardExpenseAccountPayableSelectPage({policy}: WithPolicyConnectionsProps) {
const {translate} = useLocalize();
const styles = useThemeStyles();
const {accountsPayable} = policy?.connections?.quickbooksOnline?.data ?? {};
const {accountPayable} = policy?.connections?.quickbooksOnline?.data ?? {};
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a memo for myself. Checked this field name is correct

Screenshot 2024-05-06 at 8 59 38 AM

const {exportAccountPayable} = policy?.connections?.quickbooksOnline?.config ?? {};

const policyID = policy?.id ?? '';
const data: CardListItem[] = useMemo(
() =>
accountsPayable?.map((account) => ({
accountPayable?.map((account) => ({
value: account.name,
text: account.name,
keyForList: account.name,
isSelected: account.name === exportAccountPayable,
})) ?? [],
[exportAccountPayable, accountsPayable],
[exportAccountPayable, accountPayable],
);

const selectAccountPayable = useCallback(
Expand Down
12 changes: 10 additions & 2 deletions ...ges/workspace/accounting/qbo/export/QuickbooksCompanyCardExpenseAccountSelectCardPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ function QuickbooksCompanyCardExpenseAccountSelectCardPage({policy}: WithPolicyC
const policyID = policy?.id ?? '';
const {exportCompanyCard, syncLocations} = policy?.connections?.quickbooksOnline?.config ?? {};
const isLocationEnabled = Boolean(syncLocations && syncLocations !== CONST.INTEGRATION_ENTITY_MAP_TYPES.NONE);
const {creditCards, bankAccounts, accountPayable} = policy?.connections?.quickbooksOnline?.data ?? {};
Copy link
Contributor

Choose a reason for hiding this comment

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

memo: checked that these field names are correct

Screenshot 2024-05-06 at 9 01 06 AM


const defaultCards = useMemo<Card[]>(
() => [
Expand Down Expand Up @@ -67,11 +68,18 @@ function QuickbooksCompanyCardExpenseAccountSelectCardPage({policy}: WithPolicyC
(row: CardListItem) => {
if (row.value !== exportCompanyCard) {
Connections.updatePolicyConnectionConfig(policyID, CONST.POLICY.CONNECTIONS.NAME.QBO, CONST.QUICK_BOOKS_CONFIG.EXPORT_COMPANY_CARD, row.value);
Connections.updatePolicyConnectionConfig(policyID, CONST.POLICY.CONNECTIONS.NAME.QBO, CONST.QUICK_BOOKS_CONFIG.EXPORT_COMPANY_CARD_ACCOUNT);
if (row.value === CONST.QUICKBOOKS_EXPORT_COMPANY_CARD.VENDOR_BILL) {
Connections.updatePolicyConnectionConfig(policyID, CONST.POLICY.CONNECTIONS.NAME.QBO, CONST.QUICK_BOOKS_CONFIG.EXPORT_COMPANY_CARD_ACCOUNT);
Connections.updatePolicyConnectionConfig(policyID, CONST.POLICY.CONNECTIONS.NAME.QBO, CONST.QUICK_BOOKS_CONFIG.EXPORT_ACCOUNT_PAYABLE, accountPayable?.[0]?.name);
Connections.updatePolicyConnectionConfig(policyID, CONST.POLICY.CONNECTIONS.NAME.QBO, CONST.QUICK_BOOKS_CONFIG.AUTO_CREATE_VENDOR);
Comment on lines 70 to +74
Copy link
Contributor

Choose a reason for hiding this comment

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

New record! 4 API calls 😅 cc @hayata-suenaga same question ^

Copy link
Contributor

Choose a reason for hiding this comment

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

@s77rt is correct. This breaks the 1:1:1 API design and shouldn't be allowed.

Copy link
Contributor

@hayata-suenaga hayata-suenaga Apr 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@narefyev91

also, can you remind me why we added this field to the config object? 😄
Screenshot 2024-04-30 at 12 33 55 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

also this one
Screenshot 2024-04-30 at 12 36 15 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

were these values there before your PR? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

The changes in the backend will be done in this GH issue to update several fields of the connections configuration object, but the issue description might need to be updated. cc: @aldo-expensify

There are fields I don't recognized, so I'd wait for @narefyev91's confirmation

Copy link
Contributor

@hayata-suenaga hayata-suenaga Apr 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

yes these values were added in the PR.

Screenshot 2024-04-30 at 12 41 59 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

I think these fields don't exist on the backend 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't be adding config fields that don't already exist. This is a sample working QBO config:

{
    "realmId": "2222",
    "credentials": {},
    "companyName": "Expensify",
    "autoSync": {
        "jobID": "11111111111",
        "enabled": true
    },
    "syncPeople": true,
    "syncItems": true,
    "markChecksToBePrinted": false,
    "reimbursableExpensesExportDestination": "bill",
    "nonReimbursableExpensesExportDestination": "credit_card",
    "reimbursableExpensesAccount": {
        "glCode": "",
        "name": "Accounts Payable",
        "currency": "USD",
        "id": "54"
    },
    "nonReimbursableExpensesAccount": {
        "glCode": "",
        "name": "Expensify Card Liability Account",
        "currency": "USD",
        "id": "135"
    },
    "autoCreateVendor": true,
    "hasChosenAutoSyncOption": true,
    "syncClasses": "REPORT_FIELD",
    "syncCustomers": "REPORT_FIELD",
    "syncLocations": "REPORT_FIELD",
    "exportDate": "LAST_EXPENSE",
    "lastConfigurationTime": 1714507864961,
    "syncTax": false,
    "enableNewCategories": true,
    "export": {
        "exporter": "aldo+testqbo56242@expensifail.com"
    },
    "nonReimbursableBillDefaultVendor": "NONE",
    "receivableAccount": {
        "glCode": "",
        "name": "Accounts Receivable",
        "currency": "USD",
        "id": "53"
    },
    "reimbursementAccountID": "93",
    "collectionAccountID": "93"
}

} else {
const accountName = row.value === CONST.QUICKBOOKS_EXPORT_COMPANY_CARD.CREDIT_CARD ? creditCards?.[0]?.name : bankAccounts?.[0]?.name;
Connections.updatePolicyConnectionConfig(policyID, CONST.POLICY.CONNECTIONS.NAME.QBO, CONST.QUICK_BOOKS_CONFIG.EXPORT_COMPANY_CARD_ACCOUNT, accountName);
Comment on lines +76 to +77
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB. Shouldn't we clear EXPORT_ACCOUNT_PAYABLE too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think it's not really needed - when user will switch to debit/credit card - EXPORT_ACCOUNT_PAYABLE will not be used anywhere

}
}
Navigation.goBack(ROUTES.POLICY_ACCOUNTING_QUICKBOOKS_ONLINE_COMPANY_CARD_EXPENSE_ACCOUNT.getRoute(policyID));
},
[exportCompanyCard, policyID],
[accountPayable, bankAccounts, creditCards, exportCompanyCard, policyID],
);

return (
Expand Down
6 changes: 3 additions & 3 deletions src/pages/workspace/accounting/qbo/export/QuickbooksOutOfPocketExpenseAccountSelectPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type CardListItem = ListItem & {
function QuickbooksOutOfPocketExpenseAccountSelectPage({policy}: WithPolicyConnectionsProps) {
const {translate} = useLocalize();
const styles = useThemeStyles();
const {bankAccounts, journalEntryAccounts, accountsPayable} = policy?.connections?.quickbooksOnline?.data ?? {};
const {bankAccounts, journalEntryAccounts, accountPayable} = policy?.connections?.quickbooksOnline?.data ?? {};

const {exportEntity, exportAccount} = policy?.connections?.quickbooksOnline?.config ?? {};

Expand All @@ -34,7 +34,7 @@ function QuickbooksOutOfPocketExpenseAccountSelectPage({policy}: WithPolicyConne
accounts = bankAccounts ?? [];
break;
case CONST.QUICKBOOKS_EXPORT_ENTITY.VENDOR_BILL:
accounts = accountsPayable ?? [];
accounts = accountPayable ?? [];
break;
case CONST.QUICKBOOKS_EXPORT_ENTITY.JOURNAL_ENTRY:
accounts = journalEntryAccounts ?? [];
Expand All @@ -49,7 +49,7 @@ function QuickbooksOutOfPocketExpenseAccountSelectPage({policy}: WithPolicyConne
keyForList: card.name,
isSelected: card.name === exportAccount,
}));
}, [accountsPayable, bankAccounts, exportAccount, exportEntity, journalEntryAccounts]);
}, [accountPayable, bankAccounts, exportAccount, exportEntity, journalEntryAccounts]);

const policyID = policy?.id ?? '';

Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/accounting/qbo/export/QuickbooksOutOfPocketExpenseConfigurationPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ function QuickbooksOutOfPocketExpenseConfigurationPage({policy}: WithPolicyConne
<Text style={[styles.ph5, styles.mutedNormalTextLabel, styles.pt1, styles.pb2]}>{translate('workspace.qbo.exportVendorBillDescription')}</Text>
)}
{isLocationEnabled && <Text style={[styles.ph5, styles.mutedNormalTextLabel, styles.pt1]}>{translate('workspace.qbo.outOfPocketLocationEnabledDescription')}</Text>}
{!isLocationEnabled && (
{!isLocationEnabled && !!exportEntity && (
<OfflineWithFeedback pendingAction={pendingFields?.exportAccount}>
<MenuItemWithTopDescription
title={exportAccount}
Expand Down
18 changes: 17 additions & 1 deletion src/pages/workspace/accounting/qbo/export/QuickbooksOutOfPocketExpenseEntitySelectPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ function QuickbooksOutOfPocketExpenseEntitySelectPage({policy}: WithPolicyConnec
const isTaxError = isTaxesEnabled && exportEntity === CONST.QUICKBOOKS_EXPORT_ENTITY.JOURNAL_ENTRY;
const isLocationError = isLocationsEnabled && exportEntity !== CONST.QUICKBOOKS_EXPORT_ENTITY.JOURNAL_ENTRY;
const policyID = policy?.id ?? '';
const {bankAccounts, journalEntryAccounts, accountPayable} = policy?.connections?.quickbooksOnline?.data ?? {};

useEffect(() => {
if (!isTaxError && !isLocationError) {
Expand Down Expand Up @@ -73,11 +74,26 @@ function QuickbooksOutOfPocketExpenseEntitySelectPage({policy}: WithPolicyConnec
const selectExportEntity = useCallback(
(row: CardListItem) => {
if (row.value !== exportEntity) {
let accountName;
switch (row.value) {
case CONST.QUICKBOOKS_EXPORT_ENTITY.CHECK:
accountName = bankAccounts?.[0]?.name;
break;
case CONST.QUICKBOOKS_EXPORT_ENTITY.VENDOR_BILL:
accountName = accountPayable?.[0]?.name;
break;
case CONST.QUICKBOOKS_EXPORT_ENTITY.JOURNAL_ENTRY:
accountName = journalEntryAccounts?.[0]?.name;
break;
default:
accountName = undefined;
}
Connections.updatePolicyConnectionConfig(policyID, CONST.POLICY.CONNECTIONS.NAME.QBO, CONST.QUICK_BOOKS_CONFIG.EXPORT_ENTITY, row.value);
Connections.updatePolicyConnectionConfig(policyID, CONST.POLICY.CONNECTIONS.NAME.QBO, CONST.QUICK_BOOKS_CONFIG.EXPORT_ACCOUNT, accountName);
Comment on lines 91 to +92
Copy link
Contributor

Choose a reason for hiding this comment

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

Two API calls. Should we add this to https://github.com/Expensify/Expensify/issues/390731 @hayata-suenaga

}
Navigation.goBack(ROUTES.POLICY_ACCOUNTING_QUICKBOOKS_ONLINE_EXPORT_OUT_OF_POCKET_EXPENSES.getRoute(policyID));
},
[exportEntity, policyID],
[accountPayable, bankAccounts, exportEntity, journalEntryAccounts, policyID],
);

return (
Expand Down
2 changes: 1 addition & 1 deletion src/types/onyx/Policy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ type QBOConnectionData = {
bankAccounts: Account[];
creditCards: Account[];
accountsReceivable: Account[];
accountsPayable: Account[];
accountPayable: Account[];
otherCurrentAssetAccounts: Account[];

taxCodes: TaxCode[];
Expand Down
Loading