Browse Source

feat: stage guided import files

master
Codex 2 months ago
parent
commit
abdb88d3f1
  1. 15
      README.md
  2. 2
      core/templates/core/imports/index.html
  3. 42
      core/views.py
  4. 20
      docs/local-workflows.md
  5. 58
      tests/test_guided_import_views.py
  6. 6
      tests/test_mvp_acceptance.py

15
README.md

@ -33,13 +33,14 @@ from reviewed import through reports, export, and verified backup restoration.
## Import local files ## Import local files
Create the destination accounts, then copy up to 10 bank CSV or Ourocard TXT Create the destination accounts, then copy bank CSV or Ourocard TXT files into
files into the ignored `imports/` folder. Open **Imports**, choose **Scan files**, the ignored `imports/` folder. Open **Imports**, choose **Scan files**, map each
map each section to an active BRL account, and choose **Prepare imports**. Check section to an active BRL account, and choose **Prepare imports**. Check the
the per-file status and counts, then choose **Import ready files**. Exact per-file status and counts, then choose **Import ready files**. During an import,
reimports are skipped; files needing attention remain separate and do not stop files move through `imports/staging/`; after a successful commit they appear in
ready files. Resolve their listed codes and scan again. No transactions are `imports/processed/`. Exceptions return to incoming `imports/`, where they are
written until you import the ready files. 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 [local workflow guide](docs/local-workflows.md#import-local-files) explains
the guided path and the advanced `import_review` command for manual review. the guided path and the advanced `import_review` command for manual review.

2
core/templates/core/imports/index.html

@ -2,7 +2,7 @@
{% block title %}Imports · Gizmoduck{% endblock %} {% block title %}Imports · Gizmoduck{% endblock %}
{% block content %} {% block content %}
<h1>Imports</h1> <h1>Imports</h1>
<p>Copy up to 10 bank CSV or Ourocard TXT files into the local <code>imports/</code> folder, then scan and map each section to an active BRL account.</p> <p>Copy bank CSV or Ourocard TXT files into the local <code>imports/</code> folder, then scan and map each section to an active BRL account.</p>
<p><a href="{% url 'account-create' %}">Create an account</a> before mapping if needed.</p> <p><a href="{% url 'account-create' %}">Create an account</a> before mapping if needed.</p>
{% if error %}<p class="error-summary" role="alert">{{ error }}</p>{% endif %} {% if error %}<p class="error-summary" role="alert">{{ error }}</p>{% endif %}
<form method="post"> <form method="post">

42
core/views.py

@ -48,7 +48,6 @@ DOMAIN_MESSAGES = {
"zero_amount": "The amount cannot be zero.", "zero_amount": "The amount cannot be zero.",
} }
UNKNOWN_DOMAIN_MESSAGE = "Unable to save this change." UNKNOWN_DOMAIN_MESSAGE = "Unable to save this change."
MAX_GUIDED_FILES = 10
@dataclass(frozen=True) @dataclass(frozen=True)
@ -86,18 +85,12 @@ def imports_index(request):
action = request.POST.get("action") action = request.POST.get("action")
if action == "scan": if action == "scan":
discovered = guided_imports.discover_imports() 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 = [{ files = [{
"name": row.name, "profile": row.profile, "sections": list(row.sections), "name": row.name, "profile": row.profile, "sections": list(row.sections),
"state": "NEEDS_ATTENTION" if row.code else "MAPPING_REQUIRED", "state": "NEEDS_ATTENTION" if row.code else "MAPPING_REQUIRED",
"code": row.code, "counts": None, "code": row.code, "counts": None,
} for row in discovered] } for row in discovered]
scannable = [_import_metadata(row) for row in discovered if not row.code] 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( return _render_imports(request, files=files, scan_token=signing.dumps(
scannable, salt="guided-imports-scan", compress=True, scannable, salt="guided-imports-scan", compress=True,
) if scannable else None) ) if scannable else None)
@ -111,14 +104,12 @@ def imports_index(request):
if action == "prepare": if action == "prepare":
return _prepare_import_files(request, payload) return _prepare_import_files(request, payload)
else: else:
_commit_import_files(request, payload) return _commit_import_files(request, payload)
else: else:
return render(request, "core/imports/index.html", { return render(request, "core/imports/index.html", {
"error": "Choose an import action. Scan again before continuing.", "error": "Choose an import action. Scan again before continuing.",
}, status=400) }, status=400)
return redirect("imports-index") return _render_imports(request, files=[])
return _render_imports(request, files=request.session.get("guided_imports", []))
def _render_imports(request, *, files, **context): def _render_imports(request, *, files, **context):
@ -150,7 +141,7 @@ def _load_import_token(request, *, prepared):
keys = {"name", "document_hash", "profile", "sections"} keys = {"name", "document_hash", "profile", "sections"}
if prepared: if prepared:
keys |= {"batch_id", "revision"} 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 raise ValueError
for row in payload: for row in payload:
if not isinstance(row, dict) or set(row) != keys: 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): def _prepare_import_files(request, payload):
results, ready = [], [] results, ready = [], []
for index, metadata in enumerate(payload): for index, metadata in enumerate(payload):
@ -221,9 +193,7 @@ def _prepare_import_files(request, payload):
results.append(_import_summary(result)) results.append(_import_summary(result))
if result.state in {"READY", "ALREADY_IMPORTED"}: if result.state in {"READY", "ALREADY_IMPORTED"}:
ready.append(_import_metadata(result, prepared=True)) ready.append(_import_metadata(result, prepared=True))
files = _combined_import_results(request, results) return _render_imports(request, files=results, prepared_token=signing.dumps(
_remember_import_summaries(request, [row for row in files if row["state"] == "NEEDS_ATTENTION"])
return _render_imports(request, files=files, prepared_token=signing.dumps(
ready, salt="guided-imports-prepared", compress=True, ready, salt="guided-imports-prepared", compress=True,
) if ready else None) ) if ready else None)
@ -233,7 +203,7 @@ def _commit_import_files(request, payload):
for metadata in payload: for metadata in payload:
prepared = guided_imports.PreparedImport( prepared = guided_imports.PreparedImport(
**{**metadata, "sections": tuple(metadata["sections"])}, **{**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: try:
result = guided_imports.commit_prepared_import(prepared=prepared, actor=LOCAL_ACTOR) 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)) results.append(_import_failure(metadata))
continue continue
results.append(_import_summary(result)) results.append(_import_summary(result))
_remember_import_summaries(request, _combined_import_results(request, results)) return _render_imports(request, files=results)
def reconciliation_index(request): def reconciliation_index(request):

20
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 1. Create the destination accounts through **Accounts**. Bank statements need a
bank account; Ourocard sections need credit-card accounts. Only active BRL bank account; Ourocard sections need credit-card accounts. Only active BRL
accounts can be mapped. 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 directory. Use bank-account CSV or Banco do Brasil/Ourocard TXT. Keep real
documents out of `tests/fixtures/`. The web and management services share documents out of `tests/fixtures/`. The web and management services share
this local directory through a Compose bind mount. 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 exact reimport skips. Each file commits independently; one file needing
attention does not stop another ready file. The result shows created, attention does not stop another ready file. The result shows created,
matched, and skipped counts without displaying source rows or amounts. matched, and skipped counts without displaying source rows or amounts.
6. Resolve the listed exceptions and scan again. After checking successful 6. Each file moves through `imports/staging/` while its import runs. After a
results, move processed documents to another ignored local directory such successful commit it appears in `imports/processed/`. Exceptions return to
as `documents/` before copying the next group into `imports/`. incoming `imports/`, where they are safe to correct and scan again.
More than 10 files returns `SCAN_TOO_LARGE` and leaves the entire scan untouched. Scans and prepared selections expire after 15 minutes; refresh by scanning
Reduce the group and scan again. Scans and prepared selections expire after again if a selection expires or a file changes. Signed selections stay in the
15 minutes; refresh by scanning again if a selection expires or a file changes. forms. Import actions use CSRF protection and record the local human actor
The signed selections stay in the forms, while the browser session retains `HUMAN/local-owner`.
only compact safe result summaries. 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 `INVALID_SECTION_MAPPING` requires a current active BRL account for every
section. `FILE_CHANGED`, `STALE_REVISION`, or `STALE_PREVIEW` requires a fresh section. `FILE_CHANGED`, `STALE_REVISION`, or `STALE_PREVIEW` requires a fresh

58
tests/test_guided_import_views.py

@ -1,8 +1,6 @@
"""The local owner imports synthetic files without sending source rows to the UI.""" """The local owner imports synthetic files without sending source rows to the UI."""
from pathlib import Path from pathlib import Path
import random
import string
import pytest import pytest
from django.core import signing 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") payload = signing.loads(prepared.context["prepared_token"], salt="guided-imports-prepared")
assert set(payload[0]) == {"name", "document_hash", "profile", "sections", "batch_id", "revision"} assert set(payload[0]) == {"name", "document_hash", "profile", "sections", "batch_id", "revision"}
response = commit(client, prepared) 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]["state"] == "COMMITTED"
assert response.context["files"][0]["counts"]["created"] == 2 assert response.context["files"][0]["counts"]["created"] == 2
assert Transaction.objects.count() == SourceRecord.objects.count() == 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)) 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): def test_exact_reimport_reports_skips_without_new_transactions(client, source, bank):
commit(client, prepare(client, scan(client), bank)) commit(client, prepare(client, scan(client), bank))
(source / "bank.csv").write_bytes(DATA)
prepared = prepare(client, scan(client), bank) prepared = prepare(client, scan(client), bank)
assert prepared.context["files"][0]["state"] == "ALREADY_IMPORTED" assert prepared.context["files"][0]["state"] == "ALREADY_IMPORTED"
result = commit(client, prepared) 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]) (source / "ambiguous.csv").write_bytes(DATA + DATA.splitlines(keepends=True)[1])
prepared = prepare(client, scan(client), bank, **{"file-1-section_0": str(bank.pk)}) 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 [row["state"] for row in prepared.context["files"]] == ["NEEDS_ATTENTION", "READY"]
assert prepared.context["files"][0]["code"] == "REVIEW_REQUIRED"
result = commit(client, prepared) result = commit(client, prepared)
assert [row["state"] for row in result.context["files"]] == ["NEEDS_ATTENTION", "COMMITTED"] assert [row["state"] for row in result.context["files"]] == ["COMMITTED"]
assert result.context["files"][0]["code"] == "REVIEW_REQUIRED"
assert Transaction.objects.count() == 2 assert Transaction.objects.count() == 2
assert not ImportBatch.objects.filter(state="PREVIEW").exists() 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) monkeypatch.setattr(views.guided_imports, service_name, fail_one_file)
if stage == "prepare": if stage == "prepare":
prepared = prepare(client, scanned, bank, **{"file-1-section_0": str(bank.pk)}) prepared = prepare(client, scanned, bank, **{"file-1-section_0": str(bank.pk)})
assert prepared.context["files"][0]["code"] == "IMPORT_FAILED"
result = commit(client, prepared) result = commit(client, prepared)
if stage == "commit":
assert [row["state"] for row in result.context["files"]] == ["NEEDS_ATTENTION", "COMMITTED"] assert [row["state"] for row in result.context["files"]] == ["NEEDS_ATTENTION", "COMMITTED"]
assert result.context["files"][0]["code"] == "IMPORT_FAILED" assert result.context["files"][0]["code"] == "IMPORT_FAILED"
assert "Synthetic private source text" not in result.content.decode() else:
assert "1234.56" not in result.content.decode() 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 Transaction.objects.count() == 2
assert_private(result, source) assert_private(response, source)
@pytest.mark.parametrize("stage", ["prepare", "commit"]) @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): def test_changed_source_rejects_previously_signed_metadata(client, source, bank, stage):
scanned = scan(client) scanned = scan(client)
prepared = prepare(client, scanned, bank) if stage == "commit" else None 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) response = commit(client, prepared) if prepared else prepare(client, scanned, bank)
assert response.context["files"][0]["code"] == "FILE_CHANGED" assert response.context["files"][0]["code"] == "FILE_CHANGED"
assert not Transaction.objects.exists() assert not Transaction.objects.exists()

6
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")) page = browser.get(reverse("imports-index"))
csrf = page.cookies["csrftoken"].value csrf = page.cookies["csrftoken"].value
for expected in ({"created": 2, "matched": 0, "skipped": 0}, for index, expected in enumerate(({"created": 2, "matched": 0, "skipped": 0},
{"created": 0, "matched": 0, "skipped": 2}): {"created": 0, "matched": 0, "skipped": 2})):
if index:
(tmp_path / "owner-bank.csv").write_bytes(FIXTURE.read_bytes())
scanned = browser.post(reverse("imports-index"), { scanned = browser.post(reverse("imports-index"), {
"action": "scan", "csrfmiddlewaretoken": csrf, "action": "scan", "csrfmiddlewaretoken": csrf,
}, follow=True) }, follow=True)

Loading…
Cancel
Save