fix(membership): keep the partial-delivery warning on screen #53

Merged

laoc merged 1 commit from fix/membership-partial-delivery-visible into dev

2026-07-30 05:27:28 +00:00

laoc commented

2026-07-29 12:23:13 +00:00

Owner

The partial-delivery warning was set and then destroyed on the same tick, so it never painted a frame. Found by TestOER against merged dev with a relay proxy that answers OK: false to events p-tagged to the second admin.

The bug

MembershipApplicationForm.svelte ends a submit with:

submitted = true;
onsubmitted?.();

partialDelivery renders inside {:else if submitted} — a branch that only becomes reachable on the tick MembershipApplyModal unmounts the form via its onsubmitted handler. TestOER sampled the DOM every 100 ms for 15 s and got zero frames with the warning.

This is the only signal an applicant gets that a reviewer was missed and the review may sit longer than usual. The surface behind the modal cannot carry it — it flips to "waiting for review" off the mirrored event, which a partial fan-out still produces.

The change

onsubmitted now receives the delivery outcome, and the modal stays open on a partial so the applicant can read the warning and dismiss it themselves. A clean submit closes exactly as before.

SignupModal is unaffected: it keeps the form mounted under currentStep === 5, so the warning already rendered there. I confirmed that by reading — handleApplied only switches the footer button, it does not change step.

Why the suite was green

MembershipApplicationForm.test.js:505 renders with no onsubmitted prop, so nothing tore the form down and findByTestId resolved. The test was correct about the state and blind to the lifecycle. New coverage sits at the modal, where the lifecycle actually exists.

Verification

Full suite 5139/5139, 467 files. Lint clean, pnpm check 0 errors.
Negative control: with handleSubmitted reverted to always close, the new "stays open" test fails. It tests the fix.

One thing to know about `pnpm test` on dev

Every run reports Errors: N from an EnvironmentTeardownError in GlobalFAB.test.js — a lazy svelte/reactivity/media-query import landing after teardown. All tests pass; the run still exits non-zero.

I nearly attributed it to this branch. Two baseline runs on clean dev came back with 0 errors, which looked like proof. Sampling five runs each says otherwise:

this branch : 5, 1, 1, 2, 0
clean dev   : 0, 0, 1, 0, 2

Same distribution, and the callstack contains no membership code. It is a pre-existing race on dev, and it means CI on dev is red at random.

The partial-delivery warning was set and then destroyed on the same tick, so it never painted a frame. Found by TestOER against merged `dev` with a relay proxy that answers `OK: false` to events p-tagged to the second admin. ## The bug `MembershipApplicationForm.svelte` ends a submit with: ```js submitted = true; onsubmitted?.(); ``` `partialDelivery` renders inside `{:else if submitted}` — a branch that only becomes reachable on the tick `MembershipApplyModal` unmounts the form via its `onsubmitted` handler. TestOER sampled the DOM every 100 ms for 15 s and got **zero frames** with the warning. This is the only signal an applicant gets that a reviewer was missed and the review may sit longer than usual. The surface behind the modal cannot carry it — it flips to "waiting for review" off the mirrored event, which a partial fan-out still produces. ## The change `onsubmitted` now receives the delivery outcome, and the modal stays open on a partial so the applicant can read the warning and dismiss it themselves. A clean submit closes exactly as before. `SignupModal` is unaffected: it keeps the form mounted under `currentStep === 5`, so the warning already rendered there. I confirmed that by reading — `handleApplied` only switches the footer button, it does not change step. ## Why the suite was green `MembershipApplicationForm.test.js:505` renders with **no `onsubmitted` prop**, so nothing tore the form down and `findByTestId` resolved. The test was correct about the state and blind to the lifecycle. New coverage sits at the modal, where the lifecycle actually exists. ## Verification - Full suite **5139/5139**, 467 files. Lint clean, `pnpm check` **0 errors**. - **Negative control:** with `handleSubmitted` reverted to always close, the new "stays open" test fails. It tests the fix. ## One thing to know about `pnpm test` on dev Every run reports `Errors: N` from an `EnvironmentTeardownError` in `GlobalFAB.test.js` — a lazy `svelte/reactivity/media-query` import landing after teardown. **All tests pass; the run still exits non-zero.** I nearly attributed it to this branch. Two baseline runs on clean `dev` came back with 0 errors, which looked like proof. Sampling five runs each says otherwise: ``` this branch : 5, 1, 1, 2, 0 clean dev : 0, 0, 1, 0, 2 ``` Same distribution, and the callstack contains no membership code. It is a pre-existing race on `dev`, and it means CI on `dev` is red at random.

laoc added 1 commit

2026-07-29 12:23:13 +00:00

fix(membership): keep the partial-delivery warning on screen 8c50194dea

The warning that an application reached only some admins was set and then
destroyed on the same tick. `submitted = true; onsubmitted?.()` runs
back-to-back, and MembershipApplyModal's onsubmitted closed the modal —
so the `{:else if submitted}` branch that renders the warning became
reachable exactly as ModalManager unmounted it. It never painted a frame.

That is the only signal an applicant gets that a reviewer was missed and
the review may sit longer than usual. The surface behind the modal cannot
carry it: it flips to "waiting for review" off the mirrored event, which
a partial fan-out still produces.

onsubmitted now receives the delivery outcome, and the modal stays open
on a partial so the applicant can read the warning and dismiss it
themselves. A clean submit closes as before. SignupModal is unaffected —
it keeps the form mounted under `currentStep === 5`, so the warning
already rendered there.

The suite was green on this because MembershipApplicationForm.test.js
renders with no onsubmitted at all, so nothing tore the form down. The
new coverage is at the modal, where the lifecycle actually exists.

Found by TestOER against dev @ 2f507ff5, using a relay proxy that answers
OK: false to events p-tagged to the second admin.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

laoc merged commit 09842a48fe into dev

2026-07-30 05:27:28 +00:00

laoc referenced this pull request from a commit

2026-07-30 05:27:28 +00:00

Merge pull request 'fix(membership): keep the partial-delivery warning on screen' (#53) from fix/membership-partial-delivery-visible into dev