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

[PaymentSheet] Fix issue when used with hyperion and mochi #5321

Merged
merged 17 commits into from
Jul 26, 2022
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
8 changes: 6 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
# CHANGELOG

## X.X.X

### PaymentSheet
[FIXED][5321](https://github.com/stripe/stripe-android/pull/5321) Fixed issue with forever loading and mochi library.

### Payments
[Fixed][5308](https://github.com/stripe/stripe-android/pull/5308) OXXO so that processing is considered a successful terminal state, similar to Konbini and Boleto.
[Fixed][5138](https://github.com/stripe/stripe-android/pull/5138) Fixed an issue where PaymentSheet will show a failure even when 3DS2 Payment/SetupIntent is successful
[FIXED][5308](https://github.com/stripe/stripe-android/pull/5308) OXXO so that processing is considered a successful terminal state, similar to Konbini and Boleto.
[FIXED][5138](https://github.com/stripe/stripe-android/pull/5138) Fixed an issue where PaymentSheet will show a failure even when 3DS2 Payment/SetupIntent is successful

## 20.7.0 - 2022-07-06
* This release adds additional support for Afterpay/Clearpay in PaymentSheet.
Expand Down
2 changes: 2 additions & 0 deletions payments-ui-core/api/payments-ui-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,7 @@ public final class com/stripe/android/ui/core/elements/IdentifierSpec$Companion
public final fun getCardNumber ()Lcom/stripe/android/ui/core/elements/IdentifierSpec;
public final fun getCity ()Lcom/stripe/android/ui/core/elements/IdentifierSpec;
public final fun getCountry ()Lcom/stripe/android/ui/core/elements/IdentifierSpec;
public final fun getDependentLocality ()Lcom/stripe/android/ui/core/elements/IdentifierSpec;
public final fun getEmail ()Lcom/stripe/android/ui/core/elements/IdentifierSpec;
public final fun getLine1 ()Lcom/stripe/android/ui/core/elements/IdentifierSpec;
public final fun getLine2 ()Lcom/stripe/android/ui/core/elements/IdentifierSpec;
Expand All @@ -420,6 +421,7 @@ public final class com/stripe/android/ui/core/elements/IdentifierSpec$Companion
public final fun getPhone ()Lcom/stripe/android/ui/core/elements/IdentifierSpec;
public final fun getPostalCode ()Lcom/stripe/android/ui/core/elements/IdentifierSpec;
public final fun getSaveForFutureUse ()Lcom/stripe/android/ui/core/elements/IdentifierSpec;
public final fun getSortingCode ()Lcom/stripe/android/ui/core/elements/IdentifierSpec;
public final fun getState ()Lcom/stripe/android/ui/core/elements/IdentifierSpec;
public final fun serializer ()Lkotlinx/serialization/KSerializer;
}
Expand Down
9 changes: 1 addition & 8 deletions payments-ui-core/src/main/assets/addressinfo/CI.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,5 @@
"schema": {
"nameType": "city"
}
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

THis is a duplicate field, removing the duplicate

{
"type": "sortingCode",
"required": false,
"schema": {
"nameType": "cedex"
}
}
]
]
Original file line number Diff line number Diff line change
Expand Up @@ -92,16 +92,18 @@ class FieldValuesToParamsMapConverter {

@VisibleForTesting
internal fun addPath(map: MutableMap<String, Any?>, keys: List<String>, value: String?) {
val key = keys[0]
if (keys.size == 1) {
map[key] = value
} else {
var mapValueOfKey = map[key] as? MutableMap<String, Any?>
if (mapValueOfKey == null) {
mapValueOfKey = mutableMapOf()
map[key] = mapValueOfKey
if (keys.isNotEmpty()) {
val key = keys[0]
if (keys.size == 1) {
map[key] = value
} else {
var mapValueOfKey = map[key] as? MutableMap<String, Any?>
if (mapValueOfKey == null) {
mapValueOfKey = mutableMapOf()
map[key] = mapValueOfKey
}
addPath(mapValueOfKey, keys.subList(1, keys.size), value)
}
addPath(mapValueOfKey, keys.subList(1, keys.size), value)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,63 +12,81 @@ import com.stripe.android.ui.core.elements.SectionSingleFieldElement
import com.stripe.android.ui.core.elements.SimpleTextElement
import com.stripe.android.ui.core.elements.SimpleTextFieldConfig
import com.stripe.android.ui.core.elements.SimpleTextFieldController
import kotlinx.serialization.KSerializer
import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable
import kotlinx.serialization.decodeFromString
import kotlinx.serialization.descriptors.PrimitiveKind
import kotlinx.serialization.descriptors.PrimitiveSerialDescriptor
import kotlinx.serialization.descriptors.SerialDescriptor
import kotlinx.serialization.encoding.Decoder
import kotlinx.serialization.encoding.Encoder
import kotlinx.serialization.json.Json
import java.io.InputStream
import java.util.UUID

@Serializable(with = FieldTypeAsStringSerializer::class)
@Serializable
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored this so we don't require a special deserializer.

internal enum class FieldType(
val serializedValue: String,
val identifierSpec: IdentifierSpec,
@StringRes val defaultLabel: Int,
val capitalization: KeyboardCapitalization
@StringRes val defaultLabel: Int
) {
@SerialName("addressLine1")
AddressLine1(
"addressLine1",
IdentifierSpec.Line1,
R.string.address_label_address_line1,
KeyboardCapitalization.Words
R.string.address_label_address_line1
),

@SerialName("addressLine2")
AddressLine2(
"addressLine2",
IdentifierSpec.Line2,
R.string.address_label_address_line2,
KeyboardCapitalization.Words
R.string.address_label_address_line2
),

@SerialName("locality")
Locality(
"locality",
IdentifierSpec.City,
R.string.address_label_city,
KeyboardCapitalization.Words
R.string.address_label_city
),

@SerialName("dependentLocality")
DependentLocality(
"dependentLocality",
IdentifierSpec.DependentLocality,
R.string.address_label_city
),

@SerialName("postalCode")
PostalCode(
"postalCode",
IdentifierSpec.PostalCode,
R.string.address_label_postal_code,
KeyboardCapitalization.None
),
R.string.address_label_postal_code
) {
override fun capitalization() = KeyboardCapitalization.None
},

@SerialName("sortingCode")
SortingCode(
"sortingCode",
IdentifierSpec.SortingCode,
R.string.address_label_postal_code
) {
override fun capitalization() = KeyboardCapitalization.None
},

@SerialName("administrativeArea")
AdministrativeArea(
"administrativeArea",
IdentifierSpec.State,
NameType.State.stringResId,
KeyboardCapitalization.Words
NameType.State.stringResId
),

@SerialName("name")
Name(
"name",
IdentifierSpec.Name,
R.string.address_label_name,
KeyboardCapitalization.Words
R.string.address_label_name
);

open fun capitalization() = KeyboardCapitalization.Words

companion object {
fun from(value: String) = values().firstOrNull {
it.serializedValue == value
Expand Down Expand Up @@ -167,7 +185,7 @@ internal class FieldSchema(
@SerialName("isNumeric")
val isNumeric: Boolean = false,
@SerialName("examples")
val examples: List<String> = emptyList(),
val examples: ArrayList<String> = arrayListOf(),
@SerialName("nameType")
val nameType: NameType // label,
)
Expand All @@ -186,43 +204,35 @@ private val format = Json { ignoreUnknownKeys = true }

internal fun parseAddressesSchema(inputStream: InputStream?) =
getJsonStringFromInputStream(inputStream)?.let {
format.decodeFromString<List<CountryAddressSchema>>(
format.decodeFromString<ArrayList<CountryAddressSchema>>(
it
)
}

private object FieldTypeAsStringSerializer : KSerializer<FieldType?> {
override val descriptor: SerialDescriptor =
PrimitiveSerialDescriptor("FieldType", PrimitiveKind.STRING)

override fun serialize(encoder: Encoder, value: FieldType?) {
encoder.encodeString(value?.serializedValue ?: "")
}

override fun deserialize(decoder: Decoder): FieldType? {
return FieldType.from(decoder.decodeString())
}
}

private fun getJsonStringFromInputStream(inputStream: InputStream?) =
inputStream?.bufferedReader().use { it?.readText() }

internal fun List<CountryAddressSchema>.transformToElementList(): List<SectionFieldElement> {
val countryAddressElements = this.mapNotNull { addressField ->
addressField.type?.let {
SimpleTextElement(
addressField.type.identifierSpec,
SimpleTextFieldController(
SimpleTextFieldConfig(
label = addressField.schema?.nameType?.stringResId ?: it.defaultLabel,
capitalization = it.capitalization,
keyboard = getKeyboard(addressField.schema)
),
showOptionalLabel = !addressField.required
val countryAddressElements = this
.filterNot {
it.type == FieldType.SortingCode ||
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are two new types that are valid, but we don't support showing.

it.type == FieldType.DependentLocality
}
.mapNotNull { addressField ->
addressField.type?.let {
SimpleTextElement(
addressField.type.identifierSpec,
SimpleTextFieldController(
SimpleTextFieldConfig(
label = addressField.schema?.nameType?.stringResId ?: it.defaultLabel,
capitalization = it.capitalization(),
keyboard = getKeyboard(addressField.schema)
),
showOptionalLabel = !addressField.required
)
)
)
}
}
}

// Put it in a single row
return combineCityAndPostal(countryAddressElements)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,13 @@ data class IdentifierSpec(val v1: String) {

val City = IdentifierSpec("billing_details[address][city]")

// FieldValuesToParamsMapConverter will ignore this in the parameter list
val DependentLocality = IdentifierSpec("")

val PostalCode = IdentifierSpec("billing_details[address][postal_code]")

val SortingCode = IdentifierSpec("")

val State = IdentifierSpec("billing_details[address][state]")

val Country = IdentifierSpec("billing_details[address][country]")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ internal class LpmSerializer {
emptyList()
} else {
try {
format.decodeFromString<List<SharedDataSpec>>(serializer(), str)
format.decodeFromString<ArrayList<SharedDataSpec>>(serializer(), str)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ArrayList works for finding the deserializer

} catch (e: Exception) {
Log.w("STRIPE", "Error parsing LPMs", e)
emptyList()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@ internal data class SharedDataSpec(
// If a form is empty, it must still have an EmptyFormSpec
// field to get the form into a complete state (i.e. PayPal).
@SerialName("fields")
val fields: List<FormItemSpec> = listOf(EmptyFormSpec)
val fields: ArrayList<FormItemSpec> = arrayListOf(EmptyFormSpec)
)
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,10 @@ class LpmRepository constructor(
serverLpmSpecs: String?,
force: Boolean = false
) {
if (!isLoaded() || force) {
// If the expectedLpms is different form last time, we still need to reload.
var lpmsNotParsedFromServerSpec = expectedLpms
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we get passed two different PMs with different values, we might need to reparse in the library to deserialize the new LPMs that we need.

.filter { !codeToSupportedPaymentMethod.containsKey(it) }
if (!isLoaded() || force || lpmsNotParsedFromServerSpec.isNotEmpty()) {
serverSpecLoadingState = ServerSpecState.NoServerSpec(serverLpmSpecs)
if (!serverLpmSpecs.isNullOrEmpty()) {
serverSpecLoadingState = ServerSpecState.ServerNotParsed(serverLpmSpecs)
Expand All @@ -116,7 +119,7 @@ class LpmRepository constructor(

// If the server does not return specs, or they are not parsed successfully
// we will use the LPM on disk if found
val lpmsNotParsedFromServerSpec = expectedLpms
lpmsNotParsedFromServerSpec = expectedLpms
.filter { !codeToSupportedPaymentMethod.containsKey(it) }
if (lpmsNotParsedFromServerSpec.isNotEmpty()) {
val mapFromDisk: Map<String, SharedDataSpec>? =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,23 @@ class TransformAddressToElementTest {
}
}

@Test
fun `Make sure sorting code and dependent locality is never required`() {
// Sorting code and dependent locality are not actually sent to the server.
supportedCountries.forEach { countryCode ->
val schemaList = readFile("src/main/assets/addressinfo/$countryCode.json")
val invalidNameType = schemaList?.filter { addressSchema ->
addressSchema.required &&
(
addressSchema.type == FieldType.SortingCode ||
addressSchema.type == FieldType.DependentLocality
)
}
invalidNameType?.forEach { println(it.type?.name) }
assertThat(invalidNameType).isEmpty()
}
}

@Test
fun `Make sure all country code json files are serializable`() {
supportedCountries.forEach { countryCode ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,12 @@ internal class PaymentSheetActivity : BaseSheetActivity<PaymentSheetResult>() {
if (config != null) {
// We only want to do this if the loading fragment is shown. Otherwise this causes
// a new fragment to be created if the activity was destroyed and recreated.
if (supportFragmentManager.fragments.firstOrNull() is PaymentSheetLoadingFragment) {
// If hyperion is an added dependency it is loaded on top of the
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the fix for when hyperion is a dependency

// PaymentSheetLoadingFragment
if (supportFragmentManager.fragments
.filterIsInstance<PaymentSheetLoadingFragment>()
.isNotEmpty()
) {
val target = if (viewModel.paymentMethods.value.isNullOrEmpty()) {
viewModel.updateSelection(null)
PaymentSheetViewModel.TransitionTarget.AddPaymentMethodSheet(config)
Expand Down