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

Include more information in payee of split parent #3049

Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion packages/desktop-client/e2e/rules.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ test.describe('Rules', () => {
});

const transaction = accountPage.getNthTransaction(0);
await expect(transaction.payee).toHaveText('Split');
await expect(transaction.payee).toHaveText('Ikea');
await expect(transaction.notes).toHaveText('food / entertainment');
await expect(transaction.category).toHaveText('Split');
await expect(transaction.debit).toHaveText('100.00');
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion packages/desktop-client/e2e/transactions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ test.describe('Transactions', () => {
]);

const firstTransaction = accountPage.getNthTransaction(0);
await expect(firstTransaction.payee).toHaveText('Split');
await expect(firstTransaction.payee).toHaveText('Krogger');
await expect(firstTransaction.notes).toHaveText('Notes');
await expect(firstTransaction.category).toHaveText('Split');
await expect(firstTransaction.debit).toHaveText('333.33');
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,10 @@ const TransactionHeader = memo(

TransactionHeader.displayName = 'TransactionHeader';

function getPayeePretty(transaction, payee, transferAcct) {
function getPayeePretty(transaction, payee, transferAcct, numHiddenPayees = 0) {
const formatPayeeName = payeeName =>
numHiddenPayees > 0 ? `${payeeName} (+${numHiddenPayees} more)` : payeeName;

const { payee: payeeId } = transaction;

if (transferAcct) {
Expand All @@ -334,14 +337,14 @@ function getPayeePretty(transaction, payee, transferAcct) {
textOverflow: 'ellipsis',
}}
>
{transferAcct.name}
{formatPayeeName(transferAcct.name)}
</div>
</View>
);
} else if (payee) {
return payee.name;
return formatPayeeName(payee.name);
} else if (payeeId && payeeId.startsWith('new:')) {
return payeeId.slice('new:'.length);
return formatPayeeName(payeeId.slice('new:'.length));
}

return '';
Expand Down Expand Up @@ -472,15 +475,59 @@ function HeaderCell({
);
}

const useParentPayee = (
payees,
subtransactions,
transferAccountsByTransaction,
) =>
useMemo(() => {
if (!subtransactions) {
return null;
}

const { counts, mostCommonPayeeTransaction } =
subtransactions?.reduce(
({ counts, ...result }, sub) => {
if (sub.payee) {
counts[sub.payee] = (counts[sub.payee] || 0) + 1;
if (counts[sub.payee] > result.maxCount) {
return {
counts,
maxCount: counts[sub.payee],
mostCommonPayeeTransaction: sub,
};
}
}
return { counts, ...result };
},
{ counts: {}, maxCount: 0, mostCommonPayeeTransaction: null },
) || {};

if (!mostCommonPayeeTransaction) {
return 'Split (no payee)';
}

const mostCommonPayee =
getPayeesById(payees)[mostCommonPayeeTransaction.payee];
const numDistinctPayees = Object.keys(counts).length;
return getPayeePretty(
mostCommonPayeeTransaction,
mostCommonPayee,
transferAccountsByTransaction[mostCommonPayeeTransaction.id],
numDistinctPayees - 1,
);
}, [subtransactions, payees, transferAccountsByTransaction]);

function PayeeCell({
id,
payee,
focused,
payees,
accounts,
transferAccountsByTransaction,
valueStyle,
transaction,
transferAcct,
subtransactions,
isPreview,
onEdit,
onUpdate,
Expand All @@ -493,6 +540,14 @@ function PayeeCell({

const dispatch = useDispatch();

const parentPayee = useParentPayee(
payees,
subtransactions,
transferAccountsByTransaction,
);

const transferAccount = transferAccountsByTransaction[transaction.id];

return transaction.is_parent ? (
<Cell
name="payee"
Expand Down Expand Up @@ -538,7 +593,7 @@ function PayeeCell({
color: 'inherit',
width: 14,
height: 14,
marginRight: 2,
marginRight: 5,
}}
/>
<Text
Expand All @@ -548,7 +603,7 @@ function PayeeCell({
userSelect: 'none',
}}
>
Split
{parentPayee}
</Text>
</View>
</CellButton>
Expand All @@ -572,12 +627,12 @@ function PayeeCell({
isCreatingPayee.current = false;
}
}}
formatter={() => getPayeePretty(transaction, payee, transferAcct)}
formatter={() => getPayeePretty(transaction, payee, transferAccount)}
unexposedContent={props => (
<>
<PayeeIcons
transaction={transaction}
transferAccount={transferAcct}
transferAccount={transferAccount}
onNavigateToTransferAccount={onNavigateToTransferAccount}
onNavigateToSchedule={onNavigateToSchedule}
/>
Expand Down Expand Up @@ -695,6 +750,7 @@ const Transaction = memo(function Transaction({
allTransactions,
transaction: originalTransaction,
subtransactions,
transferAccountsByTransaction,
editing,
showAccount,
showBalance,
Expand Down Expand Up @@ -872,19 +928,9 @@ const Transaction = memo(function Transaction({
// Join in some data
const payee = payees && payeeId && getPayeesById(payees)[payeeId];
const account = accounts && accountId && getAccountsById(accounts)[accountId];
let transferAcct;

if (_inverse) {
transferAcct =
accounts && accountId && getAccountsById(accounts)[accountId];
} else {
transferAcct =
payee &&
payee.transfer_acct &&
getAccountsById(accounts)[payee.transfer_acct];
}

const isChild = transaction.is_child;
const transferAcct = transferAccountsByTransaction[id];
const isBudgetTransfer = transferAcct && transferAcct.offbudget === 0;
const isOffBudget = account && account.offbudget === 1;

Expand Down Expand Up @@ -1116,7 +1162,8 @@ const Transaction = memo(function Transaction({
payees={payees.filter(payee => payee.transfer_acct !== accountId)}
valueStyle={valueStyle}
transaction={transaction}
transferAcct={transferAcct}
subtransactions={subtransactions}
transferAccountsByTransaction={transferAccountsByTransaction}
importedPayee={importedPayee}
isPreview={isPreview}
onEdit={onEdit}
Expand Down Expand Up @@ -1510,6 +1557,7 @@ function NewTransaction({
accounts,
categoryGroups,
payees,
transferAccountsByTransaction,
editingTransaction,
focusedField,
showAccount,
Expand Down Expand Up @@ -1562,6 +1610,7 @@ function NewTransaction({
editing={editingTransaction === transaction.id}
transaction={transaction}
subtransactions={transaction.is_parent ? childTransactions : null}
transferAccountsByTransaction={transferAccountsByTransaction}
showAccount={showAccount}
showCategory={showCategory}
showBalance={showBalance}
Expand Down Expand Up @@ -1716,17 +1765,20 @@ function TransactionTableInner({
error &&
error.type === 'SplitTransactionError';

const emptyChildTransactions = transactions.filter(
t =>
t.parent_id === (trans.is_parent ? trans.id : trans.parent_id) &&
t.amount === 0,
);
const childTransactions = trans.is_parent
? props.transactionsByParent[trans.id]
: null;
const emptyChildTransactions = props.transactionsByParent[
trans.is_parent ? trans.id : trans.parent_id
]?.filter(t => t.amount === 0);

return (
<Transaction
allTransactions={props.transactions}
editing={editing}
transaction={trans}
transferAccountsByTransaction={props.transferAccountsByTransaction}
subtransactions={childTransactions}
showAccount={showAccount}
showCategory={showCategory}
showBalance={showBalances}
Expand Down Expand Up @@ -1802,6 +1854,9 @@ function TransactionTableInner({
>
<NewTransaction
transactions={props.newTransactions}
transferAccountsByTransaction={
props.transferAccountsByTransaction
}
editingTransaction={newNavigator.editingId}
focusedField={newNavigator.focusedField}
accounts={props.accounts}
Expand Down Expand Up @@ -1885,7 +1940,7 @@ export const TransactionTable = forwardRef((props, ref) => {
const listContainerRef = useRef(null);
const mergedRef = useMergedRefs(tableRef, ref);

const transactions = useMemo(() => {
const transactionsWithExpandedSplits = useMemo(() => {
let result;
if (splitsExpanded.state.transitionId != null) {
const index = props.transactions.findIndex(
Expand Down Expand Up @@ -1923,8 +1978,44 @@ export const TransactionTable = forwardRef((props, ref) => {
return result;
}, [props.transactions, splitsExpanded]);
const transactionMap = useMemo(() => {
return new Map(transactions.map(trans => [trans.id, trans]));
}, [transactions]);
return new Map(
transactionsWithExpandedSplits.map(trans => [trans.id, trans]),
);
}, [transactionsWithExpandedSplits]);
const transactionsByParent = useMemo(() => {
return props.transactions.reduce((acc, trans) => {
if (trans.is_child) {
acc[trans.parent_id] = [...(acc[trans.parent_id] ?? []), trans];
}
return acc;
}, {});
}, [props.transactions]);

const transferAccountsByTransaction = useMemo(() => {
if (!props.accounts) {
return {};
}
const accounts = getAccountsById(props.accounts);
const payees = getPayeesById(props.payees);

return Object.fromEntries(
props.transactions.map(t => {
if (!props.accounts) {
return [t.id, null];
}

const payee = t.payee && payees[t.payee];
let transferAccount;
if (t._inverse) {
transferAccount = t.account && accounts[t.account];
} else {
transferAccount =
payee?.transfer_acct && accounts[payee.transfer_acct];
}
return [t.id, transferAccount || null];
}),
);
}, [props.transactions, props.payees, props.accounts]);

useEffect(() => {
// If it's anchored that means we've also disabled animations. To
Expand All @@ -1937,7 +2028,10 @@ export const TransactionTable = forwardRef((props, ref) => {
}, [prevSplitsExpanded.current]);

const newNavigator = useTableNavigator(newTransactions, getFields);
const tableNavigator = useTableNavigator(transactions, getFields);
const tableNavigator = useTableNavigator(
transactionsWithExpandedSplits,
getFields,
);
const shouldAdd = useRef(false);
const latestState = useRef({ newTransactions, newNavigator, tableNavigator });
const savePending = useRef(false);
Expand Down Expand Up @@ -2297,8 +2391,10 @@ export const TransactionTable = forwardRef((props, ref) => {
tableRef={mergedRef}
listContainerRef={listContainerRef}
{...props}
transactions={transactions}
transactions={transactionsWithExpandedSplits}
transactionMap={transactionMap}
transactionsByParent={transactionsByParent}
transferAccountsByTransaction={transferAccountsByTransaction}
selectedItems={selectedItems}
isExpanded={splitsExpanded.expanded}
onSave={onSave}
Expand Down
6 changes: 6 additions & 0 deletions upcoming-release-notes/3049.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
category: Enhancements
authors: [jfdoming]
---

Include more information in payee of split parent
Loading