[Stripe Cardholder] Only pass verified phone number to Stripe#13322
Open
manuthecoder wants to merge 9 commits intomainfrom
Open
[Stripe Cardholder] Only pass verified phone number to Stripe#13322manuthecoder wants to merge 9 commits intomainfrom
manuthecoder wants to merge 9 commits intomainfrom
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
manuthecoder
pushed a commit
that referenced
this pull request
Mar 29, 2026
… phone requirement - Show a warning banner in the card creation form when the user's phone number is not verified, linking them to settings to fix it - Add an explicit pre-flight redirect in StripeCardsController#create (mirrors the existing birthday check pattern) so users get a clear error before the service layer is invoked - Add User model specs asserting that (1) changing phone_number while unverified does NOT forward the new number to Stripe, and (2) transitioning phone_number_verified false→true triggers syncing the verified number to the Stripe cardholder https://claude.ai/code/session_01RoJupKwnSehbj8SKpG5DYi
- Add phone_number_verified? guard to Api::V4::StripeCardsController#create, returning 400 instead of 500 for unverified users (consistent with web UI) - Add User model specs covering: unverified phone change does not sync to Stripe cardholder, and phone_number_verified false→true triggers sync https://claude.ai/code/session_01UD6XZs8JxA3CciVgAYTQf1
garyhtou
approved these changes
Apr 14, 2026
|
|
||
| it "does nothing when stripe cardholder has no stripe_id" do | ||
| user = create(:user, phone_number: "+18556254225", phone_number_verified: true, email: "test@example.com") | ||
| cardholder = create(:stripe_cardholder, user:, stripe_id: nil, stripe_email: "test@example.com") |
garyhtou
requested changes
Apr 14, 2026
Member
garyhtou
left a comment
There was a problem hiding this comment.
Ah, actually, a few more things come to mind:
- There are some countries HCB is unable to send SMS codes to. How do we handle this case?
- There are a large number of existing users with stripe cards that don't have their phone number verified. Merging this PR will cause their phone number to be removed from stripe the next time their cardholder is updated. Are we okay with that? If so, should we remove their phone numbers in an OTJ now so that the paper trail is more clear?
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
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.
Closes #13288