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/services/draft_urls.py b/efile_app/efile/services/draft_urls.py index 215fc203..05290656 100644 --- a/efile_app/efile/services/draft_urls.py +++ b/efile_app/efile/services/draft_urls.py @@ -21,6 +21,7 @@ "case_questions", "payment", "waiver_documents", + "document_checks", "case_review", "filing_confirmation", "expert_form", diff --git a/efile_app/efile/static/js/document-checklist.js b/efile_app/efile/static/js/document-checklist.js index fbe6bc3f..749aa07e 100644 --- a/efile_app/efile/static/js/document-checklist.js +++ b/efile_app/efile/static/js/document-checklist.js @@ -1,4 +1,11 @@ (function() { + // New files are checked on this page, so come back to them after a change. + const reloadAtChecks = () => { + window.history.replaceState(window.history.state, "", "#document-checks"); + window.location.reload(); + }; + document.getElementById("document-checks")?.addEventListener("document-checks:change", reloadAtChecks); + const form = document.getElementById("checklist-upload-form"); if (!form) return; const state = document.getElementById("checklist-upload-state"); @@ -24,7 +31,7 @@ } const result = await response.json(); if (!response.ok || !result.success) throw new Error(result.error || "Could not add documents."); - window.location.reload(); + reloadAtChecks(); } catch (error) { state.hidden = true; errorBox.textContent = error.message; diff --git a/efile_app/efile/static/js/document-checks.js b/efile_app/efile/static/js/document-checks.js new file mode 100644 index 00000000..10ecdc04 --- /dev/null +++ b/efile_app/efile/static/js/document-checks.js @@ -0,0 +1,66 @@ +/* Confirm or remove a newly prepared copy without leaving the page it was added on. + * + * Markup: a [data-document-checks] container (data-url: the document-checks + * endpoint) holding [data-document-check] blocks. Each finished action fires + * "document-checks:change" on the container with {action, data}, so the page + * can update whatever depends on its documents. + */ +const DocumentChecks = { + pending(root = document) { + return Boolean(root.querySelector("[data-document-check]")); + }, + + add(container, html) { + 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)); + return check; + }, + + async submit(button) { + const container = button.closest("[data-document-checks]"); + const check = button.closest("[data-document-check]"); + const action = button.hasAttribute("data-document-remove") ? "remove" : "confirm"; + const status = check.querySelector("[data-document-check-status]"); + const buttons = check.querySelectorAll("button"); + const body = new FormData(); + body.append("action", action); + body.append("document_id", check.dataset.documentId); + body.append("preview_fingerprint", check.dataset.fingerprint); + body.append("csrfmiddlewaretoken", apiUtils.getCSRFToken()); + buttons.forEach((element) => { + element.disabled = true; + }); + status.textContent = action === "remove" ? gettext("Removing document…") : gettext("Saving…"); + try { + const response = await fetch(window.withFilingDraft(container.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.")); + check.remove(); + container.dispatchEvent(new CustomEvent("document-checks:change", { + bubbles: true, + detail: { + action, + data + } + })); + } catch (error) { + status.textContent = error.message; + buttons.forEach((element) => { + element.disabled = false; + }); + } + } +}; +window.DocumentChecks = DocumentChecks; + +document.addEventListener("click", (event) => { + const button = event.target.closest("[data-document-checks] [data-document-confirm], [data-document-checks] [data-document-remove]"); + if (button) DocumentChecks.submit(button); +}); \ No newline at end of file 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..722d0925 100644 --- a/efile_app/efile/static/js/payment.js +++ b/efile_app/efile/static/js/payment.js @@ -1,3 +1,4 @@ +/* global DocumentChecks */ const PAYMENT_URLS = { accounts: "/api/payment-accounts/", accountTypes: "/api/payment-account-types/", @@ -57,7 +58,30 @@ 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 || DocumentChecks.pending() || !this.feeQuoteReady || !document.getElementById("selected-payment-account").value; + }, + + // A copy added on this page is confirmed here before Review. + async onDocumentCheck({ + action, + data + }) { + document.getElementById("fee-inputs-token").textContent = JSON.stringify(data.fee_inputs_token); + const confirmation = document.getElementById("waiver-upload-confirmation"); + if (action === "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(); + } + this.setFeesState(false); }, async loadAccountTypes() { @@ -318,6 +342,7 @@ const PaymentPage = { document.querySelectorAll('input[name="paymentIntent"]').forEach((input) => { input.addEventListener("change", () => this.chooseIntent()); }); + document.getElementById("document-checks").addEventListener("document-checks:change", (event) => this.onDocumentCheck(event.detail)); 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..4087c99d 100644 --- a/efile_app/efile/static/js/waiver-upload.js +++ b/efile_app/efile/static/js/waiver-upload.js @@ -1,4 +1,4 @@ -/* global PaymentPage, paymentJSON */ +/* global DocumentChecks, PaymentPage, paymentJSON */ /* Add a supporting waiver PDF without leaving payment or changing the lead. */ document.addEventListener("DOMContentLoaded", () => { const open = document.getElementById("add-waiver-document"); @@ -83,14 +83,15 @@ 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. + DocumentChecks.add(document.getElementById("document-checks"), data.check_html); + PaymentPage.setFeesState(false); confirmation.focus(); // Old requests and quotes described a different set of documents. await PaymentPage.chooseIntent(); 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/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/document_checklist.html b/efile_app/efile/templates/efile/document_checklist.html index 04161040..0f2204cc 100644 --- a/efile_app/efile/templates/efile/document_checklist.html +++ b/efile_app/efile/templates/efile/document_checklist.html @@ -183,8 +183,15 @@

{% translate "Your document plan" %}

{% endfor %} {% endif %}

{% translate "Files you have added" %}

+
+ {% for check in document_checks %} + {% include "efile/components/document_check.html" with document=check.document fingerprint=check.fingerprint removable=check.removable %} + {% endfor %} +
- {% for document in documents %} + {% for document in checked_documents %}
@@ -236,10 +243,17 @@

{% translate "Files you have added" %}

{% csrf_token %} + {% if document_checks %} +

+ {% translate "Check each new file above before you continue." %} +

+ {% endif %}
{% translate "Back" %} -
{% csrf_token %} + @@ -103,39 +104,45 @@

{% 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 %} +
@@ -173,6 +180,7 @@

{% translate "Confirmed fees" %}

{% endblock workflow_content %} {% block extra_js %} + {% endblock extra_js %} 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_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_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, '">