Skip to content

Commit

Permalink
Fix off by 1 error in PaymentMethodSwipeCallback (#2141)
Browse files Browse the repository at this point in the history
When the Google Pay button is enabled in PaymentMethodsActivity,
`PaymentMethodSwipeCallback#onSwiped()` would get the wrong
element from the `paymentMethods` list.
  • Loading branch information
mshafrir-stripe authored Feb 5, 2020
1 parent 19ce42b commit eec7ee1
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ internal class PaymentMethodSwipeCallback(
}

override fun onSwiped(viewHolder: RecyclerView.ViewHolder, direction: Int) {
val paymentMethod = adapter.paymentMethods[viewHolder.adapterPosition]
val paymentMethod = adapter.getPaymentMethodAtPosition(viewHolder.adapterPosition)
listener.onSwiped(paymentMethod)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ internal class PaymentMethodsAdapter constructor(
return ViewHolder.GooglePayViewHolder(itemView)
}

@JvmSynthetic
internal fun deletePaymentMethod(paymentMethod: PaymentMethod) {
val indexToDelete = paymentMethods.indexOfFirst { it.id == paymentMethod.id }
if (indexToDelete >= 0) {
Expand All @@ -200,21 +201,38 @@ internal class PaymentMethodsAdapter constructor(
}
}

@JvmSynthetic
internal fun resetPaymentMethod(paymentMethod: PaymentMethod) {
val indexToReset = paymentMethods.indexOfFirst { it.id == paymentMethod.id }
if (indexToReset >= 0) {
notifyItemChanged(indexToReset)
getPosition(paymentMethod)?.let {
notifyItemChanged(it)
}
}

private fun getPaymentMethodAtPosition(position: Int): PaymentMethod {
return paymentMethods[getPaymentMethodPosition(position)]
/**
* Given an adapter position, translate to a `paymentMethods` element
*/
@JvmSynthetic
internal fun getPaymentMethodAtPosition(position: Int): PaymentMethod {
return paymentMethods[getPaymentMethodIndex(position)]
}

private fun getPaymentMethodPosition(position: Int): Int {
/**
* Given an adapter position, translate to a `paymentMethods` index
*/
private fun getPaymentMethodIndex(position: Int): Int {
return position - googlePayCount
}

/**
* Given a Payment Method, get its adapter position. For example, if the Google Pay button is
* being shown, the 2nd element in [paymentMethods] is actually the 3rd item in the adapter.
*/
internal fun getPosition(paymentMethod: PaymentMethod): Int? {
return paymentMethods.indexOf(paymentMethod).takeIf { it >= 0 }?.let {
it + googlePayCount
}
}

private fun getAddableTypesPosition(position: Int): Int {
return position - paymentMethods.size - googlePayCount
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import android.content.Context
import android.widget.FrameLayout
import androidx.recyclerview.widget.RecyclerView
import androidx.test.core.app.ApplicationProvider
import com.nhaarman.mockitokotlin2.mock
import com.nhaarman.mockitokotlin2.verify
import com.stripe.android.model.PaymentMethod
import com.stripe.android.model.PaymentMethodFixtures
Expand All @@ -13,21 +14,16 @@ import kotlin.test.assertEquals
import kotlin.test.assertNotNull
import kotlin.test.assertNull
import org.junit.runner.RunWith
import org.mockito.Mock
import org.mockito.Mockito.times
import org.mockito.MockitoAnnotations
import org.robolectric.RobolectricTestRunner

/**
* Test class for [PaymentMethodsAdapter]
*/
@RunWith(RobolectricTestRunner::class)
class PaymentMethodsAdapterTest {
@Mock
private lateinit var adapterDataObserver: RecyclerView.AdapterDataObserver

@Mock
private lateinit var listener: PaymentMethodsAdapter.Listener
private val adapterDataObserver: RecyclerView.AdapterDataObserver = mock()
private val listener: PaymentMethodsAdapter.Listener = mock()

private val paymentMethodsAdapter: PaymentMethodsAdapter = PaymentMethodsAdapter(ARGS)

Expand All @@ -37,7 +33,6 @@ class PaymentMethodsAdapterTest {

@BeforeTest
fun setup() {
MockitoAnnotations.initMocks(this)
paymentMethodsAdapter.registerAdapterDataObserver(adapterDataObserver)
}

Expand Down Expand Up @@ -235,6 +230,25 @@ class PaymentMethodsAdapterTest {
verify(listener).onGooglePayClick()
}

@Test
fun getPosition_withValidPaymentMethod_returnsPosition() {
val adapter = PaymentMethodsAdapter(ARGS, shouldShowGooglePay = true)
adapter.setPaymentMethods(PaymentMethodFixtures.CARD_PAYMENT_METHODS)

assertEquals(
3,
adapter.getPosition(PaymentMethodFixtures.CARD_PAYMENT_METHODS.last())
)
}

@Test
fun getPosition_withInvalidPaymentMethod_returnsNull() {
val adapter = PaymentMethodsAdapter(ARGS, shouldShowGooglePay = true)
adapter.setPaymentMethods(PaymentMethodFixtures.CARD_PAYMENT_METHODS)

assertNull(adapter.getPosition(PaymentMethodFixtures.FPX_PAYMENT_METHOD))
}

private companion object {
private val ARGS =
PaymentMethodsActivityStarter.Args.Builder()
Expand Down

0 comments on commit eec7ee1

Please sign in to comment.