docs(join-form-flow): document the /subscribe flow, and fix a Turnstile retry - #1049
Merged
Conversation
The submit action names this document as the flow contract to keep in sync when its inputs or behaviour change, and #1031 changed both without touching it. Adds /subscribe as a third entry point, the create-versus-update axis the action now turns on, and a section on chapter sharing, which the two forms capture differently and which decides how the Airtable automations route a signup. Also corrects two statements that #1031 made false: that the basics are unconditionally required, and that OnboardingFlow has no initialEmail prop.
✅ Deploy Preview for pauseai ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…mode Review found the previous commit asserted things the code does not do. - Cut a sentence claiming the unconditional required-field check had broken the hand-off. Every create enforces all four fields, so no short row exists, and the hand-off reposts them anyway. The rule is robustness, not a fix. - Only OnboardingFlow logs the onboarding mode; SubscribeFlow fetches it to gate Turnstile and logs nothing. - The signup-time chapter choice is reposted by OnboardingFlow's step-2 form under isContinuation, not by every update, which the Signup source section already implied by noting the volunteer step carries no subscribe marker. Adds the constraint most likely to be broken by a future editor: Email, Intent, Email subscription and Data privacy policy agreed are written from the post on every call, so dropping the keep_informed hidden input would clear subscription flags silently. Also documents continuation mode, which changes the step machine the diagram describes, and drops the initialEmail note that now only narrates history.
- The subscription-preservation warning named one hidden input; there are two, one on the step-2 intent form and one on the step-3 volunteer form, and dropping either clears the flag. Naming one left the other looking safe to remove. - Data privacy policy agreed is hard-coded true on every call, not taken from the post like the fields beside it. - The submit action is not the only route to a Substack subscription: a NewsletterSignup without handoffHref, as the Collagen banner uses, posts straight to Substack. - The result shape splits on live versus stub, not create versus update. - Turnstile hostname checking is weaker than stated: an absent hostname is accepted by design, and it compares hostname rather than full origin. - Adds Paying Interest to the volunteer update fields, drops an implementation detail that carried no constraint, and stops calling the join flow four-step when a newsletter-only signup finishes at step 3. Also points at where the Airtable automation behaviour is documented, since nothing in this repository can establish it.
…ard in the code A review of the document's direction, rather than of its accuracy, argued the unit to watch is claim surface: how many independently falsifiable statements need re-verifying whenever the action changes. This branch had been growing it. - Collects create-versus-update into one section. It was spread across the endpoint section, the Signup source paragraph and two validation bullets, so the next change to the action had three places to rot and a reviewer fixing one copy would miss the others. - Merges the two bot-protection sections, which had drifted into stating the Turnstile hostname check at two different strengths, and defers the mechanism to turnstile-verify.ts, which is commented in full. - Replaces the volunteer field inventory with a pointer to the block that writes it. That list is where a field went missing, and a second copy of a list is a sync liability rather than a tripwire. The create list stays: every field in it has a rule stated elsewhere, so it works as an index. - Drops the snippet inventory, the continuation-mode cosmetics, and the homepage plumbing narration, keeping the coupling that handoffHref carries. - Removes a pointer to a pauseai-civicrm file that is untracked and exists only on one machine, so no other reader could have followed it. Adds the comments the document cannot substitute for: the hazard about Email subscription being cleared is now stated at both hidden inputs in OnboardingFlow and at the field in the action, which is where someone deleting an apparently redundant input will actually be looking.
The chapter-routing comment said notifying the chapter stays a manual Airtable process. That has not been true for as long as the onboarder-alert automation has existed, and it is what led a reviewer to read a correct paragraph in join-form-flow.md as contradicting the code. It now says why the live branch has no use for the lookup: the automations run their own, and route on the chapter-share field this action writes. The header listed only the /join entry points, and claimed the document is the full flow contract while the body states most of the same rules inline. That instruction is what generates the duplication: told to keep a full contract in sync, the next person copies their rule into the document too. It now names both routes and says which half belongs where.
The previous wording said a single-site rule belongs in a comment rather than copied into the document. The document then does exactly that for three rules the action already comments at their site, so the header contradicted the diff it shipped in, and the ban was too strong anyway: a rule important enough to document is usually worth stating at the line that governs it as well. The rule is now about where relying on one place fails, not about avoiding overlap. The document alone is what does not work, because whoever breaks the rule is editing the code and does not have it open.
…ge left stale From a review of the commits nothing had looked at yet. - The /subscribe hidden-input trim dropped intent=Keep informed as noise. It is not: the route pins that value, so every row it creates starts there whatever the person picks later, and an Airtable automation keys on exactly that pair of signup source and intent. Restored. - The rewritten header named /join and /subscribe but not the embed route, which is itself an entry point and which the document lists as Route 2. A header that presents an inventory has to be complete. - Three comments in the action still described the pre-/subscribe topology: consent bundling as universal when it is a /join arrangement, and record creation and Substack subscription as step-2 events when /subscribe does both during its own signup. Those are exactly the site-local explanations the new convention says to trust, so leaving them stale undercuts it. - The Turnstile hostname check is also skipped in dev, and dev is independent of ONBOARDING_LIVE. - The widget remount is not universal: SubscribeFlow remounts on every result, OnboardingFlow skips its unexpected-error branch, so a retry after one of those reposts a spent token.
The submit handler reset the widget on success and on a validation failure but not on the unexpected-error branch. Verification spends the token whether or not the write then succeeded, so after a 500 the next attempt reposted a spent token and failed the spam check until the user reloaded the page. Hoisted the reset to run on every result, which is what SubscribeFlow already does. Found while checking a claim the flow document makes about this contract, which was true of one form and not the other.
RisingOrange
changed the title
RisingOrange
marked this pull request as ready for review
August 20, 2026 19:48
RisingOrange
enabled auto-merge (squash)
August 20, 2026 19:50
RisingOrange
added a commit
that referenced
this pull request
Aug 20, 2026
…ing, None in the INTENTS bullet)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #1031, which changed the submit action's inputs and behaviour without touching the document that action names as its contract:
Asked for by @Wituareard on #1031.
What it adds
/subscribeas a third entry point, with its files, its hidden inputs (subscribe_form=1,intent=Keep informed,agree_gdpr=on), its three-phase machine, and the reset-on-same-page-navigation behaviour.record_idis an update, and updates skip the required-field check, never rewriteSignup source, and only overwrite a basic field when the value is non-empty./joinbundling it into the required privacy checkbox and/subscribeasking separately, and the field decides whether a signup is handed to their national chapter's leader or to global onboarding. That was the whole point of the change and the document said nothing about it.NewsletterSignup'shandoffHref, including that it reuses the?subscribe-email=parameter the Collagen banner already reads, so nobody mistakes it for a collision.What it corrects
Two statements #1031 made false:
OnboardingFlowhas noinitialEmailprop and that the Collagen prefill therefore does nothing. The prop was added for the/subscribehand-off, so that prefill now works.Beyond the document
Reviews of this branch turned up things the document could describe but not fix, so two of them are here:
Email subscriptionbeing cleared is now stated at both{#if keepInformed}hidden inputs and at the field in the action. Whoever deletes an apparently redundant hidden input is editing that file, not reading this document. Several comments in the action that still described the pre-/subscribetopology are corrected too.OnboardingFlowreset the Turnstile widget on success and on a validation failure but not on the unexpected-error branch, so after a 500 the next attempt reposted a spent token and failed the spam check until the user reloaded.SubscribeFlowalready did this correctly. Found while checking a claim the document makes about that contract.The file header used to call this document "the full flow contract" and tell readers to keep it in sync. That instruction is what generates duplication: told to keep a full contract current, the next person copies their rule in as well as commenting it. It now says the document collects the contracts, that stating a rule in both places is fine where it earns its keep, and that relying on the document alone is what fails.