Commit c46bf84a authored by Mihai Branescu's avatar Mihai Branescu Committed by Tiger Oakes
Browse files

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
parent fe034226
Loading
Loading
Loading
Loading
+37 −8
Original line number Diff line number Diff line
@@ -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<Tab>, name: String) {
        dismiss()

@@ -124,17 +130,40 @@ class DefaultCollectionCreationController(
    }

    override fun saveTabsToCollection(tabs: List<Tab>) {
        store.dispatch(CollectionCreationAction.StepChanged(
        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) {
+8 −3
Original line number Diff line number Diff line
@@ -40,7 +40,8 @@ data class CollectionCreationState(
    val selectedTabs: Set<Tab> = emptySet(),
    val saveCollectionStep: SaveCollectionStep = SaveCollectionStep.SelectTabs,
    val tabCollections: List<TabCollection> = 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)
}
+1 −1
Original line number Diff line number Diff line
@@ -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)
+71 −1
Original line number Diff line number Diff line
@@ -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<TabCollection> = 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<TabCollection> = 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<TabCollection> = 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<TabCollection> = 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<Session>().apply {