diff --git a/packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.test.tsx b/packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.test.tsx new file mode 100644 index 00000000000..0bc3f317b3a --- /dev/null +++ b/packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.test.tsx @@ -0,0 +1,239 @@ +import { render, type Screen, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { vi } from 'vitest'; + +import { generateAccount } from 'loot-core/src/mocks'; +import { TestProvider } from 'loot-core/src/mocks/redux'; +import type { AccountEntity, PayeeEntity } from 'loot-core/types/models'; + +import { useCommonPayees } from '../../hooks/usePayees'; +import { ResponsiveProvider } from '../../ResponsiveProvider'; + +import { + PayeeAutocomplete, + type PayeeAutocompleteItem, + type PayeeAutocompleteProps, +} from './PayeeAutocomplete'; + +const PAYEE_SELECTOR = '[data-testid][role=option]'; +const PAYEE_SECTION_SELECTOR = '[data-testid$="-item-group"]'; + +const payees = [ + makePayee('Bob', { favorite: true }), + makePayee('Alice', { favorite: true }), + makePayee('This guy on the side of the road'), +]; + +const accounts: AccountEntity[] = [ + generateAccount('Bank of Montreal', false, false), +]; +const defaultProps = { + value: null, + embedded: true, + payees, + accounts, +}; + +function makePayee(name: string, options?: { favorite: boolean }): PayeeEntity { + return { + id: name.toLowerCase() + '-id', + name, + favorite: options?.favorite ?? false, + transfer_acct: undefined, + }; +} + +function extractPayeesAndHeaderNames(screen: Screen) { + return [ + ...screen + .getByTestId('autocomplete') + .querySelectorAll(`${PAYEE_SELECTOR}, ${PAYEE_SECTION_SELECTOR}`), + ] + .map(e => e.getAttribute('data-testid')) + .map(firstOrIncorrect); +} + +function renderPayeeAutocomplete( + props?: Partial, +): HTMLElement { + const autocompleteProps = { + ...defaultProps, + ...props, + }; + + render( + + +
+ +
+ + , + ); + return screen.getByTestId('autocomplete-test'); +} + +// Not good, see `Autocomplete.js` for details +function waitForAutocomplete() { + return new Promise(resolve => setTimeout(resolve, 0)); +} + +async function clickAutocomplete(autocomplete: HTMLElement) { + const input = autocomplete.querySelector(`input`); + if (input != null) { + await userEvent.click(input); + } + await waitForAutocomplete(); +} + +vi.mock('../../hooks/usePayees', () => ({ + useCommonPayees: vi.fn(), + usePayees: vi.fn().mockReturnValue([]), +})); + +function firstOrIncorrect(id: string | null): string { + return id?.split('-', 1)[0] || 'incorrect'; +} + +describe('PayeeAutocomplete.getPayeeSuggestions', () => { + beforeEach(() => { + vi.mocked(useCommonPayees).mockReturnValue([]); + }); + + test('favorites get sorted alphabetically', async () => { + const autocomplete = renderPayeeAutocomplete(); + await clickAutocomplete(autocomplete); + + expect( + [ + ...screen.getByTestId('autocomplete').querySelectorAll(PAYEE_SELECTOR), + ].map(e => e.getAttribute('data-testid')), + ).toStrictEqual([ + 'Alice-payee-item', + 'Bob-payee-item', + 'This guy on the side of the road-payee-item', + ]); + }); + + test('list with less than the maximum favorites adds common payees', async () => { + //Note that the payees list assumes the payees are already sorted + const payees: PayeeAutocompleteItem[] = [ + makePayee('Alice'), + makePayee('Bob'), + makePayee('Eve', { favorite: true }), + makePayee('Bruce'), + makePayee('Carol'), + makePayee('Natasha'), + makePayee('Steve'), + makePayee('Tony'), + ]; + vi.mocked(useCommonPayees).mockReturnValue([ + makePayee('Bruce'), + makePayee('Natasha'), + makePayee('Steve'), + makePayee('Tony'), + makePayee('Carol'), + ]); + const expectedPayeeOrder = [ + 'Suggested Payees', + 'Eve', + 'Bruce', + 'Natasha', + 'Steve', + 'Tony', + 'Payees', + 'Alice', + 'Bob', + 'Carol', + ]; + await clickAutocomplete(renderPayeeAutocomplete({ payees })); + + expect( + [ + ...screen + .getByTestId('autocomplete') + .querySelectorAll(`${PAYEE_SELECTOR}, ${PAYEE_SECTION_SELECTOR}`), + ] + .map(e => e.getAttribute('data-testid')) + .map(firstOrIncorrect), + ).toStrictEqual(expectedPayeeOrder); + }); + + test('list with more than the maximum favorites only lists favorites', async () => { + //Note that the payees list assumes the payees are already sorted + const payees = [ + makePayee('Alice', { favorite: true }), + makePayee('Bob', { favorite: true }), + makePayee('Eve', { favorite: true }), + makePayee('Bruce', { favorite: true }), + makePayee('Carol', { favorite: true }), + makePayee('Natasha'), + makePayee('Steve'), + makePayee('Tony', { favorite: true }), + ]; + vi.mocked(useCommonPayees).mockReturnValue([ + makePayee('Bruce'), + makePayee('Natasha'), + makePayee('Steve'), + makePayee('Tony'), + makePayee('Carol'), + ]); + const expectedPayeeOrder = [ + 'Suggested Payees', + 'Alice', + 'Bob', + 'Bruce', + 'Carol', + 'Eve', + 'Tony', + 'Payees', + 'Natasha', + 'Steve', + ]; + const autocomplete = renderPayeeAutocomplete({ payees }); + await clickAutocomplete(autocomplete); + + expect(extractPayeesAndHeaderNames(screen)).toStrictEqual( + expectedPayeeOrder, + ); + }); + + test('list with no favorites shows just the payees list', async () => { + //Note that the payees list assumes the payees are already sorted + const payees = [ + makePayee('Alice'), + makePayee('Bob'), + makePayee('Eve'), + makePayee('Natasha'), + makePayee('Steve'), + ]; + const expectedPayeeOrder = ['Alice', 'Bob', 'Eve', 'Natasha', 'Steve']; + const autocomplete = renderPayeeAutocomplete({ payees }); + await clickAutocomplete(autocomplete); + + expect( + [ + ...screen + .getByTestId('autocomplete') + .querySelectorAll('[data-testid][role=option]'), + ] + .map(e => e.getAttribute('data-testid')) + .flatMap(firstOrIncorrect), + ).toStrictEqual(expectedPayeeOrder); + expect( + [ + ...screen + .getByTestId('autocomplete') + .querySelectorAll('[data-testid$="-item-group"]'), + ] + .map(e => e.getAttribute('data-testid')) + .flatMap(firstOrIncorrect), + ).toStrictEqual(['Payees']); + }); +}); diff --git a/packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx b/packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx index 55b452ad5f7..ea4d4791394 100644 --- a/packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx +++ b/packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx @@ -39,7 +39,7 @@ import { } from './Autocomplete'; import { ItemHeader } from './ItemHeader'; -type PayeeAutocompleteItem = PayeeEntity; +export type PayeeAutocompleteItem = PayeeEntity; const MAX_AUTO_SUGGESTIONS = 5; @@ -47,30 +47,37 @@ function getPayeeSuggestions( commonPayees: PayeeAutocompleteItem[], payees: PayeeAutocompleteItem[], ): (PayeeAutocompleteItem & PayeeItemType)[] { + const favoritePayees = payees + .filter(p => p.favorite) + .map(p => { + return { ...p, itemType: determineItemType(p, true) }; + }) + .sort((a, b) => a.name.localeCompare(b.name)); + + let additionalCommonPayees: (PayeeAutocompleteItem & PayeeItemType)[] = []; if (commonPayees?.length > 0) { - const favoritePayees = payees.filter(p => p.favorite); - let additionalCommonPayees: PayeeAutocompleteItem[] = []; if (favoritePayees.length < MAX_AUTO_SUGGESTIONS) { additionalCommonPayees = commonPayees .filter( p => !(p.favorite || favoritePayees.map(fp => fp.id).includes(p.id)), ) - .slice(0, MAX_AUTO_SUGGESTIONS - favoritePayees.length); + .slice(0, MAX_AUTO_SUGGESTIONS - favoritePayees.length) + .map(p => { + return { ...p, itemType: determineItemType(p, true) }; + }) + .sort((a, b) => a.name.localeCompare(b.name)); } - const frequentPayees: (PayeeAutocompleteItem & PayeeItemType)[] = - favoritePayees.concat(additionalCommonPayees).map(p => { - return { ...p, itemType: 'common_payee' }; - }); + } + if (favoritePayees.length + additionalCommonPayees.length) { const filteredPayees: (PayeeAutocompleteItem & PayeeItemType)[] = payees - .filter(p => !frequentPayees.find(fp => fp.id === p.id)) + .filter(p => !favoritePayees.find(fp => fp.id === p.id)) + .filter(p => !additionalCommonPayees.find(fp => fp.id === p.id)) .map(p => { return { ...p, itemType: determineItemType(p, false) }; }); - return frequentPayees - .sort((a, b) => a.name.localeCompare(b.name)) - .concat(filteredPayees); + return favoritePayees.concat(additionalCommonPayees).concat(filteredPayees); } return payees.map(p => { @@ -245,7 +252,7 @@ function PayeeList({ ); } -type PayeeAutocompleteProps = ComponentProps< +export type PayeeAutocompleteProps = ComponentProps< typeof Autocomplete > & { showMakeTransfer?: boolean; diff --git a/packages/desktop-client/src/components/transactions/TransactionsTable.test.jsx b/packages/desktop-client/src/components/transactions/TransactionsTable.test.jsx index c9abf7956f1..a9644f97704 100644 --- a/packages/desktop-client/src/components/transactions/TransactionsTable.test.jsx +++ b/packages/desktop-client/src/components/transactions/TransactionsTable.test.jsx @@ -37,8 +37,27 @@ vi.mock('../../hooks/useSyncedPref', () => ({ const accounts = [generateAccount('Bank of America')]; const payees = [ - { id: 'payed-to', favorite: true, name: 'Payed To' }, - { id: 'guy', favorite: false, name: 'This guy on the side of the road' }, + { + id: 'bob-id', + name: 'Bob', + favorite: true, + transfer_acct: null, + category: null, + }, + { + id: 'alice-id', + name: 'Alice', + favorite: true, + transfer_acct: null, + category: null, + }, + { + id: 'guy', + favorite: false, + transfer_acct: null, + category: null, + name: 'This guy on the side of the road', + }, ]; const categoryGroups = generateCategoryGroups([ { @@ -67,6 +86,7 @@ function generateTransactions(count, splitAtIndexes = [], showError = false) { generateTransaction( { account: accounts[0].id, + payee: 'alice-id', category: i === 0 ? null @@ -284,6 +304,41 @@ function editField(container, name, rowIndex) { return _editField(field, container); } +expect.extend({ + payeesToHaveFavoriteStars(container, validPayeeListWithFavorite) { + const incorrectStarList = []; + const foundStarList = []; + validPayeeListWithFavorite.forEach(payeeItem => { + const shouldHaveFavorite = payeeItem != null; + let found = false; + if (container[0].querySelectorAll('svg').length === 1) { + found = true; + foundStarList.push(payeeItem); + } + if (shouldHaveFavorite !== found) { + incorrectStarList.push(payeeItem); + } + }); + if ( + foundStarList.length !== validPayeeListWithFavorite.length || + incorrectStarList.length > 0 + ) { + return { + message: () => + `Expected ${validPayeeListWithFavorite.join(', ')} to have favorite stars.` + + `Received ${foundStarList.length} items with favorite stars. Incorrect: ${incorrectStarList.join(', ')}`, + pass: false, + }; + } else { + return { + message: () => + `Expected ${validPayeeListWithFavorite} to have favorite stars`, + pass: true, + }; + } + }, +}); + function expectToBeEditingField(container, name, rowIndex, isNew) { let field; if (isNew) { @@ -590,6 +645,54 @@ describe('Transactions', () => { ); }); + test('dropdown payee displays on new transaction with account list column', async () => { + const { container, updateProps, queryByTestId } = renderTransactions({ + currentAccountId: null, + }); + updateProps({ isAdding: true }); + expect(queryByTestId('new-transaction')).toBeTruthy(); + + await editNewField(container, 'payee'); + + const renderedPayees = screen + .getByTestId('autocomplete') + .querySelectorAll('[data-testid$="payee-item"]'); + + expect( + Array.from(renderedPayees.values()).map(p => + p.getAttribute('data-testid'), + ), + ).toStrictEqual([ + 'Alice-payee-item', + 'Bob-payee-item', + 'This guy on the side of the road-payee-item', + ]); + expect(renderedPayees).payeesToHaveFavoriteStars([ + 'Alice-payee-item', + 'Bob-payee-item', + ]); + }); + + test('dropdown payee displays on existing non-transfer transaction', async () => { + const { container } = renderTransactions(); + + await editField(container, 'payee', 2); + + const renderedPayees = screen + .getByTestId('autocomplete') + .querySelectorAll('[data-testid$="payee-item"]'); + + expect( + Array.from(renderedPayees.values()).map(p => + p.getAttribute('data-testid'), + ), + ).toStrictEqual([ + 'Alice-payee-item', + 'Bob-payee-item', + 'This guy on the side of the road-payee-item', + ]); + }); + // TODO: fix this test test.skip('dropdown invalid value resets correctly', async () => { const { container, getTransactions } = renderTransactions(); @@ -866,7 +969,7 @@ describe('Transactions', () => { id: expect.any(String), is_parent: true, notes: 'Notes', - payee: 'payed-to', + payee: 'alice-id', sort_order: 0, }, { @@ -879,7 +982,7 @@ describe('Transactions', () => { id: expect.any(String), is_child: true, parent_id: parentId, - payee: 'payed-to', + payee: 'alice-id', sort_order: -1, starting_balance_flag: null, }, @@ -893,7 +996,7 @@ describe('Transactions', () => { id: expect.any(String), is_child: true, parent_id: parentId, - payee: 'payed-to', + payee: 'alice-id', sort_order: -2, starting_balance_flag: null, }, diff --git a/packages/loot-core/src/mocks/index.ts b/packages/loot-core/src/mocks/index.ts index e42823ed7c8..e04cbb7db51 100644 --- a/packages/loot-core/src/mocks/index.ts +++ b/packages/loot-core/src/mocks/index.ts @@ -2,20 +2,44 @@ import { v4 as uuidv4 } from 'uuid'; import * as monthUtils from '../shared/months'; -import type { TransactionEntity } from '../types/models'; +import type { + _SyncFields, + AccountEntity, + TransactionEntity, +} from '../types/models'; import { random } from './random'; -export function generateAccount(name, isConnected, offbudget) { +export function generateAccount( + name, + isConnected, + offbudget, +): AccountEntity & { bankId: number; bankName: string } { return { id: uuidv4(), name, balance_current: isConnected ? Math.floor(random() * 100000) : null, - bank: isConnected ? Math.floor(random() * 10000) : null, bankId: isConnected ? Math.floor(random() * 10000) : null, bankName: isConnected ? 'boa' : null, + bank: isConnected ? Math.floor(random() * 10000).toString() : null, offbudget: offbudget ? 1 : 0, + sort_order: 0, + tombstone: 0, closed: 0, + ...emptySyncFields(), + }; +} + +function emptySyncFields(): _SyncFields { + return { + account_id: null, + bank: null, + mask: null, + official_name: null, + balance_current: null, + balance_available: null, + balance_limit: null, + account_sync_source: null, }; } diff --git a/upcoming-release-notes/3412.md b/upcoming-release-notes/3412.md new file mode 100644 index 00000000000..7647c9a523c --- /dev/null +++ b/upcoming-release-notes/3412.md @@ -0,0 +1,6 @@ +--- +category: Bugfix +authors: [qedi-r] +--- + +Sort suggested payee popup section by favorite status first, then alphabetically.