Skip to content
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

[Payment Methods] Support optional card type and error handling for payment methods #596

Merged
merged 8 commits into from
Feb 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ internal final class PaymentMethodsViewController: UIViewController, MessageBann
self?.goToAddCardScreen()
}

self.viewModel.outputs.errorLoadingPaymentMethods
.observeForUI()
.observeValues { [weak self] message in
self?.messageBannerViewController?.showBanner(with: .error, message: message)
}

self.viewModel.outputs.presentBanner
.observeForUI()
.observeValues { [weak self] message in
Expand Down
12 changes: 8 additions & 4 deletions Kickstarter.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@
77CD894921791B05003066DA /* ProjectStatsTemplate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 77CD894821791B05003066DA /* ProjectStatsTemplate.swift */; };
77CD8981217FA01B003066DA /* ProjectPamphletContentViewControllerConversionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 77CD8980217FA01B003066DA /* ProjectPamphletContentViewControllerConversionTests.swift */; };
77D7A739214187A9003F258C /* SettingsSwitchCellType.swift in Sources */ = {isa = PBXBuildFile; fileRef = 77D7A738214187A9003F258C /* SettingsSwitchCellType.swift */; };
77DB532A2215D0AA0078991C /* GraphUserCreditCardTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 77DB53292215D0AA0078991C /* GraphUserCreditCardTests.swift */; };
77E44B7421823A56006446B8 /* ChangePasswordInput.swift in Sources */ = {isa = PBXBuildFile; fileRef = 77E44B7321823A56006446B8 /* ChangePasswordInput.swift */; };
77E6440120F64F0B005F6B38 /* HelpDataSource.swift in Sources */ = {isa = PBXBuildFile; fileRef = 77E6440020F64F0B005F6B38 /* HelpDataSource.swift */; };
77E6440320F65074005F6B38 /* HelpViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 77E6440220F65074005F6B38 /* HelpViewController.swift */; };
Expand Down Expand Up @@ -1002,7 +1003,7 @@
D63BBCDA217E7802007E01F0 /* CreditCardCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = D63BBCD8217E7802007E01F0 /* CreditCardCell.swift */; };
D63BBCDB217E7802007E01F0 /* CreditCardCell.xib in Resources */ = {isa = PBXBuildFile; fileRef = D63BBCD9217E7802007E01F0 /* CreditCardCell.xib */; };
D63BBCF9217F666B007E01F0 /* PaymentMethodsDataSource.swift in Sources */ = {isa = PBXBuildFile; fileRef = D63BBCF8217F666B007E01F0 /* PaymentMethodsDataSource.swift */; };
D63BBD31217F7212007E01F0 /* UserCreditCard.swift in Sources */ = {isa = PBXBuildFile; fileRef = D63BBD30217F7212007E01F0 /* UserCreditCard.swift */; };
D63BBD31217F7212007E01F0 /* GraphUserCreditCard.swift in Sources */ = {isa = PBXBuildFile; fileRef = D63BBD30217F7212007E01F0 /* GraphUserCreditCard.swift */; };
D63BBD33217F9CDE007E01F0 /* GraphCreditCardTemplate.swift in Sources */ = {isa = PBXBuildFile; fileRef = D63BBD32217F9CDE007E01F0 /* GraphCreditCardTemplate.swift */; };
D63BBD35217FAB85007E01F0 /* PaymentMethodsViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = D63BBD34217FAB85007E01F0 /* PaymentMethodsViewModel.swift */; };
D63BBD37217FC224007E01F0 /* CreditCardCellViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = D63BBD36217FC224007E01F0 /* CreditCardCellViewModel.swift */; };
Expand Down Expand Up @@ -1936,6 +1937,7 @@
77CD894821791B05003066DA /* ProjectStatsTemplate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProjectStatsTemplate.swift; sourceTree = "<group>"; };
77CD8980217FA01B003066DA /* ProjectPamphletContentViewControllerConversionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProjectPamphletContentViewControllerConversionTests.swift; sourceTree = "<group>"; };
77D7A738214187A9003F258C /* SettingsSwitchCellType.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsSwitchCellType.swift; sourceTree = "<group>"; };
77DB53292215D0AA0078991C /* GraphUserCreditCardTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GraphUserCreditCardTests.swift; sourceTree = "<group>"; };
77E44B7321823A56006446B8 /* ChangePasswordInput.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ChangePasswordInput.swift; sourceTree = "<group>"; };
77E6440020F64F0B005F6B38 /* HelpDataSource.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HelpDataSource.swift; sourceTree = "<group>"; };
77E6440220F65074005F6B38 /* HelpViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HelpViewController.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2743,7 +2745,7 @@
D63BBCD8217E7802007E01F0 /* CreditCardCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CreditCardCell.swift; sourceTree = "<group>"; };
D63BBCD9217E7802007E01F0 /* CreditCardCell.xib */ = {isa = PBXFileReference; lastKnownFileType = file.xib; path = CreditCardCell.xib; sourceTree = "<group>"; };
D63BBCF8217F666B007E01F0 /* PaymentMethodsDataSource.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PaymentMethodsDataSource.swift; sourceTree = "<group>"; };
D63BBD30217F7212007E01F0 /* UserCreditCard.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserCreditCard.swift; sourceTree = "<group>"; };
D63BBD30217F7212007E01F0 /* GraphUserCreditCard.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GraphUserCreditCard.swift; sourceTree = "<group>"; };
D63BBD32217F9CDE007E01F0 /* GraphCreditCardTemplate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GraphCreditCardTemplate.swift; sourceTree = "<group>"; };
D63BBD34217FAB85007E01F0 /* PaymentMethodsViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PaymentMethodsViewModel.swift; sourceTree = "<group>"; };
D63BBD36217FC224007E01F0 /* CreditCardCellViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CreditCardCellViewModel.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -4368,7 +4370,8 @@
D01587971EEB2ED6006E7684 /* lenses */,
D01587EE1EEB2ED7006E7684 /* templates */,
D6FB2A3C1FA27D0300B0D282 /* CategoryTests.swift */,
D63BBD30217F7212007E01F0 /* UserCreditCard.swift */,
D63BBD30217F7212007E01F0 /* GraphUserCreditCard.swift */,
77DB53292215D0AA0078991C /* GraphUserCreditCardTests.swift */,
D62B14AF2212184500AC05C8 /* DeletePaymentMethodEnvelopeTests.swift */,
);
path = models;
Expand Down Expand Up @@ -6475,7 +6478,7 @@
D015899B1EEB2ED7006E7684 /* Service.swift in Sources */,
D6ED1B39216D50BE007F7547 /* UserEmailFields.swift in Sources */,
D01588731EEB2ED7006E7684 /* FindFriendsEnvelope.swift in Sources */,
D63BBD31217F7212007E01F0 /* UserCreditCard.swift in Sources */,
D63BBD31217F7212007E01F0 /* GraphUserCreditCard.swift in Sources */,
D79CF32E219CE51A00ECB73A /* CreatePaymentSourceInput.swift in Sources */,
D01588A51EEB2ED7006E7684 /* Project.DatesLenses.swift in Sources */,
D01588EB1EEB2ED7006E7684 /* MessageSubject.swift in Sources */,
Expand Down Expand Up @@ -6576,6 +6579,7 @@
D0D10C211EEB4550005EBAD0 /* User.NotificationsTests.swift in Sources */,
D0D10C1C1EEB4550005EBAD0 /* UpdateDraftTests.swift in Sources */,
D0D10C0D1EEB4550005EBAD0 /* FriendStatsEnvelopeTests.swift in Sources */,
77DB532A2215D0AA0078991C /* GraphUserCreditCardTests.swift in Sources */,
D01589A21EEB2ED7006E7684 /* ServiceTypeTests.swift in Sources */,
D0D10C0F1EEB4550005EBAD0 /* LocationTests.swift in Sources */,
D0D10C121EEB4550005EBAD0 /* Project.CountryTests.swift in Sources */,
Expand Down
1 change: 1 addition & 0 deletions KsApi/models/DeletePaymentMethodEnvelopeTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class DeletePaymentMethodEnvelopeTests: XCTestCase {
let data = jsonString.data(using: .utf8)

do {
//swiftlint:disable force_unwrapping
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you getting a Swiftlint warning here? Is this by any chance a newer version of Swiftlint than what we're using (I think we hardcoded the build script to use 0.29.2).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not, but that's because swiftlint is currently broken 😬 once its fixed, this line is needed

let envelope = try JSONDecoder().decode(DeletePaymentMethodEnvelope.self, from: data!)
XCTAssertEqual(envelope.totalCount, 0)
} catch {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,18 @@ public struct GraphUserCreditCard: Swift.Decodable {
public private(set) var expirationDate: String
public private(set) var id: String
public private(set) var lastFour: String
public private(set) var type: CreditCardType
public private(set) var type: CreditCardType?

public var formattedExpirationDate: String {
return String(expirationDate.dropLast(3))
}

public var imageName: String {
switch self.type {
case .generic:
case nil, .some(.generic):
return "icon--generic"
default:
return "icon--\(self.type.rawValue.lowercased())"
case .some(let type):
return "icon--\(type.rawValue.lowercased())"
}
}
}
Expand Down
107 changes: 107 additions & 0 deletions KsApi/models/GraphUserCreditCardTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
import XCTest
@testable import KsApi

final class GraphUserCreditCardTests: XCTestCase {

func testCreditCardsDecoding_noCards() {
let jsonString = """
{
"storedCards": {
"nodes": [],
"totalCount": 0
}
}
"""
let data = jsonString.data(using: .utf8)

do {
//swiftlint:disable force_unwrapping
let cards = try JSONDecoder().decode(GraphUserCreditCard.self, from: data!)

XCTAssertEqual(cards.storedCards.nodes.count, 0)
} catch {
XCTFail("Failed to decode GraphUserCreditCard")
}
}

func testCreditCardsDecoding_hasCards() {
let jsonString = """
{
"storedCards": {
"nodes": [
{
"expirationDate": "2023-02-01",
"lastFour": "4242",
"id": "3021",
"type": "VISA"
},
{
"expirationDate": "2020-02-01",
"lastFour": "1111",
"id": "2768",
"type": "VISA"
}
],
"totalCount": 0
}
}
"""

let data = jsonString.data(using: .utf8)

do {
//swiftlint:disable force_unwrapping
let cards = try JSONDecoder().decode(GraphUserCreditCard.self, from: data!)

XCTAssertEqual(cards.storedCards.nodes.count, 2)

guard let firstCard = cards.storedCards.nodes.first else {
XCTFail("Failed to decode GraphUserCreditCard")
return
}

XCTAssertEqual(firstCard.type, GraphUserCreditCard.CreditCardType.visa)
XCTAssertEqual(firstCard.lastFour, "4242")
XCTAssertEqual(firstCard.expirationDate, "2023-02-01")
XCTAssertEqual(firstCard.id, "3021")
} catch {
XCTFail("Failed to decode GraphUserCreditCard")
}
}

func testCreditCardsDecoding_unknownCard() {
let jsonString = """
{
"storedCards": {
"nodes": [
{
"expirationDate": "2020-02-01",
"lastFour": "1111",
"id": "2768",
}
],
"totalCount": 0
}
}
"""

let data = jsonString.data(using: .utf8)

do {
//swiftlint:disable force_unwrapping
let cards = try JSONDecoder().decode(GraphUserCreditCard.self, from: data!)

guard let card = cards.storedCards.nodes.first else {
XCTFail("Failed to decode GraphUserCreditCard")
return
}

XCTAssertNil(card.type)
XCTAssertEqual(card.lastFour, "1111")
XCTAssertEqual(card.expirationDate, "2020-02-01")
XCTAssertEqual(card.id, "2768")
} catch {
XCTFail("Failed to decode GraphUserCreditCard")
}
}
}
6 changes: 4 additions & 2 deletions Library/ViewModels/CreditCardCellViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,10 @@ CreditCardCellViewModelOutputs, CreditCardCellViewModelType {
.map(cardImage(with:))

self.cardNumberAccessibilityLabel = self.cardProperty.signal.skipNil()
.map { [$0.type.description, Strings.Card_ending_in_last_four(last_four: $0.lastFour)]
.compactMap{ $0 }.joined(separator: ", ")
.map {
return [$0.type?.description, Strings.Card_ending_in_last_four(last_four: $0.lastFour)]
.compactMap { $0 }
.joined(separator: ", ")
}

self.cardNumberText = self.cardProperty.signal.skipNil()
Expand Down
11 changes: 11 additions & 0 deletions Library/ViewModels/CreditCardCellViewModelTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,15 @@ internal final class CreditCardCellViewModelTests: TestCase {
self.cardNumberText.assertValue("Card ending in 1882")
self.expirationDateText.assertValue("Expires 01/2024")
}

func testCardInfoForUnknownCardType() {
let unknownCard = GraphUserCreditCard.generic |> \.type .~ nil

self.vm.inputs.configureWith(creditCard: unknownCard)

self.cardImage.assertValue(UIImage(named: "icon--generic"))
self.cardNumberAccessibilityLabel.assertLastValue("Card ending in 1882")
self.cardNumberText.assertValue("Card ending in 1882")
self.expirationDateText.assertValue("Expires 01/2024")
}
}
4 changes: 4 additions & 0 deletions Library/ViewModels/PaymentMethodsViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public protocol PaymentMethodsViewModelInputs {
public protocol PaymentMethodsViewModelOutputs {
/// Emits the user's stored cards
var editButtonIsEnabled: Signal<Bool, NoError> { get }
var errorLoadingPaymentMethods: Signal<String, NoError> { get }
var goToAddCardScreen: Signal<Void, NoError> { get }
var paymentMethods: Signal<[GraphUserCreditCard.CreditCard], NoError> { get }
var presentBanner: Signal<String, NoError> { get }
Expand Down Expand Up @@ -67,6 +68,8 @@ PaymentMethodsViewModelInputs, PaymentMethodsViewModelOutputs {

let paymentMethodsValues = paymentMethodsEvent.values().map { $0.me.storedCards.nodes }

self.errorLoadingPaymentMethods = paymentMethodsEvent.errors().map { $0.localizedDescription }

self.paymentMethods = Signal.merge(
paymentMethodsValues,
paymentMethodsValues.takeWhen(deletePaymentMethodEventsErrors)
Expand Down Expand Up @@ -143,6 +146,7 @@ PaymentMethodsViewModelInputs, PaymentMethodsViewModelOutputs {
}

public let editButtonIsEnabled: Signal<Bool, NoError>
public let errorLoadingPaymentMethods: Signal<String, NoError>
public let goToAddCardScreen: Signal<Void, NoError>
public let paymentMethods: Signal<[GraphUserCreditCard.CreditCard], NoError>
public let presentBanner: Signal<String, NoError>
Expand Down
39 changes: 30 additions & 9 deletions Library/ViewModels/PaymentMethodsViewModelTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,22 @@ import Prelude

internal final class PaymentMethodsViewModelTests: TestCase {

let vm = PaymentMethodsViewModel()
let editButtonIsEnabled = TestObserver<Bool, NoError>()
let goToAddCardScreen = TestObserver<Void, NoError>()
let paymentMethods = TestObserver<[GraphUserCreditCard.CreditCard], NoError>()
let presentBanner = TestObserver<String, NoError>()
let reloadData = TestObserver<Void, NoError>()
let showAlert = TestObserver<String, NoError>()
let tableViewIsEditing = TestObserver<Bool, NoError>()
private let vm = PaymentMethodsViewModel()
private let editButtonIsEnabled = TestObserver<Bool, NoError>()
private let errorLoadingPaymentMethods = TestObserver<String, NoError>()
private let goToAddCardScreen = TestObserver<Void, NoError>()
private let paymentMethods = TestObserver<[GraphUserCreditCard.CreditCard], NoError>()
private let presentBanner = TestObserver<String, NoError>()
private let reloadData = TestObserver<Void, NoError>()
private let showAlert = TestObserver<String, NoError>()
private let tableViewIsEditing = TestObserver<Bool, NoError>()

internal override func setUp() {
super.setUp()

self.vm.outputs.editButtonIsEnabled.observe(self.editButtonIsEnabled.observer)
self.vm.outputs.goToAddCardScreen.observe(self.goToAddCardScreen.observer)
self.vm.outputs.errorLoadingPaymentMethods.observe(self.errorLoadingPaymentMethods.observer)
self.vm.outputs.paymentMethods.observe(self.paymentMethods.observer)
self.vm.outputs.presentBanner.observe(self.presentBanner.observer)
self.vm.outputs.reloadData.observe(self.reloadData.observer)
Expand All @@ -45,7 +47,26 @@ internal final class PaymentMethodsViewModelTests: TestCase {
}
}

func testPaymentMethodsFetch_errorFetchingPaymentMethods() {
let error = GraphResponseError(message: "Something went wrong")
let apiService = MockService(fetchGraphCreditCardsError: GraphError.decodeError(error))

withEnvironment(apiService: apiService) {
self.vm.inputs.viewDidLoad()

self.reloadData.assertDidEmitValue()

self.vm.inputs.viewWillAppear()

self.scheduler.advance()

self.errorLoadingPaymentMethods.assertValue(error.message)
self.paymentMethods.assertDidNotEmitValue()
}
}

func testPaymentMethodsFetch_OnAddNewCardSucceeded() {

let response = UserEnvelope<GraphUserCreditCard>(me: GraphUserCreditCard.template)
let apiService = MockService(fetchGraphCreditCardsResponse: response)

Expand Down Expand Up @@ -133,7 +154,7 @@ internal final class PaymentMethodsViewModelTests: TestCase {
XCTFail("Card should exist")
return
}

let apiService = MockService(deletePaymentMethodResult: .success(.init(totalCount: 3)))
withEnvironment(apiService: apiService) {
self.editButtonIsEnabled.assertDidNotEmitValue()
Expand Down