From a81443ff677e0886ecd1371d5658e1948fd0dcd6 Mon Sep 17 00:00:00 2001 From: bitcoinafterlife Date: Wed, 7 Oct 2026 15:03:25 +0200 Subject: [PATCH] i18n review fixes (#63) Truman's i18n review: release gate runs the i18n, CLI and import tests and checks Babel first; ruff known-first-party = bal; no translated text stored in the wallet (executor push records are copies, status history always saved as 'New', old 'Nuovo'/'Nuevo' mapped back on load). Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01HxDDvLym1NEbFNYMc1rr7S --- AGENTS.md | 5 +- CHANGELOG.md | 63 ++++++++++ bal/cli/controller.py | 4 +- bal/core/will.py | 37 +++++- bal/core/willexecutors.py | 23 +++- bal/gui/qt/window.py | 4 +- make-release.sh | 10 +- pyproject.toml | 6 + tests/test_core_status_language_neutral.py | 136 +++++++++++++++++++++ 9 files changed, 278 insertions(+), 10 deletions(-) create mode 100644 tests/test_core_status_language_neutral.py diff --git a/AGENTS.md b/AGENTS.md index f4b0a93..a05d3aa 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -117,7 +117,10 @@ python3 build_zip.py # -> bal-electrum-plugin.zip (deterministic, prints sha25 ``` - `build_zip.py` needs **Babel** (`pip install babel`): it compiles the - translation catalogs into the zip and stops if Babel is missing. + translation catalogs into the zip and stops if Babel is missing. The i18n + tests need it too. Install it in the **runtime** env used for releases + (`source "$BAL_HOME/electrum/env/bin/activate" && pip install babel`); + `make-release.sh` checks it before running anything. - `make-release.sh` requires gpg and Gitea credentials (`~/.git-credentials` or `GITEA_USER`/`GITEA_TOKEN`). It bumps `bal/manifest.json` — bump the version there, never invent a new source of truth. diff --git a/CHANGELOG.md b/CHANGELOG.md index 6215885..0c097f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3549,3 +3549,66 @@ delivery; when it is not, the plugin raises an explicit error (see below). catalogs refreshed (removed format strings marked obsolete). **Outcome:** DONE. + +--- + +## 63. i18n review fixes (Truman's review of the translation PR) + +**Date:** 2026-10-07 + +**What changed:** + +- `make-release.sh` + - Step 3 now also runs `tests/test_i18n.py`, `tests/test_translations.py`, + `tests/test_cli_*.py` and `tests/test_import_will_details.py` (the i18n + tests were never run by the release gate; the CLI and will-import tests + were not either). + - Checks up front that Babel is importable by `python3` and stops with + "pip install babel" otherwise: the i18n tests (step 3) and `build_zip.py` + (step 5) both need it, so a release no longer stops half-way. +- `AGENTS.md`: Babel must be installed in the runtime env used for releases. +- `pyproject.toml`: `[tool.ruff.lint.isort] known-first-party = ["bal"]`. + WHY: the reported I001 in `tests/test_translations.py` is not reproducible + from the repository root (ruff 0.15.11 and 0.6.9: clean), but it appears as + soon as ruff classifies `bal` as third-party (then 20 files report I001). + Removing the blank line would break the check where `bal` is first-party; + declaring it makes the result the same on every machine. +- `bal/core/willexecutors.py` - `get_willexecutor_transactions` returns COPIES + of the executor records. It used to write `txs`, `txsids` and the + translated `broadcast_status` into the will item's own `we` dict, which is + saved in the wallet (stored data depended on the UI language). +- `bal/core/will.py` - `WillItem` drops those transient fields when loading a + will saved by an older version, and maps a translated leading "New" + ("Nuovo", "Nuevo" - saved by i18n test builds) back to "New". +- `bal/gui/qt/window.py`, `bal/cli/controller.py`: the status history of a new + will item is saved as the literal "New" (was `_("New")`). +- `tests/test_core_status_language_neutral.py` (new, 7 tests; 5 fail on + `main`): English "New" stored, translated "New" mapped back, the mapping + covers every catalog's translation of "New", the push helper does not + modify the will item, old transient fields dropped on load. + +**Scope:** this entry contains ONLY the i18n review fixes. The other fixes +prepared in the same session (invalidation fee, Check `KeyError`, cascade, +deselected executor) are not part of it and will be proposed separately. + +**Verification (on `main` 827a1d1):** + +- New test file `tests/test_core_status_language_neutral.py`: 5 of 7 fail on + `main`, 7 of 7 pass with this change. +- Extended release test list (as above) plus the new test file: 446 passed, + 1 failed (`main`: 439 passed, same failure). The failure is pre-existing: + `test_group_e_mock_karen7::test_e5` needs `tests/karen7`, a wallet file that + is not in the repository. +- Whole `tests/` directory: 609 passed, 26 failed, 3 collection errors + (`main`: 602 passed, and the SAME 26 failures and 3 errors, test by test). + All pre-existing and unrelated: 9 need `tests/karen7`; 21 in + `test_auto_rebuild_on_new_tx.py`, `test_cli_autorebuild.py` and + `test_cli_controller_offline.py` fail only when the whole directory runs + together (they pass when each file runs alone); 3 audio tests need an audio + device. `test_gui_export_dialogs.py::test_audio_import_receive_wiring` hangs + forever, also on `main`, so it was deselected. +- `ruff check bal/ tests/`: the same 3 pre-existing I001 as on `main` + (`dialogs.py`, `test_core_animated_qr.py`, `test_core_qr_transfer.py`), + none new. + +**Outcome:** DONE (pending Truman's review). diff --git a/bal/cli/controller.py b/bal/cli/controller.py index 5157ee8..3722668 100644 --- a/bal/cli/controller.py +++ b/bal/cli/controller.py @@ -385,7 +385,9 @@ class BalController: tx["heirsvalue"] = txs[txid].heirsvalue tx["description"] = txs[txid].description tx["willexecutor"] = copy_structure(txs[txid].willexecutor) - tx["status"] = _("New") + # Stored history: always English (translated only for display by + # format_status_history); never _() here. + tx["status"] = "New" tx["baltx_fees"] = txs[txid].tx_fees tx["time"] = creation_time tx["heirs"] = copy_structure(txs[txid].heirs) diff --git a/bal/core/will.py b/bal/core/will.py index 46a00d8..1c205c0 100644 --- a/bal/core/will.py +++ b/bal/core/will.py @@ -1364,6 +1364,37 @@ class WillItem(Logger): # format_status_history() still translates them. LEGACY_STATUS_LABELS = (N_("New"),) + # Translations of "New" that test builds of the i18n work saved INTO the + # status history (window.py / the CLI used ``_("New")`` when building). + # They are mapped back to "New" on load so the history is language + # neutral again. Keep in sync with the "New" msgstr of every catalog in + # bal/locale (tests/test_core_status_language_neutral.py checks it). + TRANSLATED_NEW_LABELS = ("Nuovo", "Nuevo") + + # Fields that Willexecutors.get_willexecutor_transactions / + # push_transactions_to_willexecutor used to write into the item's own + # executor record (older versions); they are transient push data. + TRANSIENT_PUSH_FIELDS = ("txs", "txsids", "broadcast_status") + + @staticmethod + def _untranslate_new_status(status): + """Map a translated leading "New" of a saved history back to "New".""" + head, sep, rest = status.partition(".") + if head in WillItem.TRANSLATED_NEW_LABELS: + return "New" + sep + rest + return status + + @staticmethod + def _without_transient_push_fields(we): + """Return ``we`` without the transient push fields (a copy if needed).""" + if not isinstance(we, dict) or not any( + k in we for k in WillItem.TRANSIENT_PUSH_FIELDS + ): + return we + return { + k: v for k, v in we.items() if k not in WillItem.TRANSIENT_PUSH_FIELDS + } + def set_status(self, status, value=True): """Set a status flag and apply the related side effects. @@ -1445,8 +1476,10 @@ class WillItem(Logger): w = data self.tx = Will.get_tx_from_any(w["tx"]) self.heirs = w.get("heirs", None) - self.we = w.get("willexecutor", None) - self.status = w.get("status") or "" + self.we = WillItem._without_transient_push_fields( + w.get("willexecutor", None) + ) + self.status = WillItem._untranslate_new_status(w.get("status") or "") self.description = w.get("description", None) self.time = w.get("time", None) self.change = w.get("change", None) diff --git a/bal/core/willexecutors.py b/bal/core/willexecutors.py index 54bbec0..095a262 100644 --- a/bal/core/willexecutors.py +++ b/bal/core/willexecutors.py @@ -231,6 +231,13 @@ class Willexecutors: @staticmethod def get_willexecutor_transactions(will, force=False): + """Group the valid, signed, not-yet-pushed will txs by will-executor. + + Returns ``{url: executor_record}`` where each record gets ``txs`` (the + raw txs, one per line) and ``txsids``. + + The returned records are COPIES: the will items are never modified. + """ willexecutors = {} for wid, willitem in will.items(): if willitem.get_status("VALID"): @@ -240,10 +247,18 @@ class Willexecutors: url = willexecutor["url"] if willexecutor and Willexecutors.is_selected(willexecutor): if url not in willexecutors: - willexecutor["txs"] = "" - willexecutor["txsids"] = [] - willexecutor["broadcast_status"] = _("Waiting...") - willexecutors[url] = willexecutor + # Work on a COPY of the record: the item's + # own ``we`` dict is saved in the wallet, so + # the transient push fields below (raw txs, + # txids and the - translated - broadcast + # status shown in the push dialog) must + # never end up in it (stored data must not + # depend on the UI language, see bal/i18n.py). + record = dict(willexecutor) + record["txs"] = "" + record["txsids"] = [] + record["broadcast_status"] = _("Waiting...") + willexecutors[url] = record willexecutors[url]["txs"] += str(willitem.tx) + "\n" willexecutors[url]["txsids"].append(wid) diff --git a/bal/gui/qt/window.py b/bal/gui/qt/window.py index be021e9..57d1b38 100644 --- a/bal/gui/qt/window.py +++ b/bal/gui/qt/window.py @@ -551,7 +551,9 @@ class BalWindow: tx["heirsvalue"] = txs[txid].heirsvalue tx["description"] = txs[txid].description tx["willexecutor"] = copy_structure(txs[txid].willexecutor) - tx["status"] = _("New") + # Stored history: always English (translated only for display by + # format_status_history); never _() here. + tx["status"] = "New" tx["baltx_fees"] = txs[txid].tx_fees tx["time"] = creation_time tx["heirs"] = copy_structure(txs[txid].heirs) diff --git a/make-release.sh b/make-release.sh index a2d7cbf..b713e29 100755 --- a/make-release.sh +++ b/make-release.sh @@ -5,7 +5,7 @@ # ./make-release.sh # read version from bal/manifest.json # ./make-release.sh v0.6.2 # bump manifest to 0.6.2, then release # -# Requires: git, gpg, curl, python3, sha256sum +# Requires: git, gpg, curl, python3, sha256sum, Babel for python3 (pip install babel) # Optional: ruff (lint skipped if not installed) # Credentials: ~/.git-credentials or GITEA_USER / GITEA_TOKEN env vars @@ -33,6 +33,12 @@ else NEW_VER="" fi +# Babel is needed by the i18n tests (step 3) and by build_zip.py, which +# compiles the translation catalogs into the zip (step 5). Check it up front +# so a release never stops half-way. +python3 -c "import babel" 2>/dev/null \ + || die "Babel is missing for $(command -v python3): pip install babel" + echo "=== Release ${TAG} ===" echo "Current manifest version: ${CURRENT_VER}" [ -n "$NEW_VER" ] && echo "New version (will bump): ${NEW_VER}" @@ -68,6 +74,8 @@ if QT_QPA_PLATFORM=offscreen PYTHONPATH=electrum-src python3 -m pytest \ tests/test_group_d_alarms.py tests/test_group_e_mock_karen7.py \ tests/test_group_f_heir_change_rebuild.py tests/test_group_g_basic_calendar.py \ tests/test_group_h_v048.py \ + tests/test_i18n.py tests/test_translations.py \ + tests/test_cli_*.py tests/test_import_will_details.py \ -q 2>&1; then echo "All tests passed." else diff --git a/pyproject.toml b/pyproject.toml index 879f935..18aca70 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,6 +6,12 @@ target-version = "py312" select =["E", "W", "F", "I", "N", "B", "INT"] # INT: gettext rules (see bal/i18n.py) ignore = ["E501"] +[tool.ruff.lint.isort] +# Classify the plugin package explicitly: otherwise ruff decides from where it +# is run / how the project root is detected, and the same import block is +# "sorted" on one machine and I001 on another. +known-first-party = ["bal"] + [tool.ruff.lint.pep8-naming] classmethod-decorators = ["classmethod", "classproperty"] # electrum.util.classproperty uses cls diff --git a/tests/test_core_status_language_neutral.py b/tests/test_core_status_language_neutral.py new file mode 100644 index 0000000..856c05e --- /dev/null +++ b/tests/test_core_status_language_neutral.py @@ -0,0 +1,136 @@ +""" +Stored will data must not depend on the GUI language (rule in bal/i18n.py). + +Covers the two findings of the i18n review: + +* the status history of a new will item is saved as English "New" (not + ``_("New")``), and histories saved by test builds as "Nuovo"/"Nuevo" are + mapped back to "New" on load; +* the push helper no longer writes transient fields (``txs``, ``txsids`` and + the translated ``broadcast_status``) into the will item's executor record, + and such fields saved by older versions are dropped on load. + +Run (standalone or with pytest): + QT_QPA_PLATFORM=offscreen PYTHONPATH=electrum-src \ + python3 tests/test_core_status_language_neutral.py +""" + +import os +import re +import sys +from pathlib import Path +from unittest.mock import patch + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), os.pardir)) + +from electrum.transaction import Transaction + +from bal.core.will import WillItem +from bal.core.willexecutors import Willexecutors + +# WillItem parses a raw tx and calls add_info_from_wallet; no live wallet here. +_patcher = patch.object(Transaction, "add_info_from_wallet") +_patcher.start() + +ROOT = Path(__file__).resolve().parent.parent + +_VALID_TX_HEX = ( + "01000000012a5c9a94fcde98f5581cd00162c60a13936ceb75389ea65b" + "f38633b424eb4031000000006c493046022100a82bbc57a0136751e543" + "3f41cf000b3f1a99c6744775e76ec764fb78c54ee100022100f9e80b7d" + "e89de861dc6fb0c1429d5da72c2b6b2ee2406bc9bfb1beedd729d98501" + "2102e61d176da16edd1d258a200ad9759ef63adf8e14cd97f53227bae3" + "5cdb84d2f6ffffffff0140420f00000000001976a914230ac37834073a" + "42146f11ef8414ae929feaafc388ac00000000" +) +URL = "https://we.example" + + +def _item(status="", we=None): + return WillItem( + {"tx": _VALID_TX_HEX, "heirs": {}, "willexecutor": we, "status": status} + ) + + +# ------------------------------------------------------------------ # +# Status history "New" +# ------------------------------------------------------------------ # + +def test_builders_store_english_new(): + """window.py and the CLI controller must not translate the stored status.""" + for rel in ("bal/gui/qt/window.py", "bal/cli/controller.py"): + src = (ROOT / rel).read_text(encoding="utf-8") + assert re.search(r'tx\["status"\]\s*=\s*"New"', src), rel + assert not re.search(r'tx\["status"\]\s*=\s*_\(', src), rel + + +def test_translated_new_is_mapped_back_on_load(): + assert _item("Nuovo.Signed.Pushed").status == "New.Signed.Pushed" + assert _item("Nuevo").status == "New" + assert _item("New.Signed").status == "New.Signed" + + +def test_only_the_leading_new_is_touched(): + assert _item(".Nuovo").status == ".Nuovo" + assert _item("Signed.Nuovo").status == "Signed.Nuovo" + + +def test_translated_new_labels_match_the_catalogs(): + """Every catalog's translation of "New" must be recognised on load.""" + found = set() + for po in (ROOT / "bal" / "locale").glob("*/LC_MESSAGES/bal.po"): + text = po.read_text(encoding="utf-8") + m = re.search(r'^msgid "New"\nmsgstr "(.*)"$', text, re.MULTILINE) + if m and m.group(1): + found.add(m.group(1)) + assert found, "no catalog translates 'New'" + assert found <= set(WillItem.TRANSLATED_NEW_LABELS), found + + +# ------------------------------------------------------------------ # +# Transient push fields +# ------------------------------------------------------------------ # + +class FakeItem: + def __init__(self): + self.we = {"url": URL, "selected": True, "address": "a", "base_fee": 1} + self.tx = "rawtx" + + def get_status(self, name): + return name in ("VALID", "COMPLETE") + + +def test_push_helper_does_not_modify_the_will_item(): + item = FakeItem() + before = dict(item.we) + out = Willexecutors.get_willexecutor_transactions({"t1": item}) + assert out[URL]["txsids"] == ["t1"] + assert "broadcast_status" in out[URL] + assert item.we == before + assert out[URL] is not item.we + + +def test_transient_fields_saved_by_older_versions_are_dropped_on_load(): + we = { + "url": URL, + "selected": True, + "txs": "raw\n", + "txsids": ["t1"], + "broadcast_status": "Riuscito", + } + item = _item("New", we) + assert item.we == {"url": URL, "selected": True} + assert "broadcast_status" not in item.to_dict()["willexecutor"] + + +def test_clean_executor_record_is_kept_as_is(): + we = {"url": URL, "selected": True} + assert _item("New", we).we is we + + +if __name__ == "__main__": + for name in sorted(dir()): + if name.startswith("test_"): + globals()[name]() + print(f" [OK] {name}") + print("[OK] All language-neutral status tests passed")