Support CollectData feature #765
Conversation
EricLin-BBpos
requested review from
billfinn-stripe,
bric-stripe,
nazli-stripe,
ugochukwu-stripe,
ebarrenechea-stripe and
mshafrir-stripe
as code owners
There was a problem hiding this comment.
Looks good on the Android and TS side of things. @bric-stripe could you check the iOS changes? 🙇
ios/Mappers.swift
Outdated
| switch type { | ||
| case "unknown": return CollectDataType.unknown | ||
| case "magstripe": return CollectDataType.magstripe | ||
| default: return CollectDataType.unknown |
There was a problem hiding this comment.
preference for unknown default so we get warnings/errors when new enum values are added in the native SDK
and I think we should return nil for the default case since this is marked nullable and null is handled in the caller
| default: return CollectDataType.unknown | |
| @unknown default: return nil |
There was a problem hiding this comment.
emm..it seems that @unknown cannot be used here because it can only be utilized in switch statements within enumeration types.
There was a problem hiding this comment.
ah, that makes sense I didn't think about that since this is going back from strings. ignore that bit then 👍
There was a problem hiding this comment.
I think its still worth returning nil though so we get a RN side error when the input is validated
ios/StripeTerminalReactNative.swift
Outdated
| } else { | ||
| resolve(collectedData != nil ? Mappers.mapFromCollectedData(collectedData!) : [:]) | ||
| } |
There was a problem hiding this comment.
nit but I think this reads a little better and avoids the force unwrap
| } else { | |
| resolve(collectedData != nil ? Mappers.mapFromCollectedData(collectedData!) : [:]) | |
| } | |
| } else if let collectedData { | |
| resolve(Mappers.mapFromCollectedData(collectedData)) | |
| } else { | |
| // Unexpected | |
| resolve([:]) | |
| } |
|
|
||
| fun mapFromCollectedData(collectData: CollectedData): ReadableMap { | ||
| return nativeMapOf { | ||
| putString("stripeId",collectData.id) |
There was a problem hiding this comment.
formatting nit:
| putString("stripeId",collectData.id) | |
| putString("stripeId", collectData.id) |
| } | ||
|
|
||
| fun mapFromCollectDataType(type: String): CollectDataType? { | ||
| return when(type) { |
There was a problem hiding this comment.
formatting nit:
| return when(type) { | |
| return when (type) { |
| val collectDataType = requireParam(mapFromCollectDataType(collectDataTypeParam)) { | ||
| "Unknown collectDataType: $collectDataTypeParam" | ||
| } | ||
| val enableCustomerCancellation = getBoolean(params,"enableCustomerCancellation") |
There was a problem hiding this comment.
formatting nit:
| val enableCustomerCancellation = getBoolean(params,"enableCustomerCancellation") | |
| val enableCustomerCancellation = getBoolean(params, "enableCustomerCancellation") |
ios/StripeTerminalReactNative.swift
Outdated
|
|
||
| let collectDataType = Mappers.mapToCollectDataType(collectDataTypeParam) | ||
| guard let collectDataType else { | ||
| resolve(Errors.createError(code: CommonErrorType.InvalidRequiredParameter, message: "You must provide correct collectDataType parameter.")) |
There was a problem hiding this comment.
would recommend keeping this the same as android
| resolve(Errors.createError(code: CommonErrorType.InvalidRequiredParameter, message: "You must provide correct collectDataType parameter.")) | |
| resolve(Errors.createError(code: CommonErrorType.InvalidRequiredParameter, message: "You must provide a collectDataType")) |
|
|
||
| let collectDataConfig: CollectDataConfiguration | ||
| do { | ||
| collectDataConfig = try CollectDataConfigurationBuilder().setCollectDataType(collectDataType) |
There was a problem hiding this comment.
enableCustomerCancellation is missing here
There was a problem hiding this comment.
enableCustomerCancellation seem have not support in IOS SDK yet
There was a problem hiding this comment.
oh! thanks for pointing that out 👍
src/types/index.ts
Outdated
|
|
||
| export enum CollectDataType { | ||
| MAGSTRIPE = 'magstripe', | ||
| UNKNOW = 'unknown', |
There was a problem hiding this comment.
typo:
| UNKNOW = 'unknown', | |
| UNKNOWN = 'unknown', |
src/types/index.ts
Outdated
| @@ -490,3 +490,29 @@ export type AmountDetails = { | |||
| export type Amount = { | |||
| amount: number; | |||
| }; | |||
|
|
|||
| export type CollectData = { | |||
There was a problem hiding this comment.
typo - this should match the iOS and Android types:
| export type CollectData = { | |
| export type CollectedData = { |
Summary
Add collectData feature in sdk
Motivation
Support new collectData feature
Testing
Documentation
Select one: