WIP durable state models#112
Conversation
|
I looked into the current CI failure. The failing check is the Root cause: Suggested minimal test fix: diff --git a/efile_app/efile/tests/tests.py b/efile_app/efile/tests/tests.py
index 59d68dc..xxxxxxx 100644
--- a/efile_app/efile/tests/tests.py
+++ b/efile_app/efile/tests/tests.py
@@ -389,6 +389,9 @@ class TestExpertFormIntegration:
"""Create an authenticated client."""
user = User.objects.create_user(username="testuser", email="test@example.com", password="testpass123")
client.force_login(user)
+ session = client.session
+ session["auth_tokens"] = {"TYLER-TOKEN-ILLINOIS": "token"}
+ session.save()
return client
def test_expert_form_page_loads(self, authenticated_client):This matches the durable-draft tests' |
BryceStevenWilley
left a comment
BryceStevenWilley
left a comment
There was a problem hiding this comment.
Generally looks good, in the right direction, but a few notes:
- don't need to maintain a "legacy" draft thing at all: can wipe the database when this gets merged so we aren't maintaining 2 different formats
admin.pyisn't really gonna be used, can probably get rid of it- haven't looked super closely, but can probably drop
is_logged_infrom most of the responses, if redirects are already happening on the server side okay
Replace the session-blob store with the FilingDraft aggregate. The legacy_draft_bridge that mirrored session blobs into the model is gone, along with the two-formats problem it created: case_data_utils now (de)serializes the model on every read/write, keeping its request-based signatures so views, templates, the submission path, and the browser wire format are unchanged. The model is fully typed -- extra_case_data and the document/party metadata catch-alls are removed. services/drafts.py holds an explicit wire<->model map that is the entire contract; unknown keys are not persisted, so adding a field a screen needs means adding a typed column and a map entry. Parties are typed FilingParty rows (petitioner, new_name, respondent, other) matching the canonical client-side model. Also cut over every live writer to the model, require auth to persist a draft, finalize the draft on submit via mark_submitted, and drop the dead server-side submission path (transform_case_data_to_filing_payload) plus the unused admin.py and the now-outdated design doc. Migration 0002 is amended in place (unmerged; DB wiped on merge). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The localStorage cache (24h TTL, keyed only by URL) was mirroring per-user and per-draft state: case/upload data, profile, payment accounts, the Tyler auth token, and fee quotes. Now that the model is the source of truth, those stale copies could survive a submit/reset or leak on a shared browser -- and a cached auth token is its own hazard. Route all of them through the non-caching request path so they always hit the server. The cache is now reserved for static, shareable reference data (dropdowns, form config), which is what it's good for. Remove the now-dead cachedPost helper. Add js-tests/api-utils.test.js (node --test, no new deps; kept out of ./tests so Playwright ignores it) to lock in the rule: reference GETs cache, draft/profile/payment reads do not. Run with `npm run test:unit`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The remote added 13 commits (jurisdiction-token gating + step tracking on the flow views, and a submission.py wrapper that mirrors the filing result to the draft). My local work replaced the session-blob store with the durable model. Reconciliation: - Kept the remote's view gating + step tracking as-is (disjoint files). - Kept the remote submission.py wrapper, which adds mark_error on failure, and removed the now-duplicate mark_submitted/clear_current_draft from session_api.submit_final_filing so the wrapper owns draft status. - Rewrote test_durable_drafts.py to the union: the typed serialization tests, plus the remote's token-gate and submission tests ported to the model (their submission setup wrote session case/upload blobs, which the model-based submit no longer reads, so the draft is populated instead). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two review findings: - Existing-case lookup dropped classification codes. The lookup posts case_category_code / case_type_code (and court_code), but the serializer only accepted the bare case_category / case_type forms, silently losing the codes so a resumed existing case had no classification. Accept both the bare and *_code wire forms. - Submission was not idempotent. The wrapper forwarded every request and only marked the draft submitted afterward, so concurrent double-clicks could each fire the external filing API. Claim the draft into SUBMITTING with a single-winner atomic update before forwarding; a losing request gets 409 and never calls the API. A precondition failure releases the claim back to DRAFT so the user can retry. The current-draft pointer lookup now resolves SUBMITTING drafts (a draft mid-submission is still the user's current draft) while resume/listings still exclude them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three review findings: - A submission could stick permanently in SUBMITTING if a request died after claiming but before mark_submitted/mark_error. The claim now also takes over a SUBMITTING draft whose claim is older than SUBMISSION_CLAIM_TIMEOUT (15m), so a crashed submit is recoverable. - Ambiguous post-external-call failures no longer reset the draft to DRAFT. Only failures the submit view raises *before* calling the filing API (missing data, missing efile_data, etc.) release the claim; anything else is recorded as ERROR so a blind retry can't silently double-file. - Draft accessors now enforce the route/payload jurisdiction. get_case_data / get_upload_data / update_* (and the derived getters) take an optional jurisdiction, and the flow views and jurisdiction-aware API writers pass it, so a request for jurisdiction A can no longer read or write a draft pointed to from jurisdiction B. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
BryceStevenWilley
left a comment
BryceStevenWilley
left a comment
There was a problem hiding this comment.
some small concerns, but we can start addressing them after this PR
| def get_case_data(request, jurisdiction=None): | ||
| """Return the current draft serialized to the case_data blob (``{}`` if none). | ||
|
|
||
| Passing ``jurisdiction`` (the route the caller is serving) enforces isolation: | ||
| a draft pointed to from a different jurisdiction is not returned. |
There was a problem hiding this comment.
jurisdiction is another of the poorly designed parts of the litefile that i tried to fix but didn't really do well.
Given how users are currently designed, they are tightly associated with a specific jurisdiction, so IMO it doesn't make much sense trying to support multiple drafts for the same user in different jurisdiction.
There was a problem hiding this comment.
One of the problems I ran into hit a bug with this - part of the stack assumed I was in the nonexistent "default" jurisdiction and saved a draft there. I think the boundary here makes sense.
| # Errors the session submit view returns *before* it calls the external filing | ||
| # API. Only these -- and a confirmed API rejection -- are safe to release back to | ||
| # DRAFT; any other failure may have left a filing submitted. | ||
| _PRE_SUBMIT_ERROR_PREFIXES = ( | ||
| "Submission confirmation is required", | ||
| "No case data found", | ||
| "No upload data found", | ||
| "No efile data provided", | ||
| "Missing required fields in efile_data", | ||
| "Court ID is required", | ||
| ) |
There was a problem hiding this comment.
Reading through this file, not entirely sure what the main goal is, but IMO it would be nice for these strings to just be error codes instead of the specific string returned.
FWIW, the type of error code handling that plocket was recommending for the proxy server is a much better fit for LITEfile.
| is_logged_in = False | ||
| context = { | ||
| "is_logged_in": is_logged_in, | ||
| "is_logged_in": True, |
There was a problem hiding this comment.
Just to make this clear, it looks like is_logged_in should just be excluded entirely from the context, and the templates should be written to assume that the user is logged in.
There was a problem hiding this comment.
I looked into this--we're just using an existing pattern so I think would be a bigger refactor to the downstream template. Fine rolling into this PR but it's already quite big.
…cloudflare stuff still)
# Conflicts: # testing/README.md
Summary
Adds a durable filing-draft foundation for the LITEFile workflow while preserving the existing session-backed behavior for incremental migration and review.
Key changes:
FilingDraftfor one filing workflow instance, including jurisdiction, status, current step, case classification, existing-case identifiers, selected payment account, extracted guesses, optional services, extra case data, and submission response.FilingDocumentfor lead/supporting uploaded documents, including S3/public URL metadata, file metadata, Tyler filing/document/component codes, courtesy copy email, and document order.FilingPartyfor people or organizations associated with a draft, including role, party type, contact fields, name fields, address fields, and metadata.services/drafts.py.services/current_drafts.pyto resolve the current browser's draft while validating authenticated ownership, active status, and jurisdiction. The session stores only the current durable draft ID.services/legacy_draft_bridge.pyas a temporary compatibility adapter that mirrors legacycase_dataandupload_datasession blobs into durable draft/document/party records.POST /jurisdiction/<jurisdiction>/drafts/and a current-draft API atGET /api/draft/.get_workflow_step_choices()so the model'scurrent_stepchoices stay derived from the workflow registry.docs/filing-draft-data-model.mddescribing the model boundaries, compatibility bridge, rollout expectations, and follow-up migration sequence.Behavior and compatibility notes
extra_case_data, documentmetadata, and partymetadataare intentionally kept as temporary escape hatches while the UI moves from arbitrary session blobs to typed draft updates.Migration and deploy notes
efileapp previously had no migrations even though existing environments may already have theUserProfiletable.0001_initial.pytherefore baselines the existingUserProfilemodel.0002_filing_drafts.pycreates the newFilingDraft,FilingDocument, andFilingPartytables, indexes, and ordering constraints.migrate --noinput --fake-initialso existing deployments can fake the baseline migration while fresh databases still apply both migrations normally.Intended follow-up migration sequence
FilingDraft,FilingDocument, andFilingParty.legacy_draft_bridge.py, arbitrary session-merge APIs, and browser storage persistence.Open question
@BryceStevenWilley this is still a rough starting point for discussing migration to a clearer data model and draft system. Do we need backwards compatibility with the existing draft/session system, or are we free to fully discard it? If we can discard it, a fair amount of the compatibility bridge and fallback code here can be removed.