Review category issues (#4897)

* Ask for category only if exists in Peer Review

* Minor Fixes

* Fixed wrong categories issue

* Added comments

* Added comments

* Minor Changes

* Added test

* Tests

* Tests

* Tests
This commit is contained in:
Devarsh Mavani 2022-03-17 15:42:30 +05:30 committed by GitHub
parent 7bc78f67ff
commit 103e2d546e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 171 additions and 13 deletions

View file

@ -3,6 +3,7 @@ package fr.free.nrw.commons
import android.os.Parcelable
import fr.free.nrw.commons.location.LatLng
import kotlinx.android.parcel.Parcelize
import org.wikipedia.dataclient.mwapi.MwQueryPage
import org.wikipedia.page.PageTitle
import java.util.*
@ -77,7 +78,13 @@ class Media constructor(
var coordinates: LatLng? = null,
var captions: Map<String, String> = emptyMap(),
var descriptions: Map<String, String> = emptyMap(),
var depictionIds: List<String> = emptyList()
var depictionIds: List<String> = emptyList(),
/**
* This field was added to find non-hidden categories
* Stores the mapping of category title to hidden attribute
* Example: "Mountains" => false, "CC-BY-SA-2.0" => true
*/
var categoriesHiddenStatus: Map<String, Boolean> = emptyMap()
) : Parcelable {
constructor(

View file

@ -14,7 +14,7 @@ import fr.free.nrw.commons.upload.depicts.DepictsDao
* The database for accessing the respective DAOs
*
*/
@Database(entities = [Contribution::class, Depicts::class, UploadedStatus::class], version = 10, exportSchema = false)
@Database(entities = [Contribution::class, Depicts::class, UploadedStatus::class], version = 11, exportSchema = false)
@TypeConverters(Converters::class)
abstract class AppDatabase : RoomDatabase() {
abstract fun contributionDao(): ContributionDao

View file

@ -79,11 +79,21 @@ public class Converters {
return writeObjectToString(objectList);
}
@TypeConverter
public static String mapObjectToString2(Map<String,Boolean> objectList) {
return writeObjectToString(objectList);
}
@TypeConverter
public static Map<String,String> stringToMap(String objectList) {
return readObjectWithTypeToken(objectList, new TypeToken<Map<String,String>>(){});
}
@TypeConverter
public static Map<String,Boolean> stringToMap2(String objectList) {
return readObjectWithTypeToken(objectList, new TypeToken<Map<String,Boolean>>(){});
}
@TypeConverter
public static String latlngObjectToString(LatLng latlng) {
return writeObjectToString(latlng);

View file

@ -20,6 +20,10 @@ class MediaConverter @Inject constructor() {
fun convert(page: MwQueryPage, entity: Entities.Entity, imageInfo: ImageInfo): Media {
val metadata = imageInfo.metadata
requireNotNull(metadata) { "No metadata" }
// Stores mapping of title attribute to hidden attribute of each category
val myMap = mutableMapOf<String, Boolean>()
page.categories()?.forEach { myMap[it.title()] = (it.hidden()) }
return Media(
page.pageId().toString(),
imageInfo.thumbUrl.takeIf { it.isNotBlank() } ?: imageInfo.originalUrl,
@ -35,7 +39,8 @@ class MediaConverter @Inject constructor() {
metadata.latLng,
entity.labels().mapValues { it.value.value() },
entity.descriptions().mapValues { it.value.value() },
entity.depictionIds()
entity.depictionIds(),
myMap
)
}

View file

@ -3,7 +3,6 @@ package fr.free.nrw.commons.media;
import io.reactivex.Single;
import java.util.Map;
import org.wikipedia.dataclient.mwapi.MwQueryResponse;
import retrofit2.Call;
import retrofit2.http.GET;
import retrofit2.http.Query;
import retrofit2.http.QueryMap;
@ -16,6 +15,13 @@ public interface MediaInterface {
"&iiextmetadatafilter=DateTime|Categories|GPSLatitude|GPSLongitude|ImageDescription|DateTimeOriginal" +
"|Artist|LicenseShortName|LicenseUrl";
/**
* fetches category detail(title, hidden) for each category along with File information
*/
String MEDIA_PARAMS_WITH_CATEGORY_DETAILS ="&clprop=hidden&prop=categories|imageinfo&iiprop=url|extmetadata|user&&iiurlwidth=640" +
"&iiextmetadatafilter=DateTime|GPSLatitude|GPSLongitude|ImageDescription|DateTimeOriginal" +
"|Artist|LicenseShortName|LicenseUrl";
/**
* Checks if a page exists or not.
*
@ -81,7 +87,7 @@ public interface MediaInterface {
* @return
*/
@GET("w/api.php?action=query&format=json&formatversion=2" +
MEDIA_PARAMS)
MEDIA_PARAMS_WITH_CATEGORY_DETAILS)
Single<MwQueryResponse> getMedia(@Query("titles") String title);
/**

View file

@ -71,6 +71,11 @@ public class ReviewActivity extends BaseActivity {
*/
private ReviewImageFragment reviewImageFragment;
/**
* Flag to check whether there are any non-hidden categories in the File
*/
private boolean hasNonHiddenCategories = false;
final String SAVED_MEDIA = "saved_media";
private Media media;
@ -153,17 +158,35 @@ public class ReviewActivity extends BaseActivity {
@SuppressLint("CheckResult")
public boolean runRandomizer() {
hasNonHiddenCategories = false;
progressBar.setVisibility(View.VISIBLE);
reviewPager.setCurrentItem(0);
compositeDisposable.add(reviewHelper.getRandomMedia()
.subscribeOn(Schedulers.io())
.observeOn(AndroidSchedulers.mainThread())
.subscribe(media -> {
// Finds non-hidden categories from Media instance
findNonHiddenCategories(media);
}));
return true;
}
/**
* Finds non-hidden categories and updates current image
*/
private void findNonHiddenCategories(Media media) {
for(String key : media.getCategoriesHiddenStatus().keySet()) {
Boolean value = media.getCategoriesHiddenStatus().get(key);
// If non-hidden category is found then set hasNonHiddenCategories to true
// so that category review cannot be skipped
if(!value) {
hasNonHiddenCategories = true;
break;
}
}
reviewImageFragment = getInstanceOfReviewImageFragment();
reviewImageFragment.disableButtons();
updateImage(media);
}));
return true;
}
@SuppressLint("CheckResult")
@ -195,8 +218,16 @@ public class ReviewActivity extends BaseActivity {
public void swipeToNext() {
int nextPos = reviewPager.getCurrentItem() + 1;
// If currently at category fragment, then check whether the media has any non-hidden category
if (nextPos <= 3) {
reviewPager.setCurrentItem(nextPos);
if (nextPos == 2) {
// The media has no non-hidden category. Such media are already flagged by server-side bots, so no need to review manually.
if (!hasNonHiddenCategories) {
swipeToNext();
return;
}
}
} else {
runRandomizer();
}

View file

@ -19,6 +19,8 @@ import butterknife.OnClick;
import fr.free.nrw.commons.Media;
import fr.free.nrw.commons.R;
import fr.free.nrw.commons.di.CommonsDaggerSupportFragment;
import java.util.ArrayList;
import java.util.List;
public class ReviewImageFragment extends CommonsDaggerSupportFragment {
@ -52,8 +54,20 @@ public class ReviewImageFragment extends CommonsDaggerSupportFragment {
private String updateCategoriesQuestion() {
Media media = getReviewActivity().getMedia();
if (media != null && media.getCategories() != null && isAdded()) {
String catString = TextUtils.join(", ", media.getCategories());
if (media != null && media.getCategoriesHiddenStatus() != null && isAdded()) {
// Filter category name attribute from all categories
List<String> categories = new ArrayList<>();
for(String key : media.getCategoriesHiddenStatus().keySet()) {
String value = String.valueOf(key);
// Each category returned has a format like "Category:<some-category-name>"
// so remove the prefix "Category:"
int index = key.indexOf("Category:");
if(index == 0) {
value = key.substring(9);
}
categories.add(value);
}
String catString = TextUtils.join(", ", categories);
if (catString != null && !catString.equals("") && textViewQuestionContext != null) {
catString = "<b>" + catString + "</b>";
String stringToConvertHtml = String.format(getResources().getString(R.string.review_category_explanation), catString);

View file

@ -1,21 +1,37 @@
package fr.free.nrw.commons.review
import android.content.Context
import android.os.Looper.getMainLooper
import android.view.Menu
import android.view.MenuItem
import android.widget.Button
import butterknife.BindView
import com.facebook.drawee.backends.pipeline.Fresco
import com.facebook.soloader.SoLoader
import com.nhaarman.mockitokotlin2.doNothing
import fr.free.nrw.commons.Media
import fr.free.nrw.commons.TestAppAdapter
import fr.free.nrw.commons.TestCommonsApplication
import io.reactivex.Scheduler
import io.reactivex.Single
import io.reactivex.android.schedulers.AndroidSchedulers
import io.reactivex.schedulers.Schedulers
import org.junit.Assert
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.ArgumentMatchers
import org.mockito.Mock
import org.mockito.Mockito.*
import org.mockito.MockitoAnnotations
import org.mockito.Spy
import org.powermock.reflect.Whitebox
import org.robolectric.Robolectric
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
import org.robolectric.Shadows.shadowOf
import org.robolectric.annotation.Config
import org.robolectric.annotation.LooperMode
import org.robolectric.fakes.RoboMenu
import org.robolectric.fakes.RoboMenuItem
import org.wikipedia.AppAdapter
@ -23,6 +39,7 @@ import java.lang.reflect.Method
@RunWith(RobolectricTestRunner::class)
@Config(sdk = [21], application = TestCommonsApplication::class)
@LooperMode(LooperMode.Mode.PAUSED)
class ReviewActivityTest {
private lateinit var activity: ReviewActivity
@ -33,6 +50,20 @@ class ReviewActivityTest {
private lateinit var context: Context
@Mock
private lateinit var reviewPagerAdapter: ReviewPagerAdapter
@Mock
var reviewPager: ReviewViewPager? = null
var hasNonHiddenCategories: Boolean = false
@Mock
var reviewHelper: ReviewHelper? = null
@Mock
private lateinit var reviewImageFragment: ReviewImageFragment
@Before
fun setUp() {
MockitoAnnotations.initMocks(this)
@ -50,7 +81,11 @@ class ReviewActivityTest {
menuItem = RoboMenuItem(null)
menu = RoboMenu(context)
Whitebox.setInternalState(activity, "reviewPager", reviewPager);
Whitebox.setInternalState(activity, "hasNonHiddenCategories", hasNonHiddenCategories);
Whitebox.setInternalState(activity, "reviewHelper", reviewHelper);
Whitebox.setInternalState(activity, "reviewImageFragment", reviewImageFragment);
Whitebox.setInternalState(activity, "reviewPagerAdapter", reviewPagerAdapter);
}
@ -69,9 +104,46 @@ class ReviewActivityTest {
@Test
@Throws(Exception::class)
fun testSwipeToNext() {
shadowOf(getMainLooper()).idle()
doReturn(1,2).`when`(reviewPager)?.currentItem
activity.swipeToNext()
}
@Test
@Throws(Exception::class)
fun testSwipeToLastFragment() {
shadowOf(getMainLooper()).idle()
doReturn(3).`when`(reviewPager)?.currentItem
val media = mock(Media::class.java)
doReturn(mapOf<String, Boolean>("test" to false)).`when`(media).categoriesHiddenStatus
doReturn(Single.just(media)).`when`(reviewHelper)?.randomMedia
Assert.assertNotNull(reviewHelper?.randomMedia)
reviewHelper
?.randomMedia
?.test()
?.assertValue(media);
activity.swipeToNext()
}
@Test
@Throws(Exception::class)
fun testFindNonHiddenCategories() {
shadowOf(getMainLooper()).idle()
val media = mock(Media::class.java)
doReturn(mapOf<String, Boolean>("test" to false)).`when`(media).categoriesHiddenStatus
doReturn(mock(ReviewImageFragment::class.java)).`when`(reviewPagerAdapter).instantiateItem(ArgumentMatchers.any(), anyInt())
doReturn("").`when`(media).filename
doNothing().`when`(reviewImageFragment).disableButtons()
var findNonHiddenCategory: Method =
ReviewActivity::class.java.getDeclaredMethod("findNonHiddenCategories"
, Media::class.java)
findNonHiddenCategory.isAccessible = true
findNonHiddenCategory.invoke(activity, media)
}
@Test
@Throws(Exception::class)
fun testOnDestroy() {

View file

@ -11,6 +11,8 @@ import androidx.fragment.app.FragmentManager
import androidx.fragment.app.FragmentTransaction
import com.facebook.drawee.backends.pipeline.Fresco
import com.facebook.soloader.SoLoader
import com.nhaarman.mockitokotlin2.doReturn
import fr.free.nrw.commons.Media
import fr.free.nrw.commons.R
import fr.free.nrw.commons.TestAppAdapter
import fr.free.nrw.commons.TestCommonsApplication
@ -20,7 +22,10 @@ import org.junit.Test
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.runner.RunWith
import org.mockito.Mock
import org.mockito.Mockito.`when`
import org.mockito.Mockito.mock
import org.mockito.MockitoAnnotations
import org.powermock.reflect.Whitebox
import org.robolectric.Robolectric
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
@ -51,6 +56,7 @@ class ReviewImageFragmentTest {
@Mock
private lateinit var savedInstanceState: Bundle
private lateinit var activity: ReviewActivity
@Before
fun setUp() {
@ -61,7 +67,7 @@ class ReviewImageFragmentTest {
SoLoader.setInTestMode()
Fresco.initialize(context)
val activity = Robolectric.buildActivity(ReviewActivity::class.java).create().get()
activity = Robolectric.buildActivity(ReviewActivity::class.java).create().get()
fragment = ReviewImageFragment()
val bundle = Bundle()
bundle.putInt("position", 1)
@ -110,10 +116,17 @@ class ReviewImageFragmentTest {
@Test
@Throws(Exception::class)
fun testOnUpdateCategoriesQuestion() {
shadowOf(Looper.getMainLooper()).idle()
val media = mock(Media::class.java)
Whitebox.setInternalState(activity, "media", media)
Assert.assertNotNull(media)
val categories = mapOf<String, Boolean>("Category:" to false)
doReturn(categories).`when`(media).categoriesHiddenStatus
Assert.assertNotNull(media.categoriesHiddenStatus)
Assert.assertNotNull(fragment.isAdded)
val method: Method =
ReviewImageFragment::class.java.getDeclaredMethod("updateCategoriesQuestion")
method.isAccessible = true
shadowOf(Looper.getMainLooper()).idle()
method.invoke(fragment)
}