Backend: fix feature-flag gating gaps for SMS and postal verification - #8781
Conversation
Evaluate sms_enabled per-user instead of globally - send_sms always has a user in scope (its only caller, ChangePhone, has the context), so global evaluation skipped feature-usage tracking and would silently fall through to the default if the flag ever became a rollout/experiment. Gate ConfirmPostalAddress on postal_verification_enabled. Previously only the free address-validation step (InitiatePostalVerification) was gated, so a user already past step 1 could still trigger a (paid) postcard with the flag off. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Move the sms_enabled check to the servicer, consistent with every other feature flag (donations, postal verification, strong verification, recaptcha). ChangePhone now aborts UNAVAILABLE with a translatable "sms_disabled" error rather than send_sms returning a magic string surfaced as UNIMPLEMENTED. send_sms goes back to a pure sender. Removing a phone number stays ungated since it sends no SMS. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
📝 Release NotesThis PR does not need to be included in release notes. Reason: This is a backend follow-up that fixes internal feature-flag gating for SMS and postal verification. It mainly affects admin-controlled enable/disable behavior and edge cases when features are turned off, rather than adding new functionality or a broadly noticeable user-facing change. While it prevents verification actions from slipping through when disabled, this is too implementation-specific and limited to merit release notes. 🤖 Bot Debug InformationModel: |
Follow-up to #8774, tightening up two places where the recently-migrated feature flags weren't gating things correctly.
What & why
1.
sms_enablednow gated at theChangePhoneRPC. Previously the flag was read globally (anonymous) insidesend_sms, even though its only caller always has a user context. Two problems: global evaluation skips per-user feature-usage tracking and would silently fall through todefault=Falseif the flag ever became a rollout/experiment; and gating deep in a util was inconsistent with every other flag. NowChangePhonecheckssms_enabledper-user and abortsUNAVAILABLE+ translatablesms_disabled, matchingdonations/postal_verification/strong_verification.send_smsis back to a pure sender. Removing a phone number stays ungated (it sends no SMS).2.
postal_verification_enabledonly gated the free step. OnlyInitiatePostalVerification(address validation) checked the flag.ConfirmPostalAddress— the step that queues the actual (paid) postcard — did not, so a user already past step 1 could still trigger a postcard with the flag turned off. NowConfirmPostalAddressis gated too, consistent with initiation.Deliberately left as-is
recaptcha_enabled(per-user, butAntiBot/AntiBotPolicyserve logged-out signup traffic): per-user is already the most-correct choice; anonymous users can't be bucketed, so this is a config concern (use an unconditional force rule, not a % rollout, to cover logged-out users) rather than a code bug.check_mypostcard_jobs(global eval): it's a fleet-wide monitoring job with genuinely no per-user context, so global is the documented-correct path.Testing
test_postal_verification_confirm_disabledassertsConfirmPostalAddressabortsUNAVAILABLEwhen the flag is off (seeds a pending attempt directly, since initiation is gated by the same flag).test_ChangePhone_sms_disabledasserts setting a number abortsUNAVAILABLEwhensms_enabledis off, and that removing a number is still allowed.uv run pytest src/tests/test_verification.py src/tests/test_postal_verification.py src/tests/test_experimentation.py src/tests/test_account.py— all pass.make formatclean.make mypyhas two pre-existing errors in untouched files (tests/fixtures/misc.py,test_calendar_events.py), unrelated to this change.Backend checklist
developif necessary for linear migration historyFor maintainers
🤖 Generated with Claude Code