From 166f82d0bfdc598099c088275d68dc42499694f9 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Thu, 2 Jul 2026 09:10:46 +0200 Subject: feat: tx add --from-id/--to-id to disambiguate same-name accounts (v0.3.7) ISSUES.md #2: two accounts can share a name (e.g. expense id 52 and revenue id 129 both "Nexi"), making --from/--to ambiguous and unresolvable. Add --from-id/--to-id to target an account by numeric id. Per side, exactly one of the name flag or the id flag; sides independent. The id path fetches the account (resolver.account_by_id), validating existence before writing. Name-only callers unchanged; JSON/exit unchanged, so PATCH. Co-Authored-By: Claude Opus 4.8 --- tests/unit/test_commands_transaction.py | 93 ++++++++++++++++++++++++++++----- tests/unit/test_resolver.py | 10 ++++ 2 files changed, 91 insertions(+), 12 deletions(-) (limited to 'tests/unit') diff --git a/tests/unit/test_commands_transaction.py b/tests/unit/test_commands_transaction.py index 28882d3..5f455cb 100644 --- a/tests/unit/test_commands_transaction.py +++ b/tests/unit/test_commands_transaction.py @@ -19,7 +19,7 @@ class TestTxAdd(unittest.TestCase): "attributes": {}}} args = MagicMock(amount="42.50", source="Checking", dest="Groceries", desc="food", date="2026-06-30", category=None, - tags=None, type=None, dry_run=False, skip_dupes=False) + tags=None, type=None, dry_run=False, skip_dupes=False, source_id=None, dest_id=None) rc = tx.cmd_add(args, ctx) self.assertEqual(rc, 0) method, path = client.request.call_args[0][:2] @@ -40,7 +40,7 @@ class TestTxAdd(unittest.TestCase): client.request.return_value = {"data": {"id": "1", "attributes": {}}} args = MagicMock(amount="1000", source="Salary", dest="Checking", desc="pay", date=None, category=None, tags=None, - type=None, dry_run=False, skip_dupes=False) + type=None, dry_run=False, skip_dupes=False, source_id=None, dest_id=None) tx.cmd_add(args, ctx) self.assertEqual(client.request.call_args[1]["body"]["transactions"][0]["type"], "deposit") @@ -51,7 +51,7 @@ class TestTxAdd(unittest.TestCase): client.request.return_value = {"data": {"id": "1", "attributes": {}}} args = MagicMock(amount="5", source="A", dest="B", desc=None, date=None, category=None, tags="food,fun", type="transfer", - dry_run=False, skip_dupes=False) + dry_run=False, skip_dupes=False, source_id=None, dest_id=None) tx.cmd_add(args, ctx) split = client.request.call_args[1]["body"]["transactions"][0] self.assertEqual(split["type"], "transfer") @@ -64,7 +64,7 @@ class TestTxAdd(unittest.TestCase): client.request.return_value = {"data": {"id": "1", "attributes": {}}} args = MagicMock(amount="5", source="A", dest="B", desc=None, date=None, category="Brand New Cat", tags=None, type="withdrawal", - dry_run=False, skip_dupes=False) + dry_run=False, skip_dupes=False, source_id=None, dest_id=None) tx.cmd_add(args, ctx) split = client.request.call_args[1]["body"]["transactions"][0] self.assertEqual(split["category_name"], "Brand New Cat") @@ -73,7 +73,7 @@ class TestTxAdd(unittest.TestCase): ctx, client, resolver = make_ctx() resolver.account.side_effect = lambda n: {"id": "1", "type": "asset", "name": n} args = MagicMock(amount="5", source="A", dest="B", desc="x", date="2026-06-01", - category=None, tags=None, type="withdrawal", dry_run=True, skip_dupes=False) + category=None, tags=None, type="withdrawal", dry_run=True, skip_dupes=False, source_id=None, dest_id=None) rc = tx.cmd_add(args, ctx) self.assertEqual(rc, 0) client.request.assert_not_called() # accounts resolved, nothing written @@ -84,7 +84,7 @@ class TestTxAdd(unittest.TestCase): ctx, client, resolver = make_ctx() resolver.account.side_effect = ResolutionError('No account named "B"') args = MagicMock(amount="5", source="A", dest="B", desc=None, date=None, - category=None, tags=None, type="withdrawal", dry_run=True, skip_dupes=False) + category=None, tags=None, type="withdrawal", dry_run=True, skip_dupes=False, source_id=None, dest_id=None) with self.assertRaises(ResolutionError): tx.cmd_add(args, ctx) client.request.assert_not_called() @@ -101,7 +101,7 @@ class TestTxAdd(unittest.TestCase): {"id": "441", "attributes": {}}]} args = MagicMock(amount="9.99", source="A", dest="B", desc="x", date="2026-06-10", category=None, tags=None, - type=None, dry_run=False, skip_dupes=True) + type=None, dry_run=False, skip_dupes=True, source_id=None, dest_id=None) rc = tx.cmd_add(args, ctx) self.assertEqual(rc, 0) # exactly one call, the GET search; no POST @@ -127,7 +127,7 @@ class TestTxAdd(unittest.TestCase): ] args = MagicMock(amount="9.99", source="A", dest="B", desc="x", date="2026-06-10", category=None, tags=None, - type=None, dry_run=False, skip_dupes=True) + type=None, dry_run=False, skip_dupes=True, source_id=None, dest_id=None) rc = tx.cmd_add(args, ctx) self.assertEqual(rc, 0) self.assertEqual(client.request.call_count, 2) @@ -145,7 +145,7 @@ class TestTxAdd(unittest.TestCase): client.request.return_value = {"data": {"id": "1", "attributes": {}}} args = MagicMock(amount="100", source="BBVA", dest="Medio", desc=None, date=None, category=None, tags=None, type=None, - dry_run=False, skip_dupes=False) + dry_run=False, skip_dupes=False, source_id=None, dest_id=None) buf = io.StringIO() with redirect_stderr(buf): tx.cmd_add(args, ctx) @@ -158,7 +158,7 @@ class TestTxAdd(unittest.TestCase): resolver.account.side_effect = lambda n: {"id": "1", "type": "asset", "name": n} args = MagicMock(amount="5", source="A", dest="B", desc=None, date=None, category=None, tags=None, type="transfer", - dry_run=True, skip_dupes=False) + dry_run=True, skip_dupes=False, source_id=None, dest_id=None) buf = io.StringIO() with redirect_stderr(buf): tx.cmd_add(args, ctx) @@ -176,7 +176,7 @@ class TestTxAdd(unittest.TestCase): client.request.return_value = {"data": {"id": "1", "attributes": {}}} args = MagicMock(amount="5", source="Checking", dest="Groceries", desc=None, date=None, category=None, tags=None, type=None, - dry_run=False, skip_dupes=False) + dry_run=False, skip_dupes=False, source_id=None, dest_id=None) buf = io.StringIO() with redirect_stderr(buf): tx.cmd_add(args, ctx) @@ -187,11 +187,80 @@ class TestTxAdd(unittest.TestCase): resolver.account.side_effect = lambda n: {"id": "1", "type": "asset", "name": n} args = MagicMock(amount="5", source="A", dest="B", desc=None, date="2026-06-01", category=None, tags=None, type="withdrawal", - dry_run=True, skip_dupes=True) + dry_run=True, skip_dupes=True, source_id=None, dest_id=None) rc = tx.cmd_add(args, ctx) self.assertEqual(rc, 0) client.request.assert_not_called() # dry-run wins: no search, no write + def test_from_id_resolves_by_id_not_name(self): + # --to-id targets an ambiguous account by numeric id (ISSUES.md #2). + ctx, client, resolver = make_ctx() + resolver.account.side_effect = lambda n: { + "Checking": {"id": "1", "name": "Checking", "type": "asset"}}[n] + resolver.account_by_id.side_effect = lambda i: { + "129": {"id": "129", "name": "Nexi", "type": "revenue"}}[i] + client.request.return_value = {"data": {"id": "1", "attributes": {}}} + args = MagicMock(amount="5", source=None, source_id="129", dest="Checking", + dest_id=None, desc=None, date=None, category=None, + tags=None, type=None, dry_run=False, skip_dupes=False) + rc = tx.cmd_add(args, ctx) + self.assertEqual(rc, 0) + split = client.request.call_args[1]["body"]["transactions"][0] + self.assertEqual(split["source_id"], "129") + self.assertEqual(split["destination_id"], "1") + self.assertEqual(split["type"], "deposit") # revenue -> asset + resolver.account.assert_called_once_with("Checking") + + def test_missing_both_name_and_id_errors(self): + from firefly_cli.errors import FireflyError + ctx, client, resolver = make_ctx() + args = MagicMock(amount="5", source=None, source_id=None, dest="B", + dest_id=None, desc=None, date=None, category=None, + tags=None, type=None, dry_run=False, skip_dupes=False) + with self.assertRaises(FireflyError): + tx.cmd_add(args, ctx) + client.request.assert_not_called() + + def test_both_name_and_id_on_same_side_errors(self): + from firefly_cli.errors import FireflyError + ctx, client, resolver = make_ctx() + args = MagicMock(amount="5", source="A", source_id="7", dest="B", + dest_id=None, desc=None, date=None, category=None, + tags=None, type=None, dry_run=False, skip_dupes=False) + with self.assertRaises(FireflyError): + tx.cmd_add(args, ctx) + client.request.assert_not_called() + + def test_mixed_name_source_id_dest(self): + ctx, client, resolver = make_ctx() + resolver.account.side_effect = lambda n: { + "Checking": {"id": "1", "name": "Checking", "type": "asset"}}[n] + resolver.account_by_id.side_effect = lambda i: { + "52": {"id": "52", "name": "Nexi", "type": "expense"}}[i] + client.request.return_value = {"data": {"id": "1", "attributes": {}}} + args = MagicMock(amount="5", source="Checking", source_id=None, dest=None, + dest_id="52", desc=None, date=None, category=None, + tags=None, type=None, dry_run=False, skip_dupes=False) + rc = tx.cmd_add(args, ctx) + self.assertEqual(rc, 0) + split = client.request.call_args[1]["body"]["transactions"][0] + self.assertEqual(split["source_id"], "1") + self.assertEqual(split["destination_id"], "52") + self.assertEqual(split["type"], "withdrawal") # asset -> expense + + def test_bad_id_surfaces_error(self): + from firefly_cli.errors import FireflyError + ctx, client, resolver = make_ctx() + resolver.account_by_id.side_effect = FireflyError("404 not found") + resolver.account.side_effect = lambda n: {"id": "1", "type": "asset", "name": n} + args = MagicMock(amount="5", source="A", source_id=None, dest=None, + dest_id="9999", desc=None, date=None, category=None, + tags=None, type=None, dry_run=False, skip_dupes=False) + with self.assertRaises(FireflyError): + tx.cmd_add(args, ctx) + client.request.assert_not_called() + + class TestTxEdit(unittest.TestCase): def test_edit_sends_only_provided_fields(self): ctx, client, resolver = make_ctx() diff --git a/tests/unit/test_resolver.py b/tests/unit/test_resolver.py index 2e00e83..82ac30b 100644 --- a/tests/unit/test_resolver.py +++ b/tests/unit/test_resolver.py @@ -41,3 +41,13 @@ class TestResolver(unittest.TestCase): r.account("Cash") self.assertIn("3", str(ctx.exception)) self.assertIn("9", str(ctx.exception)) + + def test_account_by_id_fetches_single(self): + c = MagicMock() + c.request.return_value = {"data": { + "id": "129", "type": "accounts", + "attributes": {"name": "Nexi", "type": "revenue"}}} + r = Resolver(c) + acc = r.account_by_id("129") + self.assertEqual(acc, {"id": "129", "name": "Nexi", "type": "revenue"}) + c.request.assert_called_once_with("GET", "/api/v1/accounts/129") -- cgit v1.2.3