diff --git a/README.md b/README.md index 9b84d3e..087cf53 100644 --- a/README.md +++ b/README.md @@ -33,13 +33,14 @@ from reviewed import through reports, export, and verified backup restoration. ## Import local files -Create the destination accounts, then copy up to 10 bank CSV or Ourocard TXT -files into the ignored `imports/` folder. Open **Imports**, choose **Scan files**, -map each section to an active BRL account, and choose **Prepare imports**. Check -the per-file status and counts, then choose **Import ready files**. Exact -reimports are skipped; files needing attention remain separate and do not stop -ready files. Resolve their listed codes and scan again. No transactions are -written until you import the ready files. +Create the destination accounts, then copy bank CSV or Ourocard TXT files into +the ignored `imports/` folder. Open **Imports**, choose **Scan files**, map each +section to an active BRL account, and choose **Prepare imports**. Check the +per-file status and counts, then choose **Import ready files**. During an import, +files move through `imports/staging/`; after a successful commit they appear in +`imports/processed/`. Exceptions return to incoming `imports/`, where they are +safe to correct and scan again. No transactions are written until you import the +ready files. The [local workflow guide](docs/local-workflows.md#import-local-files) explains the guided path and the advanced `import_review` command for manual review. diff --git a/core/templates/core/imports/index.html b/core/templates/core/imports/index.html index 1e1e087..2bccfb7 100644 --- a/core/templates/core/imports/index.html +++ b/core/templates/core/imports/index.html @@ -2,7 +2,7 @@ {% block title %}Imports ยท Gizmoduck{% endblock %} {% block content %}

Imports

-

Copy up to 10 bank CSV or Ourocard TXT files into the local imports/ folder, then scan and map each section to an active BRL account.

+

Copy bank CSV or Ourocard TXT files into the local imports/ folder, then scan and map each section to an active BRL account.

Create an account before mapping if needed.

{% if error %}{% endif %}
diff --git a/core/views.py b/core/views.py index a68da0c..90acd02 100644 --- a/core/views.py +++ b/core/views.py @@ -48,7 +48,6 @@ DOMAIN_MESSAGES = { "zero_amount": "The amount cannot be zero.", } UNKNOWN_DOMAIN_MESSAGE = "Unable to save this change." -MAX_GUIDED_FILES = 10 @dataclass(frozen=True) @@ -86,18 +85,12 @@ def imports_index(request): action = request.POST.get("action") if action == "scan": discovered = guided_imports.discover_imports() - request.session.pop("guided_imports", None) - if len(discovered) > MAX_GUIDED_FILES: - return render(request, "core/imports/index.html", { - "error": f"SCAN_TOO_LARGE: Keep at most {MAX_GUIDED_FILES} files in imports/ and scan again. No files were imported.", - }) files = [{ "name": row.name, "profile": row.profile, "sections": list(row.sections), "state": "NEEDS_ATTENTION" if row.code else "MAPPING_REQUIRED", "code": row.code, "counts": None, } for row in discovered] scannable = [_import_metadata(row) for row in discovered if not row.code] - _remember_import_summaries(request, [row for row in files if row["code"]]) return _render_imports(request, files=files, scan_token=signing.dumps( scannable, salt="guided-imports-scan", compress=True, ) if scannable else None) @@ -111,14 +104,12 @@ def imports_index(request): if action == "prepare": return _prepare_import_files(request, payload) else: - _commit_import_files(request, payload) + return _commit_import_files(request, payload) else: return render(request, "core/imports/index.html", { "error": "Choose an import action. Scan again before continuing.", }, status=400) - return redirect("imports-index") - - return _render_imports(request, files=request.session.get("guided_imports", [])) + return _render_imports(request, files=[]) def _render_imports(request, *, files, **context): @@ -150,7 +141,7 @@ def _load_import_token(request, *, prepared): keys = {"name", "document_hash", "profile", "sections"} if prepared: keys |= {"batch_id", "revision"} - if not isinstance(payload, list) or not payload or len(payload) > MAX_GUIDED_FILES: + if not isinstance(payload, list) or not payload: raise ValueError for row in payload: if not isinstance(row, dict) or set(row) != keys: @@ -181,25 +172,6 @@ def _import_failure(metadata, code="IMPORT_FAILED"): } -def _combined_import_results(request, results): - # Retain exceptions from other files through each independent action. - names = {row["name"] for row in results} - previous = request.session.get("guided_imports", []) - files = [row for row in previous - if row["name"] not in names and row["state"] == "NEEDS_ATTENTION"] + results - if len(files) > MAX_GUIDED_FILES: - # Another tab may have scanned a different group since this form opened. - files = results - return sorted(files, key=lambda row: row["name"]) - - -def _remember_import_summaries(request, files): - # Do not retain hashes, batch identifiers, tokens, mappings, or source content. - request.session["guided_imports"] = [{ - key: row[key] for key in ("name", "profile", "state", "code", "counts") - } for row in files] - - def _prepare_import_files(request, payload): results, ready = [], [] for index, metadata in enumerate(payload): @@ -221,9 +193,7 @@ def _prepare_import_files(request, payload): results.append(_import_summary(result)) if result.state in {"READY", "ALREADY_IMPORTED"}: ready.append(_import_metadata(result, prepared=True)) - files = _combined_import_results(request, results) - _remember_import_summaries(request, [row for row in files if row["state"] == "NEEDS_ATTENTION"]) - return _render_imports(request, files=files, prepared_token=signing.dumps( + return _render_imports(request, files=results, prepared_token=signing.dumps( ready, salt="guided-imports-prepared", compress=True, ) if ready else None) @@ -233,7 +203,7 @@ def _commit_import_files(request, payload): for metadata in payload: prepared = guided_imports.PreparedImport( **{**metadata, "sections": tuple(metadata["sections"])}, - state="READY", counts={"created": 0, "matched": 0, "skipped": 0}, code=None, + state="READY", counts={"created": 0, "matched": 0, "skipped": 0}, code=None, staged=True, ) try: result = guided_imports.commit_prepared_import(prepared=prepared, actor=LOCAL_ACTOR) @@ -241,7 +211,7 @@ def _commit_import_files(request, payload): results.append(_import_failure(metadata)) continue results.append(_import_summary(result)) - _remember_import_summaries(request, _combined_import_results(request, results)) + return _render_imports(request, files=results) def reconciliation_index(request): diff --git a/docs/local-workflows.md b/docs/local-workflows.md index af03464..176455f 100644 --- a/docs/local-workflows.md +++ b/docs/local-workflows.md @@ -38,7 +38,7 @@ domain workflows described in [the financial domain guide](financial-domain.md). 1. Create the destination accounts through **Accounts**. Bank statements need a bank account; Ourocard sections need credit-card accounts. Only active BRL accounts can be mapped. -2. Copy up to 10 source files into the ignored repository-root `imports/` +2. Copy source files into the ignored repository-root `imports/` incoming directory. Use bank-account CSV or Banco do Brasil/Ourocard TXT. Keep real documents out of `tests/fixtures/`. The web and management services share this local directory through a Compose bind mount. @@ -50,16 +50,14 @@ domain workflows described in [the financial domain guide](financial-domain.md). exact reimport skips. Each file commits independently; one file needing attention does not stop another ready file. The result shows created, matched, and skipped counts without displaying source rows or amounts. -6. Resolve the listed exceptions and scan again. After checking successful - results, move processed documents to another ignored local directory such - as `documents/` before copying the next group into `imports/`. - -More than 10 files returns `SCAN_TOO_LARGE` and leaves the entire scan untouched. -Reduce the group and scan again. Scans and prepared selections expire after -15 minutes; refresh by scanning again if a selection expires or a file changes. -The signed selections stay in the forms, while the browser session retains -only compact safe result summaries. Import actions use CSRF protection and -record the local human actor `HUMAN/local-owner`. +6. Each file moves through `imports/staging/` while its import runs. After a + successful commit it appears in `imports/processed/`. Exceptions return to + incoming `imports/`, where they are safe to correct and scan again. + +Scans and prepared selections expire after 15 minutes; refresh by scanning +again if a selection expires or a file changes. Signed selections stay in the +forms. Import actions use CSRF protection and record the local human actor +`HUMAN/local-owner`. `INVALID_SECTION_MAPPING` requires a current active BRL account for every section. `FILE_CHANGED`, `STALE_REVISION`, or `STALE_PREVIEW` requires a fresh diff --git a/tests/test_guided_import_views.py b/tests/test_guided_import_views.py index 576a271..9a13961 100644 --- a/tests/test_guided_import_views.py +++ b/tests/test_guided_import_views.py @@ -1,8 +1,6 @@ """The local owner imports synthetic files without sending source rows to the UI.""" from pathlib import Path -import random -import string import pytest from django.core import signing @@ -108,7 +106,7 @@ def test_mapping_prepares_then_explicit_commit_creates_with_local_human_actor(cl payload = signing.loads(prepared.context["prepared_token"], salt="guided-imports-prepared") assert set(payload[0]) == {"name", "document_hash", "profile", "sections", "batch_id", "revision"} response = commit(client, prepared) - assert response.redirect_chain == [(reverse("imports-index"), 302)] + assert not response.redirect_chain assert response.context["files"][0]["state"] == "COMMITTED" assert response.context["files"][0]["counts"]["created"] == 2 assert Transaction.objects.count() == SourceRecord.objects.count() == 2 @@ -136,42 +134,10 @@ def test_scan_and_prepared_form_metadata_do_not_depend_on_session_payload(client assert "batch_id" not in str(dict(client.session)) -def test_oversized_scan_has_safe_result_and_leaves_every_file_untouched(client, source, bank): - for index in range(20): - (source / f"synthetic-{index}.csv").write_bytes(DATA) - response = scan(client) - assert "SCAN_TOO_LARGE" in response.content.decode() - assert not response.context.get("scan_token") - assert not ImportBatch.objects.exists() - assert len(list(source.iterdir())) == 21 - if "sessionid" in client.cookies: - assert len(client.cookies["sessionid"].value) < 4096 - - -def test_long_filename_summaries_fit_browser_cookie_and_do_not_accumulate_across_tabs(client, source, bank): - (source / "bank.csv").unlink() - names = random.Random(41) - filenames = ["".join(names.choices(string.ascii_letters + string.digits, k=250)) + ".csv" - for _ in range(10)] - for filename in filenames: - (source / filename).write_bytes(DATA) - scanned = scan(client) - fields = {f"file-{index}-section_0": str(bank.pk) for index in range(10)} - prepared = prepare(client, scanned, bank, **fields) - # A second tab scans other files before the first tab finishes its old form. - for filename in filenames: - (source / filename).unlink() - (source / filename.replace(".csv", ".ofx")).write_text("Synthetic unsupported") - scan(client) - response = commit(client, prepared) - assert len(response.context["files"]) == 10 - assert all(row["code"] == "UNREADABLE_FILE" for row in response.context["files"]) - assert len(client.cookies["sessionid"].OutputString().encode()) < 4096 - assert not Transaction.objects.exists() - def test_exact_reimport_reports_skips_without_new_transactions(client, source, bank): commit(client, prepare(client, scan(client), bank)) + (source / "bank.csv").write_bytes(DATA) prepared = prepare(client, scan(client), bank) assert prepared.context["files"][0]["state"] == "ALREADY_IMPORTED" result = commit(client, prepared) @@ -214,9 +180,9 @@ def test_review_exception_does_not_prevent_another_file_commit(client, source, b (source / "ambiguous.csv").write_bytes(DATA + DATA.splitlines(keepends=True)[1]) prepared = prepare(client, scan(client), bank, **{"file-1-section_0": str(bank.pk)}) assert [row["state"] for row in prepared.context["files"]] == ["NEEDS_ATTENTION", "READY"] + assert prepared.context["files"][0]["code"] == "REVIEW_REQUIRED" result = commit(client, prepared) - assert [row["state"] for row in result.context["files"]] == ["NEEDS_ATTENTION", "COMMITTED"] - assert result.context["files"][0]["code"] == "REVIEW_REQUIRED" + assert [row["state"] for row in result.context["files"]] == ["COMMITTED"] assert Transaction.objects.count() == 2 assert not ImportBatch.objects.filter(state="PREVIEW").exists() @@ -241,13 +207,18 @@ def test_unexpected_per_file_error_never_exposes_source_or_blocks_next_file(clie monkeypatch.setattr(views.guided_imports, service_name, fail_one_file) if stage == "prepare": prepared = prepare(client, scanned, bank, **{"file-1-section_0": str(bank.pk)}) + assert prepared.context["files"][0]["code"] == "IMPORT_FAILED" result = commit(client, prepared) - assert [row["state"] for row in result.context["files"]] == ["NEEDS_ATTENTION", "COMMITTED"] - assert result.context["files"][0]["code"] == "IMPORT_FAILED" - assert "Synthetic private source text" not in result.content.decode() - assert "1234.56" not in result.content.decode() + if stage == "commit": + assert [row["state"] for row in result.context["files"]] == ["NEEDS_ATTENTION", "COMMITTED"] + assert result.context["files"][0]["code"] == "IMPORT_FAILED" + else: + assert [row["state"] for row in result.context["files"]] == ["COMMITTED"] + response = prepared if stage == "prepare" else result + assert "Synthetic private source text" not in response.content.decode() + assert "1234.56" not in response.content.decode() assert Transaction.objects.count() == 2 - assert_private(result, source) + assert_private(response, source) @pytest.mark.parametrize("stage", ["prepare", "commit"]) @@ -281,7 +252,10 @@ def test_signed_workflow_rejects_altered_expired_and_wrong_stage_tokens( def test_changed_source_rejects_previously_signed_metadata(client, source, bank, stage): scanned = scan(client) prepared = prepare(client, scanned, bank) if stage == "commit" else None - (source / "bank.csv").write_bytes(DATA.replace(b"12.34", b"22.34")) + changed = source / "bank.csv" + if stage == "commit": + changed = source / "staging" / "bank.csv" + changed.write_bytes(DATA.replace(b"12.34", b"22.34")) response = commit(client, prepared) if prepared else prepare(client, scanned, bank) assert response.context["files"][0]["code"] == "FILE_CHANGED" assert not Transaction.objects.exists() diff --git a/tests/test_mvp_acceptance.py b/tests/test_mvp_acceptance.py index 428d554..528c500 100644 --- a/tests/test_mvp_acceptance.py +++ b/tests/test_mvp_acceptance.py @@ -37,8 +37,10 @@ def test_owner_guided_import_then_exact_reimport_skip(tmp_path, settings): page = browser.get(reverse("imports-index")) csrf = page.cookies["csrftoken"].value - for expected in ({"created": 2, "matched": 0, "skipped": 0}, - {"created": 0, "matched": 0, "skipped": 2}): + for index, expected in enumerate(({"created": 2, "matched": 0, "skipped": 0}, + {"created": 0, "matched": 0, "skipped": 2})): + if index: + (tmp_path / "owner-bank.csv").write_bytes(FIXTURE.read_bytes()) scanned = browser.post(reverse("imports-index"), { "action": "scan", "csrfmiddlewaretoken": csrf, }, follow=True)