fix(accounts): show credit cards as positive and exclude them from net worth (#673)
## Summary
A credit card is a spending account, not wealth. This change:
- **Excludes credit cards from net worth entirely** — they no longer add
to *or* subtract from the net worth total, the evolution chart, or its
trends. They are filtered out at the source, so every net-worth
aggregation (backend summary + frontend chart/trend/MoM) is consistent.
- **Shows the credit card balance as a positive figure** on the
per-account cards, on both the dashboard and the Accounts page (like any
other account).
- **Fixes a loan sign inconsistency**: a loan used to show *positive* on
the Accounts page but *negative* on the dashboard. The Accounts list now
applies the liability sign too, so loans render negative in both places.
### Why
A user reported that a credit card showed as a positive balance on the
Accounts screen but dragged their net worth down on the dashboard — the
same account rendered with opposite signs on the two screens. The root
cause was that credit cards were modelled as liabilities (like loans)
and each screen computed the balance through a different path. Product
decision: a credit card is spendable credit, so its balance is shown
as-is and simply doesn't participate in net worth.
## Behavior
| Account (e.g. 90k / 50k) | Per-account cards (dashboard + Accounts) |
Net worth chart & total |
| --- | --- | --- |
| **Credit card** | `+90,000` | **excluded** (not counted) |
| **Loan** | `−50,000` (was `+50,000` on Accounts) | subtracts
(unchanged) |
| Checking / savings / … | unchanged | unchanged (asset) |
## ⚠️ Existing-data impact
Previously credit cards were forced to **reduce** net worth
(`-abs(balance)`). With this change they are excluded, so **users with
existing credit-card accounts will see their net worth rise** by
whatever their cards used to subtract. This is a deliberate semantic
change, not a migration — no stored balances are altered.
## Implementation
- `AccountType::countsInNetWorth()` — new predicate, `false` only for
credit cards; `calculateNetWorthAt()` skips excluded types.
- `AccountType::reducesNetWorth()` / `LIABILITY_TYPES` — now only
`loan`, so `netWorthContribution()` renders credit cards positive on the
per-account cards.
- `net-worth-chart.tsx` — credit cards filtered out of
`includedAccounts` (one point that cascades to segments, totals, trends
and `useChartViews`).
- `Accounts/Index.tsx` — applies `netWorthContribution()` so loans
render negative, matching the dashboard.
## Testing
- Backend: `AccountTypeTest` (32) and `DashboardAnalyticsTest` (credit
card excluded, loan still subtracts) — green.
- Frontend: `chart-calculations.test.ts` (37) — green.
- `pint`, `prettier`, `eslint` — clean.
Browser QA skipped per request (the logged-in flow is also currently
blocked locally by pending Spaces migrations). Logic is fully covered by
the tests above.
This commit is contained in:
Víctor Falcón
GitHub
parent
c0816f9633
commit
cbeea2fc34
|
|
@ -23,7 +23,17 @@ enum AccountType: string
|
|||
|
||||
public function reducesNetWorth(): bool
|
||||
{
|
||||
return in_array($this, [self::CreditCard, self::Loan], true);
|
||||
return $this === self::Loan;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether this account type is part of the net worth total at all. Credit
|
||||
* cards are spending accounts, not wealth, so they are excluded entirely
|
||||
* (neither added nor subtracted) while still being tracked on their own.
|
||||
*/
|
||||
public function countsInNetWorth(): bool
|
||||
{
|
||||
return $this !== self::CreditCard;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -482,6 +482,10 @@ class DashboardAnalyticsController extends Controller
|
|||
$total = 0;
|
||||
|
||||
foreach ($accounts as $account) {
|
||||
if (! $account->type->countsInNetWorth()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$balance = $lookup->getBalanceAt($account->id, $date);
|
||||
|
||||
$convertedBalance = $this->exchangeRateService->convert(
|
||||
|
|
|
|||
|
|
@ -209,6 +209,10 @@ export function NetWorthChart({
|
|||
// All accounts included based on the toggles – used for totals & trends.
|
||||
const includedAccounts = Object.fromEntries(
|
||||
Object.entries(accounts).filter(([, account]) => {
|
||||
// Credit cards are spending accounts, never part of net worth.
|
||||
if (account.type === 'credit_card') {
|
||||
return false;
|
||||
}
|
||||
if (!includeLoansInNetWorthChart && account.type === 'loan') {
|
||||
return false;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -12,14 +12,14 @@ import {
|
|||
} from './chart-calculations';
|
||||
|
||||
describe('isLiabilityType', () => {
|
||||
it('returns true for credit_card', () => {
|
||||
expect(isLiabilityType('credit_card')).toBe(true);
|
||||
});
|
||||
|
||||
it('returns true for loan', () => {
|
||||
expect(isLiabilityType('loan')).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false for credit_card', () => {
|
||||
expect(isLiabilityType('credit_card')).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for checking', () => {
|
||||
expect(isLiabilityType('checking')).toBe(false);
|
||||
});
|
||||
|
|
@ -43,11 +43,11 @@ describe('isLiabilityType', () => {
|
|||
|
||||
describe('getAccountSign', () => {
|
||||
it('returns -1 for liabilities', () => {
|
||||
expect(getAccountSign('credit_card')).toBe(-1);
|
||||
expect(getAccountSign('loan')).toBe(-1);
|
||||
});
|
||||
|
||||
it('returns 1 for assets', () => {
|
||||
expect(getAccountSign('credit_card')).toBe(1);
|
||||
expect(getAccountSign('checking')).toBe(1);
|
||||
expect(getAccountSign('savings')).toBe(1);
|
||||
expect(getAccountSign('investment')).toBe(1);
|
||||
|
|
@ -58,7 +58,6 @@ describe('getAccountSign', () => {
|
|||
|
||||
describe('netWorthContribution', () => {
|
||||
it('subtracts the magnitude of liabilities (stored as positive)', () => {
|
||||
expect(netWorthContribution('credit_card', 5000)).toBe(-5000);
|
||||
expect(netWorthContribution('loan', 80000)).toBe(-80000);
|
||||
});
|
||||
|
||||
|
|
@ -104,12 +103,10 @@ describe('computeNetWorthSeries', () => {
|
|||
});
|
||||
|
||||
it('subtracts liabilities from net worth', () => {
|
||||
const data = [
|
||||
{ month: '2025-01', checking: 50000, credit_card: 10000 },
|
||||
];
|
||||
const data = [{ month: '2025-01', checking: 50000, loan: 10000 }];
|
||||
const accounts = createAccounts({
|
||||
checking: 'checking',
|
||||
credit_card: 'credit_card',
|
||||
loan: 'loan',
|
||||
});
|
||||
|
||||
const result = computeNetWorthSeries(data, accounts);
|
||||
|
|
@ -123,21 +120,19 @@ describe('computeNetWorthSeries', () => {
|
|||
month: '2025-01',
|
||||
savings: 100000,
|
||||
checking: 20000,
|
||||
credit_card: 5000,
|
||||
loan: 30000,
|
||||
},
|
||||
];
|
||||
const accounts = createAccounts({
|
||||
savings: 'savings',
|
||||
checking: 'checking',
|
||||
credit_card: 'credit_card',
|
||||
loan: 'loan',
|
||||
});
|
||||
|
||||
const result = computeNetWorthSeries(data, accounts);
|
||||
|
||||
// Net worth = 100000 + 20000 - 5000 - 30000 = 85000
|
||||
expect(result[0].value).toBe(85000);
|
||||
// Net worth = 100000 + 20000 - 30000 = 90000
|
||||
expect(result[0].value).toBe(90000);
|
||||
});
|
||||
|
||||
it('handles negative net worth (more liabilities than assets)', () => {
|
||||
|
|
|
|||
|
|
@ -1,9 +1,13 @@
|
|||
import { AccountType } from '@/types/account';
|
||||
|
||||
/**
|
||||
* Account types that reduce net worth (liabilities)
|
||||
* Account types that reduce net worth (liabilities).
|
||||
*
|
||||
* A credit card is deliberately NOT a liability: its balance is spendable
|
||||
* credit the user treats as an asset, so it adds to net worth like a regular
|
||||
* account. Only loans (debt that must be repaid) subtract.
|
||||
*/
|
||||
export const LIABILITY_TYPES: AccountType[] = ['credit_card', 'loan'];
|
||||
export const LIABILITY_TYPES: AccountType[] = ['loan'];
|
||||
|
||||
/**
|
||||
* Check if an account type is a liability
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import { SortableGrid } from '@/components/sortable-grid';
|
|||
import { Card, CardContent } from '@/components/ui/card';
|
||||
import { AccountWithMetrics } from '@/hooks/use-dashboard-data';
|
||||
import AppSidebarLayout from '@/layouts/app/app-sidebar-layout';
|
||||
import { netWorthContribution } from '@/lib/chart-calculations';
|
||||
import { BreadcrumbItem, SharedData } from '@/types';
|
||||
import { Account } from '@/types/account';
|
||||
import { __ } from '@/utils/i18n';
|
||||
|
|
@ -61,12 +62,25 @@ export default function AccountsIndex({ accounts, accountMetrics }: Props) {
|
|||
const accountsWithMetrics: AccountWithMetrics[] = useMemo(() => {
|
||||
return accounts.map((account) => {
|
||||
const metrics = accountMetrics?.[account.id];
|
||||
// Apply the liability sign so the Accounts list agrees with the
|
||||
// dashboard: loans render negative, everything else keeps its value.
|
||||
const currentBalance = netWorthContribution(
|
||||
account.type,
|
||||
metrics?.currentBalance ?? 0,
|
||||
);
|
||||
const previousBalance = netWorthContribution(
|
||||
account.type,
|
||||
metrics?.previousBalance ?? 0,
|
||||
);
|
||||
return {
|
||||
...account,
|
||||
currentBalance: metrics?.currentBalance ?? 0,
|
||||
previousBalance: metrics?.previousBalance ?? 0,
|
||||
diff: metrics?.diff ?? 0,
|
||||
history: metrics?.history ?? [],
|
||||
currentBalance,
|
||||
previousBalance,
|
||||
diff: currentBalance - previousBalance,
|
||||
history: (metrics?.history ?? []).map((point) => ({
|
||||
...point,
|
||||
value: netWorthContribution(account.type, point.value),
|
||||
})),
|
||||
investedAmount: metrics?.investedAmount ?? null,
|
||||
};
|
||||
});
|
||||
|
|
@ -116,7 +130,10 @@ export default function AccountsIndex({ accounts, accountMetrics }: Props) {
|
|||
);
|
||||
}, []);
|
||||
|
||||
// Build a map of linked loan metrics keyed by real estate account ID
|
||||
// Build a map of linked loan metrics keyed by real estate account ID.
|
||||
// Intentionally reads the RAW (unsigned) accountMetrics prop, not the
|
||||
// signed accountsWithMetrics: the equity math subtracts this owed amount
|
||||
// (marketValue - mortgageOwed), so it must stay a positive magnitude.
|
||||
const linkedLoanMetricsMap = useMemo(() => {
|
||||
if (!accountMetrics) return {};
|
||||
const map: Record<
|
||||
|
|
|
|||
|
|
@ -21,8 +21,7 @@ beforeEach(function () {
|
|||
$this->actingAs($this->user);
|
||||
});
|
||||
|
||||
test('net worth calculates assets minus liabilities', function () {
|
||||
// Assets
|
||||
test('net worth excludes credit card balances entirely', function () {
|
||||
$checking = Account::factory()->create([
|
||||
'user_id' => $this->user->id,
|
||||
'type' => AccountType::Checking,
|
||||
|
|
@ -34,7 +33,8 @@ test('net worth calculates assets minus liabilities', function () {
|
|||
'balance' => 500000, // $5,000.00
|
||||
]);
|
||||
|
||||
// Liabilities
|
||||
// A credit card is a spending account, not wealth: it must neither add to
|
||||
// nor subtract from net worth.
|
||||
$creditCard = Account::factory()->create([
|
||||
'user_id' => $this->user->id,
|
||||
'type' => AccountType::CreditCard,
|
||||
|
|
@ -43,7 +43,7 @@ test('net worth calculates assets minus liabilities', function () {
|
|||
AccountBalance::factory()->create([
|
||||
'account_id' => $creditCard->id,
|
||||
'balance_date' => now(),
|
||||
'balance' => -100000, // -$1,000.00
|
||||
'balance' => 100000, // $1,000.00
|
||||
]);
|
||||
|
||||
// Previous period data (30 days ago)
|
||||
|
|
@ -55,7 +55,7 @@ test('net worth calculates assets minus liabilities', function () {
|
|||
AccountBalance::factory()->create([
|
||||
'account_id' => $creditCard->id,
|
||||
'balance_date' => now()->subDays(30),
|
||||
'balance' => -50000, // -$500.00
|
||||
'balance' => 50000, // $500.00
|
||||
]);
|
||||
|
||||
$response = $this->getJson('/api/dashboard/net-worth?'.http_build_query([
|
||||
|
|
@ -65,8 +65,8 @@ test('net worth calculates assets minus liabilities', function () {
|
|||
|
||||
$response->assertOk()
|
||||
->assertJson([
|
||||
'current' => 400000, // 5000 - 1000 = 4000
|
||||
'previous' => 350000, // 4000 - 500 = 3500
|
||||
'current' => 500000, // only the checking account counts
|
||||
'previous' => 400000,
|
||||
'currency_code' => 'USD',
|
||||
]);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -27,7 +27,6 @@ it('does not support invested amount for non-investment account types', function
|
|||
it('reduces net worth for liability account types', function (AccountType $type) {
|
||||
expect($type->reducesNetWorth())->toBeTrue();
|
||||
})->with([
|
||||
'credit card' => AccountType::CreditCard,
|
||||
'loan' => AccountType::Loan,
|
||||
]);
|
||||
|
||||
|
|
@ -35,6 +34,7 @@ it('does not reduce net worth for asset account types', function (AccountType $t
|
|||
expect($type->reducesNetWorth())->toBeFalse();
|
||||
})->with([
|
||||
'checking' => AccountType::Checking,
|
||||
'credit card' => AccountType::CreditCard,
|
||||
'savings' => AccountType::Savings,
|
||||
'investment' => AccountType::Investment,
|
||||
'retirement' => AccountType::Retirement,
|
||||
|
|
@ -42,6 +42,22 @@ it('does not reduce net worth for asset account types', function (AccountType $t
|
|||
'others' => AccountType::Others,
|
||||
]);
|
||||
|
||||
it('excludes credit cards from the net worth total', function () {
|
||||
expect(AccountType::CreditCard->countsInNetWorth())->toBeFalse();
|
||||
});
|
||||
|
||||
it('counts non credit card account types in the net worth total', function (AccountType $type) {
|
||||
expect($type->countsInNetWorth())->toBeTrue();
|
||||
})->with([
|
||||
'checking' => AccountType::Checking,
|
||||
'savings' => AccountType::Savings,
|
||||
'investment' => AccountType::Investment,
|
||||
'retirement' => AccountType::Retirement,
|
||||
'real estate' => AccountType::RealEstate,
|
||||
'loan' => AccountType::Loan,
|
||||
'others' => AccountType::Others,
|
||||
]);
|
||||
|
||||
it('is non-transactional for balance-only account types', function (AccountType $type) {
|
||||
expect($type->isNonTransactional())->toBeTrue();
|
||||
})->with([
|
||||
|
|
|
|||
Loading…
Reference in New Issue