Fix Sentry transaction and dashboard crashes (#372)
## Summary - guard transaction list Inertia callbacks against non-paginated transaction props - exclude soft-deleted categories from dashboard category aggregates - add dashboard category and cursor pagination tests Fixes PHP-LARAVEL-1K Fixes PHP-LARAVEL-1J ## Tests - npm run test -- cursor-pagination.test.ts - php artisan test --compact tests/Feature/DashboardAnalyticsTest.php --filter='top categories' - vendor/bin/pint --dirty --format agent Note: npm run types still fails on existing unrelated TypeScript errors.
This commit is contained in:
parent
f4ab4a1989
commit
718cfa9e8f
|
|
@ -463,7 +463,8 @@ class DashboardAnalyticsController extends Controller
|
|||
->whereBetween('transactions.transaction_date', [$from, $to])
|
||||
->join('categories', function ($join) {
|
||||
$join->on('transactions.category_id', '=', 'categories.id')
|
||||
->where('categories.type', '=', CategoryType::Expense);
|
||||
->where('categories.type', '=', CategoryType::Expense)
|
||||
->whereNull('categories.deleted_at');
|
||||
})
|
||||
->select('transactions.category_id', DB::raw('sum(transactions.amount) as total_amount'))
|
||||
->groupBy('transactions.category_id')
|
||||
|
|
@ -513,7 +514,8 @@ class DashboardAnalyticsController extends Controller
|
|||
->whereBetween('transactions.transaction_date', [$from, $to])
|
||||
->join('categories', function ($join) {
|
||||
$join->on('transactions.category_id', '=', 'categories.id')
|
||||
->where('categories.type', '=', CategoryType::Expense);
|
||||
->where('categories.type', '=', CategoryType::Expense)
|
||||
->whereNull('categories.deleted_at');
|
||||
})
|
||||
->sum('transactions.amount');
|
||||
|
||||
|
|
@ -527,7 +529,8 @@ class DashboardAnalyticsController extends Controller
|
|||
->whereBetween('transactions.transaction_date', [$from, $to])
|
||||
->join('categories', function ($join) {
|
||||
$join->on('transactions.category_id', '=', 'categories.id')
|
||||
->where('categories.type', '=', CategoryType::Income);
|
||||
->where('categories.type', '=', CategoryType::Income)
|
||||
->whereNull('categories.deleted_at');
|
||||
})
|
||||
->sum('transactions.amount');
|
||||
|
||||
|
|
@ -536,7 +539,8 @@ class DashboardAnalyticsController extends Controller
|
|||
->whereBetween('transactions.transaction_date', [$from, $to])
|
||||
->join('categories', function ($join) {
|
||||
$join->on('transactions.category_id', '=', 'categories.id')
|
||||
->where('categories.type', '=', CategoryType::Expense);
|
||||
->where('categories.type', '=', CategoryType::Expense)
|
||||
->whereNull('categories.deleted_at');
|
||||
})
|
||||
->sum('transactions.amount');
|
||||
|
||||
|
|
|
|||
|
|
@ -97,7 +97,8 @@ class DashboardController extends Controller
|
|||
->whereBetween('transactions.transaction_date', [$from, $to])
|
||||
->join('categories', function ($join) {
|
||||
$join->on('transactions.category_id', '=', 'categories.id')
|
||||
->where('categories.type', '=', CategoryType::Expense);
|
||||
->where('categories.type', '=', CategoryType::Expense)
|
||||
->whereNull('categories.deleted_at');
|
||||
})
|
||||
->select('transactions.category_id', DB::raw('sum(transactions.amount) as total_amount'))
|
||||
->groupBy('transactions.category_id')
|
||||
|
|
|
|||
|
|
@ -11,7 +11,11 @@ import { Progress } from '@/components/ui/progress';
|
|||
import { useChartColors } from '@/hooks/use-chart-color-scheme';
|
||||
import { cn } from '@/lib/utils';
|
||||
import { SharedData } from '@/types';
|
||||
import { Category, getCategoryColorClasses } from '@/types/category';
|
||||
import {
|
||||
Category,
|
||||
type CategoryColor,
|
||||
getCategoryColorClasses,
|
||||
} from '@/types/category';
|
||||
import { __ } from '@/utils/i18n';
|
||||
import { Link, usePage } from '@inertiajs/react';
|
||||
import { format, subDays } from 'date-fns';
|
||||
|
|
@ -20,7 +24,8 @@ import { LucideIcon } from 'lucide-react';
|
|||
import { PercentageTrendIndicator } from './percentage-trend-indicator';
|
||||
|
||||
interface CategoryData {
|
||||
category: Category;
|
||||
category: Category | null;
|
||||
category_id?: string | null;
|
||||
amount: number;
|
||||
previous_amount: number;
|
||||
total_amount: number;
|
||||
|
|
@ -75,8 +80,16 @@ export function TopCategoriesCard({
|
|||
<CardContent>
|
||||
<div className="space-y-4">
|
||||
{categories.map((item, index) => {
|
||||
const category = item.category;
|
||||
const categoryId =
|
||||
category?.id ?? item.category_id ?? 'uncategorized';
|
||||
const categoryName =
|
||||
category?.name ?? __('Uncategorized');
|
||||
const categoryIcon = category?.icon ?? 'HelpCircle';
|
||||
const categoryColorName =
|
||||
category?.color ?? ('gray' as CategoryColor);
|
||||
const Icon = (Icons[
|
||||
item.category.icon as keyof typeof Icons
|
||||
categoryIcon as keyof typeof Icons
|
||||
] || Icons.HelpCircle) as LucideIcon;
|
||||
|
||||
const percentageChange =
|
||||
|
|
@ -89,17 +102,16 @@ export function TopCategoriesCard({
|
|||
item.total_amount > 0
|
||||
? (item.amount / item.total_amount) * 100
|
||||
: 0;
|
||||
const categoryColor = getCategoryColorClasses(
|
||||
item.category.color,
|
||||
);
|
||||
const categoryColor =
|
||||
getCategoryColorClasses(categoryColorName);
|
||||
const chartColor = categoryBarColor(
|
||||
item.category.color,
|
||||
categoryColorName,
|
||||
index,
|
||||
);
|
||||
|
||||
const categoryUrl = transactionsIndex({
|
||||
query: {
|
||||
category_ids: item.category.id,
|
||||
category_ids: categoryId,
|
||||
date_from: dateFrom,
|
||||
date_to: dateTo,
|
||||
},
|
||||
|
|
@ -107,9 +119,9 @@ export function TopCategoriesCard({
|
|||
|
||||
return (
|
||||
<Link
|
||||
key={item.category.id}
|
||||
key={categoryId}
|
||||
href={categoryUrl}
|
||||
className="-mx-1.5 my-1.5 block space-y-2 rounded-md px-1.5 py-1 transition-colors hover:bg-muted group"
|
||||
className="group -mx-1.5 my-1.5 block space-y-2 rounded-md px-1.5 py-1 transition-colors hover:bg-muted"
|
||||
>
|
||||
<div className="flex min-w-0 items-center gap-2">
|
||||
<div
|
||||
|
|
@ -121,7 +133,7 @@ export function TopCategoriesCard({
|
|||
<Icon className="size-4" />
|
||||
</div>
|
||||
<span className="min-w-0 flex-1 truncate text-sm font-medium">
|
||||
{item.category.name}
|
||||
{categoryName}
|
||||
</span>
|
||||
{percentageChange !== null && (
|
||||
<PercentageTrendIndicator
|
||||
|
|
|
|||
|
|
@ -0,0 +1,23 @@
|
|||
import { describe, expect, it } from 'vitest';
|
||||
import { isCursorPaginatedResponse } from './cursor-pagination';
|
||||
|
||||
describe('isCursorPaginatedResponse', () => {
|
||||
it('accepts cursor-paginated Inertia props', () => {
|
||||
expect(
|
||||
isCursorPaginatedResponse({
|
||||
data: [],
|
||||
next_cursor: null,
|
||||
next_page_url: null,
|
||||
prev_cursor: null,
|
||||
prev_page_url: null,
|
||||
per_page: 50,
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects collection props from non-index pages', () => {
|
||||
expect(isCursorPaginatedResponse([])).toBe(false);
|
||||
expect(isCursorPaginatedResponse({ transactions: [] })).toBe(false);
|
||||
expect(isCursorPaginatedResponse(undefined)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,20 @@
|
|||
export interface CursorPaginatedResponse<T> {
|
||||
data: T[];
|
||||
next_cursor: string | null;
|
||||
next_page_url: string | null;
|
||||
prev_cursor: string | null;
|
||||
prev_page_url: string | null;
|
||||
per_page: number;
|
||||
}
|
||||
|
||||
export function isCursorPaginatedResponse<T = unknown>(
|
||||
value: unknown,
|
||||
): value is CursorPaginatedResponse<T> {
|
||||
if (!value || typeof value !== 'object') {
|
||||
return false;
|
||||
}
|
||||
|
||||
const candidate = value as { data?: unknown };
|
||||
|
||||
return Array.isArray(candidate.data);
|
||||
}
|
||||
|
|
@ -62,6 +62,10 @@ import { Skeleton } from '@/components/ui/skeleton';
|
|||
import { Spinner } from '@/components/ui/spinner';
|
||||
import { TableCell, TableRow } from '@/components/ui/table';
|
||||
import AppSidebarLayout from '@/layouts/app/app-sidebar-layout';
|
||||
import {
|
||||
type CursorPaginatedResponse,
|
||||
isCursorPaginatedResponse,
|
||||
} from '@/lib/cursor-pagination';
|
||||
import { consoleDebug } from '@/lib/debug';
|
||||
import { cn } from '@/lib/utils';
|
||||
import { transactionSyncService } from '@/services/transaction-sync';
|
||||
|
|
@ -95,17 +99,8 @@ interface AppliedFilters {
|
|||
sort: string;
|
||||
}
|
||||
|
||||
interface CursorPaginatedResponse {
|
||||
data: ServerTransaction[];
|
||||
next_cursor: string | null;
|
||||
next_page_url: string | null;
|
||||
prev_cursor: string | null;
|
||||
prev_page_url: string | null;
|
||||
per_page: number;
|
||||
}
|
||||
|
||||
interface Props {
|
||||
transactions: CursorPaginatedResponse;
|
||||
transactions: CursorPaginatedResponse<ServerTransaction>;
|
||||
appliedFilters: AppliedFilters;
|
||||
categories: Category[];
|
||||
accounts: Account[];
|
||||
|
|
@ -457,8 +452,15 @@ export default function Transactions({
|
|||
preserveScroll: true,
|
||||
preserveState: true,
|
||||
onSuccess: (page) => {
|
||||
const txns = (page.props as unknown as Props)
|
||||
const txns = (page.props as { transactions?: unknown })
|
||||
.transactions;
|
||||
|
||||
if (
|
||||
!isCursorPaginatedResponse<ServerTransaction>(txns)
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
setAllTransactions(
|
||||
txns.data.map(toDecryptedTransaction),
|
||||
);
|
||||
|
|
@ -527,7 +529,13 @@ export default function Transactions({
|
|||
router.reload({
|
||||
only: ['transactions'],
|
||||
onSuccess: (page) => {
|
||||
const txns = (page.props as unknown as Props).transactions;
|
||||
const txns = (page.props as { transactions?: unknown })
|
||||
.transactions;
|
||||
|
||||
if (!isCursorPaginatedResponse<ServerTransaction>(txns)) {
|
||||
return;
|
||||
}
|
||||
|
||||
setAllTransactions(txns.data.map(toDecryptedTransaction));
|
||||
setNextCursor(txns.next_cursor);
|
||||
},
|
||||
|
|
@ -550,7 +558,7 @@ export default function Transactions({
|
|||
const response = await fetch(url, {
|
||||
headers: {
|
||||
'X-Inertia': 'true',
|
||||
'X-Inertia-Version': version,
|
||||
'X-Inertia-Version': version ?? '',
|
||||
'X-Inertia-Partial-Data': 'transactions',
|
||||
'X-Inertia-Partial-Component': component,
|
||||
Accept: 'text/html, application/xhtml+xml',
|
||||
|
|
@ -558,7 +566,11 @@ export default function Transactions({
|
|||
});
|
||||
|
||||
const json = await response.json();
|
||||
const next = json.props.transactions as CursorPaginatedResponse;
|
||||
const next = json.props.transactions;
|
||||
|
||||
if (!isCursorPaginatedResponse<ServerTransaction>(next)) {
|
||||
return;
|
||||
}
|
||||
|
||||
setAllTransactions((prev) => [
|
||||
...prev,
|
||||
|
|
|
|||
|
|
@ -247,6 +247,44 @@ test('top categories returns highest spending categories', function () {
|
|||
expect($data[1]['amount'])->toBe(3000);
|
||||
});
|
||||
|
||||
test('top categories excludes soft deleted categories', function () {
|
||||
$activeCategory = Category::factory()->create([
|
||||
'user_id' => $this->user->id,
|
||||
'type' => CategoryType::Expense,
|
||||
'name' => 'Food',
|
||||
]);
|
||||
$deletedCategory = Category::factory()->create([
|
||||
'user_id' => $this->user->id,
|
||||
'type' => CategoryType::Expense,
|
||||
'name' => 'Old category',
|
||||
]);
|
||||
|
||||
Transaction::factory()->create([
|
||||
'user_id' => $this->user->id,
|
||||
'category_id' => $activeCategory->id,
|
||||
'amount' => -1000,
|
||||
'transaction_date' => now(),
|
||||
]);
|
||||
Transaction::factory()->create([
|
||||
'user_id' => $this->user->id,
|
||||
'category_id' => $deletedCategory->id,
|
||||
'amount' => -5000,
|
||||
'transaction_date' => now(),
|
||||
]);
|
||||
|
||||
$deletedCategory->delete();
|
||||
|
||||
$response = $this->getJson('/api/dashboard/top-categories?'.http_build_query([
|
||||
'from' => now()->startOfMonth()->toDateString(),
|
||||
'to' => now()->endOfMonth()->toDateString(),
|
||||
]));
|
||||
|
||||
$response->assertOk();
|
||||
|
||||
expect($response->json())->toHaveCount(1)
|
||||
->and($response->json('0.category.id'))->toBe($activeCategory->id);
|
||||
});
|
||||
|
||||
test('net worth evolution returns monthly data points with per-account balances', function () {
|
||||
$account1 = Account::factory()->create([
|
||||
'user_id' => $this->user->id,
|
||||
|
|
|
|||
Loading…
Reference in New Issue