feat(invitations): claim invite token at open signup when userFacing is on - #3982
Conversation
…en userFacing is on Closes #3981 The open-signup gate (#3833) always resolved a presented invite token but never claimed/finalized it while public signup was open, so the referral loop could never convert on open-signup deployments — `invitation.accepted` never fired and no reward was granted. Extend the claim/finalize/release gate from "signup was closed" to "signup was closed OR invitations.userFacing is on" in both the eligibility checker (invitations.init.js) and the signup controller (the new `inviteHonored` derivation in auth.controller.js) — the two conditions are read from the same config and must mirror exactly, since a mismatch would either finalize an invite that was never claimed or leave a claimed one stuck. All existing token guards (email pin, single-use, expiry, self-referral) stay enforced unchanged; a signup without a token still never invents attribution. `userFacing: false` (the default) preserves today's behavior byte-for-byte. Also expose `invitations.userFacing` in GET /api/auth/config (same top-level, unauthenticated pattern as `sign.up`) so a consumer can gate referral UI on it.
…e claim condition in auth Pre-push review (Phase 0 gate) flagged a high-severity risk: auth.controller.js and invitations.init.js each independently re-derived the same closed-signup / userFacing condition from config to decide whether an invite needed finalize/release. A future edit to one side without the other would either finalize an invite that was never claimed, or leave a claimed one stuck. invitations.init.js is the only code that calls claim() — make it the single source of truth by relaying a `claimed` boolean on the opaque eligibility result. auth.controller.js now trusts `eligibility.claimed` verbatim instead of re-deriving the condition from `config.invitations.userFacing`. Behavior is unchanged; only the internal wiring moved.
Phase 0 gate iteration-2 nit: the JSDoc example on assertSignupEligible still described the pre-#3981 { invite, finalize, release } shape.
|
Warning Review limit reached
Next review available in: 8 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3982 +/- ##
==========================================
+ Coverage 93.15% 93.28% +0.13%
==========================================
Files 170 170
Lines 5639 5647 +8
Branches 1816 1817 +1
==========================================
+ Hits 5253 5268 +15
+ Misses 311 306 -5
+ Partials 75 73 -2
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
codecov/patch flagged these two release call sites as newly-uncovered (the diff rewrote their guarding condition to inviteHonored, so the whole line counts as added) — no prior test in the suite exercised a claimed invite alongside a verify-step or org-provisioning throw.
Fallback critical review (CodeRabbit rate-limited) caught a regression: on userFacing open-signup, a lost claim() race (e.g. a double-submit / client retry of the same invite link) threw AppError(422) out of the eligibility checker, hard-failing the entire signup — even though open signup's own invariant is that a presented token must never be able to block an otherwise-valid signup. Pre-#3981, this same race on open signup never called claim() at all, so it never blocked. Scope the throw-on-claim-failure behavior to the closed-signup branch only (where the invite is genuinely required, so a lost race legitimately blocks signup — unchanged). The userFacing open-signup branch now catches a claim failure and downgrades to unclaimed, letting signup proceed exactly as if the token had merely been presented-but-not-required. Also refreshes two stale auth.controller.js comments (still described the pre-#3981 { invite, finalize, release } contract and claimed open signup "never burns/locks a presented token" unconditionally) flagged in the same review pass.
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 8 minutes. |
1 participant
Summary
invitations.userFacingistrue, the signup flow now claims and finalizes a presented invite token even while public signup is open —invitation.accepted, the analytics redemption event, referral reward, and referrer notification fire exactly as in closed-signup mode.userFacing: false(the default) keeps today's behavior unchanged, byte-for-byte. Also exposesinvitations.userFacinginGET /api/auth/config(same top-level, unauthenticated pattern assign.up) so a consumer can gate referral UI on it. The eligibility checker (the only code that callsclaim()) now relays aclaimedboolean on its opaque result soauth.controller.jstrusts a single source of truth instead of re-deriving the same condition from config on both sides.Scope
auth(signup controller, eligibility JSDoc),invitations(eligibility checker, config comment, README)none(the existing genericregisterSignupEligibilityseam — auth still never imports invitation code)low(change is behind an existing config flag defaulting tofalse; default-path behavior is unchanged, covered by a byte-for-byte regression test)Validation
npm run lintnpm test(npm run test:unit— 2245 tests,npm run test:integration— 499 tests,npm run test:e2e— 16 tests, all green)referredBy, and theinvitation.acceptedevent all fire in the new cellGuardrails check
.env*,secrets/**, keys, tokens)Notes for reviewers
create()-time self-referral guard) — this PR only widens WHEN the existing claim/finalize path runs, never how it validates a token. A signup without a token never invents attribution in either flag state.user_signed_upanalytics event'sinvited/invitationId/invitedByfields key off a resolved invite, not off actual attribution (eligibility.claimed) — a narrow, pre-existing gap this PR's own claim-race downgrade slightly widens (found in independent review, filed rather than expanding this PR's scope).