Fix merging split transactions (#6240)

* fix(transactions): preserve split categories when merging with imported uncategorized tx; avoid orphaned subtransactions

When merging a split-categorized manual transaction with an uncategorized
imported one, keep the split lines (including categoryId) and avoid leaving
orphaned subtransactions by properly transferring parent_id references.

Fixes #5801.

* docs: add release note for PR #5856

* fix: resolve lint and typecheck errors

- Rename unused variables sub1, sub2 to _sub1, _sub2 to fix lint warnings
- Fix typecheck error by using double type assertion (unknown) in merge.ts

* refactor: optimize subtransaction queries and test db access

Address code review feedback:
- Merge two SQL queries into one using IN clause for better performance
- Use db.first in test to avoid conversion path and match db.all semantics

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* [autofix.ci] apply automated fixes

* refactor: apply code review feedback with stable branch logic

- Revert test to use db.getTransaction (not db.first) to preserve type conversion
- Refactor SQL query to use explicit branches (IN for 2 parents, = for 1, skip for 0)
- This approach is more stable across different SQL drivers

Addresses: #5856 (review feedback from @joel-jeremy)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* chore(release-notes): set category to Enhancements and shorten text; fix(merge): use shared deleteTransaction with grouped fetch + batchUpdateTransactions

* resolving lint warnings and recommendation on the test query

* switching to use the getTransaction helper

* corrected is_parent assertion to be true

* updated the PR number

---------

Co-authored-by: Golenspade <2023004079@mails.cust.edu.cn>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: fankex <2112325885@qq.com>
Co-authored-by: youngcw <calebyoung94@gmail.com>

packages/loot-core/src/server/transactions/merge.test.ts

@@ -283,4 +283,88 @@ describe('Merging success', () => {
       imported_id: 'imported_1',
     });
   });
+
+  it('preserves split categories when merging split transaction with uncategorized imported transaction', async () => {
+    // Create a manual transaction with splits
+    const manualParent = await db.insertTransaction({
+      account: 'one',
+      amount: 100,
+      date: '2025-01-01',
+      is_parent: true,
+      category: null,
+    });
+
+    const _sub1 = await db.insertTransaction({
+      account: 'one',
+      amount: 60,
+      date: '2025-01-01',
+      category: '1',
+      notes: 'Food',
+      is_child: true,
+      parent_id: manualParent,
+    });
+
+    const _sub2 = await db.insertTransaction({
+      account: 'one',
+      amount: 40,
+      date: '2025-01-01',
+      category: '2',
+      notes: 'Transport',
+      is_child: true,
+      parent_id: manualParent,
+    });
+
+    // Create an uncategorized imported transaction
+    const imported = await db.insertTransaction({
+      account: 'one',
+      amount: 100,
+      date: '2025-01-02',
+      imported_id: 'imported_1',
+      category: null,
+    });
+
+    // Merge: imported transaction should be kept (because it has imported_id)
+    const keptId = await mergeTransactions([
+      { id: manualParent },
+      { id: imported },
+    ]);
+    expect(keptId).toBe(imported);
+
+    // Check that the kept transaction is now a parent
+    const keptTransaction = await db.getTransaction(imported);
+    expect(keptTransaction?.is_parent).toBe(true);
+    expect(keptTransaction?.category).toBeNull();
+    expect(keptTransaction?.imported_id).toBe('imported_1');
+
+    // Check that subtransactions were transferred and still exist
+    const allTransactions = await getAllTransactions();
+    const childTransactions = allTransactions.filter(
+      t => t.parent_id === imported,
+    );
+    expect(childTransactions.length).toBe(2);
+
+    // Verify the subtransactions have the correct parent_id and preserved their categories
+    const child1 = childTransactions.find(t => t.amount === 60);
+    const child2 = childTransactions.find(t => t.amount === 40);
+
+    expect(child1).toMatchObject({
+      parent_id: imported,
+      category: '1',
+      notes: 'Food',
+      is_child: 1, // From getAllTransactions (db.all), not converted
+    });
+
+    expect(child2).toMatchObject({
+      parent_id: imported,
+      category: '2',
+      notes: 'Transport',
+      is_child: 1, // From getAllTransactions (db.all), not converted
+    });
+
+    // Verify no orphaned subtransactions remain
+    const orphanedChildren = allTransactions.filter(
+      t => t.parent_id === manualParent,
+    );
+    expect(orphanedChildren.length).toBe(0);
+  });
 });

packages/loot-core/src/server/transactions/merge.ts

@@ -1,6 +1,14 @@
+import { q } from '../../shared/query';
+import {
+  ungroupTransactions,
+  deleteTransaction as sharedDeleteTransaction,
+} from '../../shared/transactions';
 import { type TransactionEntity } from '../../types/models';
+import { aqlQuery } from '../aql';
 import * as db from '../db';
 
+import { batchUpdateTransactions } from '.';
+
 export async function mergeTransactions(
   transactions: Pick<TransactionEntity, 'id'>[],
 ): Promise<TransactionEntity['id']> {
@@ -24,17 +32,82 @@ export async function mergeTransactions(
   }
   const { keep, drop } = determineKeepDrop(a, b);
 
-  await Promise.all([
-    db.updateTransaction({
+  // Load subtransactions with a single query, then split by parent_id in memory
+  const keepSubtransactions: TransactionEntity[] = [];
+  const dropSubtransactions: TransactionEntity[] = [];
+  const parents: string[] = [];
+  if (keep.is_parent) parents.push(keep.id);
+  if (drop.is_parent) parents.push(drop.id);
+
+  let rows: TransactionEntity[] = [];
+  if (parents.length === 2) {
+    rows = await db.all<TransactionEntity>(
+      'SELECT * FROM v_transactions WHERE parent_id IN (?, ?)',
+      parents,
+    );
+  } else if (parents.length === 1) {
+    rows = await db.all<TransactionEntity>(
+      'SELECT * FROM v_transactions WHERE parent_id = ?',
+      parents,
+    );
+  } // else: both are non-parents → rows stays []
+
+  for (const row of rows) {
+    if (row.parent_id === keep.id) keepSubtransactions.push(row);
+    else if (row.parent_id === drop.id) dropSubtransactions.push(row);
+  }
+
+  // Determine which transaction has subtransactions (split categories)
+  const keepHasSubtransactions = keepSubtransactions.length > 0;
+  const dropHasSubtransactions = dropSubtransactions.length > 0;
+
+  // If keep doesn't have subtransactions but drop does, transfer them
+  if (!keepHasSubtransactions && dropHasSubtransactions) {
+    // Update each subtransaction to point to the kept parent
+    await Promise.all(
+      dropSubtransactions.map(sub =>
+        db.updateTransaction({
+          id: sub.id,
+          parent_id: keep.id,
+        } as TransactionEntity),
+      ),
+    );
+    // Mark keep as a parent transaction
+    await db.updateTransaction({
+      id: keep.id,
+      is_parent: true,
+      category: null, // Parent transactions with splits shouldn't have a category
+      payee: keep.payee || drop.payee,
+      notes: keep.notes || drop.notes,
+      cleared: keep.cleared || drop.cleared,
+      reconciled: keep.reconciled || drop.reconciled,
+    } as unknown as TransactionEntity);
+  } else {
+    // Normal merge without subtransactions
+    await db.updateTransaction({
       id: keep.id,
       payee: keep.payee || drop.payee,
       category: keep.category || drop.category,
       notes: keep.notes || drop.notes,
       cleared: keep.cleared || drop.cleared,
       reconciled: keep.reconciled || drop.reconciled,
-    } as TransactionEntity),
-    db.deleteTransaction(drop),
-  ]);
+    } as TransactionEntity);
+  }
+
+  // Delete the dropped transaction using shared deleteTransaction to
+  // intelligently handle possible parent/child cascading logic
+  const { data: transactionsToDelete } = await aqlQuery(
+    q('transactions')
+      .filter({ id: drop.id })
+      .select('*')
+      .options({ splits: 'grouped' }),
+  );
+  const ungroupedTransactions = ungroupTransactions(transactionsToDelete);
+  if (ungroupedTransactions.length > 0) {
+    const { diff } = sharedDeleteTransaction(ungroupedTransactions, drop.id);
+    await batchUpdateTransactions(diff);
+  }
+
   return keep.id;
 }
 

upcoming-release-notes/6240.md

@@ -0,0 +1,6 @@
+---
+category: Enhancements
+authors: [Golenspade, Respheal]
+---
+
+Preserve splits when manually merging transactions