Update the weighting algorithm to allow multiple types in the list. A…

…dd tests

app/src/main/java/io/plaidapp/ui/HomeActivity.java

@@ -132,6 +132,8 @@ protected void onCreate(Bundle savedInstanceState) {
 
         inject(this);
 
+        viewModel.setColumns(columns);
+
         boolean pocketInstalled = PocketUtils.isPocketInstalled(this);
 
         adapter = new FeedAdapter(this, columns, pocketInstalled);

app/src/main/java/io/plaidapp/ui/HomeViewModel.kt

@@ -99,6 +99,9 @@ class HomeViewModel(
         }
     }
 
+    // TODO - find a better solution
+    var columns = 2
+
     init {
         sourcesRepository.registerFilterChangedCallback(filtersChangedCallbacks)
         dataManager.setOnDataLoadedCallback(onDataLoadedCallback)
@@ -123,7 +126,7 @@ class HomeViewModel(
     }
 
     fun addSources(query: String, isDribbble: Boolean, isDesignerNews: Boolean) {
-        if(query.isBlank()){
+        if (query.isBlank()) {
             return
         }
         val sources = mutableListOf<Source>()
@@ -198,7 +201,7 @@ class HomeViewModel(
     }
 
     private fun updateFeedData(oldItems: List<PlaidItem>, newItems: List<PlaidItem>) {
-        val items = getPlaidItemsForDisplay(oldItems, newItems, 2)
+        val items = getPlaidItemsForDisplay(oldItems, newItems, columns)
         _feed.value = FeedUiModel(items)
     }
 
@@ -207,7 +210,7 @@ class HomeViewModel(
         items.removeAll {
             dataSourceKey == it.dataSource
         }
-        expandPopularItems(items, 2)
+        expandPopularItems(items, columns)
         _feed.value = FeedUiModel(items)
     }
 

app/src/test/java/io/plaidapp/TestData.kt

@@ -0,0 +1,92 @@
+/*
+ * Copyright 2019 Google, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.plaidapp
+
+import io.plaidapp.core.data.Source
+import io.plaidapp.core.designernews.data.stories.model.Story
+import io.plaidapp.core.designernews.data.stories.model.StoryLinks
+import io.plaidapp.core.dribbble.data.api.model.Images
+import io.plaidapp.core.dribbble.data.api.model.Shot
+import io.plaidapp.core.dribbble.data.api.model.User
+import io.plaidapp.core.producthunt.data.api.model.Post
+import io.plaidapp.core.ui.filter.SourceUiModel
+import java.util.Date
+import java.util.GregorianCalendar
+
+val designerNewsSource = Source.DesignerNewsSearchSource(
+    "query",
+    true
+)
+val designerNewsSourceUiModel = SourceUiModel(
+    designerNewsSource.key,
+    designerNewsSource.name,
+    designerNewsSource.active,
+    designerNewsSource.iconRes,
+    designerNewsSource.isSwipeDismissable,
+    {},
+    {}
+)
+val dribbbleSource = Source.DribbbleSearchSource("dribbble", true)
+
+val post = Post(
+    id = 345L,
+    title = "Plaid",
+    url = "www.plaid.amazing",
+    tagline = "amazing",
+    discussionUrl = "www.disc.plaid",
+    redirectUrl = "www.d.plaid",
+    commentsCount = 5,
+    votesCount = 100
+)
+
+val player = User(
+    id = 1L,
+    name = "Nick Butcher",
+    username = "nickbutcher",
+    avatarUrl = "www.prettyplaid.nb"
+)
+
+val shot = Shot(
+    id = 1L,
+    title = "Foo Nick",
+    description = "",
+    images = Images(),
+    user = player
+).also {
+    it.dataSource = dribbbleSource.key
+}
+
+const val userId = 5L
+const val storyId = 1345L
+val createdDate: Date = GregorianCalendar(2018, 1, 13).time
+val commentIds = listOf(11L, 12L)
+val storyLinks = StoryLinks(
+    user = userId,
+    comments = commentIds,
+    upvotes = emptyList(),
+    downvotes = emptyList()
+)
+
+val story = Story(
+    id = storyId,
+    title = "Plaid 2.0 was released",
+    createdAt = createdDate,
+    userId = userId,
+    links = storyLinks
+).also {
+    it.dataSource = designerNewsSource.key
+}

app/src/test/java/io/plaidapp/ui/HomeViewModelTest.kt

@@ -25,18 +25,24 @@ import com.nhaarman.mockitokotlin2.verify
 import com.nhaarman.mockitokotlin2.whenever
 import io.plaidapp.core.data.DataLoadingSubject
 import io.plaidapp.core.data.DataManager
+import io.plaidapp.core.data.OnDataLoadedCallback
+import io.plaidapp.core.data.PlaidItem
 import io.plaidapp.core.data.Source
 import io.plaidapp.core.data.prefs.SourcesRepository
 import io.plaidapp.core.designernews.data.login.LoginRepository
 import io.plaidapp.core.feed.FeedProgressUiModel
 import io.plaidapp.core.ui.filter.FiltersChangedCallback
-import io.plaidapp.core.ui.filter.SourceUiModel
 import io.plaidapp.core.ui.filter.SourcesHighlightUiModel
+import io.plaidapp.designerNewsSource
+import io.plaidapp.designerNewsSourceUiModel
+import io.plaidapp.dribbbleSource
+import io.plaidapp.post
+import io.plaidapp.shot
+import io.plaidapp.story
 import io.plaidapp.test.shared.LiveDataTestUtil
 import io.plaidapp.test.shared.provideFakeCoroutinesDispatcherProvider
 import kotlinx.coroutines.runBlocking
 import org.junit.Assert.assertEquals
-import org.junit.Assert.assertNull
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
@@ -53,29 +59,6 @@ class HomeViewModelTest {
     @get:Rule
     var instantTaskExecutorRule = InstantTaskExecutorRule()
 
-    private val designerNewsSource = Source.DesignerNewsSearchSource(
-        "query",
-        true
-    )
-    private val designerNewsSourceUiModel = SourceUiModel(
-        designerNewsSource.key,
-        designerNewsSource.name,
-        designerNewsSource.active,
-        designerNewsSource.iconRes,
-        designerNewsSource.isSwipeDismissable,
-        {},
-        {}
-    )
-    private val dribbbleSource = Source.DribbbleSearchSource("dribbble", true)
-    private val dribbbleSourceUiModel = SourceUiModel(
-        dribbbleSource.key,
-        dribbbleSource.name,
-        dribbbleSource.active,
-        dribbbleSource.iconRes,
-        dribbbleSource.isSwipeDismissable,
-        {},
-        {}
-    )
     private val dataManager: DataManager = mock()
     private val loginRepository: LoginRepository = mock()
     private val sourcesRepository: SourcesRepository = mock()
@@ -86,6 +69,9 @@ class HomeViewModelTest {
     @Captor
     private lateinit var dataLoadingCallback: ArgumentCaptor<DataLoadingSubject.DataLoadingCallbacks>
 
+    @Captor
+    private lateinit var dataLoadedCallback: ArgumentCaptor<OnDataLoadedCallback<List<PlaidItem>>>
+
     @Before
     fun setup() {
         MockitoAnnotations.initMocks(this)
@@ -148,7 +134,11 @@ class HomeViewModelTest {
         val homeViewModel = createViewModel()
 
         // When adding a Designer News source
-        homeViewModel.addSources(designerNewsSource.query, isDribbble = false, isDesignerNews = true)
+        homeViewModel.addSources(
+            designerNewsSource.query,
+            isDribbble = false,
+            isDesignerNews = true
+        )
 
         // Then a Designer News source is added to the repository
         val expected = listOf(designerNewsSource)
@@ -281,41 +271,42 @@ class HomeViewModelTest {
 
     @Test
     fun filtersRemoved() {
-        // Given a view model
-        val homeViewModel = createViewModel(listOf(designerNewsSource, dribbbleSource))
+        // Given a view model with feed data
+        val homeViewModel = createViewModelWithFeedData(listOf(post, shot, story))
         verify(sourcesRepository).registerFilterChangedCallback(
             capture(filtersChangedCallback)
         )
 
         // When a source was removed
         filtersChangedCallback.value.onFilterRemoved(dribbbleSource.key)
 
-        // Then feed emits with a new list, without the removed filter
+        // Then feed emits a new list, without the removed filter
         val feed = LiveDataTestUtil.getValue(homeViewModel.feed)
-        assertEquals(listOf(designerNewsSourceUiModel), feed)
+        assertEquals(listOf(post, story), feed!!.items)
     }
 
     @Test
     fun filtersChanged_activeSource() {
-        // Given a view model
-        val homeViewModel = createViewModel(listOf(designerNewsSource, dribbbleSource))
+        // Given a view model with feed data
+        val homeViewModel = createViewModelWithFeedData(listOf(post, shot, story))
         verify(sourcesRepository).registerFilterChangedCallback(
             capture(filtersChangedCallback)
         )
+        val initialFeed = LiveDataTestUtil.getValue(homeViewModel.feed)
 
         // When an active source was changed
         val activeSource = Source.DribbbleSearchSource("dribbble", true)
         filtersChangedCallback.value.onFiltersChanged(activeSource)
 
         // Then feed didn't emit a new value
-        val source = LiveDataTestUtil.getValue(homeViewModel.feed)
-        assertNull(source)
+        val feed = LiveDataTestUtil.getValue(homeViewModel.feed)
+        assertEquals(initialFeed, feed)
     }
 
     @Test
     fun filtersChanged_inactiveSource() {
-        // Given a view model
-        val homeViewModel = createViewModel(listOf(designerNewsSource, dribbbleSource))
+        // Given a view model with feed data
+        val homeViewModel = createViewModelWithFeedData(listOf(post, shot, story))
         verify(sourcesRepository).registerFilterChangedCallback(
             capture(filtersChangedCallback)
         )
@@ -324,9 +315,9 @@ class HomeViewModelTest {
         val inactiveSource = Source.DribbbleSearchSource("dribbble", false)
         filtersChangedCallback.value.onFiltersChanged(inactiveSource)
 
-        // Then feed emits with a new list, without the removed filter
+        // Then feed emits a new list, without the removed filter
         val feed = LiveDataTestUtil.getValue(homeViewModel.feed)
-        assertEquals(listOf(designerNewsSourceUiModel), feed)
+        assertEquals(listOf(post, story), feed!!.items)
     }
 
     @Test
@@ -357,6 +348,39 @@ class HomeViewModelTest {
         assertEquals(FeedProgressUiModel(false), progress)
     }
 
+    @Test
+    fun dataLoading_atInit() {
+        // When creating a view model
+        createViewModel()
+
+        // Then load data was called
+        verify(dataManager).loadAllDataSources()
+    }
+
+    @Test
+    fun feed_emitsWhenDataLoaded() {
+        // Given a view model
+        val homeViewModel = createViewModel()
+        verify(dataManager).setOnDataLoadedCallback(capture(dataLoadedCallback))
+
+        // When data loaded
+        dataLoadedCallback.value.onDataLoaded(listOf(post, shot, story))
+
+        // Then feed emits a new list
+        val feed = LiveDataTestUtil.getValue(homeViewModel.feed)
+        assertEquals(listOf(post, story, shot), feed!!.items)
+    }
+
+    private fun createViewModelWithFeedData(feedData: List<PlaidItem>): HomeViewModel {
+        val homeViewModel = createViewModel()
+        verify(dataManager).setOnDataLoadedCallback(capture(dataLoadedCallback))
+
+        // When data loaded return feedData
+        dataLoadedCallback.value.onDataLoaded(feedData)
+
+        return homeViewModel
+    }
+
     private fun createViewModel(
         list: List<Source> = emptyList()
     ): HomeViewModel = runBlocking {

core/src/main/java/io/plaidapp/core/data/PlaidItem.kt

@@ -17,7 +17,8 @@
 package io.plaidapp.core.data
 
 /**
- * Base class for all model types
+ * Base class for all model types.
+ * // TODO - make the item immutable
  */
 abstract class PlaidItem(
     @Transient open val id: Long,

core/src/main/java/io/plaidapp/core/ui/PlaidItemsListExtension.kt

@@ -75,6 +75,7 @@ fun expandPopularItems(items: List<PlaidItem>, columns: Int) {
         }
     }
 }
+
 /**
  * Calculate a 'weight' [0, 1] for each data type for sorting. Each data type/source has a
  * different metric for weighing it e.g. Dribbble uses likes etc. but some sources should keep
@@ -84,22 +85,22 @@ fun expandPopularItems(items: List<PlaidItem>, columns: Int) {
 private fun weighItems(items: MutableList<out PlaidItem>?) {
     if (items == null || items.isEmpty()) return
 
-    // TODO this weighting algo assumes that all items in the list are of the same type
-    // update this to something better in the future
-
     // some sources should just use the natural order i.e. as returned by the API as users
     // have an expectation about the order they appear in
-    if (SourcesRepository.SOURCE_PRODUCT_HUNT == items[0].dataSource) {
-        PlaidItemSorting.NaturalOrderWeigher().weigh(items)
-    } else {
-        // otherwise use our own weight calculation. We prefer this as it leads to a less
-        // regular pattern of items in the grid
-        when {
-            items[0] is Shot -> ShotWeigher().weigh(items as List<Shot>)
-            items[0] is Story -> StoryWeigher().weigh(items as List<Story>)
-            items[0] is Post -> PostWeigher().weigh(items as List<Post>)
-            else -> throw RuntimeException("unknown item type")
+    items.filter { SourcesRepository.SOURCE_PRODUCT_HUNT == it.dataSource }.apply {
+            PlaidItemSorting.NaturalOrderWeigher().weigh(this)
         }
+
+    // otherwise use our own weight calculation. We prefer this as it leads to a less
+    // regular pattern of items in the grid
+    items.filter { it is Shot }.apply {
+        ShotWeigher().weigh(this as List<Shot>)
+    }
+    items.filter { it is Story }.apply {
+        StoryWeigher().weigh(this as List<Story>)
+    }
+    items.filter { it is Post && SourcesRepository.SOURCE_PRODUCT_HUNT != it.dataSource }.apply {
+        PostWeigher().weigh(this as List<Post>)
     }
 }