-
Notifications
You must be signed in to change notification settings - Fork 132
[Woo POS] Update conditions for showing POS entry point #11513
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
Changes from 4 commits
db8a9b4
55d6a27
96fb32c
8cb5f81
faa680d
73e5780
23c62d4
2cc8719
32a4e39
b11439b
c708652
78a4290
84b21af
65e5751
32a3872
c6b929b
7806925
65c0098
682491d
cfa9d6b
535d184
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,35 @@ | ||
package com.woocommerce.android.ui.woopos | ||
|
||
import com.woocommerce.android.util.FeatureFlag | ||
import com.woocommerce.android.tools.SelectedSite | ||
import com.woocommerce.android.ui.payments.GetActivePaymentsPlugin | ||
import com.woocommerce.android.util.IsWindowClassExpandedAndBigger | ||
import org.wordpress.android.fluxc.model.payments.inperson.WCPaymentAccountResult | ||
import org.wordpress.android.fluxc.store.WCInPersonPaymentsStore | ||
import org.wordpress.android.fluxc.store.WCInPersonPaymentsStore.InPersonPaymentsPluginType.WOOCOMMERCE_PAYMENTS | ||
import javax.inject.Inject | ||
|
||
class IsWooPosEnabled @Inject constructor() { | ||
operator fun invoke() = FeatureFlag.WOO_POS.isEnabled() | ||
class IsWooPosEnabled @Inject constructor( | ||
private val selectedSite: SelectedSite, | ||
private val ippStore: WCInPersonPaymentsStore, | ||
private val getActivePaymentsPlugin: GetActivePaymentsPlugin, | ||
private val isWindowSizeExpandedAndBigger: IsWindowClassExpandedAndBigger, | ||
private val isWooPosFFEnabled: IsWooPosFFEnabled, | ||
) { | ||
@Suppress("ReturnCount") | ||
suspend operator fun invoke(): Boolean { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another important thing to notice - we have to check the flag first, otherwise all these checks will be performed even on the production build making it slower for no reason at the moment There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Even after we check FF first in here, we may still need to create a ticket to consider early background check + caching here. Currently, we do network requests, and the whole "more" screen doesn't show anything. Depending on the entry point we probably will need to reconsider the approach There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @samiuelson There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @samiuelson Oh I see. The VM initiated even before the fragment is opened. Then the caching probably makes sense even for debug builds - we just need to keep in mind that the app will have to be killed in case of a site change. I'd add a ticket to come back to this closer to the release - we will need cache invalidation here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, now the menu more screen should load faster even the first time because the IsWooPosEnabled-related requests are now executed in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool – I created the issue to test cache invalidation need later – #11520 |
||
val ippPlugin = getActivePaymentsPlugin() ?: return false | ||
val selectedSite = selectedSite.getOrNull() ?: return false | ||
val paymentAccount = ippStore.loadAccount(ippPlugin, selectedSite).model ?: return false | ||
val countryCode = paymentAccount.country | ||
|
||
return countryCode.lowercase() == "us" && | ||
ippPlugin == WOOCOMMERCE_PAYMENTS && | ||
paymentAccount.storeCurrencies.default.lowercase() == "usd" && | ||
isPluginSetupCompleted(paymentAccount) && | ||
isWindowSizeExpandedAndBigger() && | ||
isWooPosFFEnabled() | ||
} | ||
|
||
private fun isPluginSetupCompleted(paymentAccount: WCPaymentAccountResult): Boolean = | ||
paymentAccount.status != WCPaymentAccountResult.WCPaymentAccountStatus.NO_ACCOUNT | ||
samiuelson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
package com.woocommerce.android.ui.woopos | ||
|
||
import com.woocommerce.android.util.FeatureFlag | ||
import javax.inject.Inject | ||
|
||
class IsWooPosFFEnabled @Inject constructor() { | ||
operator fun invoke(): Boolean { | ||
return FeatureFlag.WOO_POS.isEnabled() | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -89,3 +89,7 @@ object UiHelpers { | |
class IsWindowClassLargeThanCompact @Inject constructor(val context: Context) { | ||
operator fun invoke() = context.windowSizeClass != WindowSizeClass.Compact | ||
} | ||
|
||
class IsWindowClassExpandedAndBigger @Inject constructor(val context: Context) { | ||
operator fun invoke() = context.windowSizeClass == WindowSizeClass.ExpandedAndBigger | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you know if small tablets or big phones e.g. 7-8 inch will be covered by that? Do we actually plan to support 7-8inch size devices? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current condition targets an 8" or larger diagonal (assuming a standard 16:10 ratio). I added a ticket to discuss it and adjust final values later - #11522 |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,111 @@ | ||
package com.woocommerce.android.ui.woopos | ||
|
||
import com.woocommerce.android.tools.SelectedSite | ||
import com.woocommerce.android.ui.payments.GetActivePaymentsPlugin | ||
import com.woocommerce.android.util.IsWindowClassExpandedAndBigger | ||
import com.woocommerce.android.viewmodel.BaseUnitTest | ||
import kotlinx.coroutines.ExperimentalCoroutinesApi | ||
import org.junit.Before | ||
import org.mockito.kotlin.any | ||
import org.mockito.kotlin.mock | ||
import org.mockito.kotlin.whenever | ||
import org.wordpress.android.fluxc.model.SiteModel | ||
import org.wordpress.android.fluxc.model.payments.inperson.WCPaymentAccountResult | ||
import org.wordpress.android.fluxc.model.payments.inperson.WCPaymentAccountResult.WCPaymentAccountStatus.COMPLETE | ||
import org.wordpress.android.fluxc.model.payments.inperson.WCPaymentAccountResult.WCPaymentAccountStatus.StoreCurrencies | ||
import org.wordpress.android.fluxc.network.rest.wpcom.wc.WooResult | ||
import org.wordpress.android.fluxc.store.WCInPersonPaymentsStore | ||
import org.wordpress.android.fluxc.store.WCInPersonPaymentsStore.InPersonPaymentsPluginType.STRIPE | ||
import org.wordpress.android.fluxc.store.WCInPersonPaymentsStore.InPersonPaymentsPluginType.WOOCOMMERCE_PAYMENTS | ||
import kotlin.test.Test | ||
import kotlin.test.assertFalse | ||
import kotlin.test.assertTrue | ||
|
||
@OptIn(ExperimentalCoroutinesApi::class) | ||
class IsWooPosEnabledTest : BaseUnitTest() { | ||
private val selectedSite: SelectedSite = mock() | ||
private val ippStore: WCInPersonPaymentsStore = mock() | ||
private val getActivePaymentsPlugin: GetActivePaymentsPlugin = mock() | ||
private val isWindowSizeExpandedAndBigger: IsWindowClassExpandedAndBigger = mock() | ||
|
||
private lateinit var sut: IsWooPosEnabled | ||
|
||
@Before | ||
fun setup() = testBlocking { | ||
whenever(selectedSite.getOrNull()).thenReturn(SiteModel()) | ||
whenever(getActivePaymentsPlugin()).thenReturn(WOOCOMMERCE_PAYMENTS) | ||
whenever(isWindowSizeExpandedAndBigger()).thenReturn(true) | ||
whenever(ippStore.loadAccount(any(), any())).thenReturn(buildPaymentAccountResult()) | ||
|
||
sut = IsWooPosEnabled( | ||
selectedSite = selectedSite, | ||
ippStore = ippStore, | ||
getActivePaymentsPlugin = getActivePaymentsPlugin, | ||
isWindowSizeExpandedAndBigger = isWindowSizeExpandedAndBigger | ||
) | ||
} | ||
|
||
@Test | ||
fun `given store not in the US, then return false`() = testBlocking { | ||
val result = buildPaymentAccountResult(countryCode = "CA") | ||
whenever(ippStore.loadAccount(any(), any())).thenReturn(result) | ||
assertFalse(sut()) | ||
} | ||
|
||
@Test | ||
fun `given not big enough screen, then return false`() = testBlocking { | ||
whenever(isWindowSizeExpandedAndBigger()).thenReturn(false) | ||
assertFalse(sut()) | ||
} | ||
|
||
@Test | ||
fun `given currency is not USD, then return false`() = testBlocking { | ||
val result = buildPaymentAccountResult(defaultCurrency = "CAD") | ||
whenever(ippStore.loadAccount(any(), any())).thenReturn(result) | ||
assertFalse(sut()) | ||
} | ||
|
||
@Test | ||
fun `given ipp plugin is not enabled, then return false`() = testBlocking { | ||
whenever(getActivePaymentsPlugin()).thenReturn(null) | ||
assertFalse(sut()) | ||
} | ||
|
||
@Test | ||
fun `given ipp plugin is not woo payments, then return false`() = testBlocking { | ||
whenever(getActivePaymentsPlugin()).thenReturn(STRIPE) | ||
assertFalse(sut()) | ||
} | ||
|
||
@Test | ||
fun `given woo payments setup not completed, then return false`() = testBlocking { | ||
val result = buildPaymentAccountResult(status = WCPaymentAccountResult.WCPaymentAccountStatus.NO_ACCOUNT) | ||
whenever(ippStore.loadAccount(any(), any())).thenReturn(result) | ||
assertFalse(sut()) | ||
} | ||
|
||
@Test | ||
fun `given big enough screen, woo payments enabled, USD currency and store in the US, then return true`() = testBlocking { | ||
val result = buildPaymentAccountResult(defaultCurrency = "USD", countryCode = "US", status = COMPLETE) | ||
whenever(ippStore.loadAccount(any(), any())).thenReturn(result) | ||
assertTrue(sut()) | ||
} | ||
|
||
private fun buildPaymentAccountResult( | ||
status: WCPaymentAccountResult.WCPaymentAccountStatus = COMPLETE, | ||
countryCode: String = "US", | ||
defaultCurrency: String = "USD" | ||
) = WooResult( | ||
WCPaymentAccountResult( | ||
status, | ||
hasPendingRequirements = false, | ||
hasOverdueRequirements = false, | ||
currentDeadline = null, | ||
statementDescriptor = "", | ||
storeCurrencies = StoreCurrencies(defaultCurrency, listOf(defaultCurrency)), | ||
country = countryCode, | ||
isLive = true, | ||
testMode = false, | ||
) | ||
) | ||
} |
Uh oh!
There was an error while loading. Please reload this page.