Skip to content

fix(consent): show consent once, and recover from a failed upload - #671

Open
Zeroupper wants to merge 4 commits into
testfrom
feat/consent-status-and-error-handling
Open

fix(consent): show consent once, and recover from a failed upload#671
Zeroupper wants to merge 4 commits into
testfrom
feat/consent-status-and-error-handling

Conversation

@Zeroupper

Copy link
Copy Markdown
Collaborator

The bug

Informed consent was shown twice after accepting it — the user signed, and the document came straight back on top of the app.

The page was pushed with context.push(), and go_router keeps an imperative push in its stored navigation. Configuring the study refreshes the router, the stored location is re-parsed, and the push is replayed. Only context.go() replaces it; popping doesn't clear it.

The fix

CarpAppShell renders the page inline from a ConsentStatus the view model owns, so there is no route left to replay:

  • InformedConsentViewModel owns resolving / needsSigning / given / failed, and resolve() settles it.
  • The shell is a ListenableBuilder over that status: spinner, document, app, or error page.
  • Deploying the study moves to HomePage — the first page shown once consent is in place — so the shell only gates.

This also removes a Future fired from addPostFrameCallback in initState.

Failed uploads

Pulling the network while signing froze the app on the signed document: no error, no way out.

The local "accepted" flag was set before the upload, so a failure was invisible, and carp_webservices retries a POST 15 times with backoff — about 11 minutes of silence. RPUITask also keeps DONE enabled during the upload, so taps stacked more uploads.

Accepting is atomic now:

  • the upload has to succeed before anything is committed locally,
  • capped at 20s, so a dead connection surfaces in seconds rather than minutes,
  • a second DONE is ignored while one is in flight, with the document blocked behind a spinner,
  • on failure the user is told and leaves the study — a signature that never reached CAWS means they are not enrolled, and letting them in would desync the phone from the backend.

Tests

test/informed_consent_page_test.dart — signing swaps consent for the app, and refreshing the router does not bring it back (the original bug).
test/informed_consent_upload_failure_test.dart — a failed upload is recorded once, tells the user, and leaves the study.

Separate files on purpose: research_package's global blocTask carries a finished task between tests in one file.

Not verified on hardware yet — the failure path (kill the network mid-upload → dialog → back to invitations) still wants a device run.

…load

Informed consent was shown twice after accepting it: the page was pushed
with context.push(), and go_router keeps an imperative push in its stored
navigation - so configuring the study, which refreshes the router, replayed
the push and put consent back on top of the app.

The page is now rendered inline by CarpAppShell from a ConsentStatus the
view model owns, so there is no route left to replay. Deploying the study
moves to HomePage, the first page shown once consent is in place.

Accepting is also atomic now. The local "accepted" flag used to be set
before the upload, so a failed upload left the user on a signed document
with no error and nothing to do - carp_webservices retries a POST for
about 11 minutes before surfacing anything. The upload now has to succeed
first, is capped at 20s, cannot be started twice, and on failure the user
is told and leaves the study rather than being stranded.
Consent is the third onboarding precondition, next to authentication and
having a study - but it was the only one not in the redirect, which even
had the gap to show for it: the steps ran 1, 2, 4.

It is step 3 now. A redirect replaces the location rather than pushing it,
so the original bug - a stored push replayed by every router refresh -
cannot happen, and consent stays a real route: no consent gate in the
shell's build, no local Navigator to absorb RPUITask's pops, and the page
is reachable by location like every other page.

Resolving it is a cached Future rather than a hand-maintained status: the
redirect awaits it on every navigation and it resolves once, and
leaveStudy() clears it with the rest of the view models.
@Zeroupper
Zeroupper force-pushed the feat/consent-status-and-error-handling branch 2 times, most recently from e8136b5 to 285bb0c Compare August 31, 2026 10:56
RP 3.2.0 stops RPUITask popping its own route when it is the only one in
the stack - required now that consent is a redirect target. Also fixes
the two consent tests failing on CI against RP 3.0.0.
@Zeroupper
Zeroupper requested review from jakdan99 and a balanced review from Copilot September 1, 2026 08:22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants