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] CVC bug fix #574

Merged
merged 14 commits into from
Feb 8, 2019
8 changes: 8 additions & 0 deletions Kickstarter.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1075,6 +1075,8 @@
D77744C1217A47D5008D679F /* SettingsCurrencyCellViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7774483217A465D008D679F /* SettingsCurrencyCellViewModelTests.swift */; };
D78E4E482188CB4300E99295 /* UIButton+HapticFeedback.swift in Sources */ = {isa = PBXBuildFile; fileRef = D78E4E472188CB4300E99295 /* UIButton+HapticFeedback.swift */; };
D78E4EE3218909DC00E99295 /* UIFeedbackGenerator+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = D78E4EE2218909DC00E99295 /* UIFeedbackGenerator+Extensions.swift */; };
D79440572203A63300D0A747 /* CreatePaymentSourceEnvelope.swift in Sources */ = {isa = PBXBuildFile; fileRef = D79440562203A63300D0A747 /* CreatePaymentSourceEnvelope.swift */; };
D79440902208970E00D0A747 /* CreatePaymentSourceTemplate.swift in Sources */ = {isa = PBXBuildFile; fileRef = D794408F2208970E00D0A747 /* CreatePaymentSourceTemplate.swift */; };
D796867C20FE655300E54C61 /* SettingsFollowCellViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = D796867B20FE655300E54C61 /* SettingsFollowCellViewModel.swift */; };
D796867E20FF910300E54C61 /* SettingsPrivacyDataSource.swift in Sources */ = {isa = PBXBuildFile; fileRef = D796867D20FF910300E54C61 /* SettingsPrivacyDataSource.swift */; };
D796868021013C1F00E54C61 /* SettingsPrivacyStaticCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = D796867F21013C1F00E54C61 /* SettingsPrivacyStaticCell.swift */; };
Expand Down Expand Up @@ -2811,6 +2813,8 @@
D78E4E472188CB4300E99295 /* UIButton+HapticFeedback.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UIButton+HapticFeedback.swift"; sourceTree = "<group>"; };
D78E4EE2218909DC00E99295 /* UIFeedbackGenerator+Extensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UIFeedbackGenerator+Extensions.swift"; sourceTree = "<group>"; };
D79076F8207BC161008014EC /* CrossDissolveTransitionAnimator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CrossDissolveTransitionAnimator.swift; sourceTree = "<group>"; };
D79440562203A63300D0A747 /* CreatePaymentSourceEnvelope.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CreatePaymentSourceEnvelope.swift; sourceTree = "<group>"; };
D794408F2208970E00D0A747 /* CreatePaymentSourceTemplate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CreatePaymentSourceTemplate.swift; sourceTree = "<group>"; };
D796867B20FE655300E54C61 /* SettingsFollowCellViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsFollowCellViewModel.swift; sourceTree = "<group>"; };
D796867D20FF910300E54C61 /* SettingsPrivacyDataSource.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsPrivacyDataSource.swift; sourceTree = "<group>"; };
D796867F21013C1F00E54C61 /* SettingsPrivacyStaticCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsPrivacyStaticCell.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -4281,6 +4285,7 @@
D01587931EEB2ED6006E7684 /* FriendStatsEnvelope.swift */,
D01587941EEB2ED6006E7684 /* FriendStatsEnvelopeTests.swift */,
D002CAE4218CF951009783F2 /* GraphMutationWatchProjectResponseEnvelope.swift */,
D79440562203A63300D0A747 /* CreatePaymentSourceEnvelope.swift */,
D6ED1B38216D50BE007F7547 /* UserEmailFields.swift */,
D01587951EEB2ED6006E7684 /* Item.swift */,
D01587961EEB2ED6006E7684 /* ItemTests.swift */,
Expand Down Expand Up @@ -4420,6 +4425,7 @@
D01587FB1EEB2ED7006E7684 /* FriendStatsEnvelopeTemplates.swift */,
D6ED38A11F796FE5006CAAE9 /* GraphCategoriesEnvelopeTemplate.swift */,
D08CD20221922025009F89F0 /* GraphMutationWatchProjectResponseEnvelopeTemplates.swift */,
D794408F2208970E00D0A747 /* CreatePaymentSourceTemplate.swift */,
D63BBD32217F9CDE007E01F0 /* GraphCreditCardTemplate.swift */,
D6DC65512178CDDC008CF69C /* GraphUserEmailTemplate.swift */,
D01587FC1EEB2ED7006E7684 /* ItemTemplates.swift */,
Expand Down Expand Up @@ -6392,6 +6398,7 @@
D01588AD1EEB2ED7006E7684 /* Project.StatsLenses.swift in Sources */,
D0158A211EEB30A2006E7684 /* ProjectStatsEnvelope.VideoStatsTemplates.swift in Sources */,
D0158A141EEB30A2006E7684 /* FindFriendsEnvelopeTemplates.swift in Sources */,
D79440902208970E00D0A747 /* CreatePaymentSourceTemplate.swift in Sources */,
D01589991EEB2ED7006E7684 /* ServerConfig.swift in Sources */,
D01588931EEB2ED7006E7684 /* DiscoveryParamsLenses.swift in Sources */,
D01588AB1EEB2ED7006E7684 /* Project.PhotoLenses.swift in Sources */,
Expand Down Expand Up @@ -6503,6 +6510,7 @@
D0158A1A1EEB30A2006E7684 /* Project.PhotoTemplates.swift in Sources */,
D01588B51EEB2ED7006E7684 /* ProjectNotificationLenses.swift in Sources */,
D015882B1EEB2ED7006E7684 /* NSURLSession.swift in Sources */,
D79440572203A63300D0A747 /* CreatePaymentSourceEnvelope.swift in Sources */,
D08CD1F921910DF5009F89F0 /* UnwatchProjectMutation.swift in Sources */,
D6AE52281FD1B4BA00BEC788 /* CategoryEnvelope.swift in Sources */,
D6ED386A1F796C26006CAAE9 /* GraphCategoriesEnvelopeLenses.swift in Sources */,
Expand Down
19 changes: 8 additions & 11 deletions KsApi/MockService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@ internal struct MockService: ServiceType {
internal let currency: String
internal let buildVersion: String

fileprivate let addNewCreditCardError: GraphError?
fileprivate let addNewCreditCardResult: Result<CreatePaymentSourceEnvelope, GraphError>?
cdolm92 marked this conversation as resolved.
Show resolved Hide resolved

fileprivate let changeCurrencyResponse: GraphMutationEmptyResponseEnvelope?

fileprivate let changeCurrencyError: GraphError?

fileprivate let changeEmailError: GraphError?
Expand Down Expand Up @@ -182,7 +181,7 @@ internal struct MockService: ServiceType {
language: String = "en",
currency: String = "USD",
buildVersion: String = "1",
addNewCreditCardError: GraphError? = nil,
addNewCreditCardResult: Result<CreatePaymentSourceEnvelope, GraphError>? = nil,
changeEmailError: GraphError? = nil,
changeEmailResponse: UserEnvelope<UserEmailFields>? = UserEnvelope<UserEmailFields>(
me: .template
Expand Down Expand Up @@ -283,7 +282,7 @@ internal struct MockService: ServiceType {
self.currency = currency
self.buildVersion = buildVersion

self.addNewCreditCardError = addNewCreditCardError
self.addNewCreditCardResult = addNewCreditCardResult

self.changeCurrencyResponse = changeCurrencyResponse
self.changeCurrencyError = changeCurrencyError
Expand Down Expand Up @@ -469,13 +468,10 @@ internal struct MockService: ServiceType {
self.watchProjectMutationResult = watchProjectMutationResult
}

internal func addNewCreditCard(input: CreatePaymentSourceInput)
-> SignalProducer<GraphMutationEmptyResponseEnvelope, GraphError> {
if let error = self.addNewCreditCardError {
return SignalProducer(error: error)
} else {
return SignalProducer(value: GraphMutationEmptyResponseEnvelope())
}
public func addNewCreditCard(input: CreatePaymentSourceInput)
-> SignalProducer<CreatePaymentSourceEnvelope, GraphError> {

return producer(for: addNewCreditCardResult)
}

internal func changeEmail(input: ChangeEmailInput) ->
Expand Down Expand Up @@ -1382,6 +1378,7 @@ private extension MockService {
oauthToken: $0,
language: $1.language,
buildVersion: $1.buildVersion,
addNewCreditCardResult: $1.addNewCreditCardResult,
changePaymentMethodResult: $1.changePaymentMethodResult,
deletePaymentMethodResult: $1.deletePaymentMethodResult,
createPledgeResult: $1.createPledgeResult,
Expand Down
3 changes: 2 additions & 1 deletion KsApi/Service.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public extension Bundle {
A `ServerType` that requests data from an API webservice.
*/
public struct Service: ServiceType {

public let appId: String
public let serverConfig: ServerConfigType
public let oauthToken: OauthTokenAuthType?
Expand Down Expand Up @@ -61,7 +62,7 @@ public struct Service: ServiceType {
}

public func addNewCreditCard(input: CreatePaymentSourceInput)
-> SignalProducer<GraphMutationEmptyResponseEnvelope, GraphError> {
-> SignalProducer<CreatePaymentSourceEnvelope, GraphError> {
return applyMutation(mutation: CreatePaymentSourceMutation(input: input))
}

Expand Down
2 changes: 1 addition & 1 deletion KsApi/ServiceType.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public protocol ServiceType {
SignalProducer<GraphMutationEmptyResponseEnvelope, GraphError>

func addNewCreditCard(input: CreatePaymentSourceInput) ->
SignalProducer<GraphMutationEmptyResponseEnvelope, GraphError>
SignalProducer<CreatePaymentSourceEnvelope, GraphError>

func changePaymentMethod(project: Project)
-> SignalProducer<ChangePaymentMethodEnvelope, ErrorEnvelope>
Expand Down
21 changes: 21 additions & 0 deletions KsApi/models/CreatePaymentSourceEnvelope.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import Foundation

public struct CreatePaymentSourceEnvelope: Decodable {
public private(set) var createPaymentSource: CreatePaymentSource

public struct CreatePaymentSource: Decodable {
public private(set) var errorMessage: String?
public private(set) var isSuccessful: Bool
}
}

extension CreatePaymentSourceEnvelope {
private enum CodingKeys: String, CodingKey {
case createPaymentSource
}

public init(from decoder: Decoder) throws {
let values = try decoder.container(keyedBy: CodingKeys.self)
self.createPaymentSource = try values.decode(CreatePaymentSource.self, forKey: .createPaymentSource)
}
}
7 changes: 7 additions & 0 deletions KsApi/models/templates/CreatePaymentSourceTemplate.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import Foundation

extension CreatePaymentSourceEnvelope {
internal static let paymentSourceSuccessTemplate = CreatePaymentSourceEnvelope(
createPaymentSource: .init(errorMessage: nil, isSuccessful: true)
)
}
2 changes: 2 additions & 0 deletions KsApi/mutations/CreatePaymentsSourceMutation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ public struct CreatePaymentSourceMutation<T: GraphMutationInput>: GraphMutation
mutation createPaymentSource($input: CreatePaymentSourceInput!) {
createPaymentSource(input: $input) {
clientMutationId\
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@cdolm92 cdolm92 Feb 6, 2019

Choose a reason for hiding this comment

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

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.

errorMessage
isSuccessful
}
}
"""
Expand Down
21 changes: 15 additions & 6 deletions Library/ViewModels/AddNewCardViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -99,27 +99,36 @@ AddNewCardViewModelOutputs {
let addNewCardEvent = self.stripeTokenProperty.signal.skipNil()
.map { CreatePaymentSourceInput(paymentType: PaymentType.creditCard,
stripeToken: $0.0, stripeCardId: $0.1) }
.flatMap {
.switchMap {
AppEnvironment.current.apiService.addNewCreditCard(input: $0)
.ksr_delay(AppEnvironment.current.apiDelayInterval, on: AppEnvironment.current.scheduler)
.map { (envelope: CreatePaymentSourceEnvelope) in envelope.createPaymentSource }
.materialize()
}

self.addNewCardSuccess = addNewCardEvent.values().ignoreValues()
.map { _ in Strings.Got_it_your_changes_have_been_saved() }

let stripeInvalidToken = self.stripeErrorProperty.signal.map {
$0?.localizedDescription
}.skipNil()
let graphError = addNewCardEvent.errors().map {
$0.localizedDescription
}
let addNewError = addNewCardEvent.map { $0.value?.errorMessage }.skipNil()
cdolm92 marked this conversation as resolved.
Show resolved Hide resolved

self.addNewCardFailure = Signal.merge (
let errorMessage = Signal.merge (
stripeInvalidToken,
graphError
graphError,
addNewError
)

self.addNewCardFailure = errorMessage.map { $0 }

let cardAddedSuccessfully = addNewCardEvent
.filter { $0.value?.isSuccessful == true }
.mapConst(true)

self.addNewCardSuccess = cardAddedSuccessfully
.map { _ in Strings.Got_it_your_changes_have_been_saved() }

self.activityIndicatorShouldShow = Signal.merge(
self.saveButtonTappedProperty.signal.mapConst(true),
self.addNewCardSuccess.mapConst(false),
Expand Down
51 changes: 30 additions & 21 deletions Library/ViewModels/AddNewCardViewModelTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -44,24 +44,30 @@ internal final class AddNewCardViewModelTests: TestCase {
}

func testAddCard_Success() {
self.vm.inputs.viewDidLoad()
self.vm.inputs.cardholderNameChanged("Native Squad")
self.vm.inputs.cardholderNameTextFieldReturn()
self.paymentDetailsBecomeFirstResponder.assertDidEmitValue()
self.vm.inputs.creditCardChanged(cardDetails: ("4242 4242 4242 4242", 11, 99, "123"))
self.vm.inputs.paymentInfo(isValid: true)
self.vm.inputs.cardBrand(isValid: true)
self.saveButtonIsEnabled.assertValues([true])
withEnvironment(
apiService: MockService(addNewCreditCardResult: .success(.paymentSourceSuccessTemplate))
) {
self.vm.inputs.viewDidLoad()
self.vm.inputs.cardholderNameChanged("Native Squad")
self.vm.inputs.cardholderNameTextFieldReturn()
self.paymentDetailsBecomeFirstResponder.assertDidEmitValue()
self.vm.inputs.creditCardChanged(cardDetails: ("4242 4242 4242 4242", 11, 99, "123"))

self.vm.inputs.saveButtonTapped()
self.activityIndicatorShouldShow.assertValues([true])
self.vm.inputs.paymentInfo(isValid: true)
self.vm.inputs.cardBrand(isValid: true)

self.saveButtonIsEnabled.assertValues([true])

self.vm.inputs.stripeCreated("stripe_deadbeef", stripeID: "stripe_deadbeefID")
self.vm.inputs.saveButtonTapped()
self.activityIndicatorShouldShow.assertValues([true])

self.scheduler.advance()
self.vm.inputs.stripeCreated("stripe_deadbeef", stripeID: "stripe_deadbeefID")

self.addNewCardSuccess.assertValues([Strings.Got_it_your_changes_have_been_saved()])
self.activityIndicatorShouldShow.assertValues([true, false])
self.scheduler.advance()

self.addNewCardSuccess.assertValues([Strings.Got_it_your_changes_have_been_saved()])
self.activityIndicatorShouldShow.assertValues([true, false])
}
}

func testAddCardFailure_InvalidToken() {
Expand Down Expand Up @@ -90,7 +96,7 @@ internal final class AddNewCardViewModelTests: TestCase {
func testAddCardFailure_GraphError() {
let error = GraphError.emptyResponse(nil)

withEnvironment(apiService: MockService(addNewCreditCardError: error)) {
withEnvironment(apiService: MockService(addNewCreditCardResult: .failure(.emptyResponse(nil)))) {
self.vm.inputs.viewDidLoad()
self.vm.inputs.cardholderNameChanged("Native Squad")
self.vm.inputs.cardholderNameTextFieldReturn()
Expand Down Expand Up @@ -213,17 +219,20 @@ internal final class AddNewCardViewModelTests: TestCase {
}

func testTrackSavedPaymentMethod() {
self.vm.inputs.paymentInfo(isValid: true)
self.vm.inputs.stripeCreated("stripe_deadbeef", stripeID: "stripe_deadbeefID")
withEnvironment(
apiService: MockService(addNewCreditCardResult: .success(.paymentSourceSuccessTemplate))
) {
self.vm.inputs.paymentInfo(isValid: true)
self.vm.inputs.stripeCreated("stripe_deadbeef", stripeID: "stripe_deadbeefID")

self.scheduler.advance()
self.scheduler.advance()

XCTAssertEqual(["Saved Payment Method"], self.trackingClient.events)
XCTAssertEqual(["Saved Payment Method"], self.trackingClient.events)
}
}

func testTrackFailedPaymentMethodCreation() {
let error = GraphError.emptyResponse(nil)
withEnvironment(apiService: MockService(addNewCreditCardError: error)) {
withEnvironment(apiService: MockService(addNewCreditCardResult: .failure(.emptyResponse(nil)))) {
self.vm.inputs.stripeCreated("stripe_deadbeef", stripeID: "stripe_deadbeefID")

self.scheduler.advance()
Expand Down