From 463aa4355c2f78d29640f1c731b35b10c0564293 Mon Sep 17 00:00:00 2001
From: DJ Mountney <david@twkie.net>
Date: Mon, 26 Feb 2024 09:43:06 -0800
Subject: [PATCH 1/3] Adds option to make a transfer from two selected
 transactions

- Transactions amount must match
- Transactions must be from different acounts
- Split transactions not eligible
---
 .../src/components/accounts/Account.jsx       | 58 +++++++++++++++++++
 .../src/components/accounts/Header.jsx        |  6 ++
 .../transactions/SelectedTransactions.jsx     | 37 ++++++++++++
 3 files changed, 101 insertions(+)

diff --git a/packages/desktop-client/src/components/accounts/Account.jsx b/packages/desktop-client/src/components/accounts/Account.jsx
index 48f30a99a44..64f124c52c2 100644
--- a/packages/desktop-client/src/components/accounts/Account.jsx
+++ b/packages/desktop-client/src/components/accounts/Account.jsx
@@ -1059,6 +1059,63 @@ class AccountInternal extends PureComponent {
     this.props.pushModal('edit-rule', { rule });
   };
 
+  onSetTransfer = async ids => {
+    const onConfirmTransfer = async ids => {
+      this.setState({ workingHard: true });
+
+      const payees = await this.props.getPayees();
+
+      // not already a transfer
+      // no subtransactions, parent or singles only
+      const { data: transactions } = await runQuery(
+        q('transactions')
+          .filter({ id: { $oneof: ids }, is_child: false, transfer_id: null })
+          .select('*'),
+      );
+
+      const [fromTrans, toTrans] = transactions;
+
+      // only two selected
+      // belong to two different accounts
+      // amount must zero each other out
+      if (
+        transactions.length === 2 &&
+        fromTrans.account !== toTrans.account &&
+        fromTrans.amount + toTrans.amount === 0
+      ) {
+        const fromPayee = payees.find(
+          p => p.transfer_acct === fromTrans.account,
+        );
+        const toPayee = payees.find(p => p.transfer_acct === toTrans.account);
+
+        const changes = {
+          updated: [
+            {
+              ...fromTrans,
+              payee: toPayee.id,
+              transfer_id: toTrans.id,
+            },
+            {
+              ...toTrans,
+              payee: fromPayee.id,
+              transfer_id: fromTrans.id,
+            },
+          ],
+        };
+
+        await send('transactions-batch-update', changes);
+      }
+
+      await this.refetchTransactions();
+    };
+
+    await this.checkForReconciledTransactions(
+      ids,
+      'batchEditWithReconciled',
+      onConfirmTransfer,
+    );
+  };
+
   onCondOpChange = (value, filters) => {
     this.setState({ conditionsOp: value });
     this.setState({ filterId: { ...this.state.filterId, status: 'changed' } });
@@ -1443,6 +1500,7 @@ class AccountInternal extends PureComponent {
                 onDeleteFilter={this.onDeleteFilter}
                 onApplyFilter={this.onApplyFilter}
                 onScheduleAction={this.onScheduleAction}
+                onSetTransfer={this.onSetTransfer}
               />
 
               <View style={{ flex: 1 }}>
diff --git a/packages/desktop-client/src/components/accounts/Header.jsx b/packages/desktop-client/src/components/accounts/Header.jsx
index 789131335bb..eaade96bfb2 100644
--- a/packages/desktop-client/src/components/accounts/Header.jsx
+++ b/packages/desktop-client/src/components/accounts/Header.jsx
@@ -79,6 +79,7 @@ export function AccountHeader({
   onCondOpChange,
   onDeleteFilter,
   onScheduleAction,
+  onSetTransfer,
 }) {
   const [menuOpen, setMenuOpen] = useState(false);
   const searchInput = useRef(null);
@@ -94,6 +95,9 @@ export function AccountHeader({
     canSync = !!accounts.find(account => !!account.account_id) && isUsingServer;
   }
 
+  // Only show the ability to make linked transfers on multi-account views.
+  const showMakeTransfer = !account;
+
   function onToggleSplits() {
     if (tableRef.current) {
       splitsExpanded.dispatch({
@@ -276,8 +280,10 @@ export function AccountHeader({
               onEdit={onBatchEdit}
               onUnlink={onBatchUnlink}
               onCreateRule={onCreateRule}
+              onSetTransfer={onSetTransfer}
               onScheduleAction={onScheduleAction}
               pushModal={pushModal}
+              showMakeTransfer={showMakeTransfer}
             />
           )}
           <Button
diff --git a/packages/desktop-client/src/components/transactions/SelectedTransactions.jsx b/packages/desktop-client/src/components/transactions/SelectedTransactions.jsx
index fa4cffa4b9a..0182a5ee55a 100644
--- a/packages/desktop-client/src/components/transactions/SelectedTransactions.jsx
+++ b/packages/desktop-client/src/components/transactions/SelectedTransactions.jsx
@@ -14,8 +14,10 @@ export function SelectedTransactionsButton({
   onEdit,
   onUnlink,
   onCreateRule,
+  onSetTransfer,
   onScheduleAction,
   pushModal,
+  showMakeTransfer,
 }) {
   const selectedItems = useSelectedItems();
 
@@ -43,6 +45,29 @@ export function SelectedTransactionsButton({
     );
   }, [types.preview, selectedItems, getTransaction]);
 
+  const canBeTransfer = useMemo(() => {
+    // only two selected
+    if (selectedItems.size !== 2) {
+      return false;
+    }
+    const transactions = [...selectedItems];
+    const fromTrans = getTransaction(transactions[0]);
+    const toTrans = getTransaction(transactions[1]);
+
+    if (
+      // no subtransactions
+      // not already a transfer
+      [fromTrans, toTrans].every(tran => {
+        return tran.transfer_id == null && tran.is_child === false;
+      }) &&
+      fromTrans.account !== toTrans.account && // belong to different accounts
+      fromTrans.amount + toTrans.amount === 0 // amount must zero each other out
+    ) {
+      return true;
+    }
+    return false;
+  }, [selectedItems, getTransaction]);
+
   return (
     <SelectedItemsButton
       name="transactions"
@@ -91,6 +116,15 @@ export function SelectedTransactionsButton({
                       text: 'Create rule',
                     },
                   ]),
+              ...(showMakeTransfer
+                ? [
+                    {
+                      name: 'set-transfer',
+                      text: 'Make transfer',
+                      disabled: !canBeTransfer,
+                    },
+                  ]
+                : []),
               Menu.line,
               { type: Menu.label, name: 'Edit field' },
               { name: 'date', text: 'Date' },
@@ -145,6 +179,9 @@ export function SelectedTransactionsButton({
           case 'create-rule':
             onCreateRule([...selectedItems]);
             break;
+          case 'set-transfer':
+            onSetTransfer([...selectedItems]);
+            break;
           default:
             onEdit(name, [...selectedItems]);
         }

From fe067b78eaad5ceea33ecd50fec283283d38e6b6 Mon Sep 17 00:00:00 2001
From: DJ Mountney <david@twkie.net>
Date: Mon, 26 Feb 2024 11:40:21 -0800
Subject: [PATCH 2/3] Move common transfer logic to loot-core

---
 .../src/components/accounts/Account.jsx       | 16 +---
 .../transactions/SelectedTransactions.jsx     | 15 +---
 .../loot-core/src/client/transfer.test.ts     | 75 +++++++++++++++++++
 packages/loot-core/src/client/transfer.ts     | 19 +++++
 upcoming-release-notes/2398.md                |  6 ++
 5 files changed, 106 insertions(+), 25 deletions(-)
 create mode 100644 packages/loot-core/src/client/transfer.test.ts
 create mode 100644 packages/loot-core/src/client/transfer.ts
 create mode 100644 upcoming-release-notes/2398.md

diff --git a/packages/desktop-client/src/components/accounts/Account.jsx b/packages/desktop-client/src/components/accounts/Account.jsx
index 64f124c52c2..fa8a853ee3e 100644
--- a/packages/desktop-client/src/components/accounts/Account.jsx
+++ b/packages/desktop-client/src/components/accounts/Account.jsx
@@ -5,6 +5,7 @@ import { Navigate, useParams, useLocation, useMatch } from 'react-router-dom';
 import { debounce } from 'debounce';
 import { bindActionCreators } from 'redux';
 
+import { validForTransfer } from 'loot-core/client/transfer';
 import * as actions from 'loot-core/src/client/actions';
 import { useFilters } from 'loot-core/src/client/data-hooks/filters';
 import {
@@ -1064,25 +1065,14 @@ class AccountInternal extends PureComponent {
       this.setState({ workingHard: true });
 
       const payees = await this.props.getPayees();
-
-      // not already a transfer
-      // no subtransactions, parent or singles only
       const { data: transactions } = await runQuery(
         q('transactions')
-          .filter({ id: { $oneof: ids }, is_child: false, transfer_id: null })
+          .filter({ id: { $oneof: ids } })
           .select('*'),
       );
-
       const [fromTrans, toTrans] = transactions;
 
-      // only two selected
-      // belong to two different accounts
-      // amount must zero each other out
-      if (
-        transactions.length === 2 &&
-        fromTrans.account !== toTrans.account &&
-        fromTrans.amount + toTrans.amount === 0
-      ) {
+      if (transactions.length === 2 && validForTransfer(fromTrans, toTrans)) {
         const fromPayee = payees.find(
           p => p.transfer_acct === fromTrans.account,
         );
diff --git a/packages/desktop-client/src/components/transactions/SelectedTransactions.jsx b/packages/desktop-client/src/components/transactions/SelectedTransactions.jsx
index 0182a5ee55a..cd60ec9122d 100644
--- a/packages/desktop-client/src/components/transactions/SelectedTransactions.jsx
+++ b/packages/desktop-client/src/components/transactions/SelectedTransactions.jsx
@@ -1,5 +1,7 @@
 import React, { useMemo } from 'react';
 
+import { validForTransfer } from 'loot-core/src/client/transfer';
+
 import { useSelectedItems } from '../../hooks/useSelected';
 import { Menu } from '../common/Menu';
 import { SelectedItemsButton } from '../table';
@@ -54,18 +56,7 @@ export function SelectedTransactionsButton({
     const fromTrans = getTransaction(transactions[0]);
     const toTrans = getTransaction(transactions[1]);
 
-    if (
-      // no subtransactions
-      // not already a transfer
-      [fromTrans, toTrans].every(tran => {
-        return tran.transfer_id == null && tran.is_child === false;
-      }) &&
-      fromTrans.account !== toTrans.account && // belong to different accounts
-      fromTrans.amount + toTrans.amount === 0 // amount must zero each other out
-    ) {
-      return true;
-    }
-    return false;
+    return validForTransfer(fromTrans, toTrans);
   }, [selectedItems, getTransaction]);
 
   return (
diff --git a/packages/loot-core/src/client/transfer.test.ts b/packages/loot-core/src/client/transfer.test.ts
new file mode 100644
index 00000000000..38466059c68
--- /dev/null
+++ b/packages/loot-core/src/client/transfer.test.ts
@@ -0,0 +1,75 @@
+// @ts-strict-ignore
+import * as db from '../server/db';
+
+import * as transfer from './transfer';
+
+beforeEach(global.emptyDatabase());
+
+async function prepareDatabase() {
+  await db.insertAccount({ id: 'one', name: 'one' });
+  await db.insertAccount({ id: 'two', name: 'two' });
+  await db.insertPayee({ name: '', transfer_acct: 'one' });
+  await db.insertPayee({ name: '', transfer_acct: 'two' });
+}
+
+async function createTransaction(account: string, amount: number, extra = {}) {
+  const transaction = {
+    id: null,
+    account,
+    amount,
+    payee: await db.insertPayee({ name: 'Non-transfer ' + account }),
+    date: '2017-01-01',
+    ...extra,
+  };
+  transaction.id = await db.insertTransaction(transaction);
+  return await db.getTransaction(transaction.id);
+}
+
+describe('Transfer', () => {
+  test('Transfers are properly verified', async () => {
+    await prepareDatabase();
+
+    // happy path, two valid transactions
+    expect(
+      transfer.validForTransfer(
+        await createTransaction('one', 5),
+        await createTransaction('two', -5),
+      ),
+    ).toBeTruthy();
+
+    // amount not zeroed out
+    expect(
+      transfer.validForTransfer(
+        await createTransaction('one', 5),
+        await createTransaction('two', 5),
+      ),
+    ).toBeFalsy();
+
+    // amount not match
+    expect(
+      transfer.validForTransfer(
+        await createTransaction('one', 5),
+        await createTransaction('two', -6),
+      ),
+    ).toBeFalsy();
+
+    // accounts match
+    expect(
+      transfer.validForTransfer(
+        await createTransaction('one', 5),
+        await createTransaction('one', -5),
+      ),
+    ).toBeFalsy();
+
+    // one already a transfer
+    const existingTransfer = await createTransaction('one', 5);
+    expect(
+      transfer.validForTransfer(
+        await createTransaction('one', 5),
+        await createTransaction('two', -5, {
+          transfer_id: existingTransfer.id,
+        }),
+      ),
+    ).toBeFalsy();
+  });
+});
diff --git a/packages/loot-core/src/client/transfer.ts b/packages/loot-core/src/client/transfer.ts
new file mode 100644
index 00000000000..21f58d5fde2
--- /dev/null
+++ b/packages/loot-core/src/client/transfer.ts
@@ -0,0 +1,19 @@
+import type { TransactionEntity } from '../types/models';
+
+export function validForTransfer(
+  fromTransaction: TransactionEntity,
+  toTransaction: TransactionEntity,
+) {
+  if (
+    // no subtransactions
+    // not already a transfer
+    [fromTransaction, toTransaction].every(tran => {
+      return tran.transfer_id == null && tran.is_child === false;
+    }) &&
+    fromTransaction.account !== toTransaction.account && // belong to different accounts
+    fromTransaction.amount + toTransaction.amount === 0 // amount must zero each other out
+  ) {
+    return true;
+  }
+  return false;
+}
diff --git a/upcoming-release-notes/2398.md b/upcoming-release-notes/2398.md
new file mode 100644
index 00000000000..5fbebe1cf72
--- /dev/null
+++ b/upcoming-release-notes/2398.md
@@ -0,0 +1,6 @@
+---
+category: Features
+authors: [twk3]
+---
+
+Add option to make a transfer from two selected transactions.

From 6a3f3ad2c1d8150ca5815bdcc9ba7ec4af4f3be8 Mon Sep 17 00:00:00 2001
From: DJ Mountney <david@twkie.net>
Date: Mon, 26 Feb 2024 16:24:15 -0800
Subject: [PATCH 3/3] Add end-to-end test for make transfer

---
 packages/desktop-client/e2e/accounts.test.js  | 51 ++++++++++++++++++-
 .../e2e/page-models/account-page.js           | 30 +++++++++++
 .../desktop-client/src/components/table.tsx   |  2 +
 .../transactions/SelectedTransactions.jsx     |  5 ++
 4 files changed, 86 insertions(+), 2 deletions(-)

diff --git a/packages/desktop-client/e2e/accounts.test.js b/packages/desktop-client/e2e/accounts.test.js
index 45ca7096a28..93029854ef8 100644
--- a/packages/desktop-client/e2e/accounts.test.js
+++ b/packages/desktop-client/e2e/accounts.test.js
@@ -7,6 +7,7 @@ test.describe('Accounts', () => {
   let page;
   let navigation;
   let configurationPage;
+  let accountPage;
 
   test.beforeAll(async ({ browser }) => {
     page = await browser.newPage();
@@ -22,7 +23,7 @@ test.describe('Accounts', () => {
   });
 
   test('creates a new account and views the initial balance transaction', async () => {
-    const accountPage = await navigation.createAccount({
+    accountPage = await navigation.createAccount({
       name: 'New Account',
       offBudget: false,
       balance: 100,
@@ -38,7 +39,7 @@ test.describe('Accounts', () => {
   });
 
   test('closes an account', async () => {
-    const accountPage = await navigation.goToAccountPage('Roth IRA');
+    accountPage = await navigation.goToAccountPage('Roth IRA');
 
     await expect(accountPage.accountName).toHaveText('Roth IRA');
 
@@ -50,4 +51,50 @@ test.describe('Accounts', () => {
     await expect(accountPage.accountName).toHaveText('Closed: Roth IRA');
     await expect(page).toMatchThemeScreenshots();
   });
+
+  test.describe('Budgeted Accounts', () => {
+    // Reset filters
+    test.afterEach(async () => {
+      await accountPage.removeFilter(0);
+    });
+
+    test('creates a transfer from two existing transactions', async () => {
+      accountPage = await navigation.goToAccountPage('For budget');
+      await expect(accountPage.accountName).toHaveText('Budgeted Accounts');
+
+      await accountPage.filterByNote('Test Acc Transfer');
+
+      await accountPage.createSingleTransaction({
+        account: 'Ally Savings',
+        payee: '',
+        notes: 'Test Acc Transfer',
+        category: 'Food',
+        debit: '34.56',
+      });
+
+      await accountPage.createSingleTransaction({
+        account: 'HSBC',
+        payee: '',
+        notes: 'Test Acc Transfer',
+        category: 'Food',
+        credit: '34.56',
+      });
+
+      await accountPage.selectNthTransaction(0);
+      await accountPage.selectNthTransaction(1);
+      await accountPage.clickSelectAction('Make transfer');
+
+      let transaction = accountPage.getNthTransaction(0);
+      await expect(transaction.payee).toHaveText('Ally Savings');
+      await expect(transaction.category).toHaveText('Transfer');
+      await expect(transaction.credit).toHaveText('34.56');
+      await expect(transaction.account).toHaveText('HSBC');
+
+      transaction = accountPage.getNthTransaction(1);
+      await expect(transaction.payee).toHaveText('HSBC');
+      await expect(transaction.category).toHaveText('Transfer');
+      await expect(transaction.debit).toHaveText('34.56');
+      await expect(transaction.account).toHaveText('Ally Savings');
+    });
+  });
 });
diff --git a/packages/desktop-client/e2e/page-models/account-page.js b/packages/desktop-client/e2e/page-models/account-page.js
index cabd83f9535..6edf8588560 100644
--- a/packages/desktop-client/e2e/page-models/account-page.js
+++ b/packages/desktop-client/e2e/page-models/account-page.js
@@ -25,6 +25,9 @@ export class AccountPage {
 
     this.filterButton = this.page.getByRole('button', { name: 'Filter' });
     this.filterSelectTooltip = this.page.getByTestId('filters-select-tooltip');
+
+    this.selectButton = this.page.getByTestId('transactions-select-button');
+    this.selectTooltip = this.page.getByTestId('transactions-select-tooltip');
   }
 
   /**
@@ -68,14 +71,21 @@ export class AccountPage {
     await this.cancelTransactionButton.click();
   }
 
+  async selectNthTransaction(index) {
+    const row = this.transactionTableRow.nth(index);
+    await row.getByTestId('select').click();
+  }
+
   /**
    * Retrieve the data for the nth-transaction.
    * 0-based index
    */
   getNthTransaction(index) {
     const row = this.transactionTableRow.nth(index);
+    const account = row.getByTestId('account');
 
     return {
+      ...(account ? { account } : {}),
       payee: row.getByTestId('payee'),
       notes: row.getByTestId('notes'),
       category: row.getByTestId('category'),
@@ -84,6 +94,11 @@ export class AccountPage {
     };
   }
 
+  async clickSelectAction(action) {
+    await this.selectButton.click();
+    await this.selectTooltip.getByRole('button', { name: action }).click();
+  }
+
   /**
    * Open the modal for closing the account.
    */
@@ -106,6 +121,15 @@ export class AccountPage {
     return new FilterTooltip(this.page.getByTestId('filters-menu-tooltip'));
   }
 
+  /**
+   * Filter to a specific note
+   */
+  async filterByNote(note) {
+    const filterTooltip = await this.filterBy('Note');
+    await this.page.keyboard.type(note);
+    await filterTooltip.applyButton.click();
+  }
+
   /**
    * Remove the nth filter
    */
@@ -117,6 +141,12 @@ export class AccountPage {
   }
 
   async _fillTransactionFields(transactionRow, transaction) {
+    if (transaction.account) {
+      await transactionRow.getByTestId('account').click();
+      await this.page.keyboard.type(transaction.account);
+      await this.page.keyboard.press('Tab');
+    }
+
     if (transaction.payee) {
       await transactionRow.getByTestId('payee').click();
       await this.page.keyboard.type(transaction.payee);
diff --git a/packages/desktop-client/src/components/table.tsx b/packages/desktop-client/src/components/table.tsx
index e3a51e34355..38cc9fc919f 100644
--- a/packages/desktop-client/src/components/table.tsx
+++ b/packages/desktop-client/src/components/table.tsx
@@ -801,6 +801,7 @@ export function SelectedItemsButton({ name, keyHandlers, items, onSelect }) {
         type="bare"
         style={{ color: theme.pageTextPositive }}
         onClick={() => setMenuOpen(true)}
+        data-testid={name + '-select-button'}
       >
         <SvgExpandArrow
           width={8}
@@ -816,6 +817,7 @@ export function SelectedItemsButton({ name, keyHandlers, items, onSelect }) {
           width={200}
           style={{ padding: 0, backgroundColor: theme.menuBackground }}
           onClose={() => setMenuOpen(false)}
+          data-testid={name + '-select-tooltip'}
         >
           <Menu
             onMenuSelect={name => {
diff --git a/packages/desktop-client/src/components/transactions/SelectedTransactions.jsx b/packages/desktop-client/src/components/transactions/SelectedTransactions.jsx
index cd60ec9122d..35047032755 100644
--- a/packages/desktop-client/src/components/transactions/SelectedTransactions.jsx
+++ b/packages/desktop-client/src/components/transactions/SelectedTransactions.jsx
@@ -56,6 +56,11 @@ export function SelectedTransactionsButton({
     const fromTrans = getTransaction(transactions[0]);
     const toTrans = getTransaction(transactions[1]);
 
+    // previously selected transactions aren't always present in current transaction list
+    if (!fromTrans || !toTrans) {
+      return false;
+    }
+
     return validForTransfer(fromTrans, toTrans);
   }, [selectedItems, getTransaction]);