Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import com.woocommerce.android.ui.orders.wooshippinglabels.packages.ui.Carrier
import com.woocommerce.android.ui.orders.wooshippinglabels.packages.ui.CarrierPackageGroup
import com.woocommerce.android.ui.orders.wooshippinglabels.packages.ui.PackageData
import com.woocommerce.android.ui.orders.wooshippinglabels.packages.ui.StoreOptionsForPackages
import com.woocommerce.android.util.FeatureFlag
import com.woocommerce.android.util.FeatureFlagRepository
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.transformLatest
Expand All @@ -18,7 +20,8 @@ import javax.inject.Inject
class ObserveShippingPackages @Inject constructor(
private val selectedSite: SelectedSite,
private val packageRepository: WooShippingLabelPackageRepository,
private val fetchShippingPackages: FetchShippingPackages
private val fetchShippingPackages: FetchShippingPackages,
private val featureFlagRepository: FeatureFlagRepository
) {
@OptIn(ExperimentalCoroutinesApi::class)
operator fun invoke(): Flow<PackagesState> = packageRepository.observeShippingPackages(selectedSite.get())
Expand Down Expand Up @@ -56,9 +59,11 @@ class ObserveShippingPackages @Inject constructor(
.takeIf { it.isNotEmpty() }
?.let { packageGroups -> put(Carrier.USPS, packageGroups) }

carrierPackageGroups.parseCarrierData(WooShippingPackagesEntity.CarrierType.FEDEX)
.takeIf { it.isNotEmpty() }
?.let { packageGroups -> put(Carrier.FEDEX, packageGroups) }
if (featureFlagRepository.isEnabled(FeatureFlag.WOO_SHIPPING_FEDEX)) {
carrierPackageGroups.parseCarrierData(WooShippingPackagesEntity.CarrierType.FEDEX)
.takeIf { it.isNotEmpty() }
?.let { packageGroups -> put(Carrier.FEDEX, packageGroups) }
}

carrierPackageGroups.parseCarrierData(WooShippingPackagesEntity.CarrierType.DHL)
.takeIf { it.isNotEmpty() }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,23 @@ import com.woocommerce.android.ui.orders.wooshippinglabels.rates.ui.CarrierUI
import com.woocommerce.android.ui.orders.wooshippinglabels.rates.ui.ShippingRateOptionUI
import com.woocommerce.android.ui.orders.wooshippinglabels.rates.ui.ShippingRateUI
import com.woocommerce.android.util.CurrencyFormatter
import com.woocommerce.android.util.FeatureFlag
import com.woocommerce.android.util.FeatureFlagRepository
import com.woocommerce.android.viewmodel.ResourceProvider
import java.math.BigDecimal
import javax.inject.Inject

class WooShippingRatesDomainMapper @Inject constructor(
private val resourceProvider: ResourceProvider,
private val currencyFormatter: CurrencyFormatter
private val currencyFormatter: CurrencyFormatter,
private val featureFlagRepository: FeatureFlagRepository
) {
operator fun invoke(
rates: List<WooShippingRateOptionsModel>,
currencyCode: String?
): Map<CarrierUI, List<ShippingRateUI>> {
return rates.groupBy { it.defaultRate.carrier }
.filterKeys(::isCarrierEnabled)
.map { (carrier, models) ->
getCarrier(carrier) to models
.map { getShippingRate(it, resourceProvider, currencyCode) }
Expand All @@ -49,6 +53,13 @@ class WooShippingRatesDomainMapper @Inject constructor(
)
}

private fun isCarrierEnabled(carrier: WooShippingCarrier): Boolean {
return when (carrier) {
WooShippingCarrier.FEDEX -> featureFlagRepository.isEnabled(FeatureFlag.WOO_SHIPPING_FEDEX)
else -> true
}
}

private fun getCarrier(carrier: WooShippingCarrier): CarrierUI {
return when (carrier) {
WooShippingCarrier.FEDEX -> CarrierUI(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,5 @@ enum class FeatureFlag(
AGE_ELIGIBILITY_CHECKS("age_eligibility_checks", localValue = PackageUtils.isDebugBuild()),
WOO_SELF_DRIVEN_PUSH_NOTIFICATIONS_M1("woo_self_driven_push_notifications_m1", localValue = false),
WOO_SELF_DRIVEN_PUSH_NOTIFICATIONS_M2("woo_self_driven_push_notifications_m2", localValue = false),
WOO_SHIPPING_FEDEX("woo_shipping_fedex", localValue = false),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

❓ Any reasons for not taking advantage of the new unified feature flag handling and setting the local value to true while disabling the feature remotely? Is there any benefit of having it done in a way that requires an extra app update?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We set localValue to false because the FedEx implementation is still missing on our side. Setting localValue to false means this version does not support FedEx.
We will set it to true after completing WOOMOB-2572.

}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import com.woocommerce.android.ui.orders.wooshippinglabels.packages.WooShippingL
import com.woocommerce.android.ui.orders.wooshippinglabels.packages.ui.Carrier
import com.woocommerce.android.ui.orders.wooshippinglabels.packages.ui.CarrierPackageGroup
import com.woocommerce.android.ui.orders.wooshippinglabels.packages.ui.PackageData
import com.woocommerce.android.util.FeatureFlag
import com.woocommerce.android.util.FeatureFlagRepository
import com.woocommerce.android.viewmodel.BaseUnitTest
import junit.framework.TestCase.assertTrue
import kotlinx.coroutines.ExperimentalCoroutinesApi
Expand All @@ -13,6 +15,7 @@ import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.flow.toList
import org.assertj.core.api.Assertions.assertThat
import org.junit.Test
import org.mockito.kotlin.doReturn
import org.mockito.kotlin.mock
import org.mockito.kotlin.whenever
import org.wordpress.android.fluxc.model.LocalOrRemoteId
Expand All @@ -25,10 +28,14 @@ class ObserveShippingPackagesTest : BaseUnitTest() {
private val packageRepository: WooShippingLabelPackageRepository = mock()
private val fetchShippingPackages: FetchShippingPackages = mock()
private val selectedSite: SelectedSite = mock()
private val featureFlagRepository: FeatureFlagRepository = mock {
on { isEnabled(FeatureFlag.WOO_SHIPPING_FEDEX) } doReturn true
}
private val observeShippingPackages = ObserveShippingPackages(
selectedSite,
packageRepository,
fetchShippingPackages
fetchShippingPackages,
featureFlagRepository
)

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import com.woocommerce.android.ui.orders.wooshippinglabels.models.WooShippingCar
import com.woocommerce.android.ui.orders.wooshippinglabels.rates.datasource.WooShippingRateModel
import com.woocommerce.android.ui.orders.wooshippinglabels.rates.datasource.WooShippingRateOptionsModel
import com.woocommerce.android.util.CurrencyFormatter
import com.woocommerce.android.util.FeatureFlag
import com.woocommerce.android.util.FeatureFlagRepository
import com.woocommerce.android.viewmodel.BaseUnitTest
import com.woocommerce.android.viewmodel.ResourceProvider
import junit.framework.TestCase.assertEquals
Expand All @@ -12,6 +14,7 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi
import org.junit.Test
import org.mockito.kotlin.any
import org.mockito.kotlin.anyOrNull
import org.mockito.kotlin.doReturn
import org.mockito.kotlin.mock
import java.math.BigDecimal

Expand All @@ -27,10 +30,13 @@ class WooShippingRatesDomainMapperTest : BaseUnitTest() {
on(it.getQuantityString(any(), anyOrNull(), anyOrNull(), anyOrNull()))
.thenAnswer { i -> "formatted ${i.arguments[0]}" }
}

private val featureFlagRepository: FeatureFlagRepository = mock {
on { isEnabled(FeatureFlag.WOO_SHIPPING_FEDEX) } doReturn true
}
private val sut = WooShippingRatesDomainMapper(
resourceProvider = resourceProvider,
currencyFormatter = currencyFormatter
currencyFormatter = currencyFormatter,
featureFlagRepository = featureFlagRepository
)

@Test
Expand Down
Loading