diff --git a/src/components/modals/TransactionDetailsModal.tsx b/src/components/modals/TransactionDetailsModal.tsx index bc3d286..2ca1f6e 100644 --- a/src/components/modals/TransactionDetailsModal.tsx +++ b/src/components/modals/TransactionDetailsModal.tsx @@ -1,26 +1,36 @@ import { Ionicons } from '@expo/vector-icons'; -import { useLocalSearchParams } from 'expo-router'; -import React, { useMemo } from 'react'; +import { Stack, router, useLocalSearchParams } from 'expo-router'; +import React, { useMemo, useRef } from 'react'; import { useTranslation } from 'react-i18next'; -import { ActivityIndicator, StyleSheet, Text, View } from 'react-native'; +import { ActivityIndicator, Alert, StyleSheet, Text, TouchableOpacity, View } from 'react-native'; import { EmptyState } from '../../components/EmptyState'; +import { + TransactionPresenter, + resolveTransactionCategoryLabel, + resolveWalletLabel, +} from '../../components/transactions/TransactionPresenter'; import { useCategories } from '../../hooks/useCategories'; import { useFormatting } from '../../hooks/useFormatting'; import { useTransactions } from '../../hooks/useTransactions'; import { useWallets } from '../../hooks/useWallets'; import { useTheme } from '../../theme/theme'; -import { resolveCategoryVisual } from '../../utils/categoryVisuals'; +import { isTransferCategoryId } from '../../utils/categoryVisuals'; export function TransactionDetailsModal() { const { id } = useLocalSearchParams<{ id: string }>(); const { t } = useTranslation(); - const { colors, typography, spacing, radius } = useTheme(); + const { colors, spacing, radius } = useTheme(); - const { transactions, loading } = useTransactions(); + const { transactions, loading, deleteTransaction } = useTransactions(); const { categories } = useCategories(); const { wallets } = useWallets(); - const { formatAmount, formatDate } = useFormatting(); + const { formatDate, formatAmount } = useFormatting(); + + // Once the delete is confirmed this screen is on its way out. The refreshed + // list drops the row before the dismiss finishes, so without this flag the + // "not found" branch below would flash an error over a successful delete. + const deletingRef = useRef(false); const transaction = useMemo(() => { return transactions.find((tx) => tx.id === id); // `useLocalSearchParams` automatically resolves `[id]` @@ -35,6 +45,7 @@ export function TransactionDetailsModal() { } if (!transaction) { + if (deletingRef.current) return null; return ( c.id === transaction.categoryId); - if (cat) { - // Stored icon/colour first, so a category renamed into any language keeps - // its appearance. See utils/categoryVisuals. - categoryName = cat.name; - const visual = resolveCategoryVisual(cat, colors.accent); - iconName = visual.icon; - colorHex = visual.color; - } - } - - const wallet = wallets.find((w) => w.id === transaction.walletId); - const walletName = wallet?.name || t('components.transactionList.unknown'); - - const valueColor = isTransfer ? colors.foreground : isIncome ? colors.success : colors.foreground; + const isTransfer = transaction.type === 'transfer' || isTransferCategoryId(transaction.categoryId); + const categoryName = resolveTransactionCategoryLabel(transaction, categories, t); + const walletName = resolveWalletLabel(transaction.walletId, wallets, t); + + const handleDelete = () => { + Alert.alert( + t('modals.transactionDetails.deleteTitle'), + t('modals.transactionDetails.deleteMessage'), + [ + { text: t('common.cancel'), style: 'cancel' }, + { + text: t('common.delete'), + style: 'destructive', + onPress: async () => { + // Leave first: the list refresh removes this transaction, and a + // successful delete must never land the user on an error screen. + deletingRef.current = true; + router.back(); + try { + await deleteTransaction(transaction.id); + } catch { + deletingRef.current = false; + Alert.alert(t('alerts.error'), t('modals.transactionDetails.deleteFailed')); + } + }, + }, + ], + ); + }; return ( - - - - - - {sign}{formatAmount(transaction.amount)} - - - {transaction.note || categoryName} - + {/* A transfer is two rows in two wallets with nothing linking them, so + deleting one leg is refused outright - no affordance for it here. + The use case throws as well; this only spares the user a dead end. */} + ( + + + + ), + }} + /> + + + - + - + {t('modals.addTransaction.type') || "Type"} @@ -118,7 +148,7 @@ export function TransactionDetailsModal() { {t('modals.addTransaction.date')} {formatDate(transaction.date)} - + {transaction.note ? ( @@ -148,15 +178,9 @@ const styles = StyleSheet.create({ justifyContent: 'center', alignItems: 'center', }, - iconLarge: { - width: 80, - height: 80, - alignItems: 'center', - justifyContent: 'center', - }, - amount: { - fontSize: 36, - fontWeight: '700', + headerButton: { + padding: 8, + marginEnd: -8, }, card: { padding: 20, diff --git a/src/components/modals/__tests__/TransactionDetailsModal.delete.test.tsx b/src/components/modals/__tests__/TransactionDetailsModal.delete.test.tsx new file mode 100644 index 0000000..c2965de --- /dev/null +++ b/src/components/modals/__tests__/TransactionDetailsModal.delete.test.tsx @@ -0,0 +1,224 @@ +/** + * TransactionDetailsModal - delete affordance and shared row content + * + * Two things are pinned here. + * + * 1. The detail screen and the list describe a transaction with ONE voice. They + * used to disagree (the list titled rows with the note, the detail screen + * painted transfers a blue the list never used), so the assertion compares + * the two trees directly rather than checking either against a literal. + * + * 2. Deleting is deliberate and safe: it asks first, a cancel does nothing, a + * confirm dismisses BEFORE the refreshed list drops the row (so a successful + * delete can never land on the error screen), and a transfer offers no + * affordance at all - deleting one leg would orphan the other. + * + * Only the data sources and the navigator are mocked; the presenter runs for real. + */ + +import { fireEvent, render } from '@testing-library/react-native'; +import React from 'react'; +import { Alert } from 'react-native'; + +import { Transaction, TransactionType } from '../../../domain/entities'; +import { TransactionList } from '../../transactions/TransactionList'; +import { TransactionDetailsModal } from '../TransactionDetailsModal'; + +// ─── Fixtures ───────────────────────────────────────────────────────── + +const CATEGORIES = [ + { id: 'cat-food', name: 'Groceries', type: 'expense', icon: 'restaurant', color: '#F59E0B' }, +]; + +const WALLETS = [{ id: 'wallet-cash', name: 'Cash Wallet', balance: 100000, type: 'cash' }]; + +function tx(overrides: Partial = {}): Transaction { + return { + id: 'tx-1', + type: TransactionType.EXPENSE, + amount: 6000, + categoryId: 'cat-food', + walletId: 'wallet-cash', + date: new Date('2026-03-01T10:00:00'), + createdAt: new Date('2026-03-01T10:00:00'), + ...overrides, + }; +} + +const EXPENSE = tx({ note: 'Weekly shop at the market' }); +const TRANSFER_LEG = tx({ type: TransactionType.TRANSFER, categoryId: 'transfer-out' }); + +// ─── Mocks ──────────────────────────────────────────────────────────── + +/** Mutable so a test can simulate the list refresh that follows a delete. */ +let mockTransactions: Transaction[] = [EXPENSE]; +const mockDeleteTransaction = jest.fn(); +const mockBack = jest.fn(); + +jest.mock('expo-router', () => ({ + useLocalSearchParams: () => ({ id: 'tx-1' }), + useRouter: () => ({ push: jest.fn() }), + router: { back: (...args: unknown[]) => mockBack(...args) }, + // The delete button is handed to the navigator, so render what we hand over. + Stack: { + Screen: ({ options }: { options?: { headerRight?: () => React.ReactElement } }) => + options?.headerRight ? options.headerRight() : null, + }, +})); + +jest.mock('../../../hooks/useTransactions', () => ({ + useTransactions: () => ({ + transactions: mockTransactions, + loading: false, + deleteTransaction: mockDeleteTransaction, + }), +})); + +jest.mock('../../../hooks/useCategories', () => ({ + useCategories: () => ({ categories: CATEGORIES, loading: false, error: null }), +})); + +jest.mock('../../../hooks/useWallets', () => ({ + useWallets: () => ({ wallets: WALLETS, loading: false, error: null }), +})); + +jest.mock('../../../hooks/useFormatting', () => ({ + useFormatting: () => ({ + formatAmount: (cents: number) => `$${(cents / 100).toFixed(2)}`, + formatDate: () => '03/01/2026', + }), +})); + +jest.mock('react-i18next', () => ({ + useTranslation: () => ({ t: (key: string) => key, i18n: { language: 'en' } }), +})); + +// ─── Harness ────────────────────────────────────────────────────────── + +type AlertButton = { text?: string; style?: string; onPress?: () => void | Promise }; + +/** The buttons handed to the most recent Alert.alert call. */ +function lastAlertButtons(): AlertButton[] { + const calls = (Alert.alert as unknown as jest.Mock).mock.calls; + return calls[calls.length - 1][2] as AlertButton[]; +} + +/** + * Pay the one-time cost of realising this tree ONCE, before any test runs. + * + * The first render in this file resolves React Native's lazily-required modules + * (ScrollView, TouchableOpacity) and the Ionicons glyphmap. With a cold jest cache + * that is over a second locally and several times that on the CI runner - enough to + * blow the 5s default timeout of whichever test happens to render first. Paying it + * here removes the order dependence: no arbitrary test carries the allowance, and + * reordering or adding a test cannot silently move the failure. The 30s allowance is + * sized against the CI runner, not against local timings. + */ +beforeAll(() => { + const view = render(); + view.unmount(); +}, 30_000); + +beforeEach(() => { + jest.clearAllMocks(); + jest.spyOn(Alert, 'alert').mockImplementation(() => undefined); + mockTransactions = [EXPENSE]; +}); + +// ─── Tests ──────────────────────────────────────────────────────────── + +describe('TransactionDetailsModal - one voice with the list', () => { + it('names the transaction exactly as the list row does', () => { + const modal = render(); + const list = render(); + + expect(modal.getByTestId('transaction_row_title').props.children).toEqual( + list.getByTestId('transaction_row_title').props.children, + ); + expect(modal.getByTestId('transaction_row_subtitle').props.children).toEqual( + list.getByTestId('transaction_row_subtitle').props.children, + ); + expect(modal.getByTestId('transaction_row_amount').props.children).toEqual( + list.getByTestId('transaction_row_amount').props.children, + ); + }); + + it('still shows the note in full, now that it has left the title', () => { + const { getByText } = render(); + + expect(getByText('Weekly shop at the market')).toBeTruthy(); + }); +}); + +describe('TransactionDetailsModal - delete', () => { + it('asks before deleting anything', () => { + const { getByTestId } = render(); + + fireEvent.press(getByTestId('transaction_delete_button')); + + expect(Alert.alert).toHaveBeenCalledWith( + 'modals.transactionDetails.deleteTitle', + 'modals.transactionDetails.deleteMessage', + expect.any(Array), + ); + expect(mockDeleteTransaction).not.toHaveBeenCalled(); + }); + + it('offers exactly one destructive step, not a nested second confirmation', () => { + const { getByTestId } = render(); + + fireEvent.press(getByTestId('transaction_delete_button')); + const buttons = lastAlertButtons(); + + expect(buttons).toHaveLength(2); + expect(buttons[0].style).toBe('cancel'); + expect(buttons[1].style).toBe('destructive'); + }); + + it('does nothing at all when the confirmation is cancelled', async () => { + const { getByTestId } = render(); + + fireEvent.press(getByTestId('transaction_delete_button')); + await lastAlertButtons()[0].onPress?.(); + + expect(mockDeleteTransaction).not.toHaveBeenCalled(); + expect(mockBack).not.toHaveBeenCalled(); + }); + + it('deletes and dismisses on confirm, without ever showing an error state', async () => { + const view = render(); + + fireEvent.press(view.getByTestId('transaction_delete_button')); + await lastAlertButtons()[1].onPress?.(); + + expect(mockDeleteTransaction).toHaveBeenCalledWith('tx-1'); + expect(mockBack).toHaveBeenCalled(); + + // The refreshed list now has no such transaction. The screen is on its way + // out, so it must render nothing rather than "something went wrong". + mockTransactions = []; + view.rerender(); + expect(view.queryByText('common.somethingWentWrong')).toBeNull(); + }); + + it('reports a failed delete instead of failing silently', async () => { + mockDeleteTransaction.mockRejectedValueOnce(new Error('boom')); + const { getByTestId } = render(); + + fireEvent.press(getByTestId('transaction_delete_button')); + await lastAlertButtons()[1].onPress?.(); + + expect(Alert.alert).toHaveBeenLastCalledWith( + 'alerts.error', + 'modals.transactionDetails.deleteFailed', + ); + }); + + it('offers no delete affordance on a transfer leg', () => { + mockTransactions = [TRANSFER_LEG]; + + const { queryByTestId } = render(); + + expect(queryByTestId('transaction_delete_button')).toBeNull(); + }); +}); diff --git a/src/components/transactions/TransactionList.tsx b/src/components/transactions/TransactionList.tsx index 4a8d47e..2c81336 100644 --- a/src/components/transactions/TransactionList.tsx +++ b/src/components/transactions/TransactionList.tsx @@ -1,5 +1,4 @@ -import { Ionicons } from '@expo/vector-icons'; -import React, { useCallback } from 'react'; +import React from 'react'; import { useTranslation } from 'react-i18next'; import { ActivityIndicator, @@ -16,7 +15,7 @@ import { useCategories } from '../../hooks/useCategories'; import { useFormatting } from '../../hooks/useFormatting'; import { useWallets } from '../../hooks/useWallets'; import { useTheme } from '../../theme/theme'; -import { CategoryVisual, DEFAULT_CATEGORY_ICON, resolveCategoryVisual } from '../../utils/categoryVisuals'; +import { TransactionPresenter } from './TransactionPresenter'; interface TransactionListProps { transactions: Transaction[]; @@ -101,41 +100,16 @@ export const TransactionList: React.FC = ({ loadingMore = false, }) => { const { t, i18n } = useTranslation(); - const { colors, typography, spacing, radius } = useTheme(); + const { colors, typography, spacing } = useTheme(); const { categories } = useCategories(); const { wallets } = useWallets(); const { formatAmount } = useFormatting(); const router = useRouter(); - // Helper to get category name from ID - const getCategoryName = useCallback((categoryId: string): string => { - if (categoryId === 'transfer-in' || categoryId === 'transfer-out') return t('components.transactionList.transfer'); - const category = categories.find(c => c.id === categoryId); - return category?.name || t('components.transactionList.unknown'); - }, [categories, t]); - - // Helper to get wallet name from ID - const getWalletName = useCallback((walletId: string): string => { - const wallet = wallets.find(w => w.id === walletId); - return wallet?.name || t('components.transactionList.unknown'); - }, [wallets, t]); - - // Icon + colour come from the stored category, so a renamed category keeps its - // appearance whatever language the user names it in. See utils/categoryVisuals. - const getCategoryVisual = useCallback((categoryId: string): CategoryVisual => { - if (categoryId === 'transfer-in' || categoryId === 'transfer-out') { - return { icon: DEFAULT_CATEGORY_ICON, color: colors.accent }; - } - return resolveCategoryVisual(categories.find(c => c.id === categoryId), colors.accent); - }, [categories, colors.accent]); - - // Format display amount - const displayTransactionAmount = (amount: number, type: string, categoryId: string) => { - const formatted = formatAmount(amount); - const isIncome = type === 'income' || categoryId === 'transfer-in'; - const sign = isIncome ? '+' : '-'; - return `${sign}${formatted}`; - }; + // Row CONTENT - title, subtitle, note indicator, amount - belongs to + // TransactionPresenter, so both renderers below (and the detail screen) + // cannot describe the same transaction differently. This component owns + // only the containers: padding, background and the press target. // ─── Empty state ────────────────────────────────────────────────── @@ -158,55 +132,26 @@ export const TransactionList: React.FC = ({ // ─── Flat (no headers) ──────────────────────────────────────────── if (!showDateHeaders) { - const renderFlatItem = ({ item }: ListRenderItemInfo) => { - const categoryName = getCategoryName(item.categoryId); - const { icon: categoryIcon, color: categoryColor } = getCategoryVisual(item.categoryId); - return ( - router.push(`/transaction/${item.id}`)} - style={[ - styles.transactionItem, - { - paddingVertical: spacing.sm, - paddingHorizontal: 0, - }, - ]} - > - - - - - - {item.note || getWalletName(item.walletId)} - - - {categoryName} - - - - {displayTransactionAmount(item.amount, item.type, item.categoryId)} - - - ); - }; + const renderFlatItem = ({ item }: ListRenderItemInfo) => ( + router.push(`/transaction/${item.id}`)} + style={[ + styles.transactionItem, + { + paddingVertical: spacing.sm, + paddingHorizontal: 0, + }, + ]} + > + + + ); return ( = ({ } const transaction = item.transaction; - const categoryName = getCategoryName(transaction.categoryId); - const { icon: categoryIcon, color: categoryColor } = getCategoryVisual(transaction.categoryId); return ( = ({ }, ]} > - - - - - - {transaction.note || getWalletName(transaction.walletId)} - - - {categoryName} - - - - {displayTransactionAmount(transaction.amount, transaction.type, transaction.categoryId)} - + ); }; @@ -357,9 +273,4 @@ const styles = StyleSheet.create({ alignItems: 'center', gap: 12, }, - iconContainer: { - padding: 10, - alignItems: 'center', - justifyContent: 'center', - }, }); diff --git a/src/components/transactions/TransactionPresenter.tsx b/src/components/transactions/TransactionPresenter.tsx new file mode 100644 index 0000000..074fe27 --- /dev/null +++ b/src/components/transactions/TransactionPresenter.tsx @@ -0,0 +1,273 @@ +/** + * Transaction Presenter + * + * The single component that decides what a transaction LOOKS like: its icon, + * its title, its subtitle, whether it carries a note, its sign and its colour. + * + * It exists because the two list renderers and the detail screen each used to + * answer those questions for themselves, and drifted apart: rows titled + * themselves `note || wallet`, so a row with a note and a row without showed + * different kinds of information in the same position, and the detail screen + * painted transfers with a hardcoded blue the list never used. + * + * The rule is now fixed here: the title is always the CATEGORY, the subtitle is + * always the WALLET, and a note is announced by a discreet indicator rather than + * by taking over the title. Callers choose a variant (how big) and own the outer + * container (padding, background, press target) - never the content. + * + * Categories, wallets and formatAmount arrive as props on purpose. useCategories, + * useWallets and useFormatting each own their own state and issue a repository + * read (or a settings load) per instance, so calling them inside a row would fire + * one async load per row on every render. Every call site already holds one. + */ + +import { Ionicons } from '@expo/vector-icons'; +import React from 'react'; +import { useTranslation } from 'react-i18next'; +import { StyleSheet, Text, View } from 'react-native'; + +import { Category, Transaction } from '../../domain/entities'; +import { Wallet } from '../../domain/entities/Wallet'; +import { spacing } from '../../theme/spacing'; +import { useTheme } from '../../theme/theme'; +import { typography } from '../../theme/typography'; +import { isTransferCategoryId, resolveTransactionVisual } from '../../utils/categoryVisuals'; + +/** Minimal translate signature - the presenter only ever looks up plain keys. */ +type TranslateFn = (key: string) => string; + +/** + * How much room the presenter has. + * - `compact` - the dense flat list on the dashboard and wallet screens + * - `comfortable` - the sectioned transactions list + * - `detail` - the centred hero on the transaction detail screen + */ +export type TransactionPresenterVariant = 'compact' | 'comfortable' | 'detail'; + +export interface TransactionPresenterProps { + transaction: Transaction; + categories: Category[]; + wallets: Wallet[]; + formatAmount: (amountMinor: number) => string; + variant: TransactionPresenterVariant; +} + +// ─── Shared label resolution ────────────────────────────────────────── +// Exported so the detail screen's field rows name the same category and wallet +// the presenter shows, instead of resolving them a second way. + +/** The category label, including the transfer pseudo-ids that have no Category row. */ +export function resolveTransactionCategoryLabel( + transaction: Pick, + categories: Category[], + t: TranslateFn, +): string { + if (isTransferCategoryId(transaction.categoryId)) { + return t('components.transactionList.transfer'); + } + const category = categories.find((c) => c.id === transaction.categoryId); + return category?.name || t('components.transactionList.unknown'); +} + +/** The wallet label for a transaction's wallet id. */ +export function resolveWalletLabel(walletId: string, wallets: Wallet[], t: TranslateFn): string { + const wallet = wallets.find((w) => w.id === walletId); + return wallet?.name || t('components.transactionList.unknown'); +} + +/** True when the transaction credits its wallet (income, or the incoming leg). */ +function isCredit(transaction: Pick): boolean { + return transaction.type === 'income' || transaction.categoryId === 'transfer-in'; +} + +// ─── Variant metrics ────────────────────────────────────────────────── + +interface VariantMetrics { + readonly iconSize: number; + readonly tileSize?: number; + readonly tileAlpha: string; + readonly titleSize: number; + readonly titleWeight: '500' | '600' | '700'; + readonly subtitleSize: number; + readonly amountSize: number; + readonly amountWeight: '600' | '700'; + readonly noteIconSize: number; +} + +const METRICS: Record = { + compact: { + iconSize: 20, + tileAlpha: '20', + titleSize: typography.sizes.sm, + titleWeight: typography.weights.medium, + subtitleSize: typography.sizes.xs, + amountSize: typography.sizes.sm, + amountWeight: typography.weights.semibold, + noteIconSize: 12, + }, + comfortable: { + iconSize: 22, + tileSize: 44, + tileAlpha: '15', + titleSize: typography.sizes.md, + titleWeight: typography.weights.semibold, + subtitleSize: 13, + amountSize: typography.sizes.md, + amountWeight: typography.weights.semibold, + noteIconSize: 13, + }, + detail: { + iconSize: 42, + tileSize: 80, + tileAlpha: '15', + titleSize: typography.sizes.md, + titleWeight: typography.weights.semibold, + subtitleSize: typography.sizes.sm, + amountSize: typography.sizes['5xl'], + amountWeight: typography.weights.bold, + noteIconSize: 14, + }, +}; + +// ─── Component ──────────────────────────────────────────────────────── + +export const TransactionPresenter: React.FC = ({ + transaction, + categories, + wallets, + formatAmount, + variant, +}) => { + const { t } = useTranslation(); + const { colors, radius } = useTheme(); + + const metrics = METRICS[variant]; + const isDetail = variant === 'detail'; + + const { icon, color } = resolveTransactionVisual(transaction, categories, colors.accent); + const title = resolveTransactionCategoryLabel(transaction, categories, t); + const subtitle = resolveWalletLabel(transaction.walletId, wallets, t); + const hasNote = Boolean(transaction.note && transaction.note.trim().length > 0); + + const credit = isCredit(transaction); + const amount = `${credit ? '+' : '-'}${formatAmount(transaction.amount)}`; + const amountColor = credit ? colors.success : colors.foreground; + + const iconTile = ( + + + + ); + + const titleRow = ( + + + {title} + + {hasNote ? ( + + ) : null} + + ); + + const subtitleText = ( + + {subtitle} + + ); + + const amountText = ( + + {amount} + + ); + + if (isDetail) { + return ( + + {iconTile} + {amountText} + {titleRow} + {subtitleText} + + ); + } + + return ( + <> + {iconTile} + + {titleRow} + {subtitleText} + + {amountText} + + ); +}; + +const styles = StyleSheet.create({ + iconTile: { + alignItems: 'center', + justifyContent: 'center', + }, + iconTilePadded: { + padding: 10, + }, + rowText: { + flex: 1, + }, + titleRow: { + flexDirection: 'row', + alignItems: 'center', + marginBottom: 2, + }, + centred: { + justifyContent: 'center', + }, + noteIndicator: { + marginStart: 6, + }, + detail: { + alignItems: 'center', + }, + detailAmount: { + marginTop: spacing.md, + marginBottom: spacing.xs, + }, +}); diff --git a/src/components/transactions/__tests__/TransactionPresenter.test.tsx b/src/components/transactions/__tests__/TransactionPresenter.test.tsx new file mode 100644 index 0000000..d225b5a --- /dev/null +++ b/src/components/transactions/__tests__/TransactionPresenter.test.tsx @@ -0,0 +1,194 @@ +/** + * TransactionPresenter - what a row actually says + * + * The row used to title itself `note || wallet`, so two neighbouring rows showed + * different KINDS of information in the same position: one the free-text note, + * the next a wallet name. These tests lock the rule that replaced it - the title + * is the category, the subtitle is the wallet, always - and they assert it + * through TransactionList, i.e. the tree the user actually sees, for both + * renderers. + * + * The presenter is exercised, not stubbed: only the data sources are mocked. + */ + +import { render } from '@testing-library/react-native'; +import React from 'react'; + +import { Transaction, TransactionType } from '../../../domain/entities'; +import { TransactionList } from '../TransactionList'; + +// ─── Mocks ──────────────────────────────────────────────────────────── + +const CATEGORIES = [ + { id: 'cat-food', name: 'Groceries', type: 'expense', icon: 'restaurant', color: '#F59E0B' }, +]; + +const WALLETS = [{ id: 'wallet-cash', name: 'Cash Wallet', balance: 100000, type: 'cash' }]; + +jest.mock('../../../hooks/useCategories', () => ({ + useCategories: () => ({ categories: CATEGORIES, loading: false, error: null }), +})); + +jest.mock('../../../hooks/useWallets', () => ({ + useWallets: () => ({ wallets: WALLETS, loading: false, error: null }), +})); + +jest.mock('../../../hooks/useFormatting', () => ({ + useFormatting: () => ({ + formatAmount: (cents: number) => `$${(cents / 100).toFixed(2)}`, + }), +})); + +jest.mock('react-i18next', () => ({ + useTranslation: () => ({ t: (key: string) => key, i18n: { language: 'en' } }), +})); + +jest.mock('expo-router', () => ({ + useRouter: () => ({ push: jest.fn() }), +})); + +// ─── Fixtures ───────────────────────────────────────────────────────── + +function tx(overrides: Partial = {}): Transaction { + return { + id: 'tx-1', + type: TransactionType.EXPENSE, + amount: 6000, + categoryId: 'cat-food', + walletId: 'wallet-cash', + date: new Date('2026-03-01T10:00:00'), + createdAt: new Date('2026-03-01T10:00:00'), + ...overrides, + }; +} + +const WITH_NOTE = tx({ id: 'tx-with-note', note: 'Weekly shop at the market' }); +const WITHOUT_NOTE = tx({ id: 'tx-without-note' }); + +/** Both renderers, driven by the one prop that switches between them. */ +const RENDERERS: [string, boolean][] = [ + ['flat', false], + ['sectioned', true], +]; + +// ─── Harness ────────────────────────────────────────────────────────── + +/** + * Pay the one-time cost of realising this tree ONCE, before any test runs. + * + * The first render in this file resolves React Native's lazily-required modules + * (FlatList, TouchableOpacity) and the Ionicons glyphmap. With a cold jest cache + * that is over a second locally and several times that on the CI runner - enough to + * blow the 5s default timeout of whichever test happens to render first. Paying it + * here removes the order dependence: no arbitrary test carries the allowance, and + * reordering or adding a test cannot silently move the failure. The 30s allowance is + * sized against the CI runner, not against local timings. + */ +beforeAll(() => { + const view = render(); + view.unmount(); +}, 30_000); + +// ─── Tests ──────────────────────────────────────────────────────────── + +describe('transaction row content', () => { + describe.each(RENDERERS)('%s renderer', (_label, showDateHeaders) => { + it('titles the row with the category and subtitles it with the wallet when a note exists', () => { + const { getByTestId, queryByText } = render( + , + ); + + expect(getByTestId('transaction_row_title')).toHaveTextContent('Groceries'); + expect(getByTestId('transaction_row_subtitle')).toHaveTextContent('Cash Wallet'); + // The note has left the row entirely - it is announced by the indicator + // and read in full on the detail screen. This is the assertion that the + // old `note || wallet` title fails. + expect(queryByText('Weekly shop at the market')).toBeNull(); + }); + + it('titles the row the same way when no note exists', () => { + const { getByTestId } = render( + , + ); + + expect(getByTestId('transaction_row_title')).toHaveTextContent('Groceries'); + expect(getByTestId('transaction_row_subtitle')).toHaveTextContent('Cash Wallet'); + }); + + it('shows neighbouring rows the same kind of information whether or not they carry a note', () => { + const { getAllByTestId } = render( + , + ); + + const titles = getAllByTestId('transaction_row_title').map((node) => node.props.children); + const subtitles = getAllByTestId('transaction_row_subtitle').map((n) => n.props.children); + + expect(titles).toEqual(['Groceries', 'Groceries']); + expect(subtitles).toEqual(['Cash Wallet', 'Cash Wallet']); + }); + + it('marks the row that carries a note, and only that row', () => { + const { queryByTestId, rerender } = render( + , + ); + expect(queryByTestId('transaction_note_indicator')).not.toBeNull(); + + rerender( + , + ); + expect(queryByTestId('transaction_note_indicator')).toBeNull(); + }); + + it('treats a whitespace-only note as no note at all', () => { + const { queryByTestId } = render( + , + ); + + expect(queryByTestId('transaction_note_indicator')).toBeNull(); + }); + + it('signs the amount by direction: income credits, expense debits', () => { + const { getByTestId, rerender } = render( + , + ); + expect(getByTestId('transaction_row_amount')).toHaveTextContent('-$60.00'); + + rerender( + , + ); + expect(getByTestId('transaction_row_amount')).toHaveTextContent('+$60.00'); + }); + + it('names a transfer leg by its direction-free label, still over the wallet', () => { + const { getByTestId } = render( + , + ); + + expect(getByTestId('transaction_row_title')).toHaveTextContent( + 'components.transactionList.transfer', + ); + expect(getByTestId('transaction_row_subtitle')).toHaveTextContent('Cash Wallet'); + }); + }); +}); diff --git a/src/data/migrations/v5_import_from_asyncstorage.ts b/src/data/migrations/v5_import_from_asyncstorage.ts index eca38f6..8e50538 100644 --- a/src/data/migrations/v5_import_from_asyncstorage.ts +++ b/src/data/migrations/v5_import_from_asyncstorage.ts @@ -28,7 +28,7 @@ import { SerializableTransaction, } from '../../domain/entities/Transaction'; import { deserializeWallet, SerializableWallet } from '../../domain/entities/Wallet'; -import { ledgerEffect } from '../repositories/ledger'; +import { ledgerEffect } from '../../domain/ledger/ledgerEffect'; import { WalletRepository } from '../repositories/WalletRepository'; import { budgetMapper, diff --git a/src/data/repositories/WalletRepository.ts b/src/data/repositories/WalletRepository.ts index 8374436..6c58489 100644 --- a/src/data/repositories/WalletRepository.ts +++ b/src/data/repositories/WalletRepository.ts @@ -25,7 +25,7 @@ import type { BalanceAudit, IWalletRepository } from '../../domain/repositories' import { walletMapper, sqlDelete, sqlGetAll, sqlGetById, sqlExists, sqlUpdate } from '../storage/sql/mappers'; import type { SqlDatabase } from '../storage/sql/SqlDatabase'; import { RepositoryError, RepositoryErrorType } from './IRepository'; -import { ledgerEffect } from './ledger'; +import { ledgerEffect } from '../../domain/ledger/ledgerEffect'; // Re-exported for backward compatibility; the canonical type lives in the domain. export type { BalanceAudit } from '../../domain/repositories'; diff --git a/src/domain/__tests__/useCases.test.ts b/src/domain/__tests__/useCases.test.ts index 0004b75..7c4b368 100644 --- a/src/domain/__tests__/useCases.test.ts +++ b/src/domain/__tests__/useCases.test.ts @@ -13,6 +13,7 @@ import { deleteCategory, deleteTransaction, transferFunds, + TransferDeletionNotSupportedError, } from '../useCases'; // ─── Shared test infrastructure ───────────────────────────────────── @@ -115,6 +116,10 @@ describe('deleteTransaction', () => { const updated = await walletRepo.getById(wallet.id); expect(updated!.balance).toBe(100000); + // The number alone would also pass if the row had survived the delete, so + // assert the audit too: the stored balance must equal what the remaining + // ledger recomputes to. + expect(await walletRepo.recomputeBalanceFromLedger(wallet.id)).toBe(updated!.balance); expect(eventBus.emitMultiple).toHaveBeenCalledWith(['transactions', 'wallets']); }); @@ -134,6 +139,62 @@ describe('deleteTransaction', () => { const updated = await walletRepo.getById(wallet.id); expect(updated!.balance).toBe(50000); + expect(await walletRepo.recomputeBalanceFromLedger(wallet.id)).toBe(updated!.balance); + }); + + // A transfer is two rows in two wallets with no column linking them. Deleting + // one leg leaves the other orphaned, and because each wallet still audits + // clean against its own ledger, verifyFinancialIntegrity cannot see it. The + // only safe answer is to refuse. + describe('transfer legs', () => { + async function makeTransfer() { + const source = await walletRepo.create({ name: 'Cash', balance: 100000, type: WalletType.CASH }); + const dest = await walletRepo.create({ name: 'Bank', balance: 50000, type: WalletType.BANK }); + + await transferFunds(getDeps(), { + fromWalletId: source.id, + toWalletId: dest.id, + amount: 25000, + }); + + const legs = await transactionRepo.getAll(); + // Forget the transfer's own announcement so the assertions below only + // see what the attempted deletion did. + eventBus.emitMultiple.mockClear(); + return { + source, + dest, + outgoing: legs.find((tx) => tx.categoryId === 'transfer-out')!, + incoming: legs.find((tx) => tx.categoryId === 'transfer-in')!, + }; + } + + it.each([ + ['outgoing', (t: Awaited>) => t.outgoing], + ['incoming', (t: Awaited>) => t.incoming], + ])('refuses to delete the %s leg and leaves both wallets untouched', async (_label, pick) => { + const transfer = await makeTransfer(); + + await expect(deleteTransaction(getDeps(), pick(transfer).id)).rejects.toBeInstanceOf( + TransferDeletionNotSupportedError, + ); + + // Both wallets keep the balances the transfer left them with... + expect((await walletRepo.getById(transfer.source.id))!.balance).toBe(75000); + expect((await walletRepo.getById(transfer.dest.id))!.balance).toBe(75000); + // ...and both legs are still on the ledger, so neither is orphaned. + expect(await transactionRepo.getAll()).toHaveLength(2); + // Nothing was written, so nothing should have been announced. + expect(eventBus.emitMultiple).not.toHaveBeenCalled(); + }); + + it('carries a code the UI can branch on without matching message strings', async () => { + const transfer = await makeTransfer(); + + await expect(deleteTransaction(getDeps(), transfer.outgoing.id)).rejects.toMatchObject({ + code: 'TRANSFER_DELETION_NOT_SUPPORTED', + }); + }); }); }); diff --git a/src/data/repositories/ledger.ts b/src/domain/ledger/ledgerEffect.ts similarity index 61% rename from src/data/repositories/ledger.ts rename to src/domain/ledger/ledgerEffect.ts index 7729673..f70ea34 100644 --- a/src/data/repositories/ledger.ts +++ b/src/domain/ledger/ledgerEffect.ts @@ -3,11 +3,16 @@ * * Pure rule mapping a transaction to its signed effect (in cents) on its * wallet balance. Single source of truth for "how a transaction moves money", - * used by balance recomputation/auditing and by the import migration to - * derive each wallet's opening balance anchor. + * used by balance recomputation/auditing, by the import migration to derive + * each wallet's opening balance anchor, and by deletion to reverse a + * transaction by exact negation. + * + * Lives in the domain layer because it is a business rule with no storage + * knowledge, and because use cases must be able to reach it (the Clean + * Architecture dependency rule forbids domain -> data imports). */ -import { Transaction, TransactionType } from '../../domain/entities/Transaction'; +import { Transaction, TransactionType } from '../entities/Transaction'; /** Signed cents a transaction contributes to its wallet's balance. */ export function ledgerEffect( diff --git a/src/domain/useCases/deleteTransaction.ts b/src/domain/useCases/deleteTransaction.ts index 4224a88..73342a2 100644 --- a/src/domain/useCases/deleteTransaction.ts +++ b/src/domain/useCases/deleteTransaction.ts @@ -1,11 +1,20 @@ /** * Delete Transaction Use Case * - * Deletes a transaction and reverts the wallet balance adjustment. - * For expenses, credits the wallet back; for income, debits it. + * Deletes a transaction and reverts the wallet balance adjustment it made. + * + * The reversal is the exact negation of `ledgerEffect` - the single rule that + * says how a transaction moves money - so the two can never disagree. Deriving + * the sign here a second time is what let a transfer leg be reversed with the + * wrong sign, and since updateBalance applies a delta the error was 2x amount. + * + * Deleting a transfer leg is refused outright: see + * TransferDeletionNotSupportedError. */ import { TransactionType } from '../entities'; +import { ledgerEffect } from '../ledger/ledgerEffect'; +import { TransferDeletionNotSupportedError } from './errors'; import type { UseCaseDeps } from './types'; export async function deleteTransaction( @@ -17,16 +26,17 @@ export async function deleteTransaction( // Look up the transaction to determine reversal amount const transaction = await transactionRepo.getById(transactionId); + // Refuse before anything is written, so the pair is left exactly as it was. + if (transaction?.type === TransactionType.TRANSFER) { + throw new TransferDeletionNotSupportedError(); + } + // Atomic: the balance reversal and the record deletion commit together // or not at all. await runInTransaction(async () => { if (transaction) { - // Revert balance: expense was debited -> credit back; income was credited -> debit back - const reversalAmount = transaction.type === TransactionType.EXPENSE - ? transaction.amount - : -transaction.amount; - - await walletRepo.updateBalance(transaction.walletId, reversalAmount); + // Undo precisely the effect this transaction had on its wallet. + await walletRepo.updateBalance(transaction.walletId, -ledgerEffect(transaction)); } await transactionRepo.delete(transactionId); diff --git a/src/domain/useCases/errors.ts b/src/domain/useCases/errors.ts index 21f5129..96a0f08 100644 --- a/src/domain/useCases/errors.ts +++ b/src/domain/useCases/errors.ts @@ -18,3 +18,21 @@ export class InsufficientFundsError extends Error { this.name = 'InsufficientFundsError'; } } + +/** + * Raised when a caller tries to delete one leg of a transfer. + * + * A transfer is two rows in two wallets, and no column links them. Deleting one + * leg would leave the other orphaned: each wallet still audits clean on its own + * ledger, so the resulting imbalance between the pair is invisible to + * verifyFinancialIntegrity. Deleting transfers is not supported, and the refusal + * lives here rather than only in the UI so no future caller can reintroduce it. + */ +export class TransferDeletionNotSupportedError extends Error { + readonly code = 'TRANSFER_DELETION_NOT_SUPPORTED' as const; + + constructor(message = 'Transfers cannot be deleted one leg at a time') { + super(message); + this.name = 'TransferDeletionNotSupportedError'; + } +} diff --git a/src/domain/useCases/index.ts b/src/domain/useCases/index.ts index d60a5dd..7b1a441 100644 --- a/src/domain/useCases/index.ts +++ b/src/domain/useCases/index.ts @@ -6,7 +6,7 @@ export { createTransaction, type CreateTransactionInput } from './createTransact export { createWallet } from './createWallet'; export { deleteCategory } from './deleteCategory'; export { deleteTransaction } from './deleteTransaction'; -export { InsufficientFundsError } from './errors'; +export { InsufficientFundsError, TransferDeletionNotSupportedError } from './errors'; export { transferFunds, type TransferFundsInput } from './transferFunds'; export { verifyFinancialIntegrity } from './verifyFinancialIntegrity'; export type { EventBus, UseCaseDeps } from './types'; diff --git a/src/hooks/__tests__/useFormatting.test.ts b/src/hooks/__tests__/useFormatting.test.ts index 31e6ffa..1200173 100644 --- a/src/hooks/__tests__/useFormatting.test.ts +++ b/src/hooks/__tests__/useFormatting.test.ts @@ -125,6 +125,29 @@ describe('useFormatting', () => { expect(result.current.parseAmountToCents('12,50')).toBe(1250); }); + it('carries the exponent of a 3-decimal currency through format and parse', async () => { + // JOD is a 3-decimal currency. Symbol and exponent both come from the real + // registry - only dataEvents and settingsService are mocked in this suite. + mockLoadSettings.mockResolvedValue({ + currency: 'JOD', + decimalSeparator: 'dot', + dateFormat: 'MM/DD/YYYY', + }); + + const { result } = renderHook(() => useFormatting()); + + await waitFor(() => { + expect(result.current.settings).not.toBeNull(); + }); + + expect(result.current.decimals).toBe(3); + expect(result.current.formatAmount(1234)).toContain('JD1.234'); + // A lone grouping separator filling the fraction reads as the fraction, so this + // stores 1.234 JOD and not 1234 JOD - a silent 1000x error before the tie-break. + expect(result.current.parseAmount('1,234')).toBe(1.234); + expect(result.current.parseAmountToCents('1,234')).toBe(1234); + }); + it('parses with the dot fallback before settings load', () => { mockLoadSettings.mockReturnValue(new Promise(() => {})); diff --git a/src/localization/locales/en.json b/src/localization/locales/en.json index de2cd24..d001132 100644 --- a/src/localization/locales/en.json +++ b/src/localization/locales/en.json @@ -271,6 +271,11 @@ "errorEmpty": "Please enter a category name", "errorSave": "Failed to save category", "error": "Error" + }, + "transactionDetails": { + "deleteTitle": "Delete Transaction", + "deleteMessage": "Are you sure you want to delete this transaction? The wallet balance will be adjusted.", + "deleteFailed": "Failed to delete transaction" } }, "about": { @@ -602,6 +607,7 @@ "editWallet": "Edit wallet {{name}}", "editRule": "Edit rule", "deleteRule": "Delete rule", + "deleteTransaction": "Delete transaction", "pauseRule": "Pause rule", "resumeRule": "Resume rule", "searchTransactions": "Search transactions", @@ -630,7 +636,8 @@ "toggleEndDate": "Toggle end date", "submitForm": "Submit form", "expandFaq": "Expand answer", - "collapseFaq": "Collapse answer" + "collapseFaq": "Collapse answer", + "hasNote": "Has a note" }, "storeRecovery": { "title": "Couldn't open your data", diff --git a/src/localization/locales/es.json b/src/localization/locales/es.json index b891bf2..3d1f97f 100644 --- a/src/localization/locales/es.json +++ b/src/localization/locales/es.json @@ -271,6 +271,11 @@ "errorEmpty": "Ingresa un nombre de categoría", "errorSave": "No se pudo guardar la categoría", "error": "Error" + }, + "transactionDetails": { + "deleteTitle": "Eliminar transacción", + "deleteMessage": "¿Seguro que quieres eliminar esta transacción? El saldo de la cartera se ajustará.", + "deleteFailed": "No se pudo eliminar la transacción" } }, "about": { @@ -602,6 +607,7 @@ "editWallet": "Editar cartera {{name}}", "editRule": "Editar regla", "deleteRule": "Eliminar regla", + "deleteTransaction": "Eliminar transacción", "pauseRule": "Pausar regla", "resumeRule": "Reanudar regla", "searchTransactions": "Buscar transacciones", @@ -630,7 +636,8 @@ "toggleEndDate": "Activar o desactivar fecha de fin", "submitForm": "Enviar formulario", "expandFaq": "Expandir respuesta", - "collapseFaq": "Contraer respuesta" + "collapseFaq": "Contraer respuesta", + "hasNote": "Tiene una nota" }, "storeRecovery": { "title": "No se pudieron abrir tus datos", diff --git a/src/localization/locales/fr.json b/src/localization/locales/fr.json index 3ae2aa3..07a30bf 100644 --- a/src/localization/locales/fr.json +++ b/src/localization/locales/fr.json @@ -271,6 +271,11 @@ "errorEmpty": "Veuillez entrer un nom de catégorie", "errorSave": "Échec de l'enregistrement de la catégorie", "error": "Erreur" + }, + "transactionDetails": { + "deleteTitle": "Supprimer la transaction", + "deleteMessage": "Êtes-vous sûr de vouloir supprimer cette transaction ? Le solde du portefeuille sera ajusté.", + "deleteFailed": "Échec de la suppression de la transaction" } }, "about": { @@ -602,6 +607,7 @@ "editWallet": "Modifier le portefeuille {{name}}", "editRule": "Modifier la règle", "deleteRule": "Supprimer la règle", + "deleteTransaction": "Supprimer la transaction", "pauseRule": "Mettre en pause la règle", "resumeRule": "Reprendre la règle", "searchTransactions": "Rechercher des transactions", @@ -630,7 +636,8 @@ "toggleEndDate": "Définir une date de fin", "submitForm": "Soumettre le formulaire", "expandFaq": "Développer la réponse", - "collapseFaq": "Réduire la réponse" + "collapseFaq": "Réduire la réponse", + "hasNote": "Contient une note" }, "storeRecovery": { "title": "Impossible d'ouvrir vos données", diff --git a/src/localization/locales/pt.json b/src/localization/locales/pt.json index 4fbe0de..66d36f8 100644 --- a/src/localization/locales/pt.json +++ b/src/localization/locales/pt.json @@ -271,6 +271,11 @@ "errorEmpty": "Insira um nome para a categoria", "errorSave": "Falha ao salvar a categoria", "error": "Erro" + }, + "transactionDetails": { + "deleteTitle": "Excluir transação", + "deleteMessage": "Tem certeza de que deseja excluir esta transação? O saldo da carteira será ajustado.", + "deleteFailed": "Falha ao excluir a transação" } }, "about": { @@ -602,6 +607,7 @@ "editWallet": "Editar carteira {{name}}", "editRule": "Editar regra", "deleteRule": "Excluir regra", + "deleteTransaction": "Excluir transação", "pauseRule": "Pausar regra", "resumeRule": "Retomar regra", "searchTransactions": "Buscar transações", @@ -630,7 +636,8 @@ "toggleEndDate": "Alternar data final", "submitForm": "Enviar formulário", "expandFaq": "Expandir resposta", - "collapseFaq": "Recolher resposta" + "collapseFaq": "Recolher resposta", + "hasNote": "Contém uma nota" }, "storeRecovery": { "title": "Não foi possível abrir seus dados", diff --git a/src/localization/locales/ru.json b/src/localization/locales/ru.json index 9234836..7078ae1 100644 --- a/src/localization/locales/ru.json +++ b/src/localization/locales/ru.json @@ -271,6 +271,11 @@ "errorEmpty": "Введите название категории", "errorSave": "Не удалось сохранить категорию", "error": "Ошибка" + }, + "transactionDetails": { + "deleteTitle": "Удалить операцию", + "deleteMessage": "Удалить эту операцию? Баланс кошелька будет пересчитан.", + "deleteFailed": "Не удалось удалить операцию" } }, "about": { @@ -602,6 +607,7 @@ "editWallet": "Редактировать кошелёк {{name}}", "editRule": "Редактировать правило", "deleteRule": "Удалить правило", + "deleteTransaction": "Удалить операцию", "pauseRule": "Приостановить правило", "resumeRule": "Возобновить правило", "searchTransactions": "Поиск транзакций", @@ -630,7 +636,8 @@ "toggleEndDate": "Переключить дату окончания", "submitForm": "Отправить форму", "expandFaq": "Развернуть ответ", - "collapseFaq": "Свернуть ответ" + "collapseFaq": "Свернуть ответ", + "hasNote": "Есть заметка" }, "storeRecovery": { "title": "Не удалось открыть ваши данные", diff --git a/src/utils/__tests__/normalizeAmount.test.ts b/src/utils/__tests__/normalizeAmount.test.ts index 24c34b1..fa69696 100644 --- a/src/utils/__tests__/normalizeAmount.test.ts +++ b/src/utils/__tests__/normalizeAmount.test.ts @@ -230,3 +230,134 @@ describe('currency exponent', () => { } }); }); + +// ─── Separator tie-break, pinned BY DECIMALS CLASS ───────────────────── +// +// Organised by exponent class rather than by currency on purpose. The rule that +// decides between "grouping" and "decimal" is a function of `decimals` alone, so a +// future trade-off has to break a named class here rather than slip past a stray +// per-currency case. +// +// Policy under test: a lone grouping separator with exactly `decimals` digits after it +// and no decimal separator reads as a DECIMAL. Grouping stays authoritative wherever +// the string is unambiguous - two separators, or several groups. +describe('separator tie-break by decimals class', () => { + describe('decimals = 0 (unchanged)', () => { + it('reads a lone group as grouping under the dot preference', () => { + expect(parseAmountInput('1,234', 'dot', 0)).toBe(1234); + expect(parseAmountInput('0,500', 'dot', 0)).toBe(500); + expect(parseAmountInput('123,456', 'dot', 0)).toBe(123456); + expect(parseAmountInput('1,000', 'dot', 0)).toBe(1000); + }); + + it('reads a lone group as grouping under the comma preference', () => { + expect(parseAmountInput('1.234', 'comma', 0)).toBe(1234); + expect(parseAmountInput('0.500', 'comma', 0)).toBe(500); + expect(parseAmountInput('123.456', 'comma', 0)).toBe(123456); + expect(parseAmountInput('1.000', 'comma', 0)).toBe(1000); + }); + + it('keeps multi-group and plain readings', () => { + expect(parseAmountInput('1,234,567', 'dot', 0)).toBe(1234567); + expect(parseAmountInput('1.234.567', 'comma', 0)).toBe(1234567); + expect(parseAmountInput('1234', 'dot', 0)).toBe(1234); + expect(parseAmountInput('1234', 'comma', 0)).toBe(1234); + }); + + it('still rejects any fraction at all', () => { + expect(parseAmountInput('12.50', 'dot', 0)).toBeNull(); + expect(parseAmountInput('1.23', 'dot', 0)).toBeNull(); + expect(parseAmountInput('12,50', 'comma', 0)).toBeNull(); + expect(parseAmountInput('1,23', 'comma', 0)).toBeNull(); + }); + }); + + describe('decimals = 2 (unchanged)', () => { + it('reads a lone group as grouping under the dot preference', () => { + expect(parseAmountInput('1,234', 'dot', 2)).toBe(1234); + expect(parseAmountInput('0,500', 'dot', 2)).toBe(500); + expect(parseAmountInput('123,456', 'dot', 2)).toBe(123456); + expect(parseAmountInput('1,000', 'dot', 2)).toBe(1000); + }); + + it('reads a lone group as grouping under the comma preference', () => { + expect(parseAmountInput('1.234', 'comma', 2)).toBe(1234); + expect(parseAmountInput('0.500', 'comma', 2)).toBe(500); + expect(parseAmountInput('123.456', 'comma', 2)).toBe(123456); + expect(parseAmountInput('1.000', 'comma', 2)).toBe(1000); + }); + + it('keeps grouped-with-fraction, multi-group and 2-digit fraction readings', () => { + expect(parseAmountInput('2,000.50', 'dot', 2)).toBe(2000.5); + expect(parseAmountInput('2.000,50', 'comma', 2)).toBe(2000.5); + expect(parseAmountInput('1,234,567', 'dot', 2)).toBe(1234567); + expect(parseAmountInput('1.234.567', 'comma', 2)).toBe(1234567); + expect(parseAmountInput('12,50', 'dot', 2)).toBe(12.5); + expect(parseAmountInput('12.50', 'comma', 2)).toBe(12.5); + expect(parseAmountInput('1.23', 'dot', 2)).toBe(1.23); + expect(parseAmountInput('1,23', 'comma', 2)).toBe(1.23); + }); + + it('still rejects a 3-digit fraction as over-precision', () => { + expect(parseAmountInput('1.234', 'dot', 2)).toBeNull(); + expect(parseAmountInput('1,234', 'comma', 2)).toBeNull(); + expect(parseAmountInput('1234.567', 'dot', 2)).toBeNull(); + expect(parseAmountInput('1234,567', 'comma', 2)).toBeNull(); + }); + }); + + describe('decimals = 3 (the decimal reading wins)', () => { + it('reads a lone group as the fraction under the dot preference', () => { + // Was 1234 - branch B1 claimed the body and stripped the separator. + expect(parseAmountInput('1,234', 'dot', 3)).toBe(1.234); + expect(parseAmountInput('1,000', 'dot', 3)).toBe(1); + }); + + it('reads a lone group as the fraction under the comma preference', () => { + expect(parseAmountInput('1.234', 'comma', 3)).toBe(1.234); + expect(parseAmountInput('1.000', 'comma', 3)).toBe(1); + }); + + it('reads a leading-zero body as the fraction, which grouping cannot explain', () => { + // Not an ambiguous tie: no grouping convention writes a leading zero group. + expect(parseAmountInput('0,500', 'dot', 3)).toBe(0.5); + expect(parseAmountInput('0.500', 'comma', 3)).toBe(0.5); + }); + + it('reads a 3-digit integer part with a lone group as the fraction', () => { + expect(parseAmountInput('123,456', 'dot', 3)).toBe(123.456); + expect(parseAmountInput('123.456', 'comma', 3)).toBe(123.456); + }); + + it('reads a 4-digit integer part as the fraction (never was a valid group)', () => { + expect(parseAmountInput('1234,567', 'dot', 3)).toBe(1234.567); + expect(parseAmountInput('1234.567', 'comma', 3)).toBe(1234.567); + }); + + it('keeps grouping when the string carries BOTH separators', () => { + expect(parseAmountInput('1,234.567', 'dot', 3)).toBe(1234.567); + expect(parseAmountInput('1.234,567', 'comma', 3)).toBe(1234.567); + }); + + it('keeps grouping when the string carries SEVERAL groups', () => { + expect(parseAmountInput('1,234,567', 'dot', 3)).toBe(1234567); + expect(parseAmountInput('1.234.567', 'comma', 3)).toBe(1234567); + }); + + it('leaves plain bodies alone', () => { + expect(parseAmountInput('1234', 'dot', 3)).toBe(1234); + expect(parseAmountInput('1234', 'comma', 3)).toBe(1234); + expect(parseAmountInput('1.23', 'dot', 3)).toBe(1.23); + expect(parseAmountInput('1,23', 'comma', 3)).toBe(1.23); + }); + + it('round-trips parse(format(x)) === x, proving B1 was not over-narrowed', () => { + for (const minor of [1500, 1234, 500, 1234567, 999]) { + for (const separator of ['dot', 'comma'] as const) { + const rendered = formatAmount(minor, '', separator, 3); + expect(parseAndNormalizeAmount(rendered, separator, 3)).toBe(minor); + } + } + }); + }); +}); diff --git a/src/utils/categoryVisuals.ts b/src/utils/categoryVisuals.ts index 2aad2e9..d46f2b7 100644 --- a/src/utils/categoryVisuals.ts +++ b/src/utils/categoryVisuals.ts @@ -101,3 +101,51 @@ export function resolveCategoryVisual( return { icon, color }; } + +// ─── Transfers ──────────────────────────────────────────────────────── + +/** + * The two legs of a transfer carry pseudo category ids: no Category row exists + * for them, so they never resolve through the ladder above and every call site + * used to invent its own treatment. They are handled here instead, once. + */ +const TRANSFER_CATEGORY_IDS = ['transfer-in', 'transfer-out'] as const; + +/** The one glyph that means "transfer" anywhere in the app. */ +export const TRANSFER_ICON: IoniconName = 'swap-horizontal-outline'; + +/** True when the category id is one of the transfer pseudo-ids. */ +export function isTransferCategoryId(categoryId: string): boolean { + return (TRANSFER_CATEGORY_IDS as readonly string[]).includes(categoryId); +} + +/** Shape needed to look a category up by id - a subset of the Category entity. */ +export interface IdentifiedCategoryVisualSource extends CategoryVisualSource { + readonly id: string; +} + +/** + * Resolve the icon + colour for a whole transaction, transfers included. + * + * Prefer this over resolveCategoryVisual at any site that renders a transaction: + * it is what keeps the list rows and the detail screen showing one transfer + * visual instead of three. + * + * @param transaction - the transaction being rendered + * @param categories - the loaded categories, searched by id + * @param accentColor - theme colour for transfers and for the fallback (colors.accent) + */ +export function resolveTransactionVisual( + transaction: { readonly type: string; readonly categoryId: string }, + categories: readonly IdentifiedCategoryVisualSource[], + accentColor: string, +): CategoryVisual { + if (transaction.type === 'transfer' || isTransferCategoryId(transaction.categoryId)) { + return { icon: TRANSFER_ICON, color: accentColor }; + } + + return resolveCategoryVisual( + categories.find((category) => category.id === transaction.categoryId), + accentColor, + ); +} diff --git a/src/utils/normalizeAmount.ts b/src/utils/normalizeAmount.ts index e9859b0..d37fc4b 100644 --- a/src/utils/normalizeAmount.ts +++ b/src/utils/normalizeAmount.ts @@ -50,13 +50,18 @@ const SEPARATORS: Record< * * A thousands separator is only read as grouping where it forms a valid 3-digit group; * otherwise it is taken as the decimal point, so a comma-locale user typing "12,50" - * under the default dot preference gets 12.5 rather than 12. This grouping rule is - * exponent-blind by design: "1.500" means 1500 units in every currency (the decimal - * separator, which formatAmount emits, always routes to the decimal branch). + * under the default dot preference gets 12.5 rather than 12. The grouping rule is + * exponent-AWARE for the one shape where the two readings collide: a lone group whose + * digits would exactly fill the currency's fraction reads as the fraction, so at 3 + * decimals "1,234" is 1.234 and not 1234. See the tie-break comment at the branches. + * Grouping still wins wherever the string is unambiguous - two separators, or several + * groups - so parse(format(x)) === x holds at every exponent. * * @example parseAmountInput('12,50', 'dot') -> 12.5 * @example parseAmountInput('2.000,50', 'comma') -> 2000.5 * @example parseAmountInput('1,000', 'dot') -> 1000 + * @example parseAmountInput('1,234', 'dot', 3) -> 1.234 (lone group fills the fraction) + * @example parseAmountInput('1,234.567', 'dot', 3) -> 1234.567 (two separators: grouping) * @example parseAmountInput('1,2,3', 'dot') -> null * @example parseAmountInput('12.5', 'dot', 0) -> null (more fraction digits than allowed) * @example parseAmountInput('12.555', 'dot', 2) -> null @@ -73,8 +78,20 @@ export function parseAmountInput( const body = sign ? trimmed.slice(1) : trimmed; const { decimal, thousands, decimalRe, thousandsRe } = SEPARATORS[separator]; + // Tie-break: a body with a LONE grouping separator, no decimal separator, and exactly + // `decimals` digits after it reads as a DECIMAL, not as grouping. This is a deliberate + // decision, not an accident. Grouping is a writing convenience; the decimal separator + // carries value. A user who means one thousand two hundred thirty-four can always type + // "1234", whereas at 3 decimals a user who means 1.234 has no other way to express it + // with their keyboard's separator. Grouping stays authoritative wherever the string is + // unambiguous: two separators ("1,234.567") and several groups ("1,234,567") both still + // take the grouping branch below. The grouping branch only ever claims a trailing run of + // exactly 3 digits, so this can only divert input at decimals === 3 - it is a no-op for + // 0- and 2-decimal currencies. + const loneGroupIsFraction = new RegExp(`^\\d{1,3}${thousandsRe}\\d{${decimals}}$`).test(body); + let cleaned: string | null = null; - if (new RegExp(`^\\d{1,3}(?:${thousandsRe}\\d{3})+(?:${decimalRe}\\d*)?$`).test(body)) { + if (!loneGroupIsFraction && new RegExp(`^\\d{1,3}(?:${thousandsRe}\\d{3})+(?:${decimalRe}\\d*)?$`).test(body)) { cleaned = body.split(thousands).join('').split(decimal).join('.'); } else if (new RegExp(`^(?:\\d+(?:${decimalRe}\\d*)?|${decimalRe}\\d+)$`).test(body)) { cleaned = body.split(decimal).join('.');