Bugfix/categories search (#3913)

* Fixes #3734
* Donot ignore case while searching categories

* Fixed test-cases to ensure search terms are passed as it is to the CategoryClient

* Used a First_Char_Caps title list term just to ensure test case tests as intended

* Fixed searchAll with empty term test case
This commit is contained in:
Ashish 2020-08-31 21:30:36 +05:30 committed by GitHub
parent 7f90361ffd
commit 4109d23023
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 34 additions and 10 deletions

View file

@ -91,7 +91,7 @@ class CategoriesModel @Inject constructor(
Function4(::combine) Function4(::combine)
) )
else else
categoryClient.searchCategoriesForPrefix(term.toLowerCase(), SEARCH_CATS_LIMIT) categoryClient.searchCategoriesForPrefix(term, SEARCH_CATS_LIMIT)
.map { it.sortedWith(StringSortingUtils.sortBySimilarity(term)) } .map { it.sortedWith(StringSortingUtils.sortBySimilarity(term)) }
.toObservable() .toObservable()
} }
@ -122,12 +122,11 @@ class CategoriesModel @Inject constructor(
/** /**
* Return category for single title * Return category for single title
* title is converted to lower case to make search case-insensitive
* @param title * @param title
* @return * @return
*/ */
private fun getTitleCategories(title: String): Observable<List<String>> { private fun getTitleCategories(title: String): Observable<List<String>> {
return categoryClient.searchCategories(title.toLowerCase(), SEARCH_CATS_LIMIT).toObservable() return categoryClient.searchCategories(title, SEARCH_CATS_LIMIT).toObservable()
} }
@ -160,6 +159,6 @@ class CategoriesModel @Inject constructor(
} }
companion object { companion object {
private const val SEARCH_CATS_LIMIT = 25 const val SEARCH_CATS_LIMIT = 25
} }
} }

View file

@ -2,6 +2,7 @@ package fr.free.nrw.commons.category
import categoryItem import categoryItem
import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.mock
import com.nhaarman.mockitokotlin2.verify
import com.nhaarman.mockitokotlin2.whenever import com.nhaarman.mockitokotlin2.whenever
import depictedItem import depictedItem
import fr.free.nrw.commons.upload.GpsCategoryModel import fr.free.nrw.commons.upload.GpsCategoryModel
@ -9,6 +10,7 @@ import io.reactivex.Single
import io.reactivex.subjects.BehaviorSubject import io.reactivex.subjects.BehaviorSubject
import org.junit.Before import org.junit.Before
import org.junit.Test import org.junit.Test
import org.mockito.ArgumentMatchers
import org.mockito.Mock import org.mockito.Mock
import org.mockito.MockitoAnnotations import org.mockito.MockitoAnnotations
@ -27,26 +29,39 @@ class CategoriesModelTest {
MockitoAnnotations.initMocks(this) MockitoAnnotations.initMocks(this)
} }
// Test Case for verifying that Categories search (MW api calls) are case-insensitive // Test Case for verifying that Categories search (MW api calls)
@Test @Test
fun searchAllFoundCaseTest() { fun searchAllFoundCaseTest() {
val categoriesModel = CategoriesModel(categoryClient, mock(), mock()) val categoriesModel = CategoriesModel(categoryClient, mock(), mock())
val expectedList = listOf("Test") val expectedList = listOf("Test")
whenever(categoryClient.searchCategoriesForPrefix("tes", 25)) whenever(
categoryClient.searchCategoriesForPrefix(
ArgumentMatchers.anyString(),
ArgumentMatchers.anyInt(),
ArgumentMatchers.anyInt()
)
)
.thenReturn(Single.just(expectedList)) .thenReturn(Single.just(expectedList))
// Checking if both return "Test" // Checking if both return "Test"
val expectedItems = expectedList.map { CategoryItem(it, false) } val expectedItems = expectedList.map { CategoryItem(it, false) }
categoriesModel.searchAll("tes", emptyList(), emptyList()) var categoryTerm = "Test"
categoriesModel.searchAll(categoryTerm, emptyList(), emptyList())
.test() .test()
.assertValues(expectedItems) .assertValues(expectedItems)
categoriesModel.searchAll("Tes", emptyList(), emptyList()) verify(categoryClient).searchCategoriesForPrefix(
categoryTerm,
CategoriesModel.SEARCH_CATS_LIMIT
)
categoriesModel.searchAll(categoryTerm, emptyList(), emptyList())
.test() .test()
.assertValues(expectedItems) .assertValues(expectedItems)
} }
@Test @Test
fun `searchAll with empty search terms creates results from gps, title search & recents`() { fun `searchAll with empty search terms creates results from gps, title search & recents`() {
val gpsCategoryModel: GpsCategoryModel = mock() val gpsCategoryModel: GpsCategoryModel = mock()
@ -54,11 +69,18 @@ class CategoriesModelTest {
whenever(gpsCategoryModel.categoriesFromLocation) whenever(gpsCategoryModel.categoriesFromLocation)
.thenReturn(BehaviorSubject.createDefault(listOf("gpsCategory"))) .thenReturn(BehaviorSubject.createDefault(listOf("gpsCategory")))
whenever(categoryClient.searchCategories("tes", 25)) whenever(
categoryClient.searchCategories(
ArgumentMatchers.anyString(),
ArgumentMatchers.anyInt(),
ArgumentMatchers.anyInt()
)
)
.thenReturn(Single.just(listOf("titleSearch"))) .thenReturn(Single.just(listOf("titleSearch")))
whenever(categoryDao.recentCategories(25)).thenReturn(listOf("recentCategories")) whenever(categoryDao.recentCategories(25)).thenReturn(listOf("recentCategories"))
val imageTitleList = listOf("Test")
CategoriesModel(categoryClient, categoryDao, gpsCategoryModel) CategoriesModel(categoryClient, categoryDao, gpsCategoryModel)
.searchAll("", listOf("tes"), listOf(depictedItem)) .searchAll("", imageTitleList, listOf(depictedItem))
.test() .test()
.assertValue( .assertValue(
listOf( listOf(
@ -68,5 +90,8 @@ class CategoriesModelTest {
categoryItem("recentCategories") categoryItem("recentCategories")
) )
) )
imageTitleList.forEach {
verify(categoryClient).searchCategories(it, CategoriesModel.SEARCH_CATS_LIMIT)
}
} }
} }