diff --git a/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationFragment.kt b/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationFragment.kt index 4e15537f7..0a8853caf 100644 --- a/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationFragment.kt @@ -9,7 +9,6 @@ import android.os.Bundle import android.view.LayoutInflater import android.view.View import android.view.ViewGroup -import androidx.annotation.VisibleForTesting import androidx.fragment.app.DialogFragment import androidx.lifecycle.lifecycleScope import androidx.navigation.fragment.navArgs @@ -22,10 +21,7 @@ import mozilla.components.lib.publicsuffixlist.PublicSuffixList import mozilla.components.lib.state.ext.consumeFrom import org.mozilla.fenix.R import org.mozilla.fenix.components.StoreProvider -import org.mozilla.fenix.ext.getMediaStateForSession import org.mozilla.fenix.ext.requireComponents -import org.mozilla.fenix.ext.toShortUrl -import org.mozilla.fenix.home.Tab @ExperimentalCoroutinesApi class CollectionCreationFragment : DialogFragment() { @@ -47,27 +43,16 @@ class CollectionCreationFragment : DialogFragment() { val view = inflater.inflate(R.layout.fragment_create_collection, container, false) val args: CollectionCreationFragmentArgs by navArgs() - val store = requireComponents.core.store - val publicSuffixList = requireComponents.publicSuffixList - val tabs = store.state.getTabs(args.tabIds, publicSuffixList) - val selectedTabs = if (args.selectedTabIds != null) { - store.state.getTabs(args.selectedTabIds, publicSuffixList).toSet() - } else { - if (tabs.size == 1) setOf(tabs.first()) else emptySet() - } - - val tabCollections = requireComponents.core.tabCollectionStorage.cachedTabCollections - val selectedTabCollection = args.selectedTabCollectionId - .let { id -> tabCollections.firstOrNull { it.id == id } } - collectionCreationStore = StoreProvider.get(this) { CollectionCreationStore( - CollectionCreationState( - tabs = tabs, - selectedTabs = selectedTabs, + createInitialCollectionCreationState( + browserState = requireComponents.core.store.state, + tabCollectionStorage = requireComponents.core.tabCollectionStorage, + publicSuffixList = requireComponents.publicSuffixList, saveCollectionStep = args.saveCollectionStep, - tabCollections = tabCollections, - selectedTabCollection = selectedTabCollection + tabIds = args.tabIds, + selectedTabIds = args.selectedTabIds, + selectedTabCollectionId = args.selectedTabCollectionId ) ) } @@ -110,31 +95,3 @@ class CollectionCreationFragment : DialogFragment() { return dialog } } - -@VisibleForTesting -internal fun BrowserState.getTabs( - tabIds: Array?, - publicSuffixList: PublicSuffixList -): List { - return tabIds - ?.mapNotNull { id -> findTab(id) } - ?.map { it.toTab(this, publicSuffixList) } - .orEmpty() -} - -private fun TabSessionState.toTab( - state: BrowserState, - publicSuffixList: PublicSuffixList, - selected: Boolean? = null -): Tab { - val url = readerState.activeUrl ?: content.url - return Tab( - sessionId = this.id, - url = url, - hostname = url.toShortUrl(publicSuffixList), - title = content.title, - selected = selected, - icon = content.icon, - mediaState = state.getMediaStateForSession(this.id) - ) -} 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 f15ccf963..750ab0090 100644 --- a/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationStore.kt +++ b/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationStore.kt @@ -4,11 +4,19 @@ package org.mozilla.fenix.collections +import androidx.annotation.VisibleForTesting +import mozilla.components.browser.state.selector.findTab +import mozilla.components.browser.state.state.BrowserState +import mozilla.components.browser.state.state.TabSessionState import mozilla.components.feature.tab.collections.TabCollection +import mozilla.components.lib.publicsuffixlist.PublicSuffixList import mozilla.components.lib.state.Action import mozilla.components.lib.state.State import mozilla.components.lib.state.Store import org.mozilla.fenix.collections.CollectionCreationAction.StepChanged +import org.mozilla.fenix.components.TabCollectionStorage +import org.mozilla.fenix.ext.getMediaStateForSession +import org.mozilla.fenix.ext.toShortUrl import org.mozilla.fenix.home.Tab class CollectionCreationStore( @@ -43,6 +51,61 @@ data class CollectionCreationState( val defaultCollectionNumber: Int = 1 ) : State +fun createInitialCollectionCreationState( + browserState: BrowserState, + tabCollectionStorage: TabCollectionStorage, + publicSuffixList: PublicSuffixList, + saveCollectionStep: SaveCollectionStep, + tabIds: Array?, + selectedTabIds: Array?, + selectedTabCollectionId: Long +) : CollectionCreationState { + val tabs = browserState.getTabs(tabIds, publicSuffixList) + val selectedTabs = if (selectedTabIds != null) { + browserState.getTabs(selectedTabIds, publicSuffixList).toSet() + } else { + if (tabs.size == 1) setOf(tabs.first()) else emptySet() + } + + val tabCollections = tabCollectionStorage.cachedTabCollections + val selectedTabCollection = tabCollections.firstOrNull { it.id == selectedTabCollectionId } + + return CollectionCreationState( + tabs = tabs, + selectedTabs = selectedTabs, + saveCollectionStep = saveCollectionStep, + tabCollections = tabCollections, + selectedTabCollection = selectedTabCollection + ) +} + +@VisibleForTesting +internal fun BrowserState.getTabs( + tabIds: Array?, + publicSuffixList: PublicSuffixList +): List { + return tabIds + ?.mapNotNull { id -> findTab(id) } + ?.map { it.toTab(this, publicSuffixList) } + .orEmpty() +} + +private fun TabSessionState.toTab( + state: BrowserState, + publicSuffixList: PublicSuffixList +): Tab { + val url = readerState.activeUrl ?: content.url + return Tab( + sessionId = this.id, + url = url, + hostname = url.toShortUrl(publicSuffixList), + title = content.title, + selected = null, + icon = content.icon, + mediaState = state.getMediaStateForSession(this.id) + ) +} + sealed class CollectionCreationAction : Action { object AddAllTabs : CollectionCreationAction() object RemoveAllTabs : CollectionCreationAction() diff --git a/app/src/test/java/org/mozilla/fenix/collections/CollectionCreationFragmentTest.kt b/app/src/test/java/org/mozilla/fenix/collections/CollectionCreationFragmentTest.kt index 913590f0d..bd04dc4ef 100644 --- a/app/src/test/java/org/mozilla/fenix/collections/CollectionCreationFragmentTest.kt +++ b/app/src/test/java/org/mozilla/fenix/collections/CollectionCreationFragmentTest.kt @@ -10,13 +10,10 @@ import io.mockk.impl.annotations.MockK import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.ExperimentalCoroutinesApi import mozilla.components.browser.state.state.BrowserState -import mozilla.components.browser.state.state.ReaderState import mozilla.components.browser.state.state.createTab -import mozilla.components.feature.tab.collections.Tab import mozilla.components.lib.publicsuffixlist.PublicSuffixList import mozilla.components.support.test.robolectric.createAddedTestFragment import mozilla.components.support.test.robolectric.testContext -import org.junit.Assert.assertEquals import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Assert.assertTrue @@ -31,9 +28,6 @@ private const val SESSION_ID_MOZILLA = "0" private const val URL_BCC = "www.bcc.co.uk" private const val SESSION_ID_BCC = "1" -private const val SESSION_ID_BAD_1 = "not a real session id" -private const val SESSION_ID_BAD_2 = "definitely not a real session id" - @ExperimentalCoroutinesApi @RunWith(FenixRobolectricTestRunner::class) class CollectionCreationFragmentTest { @@ -71,63 +65,4 @@ class CollectionCreationFragmentTest { fragment.dismiss() assertNull(fragment.dialog) } - - @Test - fun `GIVEN tabs are present in state WHEN getTabs is called THEN tabs will be returned`() { - val tabs = state.getTabs(arrayOf(SESSION_ID_MOZILLA, SESSION_ID_BCC), publicSuffixList) - - val hosts = tabs.map { it.hostname } - - assertEquals(URL_MOZILLA, hosts[0]) - assertEquals(URL_BCC, hosts[1]) - } - - @Test - fun `GIVEN some tabs are present in state WHEN getTabs is called THEN only valid tabs will be returned`() { - val tabs = state.getTabs(arrayOf(SESSION_ID_MOZILLA, SESSION_ID_BAD_1), publicSuffixList) - - val hosts = tabs.map { it.hostname } - - assertEquals(URL_MOZILLA, hosts[0]) - assertEquals(1, hosts.size) - } - - @Test - fun `GIVEN tabs are not present in state WHEN getTabs is called THEN an empty list will be returned`() { - val tabs = state.getTabs(arrayOf(SESSION_ID_BAD_1, SESSION_ID_BAD_2), publicSuffixList) - - assertEquals(emptyList(), tabs) - } - - @Test - fun `WHEN getTabs is called will null tabIds THEN an empty list will be returned`() { - val tabs = state.getTabs(null, publicSuffixList) - - assertEquals(emptyList(), tabs) - } - - @Test - fun `toTab uses active reader URL`() { - val tabWithoutReaderState = createTab(url = "https://example.com", id = "1") - - val tabWithInactiveReaderState = createTab(url = "https://blog.mozilla.org", id = "2", - readerState = ReaderState(active = false, activeUrl = null) - ) - - val tabWithActiveReaderState = createTab(url = "moz-extension://123", id = "3", - readerState = ReaderState(active = true, activeUrl = "https://blog.mozilla.org/123") - ) - - val state = BrowserState( - tabs = listOf(tabWithoutReaderState, tabWithInactiveReaderState, tabWithActiveReaderState) - ) - val tabs = state.getTabs( - arrayOf(tabWithoutReaderState.id, tabWithInactiveReaderState.id, tabWithActiveReaderState.id), - publicSuffixList - ) - - assertEquals(tabWithoutReaderState.content.url, tabs[0].url) - assertEquals(tabWithInactiveReaderState.content.url, tabs[1].url) - assertEquals("https://blog.mozilla.org/123", tabs[2].url) - } } diff --git a/app/src/test/java/org/mozilla/fenix/collections/CollectionCreationStoreTest.kt b/app/src/test/java/org/mozilla/fenix/collections/CollectionCreationStoreTest.kt index e6d42072d..3415bc4ca 100644 --- a/app/src/test/java/org/mozilla/fenix/collections/CollectionCreationStoreTest.kt +++ b/app/src/test/java/org/mozilla/fenix/collections/CollectionCreationStoreTest.kt @@ -4,17 +4,54 @@ package org.mozilla.fenix.collections +import io.mockk.MockKAnnotations +import io.mockk.every +import io.mockk.impl.annotations.MockK import io.mockk.mockk +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.ExperimentalCoroutinesApi +import mozilla.components.browser.state.state.BrowserState +import mozilla.components.browser.state.state.ReaderState +import mozilla.components.browser.state.state.createTab +import mozilla.components.lib.publicsuffixlist.PublicSuffixList import mozilla.components.support.test.ext.joinBlocking import org.junit.Assert.assertEquals +import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import org.mozilla.fenix.components.TabCollectionStorage import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.home.Tab +private const val URL_MOZILLA = "www.mozilla.org" +private const val SESSION_ID_MOZILLA = "0" +private const val URL_BCC = "www.bcc.co.uk" +private const val SESSION_ID_BCC = "1" + +private const val SESSION_ID_BAD_1 = "not a real session id" +private const val SESSION_ID_BAD_2 = "definitely not a real session id" + +@ExperimentalCoroutinesApi @RunWith(FenixRobolectricTestRunner::class) class CollectionCreationStoreTest { + @MockK private lateinit var tabCollectionStorage: TabCollectionStorage + @MockK(relaxed = true) private lateinit var publicSuffixList: PublicSuffixList + + private val sessionMozilla = createTab(URL_MOZILLA, id = SESSION_ID_MOZILLA) + private val sessionBcc = createTab(URL_BCC, id = SESSION_ID_BCC) + private val state = BrowserState( + tabs = listOf(sessionMozilla, sessionBcc) + ) + + @Before + fun before() { + MockKAnnotations.init(this) + every { tabCollectionStorage.cachedTabCollections } returns emptyList() + every { publicSuffixList.stripPublicSuffix(URL_MOZILLA) } returns CompletableDeferred(URL_MOZILLA) + every { publicSuffixList.stripPublicSuffix(URL_BCC) } returns CompletableDeferred(URL_BCC) + } + @Test fun `select and deselect all tabs`() { val tabs = listOf(mockk(), mockk()) @@ -73,4 +110,121 @@ class CollectionCreationStoreTest { assertEquals(SaveCollectionStep.RenameCollection, store.state.saveCollectionStep) assertEquals(3, store.state.defaultCollectionNumber) } + + @Test + fun `GIVEN no selected tab ids WHEN create initial state THEN only tab will be selected`() { + val result = createInitialCollectionCreationState( + browserState = state, + tabCollectionStorage = tabCollectionStorage, + publicSuffixList = publicSuffixList, + saveCollectionStep = SaveCollectionStep.NameCollection, + tabIds = arrayOf(SESSION_ID_MOZILLA), + selectedTabIds = null, + selectedTabCollectionId = 0 + ) + + assertEquals(SaveCollectionStep.NameCollection, result.saveCollectionStep) + assertEquals(1, result.tabs.size) + assertEquals(SESSION_ID_MOZILLA, result.tabs[0].sessionId) + assertEquals(1, result.selectedTabs.size) + assertEquals(SESSION_ID_MOZILLA, result.selectedTabs.first().sessionId) + } + + @Test + fun `GIVEN no selected tab ids WHEN create initial state with many tabs THEN nothing will be selected`() { + val result = createInitialCollectionCreationState( + browserState = state, + tabCollectionStorage = tabCollectionStorage, + publicSuffixList = publicSuffixList, + saveCollectionStep = SaveCollectionStep.NameCollection, + tabIds = arrayOf(SESSION_ID_MOZILLA, SESSION_ID_BCC), + selectedTabIds = null, + selectedTabCollectionId = 0 + ) + + assertEquals(SaveCollectionStep.NameCollection, result.saveCollectionStep) + assertEquals(2, result.tabs.size) + assertEquals(SESSION_ID_MOZILLA, result.tabs[0].sessionId) + assertEquals(SESSION_ID_BCC, result.tabs[1].sessionId) + assertEquals(0, result.selectedTabs.size) + } + + @Test + fun `GIVEN selected tab ids WHEN create initial state THEN select tabs`() { + val result = createInitialCollectionCreationState( + browserState = state, + tabCollectionStorage = tabCollectionStorage, + publicSuffixList = publicSuffixList, + saveCollectionStep = SaveCollectionStep.RenameCollection, + tabIds = arrayOf(SESSION_ID_MOZILLA, SESSION_ID_BCC), + selectedTabIds = arrayOf(SESSION_ID_BCC), + selectedTabCollectionId = 0 + ) + + assertEquals(SaveCollectionStep.RenameCollection, result.saveCollectionStep) + assertEquals(2, result.tabs.size) + assertEquals(SESSION_ID_MOZILLA, result.tabs[0].sessionId) + assertEquals(SESSION_ID_BCC, result.tabs[1].sessionId) + assertEquals(1, result.selectedTabs.size) + assertEquals(SESSION_ID_BCC, result.selectedTabs.first().sessionId) + } + + @Test + fun `GIVEN tabs are present in state WHEN getTabs is called THEN tabs will be returned`() { + val tabs = state.getTabs(arrayOf(SESSION_ID_MOZILLA, SESSION_ID_BCC), publicSuffixList) + + val hosts = tabs.map { it.hostname } + + assertEquals(URL_MOZILLA, hosts[0]) + assertEquals(URL_BCC, hosts[1]) + } + + @Test + fun `GIVEN some tabs are present in state WHEN getTabs is called THEN only valid tabs will be returned`() { + val tabs = state.getTabs(arrayOf(SESSION_ID_MOZILLA, SESSION_ID_BAD_1), publicSuffixList) + + val hosts = tabs.map { it.hostname } + + assertEquals(URL_MOZILLA, hosts[0]) + assertEquals(1, hosts.size) + } + + @Test + fun `GIVEN tabs are not present in state WHEN getTabs is called THEN an empty list will be returned`() { + val tabs = state.getTabs(arrayOf(SESSION_ID_BAD_1, SESSION_ID_BAD_2), publicSuffixList) + + assertEquals(emptyList(), tabs) + } + + @Test + fun `WHEN getTabs is called will null tabIds THEN an empty list will be returned`() { + val tabs = state.getTabs(null, publicSuffixList) + + assertEquals(emptyList(), tabs) + } + + @Test + fun `toTab uses active reader URL`() { + val tabWithoutReaderState = createTab(url = "https://example.com", id = "1") + + val tabWithInactiveReaderState = createTab(url = "https://blog.mozilla.org", id = "2", + readerState = ReaderState(active = false, activeUrl = null) + ) + + val tabWithActiveReaderState = createTab(url = "moz-extension://123", id = "3", + readerState = ReaderState(active = true, activeUrl = "https://blog.mozilla.org/123") + ) + + val state = BrowserState( + tabs = listOf(tabWithoutReaderState, tabWithInactiveReaderState, tabWithActiveReaderState) + ) + val tabs = state.getTabs( + arrayOf(tabWithoutReaderState.id, tabWithInactiveReaderState.id, tabWithActiveReaderState.id), + publicSuffixList + ) + + assertEquals(tabWithoutReaderState.content.url, tabs[0].url) + assertEquals(tabWithInactiveReaderState.content.url, tabs[1].url) + assertEquals("https://blog.mozilla.org/123", tabs[2].url) + } }