Payment methods #597

justinswart · 2019-02-14T23:13:17Z

📲 What

This PR represents a larger body of work to bring payment methods to settings.

🤔 Why

This allows users to add/edit/delete payment methods in the app.

🛠 How

https://github.com/kickstarter/ios-oss/pulls?utf8=%E2%9C%93&q=is%3Apr+payment-methods+

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

* Add payment method deletion * Test datasource * Test view model * Clean-up and address PR feedback * Rename credit card -> payment method * Fix test

* 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

…feature-payment-methods

* 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

* 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

…feature-payment-methods

…feature-payment-methods # Conflicts: # Kickstarter-iOS/Views/Storyboards/Settings.storyboard # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_de_device_pad@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_de_device_phone4_7inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_de_device_phone4inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_de_device_phone5_5inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_de_device_phone5_8inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_en_device_pad@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_en_device_phone4_7inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_en_device_phone4inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_en_device_phone5_5inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_en_device_phone5_8inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_es_device_pad@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_es_device_phone4_7inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_es_device_phone4inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_es_device_phone5_5inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_es_device_phone5_8inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_fr_device_pad@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_fr_device_phone4_7inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_fr_device_phone4inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_fr_device_phone5_5inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_fr_device_phone5_8inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_ja_device_pad@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_ja_device_phone4_7inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_ja_device_phone4inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_ja_device_phone5_5inch@2x.png # Screenshots/_64/Kickstarter_Framework_iOSTests.ChangePasswordViewControllerTests/testChangePassword_lang_ja_device_phone5_8inch@2x.png

* 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

* tableViewIsEditing false and show banner after dismiss * Swiftlint * Adding StripePublishableKey to example file * Update publishableKey test * Spacing * Point-free helper

* Update padding and image view size * Update stack view's spacing * Update snapshots * Use layoutMargins to set the padding

…feature-payment-methods

* 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

* Refresh payment methods table view properly * Remove reference to weak self * Rename delegate methods

…y (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

* Add card name to the a11y label * Add comment

* Design fixes * Fresh screenshots ✨ * Remove recordMode * Deleting stale screenshots * Re-add accidentally deleted line

# Conflicts: # Kickstarter-iOS/Locales/fr.lproj/Localizable.strings # Kickstarter-iOS/Locales/ja.lproj/Localizable.strings # Library/Strings.swift

…ayment methods (#596) * Optional card type, payment methods load error * Optional support for imageName * Fix accessibility label * Fix merge conflict spacing * Swiftlint * Implementing PR suggestions

* error message present and return key to done * removed error message banner for incomplete payment details * new snapshots for correct insets

…#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.

dusi · 2019-02-15T19:01:59Z

Kickstarter-iOS/DataSources/PaymentMethodsDataSource.swift

+    case let (cell as CreditCardCell, value as GraphUserCreditCard.CreditCard):
+      cell.configureWith(value: value)
+    default:
+      assertionFailure("Unrecognized (cell, viewModel) combo.")


We sure we want to crash if something goes wrong?

Can someone double check that we're not currently testing this behaviour?

This is in all of our data sources, there isn't really an alternative 🤷‍♂️ . Will assertionFailure crash in production though? Thought asserts are ignored if not running in debug.

dusi · 2019-02-15T19:07:54Z

Kickstarter-iOS/Views/Cells/CreditCardCell.swift

+import KsApi
+import Library
+import Prelude
+import ReactiveSwift


Looks like this import is not necessary. Maybe we can remove it in the future (should not block the release)

dusi · 2019-02-15T19:09:06Z

Kickstarter-iOS/Views/Controllers/AddNewCardViewController.swift

+import KsApi
+import Library
+import Prelude
+import ReactiveSwift


Same as above

justinswart · 2019-02-15T19:09:56Z

KsApi/models/GraphUserCreditCardTests.swift

+        }
+        """
+
+    let data = jsonString.data(using: .utf8)


In future we should try to replace these with the non-failable initializer:

let data = Data(bytes: jsonString.utf8)

Then we won't need the force unwraps.

justinswart · 2019-02-15T19:12:26Z

Library/ViewModels/AddNewCardViewModel.swift

+
+  private let stripeTokenProperty = MutableProperty<(String, String)?>(nil)
+  public func stripeCreated(_ token: String?, stripeID: String?) {
+    if let token = token, let stripeID = stripeID {


Our inputs should not contain any logic.

ifbarrera

dusi · 2019-02-15T19:19:49Z

KsApi/models/DeletePaymentMethodEnvelopeTests.swift

+    let data = jsonString.data(using: .utf8)
+
+    do {
+      //swiftlint:disable force_unwrapping


With a lot of these disables we do not re-enable them after the interested scope. This applies to some of the other rules in this PR. Should we use <disable><enable> or <disable-next-line>?

cdolm92 and others added 30 commits November 8, 2018 10:40

Payment methods (#457)

6f90116

Delete payment methods (#479)

20c5d50

* Add payment method deletion * Test datasource * Test view model * Clean-up and address PR feedback * Rename credit card -> payment method * Fix test

Payment methods event tracking (#496)

c1f460b

Merge remote-tracking branch 'oss/master' into feature-payment-methods

5dcb0e2

Merge remote-tracking branch 'oss/master' into feature-payment-methods

bdcc61a

Reverted code that deleted SettingsNewsletters from Storyboard.swift

ddd33c8

swiftlint

463c5c9

Reverted code to instantiate settings newsletters vc on tests

4a19fbe

Alphabetized storyboard enum

c726171

Settings payments colors (#530)

67c1506

Merge branch 'master' of https://github.com/kickstarter/ios-oss into …

3ffeb56

…feature-payment-methods

Merge branch 'master' of https://github.com/kickstarter/ios-oss into …

7d430ae

…feature-payment-methods

Merge branch 'master' of https://github.com/kickstarter/ios-oss into …

63b7b31

…feature-payment-methods

Regenerating ChangePassword screenshots

a6520b0

[Payment methods] Minor bug fixes (#579)

fed113d

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

c9c2290

* Update padding and image view size * Update stack view's spacing * Update snapshots * Use layoutMargins to set the padding

Merge branch 'master' of https://github.com/kickstarter/ios-oss into …

ddc528c

…feature-payment-methods

[Payment methods] Disable edit button if no payment methods (#586)

de671a7

[Payment methods] Refresh payment methods table view properly (#588)

e380302

* Refresh payment methods table view properly * Remove reference to weak self * Rename delegate methods

[Payment methods] Refetch payment methods on viewDidLoad or explicitl…

33bfda9

…y (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)

3f73a7a

* Add card name to the a11y label * Add comment

[Payment Methods] Design Fixes (#592)

71c9daf

* Design fixes * Fresh screenshots ✨ * Remove recordMode * Deleting stale screenshots * Re-add accidentally deleted line

justinswart and others added 7 commits February 14, 2019 15:16

update strings

59bea31

Merge branch 'master' into feature-payment-methods

42c3687

# Conflicts: # Kickstarter-iOS/Locales/fr.lproj/Localizable.strings # Kickstarter-iOS/Locales/ja.lproj/Localizable.strings # Library/Strings.swift

[Payment Methods] Support optional card type and error handling for p…

796d58a

…ayment 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)

125fb3b

[Payment Methods] Add New Card Design Fixes (#594)

59f90ba

* error message present and return key to done * removed error message banner for incomplete payment details * new snapshots for correct insets

Fixing potential reference cycles (#598)

51649a7

dusi reviewed Feb 15, 2019

View reviewed changes

justinswart commented Feb 15, 2019

View reviewed changes

justinswart changed the title ~~[WIP] Payment methods~~ Payment methods Feb 15, 2019

ifbarrera approved these changes Feb 15, 2019

View reviewed changes

justinswart merged commit ccfdcaa into master Feb 15, 2019

justinswart deleted the feature-payment-methods branch February 15, 2019 19:17

dusi reviewed Feb 15, 2019

View reviewed changes

justinswart restored the feature-payment-methods branch February 15, 2019 21:23

dusi deleted the feature-payment-methods branch February 25, 2019 23:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Payment methods #597

Payment methods #597

justinswart commented Feb 14, 2019

dusi Feb 15, 2019

dusi Feb 15, 2019

justinswart Feb 15, 2019

dusi Feb 15, 2019

dusi Feb 15, 2019

justinswart Feb 15, 2019

justinswart Feb 15, 2019

ifbarrera left a comment

dusi Feb 15, 2019

Payment methods #597

Payment methods #597

Conversation

justinswart commented Feb 14, 2019

📲 What

🤔 Why

🛠 How

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

ifbarrera left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment