Add credit card implementation #503

cdolm92 · 2018-11-30T23:41:22Z

📲 What

Adds the ability for user to save a new credit card payment using Stripe.

🤔 Why

Allowing users to add a new credit card payment increases the manageability of payment information in the app.

🛠 How

We built a custom form using the STPPaymentCardTextField and STPAPIClientclass. The STPAPIClientclass is used to handle the tokenizing the details.

We first create an STPCardParams instance with the payment details provided in the STPPaymentCardTextField and cardholderNameTextField. The STPAPIClient class then converts that into an STPToken instance where we pass the tokenID and stripeID to the CreatePaymentSourceInput() . More details on Stripe's Custom iOS Integration Guide
.

👀 See

Add New Card No Error	Add New Card w/ Error

✅ Acceptance criteria

Saving New Payment Method No Error

Test Environment: Staged w/ Test Cards and Tokens

Saving New Payment Method w/ Error

Test Environment: Production w/ Test Cards and Tokens

Todo

Add ACs and GIF for Saving New Payment Method w/ Error on this PR
Investigate 🤔PaymentMethodsViewModelTests failures.
Update Why explanation 🤷🏾‍♀️

* 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 * Fix indentation

…redit-card-implementation

cdolm92 · 2018-12-03T19:53:27Z

Kickstarter-iOS/Views/Controllers/AddNewCardViewController.swift

  @IBOutlet private weak var cardholderNameLabel: UILabel!
  @IBOutlet private weak var cardholderNameTextField: UITextField!
-  @IBOutlet private weak var paymentField: STPPaymentCardTextField!
+  @IBOutlet private weak var paymentTextField: STPPaymentCardTextField!


paymentTextField sounds too general here and it's obvious that fields here pertain to credit card information so going to rename this to creditCardTextField instead.

cdolm92 · 2018-12-03T20:13:41Z

Library/ViewModels/AddNewCardViewModel.swift

+    self.saveButtonIsEnabled = Signal.combineLatest(
+      cardholderName.map { !$0.isEmpty },
+      self.paymentInfoIsValidProperty.signal
+      ).map { cardholderName, validation in cardholderName && validation }


renaming tocardholderNameFieldNotEmpty would be good here.

cdolm92 · 2018-12-03T20:17:55Z

Library/ViewModels/AddNewCardViewModel.swift

+
+    let paymentInput = Signal.combineLatest(cardholderName, paymentDetails)
+      .map { cardholderName, paymentInfo in
+        (cardholderName, paymentInfo.0, paymentInfo.1, paymentInfo.2, paymentInfo.3) }


renaming paymentInfo to creditCardDetails

cdolm92 · 2018-12-03T20:21:35Z

Library/ViewModels/AddNewCardViewModel.swift

+  var addNewCardSuccess: Signal<String, NoError> { get }
+  var cardholderNameBecomeFirstResponder: Signal<Void, NoError> { get }
+  var dismissKeyboard: Signal<Void, NoError> { get }
+  var paymentDetails: Signal<(String, String, Int, Int, String), NoError> { get }


renaming to creditCardDetails

decided not to. I now think that payment details is fine.

Scollaco

Awesome job, @cdolm92 🌟 I just left a few minor comments.

Scollaco · 2018-12-04T18:46:35Z

Kickstarter-iOS/Views/Controllers/AddNewCardViewController.swift

+  }
+
+  @objc func cardholderNameTextFieldChanged(_ textField: UITextField) {
+    self.viewModel.inputs.cardholderNameChanged(textField.text ?? "")


Here I would make cardholderName optional and simply pass textField.text. This way make the viewModel responsible for taking care of this logic.

Scollaco · 2018-12-04T19:10:23Z

Kickstarter-iOS/Views/Controllers/AddNewCardViewController.swift

+    self.viewModel.inputs.paymentInfo(valid: textField.isValid)
+  }
+
+  func stripeToken(cardholderName: String,


Could you make this func private? Also, I would rename it. It's not clear of what's the function about until we see its implementation. Here I would rename it to createStripeToken(...

Scollaco · 2018-12-04T19:15:16Z

Kickstarter-iOS/Views/Controllers/AddNewCardViewController.swift

+    self.viewModel.inputs.cardholderNameTextFieldReturn()
+  }
+
+  func paymentCardTextFieldDidChange(_ textField: STPPaymentCardTextField) {


For consistency, could you add internal keyword here?

Scollaco · 2018-12-04T19:46:07Z

Library/ViewModels/AddNewCardViewModel.swift

+      .map { cardholderName, creditCardDetails in
+        (cardholderName, creditCardDetails.0, creditCardDetails.1, creditCardDetails.2, creditCardDetails.3) }
+
+    let tryAddCardAction = self.saveButtonTappedProperty.signal


Since we are using this just to dismiss the keyboard and to hide/show the activity indicator, it's ok to use the saveButtonTappedProperty, it gets easier to read this way

Scollaco · 2018-12-04T20:12:23Z

Library/ViewModels/AddNewCardViewModelTests.swift

+    }
+  }
+
+  func testBecomeFirstResponderAndSaveEnabled() {


I think this test could be broken up in 2 pieces: one for becomeFirstResponder part and the other for saveEnabled

Scollaco · 2018-12-04T20:14:10Z

Library/ViewModels/AddNewCardViewModelTests.swift

+    self.saveButtonIsEnabled.assertValues([false, false, true], "Enabled when form is valid.")
+  }
+
+  func testSetPublishableKey_CardInfoInValid() {


I think InValid here should be Invalid

Scollaco · 2018-12-04T21:41:57Z

Kickstarter-iOS/Views/Controllers/AddNewCardViewController.swift

@@ -123,7 +123,7 @@ internal final class AddNewCardViewController: UIViewController, STPPaymentCardT
    self.viewModel.outputs.paymentDetails
      .observeForUI()
      .observeValues { [weak self] cardholderName, cardNumber, expMonth, expYear, cvc in
-        self?.stripeToken(cardholderName: cardholderName,
+        self?.createStripeToken(cardholderName: cardholderName,


Alignment is off here

* 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)

ifbarrera and others added 20 commits November 12, 2018 17:34

Email undeliverable/unverified (#478)

e3cd226

* 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)

e9a1005

* Use correct function to log events in crash logs * Fix indentation

Merge remote-tracking branch 'oss/feature-payment-methods' into add-c…

44ff603

…redit-card-implementation

changed ui colors for textfield text and font size of text label

cc49f7c

vm and work on enabling save

01083af

wip- get stripe token

1859e8c

wip- payment source mutation

6f9dd6f

wip- keyboard response

b066666

wip- getting a stripe error here

8e98ec6

wip- error fix w/ publishable key, error banner showing

3237915

wip- saving with error and keyboard functionality

06cafc2

wip- updating card immediately

654102a

wip

98d07de

Merge remote-tracking branch 'oss/feature-payment-methods' into add-c…

a6144aa

…redit-card-implementation

wip - ACs met

fb3b702

wip - refactor in view model, made IDs testable, begane VM tests

0a221f0

wip -refactor on vm/vmtest

aff4384

wip -refactor deleted comments on vmt

56a0df0

wip -snapshot tests

1759300

swiftlint fixes

a1dbe87

cdolm92 requested review from Scollaco, justinswart, ifbarrera and dusi November 30, 2018 23:47

cdolm92 commented Dec 3, 2018

View reviewed changes

renaming/refactor, corrected paymentmethodstests

40ecf69

cdolm92 added the needs review label Dec 3, 2018

Scollaco reviewed Dec 4, 2018

View reviewed changes

cdolm92 added 2 commits December 4, 2018 16:01

pr feedback

84a4253

swiftlint fixes

8883d58

changed function name

66030fd

Scollaco reviewed Dec 4, 2018

View reviewed changes

indentation

9cd3977

Scollaco approved these changes Dec 4, 2018

View reviewed changes

cdolm92 merged commit bcaf066 into feature-payment-methods Dec 4, 2018

cdolm92 deleted the add-credit-card-implementation branch December 4, 2018 22:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add credit card implementation #503

Add credit card implementation #503

cdolm92 commented Nov 30, 2018 •

edited by Scollaco

cdolm92 Dec 3, 2018

cdolm92 Dec 3, 2018

cdolm92 Dec 3, 2018

cdolm92 Dec 3, 2018

cdolm92 Dec 3, 2018

Scollaco left a comment

Scollaco Dec 4, 2018

Scollaco Dec 4, 2018

Scollaco Dec 4, 2018

Scollaco Dec 4, 2018

Scollaco Dec 4, 2018

Scollaco Dec 4, 2018

Scollaco Dec 4, 2018

Add credit card implementation #503

Add credit card implementation #503

Conversation

cdolm92 commented Nov 30, 2018 • edited by Scollaco

📲 What

🤔 Why

🛠 How

👀 See

✅ Acceptance criteria

Saving New Payment Method No Error

Saving New Payment Method w/ Error

Todo

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Scollaco left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdolm92 commented Nov 30, 2018 •

edited by Scollaco