Fix wrong scheduled transfer payment direction on PWA (#3402)

* Fix #3230 * Release notes * Rename hook function * [chore] Comment * Coderabbit feedback * Fix loading states * Code rabbit * No payee text * Update VRT * Update release notes --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
actualbudget · Nov 19, 2024 · 881410b · 881410b
1 parent b4d2d6a
commit 881410b
Show file tree

Hide file tree

Showing 17 changed files with 235 additions and 206 deletions.
diff --git a/...dividual-account-page-and-checks-that-filtering-is-working-1-chromium-linux.png b/...dividual-account-page-and-checks-that-filtering-is-working-1-chromium-linux.png
diff --git a/...dividual-account-page-and-checks-that-filtering-is-working-2-chromium-linux.png b/...dividual-account-page-and-checks-that-filtering-is-working-2-chromium-linux.png
diff --git a/...dividual-account-page-and-checks-that-filtering-is-working-3-chromium-linux.png b/...dividual-account-page-and-checks-that-filtering-is-working-3-chromium-linux.png
diff --git a/packages/desktop-client/src/components/accounts/Account.tsx b/packages/desktop-client/src/components/accounts/Account.tsx
@@ -5,6 +5,7 @@ import React, {
   createRef,
   useMemo,
   type ReactElement,
+  useEffect,
 } from 'react';
 import { Trans } from 'react-i18next';
 import { useSelector } from 'react-redux';
@@ -30,7 +31,6 @@ import {
 import { send, listen } from 'loot-core/src/platform/client/fetch';
 import { currentDay } from 'loot-core/src/shared/months';
 import { q, type Query } from 'loot-core/src/shared/query';
-import { getScheduledAmount } from 'loot-core/src/shared/schedules';
 import {
   updateTransaction,
   realizeTempTransactions,
@@ -50,14 +50,14 @@ import {
   type TransactionFilterEntity,
 } from 'loot-core/src/types/models';
 
+import { useAccountPreviewTransactions } from '../../hooks/useAccountPreviewTransactions';
 import { useAccounts } from '../../hooks/useAccounts';
 import { useActions } from '../../hooks/useActions';
 import { useCategories } from '../../hooks/useCategories';
 import { useDateFormat } from '../../hooks/useDateFormat';
 import { useFailedAccounts } from '../../hooks/useFailedAccounts';
 import { useLocalPref } from '../../hooks/useLocalPref';
 import { usePayees } from '../../hooks/usePayees';
-import { usePreviewTransactions } from '../../hooks/usePreviewTransactions';
 import {
   SelectedProviderWithItems,
   type Actions,
@@ -147,7 +147,6 @@ type AllTransactionsProps = {
     transactions: TransactionEntity[],
     balances: Record<string, { balance: number }> | null,
   ) => ReactElement;
-  collapseTransactions: (ids: string[]) => void;
 };
 
 function AllTransactions({
@@ -157,14 +156,24 @@ function AllTransactions({
   showBalances,
   filtered,
   children,
-  collapseTransactions,
 }: AllTransactionsProps) {
   const accountId = account?.id;
-  const prependTransactions: (TransactionEntity & { _inverse?: boolean })[] =
-    usePreviewTransactions(collapseTransactions).map(trans => ({
-      ...trans,
-      _inverse: accountId ? accountId !== trans.account : false,
-    }));
+  const { dispatch: splitsExpandedDispatch } = useSplitsExpanded();
+  const { previewTransactions, isLoading: isPreviewTransactionsLoading } =
+    useAccountPreviewTransactions({ accountId });
+
+  useEffect(() => {
+    if (!isPreviewTransactionsLoading) {
+      splitsExpandedDispatch({
+        type: 'close-splits',
+        ids: previewTransactions.filter(t => t.is_parent).map(t => t.id),
+      });
+    }
+  }, [
+    isPreviewTransactionsLoading,
+    previewTransactions,
+    splitsExpandedDispatch,
+  ]);
 
   transactions ??= [];
 
@@ -184,29 +193,26 @@ function AllTransactions({
     }
 
     // Reverse so we can calculate from earliest upcoming schedule.
-    const scheduledBalances = [...prependTransactions]
+    const previewBalances = [...previewTransactions]
       .reverse()
-      .map(scheduledTransaction => {
-        const amount =
-          (scheduledTransaction._inverse ? -1 : 1) *
-          getScheduledAmount(scheduledTransaction.amount);
+      .map(previewTransaction => {
         return {
           // TODO: fix me
           // eslint-disable-next-line react-hooks/exhaustive-deps
-          balance: (runningBalance += amount),
-          id: scheduledTransaction.id,
+          balance: (runningBalance += previewTransaction.amount),
+          id: previewTransaction.id,
         };
       });
-    return groupById(scheduledBalances);
-  }, [showBalances, prependTransactions, runningBalance]);
+    return groupById(previewBalances);
+  }, [showBalances, previewTransactions, runningBalance]);
 
   const allTransactions = useMemo(() => {
     // Don't prepend scheduled transactions if we are filtering
-    if (!filtered && prependTransactions.length > 0) {
-      return prependTransactions.concat(transactions);
+    if (!filtered && previewTransactions.length > 0) {
+      return previewTransactions.concat(transactions);
     }
     return transactions;
-  }, [filtered, prependTransactions, transactions]);
+  }, [filtered, previewTransactions, transactions]);
 
   const allBalances = useMemo(() => {
     // Don't prepend scheduled transactions if we are filtering
@@ -216,7 +222,7 @@ function AllTransactions({
     return balances;
   }, [filtered, prependBalances, balances]);
 
-  if (!prependTransactions?.length || filtered) {
+  if (!previewTransactions?.length || filtered) {
     return children(transactions, balances);
   }
   return children(allTransactions, allBalances);
@@ -1669,9 +1675,6 @@ class AccountInternal extends PureComponent<
         balances={balances}
         showBalances={showBalances}
         filtered={transactionsFiltered}
-        collapseTransactions={ids =>
-          this.props.splitsExpandedDispatch({ type: 'close-splits', ids })
-        }
       >
         {(allTransactions, allBalances) => (
           <SelectedProviderWithItems

diff --git a/packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx b/packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx
@@ -22,7 +22,6 @@ import {
   SchedulesProvider,
 } from 'loot-core/client/data-hooks/schedules';
 import {
-  usePreviewTransactions,
   useTransactions,
   useTransactionsSearch,
 } from 'loot-core/client/data-hooks/transactions';
@@ -35,6 +34,7 @@ import {
   type TransactionEntity,
 } from 'loot-core/types/models';
 
+import { useAccountPreviewTransactions } from '../../../hooks/useAccountPreviewTransactions';
 import { useDateFormat } from '../../../hooks/useDateFormat';
 import { useFailedAccounts } from '../../../hooks/useFailedAccounts';
 import { useNavigate } from '../../../hooks/useNavigate';
@@ -239,7 +239,9 @@ function TransactionListWithPreviews({
     query: transactionsQuery,
   });
 
-  const { data: previewTransactions } = usePreviewTransactions();
+  const { previewTransactions } = useAccountPreviewTransactions({
+    accountId: account?.id || '',
+  });
 
   const dateFormat = useDateFormat() || 'MM/dd/yyyy';
   const dispatch = useDispatch();

diff --git a/packages/desktop-client/src/components/mobile/transactions/TransactionEdit.jsx b/packages/desktop-client/src/components/mobile/transactions/TransactionEdit.jsx
@@ -64,25 +64,14 @@ import { MobilePageHeader, Page } from '../../Page';
 import { AmountInput } from '../../util/AmountInput';
 import { MobileBackButton } from '../MobileBackButton';
 import { FieldLabel, TapField, InputField, ToggleField } from '../MobileForms';
+import { getPrettyPayee } from '../utils';
 
 import { FocusableAmountInput } from './FocusableAmountInput';
 
 function getFieldName(transactionId, field) {
   return `${field}-${transactionId}`;
 }
 
-export function getDescriptionPretty(transaction, payee, transferAcct) {
-  const { amount } = transaction;
-
-  if (transferAcct) {
-    return `Transfer ${amount > 0 ? 'from' : 'to'} ${transferAcct.name}`;
-  } else if (payee) {
-    return payee.name;
-  }
-
-  return '';
-}
-
 function serializeTransaction(transaction, dateFormat) {
   const { date, amount } = transaction;
   return {
@@ -291,7 +280,8 @@ const ChildTransactionEdit = forwardRef(
       amountFocused,
       amountSign,
       getCategory,
-      getPrettyPayee,
+      getPayee,
+      getTransferAccount,
       isOffBudget,
       isBudgetTransfer,
       onEditField,
@@ -302,6 +292,11 @@ const ChildTransactionEdit = forwardRef(
   ) => {
     const { editingField, onRequestActiveEdit, onClearActiveEdit } =
       useSingleActiveEditForm();
+    const prettyPayee = getPrettyPayee({
+      transaction,
+      payee: getPayee(transaction),
+      transferAccount: getTransferAccount(transaction),
+    });
     return (
       <View
         innerRef={ref}
@@ -325,7 +320,7 @@ const ChildTransactionEdit = forwardRef(
                 editingField &&
                 editingField !== getFieldName(transaction.id, 'payee')
               }
-              value={getPrettyPayee(transaction)}
+              value={prettyPayee}
               onClick={() => onEditField(transaction.id, 'payee')}
               data-testid={`payee-field-${transaction.id}`}
             />
@@ -503,32 +498,20 @@ const TransactionEditInner = memo(function TransactionEditInner({
     [payeesById],
   );
 
-  const getTransferAcct = useCallback(
+  const getTransferAccount = useCallback(
     trans => {
       const payee = trans && getPayee(trans);
       return payee?.transfer_acct && accountsById?.[payee.transfer_acct];
     },
     [accountsById, getPayee],
   );
 
-  const getPrettyPayee = useCallback(
-    trans => {
-      if (trans?.is_parent) {
-        return 'Split';
-      }
-      const transPayee = trans && getPayee(trans);
-      const transTransferAcct = trans && getTransferAcct(trans);
-      return getDescriptionPretty(trans, transPayee, transTransferAcct);
-    },
-    [getPayee, getTransferAcct],
-  );
-
   const isBudgetTransfer = useCallback(
     trans => {
-      const transferAcct = trans && getTransferAcct(trans);
+      const transferAcct = trans && getTransferAccount(trans);
       return transferAcct && !transferAcct.offbudget;
     },
-    [getTransferAcct],
+    [getTransferAccount],
   );
 
   const getCategory = useCallback(
@@ -759,11 +742,11 @@ const TransactionEditInner = memo(function TransactionEditInner({
 
   const account = getAccount(transaction);
   const isOffBudget = account && !!account.offbudget;
-  const title = getDescriptionPretty(
+  const title = getPrettyPayee({
     transaction,
-    getPayee(transaction),
-    getTransferAcct(transaction),
-  );
+    payee: getPayee(transaction),
+    transferAccount: getTransferAccount(transaction),
+  });
 
   const transactionDate = parseDate(transaction.date, dateFormat, new Date());
   const dateDefaultValue = monthUtils.dayFromDate(transactionDate);
@@ -834,7 +817,7 @@ const TransactionEditInner = memo(function TransactionEditInner({
                 fontWeight: 300,
               }),
             }}
-            value={getPrettyPayee(transaction)}
+            value={title}
             disabled={
               editingField &&
               editingField !== getFieldName(transaction.id, 'payee')
@@ -882,7 +865,8 @@ const TransactionEditInner = memo(function TransactionEditInner({
             }}
             isOffBudget={isOffBudget}
             getCategory={getCategory}
-            getPrettyPayee={getPrettyPayee}
+            getPayee={getPayee}
+            getTransferAccount={getTransferAccount}
             isBudgetTransfer={isBudgetTransfer}
             onUpdate={onUpdateInner}
             onEditField={onEditFieldInner}

diff --git a/packages/desktop-client/src/components/mobile/transactions/TransactionListItem.tsx b/packages/desktop-client/src/components/mobile/transactions/TransactionListItem.tsx
@@ -12,7 +12,6 @@ import {
   useLongPress,
 } from '@react-aria/interactions';
 
-import { getScheduledAmount } from 'loot-core/src/shared/schedules';
 import { isPreviewId } from 'loot-core/src/shared/transactions';
 import { integerToCurrency } from 'loot-core/src/shared/util';
 import { type TransactionEntity } from 'loot-core/types/models';
@@ -32,8 +31,9 @@ import { Button } from '../../common/Button2';
 import { Text } from '../../common/Text';
 import { TextOneLine } from '../../common/TextOneLine';
 import { View } from '../../common/View';
+import { getPrettyPayee } from '../utils';
 
-import { lookupName, getDescriptionPretty, Status } from './TransactionEdit';
+import { lookupName, Status } from './TransactionEdit';
 
 const ROW_HEIGHT = 60;
 
@@ -56,7 +56,7 @@ export function TransactionListItem({
 
   const payee = usePayee(transaction?.payee || '');
   const account = useAccount(transaction?.account || '');
-  const transferAcct = useAccount(payee?.transfer_acct || '');
+  const transferAccount = useAccount(payee?.transfer_acct || '');
   const isPreview = isPreviewId(transaction?.id || '');
 
   const newTransactions = useSelector(state => state.queries.newTransactions);
@@ -84,32 +84,25 @@ export function TransactionListItem({
 
   const {
     id,
-    amount: originalAmount,
+    amount,
     category: categoryId,
     cleared: isCleared,
     reconciled: isReconciled,
     is_parent: isParent,
     is_child: isChild,
-    schedule,
+    schedule: scheduleId,
   } = transaction;
 
   const isAdded = newTransactions.includes(id);
-
-  let amount = originalAmount;
-  if (isPreview) {
-    amount = getScheduledAmount(amount);
-  }
-
   const categoryName = lookupName(categories, categoryId);
-
-  const prettyDescription = getDescriptionPretty(
+  const prettyPayee = getPrettyPayee({
     transaction,
     payee,
-    transferAcct,
-  );
+    transferAccount,
+  });
   const specialCategory = account?.offbudget
     ? 'Off Budget'
-    : transferAcct && !transferAcct.offbudget
+    : transferAccount && !transferAccount.offbudget
       ? 'Transfer'
       : isParent
         ? 'Split'
@@ -169,7 +162,7 @@ export function TransactionListItem({
             >
               <View>
                 <View style={{ flexDirection: 'row', alignItems: 'center' }}>
-                  {schedule && (
+                  {scheduleId && (
                     <SvgArrowsSynchronize
                       style={{
                         width: 12,
@@ -183,13 +176,13 @@ export function TransactionListItem({
                     style={{
                       ...textStyle,
                       fontWeight: isAdded ? '600' : '400',
-                      ...(prettyDescription === '' && {
+                      ...(prettyPayee === '' && {
                         color: theme.tableTextLight,
                         fontStyle: 'italic',
                       }),
                     }}
                   >
-                    {prettyDescription || 'Empty'}
+                    {prettyPayee || '(No payee)'}
                   </TextOneLine>
                 </View>
                 {isPreview ? (