From 8cd9bd5524248e51ec626aeb2b0a62b3d320fd27 Mon Sep 17 00:00:00 2001 From: Ashish Kumar Date: Sun, 21 Jul 2019 19:20:35 +0530 Subject: [PATCH] Fix category search bug (#3080) * Closes #3027 * Concat category search with search with image title list and default categories * Fixed unit test CategoriesPresenterTest.searchForCategoriesTest * bug fix [search for categories even when the edit text is empty (perform title based search)] * Category items should show the selected item on top --- .../nrw/commons/category/CategoriesModel.java | 51 ++++++++++++++++++- .../repository/UploadRemoteDataSource.java | 21 ++++++-- .../commons/repository/UploadRepository.java | 16 +++++- .../categories/CategoriesPresenter.java | 3 ++ .../categories/UploadCategoriesFragment.java | 4 +- .../commons/upload/CategoriesPresenterTest.kt | 4 +- 6 files changed, 89 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java b/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java index 9b084da49..21c39bf97 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java @@ -48,8 +48,21 @@ public class CategoriesModel{ */ public Comparator sortBySimilarity(final String filter) { Comparator stringSimilarityComparator = StringSortingUtils.sortBySimilarity(filter); - return (firstItem, secondItem) -> stringSimilarityComparator - .compare(firstItem.getName(), secondItem.getName()); + return (firstItem, secondItem) -> { + //if the category is selected, it should get precedence + if (null != firstItem && firstItem.isSelected()) { + if (null != secondItem && secondItem.isSelected()) { + return stringSimilarityComparator + .compare(firstItem.getName(), secondItem.getName()); + } + return -1; + } + if (null != secondItem && secondItem.isSelected()) { + return 1; + } + return stringSimilarityComparator + .compare(firstItem.getName(), secondItem.getName()); + }; } /** @@ -255,4 +268,38 @@ public class CategoriesModel{ this.categoriesCache.clear(); this.selectedCategories.clear(); } + + /** + * Search for categories + */ + public Observable searchCategories(String query, List imageTitleList) { + if (TextUtils.isEmpty(query)) { + return gpsCategories() + .concatWith(titleCategories(imageTitleList)) + .concatWith(recentCategories()); + } + + return mwApi + .searchCategories(query, SEARCH_CATS_LIMIT) + .map(s -> new CategoryItem(s, false)); + } + + /** + * Returns default categories + */ + public Observable getDefaultCategories(List titleList) { + Observable directCategories = directCategories(); + if (hasDirectCategories()) { + Timber.d("Image has direct Categories"); + return directCategories + .concatWith(gpsCategories()) + .concatWith(titleCategories(titleList)) + .concatWith(recentCategories()); + } else { + Timber.d("Image has no direct Categories"); + return gpsCategories() + .concatWith(titleCategories(titleList)) + .concatWith(recentCategories()); + } + } } diff --git a/app/src/main/java/fr/free/nrw/commons/repository/UploadRemoteDataSource.java b/app/src/main/java/fr/free/nrw/commons/repository/UploadRemoteDataSource.java index 938b6f30d..1ebebb7b7 100644 --- a/app/src/main/java/fr/free/nrw/commons/repository/UploadRemoteDataSource.java +++ b/app/src/main/java/fr/free/nrw/commons/repository/UploadRemoteDataSource.java @@ -11,10 +11,8 @@ import fr.free.nrw.commons.upload.UploadModel; import fr.free.nrw.commons.upload.UploadModel.UploadItem; import io.reactivex.Observable; import io.reactivex.Single; - import java.util.Comparator; import java.util.List; - import javax.inject.Inject; import javax.inject.Singleton; @@ -167,7 +165,7 @@ public class UploadRemoteDataSource { } /** - * ask the UplaodModel for the image quality of the UploadItem + * ask the UploadModel for the image quality of the UploadItem * * @param uploadItem * @param shouldValidateTitle @@ -176,4 +174,21 @@ public class UploadRemoteDataSource { public Single getImageQuality(UploadItem uploadItem, boolean shouldValidateTitle) { return uploadModel.getImageQuality(uploadItem, shouldValidateTitle); } + + /** + * Ask the CategoriesModel to search categories + * @param query + * @param imageTitleList + * @return + */ + public Observable searchCategories(String query, List imageTitleList) { + return categoriesModel.searchCategories(query, imageTitleList); + } + + /** + * Ask the CategoriesModel for default categories + */ + public Observable getDefaultCategories(List imageTitleList) { + return categoriesModel.getDefaultCategories(imageTitleList); + } } diff --git a/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java b/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java index dbd0f6134..9a157edce 100644 --- a/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java +++ b/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java @@ -8,10 +8,8 @@ import fr.free.nrw.commons.upload.SimilarImageInterface; import fr.free.nrw.commons.upload.UploadModel.UploadItem; import io.reactivex.Observable; import io.reactivex.Single; - import java.util.Comparator; import java.util.List; - import javax.inject.Inject; import javax.inject.Singleton; @@ -262,4 +260,18 @@ public class UploadRepository { public void setSelectedLicense(String licenseName) { localDataSource.setSelectedLicense(licenseName); } + + /** + * Ask the RemoteDataSource to search for categories + */ + public Observable searchCategories(String query, List imageTitleList) { + return remoteDataSource.searchCategories(query, imageTitleList); + } + + /** + * Ask the RemoteDataSource to get default categories + */ + public Observable getDefaultCategories(List imageTitleList) { + return remoteDataSource.getDefaultCategories(imageTitleList); + } } diff --git a/app/src/main/java/fr/free/nrw/commons/upload/categories/CategoriesPresenter.java b/app/src/main/java/fr/free/nrw/commons/upload/categories/CategoriesPresenter.java index a0a776246..f690d1de6 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/categories/CategoriesPresenter.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/categories/CategoriesPresenter.java @@ -80,6 +80,9 @@ public class CategoriesPresenter implements CategoriesContract.UserActionListene .observeOn(ioScheduler) .concatWith( repository.searchAll(query, imageTitleList) + .mergeWith(repository.searchCategories(query, imageTitleList)) + .concatWith(TextUtils.isEmpty(query) ? repository + .getDefaultCategories(imageTitleList) : Observable.empty()) ) .filter(categoryItem -> !repository.containsYear(categoryItem.getName())) .distinct(); diff --git a/app/src/main/java/fr/free/nrw/commons/upload/categories/UploadCategoriesFragment.java b/app/src/main/java/fr/free/nrw/commons/upload/categories/UploadCategoriesFragment.java index 48485c17a..77fb6ecb3 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/categories/UploadCategoriesFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/categories/UploadCategoriesFragment.java @@ -89,7 +89,7 @@ public class UploadCategoriesFragment extends UploadBaseFragment implements Cate @Override public void onResume() { super.onResume(); - if (presenter != null && isVisible && (categories == null || categories.isEmpty())) { + if (presenter != null && isVisible) { presenter.searchForCategories(null); } } @@ -193,7 +193,7 @@ public class UploadCategoriesFragment extends UploadBaseFragment implements Cate super.setUserVisibleHint(isVisibleToUser); isVisible = isVisibleToUser; - if (presenter != null && isResumed() && (categories == null || categories.isEmpty())) { + if (presenter != null && isResumed()) { presenter.searchForCategories(null); } } diff --git a/app/src/test/kotlin/fr/free/nrw/commons/upload/CategoriesPresenterTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/upload/CategoriesPresenterTest.kt index eae8defc5..c54e9c2d9 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/upload/CategoriesPresenterTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/upload/CategoriesPresenterTest.kt @@ -57,7 +57,9 @@ class CategoriesPresenterTest { fun searchForCategoriesTest() { Mockito.`when`(repository?.sortBySimilarity(ArgumentMatchers.anyString())).thenReturn(Comparator { _, _ -> 1 }) Mockito.`when`(repository?.selectedCategories).thenReturn(categoryItems) - Mockito.`when`(repository?.searchAll(ArgumentMatchers.anyString(), ArgumentMatchers.anyList())).thenReturn(Observable.empty()) + Mockito.`when`(repository?.searchAll(ArgumentMatchers.anyString(), ArgumentMatchers.anyList())).thenReturn(testObservable) + Mockito.`when`(repository?.searchCategories(ArgumentMatchers.anyString(), ArgumentMatchers.anyList())).thenReturn(testObservable) + Mockito.`when`(repository?.getDefaultCategories(ArgumentMatchers.anyList())).thenReturn(testObservable) categoriesPresenter?.searchForCategories("test") verify(view)?.showProgress(true) verify(view)?.showError(null)