[Payment methods] CVC bug fix #574

cdolm92 · 2019-02-04T16:22:49Z

📲 What

We were presenting a successful message banner when a credit card w/ an incorrect CVC or date was entered.

🤔 Why

Instead we want to inform the user that the card failed to be added as a payment method.

🛠 How

addNewCreditCard(input: CreatePaymentSourceInput) was returning SignalProducer<GraphMutationEmptyResponseEnvelope, GraphError> instead of a SignalProducer<CreatePaymentSourceEnvelope, GraphError>. The CreatePaymentSourceEnvelope has the field isSuccessful that determines if creating the payment source was successful and the field errorMessage to present when there is an error.

♿️ Accessibility

⚠️Found some a11y issues w/ this screen that would be best fixed in another PR ⚠️

Tap targets use minimum of 44x44 pts dimensions
Works with VoiceOver
Supports Dynamic Type

✅ Acceptance criteria

Test on Production

Test w/ correct CVC

Go to Settings navigate to Account > Payment Methods tap + Add a new card
Enter your name and credit card info w/ correct CVC
Tap Save you should see an activity indicator and the view dismisses back to Payment Methods screen w/ a success banner. You should see your credit card listed

Test w/ incorrect CVC

Go to Settings navigate to Account > Payment Methods tap + Add a new card
Enter your name and credit card info w/ incorrect CVC
Tap Save you should see an activity indicator, followed by a error banner. Navigate back to Payment Methods screen and you should not see your credit card listed

⏰ TODO

Fix merge issue
Testing a11y
Writing acceptance criteria

…ter/ios-oss into add-card-bug-fix

justinswart

Cool, some minor comments, and a question!

KsApi/MockService.swift

justinswart · 2019-02-05T01:42:39Z

KsApi/mutations/CreatePaymentsSourceMutation.swift

@@ -12,6 +12,8 @@ public struct CreatePaymentSourceMutation<T: GraphMutationInput>: GraphMutation
    mutation createPaymentSource($input: CreatePaymentSourceInput!) {
    createPaymentSource(input: $input) {
      clientMutationId\


Is the \ here correct? What does that mean in GraphQL? I would think that there's no \ and the three properties should align.

Thanks! we do not need this here. I've seen us do this in other schemas. It also seems like GraphQL does not require \ in schemas so I went ahead and took those out.

This was just to add a line break in a multi-line string literal. Not technically needed but it reads nicer when we print the mutations in the debug logs.

Oh right, ok well I guess for consistency let's remove it if everyone's cool with that?

Thanks Isabel! I think we should keep them since it helps w/ readability on the debug log.
Update: going to delete tested it without \ and we still get the line break we want on the log.

Library/ViewModels/AddNewCardViewModel.swift

cdolm92 · 2019-02-06T15:19:38Z

@justinswart g2g? 👀

justinswart · 2019-02-06T16:22:17Z

@justinswart g2g? 👀

I think you just have a merge conflict to resolve and i still need to test it

# Conflicts: # Library/ViewModels/AddNewCardViewModelTests.swift

justinswart

👍

* New card screen ui (#471) * add card view controller * transition to add new card and uibarbuttons * settings style applied to add new card screen * changed keyboard style for textfields * view controller tests * placeholder for cardholder name textfield * Record new screenshots * new strings, added strings as place holder and text label * pr feedback - inferring type for cancel button tintcolor * pr feedback - keypath, no more lenses * swiftlint fix * new snapshots * Payment methods (#457) * Stripe element add new card (#473) * New card screen ui (#471) * add card view controller * transition to add new card and uibarbuttons * settings style applied to add new card screen * changed keyboard style for textfields * view controller tests * placeholder for cardholder name textfield * Record new screenshots * new strings, added strings as place holder and text label * pr feedback - inferring type for cancel button tintcolor * pr feedback - keypath, no more lenses * swiftlint fix * new snapshots * new stppaymentcardtextfield showing * wip - placeholder colors * snapshot tests * Email undeliverable/unverified (#478) * Adding emailIsVerified * Renaming GraphUserEmail * ViewModel tests * ChangeEmailViewController tests * Strings, swiftlint * Strings & new screenshots * Cleanup * PR comments * PR updates * Test naming * pr feedback * new snapshots * new placeholder colors * new snapshots * Delete payment methods (#479) * Add payment method deletion * Test datasource * Test view model * Clean-up and address PR feedback * Rename credit card -> payment method * Fix test * Add credit card implementation (#503) * Email undeliverable/unverified (#478) * Adding emailIsVerified * Renaming GraphUserEmail * ViewModel tests * ChangeEmailViewController tests * Strings, swiftlint * Strings & new screenshots * Cleanup * PR comments * PR updates * Test naming * Use correct function to log events in crash logs (#481) * Use correct function to log events in crash logs * Fix indentation * changed ui colors for textfield text and font size of text label * vm and work on enabling save * wip- get stripe token * wip- payment source mutation * wip- keyboard response * wip- getting a stripe error here * wip- error fix w/ publishable key, error banner showing * wip- saving with error and keyboard functionality * wip- updating card immediately * wip * wip - ACs met * wip - refactor in view model, made IDs testable, begane VM tests * wip -refactor on vm/vmtest * wip -refactor deleted comments on vmt * wip -snapshot tests * swiftlint fixes * renaming/refactor, corrected paymentmethodstests * pr feedback * swiftlint fixes * changed function name * indentation * Payment methods event tracking (#496) * Reverted code that deleted SettingsNewsletters from Storyboard.swift * swiftlint * Reverted code to instantiate settings newsletters vc on tests * Alphabetized storyboard enum * Settings payments colors (#530) * Unsupported Credit Cards (#561) * Viewmodel logic for unsupported cards * Tests and accessibility fixes * Simplify add card button * Screenshots and string updates * Cleanup design and remove logs * Improving comment message * Removing filter debug builds * Addressing PR comments * Updating payment methods screenshots * Screenshots * Possible fix for alignment issues * Moving iOS 10 handling to output signal closure * Remove line * Renaming * Last cleanup * Strings & Asset update * Switching from unsupported to supported * Zipcode field in Add New Card screen (#566) * Viewmodel logic for unsupported cards * Tests and accessibility fixes * Simplify add card button * Screenshots and string updates * Cleanup design and remove logs * Improving comment message * Removing filter debug builds * Addressing PR comments * Updating payment methods screenshots * Screenshots * Possible fix for alignment issues * Moving iOS 10 handling to output signal closure * Remove line * Renaming * Last cleanup * Shared styled form field * Adding functionality for zipcode form field * More functionality * View model & screenshot tests * Swiftlint * Cleanup * Improving test and cleanup * PR feedback and autocapitalizing zipcode * Regenerating ChangePassword screenshots * [Payment methods] CVC bug fix (#574) * wip * wip * wip * wip-swiftlint * moved CreatePaymentSourceEnvelope to its own file, Mockservice * swiftlint fix * deleted unnecesary debugging code * new paymentsource temploate and vm tests * swiftlint fix * fixed vm test * PR feedback * update schema * [Payment methods] Minor bug fixes (#579) * tableViewIsEditing false and show banner after dismiss * Swiftlint * Adding StripePublishableKey to example file * Update publishableKey test * Spacing * Point-free helper * [Payment Methods] Update padding and image view size (#581) * Update padding and image view size * Update stack view's spacing * Update snapshots * Use layoutMargins to set the padding * [Payment methods] Disable edit button if no payment methods (#586) * [Payment methods] Bugs & visual fixes (#578) * Adding custom UITableViewHeader and always calling reload data on viewDidLoad * Updating screenshots * Cleanup autolayout warnings * Screenshots * PR comments * No autolayout for footer view * Cleanup * Remove intrinsicContentSize * Tableview appears without delay fix (#587) * Fix header/footer auto sizing * Update header background color * Update snapshots * Screenshots again * Updating constraint priority to resolve ambiguity * [Payment methods] Refresh payment methods table view properly (#588) * Refresh payment methods table view properly * Remove reference to weak self * Rename delegate methods * [Payment methods] Refetch payment methods on viewDidLoad or explicitly (by delegate) (#591) * Do not refresh on viewWillAppear * Use optional string to avoid unnecessary initializer * Bring back viewWillAppear signal to better reflect view controller's lifecycle state * [Payment methods] A11y - Credit card name (#593) * Add card name to the a11y label * Add comment * [Payment Methods] Design Fixes (#592) * Design fixes * Fresh screenshots ✨ * Remove recordMode * Deleting stale screenshots * Re-add accidentally deleted line * update strings * [Payment Methods] Support optional card type and error handling for payment methods (#596) * Optional card type, payment methods load error * Optional support for imageName * Fix accessibility label * Fix merge conflict spacing * Swiftlint * Implementing PR suggestions * Fixed bug that shows wrong expiration date (#595) * [Payment Methods] Add New Card Design Fixes (#594) * error message present and return key to done * removed error message banner for incomplete payment details * new snapshots for correct insets * [Payment methods] Optimistically disable edit button on card deletion (#589) * Disable edit button optimistically, use stored cards in result from card deletion * Fix issue where edit button is not re-enabled upon the failed deletion of the last card. * Remove unnecessary .init * Add skipRepeats() to editButtonIsEnabled signal. * Fixing potential reference cycles (#598)

cdolm92 added 10 commits January 30, 2019 15:09

wip

e72109b

wip

b2ccbde

wip

9aa1580

wip-swiftlint

fa3869c

moved CreatePaymentSourceEnvelope to its own file, Mockservice

63356f5

swiftlint fix

d2d3871

deleted unnecesary debugging code

384ffbb

new paymentsource temploate and vm tests

ea63668

swiftlint fix

7b8f79d

Merge branch 'feature-payment-methods' of https://github.com/kickstar…

2cdd18e

…ter/ios-oss into add-card-bug-fix

cdolm92 added the WIP label Feb 4, 2019

fixed vm test

04ffda4

cdolm92 changed the title ~~[WIP]Add card bug fix~~ Add card bug fix Feb 4, 2019

cdolm92 added needs review and removed WIP labels Feb 4, 2019

cdolm92 requested review from ifbarrera and justinswart February 4, 2019 17:29

justinswart reviewed Feb 5, 2019

View reviewed changes

cdolm92 assigned justinswart Feb 5, 2019

cdolm92 added 2 commits February 5, 2019 14:56

PR feedback

7e70002

update schema

4415adf

cdolm92 changed the title ~~Add card bug fix~~ [Payment methods] Add card bug fix Feb 7, 2019

cdolm92 changed the title ~~[Payment methods] Add card bug fix~~ [Payment methods] CVC bug fix Feb 7, 2019

Merge branch 'feature-payment-methods' into add-card-bug-fix

f66fea4

# Conflicts: # Library/ViewModels/AddNewCardViewModelTests.swift

justinswart force-pushed the add-card-bug-fix branch from 9309fda to f66fea4 Compare February 8, 2019 18:44

justinswart approved these changes Feb 8, 2019

View reviewed changes

cdolm92 merged commit 7a83fd4 into feature-payment-methods Feb 8, 2019

cdolm92 deleted the add-card-bug-fix branch February 8, 2019 19:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Payment methods] CVC bug fix #574

[Payment methods] CVC bug fix #574

cdolm92 commented Feb 4, 2019 •

edited by justinswart

justinswart left a comment

justinswart Feb 5, 2019

cdolm92 Feb 5, 2019

ifbarrera Feb 5, 2019

justinswart Feb 6, 2019

cdolm92 Feb 6, 2019 •

edited

cdolm92 commented Feb 6, 2019

justinswart commented Feb 6, 2019

justinswart left a comment

[Payment methods] CVC bug fix #574

[Payment methods] CVC bug fix #574

Conversation

cdolm92 commented Feb 4, 2019 • edited by justinswart

📲 What

🤔 Why

🛠 How

♿️ Accessibility

✅ Acceptance criteria

⏰ TODO

justinswart left a comment

Choose a reason for hiding this comment

justinswart Feb 5, 2019

Choose a reason for hiding this comment

cdolm92 Feb 5, 2019

Choose a reason for hiding this comment

ifbarrera Feb 5, 2019

Choose a reason for hiding this comment

justinswart Feb 6, 2019

Choose a reason for hiding this comment

cdolm92 Feb 6, 2019 • edited

Choose a reason for hiding this comment

cdolm92 commented Feb 6, 2019

justinswart commented Feb 6, 2019

justinswart left a comment

Choose a reason for hiding this comment

cdolm92 commented Feb 4, 2019 •

edited by justinswart

cdolm92 Feb 6, 2019 •

edited