From 38d7357f36e6eeb91216d1c5668fb29406c7e076 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Tue, 30 Jun 2026 17:55:30 +0200 Subject: feat: add tx edit and tx delete (v0.3.0) Implements ISSUES.md #1, the missing other half of an import tool: correcting and removing mis-imported transactions without the web UI. - tx edit : PATCH a single-split journal; only the fields passed are sent (--amount/--date/--desc/--from/--to/--category/--tags/--type). Errors if no field is given. Accounts resolve to ids; category/tags pass raw. - tx delete : requires --yes (no interactive prompt, agent-first); prints {"deleted": ""} on success. Also fix gen_completion.py: running it as a script put scripts/ on sys.path[0] and imported the installed (stale) firefly_cli, so the generated completion drifted (missing account balance/get, tx get/search). Prepend the repo root so the documented regen command uses this tree. The regenerated completion now reflects the full command surface. MINOR bump per the contract-keyed scheme: new commands, no breaking change. Co-Authored-By: Claude Opus 4.8 --- SKILL.md | 15 ++++++++ completions/firefly.bash | 4 +- firefly_cli/__init__.py | 2 +- firefly_cli/commands/transaction.py | 51 +++++++++++++++++++++++++ pyproject.toml | 2 +- scripts/gen_completion.py | 6 +++ tests/unit/test_commands_transaction.py | 67 +++++++++++++++++++++++++++++++++ 7 files changed, 144 insertions(+), 3 deletions(-) diff --git a/SKILL.md b/SKILL.md index 7956bdd..b90ad2e 100644 --- a/SKILL.md +++ b/SKILL.md @@ -57,6 +57,10 @@ firefly account create --type asset|expense|revenue [--opening-balance N] [--currency CODE] firefly tx add --from --to [--desc TEXT] [--date YYYY-MM-DD] [--category NAME] [--tags a,b] [--type T] +firefly tx edit + [--amount N] [--date YYYY-MM-DD] [--desc TEXT] [--from ] [--to ] + [--category NAME] [--tags a,b] [--type T] # only fields passed are changed +firefly tx delete --yes # --yes required, no prompt firefly tx list [--since YYYY-MM-DD] [--until YYYY-MM-DD] [--account NAME] [--limit N] firefly tx get firefly tx search @@ -116,6 +120,14 @@ Returns a transaction group: `{"id", ..., "transactions": [ {split}, ... ]}`. The real fields (type, amount, description, source/destination, tags) are in `transactions[0]` for a single-split transaction. +**Fix a mis-imported transaction.** `tx edit` patches only the fields you pass; +everything else is left untouched. To flip a reversed transfer, swap the ends: +```bash +firefly tx edit 75 --from BBVA --to Mediolanum +firefly tx edit 75 --amount 50.00 --date 2026-06-15 +firefly tx delete 76 --yes # remove a duplicate +``` + ## Gotchas - `tx list` with no transactions in range returns `[]`. Empty is not an error; @@ -126,6 +138,9 @@ The real fields (type, amount, description, source/destination, tags) are in default period, which may hide older transactions. Pass an explicit `--since` to be sure. - `--tags` is a single comma-separated argument: `--tags food,fun`. +- `tx edit` handles single-split journals only. `tx delete` will not run + without `--yes` (there is no interactive prompt); on success it prints + `{"deleted": ""}`. ## Extending diff --git a/completions/firefly.bash b/completions/firefly.bash index c118f52..b66b400 100644 --- a/completions/firefly.bash +++ b/completions/firefly.bash @@ -40,6 +40,8 @@ _firefly() { "account create") leaf_opts="--currency --opening-balance --type";; "account list") leaf_opts="--type";; "tx add") leaf_opts="--category --date --desc --from --tags --to --type";; + "tx delete") leaf_opts="--yes";; + "tx edit") leaf_opts="--amount --category --date --desc --from --tags --to --type";; "tx list") leaf_opts="--account --limit --since --until";; esac @@ -50,7 +52,7 @@ _firefly() { account) leaves="balance create get list";; category) leaves="list";; tag) leaves="list";; - tx) leaves="add get list search";; + tx) leaves="add delete edit get list search";; esac if [[ -z $group ]]; then diff --git a/firefly_cli/__init__.py b/firefly_cli/__init__.py index 6173b2b..f41267d 100644 --- a/firefly_cli/__init__.py +++ b/firefly_cli/__init__.py @@ -2,4 +2,4 @@ # Copyright (C) 2026 Danilo M. # Licensed under the GNU General Public License v2.0 only. -__version__ = "0.2.2" +__version__ = "0.3.0" diff --git a/firefly_cli/commands/transaction.py b/firefly_cli/commands/transaction.py index 67d8b71..af8a4fb 100644 --- a/firefly_cli/commands/transaction.py +++ b/firefly_cli/commands/transaction.py @@ -1,5 +1,6 @@ # Copyright (C) 2026 Danilo M. GPL-2.0-only from firefly_cli import registry, output +from firefly_cli.errors import FireflyError # Inference table keyed by (source_type, destination_type) -> firefly tx type. def _infer_type(src_type, dst_type): @@ -54,6 +55,56 @@ def cmd_add(args, ctx): output.emit(output.unwrap(resp), human=ctx.human) return 0 +def _edit_args(p): + p.add_argument("id") + p.add_argument("--amount", default=None) + p.add_argument("--date", default=None, help="YYYY-MM-DD") + p.add_argument("--desc", default=None) + p.add_argument("--from", dest="source", default=None, help="source account") + p.add_argument("--to", dest="dest", default=None, help="destination account") + p.add_argument("--category", default=None) + p.add_argument("--tags", default=None, help="comma-separated") + p.add_argument("--type", default=None, help="withdrawal|deposit|transfer") + +# ponytail: single-split journals only; multi-split edits need transaction_journal_id per row. +@registry.command("tx edit", help="modify one transaction by id; only the fields you pass change", args=_edit_args) +def cmd_edit(args, ctx): + split = {} + if args.amount is not None: + split["amount"] = str(args.amount) + if args.date is not None: + split["date"] = args.date + if args.desc is not None: + split["description"] = args.desc + if args.source is not None: + split["source_id"] = ctx.resolver.account(args.source)["id"] + if args.dest is not None: + split["destination_id"] = ctx.resolver.account(args.dest)["id"] + if args.category is not None: + split["category_name"] = args.category + if args.tags is not None: + split["tags"] = [t.strip() for t in args.tags.split(",") if t.strip()] + if args.type is not None: + split["type"] = args.type + if not split: + raise FireflyError("tx edit: nothing to change; pass at least one field") + resp = ctx.client.request("PUT", f"/api/v1/transactions/{args.id}", + body={"transactions": [split]}) + output.emit(output.unwrap(resp), human=ctx.human) + return 0 + +def _delete_args(p): + p.add_argument("id") + p.add_argument("--yes", action="store_true", help="confirm deletion (required)") + +@registry.command("tx delete", help="delete one transaction by id (requires --yes)", args=_delete_args) +def cmd_delete(args, ctx): + if not args.yes: + raise FireflyError(f"tx delete {args.id}: refusing without --yes") + ctx.client.request("DELETE", f"/api/v1/transactions/{args.id}") + output.emit({"deleted": args.id}, human=ctx.human) + return 0 + def _list_args(p): p.add_argument("--since", default=None, help="start date YYYY-MM-DD") p.add_argument("--until", default=None, help="end date YYYY-MM-DD") diff --git a/pyproject.toml b/pyproject.toml index 7812540..4637c1e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "firefly-iii-agent" -version = "0.2.2" +version = "0.3.0" description = "CLI tool for agent interaction with Firefly III" readme = "README.md" requires-python = ">=3.11" diff --git a/scripts/gen_completion.py b/scripts/gen_completion.py index a97f26b..4714bf1 100644 --- a/scripts/gen_completion.py +++ b/scripts/gen_completion.py @@ -8,8 +8,14 @@ No drift: groups, leaves, and per-leaf flags are read straight off the argparse subparsers the registry builds. """ import argparse +import os +import sys from collections import defaultdict +# Run from the repo, not any installed copy: prepend the repo root so +# `python scripts/gen_completion.py` imports this tree, not a stale install. +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + from firefly_cli import registry import firefly_cli.commands # noqa: F401 triggers registration diff --git a/tests/unit/test_commands_transaction.py b/tests/unit/test_commands_transaction.py index 7301002..325dcb7 100644 --- a/tests/unit/test_commands_transaction.py +++ b/tests/unit/test_commands_transaction.py @@ -67,6 +67,73 @@ class TestTxAdd(unittest.TestCase): self.assertEqual(split["category_name"], "Brand New Cat") resolver.category.assert_not_called() +class TestTxEdit(unittest.TestCase): + def test_edit_sends_only_provided_fields(self): + ctx, client, resolver = make_ctx() + client.request.return_value = {"data": {"id": "9", "attributes": {}}} + args = MagicMock(id="9", amount="12.00", date=None, desc="fixed", + source=None, dest=None, category=None, tags=None, type=None) + rc = tx.cmd_edit(args, ctx) + self.assertEqual(rc, 0) + method, path = client.request.call_args[0][:2] + split = client.request.call_args[1]["body"]["transactions"][0] + self.assertEqual((method, path), ("PUT", "/api/v1/transactions/9")) + self.assertEqual(split, {"amount": "12.00", "description": "fixed"}) + resolver.account.assert_not_called() + + def test_edit_resolves_accounts_when_given(self): + ctx, client, resolver = make_ctx() + resolver.account.side_effect = lambda n: { + "BBVA": {"id": "3", "name": "BBVA", "type": "asset"}, + "Medio": {"id": "4", "name": "Medio", "type": "asset"}, + }[n] + client.request.return_value = {"data": {"id": "9", "attributes": {}}} + args = MagicMock(id="9", amount=None, date=None, desc=None, + source="BBVA", dest="Medio", category=None, tags=None, type=None) + tx.cmd_edit(args, ctx) + split = client.request.call_args[1]["body"]["transactions"][0] + self.assertEqual(split, {"source_id": "3", "destination_id": "4"}) + + def test_edit_category_raw_and_tags_split(self): + ctx, client, resolver = make_ctx() + client.request.return_value = {"data": {"id": "9", "attributes": {}}} + args = MagicMock(id="9", amount=None, date=None, desc=None, source=None, + dest=None, category="Cat", tags="a, b", type="transfer") + tx.cmd_edit(args, ctx) + split = client.request.call_args[1]["body"]["transactions"][0] + self.assertEqual(split, + {"category_name": "Cat", "tags": ["a", "b"], "type": "transfer"}) + resolver.category.assert_not_called() + + def test_edit_with_no_fields_errors(self): + from firefly_cli.errors import FireflyError + ctx, client, _ = make_ctx() + args = MagicMock(id="9", amount=None, date=None, desc=None, source=None, + dest=None, category=None, tags=None, type=None) + with self.assertRaises(FireflyError): + tx.cmd_edit(args, ctx) + client.request.assert_not_called() + + +class TestTxDelete(unittest.TestCase): + def test_delete_requires_yes(self): + from firefly_cli.errors import FireflyError + ctx, client, _ = make_ctx() + args = MagicMock(id="9", yes=False) + with self.assertRaises(FireflyError): + tx.cmd_delete(args, ctx) + client.request.assert_not_called() + + def test_delete_with_yes(self): + ctx, client, _ = make_ctx() + client.request.return_value = {} + args = MagicMock(id="9", yes=True) + rc = tx.cmd_delete(args, ctx) + self.assertEqual(rc, 0) + method, path = client.request.call_args[0][:2] + self.assertEqual((method, path), ("DELETE", "/api/v1/transactions/9")) + + class TestTxList(unittest.TestCase): def test_list_passes_date_params(self): ctx, client, _ = make_ctx() -- cgit v1.2.3