fix(transactions): address split feature review findings

- keep editing an already-split transaction working when the flag is off:
  only resend splits when the split content actually changed or was undone
- make the dashboard cashflow summary and the Transaction Analysis screen
  split-aware (per-line category/type + tag attribution)
- skip split transactions on bulk label assignment (labels live on lines),
  matching the existing bulk-category skip
- reject splitting a zero-amount transaction
- split editor no longer shows a green 'Remaining: 0' with a dead Save button
  when a line is still zero
- add isSplit() helper, UUID split-line keys, clarify allocation docblocks

app/Http/Controllers/Api/TransactionAnalysisController.php

@@ -9,6 +9,7 @@ use App\Models\Label;
 use App\Models\Transaction;
 use App\Services\CategoryTree;
 use App\Services\ExchangeRateService;
+use App\Services\TransactionAllocations;
 use Carbon\Carbon;
 use Illuminate\Http\JsonResponse;
 use Illuminate\Support\Collection;
@@ -56,14 +57,19 @@ class TransactionAnalysisController extends Controller
 
         $transactions = Transaction::query()
             ->where('user_id', $user->id)
-            ->with(['account.bank', 'category', 'labels'])
+            ->with(['account.bank', 'category', 'labels', 'splits.category', 'splits.labels'])
             ->applyFilters($filters)
             ->get();
 
         $this->preloadExchangeRates($transactions, $currency);
 
-        $byCategory = $this->categoryBreakdown($transactions, $currency, $user->id);
-        $byTag = $this->tagBreakdown($transactions, $currency);
+        // Category and tag breakdowns are per-line for split transactions; payee,
+        // account and the totals stay at the transaction level (a split doesn't
+        // change who was paid or the amount that moved).
+        $allocations = TransactionAllocations::expand($transactions);
+
+        $byCategory = $this->categoryBreakdown($allocations, $currency, $user->id);
+        $byTag = $this->tagBreakdown($allocations, $currency);
         $byPayee = $this->payeeBreakdown($transactions, $currency);
         $byAccount = $this->accountBreakdown($transactions, $currency);
 

app/Http/Controllers/DashboardController.php

@@ -4,11 +4,11 @@ namespace App\Http\Controllers;
 
 use App\Enums\CategoryType;
 use App\Models\Account;
-use App\Models\Transaction;
 use App\Services\AccountMetricsService;
 use App\Services\CashflowSummaryService;
 use App\Services\CategorySpendingService;
 use App\Services\PeriodComparator;
+use App\Services\TransactionAllocations;
 use Carbon\Carbon;
 use Illuminate\Http\Request;
 use Illuminate\Support\Facades\DB;
@@ -110,9 +110,11 @@ class DashboardController extends Controller
 
     private function getTransactionSum(string $userId, Carbon $from, Carbon $to, CategoryType $type): int
     {
-        return Transaction::query()
-            ->where('transactions.user_id', $userId)
-            ->whereBetween('transactions.transaction_date', [$from, $to])
+        // Read effective allocations so a split's lines are classified by their
+        // own category type (a savings/transfer line no longer counts as a plain
+        // expense just because the parent row has no category).
+        return DB::query()
+            ->fromSub(TransactionAllocations::query($userId, $from, $to), 'transactions')
             ->where(function ($q) use ($type) {
                 $q->whereExists(function ($sub) use ($type) {
                     $sub->select(DB::raw(1))

app/Http/Controllers/TransactionController.php

@@ -353,16 +353,23 @@ class TransactionController extends Controller
             $transactions = $query->get();
         }
 
-        // Split transactions carry no single category, so a bulk category
-        // assignment skips them (it would silently destroy their lines) and we
-        // report how many were left untouched.
-        $omittedSplitIds = collect();
-        $updateData = [];
-        if ($request->has('category_id')) {
-            $newCategoryId = $request->input('category_id');
+        $labelIds = $request->input('label_ids');
+        $hasLabelUpdate = $request->has('label_ids');
+        $hasCategoryUpdate = $request->has('category_id');
 
+        // A bulk category or label assignment skips split transactions: setting a
+        // single category would destroy their lines, and labels live on the lines,
+        // not the row. Notes still apply (they are transaction-level). We report
+        // how many splits were left untouched.
+        $omittedSplitIds = collect();
+        if ($hasCategoryUpdate || $hasLabelUpdate) {
             $transactions->loadMissing('splits:id,transaction_id');
             $omittedSplitIds = $transactions->filter->isSplit()->pluck('id');
+        }
+
+        $updateData = [];
+        if ($hasCategoryUpdate) {
+            $newCategoryId = $request->input('category_id');
 
             $overrideHandler = app(CategoryOverrideHandler::class);
 
@@ -386,9 +393,6 @@ class TransactionController extends Controller
             $updateData['notes_iv'] = $request->input('notes_iv');
         }
 
-        $labelIds = $request->input('label_ids');
-        $hasLabelUpdate = $request->has('label_ids');
-
         if (empty($updateData) && ! $hasLabelUpdate) {
             return response()->json([
                 'message' => 'No update data provided.',
@@ -402,7 +406,7 @@ class TransactionController extends Controller
             } elseif ($filters !== null) {
                 $updateQuery->applyFilters($filters);
             }
-            if ($omittedSplitIds->isNotEmpty()) {
+            if ($hasCategoryUpdate && $omittedSplitIds->isNotEmpty()) {
                 $updateQuery->whereNotIn('id', $omittedSplitIds->all());
             }
             $updateQuery->update($updateData);
@@ -410,6 +414,10 @@ class TransactionController extends Controller
 
         if ($hasLabelUpdate) {
             foreach ($transactions as $transaction) {
+                if ($transaction->isSplit()) {
+                    continue;
+                }
+
                 $transaction->labels()->sync($labelIds ?? []);
                 $transaction->save();
             }

app/Http/Requests/UpdateTransactionRequest.php

@@ -75,6 +75,13 @@ class UpdateTransactionRequest extends FormRequest
             }
 
             $total = $this->effectiveAmount();
+
+            if ($total === 0) {
+                $validator->errors()->add('splits', __("You can't split a transaction with no amount."));
+
+                return;
+            }
+
             $sum = 0;
 
             foreach ($lines as $index => $line) {
@@ -83,7 +90,7 @@ class UpdateTransactionRequest extends FormRequest
 
                 if ($amount === 0) {
                     $validator->errors()->add("splits.{$index}.amount", __('Each split line must be a non-zero amount.'));
-                } elseif ($total !== 0 && ($amount > 0) !== ($total > 0)) {
+                } elseif (($amount > 0) !== ($total > 0)) {
                     $validator->errors()->add("splits.{$index}.amount", __('Split lines must have the same sign as the transaction.'));
                 }
             }

app/Services/TransactionAllocations.php

@@ -10,10 +10,13 @@ use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\DB;
 
 /**
- * The single source of truth for "effective category attribution": an unsplit
- * transaction is attributed to its own category, a split transaction to its
- * lines. Every category breakdown reads through here so a split never gets
- * double-counted or dropped.
+ * Effective category attribution: an unsplit transaction is attributed to its
+ * own category, a split transaction to its lines. The category breakdowns read
+ * through here — either the SQL {@see self::query()} or the collection
+ * {@see self::expand()} — so a split is never double-counted or dropped. The two
+ * encarnations share the "iterate lines vs the row itself" rule but not code:
+ * SQL sums raw rows, expand() needs hydrated models (account/category/labels)
+ * for currency conversion and side classification.
  */
 class TransactionAllocations
 {
@@ -50,10 +53,14 @@ class TransactionAllocations
     /**
      * Expand a loaded transaction collection into effective allocations: a split
      * transaction becomes one synthetic Transaction per line (carrying the line's
-     * amount + category while keeping the parent's account/currency/date for FX
-     * and side classification); unsplit transactions pass through unchanged.
+     * amount + category + labels while keeping the parent's account/currency/date
+     * for FX and side classification); unsplit transactions pass through
+     * unchanged.
      *
-     * Callers must eager-load `splits.category` (and `account` for FX).
+     * Callers must eager-load `splits.category` (and `splits.labels`/`account`
+     * where they read those). Synthetic line instances deliberately keep the
+     * parent's `id`, so consumers must aggregate by `category_id`/labels, never
+     * `keyBy('id')`/`unique('id')` (that would collapse a split's lines).
      *
      * @param  Collection<int, Transaction>  $transactions
      * @return Collection<int, Transaction>
@@ -76,6 +83,10 @@ class TransactionAllocations
                 $allocation->setRelation('account', $transaction->relationLoaded('account') ? $transaction->account : null);
                 $allocation->setRelation('category', $line->relationLoaded('category') ? $line->category : null);
 
+                if ($line->relationLoaded('labels')) {
+                    $allocation->setRelation('labels', $line->labels);
+                }
+
                 return $allocation;
             })->all();
         })->values();

lang/en.json

@@ -6,9 +6,11 @@
     "Remove line": "Remove line",
     "Undo split": "Undo split",
     "Remaining: :amount": "Remaining: :amount",
-    "Skipped :count split transactions": "Skipped :count split transactions",
+    "Each line needs a non-zero amount": "Each line needs a non-zero amount",
+    "Split transactions skipped: :count": "Split transactions skipped: :count",
     "A split needs at least two lines.": "A split needs at least two lines.",
     "The split lines must add up to the transaction amount.": "The split lines must add up to the transaction amount.",
     "Each split line must be a non-zero amount.": "Each split line must be a non-zero amount.",
+    "You can't split a transaction with no amount.": "You can't split a transaction with no amount.",
     "Split lines must have the same sign as the transaction.": "Split lines must have the same sign as the transaction."
 }

lang/es.json

@@ -2298,9 +2298,11 @@
     "Remove line": "Eliminar línea",
     "Undo split": "Deshacer división",
     "Remaining: :amount": "Restante: :amount",
-    "Skipped :count split transactions": "Se omitieron :count transacciones divididas",
+    "Each line needs a non-zero amount": "Cada línea necesita un importe distinto de cero",
+    "Split transactions skipped: :count": "Transacciones divididas omitidas: :count",
     "A split needs at least two lines.": "Una división necesita al menos dos líneas.",
     "The split lines must add up to the transaction amount.": "Las líneas de la división deben sumar el importe de la transacción.",
     "Each split line must be a non-zero amount.": "Cada línea de la división debe tener un importe distinto de cero.",
+    "You can't split a transaction with no amount.": "No puedes dividir una transacción sin importe.",
     "Split lines must have the same sign as the transaction.": "Las líneas de la división deben tener el mismo signo que la transacción."
 }

resources/js/components/transactions/category-cell.tsx

@@ -18,6 +18,7 @@ import { type SharedData } from '@/types';
 import { type Account, type Bank } from '@/types/account';
 import { getCategoryColorClasses, type Category } from '@/types/category';
 import {
+    isSplit,
     type DecryptedTransaction,
     type TransactionSplit,
 } from '@/types/transaction';
@@ -67,7 +68,7 @@ export function CategoryCell({
 
     // A split transaction shows its per-line breakdown instead of a single,
     // editable category. Editing happens in the modal (click the row).
-    if (transaction.splits && transaction.splits.length > 0) {
+    if (isSplit(transaction)) {
         return (
             <SplitBreakdown
                 transaction={transaction}

resources/js/components/transactions/edit-transaction-dialog.tsx

@@ -44,7 +44,7 @@ import {
 import { type AutomationRule } from '@/types/automation-rule';
 import { type Category } from '@/types/category';
 import { type Label } from '@/types/label';
-import { type DecryptedTransaction } from '@/types/transaction';
+import { isSplit, type DecryptedTransaction } from '@/types/transaction';
 import { formatDate } from '@/utils/date';
 import { __ } from '@/utils/i18n';
 import { router, usePage } from '@inertiajs/react';
@@ -124,7 +124,7 @@ export function EditTransactionDialog({
     const splittingEnabled =
         usePage<SharedData>().props.features.transactionSplitting;
     const isSplitting = splitLines.length > 0;
-    const wasSplit = (transaction?.splits?.length ?? 0) > 0;
+    const wasSplit = transaction ? isSplit(transaction) : false;
 
     function startSplit() {
         // Seed with the current category as the first line at the full amount,
@@ -496,8 +496,32 @@ export function EditTransactionDialog({
 
                 // A split change turns one row into several (or back). Persist it
                 // and let the server-rendered list re-fetch rather than patch the
-                // grouped rows by hand.
-                if (isSplitting || wasSplit) {
+                // grouped rows by hand. Only send `splits` when the split content
+                // actually changed (or was undone) — editing another field of an
+                // already-split transaction must not re-trigger the gated split
+                // write, so it keeps working even if the feature is rolled back.
+                const originalSplitSignature = JSON.stringify(
+                    (transaction.splits ?? []).map((split) => ({
+                        category_id: split.category_id,
+                        amount: split.amount,
+                        label_ids: [
+                            ...(split.labels?.map((label) => label.id) ?? []),
+                        ].sort(),
+                    })),
+                );
+                const currentSplitSignature = JSON.stringify(
+                    splitLines.map((line) => ({
+                        category_id: line.category_id,
+                        amount: line.amount,
+                        label_ids: [...line.label_ids].sort(),
+                    })),
+                );
+                const splitDirty =
+                    (isSplitting &&
+                        currentSplitSignature !== originalSplitSignature) ||
+                    (!isSplitting && wasSplit);
+
+                if (splitDirty) {
                     if (isSplitting) {
                         if (!isSplitValid(amount, splitLines)) {
                             toast.error(

resources/js/components/transactions/split-editor.test.tsx

@@ -37,4 +37,8 @@ describe('isSplitValid', () => {
     it('rejects a line whose sign differs from the total', () => {
         expect(isSplitValid(-10000, lines(-12500, 2500))).toBe(false);
     });
+
+    it('rejects splitting a zero-total transaction', () => {
+        expect(isSplitValid(0, lines(50, -50))).toBe(false);
+    });
 });

resources/js/components/transactions/split-editor.tsx

@@ -26,15 +26,11 @@ interface SplitEditorProps {
     onLabelCreated?: (label: Label) => void;
 }
 
-let keyCounter = 0;
-
 export function newSplitLine(
     partial?: Partial<SplitLineDraft>,
 ): SplitLineDraft {
-    keyCounter += 1;
-
     return {
-        key: `split-${keyCounter}`,
+        key: crypto.randomUUID(),
         category_id: null,
         amount: 0,
         label_ids: [],
@@ -48,17 +44,16 @@ export function splitRemaining(total: number, lines: SplitLineDraft[]): number {
 
 /**
  * A split is valid once it has at least two lines that each carry a non-zero
- * amount on the same side as the transaction and together reconcile to its
- * total to the cent.
+ * amount on the same side as the (non-zero) transaction total and together
+ * reconcile to it to the cent.
  */
 export function isSplitValid(total: number, lines: SplitLineDraft[]): boolean {
-    if (lines.length < 2 || splitRemaining(total, lines) !== 0) {
+    if (total === 0 || lines.length < 2 || splitRemaining(total, lines) !== 0) {
         return false;
     }
 
     return lines.every(
-        (line) =>
-            line.amount !== 0 && (total === 0 || line.amount > 0 === total > 0),
+        (line) => line.amount !== 0 && line.amount > 0 === total > 0,
     );
 }
 
@@ -88,6 +83,11 @@ export function SplitEditor({
     const addLine = () =>
         onChange([...lines, newSplitLine({ amount: remaining })]);
 
+    const valid = isSplitValid(total, lines);
+    // Remaining can be 0 while the split is still invalid (e.g. a seeded
+    // zero-amount line), so key the styling off validity, not just the balance.
+    const hasZeroLine = lines.some((line) => line.amount === 0);
+
     const formattedRemaining = new Intl.NumberFormat(locale, {
         style: 'currency',
         currency: currencyCode,
@@ -168,13 +168,17 @@ export function SplitEditor({
                 </Button>
                 <span
                     className={
-                        remaining === 0
+                        valid
                             ? 'text-sm text-muted-foreground'
                             : 'text-sm font-medium text-destructive'
                     }
                     data-testid="split-remaining"
                 >
-                    {__('Remaining: :amount', { amount: formattedRemaining })}
+                    {remaining === 0 && hasZeroLine
+                        ? __('Each line needs a non-zero amount')
+                        : __('Remaining: :amount', {
+                              amount: formattedRemaining,
+                          })}
                 </span>
             </div>
         </div>

resources/js/pages/transactions/index.tsx

@@ -104,6 +104,7 @@ import {
 import { type Category } from '@/types/category';
 import { type Label } from '@/types/label';
 import {
+    isSplit,
     type DecryptedTransaction,
     type TransactionFilters as Filters,
     type ServerTransaction,
@@ -1139,7 +1140,7 @@ export default function Transactions({
                 );
                 if (response.data.omitted_splits > 0) {
                     toast.info(
-                        __('Skipped :count split transactions', {
+                        __('Split transactions skipped: :count', {
                             count: response.data.omitted_splits,
                         }),
                     );
@@ -1159,10 +1160,7 @@ export default function Transactions({
                 // (it would wipe their lines), so skip them and report how many.
                 const splitIds = new Set(
                     allTransactions
-                        .filter(
-                            (transaction) =>
-                                (transaction.splits?.length ?? 0) > 0,
-                        )
+                        .filter((transaction) => isSplit(transaction))
                         .map((transaction) => transaction.id.toString()),
                 );
                 const targetIds = selectedIds.filter((id) => !splitIds.has(id));
@@ -1195,7 +1193,7 @@ export default function Transactions({
 
                 if (omitted > 0) {
                     toast.info(
-                        __('Skipped :count split transactions', {
+                        __('Split transactions skipped: :count', {
                             count: omitted,
                         }),
                     );

resources/js/types/transaction.ts

@@ -52,6 +52,11 @@ export interface Transaction {
     updated_at: string;
 }
 
+/** Whether a transaction is split into per-category lines. */
+export function isSplit(transaction: Pick<Transaction, 'splits'>): boolean {
+    return (transaction.splits?.length ?? 0) > 0;
+}
+
 export interface ServerTransaction extends Transaction {
     account?: Account;
     category?: Category | null;

tests/Feature/TransactionSplitTest.php

@@ -299,6 +299,86 @@ it('does not treat a fully-categorized split as uncategorized', function () {
     expect($matches->all())->not->toContain($split->id);
 });
 
+it('rejects splitting a zero-amount transaction', function () {
+    $category = expenseCategory($this->user);
+    $transaction = splittableTransaction($this->user, 0);
+
+    actingAs($this->user)
+        ->patchJson(route('transactions.update', $transaction), [
+            'splits' => [
+                ['category_id' => $category->id, 'amount' => 50],
+                ['category_id' => $category->id, 'amount' => -50],
+            ],
+        ])
+        ->assertJsonValidationErrors(['splits']);
+});
+
+it('classifies split lines by their own category type in the dashboard cash flow', function () {
+    $expense = expenseCategory($this->user, 'Groceries');
+    $savings = Category::factory()->for($this->user)->create([
+        'name' => 'Savings',
+        'type' => CategoryType::Savings,
+        'cashflow_direction' => CategoryCashflowDirection::Outflow,
+    ]);
+    $transaction = splittableTransaction($this->user, -10000);
+    $transaction->update(['transaction_date' => now()]);
+
+    app(TransactionSplitter::class)->apply($transaction, [
+        ['category_id' => $expense->id, 'amount' => -3000],
+        ['category_id' => $savings->id, 'amount' => -7000],
+    ]);
+
+    $response = actingAs($this->user)->getJson(
+        '/api/dashboard/cash-flow?from='.now()->startOfMonth()->toDateString().'&to='.now()->endOfMonth()->toDateString(),
+    )->assertOk();
+
+    // Only the expense line counts as expense; the savings line is excluded.
+    expect($response->json('current.expense'))->toBe(3000);
+});
+
+it('attributes split lines per category on the analysis screen', function () {
+    $restaurant = expenseCategory($this->user, 'Restaurant');
+    $travel = expenseCategory($this->user, 'Travel');
+    $transaction = splittableTransaction($this->user, -10000);
+    $transaction->update(['transaction_date' => now()]);
+
+    app(TransactionSplitter::class)->apply($transaction, [
+        ['category_id' => $restaurant->id, 'amount' => -2500],
+        ['category_id' => $travel->id, 'amount' => -7500],
+    ]);
+
+    $response = actingAs($this->user)->getJson(
+        '/api/transactions/analysis?date_from='.now()->subDay()->toDateString().'&date_to='.now()->addDay()->toDateString(),
+    )->assertOk();
+
+    $byCategory = collect($response->json('by_category'))->keyBy('category_id');
+
+    expect($byCategory[$restaurant->id]['amount'])->toBe(2500);
+    expect($byCategory[$travel->id]['amount'])->toBe(7500);
+});
+
+it('skips split transactions on a bulk label assignment', function () {
+    $category = expenseCategory($this->user);
+    $label = Label::factory()->for($this->user)->create();
+    $split = splittableTransaction($this->user, -10000);
+    $plain = splittableTransaction($this->user, -5000);
+
+    app(TransactionSplitter::class)->apply($split, [
+        ['category_id' => $category->id, 'amount' => -2500],
+        ['category_id' => $category->id, 'amount' => -7500],
+    ]);
+
+    actingAs($this->user)
+        ->patchJson(route('transactions.bulk-update'), [
+            'transaction_ids' => [$split->id, $plain->id],
+            'label_ids' => [$label->id],
+        ])
+        ->assertOk();
+
+    expect($split->fresh()->labels)->toHaveCount(0);
+    expect($plain->fresh()->labels->pluck('id')->all())->toContain($label->id);
+});
+
 it('skips split transactions on a bulk category assignment', function () {
     $category = expenseCategory($this->user);
     $newCategory = expenseCategory($this->user, 'New');