-
Notifications
You must be signed in to change notification settings - Fork 997
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Auto select payment method if only one #601
Auto select payment method if only one #601
Conversation
And rename a related private property for clarity
Stripe/STPPaymentMethodTuple.m
Outdated
} | ||
else if (_allPaymentMethods.count == 1) { | ||
id<STPPaymentMethod> method = _allPaymentMethods.anyObject; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: blank lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, other than the nit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, sorry – changing my approval here. I think we should write some unit tests for the new logic in STPPaymentMethodTuple
.
Ah yeah, will do. |
Also fix some logic in it based on writing the tests.
Updated |
|
||
- (void)testOnlyOneAvailableNotAllowedPaymentMethod { | ||
STPPaymentMethodTuple *tuple = [[STPPaymentMethodTuple alloc] initWithSavedPaymentMethods:nil | ||
availablePaymentTypes:@[[STPPaymentMethodType creditCard]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this test case is the same as the one above – should we delete?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah, not sure what happened here, maybe a bad copy and paste or something. I'll delete one of them.
availablePaymentTypes:@[[STPPaymentMethodType bancontact]] | ||
selectedPaymentMethod:nil]; | ||
|
||
XCTAssertEqualObjects(tuple.selectedPaymentMethod, [STPPaymentMethodType bancontact]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we also add a testOnlyOneAvailableSavedPaymentMethod
, e.g.
STPPaymentMethodTuple *tuple = [[STPPaymentMethodTuple alloc] initWithSavedPaymentMethods:@[card]
availablePaymentTypes:@[[STPPaymentMethodType creditCard]]
selectedPaymentMethod:nil];
XCTAssertEqualObjects(tuple.selectedPaymentMethod, card);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is not a valid case. You have multiple available payment methods there (new card or saved card) so nothing will be auto selected. I can add a test to make sure it stays unselected if you want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah interesting. I guess this behavior makes sense, if the customer doesn't have a defaultSource
we shouldn't auto-select a saved card.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, defaultSource logic is done elsewhere. Like theoretically if you have one, that will be passed through as the selectedPaymentMethod from the part of the code that parses customers. Similarly if there are banned saved sources, the customer parsing logic removes them before coming here.
The logic in this class is mostly independent from that sort of STPCustomer parsing, instead just making sure that the selectedPaymentMethod you passed in is actually in the list of all possible things. And that if there is only one possible thing that can be chosen (and no further details are required) that it just gets auto-selected.
XCTAssertNil(tuple.selectedPaymentMethod); | ||
} | ||
|
||
- (void)testOnlyOneAvailableUnselectablePaymentMethod { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also handle the case where your customer has saved cards, but cards aren't in your available types? (I think we'd have to update the implementation a bit)
STPPaymentMethodTuple *tuple = [[STPPaymentMethodTuple alloc] initWithSavedPaymentMethods:@[card]
availablePaymentTypes:@[[STPPaymentMethodType giropay]]
selectedPaymentMethod:nil];
XCTAssertNil(tuple.selectedPaymentMethod);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tuple does not actually do this check. I could move the logic in here? But as written that filtering of invalid saved cards is done elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, ok. i think it's fine for it to live elsewhere, we should just add some tests around that behavior to PaymentContext at some point (noted in our todos)
lgtm! |
* Functional signup flow prototype * Hide sign up form for incomplete emails * TODO cleanup kinda * Better typing delay * Cleanup : * Loading indicator * Tests and cleanup * StripePass -> Link refactor * Adds new string * Disable checkbox * [Part 2] Link Return Flows (#228) * Return flows (2FA and Cookies) * Some cleanup * Tests * Adding tests and supporting return users w/o payment details * StripePass -> Link * TODO cleanup * Linter fixes * Link off * testSignUpAndPayFlow * Cleanup to prep for Link Payment Method * Adds Link as PaymentMethod and new Link modal sign up flow * Fixes missing reference images for StackViewWithSeparatorTests * Fixes ConsumerSessionTests: Re-adds test consumer account, removes test for old method no longer used * Adds new strings * merge fix * Migrate strings * Fix test build * Adds nodocs, reverts unneeded STPViewWithSeparator changes * Disable snapshot test with new name * Import StripeCore * Re-add STPTestAPIClient+Swift to test target * Migrates from create endpoint to new lookup endpoint (#454) * Split off PayWithLinkViewController (#477) * Split PayWithLinkViewController * Animate the spinner right before the view appears * Re-implement OneTimeCodeTextField (#509) * Re-implement `OneTimeCodeTextField` * Add test for `.isComplete` * Visual tweaks * Rename method * Add snapshot tests * Tweak animation * Cleanup * Fix legacy tests * Update animation * Animation tweaks * Confirm button polish (#497) * Remove hardcoded height and allow configuring the padding * Allow customizing the radius * Updated lock icon * Fix tests * Cleanup * Allow customizing the font * Use tint color rather than the appearance proxy Allow configuring the color per instance of the button. * Add saved payment methods screen (#496) * Add saved payment methods screen * Show saved payment methods screens * Style nav bar * Prep work for supporting logout * Fix paths * Cleanup * Fix tests * Fix availability check * Add copyright notice * Add localization TODOs * Layout polish * Keep header in sync * Cleanup radio button * Remove hardcoded color * Skip snapshot tests on legacy versions * Move layout magic numbers to constants * Remove unused icon * Cleanup * Address feedback * Reuse cell content * Additional cleanup * Remove unused file * Cleanup methods * Fix Xcode 12.2 tests * Add caret animation to OneTimeCodeTextField (#563) * Add caret animation * Cleanup * Pay with Link button (#521) * Re-implement Pay with Link button * Add tests * Rename constant * Fix disabled state The disabled state is won't actually be used, it was just added for the purpose of implementation completeness. * Cleanup * Fix tests * Rework constants * Fix build * Call super * Link 2FA modal polish (#559) * Polish 2FA modal * Use Dynamic Type * Move 2FA logic to the 2FA controller * Cleanup * UX polish * Add Toast component * Make toast accessible * Use custom button * Cleanup * Add nav bar * Use constants * Specify label color * Add TODO * Cleanup toast * Extract logo height to constant * Update TODO * Move toast component * Keep Link images together * Add snapshot tests * Wallet header polish - Part 1 (#582) * Fix recurring memory leak * Show Link account in button * Simplify layout code * Apply corner radius to Apple Pay button * Refactor separator label text and add snapshot tests * Cleanup * Remove duplicated separator label code * Fix Link mandate acceptance (#603) * Update Link colors (#581) * Update colors for dark mode * Update RadioButton style * Small fixes * Update snapshot test * Update OTC field colors * Update snapshots * Fix button color * Update snapshot test * Document custom colors * Add light placeholder color * Update 2FAView snapshot * Fix typos * Fix merge conflict in STPStackViewWithSeparatorSnapshotTests * Wallet header polish - Part 2 (#596) * Show wallet header in saved PM screen * Make auto-selection optional * Hide/show pay button * Show header label * Fallback to old layout when Link is not enabled * Cleanup * Add clarifying comment Co-authored-by: Cameron <[email protected]> * Add extra safety check Co-authored-by: Cameron <[email protected]> * Add custom activity indicator (#601) * Add activity indicator * Use custom activity indicator * Add loader to Button * Switch "Join Link" button to use `Button` * Remove CA transaction * Add comments * Cleanup * Match UIActivityIndicatorView visibility toggle behavior * Make 100% API compatible with `UIActivityIndicatorView` * Cleanup * Refactor and add multiline support to CheckboxButton (#609) * Refactor and add multiline support to CheckboxButton * Address feedback * Add missing strings * Fixes cookie query and adds support for updating value (#458) * Fixes cookie query and adds support for updating value * Adds new test scheme with a hosted app for keychain tests * Remove default case * Link - Implement logout feature (#625) * Implement logout * Decouple cookie storage from ConsumerSession * Cleanup tests * Document cookie store * Logout with server * Move cookie deletion code to `ConsumerSession` * Rename tests * Fix merge conflict * Remove unused store * Additional tests * Allow login/signup for Link from Payment Sheet (#648) * Allow signing up for link by tapping on "Pay with Link" * Add 2FA to PayWithLinkViewController * Cleanup * Update button behavior to match designs * Automatically start verification if needed * Refactor into a computed property * Add missing constraints * Implement resend code feature (#623) * Implement "Resend code" feature * Consolidate localized string * Allows editing of email in Link Modal (#653) * Allows editing of email in Link Modal * Enables Link even if merchant hasn't provided email * Looks up email based on saved cookie if available * Returns to sign up screen on logout * Progressive disclosure during sign up * Updates Link account throughout PaymentSheet * Don't turn on Link in playground by default * Adds new lookup tests * Update Stripe/PayWithLinkViewController-SignUpViewController.swift Co-authored-by: ramont-stripe <[email protected]> * Import foundation * Older xcode build fix Co-authored-by: ramont-stripe <[email protected]> * Adds client_type MOBILE_SDK for extended cookies (#602) * Link UI polish (#650) * Add semantic typography system * Polish signup screen * Switch to semantic type system * Tweak primary button style * Better Dynamic Type support * Tweak mandate line height * Avoid keyboard * UI polish * Update footer to use the new typography system * Update typography of the picker component * Update toast component * Apply Link theme to ConfirmButton * Disable user interaction while a transaction is in progress * Fix layout glitch * Add title label * Import changes from stale `ramont/link-ui-polish` branch * Use a better default for the `animated` flag Pushing a VC with animation is more common than without animation. * Fix typo * Add snapshot tests for link wallet footer * Use consistent naming * Fix tests * Fix another test * Center modal * Present Link controller as form sheet on ipad * Add better Dynamic Type support to 2FA modal * Remove vertical bounce * Fix margins * Remove comment * Better iPad rotation support * Implement ConfirmButton completion block * Fix merge * Fixes signup flow requiring OTP verification (#684) * Fixes verification type parsing for signup Skips otp verification for signup verifications * Fix objc namespacing * [Link] Add Update card UI (#686) * Add UpdatePaymentViewController * Dont' use STPLocalizedString for now * Try to make size command run against base branch * Revert CI change * PR feedback * [Link] Finish update card UI (#687) * Prefill card, lock pan * Add some tests * Satisfy objc symbol checker * saveThisCardCheckboxView -> checkboxView * Add defaults for prefillDetails & inputMode on CardDetailsEditView * Use ElementDelegate instead of STPCardFormViewDelegate * Naming fix * Disable klarna test * Phone number parsing and formatting (#200) * Return flows (2FA and Cookies) * Phone number parsing and formatting * Some cleanup * Tests * Adding tests and supporting return users w/o payment details * StripePass -> Link * TODO cleanup * Merge fix * Linter fixes * Link off * testSignUpAndPayFlow * Old style pattern filling * Converts to Elements * Cleanup unneeded changes * Redacted cleanup * Adds strings * Cleanup comments from PR * Add support for non-recognized country. This fails on the backend for now * Adds different labels for in and out of the picker * Adds explicit accessibility label * Working on refactor * PhoneNumberElement as standalone element * Update e164 formatting to drop leading zeros and enforce length * Auto advances and snapshot test * Add strings. Fix regressed tests * Don't commit test values * Skip new snapshot tests. * Updates from review * Don't enable LInk * Fix test build * Project file fix * Fixes delegate updating and sign up button enable/disable state * Reset number when account changes * Update strings * Shared sorting for region codes * Stop accidentally enabling link * Fix objc symbols * [Link] Extract Link legal terms to its own view (#696) * Extract legal terms view * Extract string * Rename method * Test text wrapping and line height * Update dismiss button * Fix objc symbol check * Add terms view delegate * Add TODO * Add fallback logic * [Link] Make API calls to update card (#695) * Implement card update API * Refactor to use existing setDefault code as update code * Add testUpdatePaymentDetails * Consolidate update params into a struct * [Link] Handle default checkbox in update card flow (#703) * fix merge conflicts * Remove whitespace change * Use CompatibleColor.secondaryLabel instead of lightGray Co-authored-by: Cameron <[email protected]> Co-authored-by: Cameron <[email protected]> * Link Inline signup - Part 1 (#682) * Basic layout * Wire up UI * Move checkbox element to its own file * Add view model * Fix glitchy transitions * Transition tweaks * Implement the LinkAction API * Remove hardcoded phone number * Add phone number element * Fix layout glitch * Refactor and add tests * Display merchant name in checkbox * Remove hardcoded depedencies * Fix tests * Removed global state Inject the cookie store the same way we inject the API client. * Fix constraint warnings * Adjust spacing * Switch to LinkPhoneNumberElement * Use the appropriate API client * Show inline signup only when supported * Only offer Link when user is not registered * Remove beta header * Use the correct API client * Document no-op * Remove factory method * Rename closure * Restore code * Consolidate view model logic * Restore ConsumerSession code * Document LinkAccountServiceProtocol * Handle edge cases * Document OperationDebouncer * Refactor inline signup element Element now just conform to `Element`. Delegate `PaymentOption` building to the container. * Cleanup * Fix ambiguous layout issue * Fix objc symbol * Fix xcodebuild warning * Add a Link switch to the payment sheet playground (#706) * Add a Link switch to the payment sheet playground * Update accessibilityIdentifier on link switch * Update comment Co-authored-by: Cameron <[email protected]> Co-authored-by: Cameron <[email protected]> * Adds support for Link Instant Debits (#685) * LinkAccountSessions bindings * Launching connections redirect * Better button * Starts adding Bank bindings * Redirect working but no bank account id * Merge fixes * Livemode testing * Create payment details from linked accounts * Adds bank bindings * Cleanup TODOs * Adds bank icons * Hacky fix * Less hacky fix * Update remove copy * Remove prefilled email * Revert permissions change that got merged for some reason * Fix alignment and scaling of bank icons and card icons. Add snapshot test to include bank icon. * Remove test code * Fix user interaction * Fix test build and update snapshot tests for new cardbrandview pixel alignment * Cleanup * Fix test failure * Actually revert * Fix 12.2 build * Add new localized strings and remove beta header hacks * Fix objc symbols Co-authored-by: Ramon Torres <[email protected]> * Adds support for using Customer email with Link (#704) * * Adds email param to STPCustomer * Adds API method to retrieve Customer with ID and ephemeral key secret * Adds var on PaymentSheetLinkAccount to detect if we have a session cookie stored * Falls back to using Customer email address if available and no explicit email provided or session cookie available * Fix example UI build * Remove SkippedTests Co-authored-by: ramont-stripe <[email protected]> Co-authored-by: Nick Porter <[email protected]> Co-authored-by: Ramon Torres <[email protected]>
Adds back public accessor for available payment methods on context.
Ensures that selected payment method cant be set to something not in the available list, and that if there is only one possible method it is auto-selected.