#3490 Depiction Search in upload shows No Results before it gets results (#3491)

* #3482 Use Room in Structured Data branch - remove unused code * #3482 Use Room in Structured Data branch - fix unit test compilation * #3490 Depiction Search in upload shows No Results before it gets results - stop showing error on subscription * #3490 Depiction Search in upload shows No Results before it gets results - update test cases * make labels nullable too
2025-10-27 12:53:55 +01:00 · 2020-03-19 14:51:39 +00:00 · 2020-03-19 14:51:39 +00:00 · fb751c6fd0
commit fb751c6fd0
parent 66e195d88b
5 changed files with 81 additions and 106 deletions
--- a/app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsPresenter.java
+++ b/app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsPresenter.java
@ -5,23 +5,20 @@ import static fr.free.nrw.commons.di.CommonsApplicationModule.MAIN_THREAD;

 import fr.free.nrw.commons.explore.depictions.DepictsClient;
 import fr.free.nrw.commons.repository.UploadRepository;
-import fr.free.nrw.commons.upload.UploadModel;
 import fr.free.nrw.commons.upload.structure.depictions.DepictedItem;
 import io.reactivex.Observable;
 import io.reactivex.Scheduler;
 import io.reactivex.android.schedulers.AndroidSchedulers;
 import io.reactivex.disposables.CompositeDisposable;
 import io.reactivex.disposables.Disposable;
+import io.reactivex.schedulers.Schedulers;
 import java.lang.reflect.Proxy;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
-
 import javax.inject.Inject;
 import javax.inject.Named;
 import javax.inject.Singleton;
-
-import io.reactivex.schedulers.Schedulers;
 import timber.log.Timber;

 /**
@ -88,7 +85,6 @@ public class DepictsPresenter implements DepictsContract.UserActionListener {
                .subscribeOn(ioScheduler)
                .observeOn(mainThreadScheduler)
                .doOnSubscribe(disposable -> {
-                    view.showError(true);
                    view.showProgress(true);
                    view.setDepictsList(null);
                })
@ -102,7 +98,11 @@ public class DepictsPresenter implements DepictsContract.UserActionListener {
                .observeOn(mainThreadScheduler)
                .subscribe(
                        depictedItemList::add,
-                        Timber::e,
+                        t -> {
+                            view.showProgress(false);
+                            view.showError(true);
+                            Timber.e(t);
+                        },
                        () -> {
                            view.showProgress(false);

--- a/app/src/main/java/fr/free/nrw/commons/upload/structure/depictions/DepictedItem.kt
+++ b/app/src/main/java/fr/free/nrw/commons/upload/structure/depictions/DepictedItem.kt
@ -6,8 +6,8 @@ import fr.free.nrw.commons.wikidata.model.DepictSearchItem
 * Model class for Depicted Item in Upload and Explore
 */
 data class DepictedItem constructor(
-  val depictsLabel: String,
-  val description: String,
+  val depictsLabel: String?,
+  val description: String?,
  var imageUrl: String,
  var isSelected: Boolean,
  val entityId: String
@ -28,4 +28,8 @@ data class DepictedItem constructor(
    else -> false
  }

+  override fun hashCode(): Int {
+    return depictsLabel?.hashCode() ?: 0
+  }
+
 }
--- a/app/src/main/java/fr/free/nrw/commons/wikidata/model/DepictSearchItem.java
+++ b/app/src/main/java/fr/free/nrw/commons/wikidata/model/DepictSearchItem.java
@ -1,42 +0,0 @@
-package fr.free.nrw.commons.wikidata.model;
-
-
-/**
- * Model class for Depiction item returned from API after calling searchForDepicts
- */
-
-public class DepictSearchItem {
-    private final String id;
-    private final String pageid;
-    private final String url;
-    private final String label;
-    private final String description;
-
-    public DepictSearchItem(String id, String pageid, String url, String label, String description) {
-        this.id = id;
-        this.pageid = pageid;
-        this.url = url;
-        this.label = label;
-        this.description = description;
-    }
-
-    public String getId() {
-        return id;
-    }
-
-    public String getPageid() {
-        return pageid;
-    }
-
-    public String getUrl() {
-        return url;
-    }
-
-    public String getLabel() {
-        return label;
-    }
-
-    public String getDescription() {
-        return description;
-    }
-}
--- a/app/src/main/java/fr/free/nrw/commons/wikidata/model/DepictSearchItem.kt
+++ b/app/src/main/java/fr/free/nrw/commons/wikidata/model/DepictSearchItem.kt
@ -0,0 +1,12 @@
+package fr.free.nrw.commons.wikidata.model
+
+/**
+ * Model class for Depiction item returned from API after calling searchForDepicts
+ */
+class DepictSearchItem(
+    val id: String,
+    val pageid: String,
+    val url: String,
+    val label: String?,
+    val description: String?
+)
--- a/app/src/test/kotlin/fr/free/nrw/commons/upload/DepictsPresenterTest.kt
+++ b/app/src/test/kotlin/fr/free/nrw/commons/upload/DepictsPresenterTest.kt
@ -1,9 +1,7 @@
 package fr.free.nrw.commons.upload

-//import com.nhaarman.mockito_kotlin.verify
 import com.nhaarman.mockitokotlin2.whenever
 import fr.free.nrw.commons.category.CategoryItem
-import fr.free.nrw.commons.explore.depictions.DepictsClient
 import fr.free.nrw.commons.repository.UploadRepository
 import fr.free.nrw.commons.upload.depicts.DepictsContract
 import fr.free.nrw.commons.upload.depicts.DepictsFragment
@ -20,26 +18,22 @@ import org.mockito.MockitoAnnotations

 class DepictsPresenterTest {
    @Mock
-    internal var repository: UploadRepository? = null
+    internal lateinit var repository: UploadRepository
+
    @Mock
-    internal var view: DepictsContract.View? = null
+    internal lateinit var view: DepictsContract.View

-    var depictsPresenter: DepictsPresenter? = null
+    private lateinit var depictsPresenter: DepictsPresenter

-    var depictsFragment: DepictsFragment? = null
+    private lateinit var depictsFragment: DepictsFragment

-    var testScheduler: TestScheduler? = null
+    private lateinit var testScheduler: TestScheduler

-    var depictsClient : DepictsClient? = null
-
-    val depictedItems: ArrayList<DepictedItem> = ArrayList()
+    private val depictedItems: ArrayList<DepictedItem> = ArrayList()

    @Mock
    lateinit var depictedItem: DepictedItem

-    var testObservable: Observable<DepictedItem>? = null
-
-    private val imageTitleList = ArrayList<UploadMediaDetail>()

    /**
     * initial setup
@ -51,70 +45,77 @@ class DepictsPresenterTest {
        testScheduler = TestScheduler()
        depictedItem = DepictedItem("label", "desc", "", false, "entityId")
        depictedItems.add(depictedItem)
-        testObservable = Observable.just(depictedItem)
-        depictsPresenter = DepictsPresenter(repository, testScheduler, testScheduler, depictsClient)
+        depictsPresenter = DepictsPresenter(repository, testScheduler, testScheduler, null)
        depictsFragment = DepictsFragment()
-        depictsPresenter?.onAttachView(view)
+        depictsPresenter.onAttachView(view)
    }

    @Test
    fun searchEnglishDepictionsTest() {
-        whenever(repository?.sortBySimilarity(ArgumentMatchers.anyString())).thenReturn(Comparator<CategoryItem> { _, _ -> 1 })
-        whenever(repository?.selectedDepictions).thenReturn(depictedItems)
-        whenever(repository?.searchAllEntities(ArgumentMatchers.anyString())).thenReturn(Observable.empty())
-        depictsPresenter?.searchForDepictions("test")
-        verify(view)?.showProgress(true)
-        verify(view)?.showError(true)
-        verify(view)?.setDepictsList(null)
-        testScheduler?.triggerActions()
-        verify(view)?.showProgress(false)
+        whenever(repository.sortBySimilarity(ArgumentMatchers.anyString())).thenReturn(Comparator<CategoryItem> { _, _ -> 1 })
+        whenever(repository.selectedDepictions).thenReturn(depictedItems)
+        whenever(repository.searchAllEntities(ArgumentMatchers.anyString())).thenReturn(Observable.empty())
+        depictsPresenter.searchForDepictions("test")
+        verify(view).showProgress(true)
+        verify(view).setDepictsList(null)
+        testScheduler.triggerActions()
+        verify(view).showProgress(false)
    }

    @Test
    fun searchOtherLanguageDepictions() {
-        whenever(repository?.sortBySimilarity(ArgumentMatchers.anyString())).thenReturn(Comparator<CategoryItem> { _, _ -> 1 })
-        whenever(repository?.selectedDepictions).thenReturn(depictedItems)
-        whenever(repository?.searchAllEntities(ArgumentMatchers.anyString())).thenReturn(Observable.empty())
-        depictsPresenter?.searchForDepictions("वी")
-        verify(view)?.showProgress(true)
-        verify(view)?.showError(true)
-        verify(view)?.setDepictsList(null)
-        testScheduler?.triggerActions()
-        verify(view)?.showProgress(false)
+        whenever(repository.sortBySimilarity(ArgumentMatchers.anyString())).thenReturn(Comparator<CategoryItem> { _, _ -> 1 })
+        whenever(repository.selectedDepictions).thenReturn(depictedItems)
+        whenever(repository.searchAllEntities(ArgumentMatchers.anyString())).thenReturn(Observable.empty())
+        depictsPresenter.searchForDepictions("वी")
+        verify(view).showProgress(true)
+        verify(view).setDepictsList(null)
+        testScheduler.triggerActions()
+        verify(view).showProgress(false)
    }

    @Test
    fun searchForNonExistingDepictions() {
-        whenever(repository?.sortBySimilarity(ArgumentMatchers.anyString())).thenReturn(Comparator<CategoryItem> { _, _ -> 1 })
-        whenever(repository?.selectedDepictions).thenReturn(depictedItems)
-        whenever(repository?.searchAllEntities(ArgumentMatchers.anyString())).thenReturn(Observable.empty())
-        depictsPresenter?.searchForDepictions("******")
-        verify(view)?.showProgress(true)
-        verify(view)?.setDepictsList(null)
-        testScheduler?.triggerActions()
-        verify(view)?.setDepictsList(null)
-        verify(view)?.showProgress(false)
+        whenever(repository.sortBySimilarity(ArgumentMatchers.anyString())).thenReturn(Comparator<CategoryItem> { _, _ -> 1 })
+        whenever(repository.selectedDepictions).thenReturn(depictedItems)
+        whenever(repository.searchAllEntities(ArgumentMatchers.anyString())).thenReturn(Observable.empty())
+        depictsPresenter.searchForDepictions("******")
+        verify(view).showProgress(true)
+        verify(view).setDepictsList(null)
+        testScheduler.triggerActions()
+        verify(view).setDepictsList(null)
+        verify(view).showProgress(false)
    }

    @Test
    fun setSingleDepiction() {
-        whenever(repository?.sortBySimilarity(ArgumentMatchers.anyString())).thenReturn(Comparator<CategoryItem> { _, _ -> 1 })
-        whenever(repository?.selectedDepictions).thenReturn(depictedItems)
-        whenever(repository?.searchAllEntities(ArgumentMatchers.anyString())).thenReturn(Observable.empty())
-        depictsPresenter?.onDepictItemClicked(depictedItem)
-        depictsPresenter?.verifyDepictions()
-        verify(view)?.goToNextScreen()
+        whenever(repository.sortBySimilarity(ArgumentMatchers.anyString())).thenReturn(Comparator<CategoryItem> { _, _ -> 1 })
+        whenever(repository.selectedDepictions).thenReturn(depictedItems)
+        whenever(repository.searchAllEntities(ArgumentMatchers.anyString())).thenReturn(Observable.empty())
+        depictsPresenter.onDepictItemClicked(depictedItem)
+        depictsPresenter.verifyDepictions()
+        verify(view).goToNextScreen()
    }

    @Test
    fun setMultipleDepictions() {
-        whenever(repository?.sortBySimilarity(ArgumentMatchers.anyString())).thenReturn(Comparator<CategoryItem> { _, _ -> 1 })
-        whenever(repository?.selectedDepictions).thenReturn(depictedItems)
-        whenever(repository?.searchAllEntities(ArgumentMatchers.anyString())).thenReturn(Observable.empty())
-        depictsPresenter?.onDepictItemClicked(depictedItem)
+        whenever(repository.sortBySimilarity(ArgumentMatchers.anyString())).thenReturn(Comparator<CategoryItem> { _, _ -> 1 })
+        whenever(repository.selectedDepictions).thenReturn(depictedItems)
+        whenever(repository.searchAllEntities(ArgumentMatchers.anyString())).thenReturn(Observable.empty())
+        depictsPresenter.onDepictItemClicked(depictedItem)
        val depictedItem2 = DepictedItem("label2", "desc2", "", false, "entityid2")
-        depictsPresenter?.onDepictItemClicked(depictedItem2)
-        depictsPresenter?.verifyDepictions()
-        verify(view)?.goToNextScreen()
+        depictsPresenter.onDepictItemClicked(depictedItem2)
+        depictsPresenter.verifyDepictions()
+        verify(view).goToNextScreen()
+    }
+
+    @Test
+    fun `on Search Exception Show Error And Stop Progress`() {
+        whenever(repository.searchAllEntities(ArgumentMatchers.anyString()))
+            .thenReturn(Observable.error(Exception()))
+        depictsPresenter.searchForDepictions("******")
+        testScheduler.triggerActions()
+        verify(view).showError(true)
+        verify(view).showProgress(false)
    }
 }