From d0586a547cd1b06a23cbf1e627cf0597d5fae13b Mon Sep 17 00:00:00 2001 From: Hafiz Rahman Date: Sun, 22 Dec 2024 15:39:13 +0700 Subject: [PATCH 01/42] Update height on Dashboard Card's menu to fit items better especially on larger font size. --- .../woocommerce/android/ui/compose/component/OverflowMenu.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/compose/component/OverflowMenu.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/compose/component/OverflowMenu.kt index e13f8ea6199..48dc43a9567 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/compose/component/OverflowMenu.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/compose/component/OverflowMenu.kt @@ -3,6 +3,7 @@ package com.woocommerce.android.ui.compose.component import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.heightIn import androidx.compose.material.DropdownMenu import androidx.compose.material.DropdownMenuItem import androidx.compose.material.Icon @@ -54,7 +55,7 @@ fun WCOverflowMenu( ) { items.forEachIndexed { index, item -> DropdownMenuItem( - modifier = Modifier.height(dimensionResource(id = dimen.major_175)), + modifier = Modifier.heightIn(min = dimensionResource(id = dimen.major_175)), onClick = { showMenu = false onSelected(item) From 93c2b1ec7e08524d0c8d46947f9f691f485383fd Mon Sep 17 00:00:00 2001 From: Hafiz Rahman Date: Sun, 22 Dec 2024 15:45:35 +0700 Subject: [PATCH 02/42] Update release notes --- RELEASE-NOTES.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 92357692686..95c04450c1b 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -3,7 +3,7 @@ *** For entries which are touching the Android Wear app's, start entry with `[WEAR]` too. 21.4 ----- - +- [*] Fix Dashboard card menu sizing to fit bigger font sizes and longer text. [https://github.com/woocommerce/woocommerce-android/pull/13184] 21.3 ----- From 8b66b8a8fe9a839a0a16e93e4cbc4b6f1db78b30 Mon Sep 17 00:00:00 2001 From: Andrey Date: Mon, 23 Dec 2024 14:29:17 +0100 Subject: [PATCH 03/42] As order fetched async, add it "getable" with waiting --- .../ui/orders/details/OrderDetailFragment.kt | 32 ++- .../ui/orders/details/OrderDetailViewModel.kt | 261 +++++++++++------- 2 files changed, 182 insertions(+), 111 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailFragment.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailFragment.kt index dd11e57b71d..ab95658e0ef 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailFragment.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailFragment.kt @@ -9,7 +9,10 @@ import android.widget.FrameLayout import androidx.appcompat.content.res.AppCompatResources import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.padding +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.livedata.observeAsState +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember import androidx.compose.ui.Modifier import androidx.compose.ui.platform.ViewCompositionStrategy import androidx.compose.ui.res.stringResource @@ -20,6 +23,7 @@ import androidx.core.view.isVisible import androidx.fragment.app.activityViewModels import androidx.fragment.app.viewModels import androidx.lifecycle.LiveData +import androidx.lifecycle.lifecycleScope import androidx.navigation.fragment.findNavController import androidx.navigation.fragment.navArgs import androidx.transition.TransitionManager @@ -101,6 +105,7 @@ import com.woocommerce.android.viewmodel.fixedHiltNavGraphViewModels import com.woocommerce.android.widgets.SkeletonView import com.woocommerce.android.widgets.WCEmptyView import dagger.hilt.android.AndroidEntryPoint +import kotlinx.coroutines.launch import org.wordpress.android.fluxc.model.OrderAttributionInfo import org.wordpress.android.util.DisplayUtils import javax.inject.Inject @@ -352,6 +357,7 @@ class OrderDetailFragment : viewModel.onNextOrderClicked() true } + else -> { false } @@ -434,23 +440,31 @@ class OrderDetailFragment : showOrderNotes(it) } viewModel.orderRefunds.observe(viewLifecycleOwner) { - showOrderRefunds(it, viewModel.order) + lifecycleScope.launch { + showOrderRefunds(it, viewModel.awaitOrder()) + } } viewModel.productList.observe(viewLifecycleOwner) { - showOrderProducts(it, viewModel.order.currency) + lifecycleScope.launch { + showOrderProducts(it, viewModel.awaitOrder().currency) + } } showCustomAmounts(viewModel.feeLineList) viewModel.shipmentTrackings.observe(viewLifecycleOwner) { showShipmentTrackings(it) } viewModel.shippingLabels.observe(viewLifecycleOwner) { - showShippingLabels(it, viewModel.order.currency) + lifecycleScope.launch { + showShippingLabels(it, viewModel.awaitOrder().currency) + } } viewModel.subscriptions.observe(viewLifecycleOwner) { showSubscriptions(it) } viewModel.giftCards.observe(viewLifecycleOwner) { - showGiftCards(it, viewModel.order.currency) + lifecycleScope.launch { + showGiftCards(it, viewModel.awaitOrder().currency) + } } showShippingLines(viewModel.shippingLineList) @@ -465,9 +479,11 @@ class OrderDetailFragment : uiMessageResolver.showSnack(event.message) } } + is ShowUndoSnackbar -> { displayUndoSnackbar(event.message, event.undoAction, event.dismissAction) } + is OrderNavigationTarget -> navigator.navigate(this, event) is InstallWCShippingViewModel.InstallWcShipping -> navigateToInstallWcShippingFlow() is OrderDetailViewModel.TrashOrder -> { @@ -477,6 +493,7 @@ class OrderDetailFragment : communicationViewModel.trashOrder(event.orderId) } + is MultiLiveEvent.Event.ShowDialog -> event.showDialog() else -> event.isHandled = false } @@ -488,6 +505,11 @@ class OrderDetailFragment : binding.orderDetailShippingLines.apply { setViewCompositionStrategy(ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed) setContent { + val orderCurrency = remember { mutableStateOf("") } + LaunchedEffect(Unit) { + orderCurrency.value = viewModel.awaitOrder().currency + } + shippingLineList.observeAsState().value?.let { shippingLines -> WooThemeWithBackground { ShippingLineSection( @@ -495,7 +517,7 @@ class OrderDetailFragment : formatCurrency = { amount -> currencyFormatter.formatCurrency( amount, - currencyCode = viewModel.order.currency + currencyCode = orderCurrency.value ) }, modifier = Modifier.padding(bottom = 1.dp) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailViewModel.kt index bea5a2106b1..5abe26b03a5 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailViewModel.kt @@ -8,6 +8,7 @@ import androidx.lifecycle.Observer import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.asLiveData import androidx.lifecycle.distinctUntilChanged +import androidx.lifecycle.viewModelScope import com.google.android.material.snackbar.Snackbar import com.woocommerce.android.AppPrefs import com.woocommerce.android.R.string @@ -82,9 +83,11 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.drop import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.withIndex import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking import org.wordpress.android.fluxc.model.OrderAttributionInfo import org.wordpress.android.fluxc.store.WCOrderStore.UpdateOrderResult.OptimisticUpdateResult import org.wordpress.android.fluxc.store.WCOrderStore.UpdateOrderResult.RemoteUpdateResult @@ -121,20 +124,23 @@ class OrderDetailViewModel @Inject constructor( val performanceObserver: LifecycleObserver = orderDetailsTransactionLauncher - var order: Order - get() = requireNotNull(viewState.orderInfo?.order) - set(value) { - viewState = viewState.copy( - orderInfo = viewState.orderInfo?.copy( - order = value, - isPaymentCollectableWithCardReader = viewState.orderInfo?.isPaymentCollectableWithCardReader - ?: false - ) ?: OrderDetailViewState.OrderInfo( - value, - isPaymentCollectableWithCardReader = false - ) + private val _order = MutableStateFlow(null) + + fun updateOrder(newOrder: Order) { + _order.value = newOrder + viewState = viewState.copy( + orderInfo = viewState.orderInfo?.copy( + order = newOrder, + isPaymentCollectableWithCardReader = viewState.orderInfo?.isPaymentCollectableWithCardReader + ?: false + ) ?: OrderDetailViewState.OrderInfo( + newOrder, + isPaymentCollectableWithCardReader = false ) - } + ) + } + + suspend fun awaitOrder(): Order = _order.filterNotNull().first() // Keep track of the deleted shipment tracking number in case // the request to server fails, we need to display an error message @@ -248,7 +254,7 @@ class OrderDetailViewModel @Inject constructor( if (navArgs.orderId != -1L) { launch { orderDetailRepository.getOrderById(navArgs.orderId)?.let { - order = it + updateOrder(it) displayOrderDetails() fetchOrder(showSkeleton = false) } ?: fetchOrder(showSkeleton = true) @@ -327,11 +333,14 @@ class OrderDetailViewModel @Inject constructor( } fun hasVirtualProductsOnly(): Boolean { - return if (order.items.isNotEmpty()) { - val remoteProductIds = order.getProductIds() - orderDetailRepository.hasVirtualProductsOnly(remoteProductIds) - } else { - false + return runBlocking { + val orderFetched = awaitOrder() + if (orderFetched.items.isNotEmpty()) { + val remoteProductIds = orderFetched.getProductIds() + orderDetailRepository.hasVirtualProductsOnly(remoteProductIds) + } else { + false + } } } @@ -347,12 +356,14 @@ class OrderDetailViewModel @Inject constructor( } fun onIssueOrderRefundClicked() { - triggerEvent(IssueOrderRefund(remoteOrderId = order.id)) + viewModelScope.launch { + triggerEvent(IssueOrderRefund(remoteOrderId = awaitOrder().id)) + } } fun onEditClicked() { launch { - val isCurrencyMatch = isStoreCurrencyMatch(order.currency) + val isCurrencyMatch = isStoreCurrencyMatch(awaitOrder().currency) if (!isCurrencyMatch.isMatch) { handleEditClickWhenCurrencyMismatch(isCurrencyMatch) } else { @@ -362,25 +373,29 @@ class OrderDetailViewModel @Inject constructor( } private fun handleEditClick() { - tracker.trackEditButtonTapped(order.feesLines.size, order.shippingLines.size) - val firstGiftCard = giftCards.value?.firstOrNull() - triggerEvent( - EditOrder( - orderId = order.id, - giftCard = firstGiftCard?.code, - appliedDiscount = firstGiftCard?.used + launch { + tracker.trackEditButtonTapped(awaitOrder().feesLines.size, awaitOrder().shippingLines.size) + val firstGiftCard = giftCards.value?.firstOrNull() + triggerEvent( + EditOrder( + orderId = awaitOrder().id, + giftCard = firstGiftCard?.code, + appliedDiscount = firstGiftCard?.used + ) ) - ) + } } private fun handleEditClickWhenCurrencyMismatch(isCurrencyMatch: CurrencyMatchResult) { - tracker.trackOrderAndStoreCurrencyMismatchWhenEditButtonTapped() - triggerEvent( - ShowSnackbar( - message = string.order_detail_edit_store_currency_mismatch, - args = arrayOf(order.currency, isCurrencyMatch.storeCurrency.orEmpty()) + launch { + tracker.trackOrderAndStoreCurrencyMismatchWhenEditButtonTapped() + triggerEvent( + ShowSnackbar( + message = string.order_detail_edit_store_currency_mismatch, + args = arrayOf(awaitOrder().currency, isCurrencyMatch.storeCurrency.orEmpty()) + ) ) - ) + } } fun orderNavigationIsEnabled() = navArgs.allOrderIds?.let { @@ -429,7 +444,7 @@ class OrderDetailViewModel @Inject constructor( fun onSeeReceiptClicked() { launch { - tracker.trackReceiptViewTapped(order.id, order.status) + tracker.trackReceiptViewTapped(awaitOrder().id, awaitOrder().status) viewState = viewState.copy( orderInfo = viewState.orderInfo?.copy( @@ -437,7 +452,7 @@ class OrderDetailViewModel @Inject constructor( ) ) - val receiptResult = paymentReceiptHelper.getReceiptUrl(order.id) + val receiptResult = paymentReceiptHelper.getReceiptUrl(awaitOrder().id) viewState = viewState.copy( orderInfo = viewState.orderInfo?.copy( @@ -446,7 +461,13 @@ class OrderDetailViewModel @Inject constructor( ) if (receiptResult.isSuccess) { - triggerEvent(PreviewReceipt(order.billingAddress.email, receiptResult.getOrThrow(), order.id)) + triggerEvent( + PreviewReceipt( + awaitOrder().billingAddress.email, + receiptResult.getOrThrow(), + awaitOrder().id + ) + ) } else { paymentsFlowTracker.trackReceiptUrlFetchingFails( errorDescription = receiptResult.exceptionOrNull()?.message ?: "Unknown error", @@ -480,19 +501,27 @@ class OrderDetailViewModel @Inject constructor( } fun onViewRefundedProductsClicked() { - triggerEvent(ViewRefundedProducts(orderId = order.id)) + launch { + triggerEvent(ViewRefundedProducts(orderId = awaitOrder().id)) + } } fun onAddOrderNoteClicked() { - triggerEvent(AddOrderNote(orderId = order.id, orderNumber = order.number)) + launch { + triggerEvent(AddOrderNote(orderId = awaitOrder().id, orderNumber = awaitOrder().number)) + } } fun onRefundShippingLabelClick(shippingLabelId: Long) { - triggerEvent(RefundShippingLabel(remoteOrderId = order.id, shippingLabelId = shippingLabelId)) + launch { + triggerEvent(RefundShippingLabel(remoteOrderId = awaitOrder().id, shippingLabelId = shippingLabelId)) + } } fun onPrintShippingLabelClicked(shippingLabelId: Long) { - triggerEvent(PrintShippingLabel(remoteOrderId = order.id, shippingLabelId = shippingLabelId)) + launch { + triggerEvent(PrintShippingLabel(remoteOrderId = awaitOrder().id, shippingLabelId = shippingLabelId)) + } } fun onPrintCustomsFormClicked(shippingLabel: ShippingLabel) { @@ -502,22 +531,32 @@ class OrderDetailViewModel @Inject constructor( } fun onAddShipmentTrackingClicked() { - triggerEvent( - AddOrderShipmentTracking( - orderId = order.id, - orderTrackingProvider = appPrefs.getSelectedShipmentTrackingProviderName(), - isCustomProvider = appPrefs.getIsSelectedShipmentTrackingProviderCustom() + launch { + triggerEvent( + AddOrderShipmentTracking( + orderId = awaitOrder().id, + orderTrackingProvider = appPrefs.getSelectedShipmentTrackingProviderName(), + isCustomProvider = appPrefs.getIsSelectedShipmentTrackingProviderCustom() + ) ) - ) + } } fun onNewShipmentTrackingAdded(shipmentTracking: OrderShipmentTracking) { - tracker.trackAddOrderTrackingTapped(order.id, order.status, shipmentTracking.trackingProvider) - refreshShipmentTracking() + launch { + tracker.trackAddOrderTrackingTapped( + awaitOrder().id, + awaitOrder().status, + shipmentTracking.trackingProvider + ) + refreshShipmentTracking() + } } fun refreshShipmentTracking() { - _shipmentTrackings.value = orderDetailRepository.getOrderShipmentTrackings(order.id) + launch { + _shipmentTrackings.value = orderDetailRepository.getOrderShipmentTrackings(awaitOrder().id) + } } fun onShippingLabelRefunded() { @@ -532,7 +571,7 @@ class OrderDetailViewModel @Inject constructor( // Refresh UI from the database, as new labels are cached by FluxC after the purchase, // if for any reason, the order wasn't found, refetch it orderDetailRepository.getOrderById(navArgs.orderId)?.let { - order = it + updateOrder(it) displayOrderDetails() } ?: fetchOrder(true) } @@ -543,7 +582,9 @@ class OrderDetailViewModel @Inject constructor( } fun onOrderStatusChanged(updateSource: OrderStatusUpdateSource) { - tracker.trackOrderStatusChanged(order.id, order.status.value, updateSource.newStatus) + launch { + tracker.trackOrderStatusChanged(awaitOrder().id, awaitOrder().status.value, updateSource.newStatus) + } val snackbarMessage = when (updateSource) { is OrderStatusUpdateSource.FullFillScreen -> string.order_fulfill_completed @@ -570,31 +611,33 @@ class OrderDetailViewModel @Inject constructor( fun onDeleteShipmentTrackingClicked(trackingNumber: String) { if (networkStatus.isConnected()) { - orderDetailRepository.getOrderShipmentTrackingByTrackingNumber( - order.id, - trackingNumber - )?.let { deletedShipmentTracking -> - deletedOrderShipmentTrackingSet.add(trackingNumber) - - val shipmentTrackings = _shipmentTrackings.value?.toMutableList() ?: mutableListOf() - shipmentTrackings.remove(deletedShipmentTracking) - _shipmentTrackings.value = shipmentTrackings - - triggerEvent( - ShowUndoSnackbar( - message = resourceProvider.getString(string.order_shipment_tracking_delete_snackbar_msg), - undoAction = { onDeleteShipmentTrackingReverted(deletedShipmentTracking) }, - dismissAction = object : Snackbar.Callback() { - override fun onDismissed(transientBottomBar: Snackbar?, event: Int) { - super.onDismissed(transientBottomBar, event) - if (event != DISMISS_EVENT_ACTION) { - // delete the shipment only if user has not clicked on the undo snackbar - deleteOrderShipmentTracking(deletedShipmentTracking) + launch { + orderDetailRepository.getOrderShipmentTrackingByTrackingNumber( + awaitOrder().id, + trackingNumber + )?.let { deletedShipmentTracking -> + deletedOrderShipmentTrackingSet.add(trackingNumber) + + val shipmentTrackings = _shipmentTrackings.value?.toMutableList() ?: mutableListOf() + shipmentTrackings.remove(deletedShipmentTracking) + _shipmentTrackings.value = shipmentTrackings + + triggerEvent( + ShowUndoSnackbar( + message = resourceProvider.getString(string.order_shipment_tracking_delete_snackbar_msg), + undoAction = { onDeleteShipmentTrackingReverted(deletedShipmentTracking) }, + dismissAction = object : Snackbar.Callback() { + override fun onDismissed(transientBottomBar: Snackbar?, event: Int) { + super.onDismissed(transientBottomBar, event) + if (event != DISMISS_EVENT_ACTION) { + // delete the shipment only if user has not clicked on the undo snackbar + deleteOrderShipmentTracking(deletedShipmentTracking) + } } } - } + ) ) - ) + } } } else { triggerEvent(ShowSnackbar(string.offline_error)) @@ -628,7 +671,7 @@ class OrderDetailViewModel @Inject constructor( private fun updateOrderStatus(newStatus: String) { if (networkStatus.isConnected()) { launch { - orderDetailRepository.updateOrderStatus(order.id, newStatus) + orderDetailRepository.updateOrderStatus(awaitOrder().id, newStatus) .collect { result -> reloadOrderDetails() when (result) { @@ -658,19 +701,23 @@ class OrderDetailViewModel @Inject constructor( fun onCreateShippingLabelButtonTapped() { tracker.trackShippinhLabelTapped() - if ( - FeatureFlag.REVAMP_WOO_SHIPPING.isEnabled() && - shippingLabelOnboardingRepository.shippingPluginSupport.isWooShippingSupported() - ) { - triggerEvent(StartWooShippingLabelCreationFlow(order.id)) - } else { - triggerEvent(StartShippingLabelCreationFlow(order.id)) + launch { + if ( + FeatureFlag.REVAMP_WOO_SHIPPING.isEnabled() && + shippingLabelOnboardingRepository.shippingPluginSupport.isWooShippingSupported() + ) { + triggerEvent(StartWooShippingLabelCreationFlow(awaitOrder().id)) + } else { + triggerEvent(StartShippingLabelCreationFlow(awaitOrder().id)) + } } } fun onMarkOrderCompleteButtonTapped() { tracker.trackMarkOrderAsCompleteTapped() - triggerEvent(ViewOrderFulfillInfo(order.id)) + launch { + triggerEvent(ViewOrderFulfillInfo(awaitOrder().id)) + } } fun onViewOrderedAddonButtonTapped(orderItem: Order.Item) { @@ -699,13 +746,13 @@ class OrderDetailViewModel @Inject constructor( } private suspend fun updateOrderState() { - val isPaymentCollectable = isPaymentCollectable(order) - val orderStatus = orderDetailRepository.getOrderStatus(order.status.value) + val isPaymentCollectable = isPaymentCollectable(awaitOrder()) + val orderStatus = orderDetailRepository.getOrderStatus(awaitOrder().status.value) viewState = viewState.copy( orderInfo = OrderDetailViewState.OrderInfo( - order = order, + order = awaitOrder(), isPaymentCollectableWithCardReader = isPaymentCollectable, - receiptButtonStatus = if (paymentReceiptHelper.isReceiptAvailable(order.id) && order.isOrderPaid) { + receiptButtonStatus = if (paymentReceiptHelper.isReceiptAvailable(awaitOrder().id) && awaitOrder().isOrderPaid) { OrderDetailViewState.ReceiptButtonStatus.Visible } else { OrderDetailViewState.ReceiptButtonStatus.Hidden @@ -713,7 +760,7 @@ class OrderDetailViewModel @Inject constructor( ), orderStatus = orderStatus, toolbarTitle = resourceProvider.getString( - string.orderdetail_orderstatus_ordernum, order.number + string.orderdetail_orderstatus_ordernum, awaitOrder().number ), ) } @@ -730,7 +777,7 @@ class OrderDetailViewModel @Inject constructor( val fetchedOrder = orderDetailRepository.fetchOrderById(navArgs.orderId) orderDetailsTransactionLauncher.onOrderFetched() if (fetchedOrder != null) { - order = fetchedOrder + updateOrder(fetchedOrder) fetchOrderProducts() } else { triggerEvent(ShowSnackbar(string.order_error_fetch_generic)) @@ -751,7 +798,7 @@ class OrderDetailViewModel @Inject constructor( private suspend fun loadOrderProducts( refunds: ListInfo ): ListInfo { - val products = refunds.list.getNonRefundedProducts(order.items) + val products = refunds.list.getNonRefundedProducts(awaitOrder().items) checkAddonAvailability(products) val orderProducts = orderProductMapper.toOrderProducts(_productList.value ?: emptyList(), products) return ListInfo(isVisible = orderProducts.isNotEmpty(), list = orderProducts) @@ -762,7 +809,7 @@ class OrderDetailViewModel @Inject constructor( val ids = orderProducts.map { orderProduct -> orderProduct.product.productId } val productTypes = orderDetailRepository.getUniqueProductTypes(ids) val hasAddons = orderProducts.any { orderProduct -> orderProduct.product.containsAddons } - tracker.trackProductsLoaded(order.id, productTypes, hasAddons) + tracker.trackProductsLoaded(awaitOrder().id, productTypes, hasAddons) } private suspend fun checkAddonAvailability(products: List) { @@ -771,9 +818,9 @@ class OrderDetailViewModel @Inject constructor( // the database might be missing certain products, so we need to fetch the ones we don't have private suspend fun fetchOrderProducts() { - val productIds = order.getProductIds() + val productIds = awaitOrder().getProductIds() val numLocalProducts = orderDetailRepository.getProductCountForOrder(productIds) - if (numLocalProducts != order.items.size) { + if (numLocalProducts != awaitOrder().items.size) { orderDetailRepository.fetchProductsByRemoteIds(productIds) } } @@ -830,7 +877,7 @@ class OrderDetailViewModel @Inject constructor( orderDetailsTransactionLauncher.onSubscriptionsFetched() } - private suspend fun fetchGiftCardsAsync() = async { + private fun fetchGiftCardsAsync() = async { val plugin = pluginsInformation[WooCommerceStore.WooPlugin.WOO_GIFT_CARDS.pluginName] if (plugin != null && plugin.isOperational) { giftCardRepository.fetchGiftCardSummaryByOrderId(navArgs.orderId) @@ -846,9 +893,9 @@ class OrderDetailViewModel @Inject constructor( orderDetailsTransactionLauncher.onGiftCardsFetched() } - private fun loadOrderShippingLabels(): ListInfo { + private suspend fun loadOrderShippingLabels(): ListInfo { orderDetailRepository.getOrderShippingLabels(navArgs.orderId) - .loadProducts(order.items) + .loadProducts(awaitOrder().items) .whenNotNullNorEmpty { return ListInfo(list = it) } @@ -877,14 +924,14 @@ class OrderDetailViewModel @Inject constructor( _shipmentTrackings.value = shipmentTracking.list } - _shippingLineList.value = order.shippingLines + _shippingLineList.value = awaitOrder().shippingLines _orderAttributionInfo.value = orderDetailRepository.getOrderAttributionInfo(navArgs.orderId) val orderEligibleForInPersonPayments = viewState.orderInfo?.isPaymentCollectableWithCardReader == true val isOrderEligibleForSLCreation = shippingLabelOnboardingRepository.shippingPluginSupport.isSupported() && - orderDetailRepository.isOrderEligibleForSLCreation(order.id) && + orderDetailRepository.isOrderEligibleForSLCreation(awaitOrder().id) && !orderEligibleForInPersonPayments if (isOrderEligibleForSLCreation && @@ -893,7 +940,7 @@ class OrderDetailViewModel @Inject constructor( ) { // we check against the viewstate to avoid sending the event multiple times // if the eligibility was cached, and we had the same value after re-fetching it - tracker.trackOrderEligibleForShippingLabelCreation(order.status.value) + tracker.trackOrderEligibleForShippingLabelCreation(awaitOrder().status.value) } viewState = viewState.copy( @@ -903,20 +950,22 @@ class OrderDetailViewModel @Inject constructor( isProductListVisible = orderProducts.isVisible, areShippingLabelsVisible = shippingLabels.isVisible, wcShippingBannerVisible = shippingLabelOnboardingRepository.shouldShowWcShippingBanner( - order, + awaitOrder(), orderEligibleForInPersonPayments ), isAIThankYouNoteButtonShown = shouldShowThankYouNoteButton() ) } - private fun shouldShowThankYouNoteButton() = + private suspend fun shouldShowThankYouNoteButton() = selectedSite.getIfExists()?.isWPComAtomic == true && - order.status == Order.Status.Completed && + awaitOrder().status == Order.Status.Completed && productList.value?.isNotEmpty() == true private fun displayCustomAmounts() { - _feeLineList.value = order.feesLines + launch { + _feeLineList.value = awaitOrder().feesLines + } } override fun onProductFetched(remoteProductId: Long) { @@ -930,7 +979,7 @@ class OrderDetailViewModel @Inject constructor( private fun reloadOrderDetails() { launch { orderDetailRepository.getOrderById(navArgs.orderId)?.let { - order = it + updateOrder(it) } ?: WooLog.w(T.ORDERS, "Order ${navArgs.orderId} not found in the database.") displayOrderDetails() } @@ -954,7 +1003,7 @@ class OrderDetailViewModel @Inject constructor( product?.let { triggerEvent( OrderNavigationTarget.AIThankYouNote( - customerName = order.billingAddress.firstName, + customerName = awaitOrder().billingAddress.firstName, productName = it.name, productDescription = it.description ) From 34dc1352dc4d100fbedd6ce69a1939521a615ad7 Mon Sep 17 00:00:00 2001 From: Andrey Date: Mon, 23 Dec 2024 14:33:47 +0100 Subject: [PATCH 04/42] Fixed unit tests --- .../android/ui/orders/OrderDetailViewModelTest.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt index 95648c8803f..d5af1d16c81 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt @@ -795,7 +795,7 @@ class OrderDetailViewModelTest : BaseUnitTest() { viewModel.start() verify(orderDetailRepository, times(1)).fetchOrderById(ORDER_ID) - verify(viewModel, never()).order + verify(viewModel, never()).awaitOrder() assertThat(snackbar).isEqualTo(ShowSnackbar(string.order_error_fetch_generic)) } @@ -939,7 +939,7 @@ class OrderDetailViewModelTest : BaseUnitTest() { if (it is ShowSnackbar) snackbar = it } - viewModel.order = order + viewModel.updateOrder(order) viewModel.start() viewModel.onOrderStatusChanged( OrderStatusUpdateSource.Dialog( @@ -1098,7 +1098,7 @@ class OrderDetailViewModelTest : BaseUnitTest() { viewModel.onCardReaderPaymentCompleted() - assertThat(viewModel.order).isEqualTo(orderAfterPayment) + assertThat(viewModel.awaitOrder()).isEqualTo(orderAfterPayment) } @Test From d7da7d67dc0396c902d4696999e71530a66b2503 Mon Sep 17 00:00:00 2001 From: Andrey Date: Mon, 23 Dec 2024 14:35:35 +0100 Subject: [PATCH 05/42] Fixed formatting --- .../android/ui/orders/details/OrderDetailViewModel.kt | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailViewModel.kt index 5abe26b03a5..3ac32d77883 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailViewModel.kt @@ -746,13 +746,14 @@ class OrderDetailViewModel @Inject constructor( } private suspend fun updateOrderState() { - val isPaymentCollectable = isPaymentCollectable(awaitOrder()) - val orderStatus = orderDetailRepository.getOrderStatus(awaitOrder().status.value) + val order = awaitOrder() + val isPaymentCollectable = isPaymentCollectable(order) + val orderStatus = orderDetailRepository.getOrderStatus(order.status.value) viewState = viewState.copy( orderInfo = OrderDetailViewState.OrderInfo( - order = awaitOrder(), + order = order, isPaymentCollectableWithCardReader = isPaymentCollectable, - receiptButtonStatus = if (paymentReceiptHelper.isReceiptAvailable(awaitOrder().id) && awaitOrder().isOrderPaid) { + receiptButtonStatus = if (paymentReceiptHelper.isReceiptAvailable(order.id) && order.isOrderPaid) { OrderDetailViewState.ReceiptButtonStatus.Visible } else { OrderDetailViewState.ReceiptButtonStatus.Hidden @@ -760,7 +761,7 @@ class OrderDetailViewModel @Inject constructor( ), orderStatus = orderStatus, toolbarTitle = resourceProvider.getString( - string.orderdetail_orderstatus_ordernum, awaitOrder().number + string.orderdetail_orderstatus_ordernum, order.number ), ) } From f72e2df8e3667adebc24f25c9712ca03de052c03 Mon Sep 17 00:00:00 2001 From: Andrey Date: Mon, 23 Dec 2024 14:43:45 +0100 Subject: [PATCH 06/42] Release notes --- RELEASE-NOTES.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index f00457f1fa2..1c9f7350032 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -4,6 +4,7 @@ 21.4 ----- - [*] Fixed overlap issue in Settings > WooCommerce Version [https://github.com/woocommerce/woocommerce-android/pull/13183] +- [*] Fixed a crash on the order details [https://github.com/woocommerce/woocommerce-android/pull/13191] 21.3 From 17ce272b263ac126aafc9689e3191c1107a15ffd Mon Sep 17 00:00:00 2001 From: Alejo Date: Tue, 24 Dec 2024 16:11:55 -0300 Subject: [PATCH 07/42] add shipping labels data store --- .../android/datastore/DataStoreModule.kt | 21 +++++++++++ .../android/datastore/DataStoreType.kt | 3 +- .../datasource/ShippingLabelsDataStore.kt | 37 +++++++++++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/datasource/ShippingLabelsDataStore.kt diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreModule.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreModule.kt index ce8c27e25b4..02c195b0d0b 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreModule.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreModule.kt @@ -14,6 +14,7 @@ import com.woocommerce.android.datastore.DataStoreType.ANALYTICS_UI_CACHE import com.woocommerce.android.datastore.DataStoreType.COUPONS import com.woocommerce.android.datastore.DataStoreType.DASHBOARD_STATS import com.woocommerce.android.datastore.DataStoreType.LAST_UPDATE +import com.woocommerce.android.datastore.DataStoreType.SHIPPING_LABEL_CONFIGURATION import com.woocommerce.android.datastore.DataStoreType.TOP_PERFORMER_PRODUCTS import com.woocommerce.android.datastore.DataStoreType.TRACKER import com.woocommerce.android.di.AppCoroutineScope @@ -158,4 +159,24 @@ class DataStoreModule { }, scope = CoroutineScope(appCoroutineScope.coroutineContext + Dispatchers.IO) ) + + @Provides + @Singleton + @DataStoreQualifier(SHIPPING_LABEL_CONFIGURATION) + fun provideShippingLabelConfigurationDataStore( + appContext: Context, + crashLogging: CrashLogging, + @AppCoroutineScope appCoroutineScope: CoroutineScope + ) = PreferenceDataStoreFactory.create( + produceFile = { + appContext.preferencesDataStoreFile("shipping_label_configuration") + }, + corruptionHandler = ReplaceFileCorruptionHandler { + crashLogging.recordEvent( + "Corrupted data store. DataStore Type: ${SHIPPING_LABEL_CONFIGURATION.name}" + ) + emptyPreferences() + }, + scope = CoroutineScope(appCoroutineScope.coroutineContext + Dispatchers.IO) + ) } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreType.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreType.kt index bf3939194fc..1a0d37df145 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreType.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreType.kt @@ -7,5 +7,6 @@ enum class DataStoreType { DASHBOARD_STATS, TOP_PERFORMER_PRODUCTS, COUPONS, - LAST_UPDATE + LAST_UPDATE, + SHIPPING_LABEL_CONFIGURATION } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/datasource/ShippingLabelsDataStore.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/datasource/ShippingLabelsDataStore.kt new file mode 100644 index 00000000000..674158f8329 --- /dev/null +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/datasource/ShippingLabelsDataStore.kt @@ -0,0 +1,37 @@ +package com.woocommerce.android.ui.orders.wooshippinglabels.datasource + +import androidx.datastore.core.DataStore +import androidx.datastore.preferences.core.Preferences +import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.core.stringPreferencesKey +import com.google.gson.Gson +import com.woocommerce.android.datastore.DataStoreQualifier +import com.woocommerce.android.datastore.DataStoreType +import com.woocommerce.android.tools.SelectedSite +import com.woocommerce.android.ui.orders.wooshippinglabels.models.StoreOptionsModel +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.map +import javax.inject.Inject + +class ShippingLabelsDataStore @Inject constructor( + @DataStoreQualifier(DataStoreType.SHIPPING_LABEL_CONFIGURATION) private val dataStore: DataStore, + private val gson: Gson, + private val selectedSite: SelectedSite +) { + private fun getStoreOptionsKey() = "${selectedSite.getOrNull()?.siteId ?: ""}StoreOptions" + + fun observeStoreOptions(): Flow { + return dataStore.data.map { prefs -> + val storeOptions = prefs[stringPreferencesKey(getStoreOptionsKey())] + runCatching { + gson.fromJson(storeOptions, StoreOptionsModel::class.java) + }.getOrNull() + } + } + + suspend fun saveStoreOptions(storeOptions: StoreOptionsModel) { + dataStore.edit { preferences -> + preferences[stringPreferencesKey(getStoreOptionsKey())] = gson.toJson(storeOptions) + } + } +} From 6b7d849672ed5262f6b0e2b7da4b446441dc51f6 Mon Sep 17 00:00:00 2001 From: Alejo Date: Tue, 24 Dec 2024 16:14:07 -0300 Subject: [PATCH 08/42] usa case to observe store options --- .../orders/wooshippinglabels/ObserveStoreOptions.kt | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptions.kt diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptions.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptions.kt new file mode 100644 index 00000000000..ee43ab999f6 --- /dev/null +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptions.kt @@ -0,0 +1,12 @@ +package com.woocommerce.android.ui.orders.wooshippinglabels + +import com.woocommerce.android.ui.orders.wooshippinglabels.datasource.ShippingLabelsDataStore +import com.woocommerce.android.ui.orders.wooshippinglabels.models.StoreOptionsModel +import kotlinx.coroutines.flow.Flow +import javax.inject.Inject + +class ObserveStoreOptions @Inject constructor( + val dataStore: ShippingLabelsDataStore +) { + operator fun invoke(): Flow = dataStore.observeStoreOptions() +} From 0172e09e6aca1d89544f92921e489980d9734846 Mon Sep 17 00:00:00 2001 From: Alejo Date: Tue, 24 Dec 2024 16:15:17 -0300 Subject: [PATCH 09/42] persist store settings if the request is a success --- .../networking/WooShippingLabelRepository.kt | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/WooShippingLabelRepository.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/WooShippingLabelRepository.kt index ecddefd4a00..08986dca8cb 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/WooShippingLabelRepository.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/WooShippingLabelRepository.kt @@ -1,6 +1,7 @@ package com.woocommerce.android.ui.orders.wooshippinglabels.networking import com.woocommerce.android.model.Address +import com.woocommerce.android.ui.orders.wooshippinglabels.datasource.ShippingLabelsDataStore import com.woocommerce.android.ui.orders.wooshippinglabels.models.OriginShippingAddress import com.woocommerce.android.ui.orders.wooshippinglabels.models.PurchasedLabelData import com.woocommerce.android.ui.orders.wooshippinglabels.models.ShippingLabelStatus @@ -12,7 +13,8 @@ import javax.inject.Inject class WooShippingLabelRepository @Inject constructor( private val restClient: WooShippingLabelRestClient, - private val mapper: WooShippingNetworkingMapper + private val mapper: WooShippingNetworkingMapper, + private val dataStore: ShippingLabelsDataStore ) { suspend fun fetchShippingLabelPrinting( site: SiteModel, @@ -29,6 +31,11 @@ class WooShippingLabelRepository @Inject constructor( ) = restClient.fetchAccountSettings( site = site, ).asWooResult { mapper(it.storeOptions) } + .also { response -> + response.model + ?.takeIf { response.isError.not() } + ?.let { dataStore.saveStoreOptions(it) } + } suspend fun fetchPurchasedShippingLabels( site: SiteModel, From b94fe325934fd44f3e4e67001e7feb48d352f346 Mon Sep 17 00:00:00 2001 From: Alejo Date: Tue, 24 Dec 2024 16:15:51 -0300 Subject: [PATCH 10/42] use local store settings as the source of truth --- .../WooShippingLabelCreationViewModel.kt | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt index e88a1fe896f..5adec2a56d0 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt @@ -35,6 +35,7 @@ import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.drop +import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch @@ -51,7 +52,8 @@ class WooShippingLabelCreationViewModel @Inject constructor( private val observeOriginAddresses: ObserveOriginAddresses, private val getShippingRates: GetShippingRates, private val fetchAccountSettings: FetchAccountSettings, - private val purchaseShippingLabel: PurchaseShippingLabel + private val purchaseShippingLabel: PurchaseShippingLabel, + private val observeStoreOptions: ObserveStoreOptions ) : ScopedViewModel(savedState) { private val navArgs: WooShippingLabelCreationFragmentArgs by savedState.navArgs() @@ -104,14 +106,15 @@ class WooShippingLabelCreationViewModel @Inject constructor( } private fun getStoreOptions() { + launch { + observeStoreOptions().filterNotNull().collectLatest { options -> + storeOptions.value = options + } + } launch { fetchAccountSettings().fold( - onSuccess = { - storeOptions.value = it - }, - onFailure = { - storeOptions.value = null - } + onSuccess = {}, + onFailure = { storeOptions.value = null } ) } } From 3366e570ed011df4ffdeca3265ad952b9c337418 Mon Sep 17 00:00:00 2001 From: Alejo Date: Tue, 24 Dec 2024 16:16:00 -0300 Subject: [PATCH 11/42] fix unit tests --- .../WooShippingLabelCreationViewModelTest.kt | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt index 22c60c0f320..20d77832997 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt @@ -206,6 +206,7 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { private val getShippingRates: GetShippingRates = mock() private val fetchAccountSettings: FetchAccountSettings = mock() private val purchaseShippingLabel: PurchaseShippingLabel = mock() + private val observeStoreOptions: ObserveStoreOptions = mock() private lateinit var sut: WooShippingLabelCreationViewModel @@ -218,6 +219,7 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { getShippingRates = getShippingRates, fetchAccountSettings = fetchAccountSettings, purchaseShippingLabel = purchaseShippingLabel, + observeStoreOptions = observeStoreOptions, savedState = savedState ) } @@ -235,6 +237,7 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { whenever(getShippableItems(any())) doReturn defaultShippableItems whenever(observeOriginAddresses()) doReturn flowOf(defaultOriginAddresses) whenever(fetchAccountSettings()) doReturn Result.success(defaultStoreOptions) + whenever(observeStoreOptions()) doReturn flowOf(defaultStoreOptions) createViewModel() @@ -259,6 +262,7 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { whenever(getShippableItems(any())) doReturn defaultShippableItems whenever(observeOriginAddresses()) doReturn flowOf(defaultOriginAddresses) whenever(fetchAccountSettings()) doReturn Result.success(defaultStoreOptions) + whenever(observeStoreOptions()) doReturn flowOf(defaultStoreOptions) createViewModel() @@ -276,6 +280,7 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { val order: Order? = null whenever(orderDetailRepository.getOrderById(any())) doReturn order whenever(observeOriginAddresses()) doReturn flowOf(defaultOriginAddresses) + whenever(observeStoreOptions()) doReturn flowOf(defaultStoreOptions) createViewModel() @@ -292,6 +297,7 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { ) whenever(orderDetailRepository.getOrderById(any())) doReturn order whenever(observeOriginAddresses()) doReturn flowOf(emptyList()) + whenever(observeStoreOptions()) doReturn flowOf(defaultStoreOptions) createViewModel() @@ -310,6 +316,7 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { whenever(getShippableItems(any())) doReturn defaultShippableItems whenever(observeOriginAddresses()) doReturn flowOf(defaultOriginAddresses) whenever(fetchAccountSettings()) doReturn Result.success(defaultStoreOptions) + whenever(observeStoreOptions()) doReturn flowOf(defaultStoreOptions) createViewModel() @@ -339,6 +346,7 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { getShippingRates(any(), any(), any(), any(), any(), any()) ) doReturn Result.success(defaultShippingRates) whenever(fetchAccountSettings()) doReturn Result.success(defaultStoreOptions) + whenever(observeStoreOptions()) doReturn flowOf(defaultStoreOptions) createViewModel() @@ -368,6 +376,7 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { getShippingRates(any(), any(), any(), any(), any(), any()) ) doReturn Result.failure(Exception("Random error")) whenever(fetchAccountSettings()) doReturn Result.success(defaultStoreOptions) + whenever(observeStoreOptions()) doReturn flowOf(defaultStoreOptions) createViewModel() sut.onPackageSelected(defaultPackageData) @@ -474,6 +483,7 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { whenever(getShippableItems(any())) doReturn defaultShippableItems whenever(observeOriginAddresses()) doReturn flowOf(defaultOriginAddresses) whenever(fetchAccountSettings()) doReturn Result.success(defaultStoreOptions) + whenever(observeStoreOptions()) doReturn flowOf(defaultStoreOptions) createViewModel() @@ -514,6 +524,7 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { whenever(getShippableItems(any())) doReturn defaultShippableItems whenever(observeOriginAddresses()) doReturn flowOf(defaultOriginAddresses) whenever(fetchAccountSettings()) doReturn Result.success(defaultStoreOptions) + whenever(observeStoreOptions()) doReturn flowOf(defaultStoreOptions) createViewModel() @@ -566,6 +577,7 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { whenever(getShippableItems(any())) doReturn defaultShippableItems whenever(observeOriginAddresses()) doReturn flowOf(defaultOriginAddresses) whenever(fetchAccountSettings()) doReturn Result.success(defaultStoreOptions) + whenever(observeStoreOptions()) doReturn flowOf(defaultStoreOptions) whenever( purchaseShippingLabel(any(), any(), any(), any(), any(), any(), any(), any()) ) doReturn Result.success(defaultPurchasedLabelData) @@ -618,6 +630,7 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { whenever(getShippableItems(any())) doReturn defaultShippableItems whenever(observeOriginAddresses()) doReturn flowOf(defaultOriginAddresses) whenever(fetchAccountSettings()) doReturn Result.success(defaultStoreOptions) + whenever(observeStoreOptions()) doReturn flowOf(defaultStoreOptions) whenever( purchaseShippingLabel(any(), any(), any(), any(), any(), any(), any(), any()) ) doReturn Result.failure(Exception("Random error")) From edf278b4ea81de6153657b0c01b700c48efb8747 Mon Sep 17 00:00:00 2001 From: Alejo Date: Tue, 24 Dec 2024 16:58:30 -0300 Subject: [PATCH 12/42] add unit test --- .../WooShippingLabelCreationViewModelTest.kt | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt index 20d77832997..ab55ee83682 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt @@ -649,4 +649,22 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { val currentViewState = sut.viewState.value assertThat(currentViewState).isInstanceOf(WooShippingViewState.Error::class.java) } + + @Test + fun `when the view model is created, then get store options from the local preferences and update settings on background`() = testBlocking { + val order = OrderTestUtils.generateTestOrder(orderId = orderId) + + whenever(orderDetailRepository.getOrderById(any())) doReturn order + whenever(getShippableItems(any())) doReturn defaultShippableItems + whenever(observeOriginAddresses()) doReturn flowOf(defaultOriginAddresses) + whenever(fetchAccountSettings()) doReturn Result.success(defaultStoreOptions) + whenever(observeStoreOptions()) doReturn flowOf(defaultStoreOptions) + + createViewModel() + + advanceUntilIdle() + + verify(observeStoreOptions).invoke() + verify(fetchAccountSettings).invoke() + } } From ea14abe69be144de97595c83d4ad753516664eac Mon Sep 17 00:00:00 2001 From: Andrey Date: Wed, 25 Dec 2024 15:30:42 +0100 Subject: [PATCH 13/42] Update view state when order is updated --- .../ui/orders/details/OrderDetailViewModel.kt | 43 +++++++++++-------- .../ui/orders/OrderDetailViewModelTest.kt | 24 ----------- 2 files changed, 25 insertions(+), 42 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailViewModel.kt index 3ac32d77883..59db22bea90 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailViewModel.kt @@ -126,20 +126,6 @@ class OrderDetailViewModel @Inject constructor( private val _order = MutableStateFlow(null) - fun updateOrder(newOrder: Order) { - _order.value = newOrder - viewState = viewState.copy( - orderInfo = viewState.orderInfo?.copy( - order = newOrder, - isPaymentCollectableWithCardReader = viewState.orderInfo?.isPaymentCollectableWithCardReader - ?: false - ) ?: OrderDetailViewState.OrderInfo( - newOrder, - isPaymentCollectableWithCardReader = false - ) - ) - } - suspend fun awaitOrder(): Order = _order.filterNotNull().first() // Keep track of the deleted shipment tracking number in case @@ -248,13 +234,15 @@ class OrderDetailViewModel @Inject constructor( ) } } + + updateViewStateOnOrderChange() } fun start() { if (navArgs.orderId != -1L) { launch { orderDetailRepository.getOrderById(navArgs.orderId)?.let { - updateOrder(it) + _order.value = it displayOrderDetails() fetchOrder(showSkeleton = false) } ?: fetchOrder(showSkeleton = true) @@ -571,7 +559,7 @@ class OrderDetailViewModel @Inject constructor( // Refresh UI from the database, as new labels are cached by FluxC after the purchase, // if for any reason, the order wasn't found, refetch it orderDetailRepository.getOrderById(navArgs.orderId)?.let { - updateOrder(it) + _order.value = it displayOrderDetails() } ?: fetchOrder(true) } @@ -778,7 +766,7 @@ class OrderDetailViewModel @Inject constructor( val fetchedOrder = orderDetailRepository.fetchOrderById(navArgs.orderId) orderDetailsTransactionLauncher.onOrderFetched() if (fetchedOrder != null) { - updateOrder(fetchedOrder) + _order.value = fetchedOrder fetchOrderProducts() } else { triggerEvent(ShowSnackbar(string.order_error_fetch_generic)) @@ -969,6 +957,25 @@ class OrderDetailViewModel @Inject constructor( } } + private fun updateViewStateOnOrderChange() { + launch { + _order.collect { newOrder -> + newOrder?.let { + viewState = viewState.copy( + orderInfo = viewState.orderInfo?.copy( + order = it, + isPaymentCollectableWithCardReader = viewState.orderInfo?.isPaymentCollectableWithCardReader + ?: false + ) ?: OrderDetailViewState.OrderInfo( + it, + isPaymentCollectableWithCardReader = false + ) + ) + } + } + } + } + override fun onProductFetched(remoteProductId: Long) { viewState = viewState.copy(refreshedProductId = remoteProductId) } @@ -980,7 +987,7 @@ class OrderDetailViewModel @Inject constructor( private fun reloadOrderDetails() { launch { orderDetailRepository.getOrderById(navArgs.orderId)?.let { - updateOrder(it) + _order.value = it } ?: WooLog.w(T.ORDERS, "Order ${navArgs.orderId} not found in the database.") displayOrderDetails() } diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt index d5af1d16c81..af2c9ea4308 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt @@ -929,30 +929,6 @@ class OrderDetailViewModelTest : BaseUnitTest() { assertThat(newOrder?.status).isEqualTo(order.status) } - @Test - fun `Do not update order status when not connected`() = testBlocking { - doReturn(order).whenever(orderDetailRepository).getOrderById(any()) - doReturn(false).whenever(networkStatus).isConnected() - - var snackbar: ShowSnackbar? = null - viewModel.event.observeForever { - if (it is ShowSnackbar) snackbar = it - } - - viewModel.updateOrder(order) - viewModel.start() - viewModel.onOrderStatusChanged( - OrderStatusUpdateSource.Dialog( - oldStatus = order.status.value, - newStatus = CoreOrderStatus.PROCESSING.value - ) - ) - - verify(orderDetailRepository, never()).updateOrderStatus(any(), any()) - - assertThat(snackbar).isEqualTo(ShowSnackbar(string.offline_error)) - } - @Test fun `refresh shipping tracking items when an item is added`() = testBlocking { val shipmentTracking = OrderShipmentTracking( From e0da78ed725195f8b7d3d85061497e42829b2cfd Mon Sep 17 00:00:00 2001 From: Andrey Date: Wed, 25 Dec 2024 15:36:54 +0100 Subject: [PATCH 14/42] Added a test to test the update part of the view model state when order is updated --- .../ui/orders/OrderDetailViewModelTest.kt | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt index af2c9ea4308..6ef5098047d 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt @@ -2463,4 +2463,26 @@ class OrderDetailViewModelTest : BaseUnitTest() { assertThat(viewModel.event.value).isNotInstanceOf(ShowSnackbar::class.java) assertThat(viewModel.event.value).isInstanceOf(EditOrder::class.java) } + + @Test + fun `given order in db, when viewmodel start, then view state is updated with order`() = testBlocking { + // GIVEN + val newOrder = order.copy( + status = Order.Status.Processing, + number = "NewOrderNumber" + ) + doReturn(newOrder).whenever(orderDetailRepository).getOrderById(any()) + + var observedViewState: OrderDetailViewState? = null + viewModel.viewStateData.observeForever { _, newState -> observedViewState = newState } + + // WHEN + viewModel.start() + + // THEN + assertThat(observedViewState!!.orderInfo!!.order).isEqualTo(newOrder) + assertThat(observedViewState.orderInfo.isPaymentCollectableWithCardReader).isFalse() + + } + } From d1231d3fd891edff6f59fccb36a4be63b4150b4d Mon Sep 17 00:00:00 2001 From: Andrey Date: Wed, 25 Dec 2024 15:37:48 +0100 Subject: [PATCH 15/42] Fixed formatting --- .../woocommerce/android/ui/orders/OrderDetailViewModelTest.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt index 6ef5098047d..e868d79d515 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt @@ -2482,7 +2482,5 @@ class OrderDetailViewModelTest : BaseUnitTest() { // THEN assertThat(observedViewState!!.orderInfo!!.order).isEqualTo(newOrder) assertThat(observedViewState.orderInfo.isPaymentCollectableWithCardReader).isFalse() - } - } From 0cf2e5a5f27ac112409b91e0bd13c7e555f8da53 Mon Sep 17 00:00:00 2001 From: Alejo Date: Wed, 25 Dec 2024 11:48:13 -0300 Subject: [PATCH 16/42] update getting options logic --- .../WooShippingLabelCreationViewModel.kt | 712 +++++++++--------- 1 file changed, 363 insertions(+), 349 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt index 5adec2a56d0..79e062efb01 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt @@ -31,11 +31,11 @@ import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.collectIndexed import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.drop -import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch @@ -107,10 +107,24 @@ class WooShippingLabelCreationViewModel @Inject constructor( private fun getStoreOptions() { launch { - observeStoreOptions().filterNotNull().collectLatest { options -> - storeOptions.value = options + observeStoreOptions().collectIndexed { index, options -> + when { + index == 0 && options == null -> { + refreshStoreOptions() + } + + index == 0 && options != null -> { + storeOptions.value = options + refreshStoreOptions() + } + + else -> storeOptions.value = options + } } } + } + + private fun refreshStoreOptions() { launch { fetchAccountSettings().fold( onSuccess = {}, @@ -119,404 +133,404 @@ class WooShippingLabelCreationViewModel @Inject constructor( } } - @Suppress("ComplexCondition") - @OptIn(FlowPreview::class) - private suspend fun observeShippingRates() { - combine( - packageSelected, - shippingAddresses, - packageWeight, - refreshShippingRates.onStart { emit(Unit) } - ) { selectedPackage, addresses, packageWeight, _ -> - if ( - selectedPackage != null && - addresses != null && - packageWeight != null && - addresses.shipTo != Address.EMPTY - ) { - ShippingRatesInfo( - orderId = navArgs.orderId, - packageSelected = selectedPackage, - shipFrom = addresses.shipFrom, - shipTo = addresses.shipTo, - weight = packageWeight.totalWeight, - currencyCode = order.value?.currency - ) - } else { - null + @Suppress("ComplexCondition") + @OptIn(FlowPreview::class) + private suspend fun observeShippingRates() { + combine( + packageSelected, + shippingAddresses, + packageWeight, + refreshShippingRates.onStart { emit(Unit) } + ) { selectedPackage, addresses, packageWeight, _ -> + if ( + selectedPackage != null && + addresses != null && + packageWeight != null && + addresses.shipTo != Address.EMPTY + ) { + ShippingRatesInfo( + orderId = navArgs.orderId, + packageSelected = selectedPackage, + shipFrom = addresses.shipFrom, + shipTo = addresses.shipTo, + weight = packageWeight.totalWeight, + currencyCode = order.value?.currency + ) + } else { + null + } } + .debounce(MULTIPLE_CALLS_DELAY) + .collectLatest { + updateShippingRates(it) + } } - .debounce(MULTIPLE_CALLS_DELAY) - .collectLatest { - updateShippingRates(it) + + private suspend fun observeShippingRatesState() { + combine( + shippingRates, + selectedRate, + selectedRatesSortOrder + ) { shippingRates, selectedRate, selectedRatesSortOrder -> + if (shippingRates.isEmpty()) { + ShippingRatesState.NoAvailable + } else { + ShippingRatesState.DataState( + selectedRatesSortOrder, + sortShippingRates(selectedRatesSortOrder, shippingRates), + selectedRate + ) + } + }.collectLatest { + shippingRatesState.value = it } - } + } - private suspend fun observeShippingRatesState() { - combine( - shippingRates, - selectedRate, - selectedRatesSortOrder - ) { shippingRates, selectedRate, selectedRatesSortOrder -> - if (shippingRates.isEmpty()) { - ShippingRatesState.NoAvailable - } else { - ShippingRatesState.DataState( - selectedRatesSortOrder, - sortShippingRates(selectedRatesSortOrder, shippingRates), - selectedRate + @OptIn(FlowPreview::class) + private suspend fun observePackageWeight() { + combine( + shippableItems, + packageSelected, + snapshotFlow { customWeight }.debounce(TYPING_DELAY) + ) { shippableItems, selectedPackage, customWeightString -> + val itemsWeight = shippableItems.sumByFloat { it.weight } + val packageWeight = selectedPackage?.weight?.toFloatOrNull() + PackageWeight( + itemsWeight = itemsWeight, + packageWeight = packageWeight, + customWeight = customWeightString.toFloatOrNull() ) + }.collectLatest { + packageWeight.value = it } - }.collectLatest { - shippingRatesState.value = it } - } - @OptIn(FlowPreview::class) - private suspend fun observePackageWeight() { - combine( - shippableItems, - packageSelected, - snapshotFlow { customWeight }.debounce(TYPING_DELAY) - ) { shippableItems, selectedPackage, customWeightString -> - val itemsWeight = shippableItems.sumByFloat { it.weight } - val packageWeight = selectedPackage?.weight?.toFloatOrNull() - PackageWeight( - itemsWeight = itemsWeight, - packageWeight = packageWeight, - customWeight = customWeightString.toFloatOrNull() - ) - }.collectLatest { - packageWeight.value = it + private suspend fun observePackageChanges() { + combine( + packageSelected, + packageWeight, + storeOptions + ) { packageSelected, packageWeight, storeOptions -> + if (packageSelected == null || packageWeight == null) { + NotSelected + } else { + DataAvailable( + selectedPackage = packageSelected, + defaultWeight = packageWeight.defaultWeight.toString(), + weightUnit = storeOptions?.weightUnit ?: "" + ) + } + }.collectLatest { + packageSelection.value = it + } } - } - private suspend fun observePackageChanges() { - combine( - packageSelected, - packageWeight, - storeOptions - ) { packageSelected, packageWeight, storeOptions -> - if (packageSelected == null || packageWeight == null) { - NotSelected - } else { - DataAvailable( - selectedPackage = packageSelected, - defaultWeight = packageWeight.defaultWeight.toString(), - weightUnit = storeOptions?.weightUnit ?: "" - ) - } - }.collectLatest { - packageSelection.value = it + private suspend fun getShippingAddresses() { + order.combine(observeOriginAddresses()) { order, originAddresses -> + if (order != null && originAddresses.isNotEmpty()) { + val selectedOriginAddress = getSelectedOriginAddress(originAddresses) + WooShippingAddresses( + shipFrom = selectedOriginAddress, + originAddresses = originAddresses, + shipTo = order.shippingAddress + ) + } else { + null + } + }.collect { shippingAddresses.value = it } } - } - private suspend fun getShippingAddresses() { - order.combine(observeOriginAddresses()) { order, originAddresses -> - if (order != null && originAddresses.isNotEmpty()) { - val selectedOriginAddress = getSelectedOriginAddress(originAddresses) - WooShippingAddresses( - shipFrom = selectedOriginAddress, - originAddresses = originAddresses, - shipTo = order.shippingAddress + private suspend fun updateShippingRates(shippingRatesInfo: ShippingRatesInfo?) { + if (shippingRatesInfo != null) { + val sortOrder = selectedRatesSortOrder.value + shippingRatesState.value = ShippingRatesState.Loading(sortOrder) + + val shippingRatesResult = getShippingRates( + shippingRatesInfo.orderId, + shippingRatesInfo.packageSelected, + shippingRatesInfo.shipTo, + shippingRatesInfo.shipFrom, + shippingRatesInfo.weight, + shippingRatesInfo.currencyCode ) - } else { - null - } - }.collect { shippingAddresses.value = it } - } - - private suspend fun updateShippingRates(shippingRatesInfo: ShippingRatesInfo?) { - if (shippingRatesInfo != null) { - val sortOrder = selectedRatesSortOrder.value - shippingRatesState.value = ShippingRatesState.Loading(sortOrder) - - val shippingRatesResult = getShippingRates( - shippingRatesInfo.orderId, - shippingRatesInfo.packageSelected, - shippingRatesInfo.shipTo, - shippingRatesInfo.shipFrom, - shippingRatesInfo.weight, - shippingRatesInfo.currencyCode - ) - if (shippingRatesResult.isSuccess && shippingRatesResult.getOrThrow().isNotEmpty()) { - shippingRates.value = shippingRatesResult.getOrThrow() + if (shippingRatesResult.isSuccess && shippingRatesResult.getOrThrow().isNotEmpty()) { + shippingRates.value = shippingRatesResult.getOrThrow() + } else { + shippingRatesState.value = ShippingRatesState.Error + } + selectedRate.value = null } else { - shippingRatesState.value = ShippingRatesState.Error + shippingRatesState.value = ShippingRatesState.NoAvailable } - selectedRate.value = null - } else { - shippingRatesState.value = ShippingRatesState.NoAvailable } - } - @Suppress("ComplexCondition") - private suspend fun observeShippingLabelInformation() { - combine( - storeOptions.drop(1), - order.drop(1), - shippingAddresses.drop(1), - shippingRatesState, - packageSelection, - markOrderComplete, - purchaseState - ) { storeOptions, order, addresses, shippingRates, packageSelection, markOrderComplete, purchaseState -> - if (order == null || storeOptions == null || addresses == null || purchaseState is PurchaseState.Error) { - return@combine WooShippingViewState.Error + @Suppress("ComplexCondition") + private suspend fun observeShippingLabelInformation() { + combine( + storeOptions.drop(1), + order.drop(1), + shippingAddresses.drop(1), + shippingRatesState, + packageSelection, + markOrderComplete, + purchaseState + ) { storeOptions, order, addresses, shippingRates, packageSelection, markOrderComplete, purchaseState -> + if (order == null || storeOptions == null || addresses == null || purchaseState is PurchaseState.Error) { + return@combine WooShippingViewState.Error + } + + val items = getShippableItems(order) + shippableItems.value = items + + val shippableItemsUI = items.map { item -> item.toUIModel(currencyFormatter, storeOptions) } + val formattedTotalPrice = getTotalPrice(items) + val formattedTotalWeight = getTotalWeight(items, storeOptions) + + val shippingLineSummary = getShippingLinesSummary(order) + + return@combine WooShippingViewState.DataState( + shippableItems = ShippableItemsUI( + shippableItems = shippableItemsUI, + formattedTotalWeight = formattedTotalWeight, + formattedTotalPrice = formattedTotalPrice + ), + shippingLines = shippingLineSummary, + shippingAddresses = addresses, + shippingRates = shippingRates, + packageSelection = packageSelection, + markOrderComplete = markOrderComplete, + purchaseState = purchaseState + ) + }.collectLatest { + viewState.value = it } - - val items = getShippableItems(order) - shippableItems.value = items - - val shippableItemsUI = items.map { item -> item.toUIModel(currencyFormatter, storeOptions) } - val formattedTotalPrice = getTotalPrice(items) - val formattedTotalWeight = getTotalWeight(items, storeOptions) - - val shippingLineSummary = getShippingLinesSummary(order) - - return@combine WooShippingViewState.DataState( - shippableItems = ShippableItemsUI( - shippableItems = shippableItemsUI, - formattedTotalWeight = formattedTotalWeight, - formattedTotalPrice = formattedTotalPrice - ), - shippingLines = shippingLineSummary, - shippingAddresses = addresses, - shippingRates = shippingRates, - packageSelection = packageSelection, - markOrderComplete = markOrderComplete, - purchaseState = purchaseState - ) - }.collectLatest { - viewState.value = it } - } - private fun getSelectedOriginAddress(originAddresses: List): OriginShippingAddress { - return shippingAddresses.value?.shipFrom?.takeIf { - it != OriginShippingAddress.EMPTY - } ?: originAddresses.first() - } - - fun onShippingFromAddressChange(address: OriginShippingAddress) { - shippingAddresses.value?.let { - shippingAddresses.value = it.copy(shipFrom = address) + private fun getSelectedOriginAddress(originAddresses: List): OriginShippingAddress { + return shippingAddresses.value?.shipFrom?.takeIf { + it != OriginShippingAddress.EMPTY + } ?: originAddresses.first() } - } - fun onShippingToAddressChange(address: Address) { - shippingAddresses.value?.let { - shippingAddresses.value = it.copy(shipTo = address) + fun onShippingFromAddressChange(address: OriginShippingAddress) { + shippingAddresses.value?.let { + shippingAddresses.value = it.copy(shipFrom = address) + } } - } - fun onRefreshShippingRates() { - launch { refreshShippingRates.emit(Unit) } - } - - fun onMarkOrderCompleteChange(value: Boolean) { - markOrderComplete.value = value - } - - private fun getTotalPrice(items: List): String { - val totalPrice = items.sumOf { it.price } - val formattedTotalPrice = items.firstOrNull()?.currency?.let { - currencyFormatter.formatCurrency(totalPrice, it) - } ?: currencyFormatter.formatCurrency(totalPrice) - return formattedTotalPrice - } - - private fun getTotalWeight(items: List, storeOptions: StoreOptionsModel): String { - val totalWeight = items.sumByFloat { it.weight * it.quantity } - return "${totalWeight.formatToString()} ${storeOptions.weightUnit}" - } + fun onShippingToAddressChange(address: Address) { + shippingAddresses.value?.let { + shippingAddresses.value = it.copy(shipTo = address) + } + } - private fun getShippingLinesSummary(order: Order): List { - return order.shippingLines.map { - ShippingLineSummaryUI( - title = it.methodTitle, - amount = currencyFormatter.formatCurrency(it.total, order.currency) - ) + fun onRefreshShippingRates() { + launch { refreshShippingRates.emit(Unit) } } - } - fun onSelectPackageClicked() { - triggerEvent(StartPackageSelection) - } + fun onMarkOrderCompleteChange(value: Boolean) { + markOrderComplete.value = value + } - @Suppress("ComplexCondition") - fun onPurchaseShippingLabel() { - val selectedPackage = packageSelected.value - val addresses = shippingAddresses.value - val shippingRate = selectedRate.value?.selectedOption?.rate - val weight = packageWeight.value?.totalWeight + private fun getTotalPrice(items: List): String { + val totalPrice = items.sumOf { it.price } + val formattedTotalPrice = items.firstOrNull()?.currency?.let { + currencyFormatter.formatCurrency(totalPrice, it) + } ?: currencyFormatter.formatCurrency(totalPrice) + return formattedTotalPrice + } - if (selectedPackage == null || addresses == null || shippingRate == null || weight == null) return + private fun getTotalWeight(items: List, storeOptions: StoreOptionsModel): String { + val totalWeight = items.sumByFloat { it.weight * it.quantity } + return "${totalWeight.formatToString()} ${storeOptions.weightUnit}" + } - val orderId = navArgs.orderId - val lastOrderComplete = markOrderComplete.value - val shippableItemsIdList = shippableItems.value.map { it.productId } + private fun getShippingLinesSummary(order: Order): List { + return order.shippingLines.map { + ShippingLineSummaryUI( + title = it.methodTitle, + amount = currencyFormatter.formatCurrency(it.total, order.currency) + ) + } + } - purchaseState.value = PurchaseState.InProgress + fun onSelectPackageClicked() { + triggerEvent(StartPackageSelection) + } - launch { - val result = purchaseShippingLabel( - orderId, - shippableItemsIdList, - selectedPackage, - addresses.shipTo, - addresses.shipFrom, - shippingRate, - weight, - lastOrderComplete - ) - if (result.isSuccess) { - purchaseState.value = PurchaseState.Success - result.getOrNull() - ?.labels - ?.firstOrNull() - ?.let { purchasedLabel -> - val currentViewState = (viewState.value as? WooShippingViewState.DataState) - val selectedRate = selectedRate.value - if (currentViewState != null && selectedRate != null) { - PurchasedShippingLabelData( - labelId = purchasedLabel.labelId, - orderId = navArgs.orderId, - carrierId = purchasedLabel.carrierId, - trackingNumber = purchasedLabel.tracking, - addresses = currentViewState.shippingAddresses, - items = currentViewState.shippableItems, - rateSummary = selectedRate.summary, - shippingLines = currentViewState.shippingLines - ) - } else { - null - } - }?.let { triggerEvent(LabelPurchased(purchaseData = it)) } - } else { - purchaseState.value = PurchaseState.Error + @Suppress("ComplexCondition") + fun onPurchaseShippingLabel() { + val selectedPackage = packageSelected.value + val addresses = shippingAddresses.value + val shippingRate = selectedRate.value?.selectedOption?.rate + val weight = packageWeight.value?.totalWeight + + if (selectedPackage == null || addresses == null || shippingRate == null || weight == null) return + + val orderId = navArgs.orderId + val lastOrderComplete = markOrderComplete.value + val shippableItemsIdList = shippableItems.value.map { it.productId } + + purchaseState.value = PurchaseState.InProgress + + launch { + val result = purchaseShippingLabel( + orderId, + shippableItemsIdList, + selectedPackage, + addresses.shipTo, + addresses.shipFrom, + shippingRate, + weight, + lastOrderComplete + ) + if (result.isSuccess) { + purchaseState.value = PurchaseState.Success + result.getOrNull() + ?.labels + ?.firstOrNull() + ?.let { purchasedLabel -> + val currentViewState = (viewState.value as? WooShippingViewState.DataState) + val selectedRate = selectedRate.value + if (currentViewState != null && selectedRate != null) { + PurchasedShippingLabelData( + labelId = purchasedLabel.labelId, + orderId = navArgs.orderId, + carrierId = purchasedLabel.carrierId, + trackingNumber = purchasedLabel.tracking, + addresses = currentViewState.shippingAddresses, + items = currentViewState.shippableItems, + rateSummary = selectedRate.summary, + shippingLines = currentViewState.shippingLines + ) + } else { + null + } + }?.let { triggerEvent(LabelPurchased(purchaseData = it)) } + } else { + purchaseState.value = PurchaseState.Error + } } } - } - fun onSelectedRateSortOrderChanged(option: ShippingSortOption) { - selectedRatesSortOrder.value = option - } + fun onSelectedRateSortOrderChanged(option: ShippingSortOption) { + selectedRatesSortOrder.value = option + } - fun onSelectedSippingRateChanged(rate: ShippingRateUI) { - selectedRate.update { rate } - } + fun onSelectedSippingRateChanged(rate: ShippingRateUI) { + selectedRate.update { rate } + } - private fun sortShippingRates( - option: ShippingSortOption, - shippingRates: Map> - ): Map> { - val comparator = when (option) { - ShippingSortOption.CHEAPEST -> { - cheapestComparator + private fun sortShippingRates( + option: ShippingSortOption, + shippingRates: Map> + ): Map> { + val comparator = when (option) { + ShippingSortOption.CHEAPEST -> { + cheapestComparator + } + + ShippingSortOption.FASTEST -> { + fastestComparator + } } + return shippingRates.mapValues { it.value.sortedWith(comparator) } + } - ShippingSortOption.FASTEST -> { - fastestComparator - } + fun onPackageSelected(packageData: PackageData) { + packageSelected.value = packageData } - return shippingRates.mapValues { it.value.sortedWith(comparator) } - } - fun onPackageSelected(packageData: PackageData) { - packageSelected.value = packageData - } + fun onCustomWeightChange(input: String) { + customWeight = input + } - fun onCustomWeightChange(input: String) { - customWeight = input - } + data object StartPackageSelection : Event() + data class LabelPurchased(val purchaseData: PurchasedShippingLabelData) : Event() + + sealed class WooShippingViewState { + data object Error : WooShippingViewState() + data object Loading : WooShippingViewState() + data class DataState( + val shippableItems: ShippableItemsUI, + val shippingLines: List, + val shippingAddresses: WooShippingAddresses, + val shippingRates: ShippingRatesState, + val packageSelection: PackageSelectionState, + val markOrderComplete: Boolean, + val purchaseState: PurchaseState + ) : WooShippingViewState() + } - data object StartPackageSelection : Event() - data class LabelPurchased(val purchaseData: PurchasedShippingLabelData) : Event() - - sealed class WooShippingViewState { - data object Error : WooShippingViewState() - data object Loading : WooShippingViewState() - data class DataState( - val shippableItems: ShippableItemsUI, - val shippingLines: List, - val shippingAddresses: WooShippingAddresses, - val shippingRates: ShippingRatesState, - val packageSelection: PackageSelectionState, - val markOrderComplete: Boolean, - val purchaseState: PurchaseState - ) : WooShippingViewState() - } + sealed class ShippingRatesState { + data object NoAvailable : ShippingRatesState() + data object Error : ShippingRatesState() - sealed class ShippingRatesState { - data object NoAvailable : ShippingRatesState() - data object Error : ShippingRatesState() + data class Loading( + val selectedRatesSortOrder: ShippingSortOption + ) : ShippingRatesState() - data class Loading( - val selectedRatesSortOrder: ShippingSortOption - ) : ShippingRatesState() + data class DataState( + val selectedRatesSortOrder: ShippingSortOption, + val shippingRates: Map>, + val selectedRate: ShippingRateUI? = null + ) : ShippingRatesState() + } - data class DataState( - val selectedRatesSortOrder: ShippingSortOption, - val shippingRates: Map>, - val selectedRate: ShippingRateUI? = null - ) : ShippingRatesState() - } + sealed class PackageSelectionState { + data object NotSelected : PackageSelectionState() + data class DataAvailable( + val selectedPackage: PackageData, + val defaultWeight: String, + val weightUnit: String + ) : PackageSelectionState() + } - sealed class PackageSelectionState { - data object NotSelected : PackageSelectionState() - data class DataAvailable( - val selectedPackage: PackageData, - val defaultWeight: String, - val weightUnit: String - ) : PackageSelectionState() - } + sealed class PurchaseState { + data object NoStarted : PurchaseState() + data object InProgress : PurchaseState() + data object Success : PurchaseState() + data object Error : PurchaseState() + } - sealed class PurchaseState { - data object NoStarted : PurchaseState() - data object InProgress : PurchaseState() - data object Success : PurchaseState() - data object Error : PurchaseState() - } + data class PackageWeight( + val itemsWeight: Float, + val packageWeight: Float? = null, + val customWeight: Float? = null + ) { + val defaultWeight: Float + get() = itemsWeight + (packageWeight ?: 0f) + val totalWeight: Float + get() = customWeight ?: defaultWeight + } - data class PackageWeight( - val itemsWeight: Float, - val packageWeight: Float? = null, - val customWeight: Float? = null - ) { - val defaultWeight: Float - get() = itemsWeight + (packageWeight ?: 0f) - val totalWeight: Float - get() = customWeight ?: defaultWeight + data class ShippingRatesInfo( + val orderId: Long, + val packageSelected: PackageData, + val shipFrom: OriginShippingAddress, + val shipTo: Address, + val weight: Float, + val currencyCode: String? + ) + + companion object { + private const val TYPING_DELAY = 800L + private const val MULTIPLE_CALLS_DELAY = 50L + } } - data class ShippingRatesInfo( - val orderId: Long, - val packageSelected: PackageData, + @Parcelize + data class WooShippingAddresses( val shipFrom: OriginShippingAddress, val shipTo: Address, - val weight: Float, - val currencyCode: String? - ) - - companion object { - private const val TYPING_DELAY = 800L - private const val MULTIPLE_CALLS_DELAY = 50L - } -} - -@Parcelize -data class WooShippingAddresses( - val shipFrom: OriginShippingAddress, - val shipTo: Address, - val originAddresses: List -) : Parcelable { - companion object { - val EMPTY = WooShippingAddresses( - shipFrom = OriginShippingAddress.EMPTY, - shipTo = Address.EMPTY, - originAddresses = emptyList() - ) + val originAddresses: List + ) : Parcelable { + companion object { + val EMPTY = WooShippingAddresses( + shipFrom = OriginShippingAddress.EMPTY, + shipTo = Address.EMPTY, + originAddresses = emptyList() + ) + } } -} From afcf87f039221212a273e4c3a6b410a4424ce430 Mon Sep 17 00:00:00 2001 From: Alejo Date: Wed, 25 Dec 2024 11:48:29 -0300 Subject: [PATCH 17/42] unit test failure --- .../WooShippingLabelCreationViewModelTest.kt | 30 ++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt index ab55ee83682..aa5d41bd434 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt @@ -651,20 +651,42 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { } @Test - fun `when the view model is created, then get store options from the local preferences and update settings on background`() = testBlocking { + fun `when the view model is created, then get store options from the local preferences and update settings on background`() = + testBlocking { + val order = OrderTestUtils.generateTestOrder(orderId = orderId) + + whenever(orderDetailRepository.getOrderById(any())) doReturn order + whenever(getShippableItems(any())) doReturn defaultShippableItems + whenever(observeOriginAddresses()) doReturn flowOf(defaultOriginAddresses) + whenever(fetchAccountSettings()) doReturn Result.success(defaultStoreOptions) + whenever(observeStoreOptions()) doReturn flowOf(null, defaultStoreOptions) + + createViewModel() + + advanceUntilIdle() + + verify(observeStoreOptions).invoke() + // update the settings only once + verify(fetchAccountSettings).invoke() + } + + @Test + fun `when there is no cached store options and API request fails then display error`() = testBlocking { val order = OrderTestUtils.generateTestOrder(orderId = orderId) whenever(orderDetailRepository.getOrderById(any())) doReturn order whenever(getShippableItems(any())) doReturn defaultShippableItems whenever(observeOriginAddresses()) doReturn flowOf(defaultOriginAddresses) - whenever(fetchAccountSettings()) doReturn Result.success(defaultStoreOptions) - whenever(observeStoreOptions()) doReturn flowOf(defaultStoreOptions) + whenever(fetchAccountSettings()) doReturn Result.failure(Exception("Random error")) + whenever(observeStoreOptions()) doReturn flowOf(null) createViewModel() advanceUntilIdle() - verify(observeStoreOptions).invoke() verify(fetchAccountSettings).invoke() + + val currentViewState = sut.viewState.value + assertThat(currentViewState).isInstanceOf(WooShippingViewState.Error::class.java) } } From 3a53d83f64c41ccb5f966c911434b231e088d384 Mon Sep 17 00:00:00 2001 From: Andrey Date: Wed, 25 Dec 2024 16:05:13 +0100 Subject: [PATCH 18/42] Fixed the test --- .../woocommerce/android/ui/orders/OrderDetailViewModelTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt index e868d79d515..7d4ed6d65e1 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt @@ -2481,6 +2481,6 @@ class OrderDetailViewModelTest : BaseUnitTest() { // THEN assertThat(observedViewState!!.orderInfo!!.order).isEqualTo(newOrder) - assertThat(observedViewState.orderInfo.isPaymentCollectableWithCardReader).isFalse() + assertThat(observedViewState!!.orderInfo!!.isPaymentCollectableWithCardReader).isFalse() } } From 14a1660bff8840600fcb50a5eac7f6af6e387b68 Mon Sep 17 00:00:00 2001 From: Alejo Date: Wed, 25 Dec 2024 13:00:36 -0300 Subject: [PATCH 19/42] fix detekt issues --- .../WooShippingLabelCreationViewModel.kt | 690 +++++++++--------- 1 file changed, 345 insertions(+), 345 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt index 79e062efb01..a4d4e82dec9 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt @@ -133,404 +133,404 @@ class WooShippingLabelCreationViewModel @Inject constructor( } } - @Suppress("ComplexCondition") - @OptIn(FlowPreview::class) - private suspend fun observeShippingRates() { - combine( - packageSelected, - shippingAddresses, - packageWeight, - refreshShippingRates.onStart { emit(Unit) } - ) { selectedPackage, addresses, packageWeight, _ -> - if ( - selectedPackage != null && - addresses != null && - packageWeight != null && - addresses.shipTo != Address.EMPTY - ) { - ShippingRatesInfo( - orderId = navArgs.orderId, - packageSelected = selectedPackage, - shipFrom = addresses.shipFrom, - shipTo = addresses.shipTo, - weight = packageWeight.totalWeight, - currencyCode = order.value?.currency - ) - } else { - null - } + @Suppress("ComplexCondition") + @OptIn(FlowPreview::class) + private suspend fun observeShippingRates() { + combine( + packageSelected, + shippingAddresses, + packageWeight, + refreshShippingRates.onStart { emit(Unit) } + ) { selectedPackage, addresses, packageWeight, _ -> + if ( + selectedPackage != null && + addresses != null && + packageWeight != null && + addresses.shipTo != Address.EMPTY + ) { + ShippingRatesInfo( + orderId = navArgs.orderId, + packageSelected = selectedPackage, + shipFrom = addresses.shipFrom, + shipTo = addresses.shipTo, + weight = packageWeight.totalWeight, + currencyCode = order.value?.currency + ) + } else { + null } - .debounce(MULTIPLE_CALLS_DELAY) - .collectLatest { - updateShippingRates(it) - } } - - private suspend fun observeShippingRatesState() { - combine( - shippingRates, - selectedRate, - selectedRatesSortOrder - ) { shippingRates, selectedRate, selectedRatesSortOrder -> - if (shippingRates.isEmpty()) { - ShippingRatesState.NoAvailable - } else { - ShippingRatesState.DataState( - selectedRatesSortOrder, - sortShippingRates(selectedRatesSortOrder, shippingRates), - selectedRate - ) - } - }.collectLatest { - shippingRatesState.value = it + .debounce(MULTIPLE_CALLS_DELAY) + .collectLatest { + updateShippingRates(it) } - } + } - @OptIn(FlowPreview::class) - private suspend fun observePackageWeight() { - combine( - shippableItems, - packageSelected, - snapshotFlow { customWeight }.debounce(TYPING_DELAY) - ) { shippableItems, selectedPackage, customWeightString -> - val itemsWeight = shippableItems.sumByFloat { it.weight } - val packageWeight = selectedPackage?.weight?.toFloatOrNull() - PackageWeight( - itemsWeight = itemsWeight, - packageWeight = packageWeight, - customWeight = customWeightString.toFloatOrNull() + private suspend fun observeShippingRatesState() { + combine( + shippingRates, + selectedRate, + selectedRatesSortOrder + ) { shippingRates, selectedRate, selectedRatesSortOrder -> + if (shippingRates.isEmpty()) { + ShippingRatesState.NoAvailable + } else { + ShippingRatesState.DataState( + selectedRatesSortOrder, + sortShippingRates(selectedRatesSortOrder, shippingRates), + selectedRate ) - }.collectLatest { - packageWeight.value = it - } - } - - private suspend fun observePackageChanges() { - combine( - packageSelected, - packageWeight, - storeOptions - ) { packageSelected, packageWeight, storeOptions -> - if (packageSelected == null || packageWeight == null) { - NotSelected - } else { - DataAvailable( - selectedPackage = packageSelected, - defaultWeight = packageWeight.defaultWeight.toString(), - weightUnit = storeOptions?.weightUnit ?: "" - ) - } - }.collectLatest { - packageSelection.value = it } + }.collectLatest { + shippingRatesState.value = it } + } - private suspend fun getShippingAddresses() { - order.combine(observeOriginAddresses()) { order, originAddresses -> - if (order != null && originAddresses.isNotEmpty()) { - val selectedOriginAddress = getSelectedOriginAddress(originAddresses) - WooShippingAddresses( - shipFrom = selectedOriginAddress, - originAddresses = originAddresses, - shipTo = order.shippingAddress - ) - } else { - null - } - }.collect { shippingAddresses.value = it } + @OptIn(FlowPreview::class) + private suspend fun observePackageWeight() { + combine( + shippableItems, + packageSelected, + snapshotFlow { customWeight }.debounce(TYPING_DELAY) + ) { shippableItems, selectedPackage, customWeightString -> + val itemsWeight = shippableItems.sumByFloat { it.weight } + val packageWeight = selectedPackage?.weight?.toFloatOrNull() + PackageWeight( + itemsWeight = itemsWeight, + packageWeight = packageWeight, + customWeight = customWeightString.toFloatOrNull() + ) + }.collectLatest { + packageWeight.value = it } + } - private suspend fun updateShippingRates(shippingRatesInfo: ShippingRatesInfo?) { - if (shippingRatesInfo != null) { - val sortOrder = selectedRatesSortOrder.value - shippingRatesState.value = ShippingRatesState.Loading(sortOrder) - - val shippingRatesResult = getShippingRates( - shippingRatesInfo.orderId, - shippingRatesInfo.packageSelected, - shippingRatesInfo.shipTo, - shippingRatesInfo.shipFrom, - shippingRatesInfo.weight, - shippingRatesInfo.currencyCode - ) - - if (shippingRatesResult.isSuccess && shippingRatesResult.getOrThrow().isNotEmpty()) { - shippingRates.value = shippingRatesResult.getOrThrow() - } else { - shippingRatesState.value = ShippingRatesState.Error - } - selectedRate.value = null + private suspend fun observePackageChanges() { + combine( + packageSelected, + packageWeight, + storeOptions + ) { packageSelected, packageWeight, storeOptions -> + if (packageSelected == null || packageWeight == null) { + NotSelected } else { - shippingRatesState.value = ShippingRatesState.NoAvailable + DataAvailable( + selectedPackage = packageSelected, + defaultWeight = packageWeight.defaultWeight.toString(), + weightUnit = storeOptions?.weightUnit ?: "" + ) } + }.collectLatest { + packageSelection.value = it } + } - @Suppress("ComplexCondition") - private suspend fun observeShippingLabelInformation() { - combine( - storeOptions.drop(1), - order.drop(1), - shippingAddresses.drop(1), - shippingRatesState, - packageSelection, - markOrderComplete, - purchaseState - ) { storeOptions, order, addresses, shippingRates, packageSelection, markOrderComplete, purchaseState -> - if (order == null || storeOptions == null || addresses == null || purchaseState is PurchaseState.Error) { - return@combine WooShippingViewState.Error - } - - val items = getShippableItems(order) - shippableItems.value = items - - val shippableItemsUI = items.map { item -> item.toUIModel(currencyFormatter, storeOptions) } - val formattedTotalPrice = getTotalPrice(items) - val formattedTotalWeight = getTotalWeight(items, storeOptions) - - val shippingLineSummary = getShippingLinesSummary(order) - - return@combine WooShippingViewState.DataState( - shippableItems = ShippableItemsUI( - shippableItems = shippableItemsUI, - formattedTotalWeight = formattedTotalWeight, - formattedTotalPrice = formattedTotalPrice - ), - shippingLines = shippingLineSummary, - shippingAddresses = addresses, - shippingRates = shippingRates, - packageSelection = packageSelection, - markOrderComplete = markOrderComplete, - purchaseState = purchaseState + private suspend fun getShippingAddresses() { + order.combine(observeOriginAddresses()) { order, originAddresses -> + if (order != null && originAddresses.isNotEmpty()) { + val selectedOriginAddress = getSelectedOriginAddress(originAddresses) + WooShippingAddresses( + shipFrom = selectedOriginAddress, + originAddresses = originAddresses, + shipTo = order.shippingAddress ) - }.collectLatest { - viewState.value = it + } else { + null } - } + }.collect { shippingAddresses.value = it } + } - private fun getSelectedOriginAddress(originAddresses: List): OriginShippingAddress { - return shippingAddresses.value?.shipFrom?.takeIf { - it != OriginShippingAddress.EMPTY - } ?: originAddresses.first() - } + private suspend fun updateShippingRates(shippingRatesInfo: ShippingRatesInfo?) { + if (shippingRatesInfo != null) { + val sortOrder = selectedRatesSortOrder.value + shippingRatesState.value = ShippingRatesState.Loading(sortOrder) + + val shippingRatesResult = getShippingRates( + shippingRatesInfo.orderId, + shippingRatesInfo.packageSelected, + shippingRatesInfo.shipTo, + shippingRatesInfo.shipFrom, + shippingRatesInfo.weight, + shippingRatesInfo.currencyCode + ) - fun onShippingFromAddressChange(address: OriginShippingAddress) { - shippingAddresses.value?.let { - shippingAddresses.value = it.copy(shipFrom = address) + if (shippingRatesResult.isSuccess && shippingRatesResult.getOrThrow().isNotEmpty()) { + shippingRates.value = shippingRatesResult.getOrThrow() + } else { + shippingRatesState.value = ShippingRatesState.Error } + selectedRate.value = null + } else { + shippingRatesState.value = ShippingRatesState.NoAvailable } + } - fun onShippingToAddressChange(address: Address) { - shippingAddresses.value?.let { - shippingAddresses.value = it.copy(shipTo = address) + @Suppress("ComplexCondition") + private suspend fun observeShippingLabelInformation() { + combine( + storeOptions.drop(1), + order.drop(1), + shippingAddresses.drop(1), + shippingRatesState, + packageSelection, + markOrderComplete, + purchaseState + ) { storeOptions, order, addresses, shippingRates, packageSelection, markOrderComplete, purchaseState -> + if (order == null || storeOptions == null || addresses == null || purchaseState is PurchaseState.Error) { + return@combine WooShippingViewState.Error } - } - fun onRefreshShippingRates() { - launch { refreshShippingRates.emit(Unit) } + val items = getShippableItems(order) + shippableItems.value = items + + val shippableItemsUI = items.map { item -> item.toUIModel(currencyFormatter, storeOptions) } + val formattedTotalPrice = getTotalPrice(items) + val formattedTotalWeight = getTotalWeight(items, storeOptions) + + val shippingLineSummary = getShippingLinesSummary(order) + + return@combine WooShippingViewState.DataState( + shippableItems = ShippableItemsUI( + shippableItems = shippableItemsUI, + formattedTotalWeight = formattedTotalWeight, + formattedTotalPrice = formattedTotalPrice + ), + shippingLines = shippingLineSummary, + shippingAddresses = addresses, + shippingRates = shippingRates, + packageSelection = packageSelection, + markOrderComplete = markOrderComplete, + purchaseState = purchaseState + ) + }.collectLatest { + viewState.value = it } + } - fun onMarkOrderCompleteChange(value: Boolean) { - markOrderComplete.value = value - } + private fun getSelectedOriginAddress(originAddresses: List): OriginShippingAddress { + return shippingAddresses.value?.shipFrom?.takeIf { + it != OriginShippingAddress.EMPTY + } ?: originAddresses.first() + } - private fun getTotalPrice(items: List): String { - val totalPrice = items.sumOf { it.price } - val formattedTotalPrice = items.firstOrNull()?.currency?.let { - currencyFormatter.formatCurrency(totalPrice, it) - } ?: currencyFormatter.formatCurrency(totalPrice) - return formattedTotalPrice + fun onShippingFromAddressChange(address: OriginShippingAddress) { + shippingAddresses.value?.let { + shippingAddresses.value = it.copy(shipFrom = address) } + } - private fun getTotalWeight(items: List, storeOptions: StoreOptionsModel): String { - val totalWeight = items.sumByFloat { it.weight * it.quantity } - return "${totalWeight.formatToString()} ${storeOptions.weightUnit}" + fun onShippingToAddressChange(address: Address) { + shippingAddresses.value?.let { + shippingAddresses.value = it.copy(shipTo = address) } + } - private fun getShippingLinesSummary(order: Order): List { - return order.shippingLines.map { - ShippingLineSummaryUI( - title = it.methodTitle, - amount = currencyFormatter.formatCurrency(it.total, order.currency) - ) - } - } + fun onRefreshShippingRates() { + launch { refreshShippingRates.emit(Unit) } + } - fun onSelectPackageClicked() { - triggerEvent(StartPackageSelection) - } + fun onMarkOrderCompleteChange(value: Boolean) { + markOrderComplete.value = value + } - @Suppress("ComplexCondition") - fun onPurchaseShippingLabel() { - val selectedPackage = packageSelected.value - val addresses = shippingAddresses.value - val shippingRate = selectedRate.value?.selectedOption?.rate - val weight = packageWeight.value?.totalWeight - - if (selectedPackage == null || addresses == null || shippingRate == null || weight == null) return - - val orderId = navArgs.orderId - val lastOrderComplete = markOrderComplete.value - val shippableItemsIdList = shippableItems.value.map { it.productId } - - purchaseState.value = PurchaseState.InProgress - - launch { - val result = purchaseShippingLabel( - orderId, - shippableItemsIdList, - selectedPackage, - addresses.shipTo, - addresses.shipFrom, - shippingRate, - weight, - lastOrderComplete - ) - if (result.isSuccess) { - purchaseState.value = PurchaseState.Success - result.getOrNull() - ?.labels - ?.firstOrNull() - ?.let { purchasedLabel -> - val currentViewState = (viewState.value as? WooShippingViewState.DataState) - val selectedRate = selectedRate.value - if (currentViewState != null && selectedRate != null) { - PurchasedShippingLabelData( - labelId = purchasedLabel.labelId, - orderId = navArgs.orderId, - carrierId = purchasedLabel.carrierId, - trackingNumber = purchasedLabel.tracking, - addresses = currentViewState.shippingAddresses, - items = currentViewState.shippableItems, - rateSummary = selectedRate.summary, - shippingLines = currentViewState.shippingLines - ) - } else { - null - } - }?.let { triggerEvent(LabelPurchased(purchaseData = it)) } - } else { - purchaseState.value = PurchaseState.Error - } - } - } + private fun getTotalPrice(items: List): String { + val totalPrice = items.sumOf { it.price } + val formattedTotalPrice = items.firstOrNull()?.currency?.let { + currencyFormatter.formatCurrency(totalPrice, it) + } ?: currencyFormatter.formatCurrency(totalPrice) + return formattedTotalPrice + } - fun onSelectedRateSortOrderChanged(option: ShippingSortOption) { - selectedRatesSortOrder.value = option - } + private fun getTotalWeight(items: List, storeOptions: StoreOptionsModel): String { + val totalWeight = items.sumByFloat { it.weight * it.quantity } + return "${totalWeight.formatToString()} ${storeOptions.weightUnit}" + } - fun onSelectedSippingRateChanged(rate: ShippingRateUI) { - selectedRate.update { rate } + private fun getShippingLinesSummary(order: Order): List { + return order.shippingLines.map { + ShippingLineSummaryUI( + title = it.methodTitle, + amount = currencyFormatter.formatCurrency(it.total, order.currency) + ) } + } - private fun sortShippingRates( - option: ShippingSortOption, - shippingRates: Map> - ): Map> { - val comparator = when (option) { - ShippingSortOption.CHEAPEST -> { - cheapestComparator - } + fun onSelectPackageClicked() { + triggerEvent(StartPackageSelection) + } - ShippingSortOption.FASTEST -> { - fastestComparator - } + @Suppress("ComplexCondition") + fun onPurchaseShippingLabel() { + val selectedPackage = packageSelected.value + val addresses = shippingAddresses.value + val shippingRate = selectedRate.value?.selectedOption?.rate + val weight = packageWeight.value?.totalWeight + + if (selectedPackage == null || addresses == null || shippingRate == null || weight == null) return + + val orderId = navArgs.orderId + val lastOrderComplete = markOrderComplete.value + val shippableItemsIdList = shippableItems.value.map { it.productId } + + purchaseState.value = PurchaseState.InProgress + + launch { + val result = purchaseShippingLabel( + orderId, + shippableItemsIdList, + selectedPackage, + addresses.shipTo, + addresses.shipFrom, + shippingRate, + weight, + lastOrderComplete + ) + if (result.isSuccess) { + purchaseState.value = PurchaseState.Success + result.getOrNull() + ?.labels + ?.firstOrNull() + ?.let { purchasedLabel -> + val currentViewState = (viewState.value as? WooShippingViewState.DataState) + val selectedRate = selectedRate.value + if (currentViewState != null && selectedRate != null) { + PurchasedShippingLabelData( + labelId = purchasedLabel.labelId, + orderId = navArgs.orderId, + carrierId = purchasedLabel.carrierId, + trackingNumber = purchasedLabel.tracking, + addresses = currentViewState.shippingAddresses, + items = currentViewState.shippableItems, + rateSummary = selectedRate.summary, + shippingLines = currentViewState.shippingLines + ) + } else { + null + } + }?.let { triggerEvent(LabelPurchased(purchaseData = it)) } + } else { + purchaseState.value = PurchaseState.Error } - return shippingRates.mapValues { it.value.sortedWith(comparator) } } + } - fun onPackageSelected(packageData: PackageData) { - packageSelected.value = packageData - } + fun onSelectedRateSortOrderChanged(option: ShippingSortOption) { + selectedRatesSortOrder.value = option + } - fun onCustomWeightChange(input: String) { - customWeight = input - } + fun onSelectedSippingRateChanged(rate: ShippingRateUI) { + selectedRate.update { rate } + } - data object StartPackageSelection : Event() - data class LabelPurchased(val purchaseData: PurchasedShippingLabelData) : Event() - - sealed class WooShippingViewState { - data object Error : WooShippingViewState() - data object Loading : WooShippingViewState() - data class DataState( - val shippableItems: ShippableItemsUI, - val shippingLines: List, - val shippingAddresses: WooShippingAddresses, - val shippingRates: ShippingRatesState, - val packageSelection: PackageSelectionState, - val markOrderComplete: Boolean, - val purchaseState: PurchaseState - ) : WooShippingViewState() + private fun sortShippingRates( + option: ShippingSortOption, + shippingRates: Map> + ): Map> { + val comparator = when (option) { + ShippingSortOption.CHEAPEST -> { + cheapestComparator + } + + ShippingSortOption.FASTEST -> { + fastestComparator + } } + return shippingRates.mapValues { it.value.sortedWith(comparator) } + } - sealed class ShippingRatesState { - data object NoAvailable : ShippingRatesState() - data object Error : ShippingRatesState() + fun onPackageSelected(packageData: PackageData) { + packageSelected.value = packageData + } - data class Loading( - val selectedRatesSortOrder: ShippingSortOption - ) : ShippingRatesState() + fun onCustomWeightChange(input: String) { + customWeight = input + } - data class DataState( - val selectedRatesSortOrder: ShippingSortOption, - val shippingRates: Map>, - val selectedRate: ShippingRateUI? = null - ) : ShippingRatesState() - } + data object StartPackageSelection : Event() + data class LabelPurchased(val purchaseData: PurchasedShippingLabelData) : Event() + + sealed class WooShippingViewState { + data object Error : WooShippingViewState() + data object Loading : WooShippingViewState() + data class DataState( + val shippableItems: ShippableItemsUI, + val shippingLines: List, + val shippingAddresses: WooShippingAddresses, + val shippingRates: ShippingRatesState, + val packageSelection: PackageSelectionState, + val markOrderComplete: Boolean, + val purchaseState: PurchaseState + ) : WooShippingViewState() + } - sealed class PackageSelectionState { - data object NotSelected : PackageSelectionState() - data class DataAvailable( - val selectedPackage: PackageData, - val defaultWeight: String, - val weightUnit: String - ) : PackageSelectionState() - } + sealed class ShippingRatesState { + data object NoAvailable : ShippingRatesState() + data object Error : ShippingRatesState() - sealed class PurchaseState { - data object NoStarted : PurchaseState() - data object InProgress : PurchaseState() - data object Success : PurchaseState() - data object Error : PurchaseState() - } + data class Loading( + val selectedRatesSortOrder: ShippingSortOption + ) : ShippingRatesState() - data class PackageWeight( - val itemsWeight: Float, - val packageWeight: Float? = null, - val customWeight: Float? = null - ) { - val defaultWeight: Float - get() = itemsWeight + (packageWeight ?: 0f) - val totalWeight: Float - get() = customWeight ?: defaultWeight - } + data class DataState( + val selectedRatesSortOrder: ShippingSortOption, + val shippingRates: Map>, + val selectedRate: ShippingRateUI? = null + ) : ShippingRatesState() + } - data class ShippingRatesInfo( - val orderId: Long, - val packageSelected: PackageData, - val shipFrom: OriginShippingAddress, - val shipTo: Address, - val weight: Float, - val currencyCode: String? - ) + sealed class PackageSelectionState { + data object NotSelected : PackageSelectionState() + data class DataAvailable( + val selectedPackage: PackageData, + val defaultWeight: String, + val weightUnit: String + ) : PackageSelectionState() + } - companion object { - private const val TYPING_DELAY = 800L - private const val MULTIPLE_CALLS_DELAY = 50L - } + sealed class PurchaseState { + data object NoStarted : PurchaseState() + data object InProgress : PurchaseState() + data object Success : PurchaseState() + data object Error : PurchaseState() + } + + data class PackageWeight( + val itemsWeight: Float, + val packageWeight: Float? = null, + val customWeight: Float? = null + ) { + val defaultWeight: Float + get() = itemsWeight + (packageWeight ?: 0f) + val totalWeight: Float + get() = customWeight ?: defaultWeight } - @Parcelize - data class WooShippingAddresses( + data class ShippingRatesInfo( + val orderId: Long, + val packageSelected: PackageData, val shipFrom: OriginShippingAddress, val shipTo: Address, - val originAddresses: List - ) : Parcelable { - companion object { - val EMPTY = WooShippingAddresses( - shipFrom = OriginShippingAddress.EMPTY, - shipTo = Address.EMPTY, - originAddresses = emptyList() - ) - } + val weight: Float, + val currencyCode: String? + ) + + companion object { + private const val TYPING_DELAY = 800L + private const val MULTIPLE_CALLS_DELAY = 50L + } +} + +@Parcelize +data class WooShippingAddresses( + val shipFrom: OriginShippingAddress, + val shipTo: Address, + val originAddresses: List +) : Parcelable { + companion object { + val EMPTY = WooShippingAddresses( + shipFrom = OriginShippingAddress.EMPTY, + shipTo = Address.EMPTY, + originAddresses = emptyList() + ) } +} From 302c0f18dc193e5477de8f2713285d98bff53b7f Mon Sep 17 00:00:00 2001 From: Alejo Date: Wed, 25 Dec 2024 21:22:21 -0300 Subject: [PATCH 20/42] fix unit tests --- .../wooshippinglabels/WooShippingLabelCreationViewModelTest.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt index aa5d41bd434..ddab20c4ee4 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt @@ -675,7 +675,6 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { val order = OrderTestUtils.generateTestOrder(orderId = orderId) whenever(orderDetailRepository.getOrderById(any())) doReturn order - whenever(getShippableItems(any())) doReturn defaultShippableItems whenever(observeOriginAddresses()) doReturn flowOf(defaultOriginAddresses) whenever(fetchAccountSettings()) doReturn Result.failure(Exception("Random error")) whenever(observeStoreOptions()) doReturn flowOf(null) From e20225f4f3b1816346f2451507382c0e035f877f Mon Sep 17 00:00:00 2001 From: Andrey Date: Thu, 26 Dec 2024 09:45:22 +0100 Subject: [PATCH 21/42] Add timeout to wait for order --- .../ui/orders/details/OrderDetailViewModel.kt | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailViewModel.kt index 59db22bea90..eafdac638fb 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailViewModel.kt @@ -88,6 +88,7 @@ import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.withIndex import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout import org.wordpress.android.fluxc.model.OrderAttributionInfo import org.wordpress.android.fluxc.store.WCOrderStore.UpdateOrderResult.OptimisticUpdateResult import org.wordpress.android.fluxc.store.WCOrderStore.UpdateOrderResult.RemoteUpdateResult @@ -126,8 +127,6 @@ class OrderDetailViewModel @Inject constructor( private val _order = MutableStateFlow(null) - suspend fun awaitOrder(): Order = _order.filterNotNull().first() - // Keep track of the deleted shipment tracking number in case // the request to server fails, we need to display an error message // and add the deleted tracking number back to the list @@ -993,6 +992,13 @@ class OrderDetailViewModel @Inject constructor( } } + suspend fun awaitOrder(): Order = + withTimeout( + TIME_TO_WAIT_ORDER_MS // time needed in order to crash in case of a deadlock + ) { + _order.filterNotNull().first() + } + fun onWcShippingBannerDismissed() { shippingLabelOnboardingRepository.markWcShippingBannerAsDismissed() } @@ -1037,4 +1043,8 @@ class OrderDetailViewModel @Inject constructor( data class ListInfo(val isVisible: Boolean = true, val list: List = emptyList()) data class TrashOrder(val orderId: Long) : MultiLiveEvent.Event() + + private companion object { + const val TIME_TO_WAIT_ORDER_MS = 3_000L + } } From a5355f48158acebb56b31523418ee6f88e7a5a25 Mon Sep 17 00:00:00 2001 From: Alejo Date: Thu, 26 Dec 2024 10:45:36 -0300 Subject: [PATCH 22/42] refactor create address package --- .../ui/orders/wooshippinglabels/ShipmentDetails.kt | 4 ++++ .../WooShippingLabelCreationScreen.kt | 2 ++ .../WooShippingLabelCreationViewModel.kt | 1 + .../wooshippinglabels/{ => address}/AddressSection.kt | 11 ++++++++--- .../{ => address}/ObserveOriginAddresses.kt | 2 +- .../WooShippingLabelCreationViewModelTest.kt | 1 + 6 files changed, 17 insertions(+), 4 deletions(-) rename WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/{ => address}/AddressSection.kt (96%) rename WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/{ => address}/ObserveOriginAddresses.kt (96%) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ShipmentDetails.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ShipmentDetails.kt index 7463c1a6934..b2731736b26 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ShipmentDetails.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ShipmentDetails.kt @@ -46,6 +46,10 @@ import com.woocommerce.android.extensions.appendWithIfNotEmpty import com.woocommerce.android.model.Address import com.woocommerce.android.ui.compose.animations.SkeletonView import com.woocommerce.android.ui.compose.theme.WooThemeWithBackground +import com.woocommerce.android.ui.orders.wooshippinglabels.address.AddressSectionLandscape +import com.woocommerce.android.ui.orders.wooshippinglabels.address.AddressSectionPortrait +import com.woocommerce.android.ui.orders.wooshippinglabels.address.getShipFrom +import com.woocommerce.android.ui.orders.wooshippinglabels.address.getShipTo import com.woocommerce.android.ui.orders.wooshippinglabels.models.OriginShippingAddress import com.woocommerce.android.util.StringUtils import kotlinx.coroutines.launch diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationScreen.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationScreen.kt index ea286ed61ca..2b881ae78c5 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationScreen.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationScreen.kt @@ -59,6 +59,8 @@ import com.woocommerce.android.ui.compose.theme.WooThemeWithBackground import com.woocommerce.android.ui.orders.wooshippinglabels.WooShippingLabelCreationViewModel.PackageSelectionState import com.woocommerce.android.ui.orders.wooshippinglabels.WooShippingLabelCreationViewModel.PackageSelectionState.DataAvailable import com.woocommerce.android.ui.orders.wooshippinglabels.WooShippingLabelCreationViewModel.PackageSelectionState.NotSelected +import com.woocommerce.android.ui.orders.wooshippinglabels.address.getShipFrom +import com.woocommerce.android.ui.orders.wooshippinglabels.address.getShipTo import com.woocommerce.android.ui.orders.wooshippinglabels.models.OriginShippingAddress import com.woocommerce.android.ui.orders.wooshippinglabels.packages.ui.PackageData import com.woocommerce.android.ui.orders.wooshippinglabels.rates.ui.ShippingRateUI diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt index e88a1fe896f..70fdbfbaac0 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt @@ -14,6 +14,7 @@ import com.woocommerce.android.model.Order import com.woocommerce.android.ui.orders.details.OrderDetailRepository import com.woocommerce.android.ui.orders.wooshippinglabels.WooShippingLabelCreationViewModel.PackageSelectionState.DataAvailable import com.woocommerce.android.ui.orders.wooshippinglabels.WooShippingLabelCreationViewModel.PackageSelectionState.NotSelected +import com.woocommerce.android.ui.orders.wooshippinglabels.address.ObserveOriginAddresses import com.woocommerce.android.ui.orders.wooshippinglabels.models.OriginShippingAddress import com.woocommerce.android.ui.orders.wooshippinglabels.models.ShippableItemModel import com.woocommerce.android.ui.orders.wooshippinglabels.models.StoreOptionsModel diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/AddressSection.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/AddressSection.kt similarity index 96% rename from WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/AddressSection.kt rename to WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/AddressSection.kt index a1d8610331c..c59c721b07b 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/AddressSection.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/AddressSection.kt @@ -1,4 +1,4 @@ -package com.woocommerce.android.ui.orders.wooshippinglabels +package com.woocommerce.android.ui.orders.wooshippinglabels.address import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.IntrinsicSize @@ -30,12 +30,17 @@ import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp import androidx.constraintlayout.compose.ConstraintLayout +import androidx.constraintlayout.compose.Dimension import com.woocommerce.android.R import com.woocommerce.android.model.Address import com.woocommerce.android.model.AmbiguousLocation import com.woocommerce.android.model.Location import com.woocommerce.android.ui.compose.theme.WooThemeWithBackground +import com.woocommerce.android.ui.orders.wooshippinglabels.RoundedCornerBoxWithBorder +import com.woocommerce.android.ui.orders.wooshippinglabels.VerticalDivider +import com.woocommerce.android.ui.orders.wooshippinglabels.WooShippingAddresses import com.woocommerce.android.ui.orders.wooshippinglabels.models.OriginShippingAddress +import com.woocommerce.android.ui.orders.wooshippinglabels.toShippingFromString @Composable @Suppress("DestructuringDeclarationWithTooManyEntries", "UnusedParameter") @@ -87,7 +92,7 @@ internal fun AddressSectionPortrait( top.linkTo(shipFromLabel.top) start.linkTo(shipFromLabel.end) end.linkTo(endBarrier) - width = androidx.constraintlayout.compose.Dimension.fillToConstraints + width = Dimension.fillToConstraints } .padding( top = dimensionResource(R.dimen.major_100), @@ -160,7 +165,7 @@ internal fun AddressSectionPortrait( top.linkTo(shipToLabel.top) start.linkTo(barrier) end.linkTo(shipToEdit.start) - width = androidx.constraintlayout.compose.Dimension.fillToConstraints + width = Dimension.fillToConstraints } .padding( top = dimensionResource(R.dimen.major_100), diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveOriginAddresses.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/ObserveOriginAddresses.kt similarity index 96% rename from WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveOriginAddresses.kt rename to WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/ObserveOriginAddresses.kt index 7546f6b03b5..93f68585f17 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveOriginAddresses.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/ObserveOriginAddresses.kt @@ -1,4 +1,4 @@ -package com.woocommerce.android.ui.orders.wooshippinglabels +package com.woocommerce.android.ui.orders.wooshippinglabels.address import com.woocommerce.android.ui.orders.wooshippinglabels.models.OriginShippingAddress import kotlinx.coroutines.delay diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt index 22c60c0f320..8e4a3490cfb 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt @@ -11,6 +11,7 @@ import com.woocommerce.android.ui.orders.wooshippinglabels.WooShippingLabelCreat import com.woocommerce.android.ui.orders.wooshippinglabels.WooShippingLabelCreationViewModel.PurchaseState import com.woocommerce.android.ui.orders.wooshippinglabels.WooShippingLabelCreationViewModel.WooShippingViewState import com.woocommerce.android.ui.orders.wooshippinglabels.WooShippingLabelCreationViewModel.WooShippingViewState.DataState +import com.woocommerce.android.ui.orders.wooshippinglabels.address.ObserveOriginAddresses import com.woocommerce.android.ui.orders.wooshippinglabels.models.OriginShippingAddress import com.woocommerce.android.ui.orders.wooshippinglabels.models.PurchasedLabelData import com.woocommerce.android.ui.orders.wooshippinglabels.models.ShippableItemModel From 0cca9c05f84958ef7268141298b6648532ebce52 Mon Sep 17 00:00:00 2001 From: Andrey Date: Thu, 26 Dec 2024 17:28:14 +0100 Subject: [PATCH 23/42] Added a test to document timeout --- .../ui/orders/OrderDetailViewModelTest.kt | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt index 7d4ed6d65e1..3c0cb1dfb71 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt @@ -59,11 +59,16 @@ import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ShowSnackbar import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ShowUndoSnackbar import com.woocommerce.android.viewmodel.ResourceProvider import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.InternalCoroutinesApi +import kotlinx.coroutines.TimeoutCancellationException import kotlinx.coroutines.delay import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.launch import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.runTest import org.assertj.core.api.Assertions.assertThat +import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.mockito.kotlin.UseConstructor @@ -2483,4 +2488,29 @@ class OrderDetailViewModelTest : BaseUnitTest() { assertThat(observedViewState!!.orderInfo!!.order).isEqualTo(newOrder) assertThat(observedViewState!!.orderInfo!!.isPaymentCollectableWithCardReader).isFalse() } + + @OptIn(InternalCoroutinesApi::class) + @Test + fun `given more than 3 second no order, when vm created, then TimeoutCancellationException is thrown`() = runTest { + // GIVEN + doReturn(null).whenever(orderDetailRepository).getOrderById(any()) + doReturn(null).whenever(orderDetailRepository).fetchOrderById(any()) + doReturn(false).whenever(orderDetailRepository).fetchOrderNotes(any()) + doReturn(false).whenever(addonsRepository).containsAddonsFrom(any()) + + // WHEN + createViewModel() + + // THEN + val job = launch { + viewModel.awaitOrder() + } + + advanceTimeBy(3_001L) + + job.join() + + val cause = job.getCancellationException().cause + assertTrue(cause is TimeoutCancellationException) + } } From 2b70103cdff3f30f2861457707bc2949997714ce Mon Sep 17 00:00:00 2001 From: Alejo Date: Thu, 26 Dec 2024 21:06:10 -0300 Subject: [PATCH 24/42] refactor observe store options --- .../wooshippinglabels/ObserveStoreOptions.kt | 34 ++++++++++++++++--- .../WooShippingLabelCreationViewModel.kt | 26 ++------------ .../WooShippingLabelCreationViewModelTest.kt | 17 ---------- 3 files changed, 32 insertions(+), 45 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptions.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptions.kt index ee43ab999f6..17533570523 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptions.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptions.kt @@ -1,12 +1,38 @@ package com.woocommerce.android.ui.orders.wooshippinglabels import com.woocommerce.android.ui.orders.wooshippinglabels.datasource.ShippingLabelsDataStore -import com.woocommerce.android.ui.orders.wooshippinglabels.models.StoreOptionsModel -import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.transformLatest import javax.inject.Inject class ObserveStoreOptions @Inject constructor( - val dataStore: ShippingLabelsDataStore + val dataStore: ShippingLabelsDataStore, + val fetchAccountSettings: FetchAccountSettings, ) { - operator fun invoke(): Flow = dataStore.observeStoreOptions() + private var isFirstValue = true + + @OptIn(ExperimentalCoroutinesApi::class) + // We will use data store as the source of truth and after the first emission we will refresh the values async. + operator fun invoke() = dataStore.observeStoreOptions().transformLatest { options -> + when { + isFirstValue && options == null -> { + // If there is no cached data, refresh the store options before emitting any value + if (fetchAccountSettings().isFailure) { + // We will use null as not available + emit(null) + } + } + + isFirstValue && options != null -> { + // If there is cached data, emit cached values and refresh the store options async + emit(options) + if (fetchAccountSettings().isFailure) { + emit(null) + } + } + + else -> emit(options) + } + isFirstValue = false + } } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt index a4d4e82dec9..27b709e92a6 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt @@ -31,7 +31,6 @@ import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.collectIndexed import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.debounce @@ -51,7 +50,6 @@ class WooShippingLabelCreationViewModel @Inject constructor( private val currencyFormatter: CurrencyFormatter, private val observeOriginAddresses: ObserveOriginAddresses, private val getShippingRates: GetShippingRates, - private val fetchAccountSettings: FetchAccountSettings, private val purchaseShippingLabel: PurchaseShippingLabel, private val observeStoreOptions: ObserveStoreOptions ) : ScopedViewModel(savedState) { @@ -107,32 +105,12 @@ class WooShippingLabelCreationViewModel @Inject constructor( private fun getStoreOptions() { launch { - observeStoreOptions().collectIndexed { index, options -> - when { - index == 0 && options == null -> { - refreshStoreOptions() - } - - index == 0 && options != null -> { - storeOptions.value = options - refreshStoreOptions() - } - - else -> storeOptions.value = options - } + observeStoreOptions().collectLatest { options -> + storeOptions.value = options } } } - private fun refreshStoreOptions() { - launch { - fetchAccountSettings().fold( - onSuccess = {}, - onFailure = { storeOptions.value = null } - ) - } - } - @Suppress("ComplexCondition") @OptIn(FlowPreview::class) private suspend fun observeShippingRates() { diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt index ddab20c4ee4..42b4c66f0a9 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModelTest.kt @@ -204,7 +204,6 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { private val observeOriginAddresses: ObserveOriginAddresses = mock() private val getShippingRates: GetShippingRates = mock() - private val fetchAccountSettings: FetchAccountSettings = mock() private val purchaseShippingLabel: PurchaseShippingLabel = mock() private val observeStoreOptions: ObserveStoreOptions = mock() @@ -217,7 +216,6 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { currencyFormatter = currencyFormatter, observeOriginAddresses = observeOriginAddresses, getShippingRates = getShippingRates, - fetchAccountSettings = fetchAccountSettings, purchaseShippingLabel = purchaseShippingLabel, observeStoreOptions = observeStoreOptions, savedState = savedState @@ -236,7 +234,6 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { whenever(orderDetailRepository.getOrderById(any())) doReturn order whenever(getShippableItems(any())) doReturn defaultShippableItems whenever(observeOriginAddresses()) doReturn flowOf(defaultOriginAddresses) - whenever(fetchAccountSettings()) doReturn Result.success(defaultStoreOptions) whenever(observeStoreOptions()) doReturn flowOf(defaultStoreOptions) createViewModel() @@ -261,7 +258,6 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { whenever(orderDetailRepository.getOrderById(any())) doReturn order whenever(getShippableItems(any())) doReturn defaultShippableItems whenever(observeOriginAddresses()) doReturn flowOf(defaultOriginAddresses) - whenever(fetchAccountSettings()) doReturn Result.success(defaultStoreOptions) whenever(observeStoreOptions()) doReturn flowOf(defaultStoreOptions) createViewModel() @@ -315,7 +311,6 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { whenever(orderDetailRepository.getOrderById(any())) doReturn order whenever(getShippableItems(any())) doReturn defaultShippableItems whenever(observeOriginAddresses()) doReturn flowOf(defaultOriginAddresses) - whenever(fetchAccountSettings()) doReturn Result.success(defaultStoreOptions) whenever(observeStoreOptions()) doReturn flowOf(defaultStoreOptions) createViewModel() @@ -345,7 +340,6 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { whenever( getShippingRates(any(), any(), any(), any(), any(), any()) ) doReturn Result.success(defaultShippingRates) - whenever(fetchAccountSettings()) doReturn Result.success(defaultStoreOptions) whenever(observeStoreOptions()) doReturn flowOf(defaultStoreOptions) createViewModel() @@ -375,7 +369,6 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { whenever( getShippingRates(any(), any(), any(), any(), any(), any()) ) doReturn Result.failure(Exception("Random error")) - whenever(fetchAccountSettings()) doReturn Result.success(defaultStoreOptions) whenever(observeStoreOptions()) doReturn flowOf(defaultStoreOptions) createViewModel() @@ -482,7 +475,6 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { whenever(orderDetailRepository.getOrderById(any())) doReturn order whenever(getShippableItems(any())) doReturn defaultShippableItems whenever(observeOriginAddresses()) doReturn flowOf(defaultOriginAddresses) - whenever(fetchAccountSettings()) doReturn Result.success(defaultStoreOptions) whenever(observeStoreOptions()) doReturn flowOf(defaultStoreOptions) createViewModel() @@ -523,7 +515,6 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { whenever(orderDetailRepository.getOrderById(any())) doReturn order whenever(getShippableItems(any())) doReturn defaultShippableItems whenever(observeOriginAddresses()) doReturn flowOf(defaultOriginAddresses) - whenever(fetchAccountSettings()) doReturn Result.success(defaultStoreOptions) whenever(observeStoreOptions()) doReturn flowOf(defaultStoreOptions) createViewModel() @@ -576,7 +567,6 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { whenever(orderDetailRepository.getOrderById(any())) doReturn order whenever(getShippableItems(any())) doReturn defaultShippableItems whenever(observeOriginAddresses()) doReturn flowOf(defaultOriginAddresses) - whenever(fetchAccountSettings()) doReturn Result.success(defaultStoreOptions) whenever(observeStoreOptions()) doReturn flowOf(defaultStoreOptions) whenever( purchaseShippingLabel(any(), any(), any(), any(), any(), any(), any(), any()) @@ -629,7 +619,6 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { whenever(orderDetailRepository.getOrderById(any())) doReturn order whenever(getShippableItems(any())) doReturn defaultShippableItems whenever(observeOriginAddresses()) doReturn flowOf(defaultOriginAddresses) - whenever(fetchAccountSettings()) doReturn Result.success(defaultStoreOptions) whenever(observeStoreOptions()) doReturn flowOf(defaultStoreOptions) whenever( purchaseShippingLabel(any(), any(), any(), any(), any(), any(), any(), any()) @@ -658,7 +647,6 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { whenever(orderDetailRepository.getOrderById(any())) doReturn order whenever(getShippableItems(any())) doReturn defaultShippableItems whenever(observeOriginAddresses()) doReturn flowOf(defaultOriginAddresses) - whenever(fetchAccountSettings()) doReturn Result.success(defaultStoreOptions) whenever(observeStoreOptions()) doReturn flowOf(null, defaultStoreOptions) createViewModel() @@ -666,8 +654,6 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { advanceUntilIdle() verify(observeStoreOptions).invoke() - // update the settings only once - verify(fetchAccountSettings).invoke() } @Test @@ -676,15 +662,12 @@ class WooShippingLabelCreationViewModelTest : BaseUnitTest() { whenever(orderDetailRepository.getOrderById(any())) doReturn order whenever(observeOriginAddresses()) doReturn flowOf(defaultOriginAddresses) - whenever(fetchAccountSettings()) doReturn Result.failure(Exception("Random error")) whenever(observeStoreOptions()) doReturn flowOf(null) createViewModel() advanceUntilIdle() - verify(fetchAccountSettings).invoke() - val currentViewState = sut.viewState.value assertThat(currentViewState).isInstanceOf(WooShippingViewState.Error::class.java) } From 4c076128dd0362205b6a15ab1474c2cdd569e27a Mon Sep 17 00:00:00 2001 From: Alejo Date: Thu, 26 Dec 2024 21:06:22 -0300 Subject: [PATCH 25/42] add unit tests --- .../ObserveStoreOptionsTest.kt | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptionsTest.kt diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptionsTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptionsTest.kt new file mode 100644 index 00000000000..f34f01f77ab --- /dev/null +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptionsTest.kt @@ -0,0 +1,64 @@ +package com.woocommerce.android.ui.orders.wooshippinglabels + +import com.woocommerce.android.ui.orders.wooshippinglabels.datasource.ShippingLabelsDataStore +import com.woocommerce.android.ui.orders.wooshippinglabels.models.StoreOptionsModel +import com.woocommerce.android.viewmodel.BaseUnitTest +import junit.framework.TestCase.assertTrue +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.toList +import org.mockito.kotlin.doReturn +import org.mockito.kotlin.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import kotlin.test.Test + +@OptIn(ExperimentalCoroutinesApi::class) +class ObserveStoreOptionsTest : BaseUnitTest() { + private val dataStore: ShippingLabelsDataStore = mock() + private val fetchAccountSettings: FetchAccountSettings = mock() + val defaultStoreOptions = StoreOptionsModel( + weightUnit = "kg", + currencySymbol = "$", + dimensionUnit = "cm", + originCountry = "US" + ) + + val sut = ObserveStoreOptions( + dataStore = dataStore, + fetchAccountSettings = fetchAccountSettings + ) + + @Test + fun `when there is NO cached data and fetch account settings fails then return null`() = testBlocking { + whenever(dataStore.observeStoreOptions()).doReturn(flowOf(null)) + whenever(fetchAccountSettings.invoke()).doReturn(Result.failure(Exception("Random error"))) + val result = sut.invoke().toList() + + assertTrue(result.size == 1) + assertTrue(result.first() == null) + } + + @Test + fun `when there is cached data and fetch account settings fails then return null`() = testBlocking { + whenever(dataStore.observeStoreOptions()).doReturn(flowOf(defaultStoreOptions)) + whenever(fetchAccountSettings.invoke()).doReturn(Result.failure(Exception("Random error"))) + val result = sut.invoke().toList() + + assertTrue(result.size == 2) + assertTrue(result.first() == defaultStoreOptions) + assertTrue(result.last() == null) + } + + @Test + fun `refresh data only once`() = testBlocking { + whenever(dataStore.observeStoreOptions()).doReturn( + flowOf(defaultStoreOptions, defaultStoreOptions.copy(currencySymbol = "€")) + ) + whenever(fetchAccountSettings.invoke()).doReturn(Result.failure(Exception("Random error"))) + + sut.invoke().toList() + + verify(fetchAccountSettings).invoke() + } +} From 96c0e5306fbe8f5b5b8e63a3dd5a59d0947069d0 Mon Sep 17 00:00:00 2001 From: Hafiz Rahman Date: Fri, 27 Dec 2024 09:54:35 +0700 Subject: [PATCH 26/42] Remove height modifier to use default value instead. --- .../com/woocommerce/android/ui/compose/component/OverflowMenu.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/compose/component/OverflowMenu.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/compose/component/OverflowMenu.kt index 48dc43a9567..e7a466b1cac 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/compose/component/OverflowMenu.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/compose/component/OverflowMenu.kt @@ -55,7 +55,6 @@ fun WCOverflowMenu( ) { items.forEachIndexed { index, item -> DropdownMenuItem( - modifier = Modifier.heightIn(min = dimensionResource(id = dimen.major_175)), onClick = { showMenu = false onSelected(item) From b97b2103bca65ed353dddf9974dc8d3dfb50eb38 Mon Sep 17 00:00:00 2001 From: Andrey Date: Fri, 27 Dec 2024 10:24:10 +0100 Subject: [PATCH 27/42] Removed unnecessary stubbing --- .../android/ui/orders/OrderDetailViewModelTest.kt | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt index 3c0cb1dfb71..88780dee50c 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt @@ -2493,10 +2493,7 @@ class OrderDetailViewModelTest : BaseUnitTest() { @Test fun `given more than 3 second no order, when vm created, then TimeoutCancellationException is thrown`() = runTest { // GIVEN - doReturn(null).whenever(orderDetailRepository).getOrderById(any()) - doReturn(null).whenever(orderDetailRepository).fetchOrderById(any()) - doReturn(false).whenever(orderDetailRepository).fetchOrderNotes(any()) - doReturn(false).whenever(addonsRepository).containsAddonsFrom(any()) + val delay = 3_001L // WHEN createViewModel() @@ -2506,7 +2503,7 @@ class OrderDetailViewModelTest : BaseUnitTest() { viewModel.awaitOrder() } - advanceTimeBy(3_001L) + advanceTimeBy(delay) job.join() From 8c04aa5f88efa230ad3e5da2a307ca62e651519a Mon Sep 17 00:00:00 2001 From: Alejo Date: Fri, 27 Dec 2024 08:59:13 -0300 Subject: [PATCH 28/42] add address data store --- .../android/datastore/DataStoreModule.kt | 21 +++++++++++++++++++ .../android/datastore/DataStoreType.kt | 3 ++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreModule.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreModule.kt index 02c195b0d0b..651a2ccea90 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreModule.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreModule.kt @@ -14,6 +14,7 @@ import com.woocommerce.android.datastore.DataStoreType.ANALYTICS_UI_CACHE import com.woocommerce.android.datastore.DataStoreType.COUPONS import com.woocommerce.android.datastore.DataStoreType.DASHBOARD_STATS import com.woocommerce.android.datastore.DataStoreType.LAST_UPDATE +import com.woocommerce.android.datastore.DataStoreType.SHIPPING_LABEL_ADDRESS import com.woocommerce.android.datastore.DataStoreType.SHIPPING_LABEL_CONFIGURATION import com.woocommerce.android.datastore.DataStoreType.TOP_PERFORMER_PRODUCTS import com.woocommerce.android.datastore.DataStoreType.TRACKER @@ -179,4 +180,24 @@ class DataStoreModule { }, scope = CoroutineScope(appCoroutineScope.coroutineContext + Dispatchers.IO) ) + + @Provides + @Singleton + @DataStoreQualifier(SHIPPING_LABEL_ADDRESS) + fun provideShippingLabelAddressDataStore( + appContext: Context, + crashLogging: CrashLogging, + @AppCoroutineScope appCoroutineScope: CoroutineScope + ) = PreferenceDataStoreFactory.create( + produceFile = { + appContext.preferencesDataStoreFile("shipping_label_address") + }, + corruptionHandler = ReplaceFileCorruptionHandler { + crashLogging.recordEvent( + "Corrupted data store. DataStore Type: ${SHIPPING_LABEL_ADDRESS.name}" + ) + emptyPreferences() + }, + scope = CoroutineScope(appCoroutineScope.coroutineContext + Dispatchers.IO) + ) } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreType.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreType.kt index 1a0d37df145..6295e074618 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreType.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreType.kt @@ -8,5 +8,6 @@ enum class DataStoreType { TOP_PERFORMER_PRODUCTS, COUPONS, LAST_UPDATE, - SHIPPING_LABEL_CONFIGURATION + SHIPPING_LABEL_CONFIGURATION, + SHIPPING_LABEL_ADDRESS } From fe0c7e3463c1ed93da09d5be26b36d90f0e1a4af Mon Sep 17 00:00:00 2001 From: Alejo Date: Fri, 27 Dec 2024 08:59:37 -0300 Subject: [PATCH 29/42] add origin address dto --- .../wooshippinglabels/networking/DTOs.kt | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/DTOs.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/DTOs.kt index c193a6dc0b4..0414574fad8 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/DTOs.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/DTOs.kt @@ -76,3 +76,21 @@ data class ShippingRatePurchaseResponseDTO( val deliveryDateGuaranteed: Boolean, val deliveryDate: String? ) + +data class OriginAddressDTO( + val id: String? = null, + @SerializedName("address_1") val address: String? = null, + @SerializedName("address_2") val address2: String? = null, + val city: String? = null, + val company: String? = null, + val country: String? = null, + val name: String? = null, + val phone: String? = null, + val postcode: String? = null, + val state: String? = null, + val email: String? = null, + @SerializedName("first_name") val firstName: String? = null, + @SerializedName("last_name") val lastName: String? = null, + @SerializedName("is_verified") val isVerified: Boolean = false, + @SerializedName("default_address") val defaultAddress: Boolean = false, +) From f2081c44367a06005e8d849f31738d9c4c0656cc Mon Sep 17 00:00:00 2001 From: Alejo Date: Fri, 27 Dec 2024 09:01:21 -0300 Subject: [PATCH 30/42] data store for origin addresses --- .../datasource/WooShippingAddressDataStore.kt | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/datasource/WooShippingAddressDataStore.kt diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/datasource/WooShippingAddressDataStore.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/datasource/WooShippingAddressDataStore.kt new file mode 100644 index 00000000000..e9bd0484432 --- /dev/null +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/datasource/WooShippingAddressDataStore.kt @@ -0,0 +1,39 @@ +package com.woocommerce.android.ui.orders.wooshippinglabels.datasource + +import androidx.datastore.core.DataStore +import androidx.datastore.preferences.core.Preferences +import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.core.stringPreferencesKey +import com.google.gson.Gson +import com.google.gson.reflect.TypeToken +import com.woocommerce.android.datastore.DataStoreQualifier +import com.woocommerce.android.datastore.DataStoreType +import com.woocommerce.android.tools.SelectedSite +import com.woocommerce.android.ui.orders.wooshippinglabels.models.OriginShippingAddress +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.map +import javax.inject.Inject + +class WooShippingAddressDataStore @Inject constructor( + @DataStoreQualifier(DataStoreType.SHIPPING_LABEL_ADDRESS) private val dataStore: DataStore, + private val gson: Gson, + private val selectedSite: SelectedSite +) { + private fun getOriginAddressesKey() = "${selectedSite.getOrNull()?.siteId ?: ""}OriginAddresses" + + fun observeOriginAddresses(): Flow?> { + val typeToken = object : TypeToken>() {}.type + return dataStore.data.map { prefs -> + val storeOptions = prefs[stringPreferencesKey(getOriginAddressesKey())] + runCatching { + gson.fromJson>(storeOptions, typeToken) + }.getOrNull() + } + } + + suspend fun saveOriginAddresses(addresses: List) { + dataStore.edit { preferences -> + preferences[stringPreferencesKey(getOriginAddressesKey())] = gson.toJson(addresses) + } + } +} From 3796b51f5a5510601c3a842959e0fef5406a5628 Mon Sep 17 00:00:00 2001 From: Alejo Date: Fri, 27 Dec 2024 09:01:50 -0300 Subject: [PATCH 31/42] refactor ShippingLabelsDataStore -> WooShippingConfigurationDataStore --- ...gLabelsDataStore.kt => WooShippingConfigurationDataStore.kt} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/datasource/{ShippingLabelsDataStore.kt => WooShippingConfigurationDataStore.kt} (96%) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/datasource/ShippingLabelsDataStore.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/datasource/WooShippingConfigurationDataStore.kt similarity index 96% rename from WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/datasource/ShippingLabelsDataStore.kt rename to WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/datasource/WooShippingConfigurationDataStore.kt index 674158f8329..49f81ee29b8 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/datasource/ShippingLabelsDataStore.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/datasource/WooShippingConfigurationDataStore.kt @@ -13,7 +13,7 @@ import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.map import javax.inject.Inject -class ShippingLabelsDataStore @Inject constructor( +class WooShippingConfigurationDataStore @Inject constructor( @DataStoreQualifier(DataStoreType.SHIPPING_LABEL_CONFIGURATION) private val dataStore: DataStore, private val gson: Gson, private val selectedSite: SelectedSite From b44cb2b6739790a6b88ef38d0161238e005879e0 Mon Sep 17 00:00:00 2001 From: Alejo Date: Fri, 27 Dec 2024 09:02:26 -0300 Subject: [PATCH 32/42] add map support for OriginAddressDTO --- .../networking/WooShippingNetworkingMapper.kt | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/WooShippingNetworkingMapper.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/WooShippingNetworkingMapper.kt index 2dcca6b10fb..5f86b0bad2f 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/WooShippingNetworkingMapper.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/WooShippingNetworkingMapper.kt @@ -102,6 +102,27 @@ class WooShippingNetworkingMapper @Inject constructor( ) } + operator fun invoke(addressListDTO: Array): List { + return addressListDTO.map { + OriginShippingAddress( + id = it.id.orEmpty(), + address1 = it.address.orEmpty(), + address2 = it.address2.orEmpty(), + city = it.city.orEmpty(), + state = it.state.orEmpty(), + postcode = it.postcode.orEmpty(), + country = it.country.orEmpty(), + firstName = it.firstName.orEmpty(), + lastName = it.lastName.orEmpty(), + company = it.company.orEmpty(), + phone = it.phone.orEmpty(), + email = it.email.orEmpty(), + isDefault = it.defaultAddress, + isVerified = it.isVerified, + ) + } + } + fun toOriginAddressPurchaseDTO(address: OriginShippingAddress): OriginAddressPurchaseDTO { return OriginAddressPurchaseDTO( id = address.id, From 5c1877a91cc4fe298efbd113b54055662b7f2c43 Mon Sep 17 00:00:00 2001 From: Alejo Date: Fri, 27 Dec 2024 09:03:12 -0300 Subject: [PATCH 33/42] add network support for fetching origin addresses --- .../networking/WooShippingLabelRepository.kt | 12 ++++++++++++ .../networking/WooShippingLabelRestClient.kt | 11 +++++++++++ 2 files changed, 23 insertions(+) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/WooShippingLabelRepository.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/WooShippingLabelRepository.kt index 08986dca8cb..d1052ead007 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/WooShippingLabelRepository.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/WooShippingLabelRepository.kt @@ -90,4 +90,16 @@ class WooShippingLabelRepository @Inject constructor( markOrderComplete = lastOrderComplete ).asWooResult { mapper(it) } } + + suspend fun fetchOriginAddresses( + site: SiteModel + ) = restClient.fetchOriginAddresses(site = site) + .asWooResult { mapper(it) } + .also { response -> + response.model + ?.takeIf { response.isError.not() } + ?.let { + addressDataStore.saveOriginAddresses(it) + } + } } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/WooShippingLabelRestClient.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/WooShippingLabelRestClient.kt index 9eed218d8d6..56f99205153 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/WooShippingLabelRestClient.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/WooShippingLabelRestClient.kt @@ -107,4 +107,15 @@ class WooShippingLabelRestClient @Inject constructor( clazz = PurchasedShippingLabelResponseDTO::class.java, ).toWooPayload() } + + suspend fun fetchOriginAddresses( + site: SiteModel, + ): WooPayload> { + val url = "/wcshipping/v1/origin-addresses/" + return wooNetwork.executeGetGsonRequest( + site = site, + path = url, + clazz = Array::class.java, + ).toWooPayload() + } } From c8041941a6cd4275345031d3758fa599ff7eb388 Mon Sep 17 00:00:00 2001 From: Alejo Date: Fri, 27 Dec 2024 09:03:40 -0300 Subject: [PATCH 34/42] add fetch origin address use case --- .../address/FetchOriginAddresses.kt | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/FetchOriginAddresses.kt diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/FetchOriginAddresses.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/FetchOriginAddresses.kt new file mode 100644 index 00000000000..e452f4688df --- /dev/null +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/FetchOriginAddresses.kt @@ -0,0 +1,29 @@ +package com.woocommerce.android.ui.orders.wooshippinglabels.address + +import com.woocommerce.android.tools.SelectedSite +import com.woocommerce.android.ui.orders.wooshippinglabels.models.OriginShippingAddress +import com.woocommerce.android.ui.orders.wooshippinglabels.networking.WooShippingLabelRepository +import javax.inject.Inject + +class FetchOriginAddresses @Inject constructor( + private val shippingRepository: WooShippingLabelRepository, + private val selectedSite: SelectedSite +) { + suspend operator fun invoke(): Result> { + return selectedSite.getOrNull()?.let { + val response = shippingRepository.fetchOriginAddresses(it) + val result = response.model + when { + response.isError.not() && !result.isNullOrEmpty() -> { + Result.success(result) + } + + else -> { + val message = + response.error?.message ?: if (result.isNullOrEmpty()) "Empty result" else "Unknown error" + Result.failure(Exception(message)) + } + } + } ?: Result.failure(Exception("No site selected")) + } +} From 9c2948158fd4a097f7679f845c351a11b6591695 Mon Sep 17 00:00:00 2001 From: Alejo Date: Fri, 27 Dec 2024 09:13:13 -0300 Subject: [PATCH 35/42] refactor data store shipping label -> shipping configuration --- .../ui/orders/wooshippinglabels/ObserveStoreOptions.kt | 8 ++++---- .../networking/WooShippingLabelRepository.kt | 10 +++++++--- .../wooshippinglabels/ObserveStoreOptionsTest.kt | 8 ++++---- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptions.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptions.kt index 17533570523..b95bdadf81b 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptions.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptions.kt @@ -1,19 +1,19 @@ package com.woocommerce.android.ui.orders.wooshippinglabels -import com.woocommerce.android.ui.orders.wooshippinglabels.datasource.ShippingLabelsDataStore +import com.woocommerce.android.ui.orders.wooshippinglabels.datasource.WooShippingConfigurationDataStore import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.transformLatest import javax.inject.Inject class ObserveStoreOptions @Inject constructor( - val dataStore: ShippingLabelsDataStore, - val fetchAccountSettings: FetchAccountSettings, + private val configurationDataStore: WooShippingConfigurationDataStore, + private val fetchAccountSettings: FetchAccountSettings, ) { private var isFirstValue = true @OptIn(ExperimentalCoroutinesApi::class) // We will use data store as the source of truth and after the first emission we will refresh the values async. - operator fun invoke() = dataStore.observeStoreOptions().transformLatest { options -> + operator fun invoke() = configurationDataStore.observeStoreOptions().transformLatest { options -> when { isFirstValue && options == null -> { // If there is no cached data, refresh the store options before emitting any value diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/WooShippingLabelRepository.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/WooShippingLabelRepository.kt index d1052ead007..a88521b9888 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/WooShippingLabelRepository.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/networking/WooShippingLabelRepository.kt @@ -1,7 +1,8 @@ package com.woocommerce.android.ui.orders.wooshippinglabels.networking import com.woocommerce.android.model.Address -import com.woocommerce.android.ui.orders.wooshippinglabels.datasource.ShippingLabelsDataStore +import com.woocommerce.android.ui.orders.wooshippinglabels.datasource.WooShippingAddressDataStore +import com.woocommerce.android.ui.orders.wooshippinglabels.datasource.WooShippingConfigurationDataStore import com.woocommerce.android.ui.orders.wooshippinglabels.models.OriginShippingAddress import com.woocommerce.android.ui.orders.wooshippinglabels.models.PurchasedLabelData import com.woocommerce.android.ui.orders.wooshippinglabels.models.ShippingLabelStatus @@ -14,7 +15,8 @@ import javax.inject.Inject class WooShippingLabelRepository @Inject constructor( private val restClient: WooShippingLabelRestClient, private val mapper: WooShippingNetworkingMapper, - private val dataStore: ShippingLabelsDataStore + private val configurationDataStore: WooShippingConfigurationDataStore, + private val addressDataStore: WooShippingAddressDataStore ) { suspend fun fetchShippingLabelPrinting( site: SiteModel, @@ -34,7 +36,9 @@ class WooShippingLabelRepository @Inject constructor( .also { response -> response.model ?.takeIf { response.isError.not() } - ?.let { dataStore.saveStoreOptions(it) } + ?.let { + configurationDataStore.saveStoreOptions(it) + } } suspend fun fetchPurchasedShippingLabels( diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptionsTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptionsTest.kt index f34f01f77ab..b353c3d4442 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptionsTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptionsTest.kt @@ -1,6 +1,6 @@ package com.woocommerce.android.ui.orders.wooshippinglabels -import com.woocommerce.android.ui.orders.wooshippinglabels.datasource.ShippingLabelsDataStore +import com.woocommerce.android.ui.orders.wooshippinglabels.datasource.WooShippingConfigurationDataStore import com.woocommerce.android.ui.orders.wooshippinglabels.models.StoreOptionsModel import com.woocommerce.android.viewmodel.BaseUnitTest import junit.framework.TestCase.assertTrue @@ -15,9 +15,9 @@ import kotlin.test.Test @OptIn(ExperimentalCoroutinesApi::class) class ObserveStoreOptionsTest : BaseUnitTest() { - private val dataStore: ShippingLabelsDataStore = mock() + private val dataStore: WooShippingConfigurationDataStore = mock() private val fetchAccountSettings: FetchAccountSettings = mock() - val defaultStoreOptions = StoreOptionsModel( + private val defaultStoreOptions = StoreOptionsModel( weightUnit = "kg", currencySymbol = "$", dimensionUnit = "cm", @@ -25,7 +25,7 @@ class ObserveStoreOptionsTest : BaseUnitTest() { ) val sut = ObserveStoreOptions( - dataStore = dataStore, + configurationDataStore = dataStore, fetchAccountSettings = fetchAccountSettings ) From 88386022b655aee1d2d54c3726b7dac210763180 Mon Sep 17 00:00:00 2001 From: Alejo Date: Fri, 27 Dec 2024 09:26:08 -0300 Subject: [PATCH 36/42] prevent extra calls --- .../android/ui/orders/wooshippinglabels/ObserveStoreOptions.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptions.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptions.kt index b95bdadf81b..93824f4f964 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptions.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/ObserveStoreOptions.kt @@ -17,6 +17,7 @@ class ObserveStoreOptions @Inject constructor( when { isFirstValue && options == null -> { // If there is no cached data, refresh the store options before emitting any value + isFirstValue = false if (fetchAccountSettings().isFailure) { // We will use null as not available emit(null) @@ -25,6 +26,7 @@ class ObserveStoreOptions @Inject constructor( isFirstValue && options != null -> { // If there is cached data, emit cached values and refresh the store options async + isFirstValue = false emit(options) if (fetchAccountSettings().isFailure) { emit(null) @@ -33,6 +35,5 @@ class ObserveStoreOptions @Inject constructor( else -> emit(options) } - isFirstValue = false } } From 88da2a6f5bfc2827814b1009d8f30979fba56d15 Mon Sep 17 00:00:00 2001 From: Alejo Date: Fri, 27 Dec 2024 09:26:31 -0300 Subject: [PATCH 37/42] handle null results --- .../wooshippinglabels/WooShippingLabelCreationViewModel.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt index 15947f80f2b..9fb9befca50 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt @@ -206,7 +206,7 @@ class WooShippingLabelCreationViewModel @Inject constructor( private suspend fun getShippingAddresses() { order.combine(observeOriginAddresses()) { order, originAddresses -> - if (order != null && originAddresses.isNotEmpty()) { + if (order != null && !originAddresses.isNullOrEmpty()) { val selectedOriginAddress = getSelectedOriginAddress(originAddresses) WooShippingAddresses( shipFrom = selectedOriginAddress, From 69f57e6d8e8b39db736f2c6c6a117a0e4dc360c0 Mon Sep 17 00:00:00 2001 From: Alejo Date: Fri, 27 Dec 2024 09:26:56 -0300 Subject: [PATCH 38/42] connect with real data --- .../address/ObserveOriginAddresses.kt | 92 +++++++------------ 1 file changed, 33 insertions(+), 59 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/ObserveOriginAddresses.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/ObserveOriginAddresses.kt index 93f68585f17..1051fb210c6 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/ObserveOriginAddresses.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/ObserveOriginAddresses.kt @@ -1,65 +1,39 @@ package com.woocommerce.android.ui.orders.wooshippinglabels.address -import com.woocommerce.android.ui.orders.wooshippinglabels.models.OriginShippingAddress -import kotlinx.coroutines.delay -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.flowOf +import com.woocommerce.android.ui.orders.wooshippinglabels.datasource.WooShippingAddressDataStore +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.transformLatest import javax.inject.Inject -class ObserveOriginAddresses @Inject constructor() { - @Suppress("MagicNumber") - suspend operator fun invoke(): Flow> { - delay(200) - val addresses = listOf( - OriginShippingAddress( - firstName = "", - lastName = "", - company = "Shut up and sip", - phone = "55512345", - address1 = "60 29TH ST PMB 343", - address2 = "", - city = "SAN FRANCISCO", - postcode = "94110-4929", - email = "alejandro.torres@mail.com", - country = "US", - state = "CA", - id = "store_details", - isDefault = true, - isVerified = true - ), - OriginShippingAddress( - firstName = "first name", - lastName = "last name", - company = "Company", - phone = "", - address1 = "Another huge address that should be truncated", - address2 = "", - city = "Oakland", - postcode = "", - email = "email", - country = "USA", - state = "California", - id = "id_1", - isDefault = false, - isVerified = true - ), - OriginShippingAddress( - firstName = "first name", - lastName = "last name", - company = "Company", - phone = "", - address1 = "Small address", - address2 = "", - city = "Palo Alto", - postcode = "", - email = "email", - country = "USA", - state = "California", - id = "id_1", - isDefault = false, - isVerified = true - ) - ) - return flowOf(addresses) +class ObserveOriginAddresses @Inject constructor( + private val addressDataStore: WooShippingAddressDataStore, + private val fetchOriginAddresses: FetchOriginAddresses +) { + private var isFirstValue = true + + @OptIn(ExperimentalCoroutinesApi::class) + // We will use data store as the source of truth and after the first emission we will refresh the values async. + operator fun invoke() = addressDataStore.observeOriginAddresses().transformLatest { addresses -> + when { + isFirstValue && addresses == null -> { + // If there is no cached data, refresh the origin addresses before emitting any value + isFirstValue = false + if (fetchOriginAddresses().isFailure) { + // We will use null as not available + emit(null) + } + } + + isFirstValue && addresses != null -> { + // If there is cached data, emit cached values and refresh the origin addresses async + isFirstValue = false + emit(addresses) + if (fetchOriginAddresses().isFailure) { + emit(null) + } + } + + else -> emit(addresses) + } } } From 2783940e973d9c96722e7897e87c08da8e9654ae Mon Sep 17 00:00:00 2001 From: Alejo Date: Fri, 27 Dec 2024 09:54:32 -0300 Subject: [PATCH 39/42] add unit tests --- .../address/FetchOriginAddressesTest.kt | 86 +++++++++++++++++++ .../address/ObserveOriginAddressesTest.kt | 77 +++++++++++++++++ 2 files changed, 163 insertions(+) create mode 100644 WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/FetchOriginAddressesTest.kt create mode 100644 WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/ObserveOriginAddressesTest.kt diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/FetchOriginAddressesTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/FetchOriginAddressesTest.kt new file mode 100644 index 00000000000..12ab2285212 --- /dev/null +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/FetchOriginAddressesTest.kt @@ -0,0 +1,86 @@ +package com.woocommerce.android.ui.orders.wooshippinglabels.address + +import com.woocommerce.android.tools.SelectedSite +import com.woocommerce.android.ui.orders.wooshippinglabels.models.OriginShippingAddress +import com.woocommerce.android.ui.orders.wooshippinglabels.networking.WooShippingLabelRepository +import com.woocommerce.android.viewmodel.BaseUnitTest +import kotlinx.coroutines.ExperimentalCoroutinesApi +import org.mockito.kotlin.any +import org.mockito.kotlin.doReturn +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever +import org.wordpress.android.fluxc.model.SiteModel +import org.wordpress.android.fluxc.network.BaseRequest +import org.wordpress.android.fluxc.network.rest.wpcom.wc.WooError +import org.wordpress.android.fluxc.network.rest.wpcom.wc.WooErrorType +import org.wordpress.android.fluxc.network.rest.wpcom.wc.WooResult +import kotlin.test.Test + +@OptIn(ExperimentalCoroutinesApi::class) +class FetchOriginAddressesTest : BaseUnitTest() { + private val shippingRepository: WooShippingLabelRepository = mock() + private val selectedSite: SelectedSite = mock { + on { getOrNull() } doReturn SiteModel().apply { + url = "https://example.com" + } + } + + private val defaultOriginAddresses = OriginShippingAddress( + id = "1", + firstName = "John", + lastName = "Doe", + company = "", + address1 = "123 Main St", + address2 = "", + city = "Anytown", + state = "CA", + postcode = "12345", + country = "US", + email = "william.henry.harrison@example-pet-store.com", + phone = "555-555-5555", + isDefault = true, + isVerified = true + ) + + val sut = FetchOriginAddresses( + shippingRepository = shippingRepository, + selectedSite = selectedSite + ) + + @Test + fun `when selected site is null then return failure`() = testBlocking { + whenever(selectedSite.getOrNull()).doReturn(null) + val result = sut.invoke() + assert(result.isFailure) + } + + @Test + fun `when fetch origin addresses fails then return failure`() = testBlocking { + whenever(shippingRepository.fetchOriginAddresses(any())).doReturn( + WooResult(WooError(WooErrorType.GENERIC_ERROR, BaseRequest.GenericErrorType.UNKNOWN)) + ) + + val result = sut.invoke() + assert(result.isFailure) + } + + @Test + fun `when fetch origin addresses is empty then return failure`() = testBlocking { + whenever(shippingRepository.fetchOriginAddresses(any())).doReturn( + WooResult(emptyList()) + ) + + val result = sut.invoke() + assert(result.isFailure) + } + + @Test + fun `when fetch origin addresses succeed then return success`() = testBlocking { + whenever(shippingRepository.fetchOriginAddresses(any())).doReturn( + WooResult(listOf(defaultOriginAddresses)) + ) + + val result = sut.invoke() + assert(result.isSuccess) + } +} diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/ObserveOriginAddressesTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/ObserveOriginAddressesTest.kt new file mode 100644 index 00000000000..c25b2ef6ab9 --- /dev/null +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/address/ObserveOriginAddressesTest.kt @@ -0,0 +1,77 @@ +package com.woocommerce.android.ui.orders.wooshippinglabels.address + +import com.woocommerce.android.ui.orders.wooshippinglabels.datasource.WooShippingAddressDataStore +import com.woocommerce.android.ui.orders.wooshippinglabels.models.OriginShippingAddress +import com.woocommerce.android.viewmodel.BaseUnitTest +import junit.framework.TestCase.assertTrue +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.toList +import org.mockito.kotlin.doReturn +import org.mockito.kotlin.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import kotlin.test.Test + +@OptIn(ExperimentalCoroutinesApi::class) +class ObserveOriginAddressesTest : BaseUnitTest() { + private val dataStore: WooShippingAddressDataStore = mock() + private val fetchOriginAddresses: FetchOriginAddresses = mock() + private val defaultOriginAddresses = OriginShippingAddress( + id = "1", + firstName = "John", + lastName = "Doe", + company = "", + address1 = "123 Main St", + address2 = "", + city = "Anytown", + state = "CA", + postcode = "12345", + country = "US", + email = "william.henry.harrison@example-pet-store.com", + phone = "555-555-5555", + isDefault = true, + isVerified = true + ) + + val sut = ObserveOriginAddresses( + addressDataStore = dataStore, + fetchOriginAddresses = fetchOriginAddresses + ) + + @Test + fun `when there is NO cached data and fetch account settings fails then return null`() = testBlocking { + whenever(dataStore.observeOriginAddresses()).doReturn(flowOf(null)) + whenever(fetchOriginAddresses.invoke()).doReturn(Result.failure(Exception("Random error"))) + val result = sut.invoke().toList() + + assertTrue(result.size == 1) + assertTrue(result.first() == null) + } + + @Test + fun `when there is cached data and fetch account settings fails then return null`() = testBlocking { + val cachedResult = listOf(defaultOriginAddresses) + whenever(dataStore.observeOriginAddresses()).doReturn(flowOf(cachedResult)) + whenever(fetchOriginAddresses.invoke()).doReturn(Result.failure(Exception("Random error"))) + val result = sut.invoke().toList() + + assertTrue(result.size == 2) + assertTrue(result.first() == cachedResult) + assertTrue(result.last() == null) + } + + @Test + fun `refresh data only once`() = testBlocking { + val firstResult = listOf(defaultOriginAddresses) + val secondResult = listOf(defaultOriginAddresses.copy(id = "updated id")) + whenever(dataStore.observeOriginAddresses()).doReturn( + flowOf(firstResult, secondResult) + ) + whenever(fetchOriginAddresses.invoke()).doReturn(Result.failure(Exception("Random error"))) + + sut.invoke().toList() + + verify(fetchOriginAddresses).invoke() + } +} From 35ea4f799d2f66025a0aaf3215cee14654d3a2fa Mon Sep 17 00:00:00 2001 From: Hafiz Rahman Date: Mon, 30 Dec 2024 09:44:33 +0700 Subject: [PATCH 40/42] remove unused import --- .../com/woocommerce/android/ui/compose/component/OverflowMenu.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/compose/component/OverflowMenu.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/compose/component/OverflowMenu.kt index e7a466b1cac..0e7684d19aa 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/compose/component/OverflowMenu.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/compose/component/OverflowMenu.kt @@ -3,7 +3,6 @@ package com.woocommerce.android.ui.compose.component import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.height -import androidx.compose.foundation.layout.heightIn import androidx.compose.material.DropdownMenu import androidx.compose.material.DropdownMenuItem import androidx.compose.material.Icon From 0c2ba56ac832cebcb5073041054a452d8c2b78c7 Mon Sep 17 00:00:00 2001 From: jorgemucientesfayos Date: Mon, 30 Dec 2024 09:56:46 +0100 Subject: [PATCH 41/42] Several renaming for consistency --- .../com/woocommerce/android/datastore/DataStoreModule.kt | 6 +++--- .../com/woocommerce/android/datastore/DataStoreType.kt | 2 +- .../sitepicker/sitevisibility/VisibleWooSitesDataStore.kt | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreModule.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreModule.kt index 28c5b89f172..a31c5561d74 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreModule.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreModule.kt @@ -14,9 +14,9 @@ import com.woocommerce.android.datastore.DataStoreType.ANALYTICS_UI_CACHE import com.woocommerce.android.datastore.DataStoreType.COUPONS import com.woocommerce.android.datastore.DataStoreType.DASHBOARD_STATS import com.woocommerce.android.datastore.DataStoreType.LAST_UPDATE -import com.woocommerce.android.datastore.DataStoreType.SITE_PICKER_HIDDEN_SITES import com.woocommerce.android.datastore.DataStoreType.SHIPPING_LABEL_ADDRESS import com.woocommerce.android.datastore.DataStoreType.SHIPPING_LABEL_CONFIGURATION +import com.woocommerce.android.datastore.DataStoreType.SITE_PICKER_WOO_VISIBLE_SITES import com.woocommerce.android.datastore.DataStoreType.TOP_PERFORMER_PRODUCTS import com.woocommerce.android.datastore.DataStoreType.TRACKER import com.woocommerce.android.di.AppCoroutineScope @@ -164,7 +164,7 @@ class DataStoreModule { @Provides @Singleton - @DataStoreQualifier(SITE_PICKER_HIDDEN_SITES) + @DataStoreQualifier(SITE_PICKER_WOO_VISIBLE_SITES) fun provideWooVisibleSitesDataStore( appContext: Context, crashLogging: CrashLogging, @@ -174,7 +174,7 @@ class DataStoreModule { appContext.preferencesDataStoreFile("site_picker_visible_sites") }, corruptionHandler = ReplaceFileCorruptionHandler { - crashLogging.recordEvent("Corrupted data store. DataStore Type: ${SITE_PICKER_HIDDEN_SITES.name}") + crashLogging.recordEvent("Corrupted data store. DataStore Type: ${SITE_PICKER_WOO_VISIBLE_SITES.name}") emptyPreferences() }, scope = CoroutineScope(appCoroutineScope.coroutineContext + Dispatchers.IO) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreType.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreType.kt index e71a8ba20fd..3cacd5bba82 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreType.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/datastore/DataStoreType.kt @@ -8,7 +8,7 @@ enum class DataStoreType { TOP_PERFORMER_PRODUCTS, COUPONS, LAST_UPDATE, - SITE_PICKER_HIDDEN_SITES, + SITE_PICKER_WOO_VISIBLE_SITES, SHIPPING_LABEL_CONFIGURATION, SHIPPING_LABEL_ADDRESS } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/sitepicker/sitevisibility/VisibleWooSitesDataStore.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/sitepicker/sitevisibility/VisibleWooSitesDataStore.kt index b9a02a7037b..35231e7bc60 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/sitepicker/sitevisibility/VisibleWooSitesDataStore.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/sitepicker/sitevisibility/VisibleWooSitesDataStore.kt @@ -11,11 +11,11 @@ import kotlinx.coroutines.flow.map import javax.inject.Inject class VisibleWooSitesDataStore @Inject constructor( - @DataStoreQualifier(DataStoreType.SITE_PICKER_HIDDEN_SITES) private val dataStore: DataStore + @DataStoreQualifier(DataStoreType.SITE_PICKER_WOO_VISIBLE_SITES) private val dataStore: DataStore ) { suspend fun updateSiteVisibilityStatus(siteIds: Map) { - siteIds.forEach { (siteId, hidden) -> - updateSiteVisibility(siteId, hidden) + siteIds.forEach { (siteId, isVisible) -> + updateSiteVisibility(siteId, isVisible) } } From 87e92ed74ff73e45edffc4351f3062e5ff20874d Mon Sep 17 00:00:00 2001 From: jorgemucientesfayos Date: Mon, 30 Dec 2024 09:57:13 +0100 Subject: [PATCH 42/42] Simplify boolean condition --- .../ui/sitepicker/sitevisibility/VisibleWooSitesDataStore.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/sitepicker/sitevisibility/VisibleWooSitesDataStore.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/sitepicker/sitevisibility/VisibleWooSitesDataStore.kt index 35231e7bc60..a118eebe14a 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/sitepicker/sitevisibility/VisibleWooSitesDataStore.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/sitepicker/sitevisibility/VisibleWooSitesDataStore.kt @@ -20,7 +20,7 @@ class VisibleWooSitesDataStore @Inject constructor( } fun isSiteVisible(siteId: Long): Flow { - return dataStore.data.map { prefs -> prefs[booleanPreferencesKey(siteId.toString())] ?: true } + return dataStore.data.map { prefs -> prefs[booleanPreferencesKey(siteId.toString())] != false } } suspend fun clearAll() {