Skip to content

Commit 8cd9bd5

Browse files
ashishkumar468misaochan
authored andcommitted
Fix category search bug (commons-app#3080)
* Closes commons-app#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
1 parent 32715d9 commit 8cd9bd5

File tree

6 files changed

+89
-10
lines changed

6 files changed

+89
-10
lines changed

app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java

+49-2
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,21 @@ public CategoriesModel(MediaWikiApi mwApi,
4848
*/
4949
public Comparator<CategoryItem> sortBySimilarity(final String filter) {
5050
Comparator<String> stringSimilarityComparator = StringSortingUtils.sortBySimilarity(filter);
51-
return (firstItem, secondItem) -> stringSimilarityComparator
52-
.compare(firstItem.getName(), secondItem.getName());
51+
return (firstItem, secondItem) -> {
52+
//if the category is selected, it should get precedence
53+
if (null != firstItem && firstItem.isSelected()) {
54+
if (null != secondItem && secondItem.isSelected()) {
55+
return stringSimilarityComparator
56+
.compare(firstItem.getName(), secondItem.getName());
57+
}
58+
return -1;
59+
}
60+
if (null != secondItem && secondItem.isSelected()) {
61+
return 1;
62+
}
63+
return stringSimilarityComparator
64+
.compare(firstItem.getName(), secondItem.getName());
65+
};
5366
}
5467

5568
/**
@@ -255,4 +268,38 @@ public void cleanUp() {
255268
this.categoriesCache.clear();
256269
this.selectedCategories.clear();
257270
}
271+
272+
/**
273+
* Search for categories
274+
*/
275+
public Observable<CategoryItem> searchCategories(String query, List<String> imageTitleList) {
276+
if (TextUtils.isEmpty(query)) {
277+
return gpsCategories()
278+
.concatWith(titleCategories(imageTitleList))
279+
.concatWith(recentCategories());
280+
}
281+
282+
return mwApi
283+
.searchCategories(query, SEARCH_CATS_LIMIT)
284+
.map(s -> new CategoryItem(s, false));
285+
}
286+
287+
/**
288+
* Returns default categories
289+
*/
290+
public Observable<CategoryItem> getDefaultCategories(List<String> titleList) {
291+
Observable<CategoryItem> directCategories = directCategories();
292+
if (hasDirectCategories()) {
293+
Timber.d("Image has direct Categories");
294+
return directCategories
295+
.concatWith(gpsCategories())
296+
.concatWith(titleCategories(titleList))
297+
.concatWith(recentCategories());
298+
} else {
299+
Timber.d("Image has no direct Categories");
300+
return gpsCategories()
301+
.concatWith(titleCategories(titleList))
302+
.concatWith(recentCategories());
303+
}
304+
}
258305
}

app/src/main/java/fr/free/nrw/commons/repository/UploadRemoteDataSource.java

+18-3
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,8 @@
1111
import fr.free.nrw.commons.upload.UploadModel.UploadItem;
1212
import io.reactivex.Observable;
1313
import io.reactivex.Single;
14-
1514
import java.util.Comparator;
1615
import java.util.List;
17-
1816
import javax.inject.Inject;
1917
import javax.inject.Singleton;
2018

@@ -167,7 +165,7 @@ public Observable<UploadItem> preProcessImage(UploadableFile uploadableFile, Pla
167165
}
168166

169167
/**
170-
* ask the UplaodModel for the image quality of the UploadItem
168+
* ask the UploadModel for the image quality of the UploadItem
171169
*
172170
* @param uploadItem
173171
* @param shouldValidateTitle
@@ -176,4 +174,21 @@ public Observable<UploadItem> preProcessImage(UploadableFile uploadableFile, Pla
176174
public Single<Integer> getImageQuality(UploadItem uploadItem, boolean shouldValidateTitle) {
177175
return uploadModel.getImageQuality(uploadItem, shouldValidateTitle);
178176
}
177+
178+
/**
179+
* Ask the CategoriesModel to search categories
180+
* @param query
181+
* @param imageTitleList
182+
* @return
183+
*/
184+
public Observable<CategoryItem> searchCategories(String query, List<String> imageTitleList) {
185+
return categoriesModel.searchCategories(query, imageTitleList);
186+
}
187+
188+
/**
189+
* Ask the CategoriesModel for default categories
190+
*/
191+
public Observable<CategoryItem> getDefaultCategories(List<String> imageTitleList) {
192+
return categoriesModel.getDefaultCategories(imageTitleList);
193+
}
179194
}

app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java

+14-2
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,8 @@
88
import fr.free.nrw.commons.upload.UploadModel.UploadItem;
99
import io.reactivex.Observable;
1010
import io.reactivex.Single;
11-
1211
import java.util.Comparator;
1312
import java.util.List;
14-
1513
import javax.inject.Inject;
1614
import javax.inject.Singleton;
1715

@@ -262,4 +260,18 @@ public String getValue(String key, String value) {
262260
public void setSelectedLicense(String licenseName) {
263261
localDataSource.setSelectedLicense(licenseName);
264262
}
263+
264+
/**
265+
* Ask the RemoteDataSource to search for categories
266+
*/
267+
public Observable<CategoryItem> searchCategories(String query, List<String> imageTitleList) {
268+
return remoteDataSource.searchCategories(query, imageTitleList);
269+
}
270+
271+
/**
272+
* Ask the RemoteDataSource to get default categories
273+
*/
274+
public Observable<CategoryItem> getDefaultCategories(List<String> imageTitleList) {
275+
return remoteDataSource.getDefaultCategories(imageTitleList);
276+
}
265277
}

app/src/main/java/fr/free/nrw/commons/upload/categories/CategoriesPresenter.java

+3
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@ public void searchForCategories(String query) {
8080
.observeOn(ioScheduler)
8181
.concatWith(
8282
repository.searchAll(query, imageTitleList)
83+
.mergeWith(repository.searchCategories(query, imageTitleList))
84+
.concatWith(TextUtils.isEmpty(query) ? repository
85+
.getDefaultCategories(imageTitleList) : Observable.empty())
8386
)
8487
.filter(categoryItem -> !repository.containsYear(categoryItem.getName()))
8588
.distinct();

app/src/main/java/fr/free/nrw/commons/upload/categories/UploadCategoriesFragment.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ private void init() {
8989
@Override
9090
public void onResume() {
9191
super.onResume();
92-
if (presenter != null && isVisible && (categories == null || categories.isEmpty())) {
92+
if (presenter != null && isVisible) {
9393
presenter.searchForCategories(null);
9494
}
9595
}
@@ -193,7 +193,7 @@ public void setUserVisibleHint(boolean isVisibleToUser) {
193193
super.setUserVisibleHint(isVisibleToUser);
194194
isVisible = isVisibleToUser;
195195

196-
if (presenter != null && isResumed() && (categories == null || categories.isEmpty())) {
196+
if (presenter != null && isResumed()) {
197197
presenter.searchForCategories(null);
198198
}
199199
}

app/src/test/kotlin/fr/free/nrw/commons/upload/CategoriesPresenterTest.kt

+3-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,9 @@ class CategoriesPresenterTest {
5757
fun searchForCategoriesTest() {
5858
Mockito.`when`(repository?.sortBySimilarity(ArgumentMatchers.anyString())).thenReturn(Comparator<CategoryItem> { _, _ -> 1 })
5959
Mockito.`when`(repository?.selectedCategories).thenReturn(categoryItems)
60-
Mockito.`when`(repository?.searchAll(ArgumentMatchers.anyString(), ArgumentMatchers.anyList())).thenReturn(Observable.empty())
60+
Mockito.`when`(repository?.searchAll(ArgumentMatchers.anyString(), ArgumentMatchers.anyList())).thenReturn(testObservable)
61+
Mockito.`when`(repository?.searchCategories(ArgumentMatchers.anyString(), ArgumentMatchers.anyList())).thenReturn(testObservable)
62+
Mockito.`when`(repository?.getDefaultCategories(ArgumentMatchers.anyList())).thenReturn(testObservable)
6163
categoriesPresenter?.searchForCategories("test")
6264
verify(view)?.showProgress(true)
6365
verify(view)?.showError(null)

0 commit comments

Comments
 (0)