Merge pull request #13539 from woocommerce/issue/fix-missing-done-button-in-service-packages

[Shipping Labels] Fix missing done button in service packages

Author: ThomazFB

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/shippinglabels/creation/ShippingLabelCreateCustomPackageFragment.kt

@@ -1,12 +1,10 @@
 package com.woocommerce.android.ui.orders.shippinglabels.creation
 
 import android.os.Bundle
-import android.view.MenuItem
 import android.view.View
 import androidx.annotation.StringRes
-import androidx.appcompat.content.res.AppCompatResources
 import androidx.fragment.app.viewModels
-import androidx.navigation.fragment.findNavController
+import androidx.lifecycle.lifecycleScope
 import com.woocommerce.android.R
 import com.woocommerce.android.databinding.FragmentShippingLabelCreateCustomPackageBinding
 import com.woocommerce.android.extensions.takeIfNotEqualTo
@@ -16,9 +14,13 @@ import com.woocommerce.android.ui.base.UIMessageResolver
 import com.woocommerce.android.ui.main.AppBarStatus
 import com.woocommerce.android.ui.orders.shippinglabels.creation.ShippingLabelCreateCustomPackageViewModel.InputName
 import com.woocommerce.android.ui.orders.shippinglabels.creation.ShippingLabelCreateCustomPackageViewModel.PackageSuccessfullyMadeEvent
+import com.woocommerce.android.ui.orders.shippinglabels.creation.ShippingLabelCreatePackageViewModel.PackageType.CUSTOM
 import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ShowSnackbar
 import com.woocommerce.android.widgets.CustomProgressDialog
 import dagger.hilt.android.AndroidEntryPoint
+import kotlinx.coroutines.flow.filter
+import kotlinx.coroutines.flow.launchIn
+import kotlinx.coroutines.flow.onEach
 import org.wordpress.android.util.ActivityUtils
 import javax.inject.Inject
 
@@ -42,27 +44,11 @@ class ShippingLabelCreateCustomPackageFragment :
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
         super.onViewCreated(view, savedInstanceState)
         _binding = FragmentShippingLabelCreateCustomPackageBinding.bind(view)
-        setupToolbar()
 
         initializeInputFields()
         setupObservers()
     }
 
-    private fun setupToolbar() {
-        binding.toolbar.title = getString(R.string.shipping_label_package_selector_title)
-        binding.toolbar.setOnMenuItemClickListener { menuItem ->
-            onMenuItemSelected(menuItem)
-        }
-        binding.toolbar.navigationIcon = AppCompatResources.getDrawable(
-            requireActivity(),
-            R.drawable.ic_back_24dp
-        )
-        binding.toolbar.setNavigationOnClickListener {
-            findNavController().navigateUp()
-        }
-        binding.toolbar.inflateMenu(R.menu.menu_done)
-    }
-
     private fun initializeInputFields() {
         // Initialize spinner
         binding.typeSpinner.setup(
@@ -154,6 +140,13 @@ class ShippingLabelCreateCustomPackageFragment :
                 else -> event.isHandled = false
             }
         }
+
+        parentViewModel.creationDoneFlow
+            .filter { it.selectedTab == CUSTOM }
+            .onEach {
+                ActivityUtils.hideKeyboard(activity)
+                viewModel.onCustomFormDoneMenuClicked()
+            }.launchIn(viewLifecycleOwner.lifecycleScope)
     }
 
     private fun showProgressDialog(@StringRes title: Int, @StringRes message: Int) {
@@ -169,15 +162,4 @@ class ShippingLabelCreateCustomPackageFragment :
         progressDialog?.dismiss()
         progressDialog = null
     }
-
-    private fun onMenuItemSelected(item: MenuItem): Boolean {
-        return when (item.itemId) {
-            R.id.menu_done -> {
-                ActivityUtils.hideKeyboard(activity)
-                viewModel.onCustomFormDoneMenuClicked()
-                true
-            }
-            else -> false
-        }
-    }
 }

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/shippinglabels/creation/ShippingLabelCreatePackageFragment.kt

@@ -14,6 +14,7 @@ import com.woocommerce.android.extensions.navigateBackWithResult
 import com.woocommerce.android.ui.base.BaseFragment
 import com.woocommerce.android.ui.base.UIMessageResolver
 import com.woocommerce.android.ui.main.AppBarStatus
+import com.woocommerce.android.ui.orders.shippinglabels.creation.ShippingLabelCreatePackageViewModel.PackageType
 import com.woocommerce.android.ui.orders.shippinglabels.creation.ShippingPackageSelectorFragment.Companion.SELECTED_PACKAGE_RESULT
 import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ExitWithResult
 import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ShowSnackbar
@@ -40,6 +41,7 @@ class ShippingLabelCreatePackageFragment : BaseFragment(R.layout.fragment_shippi
         viewPager.adapter = adapter
 
         initializeTabs(tabLayout, viewPager)
+        initializePageChangeCallback(viewPager)
         setupObservers(viewModel)
     }
 
@@ -52,6 +54,16 @@ class ShippingLabelCreatePackageFragment : BaseFragment(R.layout.fragment_shippi
         binding.toolbar.setNavigationOnClickListener {
             findNavController().navigateUp()
         }
+        binding.toolbar.setOnMenuItemClickListener { item ->
+            when (item.itemId) {
+                R.id.menu_done -> {
+                    viewModel.onDoneButtonClicked()
+                    true
+                }
+                else -> false
+            }
+        }
+        binding.toolbar.inflateMenu(R.menu.menu_done)
     }
 
     private fun initializeTabs(tabLayout: TabLayout, viewPager: ViewPager2) {
@@ -61,6 +73,14 @@ class ShippingLabelCreatePackageFragment : BaseFragment(R.layout.fragment_shippi
         }.attach()
     }
 
+    private fun initializePageChangeCallback(viewPager: ViewPager2) {
+        viewPager.registerOnPageChangeCallback(object : ViewPager2.OnPageChangeCallback() {
+            override fun onPageSelected(position: Int) {
+                viewModel.onSelectedPageChanged(PackageType.fromOrdinal(position))
+            }
+        })
+    }
+
     private fun setupObservers(viewModel: ShippingLabelCreatePackageViewModel) {
         viewModel.event.observe(viewLifecycleOwner) { event ->
             when (event) {

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/shippinglabels/creation/ShippingLabelCreatePackageViewModel.kt

@@ -1,25 +1,36 @@
 package com.woocommerce.android.ui.orders.shippinglabels.creation
 
-import android.os.Parcelable
 import androidx.lifecycle.SavedStateHandle
+import androidx.lifecycle.viewModelScope
 import com.woocommerce.android.R
 import com.woocommerce.android.analytics.AnalyticsEvent
 import com.woocommerce.android.analytics.AnalyticsTracker
 import com.woocommerce.android.model.ShippingPackage
-import com.woocommerce.android.viewmodel.LiveDataDelegate
+import com.woocommerce.android.viewmodel.MultiLiveEvent
 import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ExitWithResult
 import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ShowSnackbar
 import com.woocommerce.android.viewmodel.ScopedViewModel
+import com.woocommerce.android.viewmodel.getStateFlow
 import com.woocommerce.android.viewmodel.navArgs
 import dagger.hilt.android.lifecycle.HiltViewModel
-import kotlinx.parcelize.Parcelize
+import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.MutableSharedFlow
+import kotlinx.coroutines.flow.asSharedFlow
+import kotlinx.coroutines.flow.update
+import kotlinx.coroutines.launch
 import javax.inject.Inject
 
 @HiltViewModel
 class ShippingLabelCreatePackageViewModel @Inject constructor(
     savedState: SavedStateHandle
 ) : ScopedViewModel(savedState) {
     private val arguments: ShippingLabelCreatePackageFragmentArgs by savedState.navArgs()
+    private val selectedTabType = savedState.getStateFlow(
+        scope = viewModelScope,
+        initialValue = PackageType.CUSTOM
+    )
+    private val _creationDoneFlow = MutableSharedFlow<OnDoneButtonClicked>()
+    val creationDoneFlow: Flow<OnDoneButtonClicked> = _creationDoneFlow.asSharedFlow()
 
     fun onPackageCreated(madePackage: ShippingPackage) {
         val type = if (madePackage.category == ShippingPackage.CUSTOM_PACKAGE_CATEGORY) "custom" else "predefined"
@@ -45,21 +56,24 @@ class ShippingLabelCreatePackageViewModel @Inject constructor(
         )
     }
 
-    /**
-     * Saving more data than necessary into the SavedState has associated risks which were not known at the time this
-     * field was implemented - after we ensure we don't save unnecessary data, we can replace @Suppress("OPT_IN_USAGE")
-     * with @OptIn(LiveDelegateSavedStateAPI::class).
-     */
-    @Suppress("OPT_IN_USAGE")
-    val viewStateData = LiveDataDelegate(savedState, ShippingLabelCreatePackageViewState())
+    fun onSelectedPageChanged(selectedTab: PackageType) {
+        selectedTabType.update { selectedTab }
+    }
 
-    @Parcelize
-    data class ShippingLabelCreatePackageViewState(
-        val selectedTab: PackageType = PackageType.CUSTOM
-    ) : Parcelable
+    fun onDoneButtonClicked() {
+        launch { _creationDoneFlow.emit(OnDoneButtonClicked(selectedTab = selectedTabType.value)) }
+    }
 
     enum class PackageType {
         CUSTOM,
-        SERVICE
+        SERVICE;
+
+        companion object {
+            fun fromOrdinal(ordinal: Int) = entries[ordinal]
+        }
     }
+
+    data class OnDoneButtonClicked(
+        val selectedTab: PackageType
+    ) : MultiLiveEvent.Event()
 }

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/shippinglabels/creation/ShippingLabelCreatePackageViewPagerAdapter.kt

@@ -8,7 +8,7 @@ class ShippingLabelCreatePackageViewPagerAdapter(container: Fragment) : Fragment
     override fun getItemCount(): Int = PackageType.values().size
 
     override fun createFragment(position: Int): Fragment {
-        return when (PackageType.values()[position]) {
+        return when (PackageType.entries[position]) {
             PackageType.CUSTOM -> ShippingLabelCreateCustomPackageFragment()
             PackageType.SERVICE -> ShippingLabelCreateServicePackageFragment()
         }

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/shippinglabels/creation/ShippingLabelCreateServicePackageFragment.kt

@@ -1,33 +1,34 @@
 package com.woocommerce.android.ui.orders.shippinglabels.creation
 
 import android.os.Bundle
-import android.view.Menu
-import android.view.MenuInflater
 import android.view.MenuItem
 import android.view.View
 import androidx.annotation.StringRes
-import androidx.core.view.MenuProvider
 import androidx.core.view.isVisible
 import androidx.fragment.app.viewModels
+import androidx.lifecycle.lifecycleScope
 import androidx.recyclerview.widget.LinearLayoutManager
 import com.woocommerce.android.R
 import com.woocommerce.android.databinding.FragmentShippingLabelCreateServicePackageBinding
 import com.woocommerce.android.extensions.takeIfNotEqualTo
 import com.woocommerce.android.ui.base.BaseFragment
 import com.woocommerce.android.ui.base.UIMessageResolver
+import com.woocommerce.android.ui.orders.shippinglabels.creation.ShippingLabelCreatePackageViewModel.PackageType.SERVICE
 import com.woocommerce.android.ui.orders.shippinglabels.creation.ShippingLabelCreateServicePackageViewModel.PackageSuccessfullyMadeEvent
 import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ShowSnackbar
 import com.woocommerce.android.widgets.CustomProgressDialog
 import com.woocommerce.android.widgets.SkeletonView
 import com.woocommerce.android.widgets.WCEmptyView
 import dagger.hilt.android.AndroidEntryPoint
+import kotlinx.coroutines.flow.filter
+import kotlinx.coroutines.flow.launchIn
+import kotlinx.coroutines.flow.onEach
 import org.wordpress.android.util.ActivityUtils
 import javax.inject.Inject
 
 @AndroidEntryPoint
 class ShippingLabelCreateServicePackageFragment :
-    BaseFragment(R.layout.fragment_shipping_label_create_service_package),
-    MenuProvider {
+    BaseFragment(R.layout.fragment_shipping_label_create_service_package) {
     @Inject lateinit var uiMessageResolver: UIMessageResolver
     private val skeletonView: SkeletonView = SkeletonView()
     private var progressDialog: CustomProgressDialog? = null
@@ -36,18 +37,9 @@ class ShippingLabelCreateServicePackageFragment :
     private val parentViewModel: ShippingLabelCreatePackageViewModel by viewModels({ requireParentFragment() })
     val viewModel: ShippingLabelCreateServicePackageViewModel by viewModels()
 
-    override fun onCreateMenu(menu: Menu, inflater: MenuInflater) {
-        menu.clear()
-        inflater.inflate(R.menu.menu_done, menu)
-        doneMenuItem = menu.findItem(R.id.menu_done)
-        doneMenuItem.isVisible = viewModel.viewStateData.liveData.value?.canSave ?: false
-    }
-
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
         super.onViewCreated(view, savedInstanceState)
 
-        requireActivity().addMenuProvider(this, viewLifecycleOwner)
-
         val binding = FragmentShippingLabelCreateServicePackageBinding.bind(view)
         val packagesAdapter = ShippingLabelServicePackageAdapter(
             viewModel::onPackageSelected,
@@ -113,17 +105,13 @@ class ShippingLabelCreateServicePackageFragment :
                 else -> event.isHandled = false
             }
         }
-    }
 
-    override fun onMenuItemSelected(item: MenuItem): Boolean {
-        return when (item.itemId) {
-            R.id.menu_done -> {
+        parentViewModel.creationDoneFlow
+            .filter { it.selectedTab == SERVICE }
+            .onEach {
                 ActivityUtils.hideKeyboard(activity)
                 viewModel.onCustomFormDoneMenuClicked()
-                true
-            }
-            else -> false
-        }
+            }.launchIn(viewLifecycleOwner.lifecycleScope)
     }
 
     private fun showSkeleton(show: Boolean, binding: FragmentShippingLabelCreateServicePackageBinding) {

WooCommerce/src/main/res/layout/fragment_shipping_label_create_custom_package.xml

@@ -1,23 +1,10 @@
 <?xml version="1.0" encoding="utf-8"?>
 <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
     xmlns:app="http://schemas.android.com/apk/res-auto"
-    xmlns:tools="http://schemas.android.com/tools"
     android:layout_width="match_parent"
     android:layout_height="match_parent"
     android:orientation="vertical">
 
-    <com.google.android.material.appbar.MaterialToolbar
-        android:id="@+id/toolbar"
-        style="@style/Widget.Woo.Toolbar"
-        android:layout_width="match_parent"
-        android:layout_height="@dimen/toolbar_height"
-        android:elevation="@dimen/appbar_elevation"
-        app:layout_collapseMode="pin"
-        app:layout_constraintEnd_toEndOf="parent"
-        app:layout_constraintStart_toStartOf="parent"
-        app:layout_constraintTop_toTopOf="parent"
-        tools:title="@string/app_name" />
-
     <View
         android:id="@+id/app_bar_divider"
         style="@style/Woo.Divider"

WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/shippinglabels/creation/ShippingLabelCreatePackageViewModelTest.kt

@@ -3,11 +3,16 @@ package com.woocommerce.android.ui.orders.shippinglabels.creation
 import com.woocommerce.android.R
 import com.woocommerce.android.model.PackageDimensions
 import com.woocommerce.android.model.ShippingPackage
+import com.woocommerce.android.ui.orders.shippinglabels.creation.ShippingLabelCreatePackageViewModel.OnDoneButtonClicked
+import com.woocommerce.android.ui.orders.shippinglabels.creation.ShippingLabelCreatePackageViewModel.PackageType
 import com.woocommerce.android.viewmodel.BaseUnitTest
 import com.woocommerce.android.viewmodel.MultiLiveEvent.Event
 import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ShowSnackbar
 import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.flow.launchIn
+import kotlinx.coroutines.flow.onEach
 import org.assertj.core.api.Assertions.assertThat
+import org.junit.Before
 import org.junit.Test
 
 @ExperimentalCoroutinesApi
@@ -23,14 +28,14 @@ class ShippingLabelCreatePackageViewModelTest : BaseUnitTest() {
         1f
     )
 
-    fun setup() {
+    @Before
+    fun setUp() {
         val savedState = ShippingLabelCreatePackageFragmentArgs(0).toSavedStateHandle()
         viewModel = ShippingLabelCreatePackageViewModel(savedState)
     }
 
     @Test
     fun `when handling package creation success, then show Snackbar and navigate to edit label package screen`() {
-        setup()
         val events = mutableListOf<Event>()
         viewModel.event.observeForever {
             events.add(it)
@@ -53,4 +58,33 @@ class ShippingLabelCreatePackageViewModelTest : BaseUnitTest() {
             )
         )
     }
+
+    @Test
+    fun `when onDoneButtonClicked is called, then creationDoneFlow emits the correct event`() = testBlocking {
+        // Given
+        val expectedEvent = OnDoneButtonClicked(PackageType.CUSTOM)
+        var receivedEvent: OnDoneButtonClicked? = null
+        viewModel.creationDoneFlow.onEach { receivedEvent = it }.launchIn(this.backgroundScope)
+
+        // When
+        viewModel.onDoneButtonClicked()
+
+        // Then
+        assertThat(receivedEvent).isEqualTo(expectedEvent)
+    }
+
+    @Test
+    fun `when onSelectedPageChanged is called, then selectedTabType is updated`() = testBlocking {
+        // Given
+        val expectedEvent = OnDoneButtonClicked(PackageType.SERVICE)
+        var receivedEvent: OnDoneButtonClicked? = null
+        viewModel.creationDoneFlow.onEach { receivedEvent = it }.launchIn(this.backgroundScope)
+
+        // When
+        viewModel.onSelectedPageChanged(PackageType.SERVICE)
+        viewModel.onDoneButtonClicked()
+
+        // Then
+        assertThat(receivedEvent).isEqualTo(expectedEvent)
+    }
 }