From e54d8edcbabb484f37c2f041c83afbc2af282aef Mon Sep 17 00:00:00 2001 From: Quinten Steenhuis Date: Thu, 1 Oct 2026 13:56:36 -0400 Subject: [PATCH 1/3] Check fee waiver uploads on the Fees page instead of leaving it Uploading a waiver on Fees redirected to the full document preview and back, which dropped the filer's fee waiver choice. The new copy is now previewed and confirmed (or removed) in place, and Continue waits for that check. Adds an opt-in Chromium test covering the Fees flows. Co-Authored-By: Claude Opus 5.5 --- efile_app/efile/static/js/document-preview.js | 9 +- efile_app/efile/static/js/payment.js | 68 ++++- efile_app/efile/static/js/waiver-upload.js | 8 +- .../efile/components/document_check.html | 27 ++ .../efile/components/document_preview.html | 4 +- efile_app/efile/templates/efile/payment.html | 60 +++-- .../efile/tests/test_payment_flow_browser.py | 156 +++++++++++ .../efile/tests/test_waiver_documents.py | 131 ++++++++- efile_app/efile/views/payment.py | 20 +- efile_app/efile/views/waiver_documents.py | 66 ++++- efile_app/js-tests/waiver-upload.test.js | 8 +- efile_app/tests/payment-flow-browser.js | 252 ++++++++++++++++++ 12 files changed, 754 insertions(+), 55 deletions(-) create mode 100644 efile_app/efile/templates/efile/components/document_check.html create mode 100644 efile_app/efile/tests/test_payment_flow_browser.py create mode 100644 efile_app/tests/payment-flow-browser.js diff --git a/efile_app/efile/static/js/document-preview.js b/efile_app/efile/static/js/document-preview.js index d47a397c..76aaa9fc 100644 --- a/efile_app/efile/static/js/document-preview.js +++ b/efile_app/efile/static/js/document-preview.js @@ -95,7 +95,12 @@ status.textContent = gettext("The PDF did not load. Download it or close and reopen this view."); } } - document.querySelectorAll("[data-pdf-preview]").forEach((details) => { + + function attach(details) { details.addEventListener("toggle", () => openPreview(details)); - }); + // Previews added after load, already open, may never fire a toggle. + if (details.open) openPreview(details); + } + window.attachPdfPreview = attach; + document.querySelectorAll("[data-pdf-preview]").forEach(attach); })(); \ No newline at end of file diff --git a/efile_app/efile/static/js/payment.js b/efile_app/efile/static/js/payment.js index ce56450b..e8a6f620 100644 --- a/efile_app/efile/static/js/payment.js +++ b/efile_app/efile/static/js/payment.js @@ -57,7 +57,69 @@ const PaymentPage = { setFeesState(loading) { document.getElementById("loadingSpinner").style.display = loading ? "block" : "none"; - document.getElementById("submitButton").disabled = loading || this.removingAccount || this.waiverUploading || !this.feeQuoteReady || !document.getElementById("selected-payment-account").value; + document.getElementById("submitButton").disabled = loading || this.removingAccount || this.waiverUploading || this.pendingDocumentChecks() || !this.feeQuoteReady || !document.getElementById("selected-payment-account").value; + }, + + pendingDocumentChecks() { + return Boolean(document.querySelector("[data-document-check]")); + }, + + addDocumentCheck(html) { + const container = document.getElementById("document-checks"); + const template = document.createElement("template"); + template.innerHTML = html.trim(); + const check = template.content.firstElementChild; + container.appendChild(check); + check.querySelectorAll("[data-pdf-preview]").forEach((details) => window.attachPdfPreview?.(details)); + this.setFeesState(false); + }, + + async checkDocument(button) { + const check = button.closest("[data-document-check]"); + const remove = button.hasAttribute("data-document-remove"); + const status = check.querySelector("[data-document-check-status]"); + const body = new FormData(); + body.append("action", remove ? "remove" : "confirm"); + body.append("document_id", check.dataset.documentId); + body.append("preview_fingerprint", check.dataset.fingerprint); + body.append("fee_inputs_token", paymentJSON("fee-inputs-token")); + body.append("csrfmiddlewaretoken", apiUtils.getCSRFToken()); + check.querySelectorAll("button").forEach((element) => { + element.disabled = true; + }); + status.textContent = remove ? gettext("Removing document…") : gettext("Saving…"); + try { + const response = await fetch(window.withFilingDraft(document.getElementById("document-checks").dataset.url), { + method: "POST", + body, + credentials: "same-origin" + }); + const data = await response.json(); + if (!response.ok || !data.success) throw new Error(data.error || gettext("That did not work. Try again.")); + document.getElementById("fee-inputs-token").textContent = JSON.stringify(data.fee_inputs_token); + check.remove(); + const confirmation = document.getElementById("waiver-upload-confirmation"); + if (remove) { + if (confirmation) confirmation.hidden = true; + const upload = document.getElementById("waiver-upload-required"); + if (upload) upload.hidden = false; + document.getElementById("add-waiver-document")?.focus(); + paymentMessages.showSuccess(gettext("Document removed. You can upload a different file.")); + // The removed document no longer counts toward fees. + await this.chooseIntent(); + } else { + if (confirmation && !confirmation.hidden) confirmation.textContent = gettext("Fee waiver document added and checked."); + const next = document.querySelector("[data-document-check]") || document.querySelector('input[name="paymentIntent"]:checked') || document.getElementById("submitButton"); + next.focus(); + } + } catch (error) { + status.textContent = error.message; + check.querySelectorAll("button").forEach((element) => { + element.disabled = false; + }); + } finally { + this.setFeesState(false); + } }, async loadAccountTypes() { @@ -318,6 +380,10 @@ const PaymentPage = { document.querySelectorAll('input[name="paymentIntent"]').forEach((input) => { input.addEventListener("change", () => this.chooseIntent()); }); + document.getElementById("document-checks").addEventListener("click", (event) => { + const button = event.target.closest("[data-document-confirm], [data-document-remove]"); + if (button) this.checkDocument(button); + }); await this.loadAccountTypes(); this.loadAccounts().catch(() => paymentMessages.showError(gettext("We could not load payment methods."))); } diff --git a/efile_app/efile/static/js/waiver-upload.js b/efile_app/efile/static/js/waiver-upload.js index a32bc705..7106c88b 100644 --- a/efile_app/efile/static/js/waiver-upload.js +++ b/efile_app/efile/static/js/waiver-upload.js @@ -83,14 +83,14 @@ document.addEventListener("DOMContentLoaded", () => { }); const data = await response.json(); if (!response.ok || !data.success) throw new Error(data.error || gettext("The upload failed. Try again.")); - if (data.preview_url) { - window.location.assign(window.withFilingDraft(data.preview_url)); - return; - } document.getElementById("fee-inputs-token").textContent = JSON.stringify(data.fee_inputs_token); document.getElementById("waiver-upload-required").hidden = true; + file.value = ""; + status.textContent = ""; const confirmation = document.getElementById("waiver-upload-confirmation"); confirmation.hidden = false; + // The filer checks the prepared copy here, without leaving Fees. + PaymentPage.addDocumentCheck(data.check_html); confirmation.focus(); // Old requests and quotes described a different set of documents. await PaymentPage.chooseIntent(); diff --git a/efile_app/efile/templates/efile/components/document_check.html b/efile_app/efile/templates/efile/components/document_check.html new file mode 100644 index 00000000..4d31469f --- /dev/null +++ b/efile_app/efile/templates/efile/components/document_check.html @@ -0,0 +1,27 @@ +{% load i18n %} +
+

+ {% translate "Check this copy before you continue." %} + {% translate "The court will get this PDF. Open it and check every page." %} +

+ {% if not document.preparation %} +

{% translate "Not ready. Remove this file and upload it again." %}

+ {% endif %} + {% include "efile/components/document_preview.html" with document=document open_preview=True %} +
+ + {% if removable %} + + {% endif %} +
+

+
diff --git a/efile_app/efile/templates/efile/components/document_preview.html b/efile_app/efile/templates/efile/components/document_preview.html index 5639725b..cf7f8aa5 100644 --- a/efile_app/efile/templates/efile/components/document_preview.html +++ b/efile_app/efile/templates/efile/components/document_preview.html @@ -1,5 +1,7 @@ {% load i18n %} -
+
{% translate "View PDF:" %} {{ document.name|default:document.original_filename }}

diff --git a/efile_app/efile/templates/efile/payment.html b/efile_app/efile/templates/efile/payment.html index 0ee2b715..98df9897 100644 --- a/efile_app/efile/templates/efile/payment.html +++ b/efile_app/efile/templates/efile/payment.html @@ -103,39 +103,43 @@

{% translate "Estimated fees before a waiver" %}

{% if fee_estimate.waiver_exemption.requires_waiver_account %}

{{ fee_estimate.waiver_exemption.note }}

{% else %} - {% if not has_waiver_document %} -
-

{% translate "The court may reject your waiver request if you do not include the required forms." %}

-

{% if fee_waiver.document_guidance %}{{ fee_waiver.document_guidance }}{% else %}{% translate "Include your fee waiver form or a court order that waives your fees." %}{% endif %}

-
+
+

{% translate "The court may reject your waiver request if you do not include the required forms." %}

+

{% if fee_waiver.document_guidance %}{{ fee_waiver.document_guidance }}{% else %}{% translate "Include your fee waiver form or a court order that waives your fees." %}{% endif %}

+
+ + +

- - {% endif %} +
+ {% endif %}
+
+ {% for check in document_checks %} + {% include "efile/components/document_check.html" with document=check.document fingerprint=check.fingerprint removable=check.removable %} + {% endfor %} +
diff --git a/efile_app/efile/tests/test_payment_flow_browser.py b/efile_app/efile/tests/test_payment_flow_browser.py new file mode 100644 index 00000000..19a974c1 --- /dev/null +++ b/efile_app/efile/tests/test_payment_flow_browser.py @@ -0,0 +1,156 @@ +"""Opt-in Chromium validation of the Fees screen. Uses only synthetic documents. + +Run with PAYMENT_FLOW_BROWSER_TESTS=1 uv run pytest -s efile/tests/test_payment_flow_browser.py +S3 and the court's code lists are test doubles; uploads, previews, and PDF.js are real. +""" + +import io +import json +import os +import subprocess +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest +from django.conf import settings +from django.urls import reverse + +from efile.models import FilingDocument, FilingDraft, FilingParty +from efile.tests.pdf_helpers import pdf_bytes +from efile.tests.test_document_extractions import authorize +from efile.tests.test_waiver_documents import codes + +ACCOUNTS = [ + {"paymentAccountID": "wv-1", "paymentAccountTypeCode": "WV", "accountName": "Fee waiver"}, + {"paymentAccountID": "cc-1", "paymentAccountTypeCode": "CC", "accountName": "Visa"}, +] + + +def payment_ready_draft(user, objects): + draft = FilingDraft.objects.create( + user=user, + jurisdiction="illinois", + workflow_version=2, + current_step="payment", + existing_case="new", + court_code="cook:law1", + court_name="Circuit Court of Cook County", + case_category_code="civil", + case_category_name="Civil", + case_type_code="contract", + case_type_name="Contract", + document_checklist_acknowledged=True, + ) + key = f"lead/{draft.pk}" + objects[key] = pdf_bytes("Synthetic petition") + FilingDocument.objects.create( + draft=draft, + role=FilingDocument.Role.LEAD, + name="Petition.pdf", + original_filename="Petition.pdf", + s3_key=key, + preparation="unchanged", + preparation_reviewed_at="2026-01-01T00:00:00Z", + filing_type_code="petition", + filing_type_name="Petition", + document_type_code="public", + document_type_name="Public", + filing_component_code="lead", + filing_component_name="Lead document", + ) + FilingParty.objects.create( + draft=draft, + role="filer", + party_type="PLA", + party_type_name="Plaintiff/Petitioner", + is_filing_party=True, + first_name="Jordan", + last_name="Taylor", + email="jordan@example.com", + address_line_1="123 Main Street", + city="Springfield", + state="IL", + zip_code="62701", + ) + return draft + + +@pytest.mark.integration +@pytest.mark.django_db(transaction=True) +@pytest.mark.skipif(not os.getenv("PAYMENT_FLOW_BROWSER_TESTS"), reason="Opt-in: requires Chromium") +def test_fees_screen_flows_in_browser(live_server, client, django_user_model, tmp_path): + user = django_user_model.objects.create_user(username="synthetic-fees", tyler_jurisdiction="illinois") + objects = {} + first = payment_ready_draft(user, objects) + second = payment_ready_draft(user, objects) + authorize(client, first) + handler = MagicMock() + handler.bucket_name = "synthetic" + handler.validate_file.return_value = {"valid": True} + handler._ensure_initialized.return_value = True + + def store(file, **kwargs): + key = f"{kwargs['file_type']}/{len(objects)}" + objects[key] = file.read() + return {"success": True, "key": key} + + handler.upload_file.side_effect = store + + def delete(key): + objects.pop(key, None) + return {"success": True} + + handler.delete_file.side_effect = delete + handler.get_public_url.side_effect = lambda key: f"https://synthetic.invalid/{key}" + handler.s3_client.get_object.side_effect = lambda **kw: {"Body": io.BytesIO(objects[kw["Key"]])} + files = { + "waiverFile": ("waiver.pdf", pdf_bytes("Application to waive court fees", pages=2)), + "wrongFile": ("wrong.pdf", pdf_bytes("Not the waiver")), + "invalidFile": ("broken.pdf", b"not a PDF"), + } + paths = {} + for key, (name, content) in files.items(): + (tmp_path / key).mkdir() + file = tmp_path / key / name + file.write_bytes(content) + paths[key] = str(file) + payment = reverse("payment", kwargs={"jurisdiction": "illinois"}) + evidence = Path(os.getenv("PAYMENT_FLOW_EVIDENCE_DIR", str(tmp_path / "evidence"))) + config = tmp_path / "browser.json" + config.write_text( + json.dumps( + { + "baseUrl": live_server.url, + "cookie": client.cookies[settings.SESSION_COOKIE_NAME].value, + "evidence": str(evidence), + "paymentUrl": f"{payment}?draft={first.pk}", + "secondPaymentUrl": f"{payment}?draft={second.pk}", + **paths, + } + ) + ) + with ( + patch("efile.views.waiver_documents.S3UploadHandler", return_value=handler), + patch("efile.views.document_previews.S3UploadHandler", return_value=handler), + patch("efile.utils.s3_upload_handler.S3UploadHandler", return_value=handler), + patch("efile.services.waiver_documents._codes", side_effect=codes), + patch("efile.views.payment.estimate_fees", return_value={}), + patch("efile.views.payment.payment_accounts", return_value=ACCOUNTS), + patch("efile.views.review.get_case_questions", return_value=[]), + ): + result = subprocess.run( + ["node", "tests/payment-flow-browser.js", str(config)], + cwd=settings.BASE_DIR, + capture_output=True, + text=True, + timeout=240, + ) + print(result.stdout) + assert result.returncode == 0, result.stdout + result.stderr + first.refresh_from_db() + assert first.selected_payment_account_id == "wv-1" + assert [doc.name for doc in first.documents.order_by("role", "sort_order")] == ["Petition.pdf", "waiver.pdf"] + assert not first.documents.filter(preparation_reviewed_at__isnull=True).exists() + # The wrong file and the copy removed from another tab are gone, with their bytes. + assert [doc.name for doc in second.documents.order_by("role", "sort_order")] == ["Petition.pdf", "waiver.pdf"] + assert sorted(key.split("/")[0] for key in objects) == ["lead", "lead", "supporting", "supporting"] diff --git a/efile_app/efile/tests/test_waiver_documents.py b/efile_app/efile/tests/test_waiver_documents.py index fc48c027..f6348d53 100644 --- a/efile_app/efile/tests/test_waiver_documents.py +++ b/efile_app/efile/tests/test_waiver_documents.py @@ -1,3 +1,4 @@ +import re from unittest.mock import patch import pytest @@ -84,7 +85,7 @@ def test_filename_alone_does_not_hide_prompt(client, payment_draft): lead.save() with patch("efile.views.payment.estimate_fees", return_value={}): page = client.get(reverse("payment", kwargs={"jurisdiction": "illinois"})) - assert b'id="add-waiver-document"' not in page.content + assert re.search(r'id="waiver-upload-required"\s+hidden', page.content.decode()) assert b"Add fee waiver documents" not in page.content @@ -107,6 +108,13 @@ def upload_data(draft, **changes): } +def confirm_data(document): + from efile.services.document_previews import preview_fingerprint + + document.refresh_from_db() + return {"action": "confirm", "document_id": document.pk, "preview_fingerprint": preview_fingerprint([document])} + + @pytest.fixture def storage(): with patch("efile.views.waiver_documents.S3UploadHandler") as constructor: @@ -223,6 +231,18 @@ def test_payment_upload_stays_with_displayed_draft_through_review(client, paymen uploaded = client.post(upload_url, upload_data(payment_draft)) assert uploaded.status_code == 200 assert other.documents.count() == 0 + with patch( + "efile.views.payment.payment_accounts", + return_value=[{"paymentAccountID": "wv", "paymentAccountTypeCode": "WV", "accountName": "Waiver"}], + ): + result = client.post(payment_url, {"selected_payment_account": "wv"}) + # The new copy is checked on Fees itself; Continue waits for that. + assert result.status_code == 302 + assert "/payment/" in result.url + added = payment_draft.documents.get(role=FilingDocument.Role.SUPPORTING) + assert f'data-document-id="{added.pk}"' in uploaded.json()["check_html"] + approved = client.post(upload_url, confirm_data(added)) + assert approved.status_code == 200 with patch( "efile.views.payment.payment_accounts", return_value=[{"paymentAccountID": "wv", "paymentAccountTypeCode": "WV", "accountName": "Waiver"}], @@ -230,17 +250,6 @@ def test_payment_upload_stays_with_displayed_draft_through_review(client, paymen result = client.post(payment_url, {"selected_payment_account": "wv"}) assert result.status_code == 302 assert f"draft={payment_draft.pk}" in result.url - preview = client.get(uploaded.json()["preview_url"] + f"&draft={payment_draft.pk}") - assert preview.status_code == 200 - approved = client.post( - uploaded.json()["preview_url"] + f"&draft={payment_draft.pk}", - { - "preview_fingerprint": preview.context["preview_fingerprint"], - "reviewed_document": [str(doc.pk) for doc in payment_draft.documents.all()], - "return_to": "review", - }, - ) - assert approved.status_code == 302 with patch("efile.views.review.get_case_questions", return_value=[]): review = client.get(result.url) assert review.status_code == 200 @@ -308,3 +317,101 @@ def test_database_failure_cleans_uploaded_original_and_filing(client, payment_dr ) assert {call.args[0] for call in storage.delete_file.call_args_list} == {"original.docx", "filing.pdf"} assert payment_draft.documents.count() == 1 + + +def upload_waiver(client, draft): + with patch("efile.services.waiver_documents._codes", side_effect=codes): + response = client.post(endpoint(draft), upload_data(draft)) + assert response.status_code == 200 + return response, draft.documents.get(role=FilingDocument.Role.SUPPORTING) + + +def test_upload_is_checked_on_fees_without_leaving_it(client, payment_draft, storage): + response, added = upload_waiver(client, payment_draft) + body = response.json() + assert "preview_url" not in body + assert "data-document-confirm" in body["check_html"] + assert "data-document-remove" in body["check_html"] + assert added.preparation_reviewed_at is None + with patch("efile.views.payment.estimate_fees", return_value={}): + page = client.get(reverse("payment", kwargs={"jurisdiction": "illinois"}) + f"?draft={payment_draft.pk}") + # A reload before confirming still shows the check on Fees, open. + assert page.context["document_checks"][0]["document"].pk == added.pk + assert f'data-document-id="{added.pk}"'.encode() in page.content + assert client.post(endpoint(payment_draft), confirm_data(added)).status_code == 200 + added.refresh_from_db() + assert added.preparation_reviewed_at is not None + assert payment_draft.documents.get(role=FilingDocument.Role.LEAD).preparation_reviewed_at is not None + + +def test_confirm_rejects_a_changed_copy_and_foreign_documents(client, payment_draft, storage): + from efile.models import FilingDraft + + _, added = upload_waiver(client, payment_draft) + stale = confirm_data(added) | {"preview_fingerprint": "old"} + assert client.post(endpoint(payment_draft), stale).status_code == 409 + other = FilingDraft.objects.create(user=payment_draft.user, jurisdiction="illinois") + foreign = FilingDocument.objects.create(draft=other, role="lead", name="x.pdf", preparation="unchanged") + assert client.post(endpoint(payment_draft), confirm_data(foreign)).status_code == 409 + added.refresh_from_db() + foreign.refresh_from_db() + assert added.preparation_reviewed_at is None + assert foreign.preparation_reviewed_at is None + + +def test_remove_only_takes_out_a_waiver_and_reprices( + client, payment_draft, storage, django_capture_on_commit_callbacks +): + _, added = upload_waiver(client, payment_draft) + lead = payment_draft.documents.get(role=FilingDocument.Role.LEAD) + payment_draft.refresh_from_db() + remove = {"action": "remove", "fee_inputs_token": fee_inputs_token(payment_draft)} + assert client.post(endpoint(payment_draft), remove | {"document_id": lead.pk}).status_code == 400 + assert ( + client.post(endpoint(payment_draft), remove | {"document_id": added.pk, "fee_inputs_token": "x"}).status_code + == 409 + ) + with ( + patch("efile.utils.s3_upload_handler.S3UploadHandler", return_value=storage), + django_capture_on_commit_callbacks(execute=True), + ): + response = client.post(endpoint(payment_draft), remove | {"document_id": added.pk}) + assert response.status_code == 200 + assert list(payment_draft.documents.all()) == [lead] + payment_draft.refresh_from_db() + assert response.json()["fee_inputs_token"] == fee_inputs_token(payment_draft) + storage.delete_file.assert_called_with("waivers/test.pdf") + + +def test_payment_waits_for_unchecked_documents(client, payment_draft, storage): + upload_waiver(client, payment_draft) + payment_url = reverse("payment", kwargs={"jurisdiction": "illinois"}) + f"?draft={payment_draft.pk}" + with patch( + "efile.views.payment.payment_accounts", + return_value=[{"paymentAccountID": "wv", "paymentAccountTypeCode": "WV", "accountName": "Waiver"}], + ) as accounts: + result = client.post(payment_url, {"selected_payment_account": "wv"}) + assert result.status_code == 302 + assert "/payment/" in result.url + accounts.assert_not_called() + payment_draft.refresh_from_db() + assert payment_draft.selected_payment_account_id != "wv" + + +def test_review_sends_an_unchecked_copy_to_preview_and_back(client, payment_draft, storage): + from efile.services.document_previews import preview_fingerprint + + upload_waiver(client, payment_draft) + payment_draft.selected_payment_account_id = "wv" + payment_draft.save() + review_url = reverse("case_review", kwargs={"jurisdiction": "illinois"}) + f"?draft={payment_draft.pk}" + bounced = client.get(review_url) + assert bounced.status_code == 302 + assert "preview-documents" in bounced.url and "return_to=review" in bounced.url + preview_url = reverse("preview_documents", kwargs={"jurisdiction": "illinois"}) + f"?draft={payment_draft.pk}" + approved = client.post( + preview_url, + {"preview_fingerprint": preview_fingerprint(list(payment_draft.documents.all())), "return_to": "review"}, + ) + assert approved.status_code == 302 + assert "/review/" in approved.url diff --git a/efile_app/efile/views/payment.py b/efile_app/efile/views/payment.py index f95106c4..5e8bc3db 100644 --- a/efile_app/efile/views/payment.py +++ b/efile_app/efile/views/payment.py @@ -9,6 +9,7 @@ from efile.models import FilingDocument, FilingParty from efile.services.appeals import appeal_answers_complete from efile.services.current_drafts import ensure_current_draft +from efile.services.document_previews import preview_fingerprint from efile.services.draft_urls import draft_url from efile.services.drafts import draft_snapshot, read_case_data from efile.services.fee_estimates import estimate_fees @@ -17,6 +18,7 @@ from efile.services.people import filing_parties from efile.services.waiver_documents import has_waiver_document from efile.utils.config_loader import config_loader +from efile.views.waiver_documents import is_removable_waiver from ..workflow import WorkflowStepKey, get_step_url, get_workflow_context @@ -51,6 +53,14 @@ def efile_payment(request, jurisdiction): messages.error(request, "Complete the lower court information before checking fees.") return redirect("case_questions", jurisdiction=jurisdiction) + documents = list(FilingDocument.objects.filter(draft=draft).order_by("role", "sort_order", "pk")) + unchecked = [document for document in documents if document.preparation_reviewed_at is None] + if request.method == "POST" and unchecked: + # The page keeps Continue off until these are confirmed; this covers + # a stale tab, so Review never sends the filer to another screen. + messages.error(request, "Check your documents before you continue.") + return redirect("payment", jurisdiction=jurisdiction) + if request.method == "POST": account_id = request.POST.get("selected_payment_account", "").strip() try: @@ -80,7 +90,15 @@ def efile_payment(request, jurisdiction): return redirect(get_step_url(WorkflowStepKey.REVIEW, jurisdiction)) context = { - "documents": FilingDocument.objects.filter(draft=draft).order_by("role", "sort_order", "pk"), + "documents": [document for document in documents if document.preparation_reviewed_at is not None], + "document_checks": [ + { + "document": document, + "fingerprint": preview_fingerprint([document]), + "removable": is_removable_waiver(document), + } + for document in unchecked + ], "waiver_upload_url": draft_url(reverse("waiver_documents", kwargs={"jurisdiction": jurisdiction}), draft.pk), "has_waiver_document": has_waiver_document(draft), "is_logged_in": True, diff --git a/efile_app/efile/views/waiver_documents.py b/efile_app/efile/views/waiver_documents.py index f98fcb02..59c94803 100644 --- a/efile_app/efile/views/waiver_documents.py +++ b/efile_app/efile/views/waiver_documents.py @@ -1,17 +1,70 @@ from django.db import transaction from django.db.models import Max from django.http import JsonResponse +from django.template.loader import render_to_string +from django.utils import timezone from django.views.decorators.http import require_http_methods from efile.api.suffolk_api_views import get_tyler_token from efile.models import FilingDocument, FilingDraft from efile.services.current_drafts import explicit_draft_id, get_current_draft -from efile.services.document_preparation import cleanup_uploads, store_prepared_document +from efile.services.document_preparation import cleanup_unreferenced_uploads, cleanup_uploads, store_prepared_document +from efile.services.document_previews import document_storage_keys, preview_fingerprint from efile.services.drafts import ACTIVE_DRAFT_STATUSES from efile.services.fee_quotes import fee_inputs_token, invalidate_fee_quote -from efile.services.waiver_documents import waiver_document_choices, waiver_filing_types +from efile.services.waiver_documents import WAIVER_TYPE, waiver_document_choices, waiver_filing_types from efile.utils.s3_upload_handler import S3UploadHandler -from efile.workflow import WorkflowStepKey, get_step_url + + +def document_check_html(request, document): + """The fees page checks a newly added copy in place, not on another screen.""" + return render_to_string( + "efile/components/document_check.html", + { + "document": document, + "jurisdiction": document.draft.jurisdiction, + "fingerprint": preview_fingerprint([document]), + "removable": is_removable_waiver(document), + }, + request=request, + ) + + +def is_removable_waiver(document): + return document.role == FilingDocument.Role.SUPPORTING and bool(WAIVER_TYPE.search(document.filing_type_name)) + + +def _check_document(request, draft, action): + """Confirm or remove one copy shown on the fees page.""" + data = request.POST + with transaction.atomic(): + draft = FilingDraft.objects.select_for_update().get(pk=draft.pk) + if draft.status not in ACTIVE_DRAFT_STATUSES: + return JsonResponse({"error": "This filing is not available to edit."}, status=409) + document = draft.documents.filter(pk=data.get("document_id") or None).first() + if document is None: + return JsonResponse( + {"error": "This document is no longer part of your filing. Reload this page."}, status=409 + ) + if action == "confirm": + if not document.preparation: + return JsonResponse({"error": "This file is not ready. Remove it and upload it again."}, status=409) + if data.get("preview_fingerprint") != preview_fingerprint([document]): + return JsonResponse({"error": "This file changed. Reload this page and check it again."}, status=409) + document.preparation_reviewed_at = timezone.now() + document.save(update_fields=["preparation_reviewed_at", "updated_at"]) + return JsonResponse({"success": True, "fee_inputs_token": fee_inputs_token(draft)}) + if not is_removable_waiver(document): + return JsonResponse({"error": "Change this document from the upload step."}, status=400) + if data.get("fee_inputs_token") != fee_inputs_token(draft): + return JsonResponse( + {"error": "This filing changed. Reload this page before removing a document."}, status=409 + ) + keys = document_storage_keys(document) + document.delete() + invalidate_fee_quote(draft) + transaction.on_commit(lambda: cleanup_unreferenced_uploads(keys)) + return JsonResponse({"success": True, "fee_inputs_token": fee_inputs_token(draft)}) @require_http_methods(["GET", "POST"]) @@ -23,6 +76,9 @@ def waiver_documents(request, jurisdiction): draft = get_current_draft(request, jurisdiction=jurisdiction) if draft is None or draft.status not in ACTIVE_DRAFT_STATUSES or not draft.court_code: return JsonResponse({"error": "This filing is not available to edit."}, status=409) + action = request.POST.get("action", "") if request.method == "POST" else "" + if action in ("confirm", "remove"): + return _check_document(request, draft, action) keys = [] handler = S3UploadHandler() try: @@ -61,7 +117,7 @@ def waiver_documents(request, jurisdiction): highest = draft.documents.filter(role=FilingDocument.Role.SUPPORTING).aggregate(order=Max("sort_order"))[ "order" ] - FilingDocument.objects.create( + document = FilingDocument.objects.create( draft=draft, role=FilingDocument.Role.SUPPORTING, sort_order=0 if highest is None else highest + 1, @@ -80,7 +136,7 @@ def waiver_documents(request, jurisdiction): { "success": True, "fee_inputs_token": fee_inputs_token(draft), - "preview_url": get_step_url(WorkflowStepKey.PREVIEW_DOCUMENTS, jurisdiction) + "?return_to=payment", + "check_html": document_check_html(request, document), } ) except ValueError as error: diff --git a/efile_app/js-tests/waiver-upload.test.js b/efile_app/js-tests/waiver-upload.test.js index 23a681b0..fb9a37ae 100644 --- a/efile_app/js-tests/waiver-upload.test.js +++ b/efile_app/js-tests/waiver-upload.test.js @@ -36,6 +36,9 @@ function harness(fetch) { }; const payment = { setFeesState() {}, + addDocumentCheck(html) { + this.addedCheck = html; + }, async chooseIntent() { this.refreshed = true; } @@ -117,7 +120,8 @@ test("upload stays on payment and replaces the stale fee token", async () => { ok: true, json: async () => ({ success: true, - fee_inputs_token: "new-token" + fee_inputs_token: "new-token", + check_html: "
" }) }; }); @@ -132,6 +136,8 @@ test("upload stays on payment and replaces the stale fee token", async () => { assert.equal(node("waiver-upload-confirmation").hidden, false); assert.equal(payment.refreshed, true); assert.equal(payment.waiverUploading, false); + // The new copy is checked right here instead of on another page. + assert.equal(payment.addedCheck, "
"); }); test("failed uploads keep the picker and file available for retry", async () => { diff --git a/efile_app/tests/payment-flow-browser.js b/efile_app/tests/payment-flow-browser.js new file mode 100644 index 00000000..59b7abfd --- /dev/null +++ b/efile_app/tests/payment-flow-browser.js @@ -0,0 +1,252 @@ +/* Browser checks for the Fees screen, invoked by the opt-in Django integration test. */ +const assert = require("node:assert/strict"); +const fs = require("node:fs"); +const path = require("node:path"); +const { + chromium +} = require("@playwright/test"); +const AxeBuilder = require("@axe-core/playwright").default; + +const WAIVER_ACCOUNT = { + paymentAccountID: "wv-1", + accountName: "Fee waiver", + paymentAccountTypeCode: "WV", + active: { + value: true + } +}; +const CARD_ACCOUNT = { + paymentAccountID: "cc-1", + accountName: "Visa", + paymentAccountTypeCode: "CC", + cardLast4: "4242", + active: { + value: true + } +}; + +async function main() { + const config = JSON.parse(fs.readFileSync(process.argv[2], "utf8")); + const browser = await chromium.launch({ + executablePath: process.env.PLAYWRIGHT_CHROMIUM_EXECUTABLE || undefined + }); + const context = await browser.newContext({ + viewport: { + width: 1280, + height: 1000 + } + }); + await context.addCookies([{ + name: "sessionid", + value: config.cookie, + url: config.baseUrl + }]); + const feeRequests = []; + // Court-side APIs are synthetic; the app's own endpoints are real. + await context.route("**/api/payment-account-types/**", (route) => route.fulfill({ + json: { + success: true, + data: [{ + code: "CC", + description: "Credit card" + }] + } + })); + await context.route("**/api/payment-accounts/**", (route) => route.fulfill({ + json: { + success: true, + data: [WAIVER_ACCOUNT, CARD_ACCOUNT] + } + })); + await context.route("**/api/waiver-account/**", (route) => route.fulfill({ + json: { + success: true, + data: WAIVER_ACCOUNT + } + })); + await context.route("**/api/payment-fees/**", (route) => { + feeRequests.push(route.request().postDataJSON()); + return route.fulfill({ + json: { + success: true, + quote_recorded: true, + api_response: { + feesCalculationAmount: { + value: "120.00" + }, + allowanceCharge: [] + } + } + }); + }); + const page = await context.newPage(); + const errors = []; + const visited = []; + page.on("pageerror", (error) => errors.push(error.message)); + page.on("console", (message) => { + if (message.type() === "error") console.log("console:", message.text()); + }); + page.on("framenavigated", (frame) => { + if (frame === page.mainFrame()) visited.push(new URL(frame.url()).pathname); + }); + page.on("dialog", (dialog) => dialog.accept()); + page.on("response", (response) => { + if (response.status() >= 400) console.log("HTTP", response.status(), response.url()); + }); + fs.mkdirSync(config.evidence, { + recursive: true + }); + const screenshot = (name) => page.screenshot({ + path: path.join(config.evidence, name), + fullPage: true + }); + const payment = config.baseUrl + config.paymentUrl; + const submit = page.locator("#submitButton"); + const checks = page.locator("[data-document-check]"); + + const chooseWaiver = async () => { + await page.locator('input[name="paymentIntent"][value="waiver"]').check(); + await page.locator("#successMessage:not([hidden])").waitFor(); + }; + const uploadWaiver = async (file) => { + if (await page.locator("#waiver-upload-panel").isHidden()) await page.locator("#add-waiver-document").click(); + await page.locator("#upload-waiver-document:not([disabled])").waitFor(); + await page.locator("#waiver-document-type").selectOption("private"); + await page.locator("#waiver-file").setInputFiles(file); + await page.locator("#upload-waiver-document").click(); + }; + const waitRendered = (locator) => locator.locator("[data-pdf-preview][data-rendered='true']").waitFor(); + + try { + // 1. The reported flow: upload on Fees, check it there, go to Review. + await page.goto(payment); + await chooseWaiver(); + visited.length = 0; + await uploadWaiver(config.waiverFile); + await checks.first().waitFor(); + assert.equal(await checks.count(), 1); + assert.ok(page.url().includes("/payment/"), `left Fees for ${page.url()}`); + assert.ok(!visited.some((url) => url.includes("preview-documents")), `visited ${visited}`); + assert.equal(await page.locator('input[name="paymentIntent"][value="waiver"]').isChecked(), true); + assert.equal(await page.locator("#waiver-upload-required").isHidden(), true); + await waitRendered(checks.first()); + assert.equal(await submit.isDisabled(), true, "Continue must wait for the copy to be checked"); + await screenshot("01-fees-check-waiver.png"); + const axe = await new AxeBuilder({ + page + }).include(".workflow-card").analyze(); + assert.deepEqual(axe.violations.map((item) => item.id), []); + await page.setViewportSize({ + width: 390, + height: 844 + }); + assert.equal(await page.evaluate(() => document.documentElement.scrollWidth <= window.innerWidth), true); + await screenshot("02-fees-check-mobile.png"); + await page.setViewportSize({ + width: 1280, + height: 1000 + }); + await checks.first().locator("[data-document-confirm]").click(); + await checks.first().waitFor({ + state: "detached" + }); + await page.locator("#submitButton:not([disabled])").waitFor(); + assert.match(await page.locator("#waiver-upload-confirmation").innerText(), /checked/); + await Promise.all([page.waitForURL(/\/review\//), submit.click()]); + assert.ok(!visited.some((url) => url.includes("preview-documents")), `visited ${visited}`); + assert.match(await page.locator("body").innerText(), /waiver\.pdf/); + await screenshot("03-review-after-waiver.png"); + + // 2. Coming back from Review: the waiver is part of the filing already. + await page.goto(payment); + assert.equal(await checks.count(), 0); + await chooseWaiver(); + assert.equal(await page.locator("#waiver-upload-required").isHidden(), true); + await page.locator("#submitButton:not([disabled])").waitFor(); + + // 3. Paying instead: the fee request includes the waiver document. + await page.locator('input[name="paymentIntent"][value="pay"]').check(); + await page.locator("#paymentSection:not([hidden])").waitFor(); + const priced = JSON.stringify(feeRequests.at(-1)); + assert.match(priced, /waiver\.pdf/, "fee request must price the added document"); + await page.locator("#submitButton:not([disabled])").waitFor(); + console.log("Scenarios 1-3 passed"); + } catch (error) { + await screenshot("browser-failure.png"); + throw error; + } + + // A second filing that has no waiver yet. + const secondPayment = config.baseUrl + config.secondPaymentUrl; + try { + // 4. Remove a wrong file and upload another, still on Fees. + await page.goto(secondPayment); + await chooseWaiver(); + await uploadWaiver(config.wrongFile); + await checks.first().waitFor(); + await checks.first().locator("[data-document-remove]").click(); + await checks.first().waitFor({ + state: "detached" + }); + await page.locator("#waiver-upload-required:not([hidden])").waitFor(); + assert.equal(await page.locator("#waiver-upload-confirmation").isHidden(), true); + assert.equal(await page.locator('input[name="paymentIntent"][value="waiver"]').isChecked(), true); + await uploadWaiver(config.waiverFile); + await checks.first().waitFor(); + assert.equal(await checks.count(), 1); + assert.match(await checks.first().innerText(), /waiver\.pdf/); + + // 5. Reload before checking: the check is still on Fees, not elsewhere. + await page.reload(); + assert.ok(page.url().includes("/payment/")); + assert.equal(await checks.count(), 1); + await waitRendered(checks.first()); + await chooseWaiver(); + assert.equal(await submit.isDisabled(), true); + + // 6. A copy removed in another tab cannot be confirmed in this one. + const other = await context.newPage(); + await other.goto(secondPayment); + await other.locator("[data-document-check] [data-document-remove]").click(); + await other.locator("[data-document-check]").waitFor({ + state: "detached" + }); + await other.close(); + await checks.first().locator("[data-document-confirm]").click(); + await checks.first().locator("[data-document-check-status]").filter({ + hasText: /no longer part of your filing/ + }).waitFor(); + assert.equal(await submit.isDisabled(), true); + await screenshot("04-stale-tab.png"); + + // 7. A file that cannot be prepared explains itself and adds nothing. + await page.goto(secondPayment); + await chooseWaiver(); + await uploadWaiver(config.invalidFile); + await page.locator("#waiver-upload-status").filter({ + hasText: /could not be read/ + }).waitFor(); + assert.equal(await checks.count(), 0); + assert.equal(await page.locator("#upload-waiver-document").isEnabled(), true); + await uploadWaiver(config.waiverFile); + await checks.first().locator("[data-document-confirm]").click(); + await checks.first().waitFor({ + state: "detached" + }); + await page.locator("#submitButton:not([disabled])").waitFor(); + await Promise.all([page.waitForURL(/\/review\//), submit.click()]); + console.log("Scenarios 4-7 passed"); + } catch (error) { + await screenshot("browser-failure-2.png"); + throw error; + } + assert.deepEqual(errors, []); + console.log("Fees browser validation passed."); + await browser.close(); +} + +main().catch((error) => { + console.error(error); + // An open browser would keep this process alive past a failure. + process.exit(1); +}); \ No newline at end of file From 19f93939ca28859328a098fd6f4c3c63d498d334 Mon Sep 17 00:00:00 2001 From: Quinten Steenhuis Date: Thu, 1 Oct 2026 14:03:26 -0400 Subject: [PATCH 2/3] Route every detour back through one model in workflow.py Change files on the preview dropped where the filer came from, so someone fixing a file from Review walked the whole flow again. Where a finished step goes is now decided by workflow.continue_url: back to Review or the handoff list, or to the one step that fixes what the change broke (Organize for a document without a filing type, Find your case for an existing case not yet found), still carrying the marker. The handoff middleware no longer rewrites every successful POST, which sent filers back to the list mid-step: adding a person on People, and finding a case before confirming it. Gates and intermediate actions keep the marker, markers are checked in one place, and only review and handoff are accepted. The organizing check no longer treats an empty document type as unfinished, since Organize saves none when the court offers no choices. Co-Authored-By: Claude Opus 5.5 --- efile_app/efile/middleware.py | 19 +-- .../templates/efile/case_confirmation.html | 1 + efile_app/efile/templates/efile/payment.html | 1 + .../templates/efile/preview_documents.html | 2 +- .../templates/efile/upload_documents.html | 13 +- efile_app/efile/tests/test_detours.py | 155 ++++++++++++++++++ .../efile/tests/test_document_previews.py | 55 ++++++- .../efile/tests/test_payment_flow_browser.py | 4 + .../efile/tests/test_reorganized_start.py | 2 + efile_app/efile/views/case_confirmation.py | 19 ++- efile_app/efile/views/case_lookup.py | 18 +- efile_app/efile/views/case_questions.py | 19 +-- efile_app/efile/views/document_checklist.py | 22 +-- efile_app/efile/views/document_previews.py | 9 +- efile_app/efile/views/extraction_review.py | 38 +++-- efile_app/efile/views/handoff.py | 15 +- efile_app/efile/views/organize_documents.py | 29 +++- efile_app/efile/views/parties.py | 22 ++- efile_app/efile/views/party_details.py | 21 ++- efile_app/efile/views/payment.py | 26 ++- efile_app/efile/views/review.py | 6 +- efile_app/efile/views/upload_documents.py | 20 ++- efile_app/efile/views/your_information.py | 12 +- efile_app/efile/workflow.py | 79 +++++++-- efile_app/tests/payment-flow-browser.js | 38 +++++ 25 files changed, 505 insertions(+), 140 deletions(-) create mode 100644 efile_app/efile/tests/test_detours.py diff --git a/efile_app/efile/middleware.py b/efile_app/efile/middleware.py index 86c59036..eef8cf14 100644 --- a/efile_app/efile/middleware.py +++ b/efile_app/efile/middleware.py @@ -1,7 +1,6 @@ from django.contrib.auth import logout from django.http import JsonResponse from django.shortcuts import render -from django.urls import reverse from django.utils.deprecation import MiddlewareMixin from efile.models import FilingDraft @@ -43,27 +42,17 @@ def process_exception(self, request, exception): def process_response(self, request, response): draft = getattr(request, "filing_draft", None) if draft is not None: - # A targeted edit opened from the interview handoff returns to its - # missing-details list, instead of re-asking completed later steps. - targeted = request.GET.get("return_to") == "handoff" or request.POST.get("return_to") == "handoff" - destination = None - if ( - targeted - and request.method == "POST" - and response.status_code < 400 - and draft.status == FilingDraft.Status.DRAFT - ): - destination = reverse("handoff_review", args=[draft.pk]) + # Where a step goes next, including back to a detour's origin, is + # the step's own decision (efile.workflow.continue_url). This only + # keeps the filing named in the URL it chose. if response.has_header("Location"): - if destination: - response["Location"] = destination response["Location"] = draft_url(response["Location"], draft.pk) elif isinstance(response, JsonResponse): import json payload = json.loads(response.content) if isinstance(payload, dict) and isinstance(payload.get("redirect_url"), str): - payload["redirect_url"] = destination or draft_url(payload["redirect_url"], draft.pk) + payload["redirect_url"] = draft_url(payload["redirect_url"], draft.pk) response.content = json.dumps(payload) return response diff --git a/efile_app/efile/templates/efile/case_confirmation.html b/efile_app/efile/templates/efile/case_confirmation.html index 520cd2c6..12e7603e 100644 --- a/efile_app/efile/templates/efile/case_confirmation.html +++ b/efile_app/efile/templates/efile/case_confirmation.html @@ -64,6 +64,7 @@

{% translate "Is this your court case?" %}

{% if availability_message %}{% endif %}
{% csrf_token %} +
{% translate "Does this match your case?" %}

{% translate "If you choose Yes, we will attach your documents to this case." %}

diff --git a/efile_app/efile/templates/efile/payment.html b/efile_app/efile/templates/efile/payment.html index 98df9897..ce87963f 100644 --- a/efile_app/efile/templates/efile/payment.html +++ b/efile_app/efile/templates/efile/payment.html @@ -66,6 +66,7 @@

{% translate "Estimated fees before a waiver" %}

{% csrf_token %} + diff --git a/efile_app/efile/templates/efile/preview_documents.html b/efile_app/efile/templates/efile/preview_documents.html index e53e38e4..d4a6f5d3 100644 --- a/efile_app/efile/templates/efile/preview_documents.html +++ b/efile_app/efile/templates/efile/preview_documents.html @@ -26,7 +26,7 @@

{% translate "Check your documents" %}

{% endfor %}
{% translate "Change files" %} + href="{% url 'upload_documents' jurisdiction %}{% if return_to %}?return_to={{ return_to }}{% endif %}">{% translate "Change files" %} diff --git a/efile_app/efile/templates/efile/upload_documents.html b/efile_app/efile/templates/efile/upload_documents.html index 742a2bde..30bcca7f 100644 --- a/efile_app/efile/templates/efile/upload_documents.html +++ b/efile_app/efile/templates/efile/upload_documents.html @@ -208,10 +208,19 @@

{% translate "Your documents" %}

never showed "What are you trying to do?", so it is not where Back goes. See efile.workflow.get_visible_workflow. {% endcomment %} - {% translate "Back" %} + {% if return_to and has_lead_document %} + {% comment %} + Came here to change files from a later screen: Back is the + preview it came from, not the start of the filing. + {% endcomment %} + {% translate "Back" %} + {% else %} + {% translate "Back" %} + {% endif %} {% translate "Preview your PDFs" %}
diff --git a/efile_app/efile/tests/test_detours.py b/efile_app/efile/tests/test_detours.py new file mode 100644 index 00000000..31915cc0 --- /dev/null +++ b/efile_app/efile/tests/test_detours.py @@ -0,0 +1,155 @@ +"""A filer sent to an earlier step from Review or the handoff list comes back. + +See the "Detours" section of efile.workflow: only a step's completion decides +where to go next, and an intermediate action keeps the marker on its screen. +""" + +import json +from unittest.mock import patch + +import pytest +from django.urls import reverse + +from efile.models import FilingDocument, FilingParty +from efile.tests.test_review_submit_flow import submission_draft as _submission_draft +from efile.workflow import ( + WorkflowStepKey, + clean_return_to, + continue_step, + continue_url, + documents_need_organizing, + with_return_to, +) + +draft = _submission_draft + +pytestmark = pytest.mark.django_db + +J = {"jurisdiction": "illinois"} + + +def path(url): + return url.partition("?")[0] + + +def step(name, draft, return_to=""): + url = reverse(name, kwargs=J) + f"?draft={draft.pk}" + return f"{url}&return_to={return_to}" if return_to else url + + +def test_only_known_origins_are_markers(): + assert clean_return_to("review") == "review" + assert clean_return_to("handoff") == "handoff" + for value in ("payment", "document_checklist", "https://example.com", "", None, '"> + {% endblock extra_js %} diff --git a/efile_app/efile/templates/efile/payment.html b/efile_app/efile/templates/efile/payment.html index ce87963f..e2c8bdab 100644 --- a/efile_app/efile/templates/efile/payment.html +++ b/efile_app/efile/templates/efile/payment.html @@ -136,7 +136,9 @@

{% translate "Estimated fees before a waiver" %}

{% endif %} -
+
{% for check in document_checks %} {% include "efile/components/document_check.html" with document=check.document fingerprint=check.fingerprint removable=check.removable %} {% endfor %} @@ -178,6 +180,7 @@

{% translate "Confirmed fees" %}

{% endblock workflow_content %} {% block extra_js %} + {% endblock extra_js %} diff --git a/efile_app/efile/tests/test_checklist_document_checks.py b/efile_app/efile/tests/test_checklist_document_checks.py new file mode 100644 index 00000000..80f349f7 --- /dev/null +++ b/efile_app/efile/tests/test_checklist_document_checks.py @@ -0,0 +1,92 @@ +"""Files added on the checklist are checked there, not after a detour from Review.""" + +from unittest.mock import patch + +import pytest +from django.core.files.uploadedfile import SimpleUploadedFile +from django.urls import reverse + +from efile.models import FilingDocument +from efile.services.document_previews import preview_fingerprint +from efile.tests import test_filing_plan_actions as plan_actions +from efile.tests.pdf_helpers import pdf_bytes + +CHECKLIST_URL = plan_actions.CHECKLIST_URL +user = plan_actions.user +draft = plan_actions.draft +signed_in = plan_actions.signed_in + +pytestmark = pytest.mark.django_db + +CHECKS_URL = reverse("document_checks", kwargs={"jurisdiction": "illinois"}) + + +def new_file(draft, **fields): + """A file as the upload leaves it: prepared, but its copy not yet confirmed.""" + return FilingDocument.objects.create( + draft=draft, + role=FilingDocument.Role.SUPPORTING, + sort_order=1, + name="fee-waiver.pdf", + preparation="unchanged", + **fields, + ) + + +def confirm(client, draft, document): + document.refresh_from_db() + return client.post( + f"{CHECKS_URL}?draft={draft.pk}", + {"action": "confirm", "document_id": document.pk, "preview_fingerprint": preview_fingerprint([document])}, + ) + + +def test_uploading_for_an_item_lands_on_its_check(client, signed_in): + def upload(draft, files, jurisdiction, **kwargs): + new_file(draft) + + with patch("efile.views.document_checklist.upload_files", side_effect=upload): + response = client.post( + CHECKLIST_URL, + {"action": "attach_item", "item_id": "fee_waiver", "document": SimpleUploadedFile("w.pdf", pdf_bytes())}, + ) + assert response.status_code == 302 + assert response.url.endswith("#document-checks") + added = signed_in.documents.get(role=FilingDocument.Role.SUPPORTING) + page = client.get(CHECKLIST_URL) + content = page.content.decode() + assert f'data-document-id="{added.pk}"' in content + assert "This copy looks right" in content + assert 'aria-describedby="checks-pending"' in content + # The confirmed lead is listed as added; the new copy is only in its check. + assert [doc.pk for doc in page.context["checked_documents"]] == [ + signed_in.documents.get(role=FilingDocument.Role.LEAD).pk + ] + + +def test_continue_waits_for_new_files_to_be_checked(client, signed_in): + added = new_file(signed_in, filing_type_code="78690", document_type_code="public") + blocked = client.post(CHECKLIST_URL, {"documents_complete": "yes"}) + assert blocked.url.partition("#")[0].partition("?")[0] == CHECKLIST_URL + assert blocked.url.endswith("#document-checks") + assert confirm(client, signed_in, added).status_code == 200 + page = client.get(CHECKLIST_URL).content.decode() + assert "checks-pending" not in page + moved_on = client.post(CHECKLIST_URL, {"documents_complete": "yes"}) + assert "organize-documents" in moved_on.url + + +def test_a_file_added_on_the_way_back_from_review_needs_no_second_preview(client, signed_in): + added = new_file(signed_in, filing_type_code="78690", document_type_code="public") + assert confirm(client, signed_in, added).status_code == 200 + signed_in.selected_payment_account_id = "account" + signed_in.save() + response = client.post( + f"{CHECKLIST_URL}?return_to=review", + {"documents_complete": "yes", "return_to": "review", "status_petition": "have"}, + ) + assert response.url.partition("?")[0] == reverse("case_review", kwargs={"jurisdiction": "illinois"}) + with patch("efile.views.review.get_case_questions", return_value=[]): + review = client.get(response.url) + # Review used to bounce here to the preview step and back. + assert review.status_code == 200 diff --git a/efile_app/efile/tests/test_document_checks.py b/efile_app/efile/tests/test_document_checks.py new file mode 100644 index 00000000..9db181a9 --- /dev/null +++ b/efile_app/efile/tests/test_document_checks.py @@ -0,0 +1,95 @@ +"""Confirming or removing a newly prepared copy on the page it was added to.""" + +from unittest.mock import MagicMock, patch + +import pytest +from django.test import Client +from django.urls import reverse + +from efile.models import FilingDocument, FilingDraft +from efile.services.document_previews import preview_fingerprint +from efile.services.fee_quotes import fee_inputs_token +from efile.tests.test_review_submit_flow import submission_draft as _submission_draft + +draft = _submission_draft + +pytestmark = pytest.mark.django_db + + +def checks(draft): + return reverse("document_checks", kwargs={"jurisdiction": draft.jurisdiction}) + f"?draft={draft.pk}" + + +@pytest.fixture +def added(draft): + return FilingDocument.objects.create( + draft=draft, + role=FilingDocument.Role.SUPPORTING, + sort_order=1, + name="exhibit.pdf", + s3_key="supporting/exhibit.pdf", + preparation="unchanged", + ) + + +def confirm(document, **changes): + document.refresh_from_db() + return { + "action": "confirm", + "document_id": document.pk, + "preview_fingerprint": preview_fingerprint([document]), + **changes, + } + + +def test_confirming_marks_only_that_copy_checked(client, draft, added): + response = client.post(checks(draft), confirm(added)) + assert response.status_code == 200 + added.refresh_from_db() + assert added.preparation_reviewed_at is not None + + +def test_a_changed_unprepared_or_foreign_copy_is_not_confirmed(client, draft, added): + assert client.post(checks(draft), confirm(added, preview_fingerprint="old")).status_code == 409 + FilingDocument.objects.filter(pk=added.pk).update(preparation="") + assert client.post(checks(draft), confirm(added)).status_code == 409 + other = FilingDraft.objects.create(user=draft.user, jurisdiction="illinois") + foreign = FilingDocument.objects.create(draft=other, role="lead", name="x.pdf", preparation="unchanged") + assert client.post(checks(draft), confirm(foreign)).status_code == 409 + added.refresh_from_db() + foreign.refresh_from_db() + assert added.preparation_reviewed_at is None + assert foreign.preparation_reviewed_at is None + + +def test_removing_takes_out_a_supporting_file_and_reprices(client, draft, added, django_capture_on_commit_callbacks): + lead = draft.documents.get(role=FilingDocument.Role.LEAD) + draft.quoted_fee_total = "100" + draft.save() + remove = {"action": "remove"} + assert client.post(checks(draft), remove | {"document_id": lead.pk}).status_code == 400 + storage = MagicMock() + storage.delete_file.return_value = {"success": True} + with ( + patch("efile.utils.s3_upload_handler.S3UploadHandler", return_value=storage), + django_capture_on_commit_callbacks(execute=True), + ): + response = client.post(checks(draft), remove | {"document_id": added.pk}) + assert response.status_code == 200 + assert list(draft.documents.all()) == [lead] + draft.refresh_from_db() + assert draft.quoted_fee_total == "" + assert response.json()["fee_inputs_token"] == fee_inputs_token(draft) + storage.delete_file.assert_called_with("supporting/exhibit.pdf") + + +def test_checks_need_a_signed_in_named_filing_and_csrf(client, draft, added): + unscoped = reverse("document_checks", kwargs={"jurisdiction": "illinois"}) + assert Client().post(unscoped, confirm(added)).status_code == 401 + # Someone else's draft is refused before the view, by the draft scope. + assert Client().post(checks(draft), confirm(added)).status_code == 409 + assert client.post(unscoped, confirm(added)).status_code == 409 + assert client.get(checks(draft)).status_code == 405 + protected = Client(enforce_csrf_checks=True) + protected.force_login(draft.user) + assert protected.post(checks(draft), confirm(added)).status_code == 403 diff --git a/efile_app/efile/tests/test_payment_flow_browser.py b/efile_app/efile/tests/test_payment_flow_browser.py index 139d66b0..56551248 100644 --- a/efile_app/efile/tests/test_payment_flow_browser.py +++ b/efile_app/efile/tests/test_payment_flow_browser.py @@ -83,6 +83,7 @@ def test_fees_screen_flows_in_browser(live_server, client, django_user_model, tm objects = {} first = payment_ready_draft(user, objects) second = payment_ready_draft(user, objects) + third = payment_ready_draft(user, objects) authorize(client, first) handler = MagicMock() handler.bucket_name = "synthetic" @@ -127,6 +128,8 @@ def delete(key): "secondPaymentUrl": f"{payment}?draft={second.pk}", "previewUrl": reverse("preview_documents", kwargs={"jurisdiction": "illinois"}) + f"?draft={first.pk}", "uploadUrl": reverse("upload_documents", kwargs={"jurisdiction": "illinois"}) + f"?draft={first.pk}", + "checklistUrl": reverse("document_checklist", kwargs={"jurisdiction": "illinois"}) + + f"?draft={third.pk}", **paths, } ) @@ -141,6 +144,7 @@ def delete(key): patch("efile.views.payment.estimate_fees", return_value={}), patch("efile.views.payment.payment_accounts", return_value=ACCOUNTS), patch("efile.views.review.get_case_questions", return_value=[]), + patch("efile.views.document_checklist.draft_unavailable_message", return_value=""), ): result = subprocess.run( ["node", "tests/payment-flow-browser.js", str(config)], @@ -157,4 +161,14 @@ def delete(key): assert not first.documents.filter(preparation_reviewed_at__isnull=True).exists() # The wrong file and the copy removed from another tab are gone, with their bytes. assert [doc.name for doc in second.documents.order_by("role", "sort_order")] == ["Petition.pdf", "waiver.pdf"] - assert sorted(key.split("/")[0] for key in objects) == ["lead", "lead", "supporting", "supporting"] + # The checklist kept the file it checked and dropped the one it removed. + assert [doc.name for doc in third.documents.order_by("role", "sort_order")] == ["Petition.pdf", "waiver.pdf"] + assert not third.documents.filter(preparation_reviewed_at__isnull=True).exists() + assert sorted(key.split("/")[0] for key in objects) == [ + "document", + "lead", + "lead", + "lead", + "supporting", + "supporting", + ] diff --git a/efile_app/efile/tests/test_waiver_documents.py b/efile_app/efile/tests/test_waiver_documents.py index f6348d53..e09c8759 100644 --- a/efile_app/efile/tests/test_waiver_documents.py +++ b/efile_app/efile/tests/test_waiver_documents.py @@ -17,6 +17,10 @@ pytestmark = pytest.mark.django_db +def checks(draft): + return reverse("document_checks", kwargs={"jurisdiction": draft.jurisdiction}) + f"?draft={draft.pk}" + + def endpoint(draft): return reverse("waiver_documents", kwargs={"jurisdiction": draft.jurisdiction}) + f"?draft={draft.pk}" @@ -241,7 +245,8 @@ def test_payment_upload_stays_with_displayed_draft_through_review(client, paymen assert "/payment/" in result.url added = payment_draft.documents.get(role=FilingDocument.Role.SUPPORTING) assert f'data-document-id="{added.pk}"' in uploaded.json()["check_html"] - approved = client.post(upload_url, confirm_data(added)) + assert reverse("document_checks", kwargs={"jurisdiction": "illinois"}) in page.context["draft_scope"]["paths"] + approved = client.post(checks(payment_draft), confirm_data(added)) assert approved.status_code == 200 with patch( "efile.views.payment.payment_accounts", @@ -338,51 +343,12 @@ def test_upload_is_checked_on_fees_without_leaving_it(client, payment_draft, sto # A reload before confirming still shows the check on Fees, open. assert page.context["document_checks"][0]["document"].pk == added.pk assert f'data-document-id="{added.pk}"'.encode() in page.content - assert client.post(endpoint(payment_draft), confirm_data(added)).status_code == 200 + assert client.post(checks(payment_draft), confirm_data(added)).status_code == 200 added.refresh_from_db() assert added.preparation_reviewed_at is not None assert payment_draft.documents.get(role=FilingDocument.Role.LEAD).preparation_reviewed_at is not None -def test_confirm_rejects_a_changed_copy_and_foreign_documents(client, payment_draft, storage): - from efile.models import FilingDraft - - _, added = upload_waiver(client, payment_draft) - stale = confirm_data(added) | {"preview_fingerprint": "old"} - assert client.post(endpoint(payment_draft), stale).status_code == 409 - other = FilingDraft.objects.create(user=payment_draft.user, jurisdiction="illinois") - foreign = FilingDocument.objects.create(draft=other, role="lead", name="x.pdf", preparation="unchanged") - assert client.post(endpoint(payment_draft), confirm_data(foreign)).status_code == 409 - added.refresh_from_db() - foreign.refresh_from_db() - assert added.preparation_reviewed_at is None - assert foreign.preparation_reviewed_at is None - - -def test_remove_only_takes_out_a_waiver_and_reprices( - client, payment_draft, storage, django_capture_on_commit_callbacks -): - _, added = upload_waiver(client, payment_draft) - lead = payment_draft.documents.get(role=FilingDocument.Role.LEAD) - payment_draft.refresh_from_db() - remove = {"action": "remove", "fee_inputs_token": fee_inputs_token(payment_draft)} - assert client.post(endpoint(payment_draft), remove | {"document_id": lead.pk}).status_code == 400 - assert ( - client.post(endpoint(payment_draft), remove | {"document_id": added.pk, "fee_inputs_token": "x"}).status_code - == 409 - ) - with ( - patch("efile.utils.s3_upload_handler.S3UploadHandler", return_value=storage), - django_capture_on_commit_callbacks(execute=True), - ): - response = client.post(endpoint(payment_draft), remove | {"document_id": added.pk}) - assert response.status_code == 200 - assert list(payment_draft.documents.all()) == [lead] - payment_draft.refresh_from_db() - assert response.json()["fee_inputs_token"] == fee_inputs_token(payment_draft) - storage.delete_file.assert_called_with("waivers/test.pdf") - - def test_payment_waits_for_unchecked_documents(client, payment_draft, storage): upload_waiver(client, payment_draft) payment_url = reverse("payment", kwargs={"jurisdiction": "illinois"}) + f"?draft={payment_draft.pk}" diff --git a/efile_app/efile/urls.py b/efile_app/efile/urls.py index 38af9f1c..03407c59 100644 --- a/efile_app/efile/urls.py +++ b/efile_app/efile/urls.py @@ -13,6 +13,7 @@ from .views.choose_jurisdiction import change_jurisdiction, choose_jurisdiction from .views.confirmation import filing_confirmation from .views.document_checklist import document_checklist +from .views.document_checks import document_checks from .views.document_previews import document_content, preview_documents from .views.draft_views import get_current_draft_view, start_filing, start_filing_from_plan from .views.extraction_review import extraction_review @@ -94,6 +95,7 @@ def jurisdiction_homepage(request, jurisdiction): path("jurisdiction//options/", efile_options, name="efile_options"), path("jurisdiction//filing-path/", filing_path, name="filing_path"), path("jurisdiction//waiver-documents/", waiver_documents, name="waiver_documents"), + path("jurisdiction//document-checks/", document_checks, name="document_checks"), path("jurisdiction//upload-documents/", upload_documents, name="upload_documents"), path("jurisdiction//preview-documents/", preview_documents, name="preview_documents"), path("jurisdiction//documents//content/", document_content, name="document_content"), diff --git a/efile_app/efile/views/document_checklist.py b/efile_app/efile/views/document_checklist.py index 82c1fed2..20199cd4 100644 --- a/efile_app/efile/views/document_checklist.py +++ b/efile_app/efile/views/document_checklist.py @@ -1,12 +1,15 @@ from django.contrib import messages from django.http import JsonResponse from django.shortcuts import redirect, render +from django.urls import reverse from django.views.decorators.http import require_http_methods from efile.api.suffolk_api_views import get_tyler_token from efile.models import FilingDocument from efile.services.current_drafts import ensure_current_draft +from efile.services.document_previews import unreviewed_documents from efile.services.document_uploads import upload_files +from efile.services.draft_urls import draft_url from efile.services.drafts import draft_snapshot from efile.services.filing_availability import draft_unavailable_message, unavailable_response from efile.services.filing_plans import ( @@ -26,6 +29,7 @@ set_filer_role, status_choices, ) +from efile.views.document_checks import unchecked_documents from efile.workflow import ( WorkflowStepKey, continue_step, @@ -81,7 +85,9 @@ def _attach_to_item(request, draft, plan, jurisdiction): messages.error(request, str(error)) return redirect(_this_page(request, jurisdiction)) document = _newest_document(draft) + uploaded = True else: + uploaded = False document = FilingDocument.objects.filter(draft=draft, pk=document_id).first() if document_id else None if document is None: @@ -101,7 +107,8 @@ def _attach_to_item(request, draft, plan, jurisdiction): document.save(update_fields=["filing_type_code", "filing_type_name", "updated_at"]) messages.success(request, f"{label} is in this filing.") - return redirect(_this_page(request, jurisdiction)) + # A new file is checked right here, before the filer moves on. + return redirect(_this_page(request, jurisdiction) + ("#document-checks" if uploaded else "")) @require_http_methods(["GET", "POST"]) @@ -168,6 +175,12 @@ def document_checklist(request, jurisdiction): if action == "save_progress": messages.success(request, "We saved your document list.") return redirect(_this_page(request, jurisdiction)) + # Files added here are checked here. The page keeps Continue off until + # they are; this covers a stale tab, so no later screen sends the filer + # to a preview and back. + if unreviewed_documents(draft).exists(): + messages.error(request, "Check each new file before you continue.") + return redirect(_this_page(request, jurisdiction) + "#document-checks") # The checklist is a guide, not a gate: the filer can continue with any # item unticked, and is never asked to say the list is complete. It # cannot know which forms a filer's situation actually needs. @@ -186,6 +199,9 @@ def document_checklist(request, jurisdiction): "is_logged_in": True, "filing_draft": draft_snapshot(draft), "documents": documents, + "checked_documents": [document for document in documents if document.preparation_reviewed_at is not None], + "document_checks": unchecked_documents(documents), + "document_checks_url": draft_url(reverse("document_checks", kwargs={"jurisdiction": jurisdiction}), draft.pk), "plan": plan, "filer_roles": filer_roles, "filer_role": draft.filer_role, diff --git a/efile_app/efile/views/document_checks.py b/efile_app/efile/views/document_checks.py new file mode 100644 index 00000000..c1b88be6 --- /dev/null +++ b/efile_app/efile/views/document_checks.py @@ -0,0 +1,85 @@ +"""Check a newly prepared copy where it was added, instead of on another screen. + +A file added after the preview step (a fee waiver on Fees, a missing document +on the checklist) is shown open on that same page. The filer confirms the +copy the court will get, or removes it, without losing their place. +""" + +from django.db import transaction +from django.http import JsonResponse +from django.template.loader import render_to_string +from django.utils import timezone +from django.views.decorators.http import require_http_methods + +from efile.api.suffolk_api_views import get_tyler_token +from efile.models import FilingDocument, FilingDraft +from efile.services.current_drafts import explicit_draft_id, get_current_draft +from efile.services.document_preparation import cleanup_unreferenced_uploads +from efile.services.document_previews import document_storage_keys, preview_fingerprint +from efile.services.drafts import ACTIVE_DRAFT_STATUSES +from efile.services.fee_quotes import fee_inputs_token, invalidate_fee_quote + + +def is_removable(document): + """Supporting files can go from here. The main document is changed on the + upload step, where a replacement is chosen and read again.""" + return document.role == FilingDocument.Role.SUPPORTING + + +def document_check(document): + return { + "document": document, + "fingerprint": preview_fingerprint([document]), + "removable": is_removable(document), + } + + +def unchecked_documents(documents): + """Template context for each document whose copy has not been confirmed.""" + return [document_check(document) for document in documents if document.preparation_reviewed_at is None] + + +def document_check_html(request, document): + return render_to_string( + "efile/components/document_check.html", + {**document_check(document), "jurisdiction": document.draft.jurisdiction}, + request=request, + ) + + +@require_http_methods(["POST"]) +def document_checks(request, jurisdiction): + if not request.user.is_authenticated or not get_tyler_token(request, jurisdiction): + return JsonResponse({"error": "Sign in again to continue."}, status=401) + if explicit_draft_id(request) is None: + return JsonResponse({"error": "Reload this page before checking a document."}, status=409) + draft = get_current_draft(request, jurisdiction=jurisdiction) + action = request.POST.get("action", "") + if draft is None or action not in ("confirm", "remove"): + return JsonResponse({"error": "This filing is not available to edit."}, status=409) + with transaction.atomic(): + draft = FilingDraft.objects.select_for_update().get(pk=draft.pk) + if draft.status not in ACTIVE_DRAFT_STATUSES: + return JsonResponse({"error": "This filing is not available to edit."}, status=409) + document = draft.documents.filter(pk=request.POST.get("document_id") or None).first() + if document is None: + return JsonResponse( + {"error": "This document is no longer part of your filing. Reload this page."}, status=409 + ) + if action == "confirm": + if not document.preparation: + return JsonResponse({"error": "This file is not ready. Remove it and upload it again."}, status=409) + if request.POST.get("preview_fingerprint") != preview_fingerprint([document]): + return JsonResponse({"error": "This file changed. Reload this page and check it again."}, status=409) + document.preparation_reviewed_at = timezone.now() + document.save(update_fields=["preparation_reviewed_at", "updated_at"]) + return JsonResponse({"success": True, "fee_inputs_token": fee_inputs_token(draft)}) + if not is_removable(document): + return JsonResponse({"error": "Change your main document from the upload step."}, status=400) + keys = document_storage_keys(document) + document.delete() + # Fewer documents can mean different fees. The page gets the new + # inputs token so a quote it asks for next describes this filing. + invalidate_fee_quote(draft) + transaction.on_commit(lambda: cleanup_unreferenced_uploads(keys)) + return JsonResponse({"success": True, "fee_inputs_token": fee_inputs_token(draft)}) diff --git a/efile_app/efile/views/payment.py b/efile_app/efile/views/payment.py index efe10e28..58f6d80b 100644 --- a/efile_app/efile/views/payment.py +++ b/efile_app/efile/views/payment.py @@ -9,7 +9,6 @@ from efile.models import FilingDocument, FilingParty from efile.services.appeals import appeal_answers_complete from efile.services.current_drafts import ensure_current_draft -from efile.services.document_previews import preview_fingerprint from efile.services.draft_urls import draft_url from efile.services.drafts import draft_snapshot, read_case_data from efile.services.fee_estimates import estimate_fees @@ -18,7 +17,7 @@ from efile.services.people import filing_parties from efile.services.waiver_documents import has_waiver_document from efile.utils.config_loader import config_loader -from efile.views.waiver_documents import is_removable_waiver +from efile.views.document_checks import unchecked_documents from ..workflow import ( WorkflowStepKey, @@ -100,14 +99,8 @@ def efile_payment(request, jurisdiction): context = { "documents": [document for document in documents if document.preparation_reviewed_at is not None], - "document_checks": [ - { - "document": document, - "fingerprint": preview_fingerprint([document]), - "removable": is_removable_waiver(document), - } - for document in unchecked - ], + "document_checks": unchecked_documents(documents), + "document_checks_url": draft_url(reverse("document_checks", kwargs={"jurisdiction": jurisdiction}), draft.pk), "waiver_upload_url": draft_url(reverse("waiver_documents", kwargs={"jurisdiction": jurisdiction}), draft.pk), "has_waiver_document": has_waiver_document(draft), "is_logged_in": True, diff --git a/efile_app/efile/views/waiver_documents.py b/efile_app/efile/views/waiver_documents.py index 59c94803..43a112a4 100644 --- a/efile_app/efile/views/waiver_documents.py +++ b/efile_app/efile/views/waiver_documents.py @@ -1,70 +1,17 @@ from django.db import transaction from django.db.models import Max from django.http import JsonResponse -from django.template.loader import render_to_string -from django.utils import timezone from django.views.decorators.http import require_http_methods from efile.api.suffolk_api_views import get_tyler_token from efile.models import FilingDocument, FilingDraft from efile.services.current_drafts import explicit_draft_id, get_current_draft -from efile.services.document_preparation import cleanup_unreferenced_uploads, cleanup_uploads, store_prepared_document -from efile.services.document_previews import document_storage_keys, preview_fingerprint +from efile.services.document_preparation import cleanup_uploads, store_prepared_document from efile.services.drafts import ACTIVE_DRAFT_STATUSES from efile.services.fee_quotes import fee_inputs_token, invalidate_fee_quote -from efile.services.waiver_documents import WAIVER_TYPE, waiver_document_choices, waiver_filing_types +from efile.services.waiver_documents import waiver_document_choices, waiver_filing_types from efile.utils.s3_upload_handler import S3UploadHandler - - -def document_check_html(request, document): - """The fees page checks a newly added copy in place, not on another screen.""" - return render_to_string( - "efile/components/document_check.html", - { - "document": document, - "jurisdiction": document.draft.jurisdiction, - "fingerprint": preview_fingerprint([document]), - "removable": is_removable_waiver(document), - }, - request=request, - ) - - -def is_removable_waiver(document): - return document.role == FilingDocument.Role.SUPPORTING and bool(WAIVER_TYPE.search(document.filing_type_name)) - - -def _check_document(request, draft, action): - """Confirm or remove one copy shown on the fees page.""" - data = request.POST - with transaction.atomic(): - draft = FilingDraft.objects.select_for_update().get(pk=draft.pk) - if draft.status not in ACTIVE_DRAFT_STATUSES: - return JsonResponse({"error": "This filing is not available to edit."}, status=409) - document = draft.documents.filter(pk=data.get("document_id") or None).first() - if document is None: - return JsonResponse( - {"error": "This document is no longer part of your filing. Reload this page."}, status=409 - ) - if action == "confirm": - if not document.preparation: - return JsonResponse({"error": "This file is not ready. Remove it and upload it again."}, status=409) - if data.get("preview_fingerprint") != preview_fingerprint([document]): - return JsonResponse({"error": "This file changed. Reload this page and check it again."}, status=409) - document.preparation_reviewed_at = timezone.now() - document.save(update_fields=["preparation_reviewed_at", "updated_at"]) - return JsonResponse({"success": True, "fee_inputs_token": fee_inputs_token(draft)}) - if not is_removable_waiver(document): - return JsonResponse({"error": "Change this document from the upload step."}, status=400) - if data.get("fee_inputs_token") != fee_inputs_token(draft): - return JsonResponse( - {"error": "This filing changed. Reload this page before removing a document."}, status=409 - ) - keys = document_storage_keys(document) - document.delete() - invalidate_fee_quote(draft) - transaction.on_commit(lambda: cleanup_unreferenced_uploads(keys)) - return JsonResponse({"success": True, "fee_inputs_token": fee_inputs_token(draft)}) +from efile.views.document_checks import document_check_html @require_http_methods(["GET", "POST"]) @@ -76,9 +23,6 @@ def waiver_documents(request, jurisdiction): draft = get_current_draft(request, jurisdiction=jurisdiction) if draft is None or draft.status not in ACTIVE_DRAFT_STATUSES or not draft.court_code: return JsonResponse({"error": "This filing is not available to edit."}, status=409) - action = request.POST.get("action", "") if request.method == "POST" else "" - if action in ("confirm", "remove"): - return _check_document(request, draft, action) keys = [] handler = S3UploadHandler() try: diff --git a/efile_app/js-tests/document-checks.test.js b/efile_app/js-tests/document-checks.test.js new file mode 100644 index 00000000..b7ba7d45 --- /dev/null +++ b/efile_app/js-tests/document-checks.test.js @@ -0,0 +1,144 @@ +const test = require("node:test"); +const assert = require("node:assert/strict"); +const fs = require("node:fs"); +const vm = require("node:vm"); +const path = require("node:path"); + +function harness(fetch, action = "confirm") { + const events = []; + const status = { + textContent: "" + }; + const buttons = [{ + disabled: false + }, { + disabled: false + }]; + const check = { + removed: false, + dataset: { + documentId: "7", + fingerprint: "abc" + }, + querySelector: () => status, + querySelectorAll: () => buttons, + remove() { + this.removed = true; + } + }; + const container = { + dataset: { + url: "/document-checks/" + }, + dispatchEvent(event) { + events.push(event); + } + }; + const button = { + hasAttribute: (name) => name === "data-document-remove" && action === "remove", + closest: (selector) => selector === "[data-document-checks]" ? container : check + }; + let click; + const context = vm.createContext({ + document: { + addEventListener(event, callback) { + if (event === "click") click = callback; + } + }, + window: { + withFilingDraft: (url) => `${url}?draft=42` + }, + CustomEvent: class { + constructor(type, options) { + this.type = type; + this.detail = options.detail; + } + }, + FormData, + fetch, + gettext: (text) => text, + apiUtils: { + getCSRFToken: () => "csrf" + } + }); + // Evaluates only the checked-in browser script. + // eslint-disable-next-line sonarjs/code-eval + vm.runInContext(fs.readFileSync(path.join(__dirname, "../efile/static/js/document-checks.js"), "utf8"), context); + return { + context, + click: () => context.window.DocumentChecks.submit(button), + hasClick: () => typeof click === "function", + check, + status, + buttons, + events + }; +} + +test("confirming posts the copy's fingerprint and tells the page", async () => { + let request; + const page = harness(async (url, options) => { + request = { + url, + body: options.body + }; + return { + ok: true, + json: async () => ({ + success: true, + fee_inputs_token: "new" + }) + }; + }); + assert.equal(page.hasClick(), true); + await page.click(); + assert.equal(request.url, "/document-checks/?draft=42"); + assert.equal(request.body.get("action"), "confirm"); + assert.equal(request.body.get("document_id"), "7"); + assert.equal(request.body.get("preview_fingerprint"), "abc"); + assert.equal(page.check.removed, true); + assert.equal(page.events.length, 1); + assert.equal(page.events[0].type, "document-checks:change"); + assert.equal(page.events[0].detail.action, "confirm"); + assert.equal(page.events[0].detail.data.fee_inputs_token, "new"); +}); + +test("removing sends the remove action", async () => { + let action; + const page = harness(async (_url, options) => { + action = options.body.get("action"); + return { + ok: true, + json: async () => ({ + success: true + }) + }; + }, "remove"); + await page.click(); + assert.equal(action, "remove"); + assert.equal(page.events[0].detail.action, "remove"); +}); + +test("a refused check stays on the page with its reason and can be retried", async () => { + const page = harness(async () => ({ + ok: false, + json: async () => ({ + error: "This file changed. Reload this page and check it again." + }) + })); + await page.click(); + assert.equal(page.check.removed, false); + assert.equal(page.events.length, 0); + assert.match(page.status.textContent, /This file changed/); + assert.deepEqual(page.buttons.map((button) => button.disabled), [false, false]); +}); + +test("pending finds any check still on the page", () => { + const page = harness(async () => ({})); + assert.equal(page.context.window.DocumentChecks.pending({ + querySelector: () => ({}) + }), true); + assert.equal(page.context.window.DocumentChecks.pending({ + querySelector: () => null + }), false); +}); \ No newline at end of file diff --git a/efile_app/js-tests/payment.test.js b/efile_app/js-tests/payment.test.js index f3ae950e..7c86ca19 100644 --- a/efile_app/js-tests/payment.test.js +++ b/efile_app/js-tests/payment.test.js @@ -4,7 +4,9 @@ const fs = require("node:fs"); const vm = require("node:vm"); const path = require("node:path"); -function pageHarness(post) { +function pageHarness(post, checks = { + pending: () => false +}) { const nodes = new Map(); const node = (id) => { if (!nodes.has(id)) nodes.set(id, { @@ -43,6 +45,7 @@ function pageHarness(post) { URLSearchParams, gettext: (text) => text, FilingPayload: {}, + DocumentChecks: checks, apiUtils: { post, getCurrentJurisdiction: () => "illinois", @@ -332,4 +335,19 @@ test("failed removal keeps the existing payment selection usable and permits ret assert.equal(payment.feeQuoteReady, true); assert.equal(payment.quoteRequestId, 7); assert.equal(node("submitButton").disabled, false); +}); +test("a copy still waiting to be checked keeps review disabled", async () => { + let pending = true; + const { + payment, + node + } = pageHarness(async () => waiver, { + pending: () => pending + }); + await payment.chooseIntent(); + assert.equal(node("selected-payment-account").value, "waiver-1"); + assert.equal(node("submitButton").disabled, true); + pending = false; + payment.setFeesState(false); + assert.equal(node("submitButton").disabled, false); }); \ No newline at end of file diff --git a/efile_app/js-tests/waiver-upload.test.js b/efile_app/js-tests/waiver-upload.test.js index fb9a37ae..db4d2ada 100644 --- a/efile_app/js-tests/waiver-upload.test.js +++ b/efile_app/js-tests/waiver-upload.test.js @@ -36,9 +36,6 @@ function harness(fetch) { }; const payment = { setFeesState() {}, - addDocumentCheck(html) { - this.addedCheck = html; - }, async chooseIntent() { this.refreshed = true; } @@ -62,6 +59,14 @@ function harness(fetch) { fetch, gettext: (text) => text, PaymentPage: payment, + DocumentChecks: { + add(container, html) { + payment.addedCheck = { + container, + html + }; + } + }, paymentJSON: () => "old-token", apiUtils: { getCSRFToken: () => "csrf" @@ -136,8 +141,9 @@ test("upload stays on payment and replaces the stale fee token", async () => { assert.equal(node("waiver-upload-confirmation").hidden, false); assert.equal(payment.refreshed, true); assert.equal(payment.waiverUploading, false); - // The new copy is checked right here instead of on another page. - assert.equal(payment.addedCheck, "
"); + // The new copy is checked right here, in the Fees page's own list. + assert.equal(payment.addedCheck.container, node("document-checks")); + assert.equal(payment.addedCheck.html, "
"); }); test("failed uploads keep the picker and file available for retry", async () => { diff --git a/efile_app/tests/payment-flow-browser.js b/efile_app/tests/payment-flow-browser.js index e2b331ca..0594ff5a 100644 --- a/efile_app/tests/payment-flow-browser.js +++ b/efile_app/tests/payment-flow-browser.js @@ -278,6 +278,37 @@ async function main() { await screenshot("browser-failure-3.png"); throw error; } + try { + // 9. A missing document added on the checklist is checked there. + const checklist = config.baseUrl + config.checklistUrl; + const addMissing = async (file) => { + await page.goto(checklist); + await page.locator(".add-missing-toggle").click(); + await page.locator("#checklist-documents").setInputFiles(file); + await Promise.all([page.waitForEvent("load"), page.locator("#checklist-upload-form button[type=submit]").click()]); + await checks.first().waitFor(); + }; + const proceed = page.locator("#checklist-confirm-form button[type=submit]"); + visited.length = 0; + await addMissing(config.wrongFile); + assert.ok(page.url().endsWith("#document-checks"), page.url()); + assert.equal(await proceed.isDisabled(), true, "Continue must wait for the new file's check"); + await waitRendered(checks.first()); + await screenshot("07-checklist-check.png"); + await Promise.all([page.waitForEvent("load"), checks.first().locator("[data-document-remove]").click()]); + assert.equal(await checks.count(), 0); + assert.equal(await proceed.isEnabled(), true); + await addMissing(config.waiverFile); + await Promise.all([page.waitForEvent("load"), checks.first().locator("[data-document-confirm]").click()]); + assert.equal(await checks.count(), 0); + assert.match(await page.locator(".checklist-files").innerText(), /waiver\.pdf/); + await Promise.all([page.waitForURL(/organize-documents/), proceed.click()]); + assert.ok(!visited.some((url) => url.includes("preview-documents")), `visited ${visited}`); + console.log("Scenario 9 passed"); + } catch (error) { + await screenshot("browser-failure-4.png"); + throw error; + } assert.deepEqual(errors, []); console.log("Fees browser validation passed."); await browser.close();