From c46bf84ac9efa46137da08a87eabba160118839a Mon Sep 17 00:00:00 2001 From: Mihai Branescu Date: Tue, 10 Dec 2019 22:10:01 +0200 Subject: [PATCH] For #6330 Collections Numbering (#6453) * For #6330 - Added logic for getting the recommended default collection name * For #6330 - Added unit test for default collection number method --- .../CollectionCreationController.kt | 45 +++++++++--- .../collections/CollectionCreationStore.kt | 11 ++- .../collections/CollectionCreationView.kt | 2 +- ...DefaultCollectionCreationControllerTest.kt | 72 ++++++++++++++++++- 4 files changed, 117 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationController.kt b/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationController.kt index 5e42c3852..c07ef117d 100644 --- a/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationController.kt +++ b/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationController.kt @@ -69,6 +69,12 @@ class DefaultCollectionCreationController( private val sessionManager: SessionManager, private val lifecycleScope: CoroutineScope ) : CollectionCreationController { + + companion object { + const val DEFAULT_INCREMENT_VALUE = 1 + const val DEFAULT_COLLECTION_NUMBER_POSITION = 1 + } + override fun saveCollectionName(tabs: List, name: String) { dismiss() @@ -124,17 +130,40 @@ class DefaultCollectionCreationController( } override fun saveTabsToCollection(tabs: List) { - store.dispatch(CollectionCreationAction.StepChanged( - saveCollectionStep = if (store.state.tabCollections.isEmpty()) { - SaveCollectionStep.NameCollection - } else { - SaveCollectionStep.SelectCollection - } - )) + store.dispatch( + CollectionCreationAction.StepChanged( + saveCollectionStep = if (store.state.tabCollections.isEmpty()) { + SaveCollectionStep.NameCollection + } else { + SaveCollectionStep.SelectCollection + }, + defaultCollectionNumber = getDefaultCollectionNumber() + ) + ) } override fun addNewCollection() { - store.dispatch(CollectionCreationAction.StepChanged(SaveCollectionStep.NameCollection)) + store.dispatch( + CollectionCreationAction.StepChanged( + SaveCollectionStep.NameCollection, + getDefaultCollectionNumber() + ) + ) + } + + /** + * Returns the new default name recommendation for a collection + * + * Algorithm: Go through all collections, make a list of their names and keep only the default ones. + * Then get the numbers from all these default names, compute the maximum number and add one. + */ + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + fun getDefaultCollectionNumber(): Int { + return (store.state.tabCollections + .map { it.title } + .filter { it.matches(Regex("Collection\\s\\d+")) } + .map { Integer.valueOf(it.split(" ")[DEFAULT_COLLECTION_NUMBER_POSITION]) } + .max() ?: 0) + DEFAULT_INCREMENT_VALUE } override fun addTabToSelection(tab: Tab) { diff --git a/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationStore.kt b/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationStore.kt index 5f3cac998..d30d8176e 100644 --- a/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationStore.kt +++ b/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationStore.kt @@ -40,7 +40,8 @@ data class CollectionCreationState( val selectedTabs: Set = emptySet(), val saveCollectionStep: SaveCollectionStep = SaveCollectionStep.SelectTabs, val tabCollections: List = emptyList(), - val selectedTabCollection: TabCollection? = null + val selectedTabCollection: TabCollection? = null, + val defaultCollectionNumber: Int = 1 ) : State sealed class CollectionCreationAction : Action { @@ -53,7 +54,10 @@ sealed class CollectionCreationAction : Action { * * This should be refactored, see kdoc on [SaveCollectionStep]. */ - data class StepChanged(val saveCollectionStep: SaveCollectionStep) : CollectionCreationAction() + data class StepChanged( + val saveCollectionStep: SaveCollectionStep, + val defaultCollectionNumber: Int = 1 + ) : CollectionCreationAction() } private fun collectionCreationReducer( @@ -64,5 +68,6 @@ private fun collectionCreationReducer( is CollectionCreationAction.RemoveAllTabs -> prevState.copy(selectedTabs = emptySet()) is CollectionCreationAction.TabAdded -> prevState.copy(selectedTabs = prevState.selectedTabs + action.tab) is CollectionCreationAction.TabRemoved -> prevState.copy(selectedTabs = prevState.selectedTabs - action.tab) - is CollectionCreationAction.StepChanged -> prevState.copy(saveCollectionStep = action.saveCollectionStep) + is CollectionCreationAction.StepChanged -> prevState.copy(saveCollectionStep = action.saveCollectionStep, + defaultCollectionNumber = action.defaultCollectionNumber) } diff --git a/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationView.kt b/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationView.kt index 4286c22e6..c2183f21e 100644 --- a/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationView.kt +++ b/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationView.kt @@ -252,7 +252,7 @@ class CollectionCreationView( name_collection_edittext.setText( view.context.getString( R.string.create_collection_default_name, - state.tabCollections.size + 1 + state.defaultCollectionNumber ) ) name_collection_edittext.setSelection(0, name_collection_edittext.text.length) diff --git a/app/src/test/java/org/mozilla/fenix/collections/DefaultCollectionCreationControllerTest.kt b/app/src/test/java/org/mozilla/fenix/collections/DefaultCollectionCreationControllerTest.kt index 38355527c..f8c3f6eba 100644 --- a/app/src/test/java/org/mozilla/fenix/collections/DefaultCollectionCreationControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/collections/DefaultCollectionCreationControllerTest.kt @@ -4,10 +4,12 @@ import io.mockk.MockKAnnotations import io.mockk.every import io.mockk.impl.annotations.MockK import io.mockk.mockk +import io.mockk.verify import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.TestCoroutineScope import mozilla.components.browser.session.Session import mozilla.components.browser.session.SessionManager +import mozilla.components.feature.tab.collections.TabCollection import mozilla.components.feature.tabs.TabsUseCases import org.junit.Assert.assertEquals import org.junit.Assert.assertNull @@ -23,7 +25,7 @@ class DefaultCollectionCreationControllerTest { private lateinit var controller: DefaultCollectionCreationController - @MockK private lateinit var store: CollectionCreationStore + @MockK(relaxed = true) private lateinit var store: CollectionCreationStore @MockK(relaxed = true) private lateinit var dismiss: () -> Unit @MockK(relaxed = true) private lateinit var analytics: Analytics @MockK private lateinit var tabCollectionStorage: TabCollectionStorage @@ -92,6 +94,74 @@ class DefaultCollectionCreationControllerTest { assertEquals(SaveCollectionStep.SelectCollection, controller.stepBack(SaveCollectionStep.NameCollection)) } + @Test + fun `GIVEN list of collections WHEN default collection number is required THEN return next default number`() { + val collections: MutableList = ArrayList() + collections.add(mockk { + every { title } returns "Collection 1" + }) + collections.add(mockk { + every { title } returns "Collection 2" + }) + collections.add(mockk { + every { title } returns "Collection 3" + }) + every { state.tabCollections } returns collections + + assertEquals(4, controller.getDefaultCollectionNumber()) + + collections.add(mockk { + every { title } returns "Collection 5" + }) + assertEquals(6, controller.getDefaultCollectionNumber()) + + collections.add(mockk { + every { title } returns "Random name" + }) + assertEquals(6, controller.getDefaultCollectionNumber()) + + collections.add(mockk { + every { title } returns "Collection 10 10" + }) + assertEquals(6, controller.getDefaultCollectionNumber()) + } + + @Test + fun `WHEN adding a new collection THEN dispatch NameCollection step changed`() { + val collections: List = ArrayList() + every { state.tabCollections } returns collections + + controller.addNewCollection() + + verify { store.dispatch(CollectionCreationAction.StepChanged(SaveCollectionStep.NameCollection, 1)) } + } + + @Test + fun `GIVEN empty list of collections WHEN saving tabs to collection THEN dispatch NameCollection step changed`() { + val collections: List = ArrayList() + every { state.tabCollections } returns collections + + controller.saveTabsToCollection(ArrayList()) + + verify { store.dispatch(CollectionCreationAction.StepChanged(SaveCollectionStep.NameCollection, 1)) } + } + + @Test + fun `GIVEN list of collections WHEN saving tabs to collection THEN dispatch NameCollection step changed`() { + val collections: MutableList = ArrayList() + collections.add(mockk { + every { title } returns "Collection 1" + }) + collections.add(mockk { + every { title } returns "Random Collection" + }) + every { state.tabCollections } returns collections + + controller.saveTabsToCollection(ArrayList()) + + verify { store.dispatch(CollectionCreationAction.StepChanged(SaveCollectionStep.SelectCollection, 2)) } + } + @Test fun `normalSessionSize only counts non-private non-custom sessions`() { fun session(isPrivate: Boolean, isCustom: Boolean) = mockk().apply {