Fixes #2337 - Show captions of image in media details (conflicts fixed) (#2933)

* Api call

* added captions

* final commit

* some minor changes

* sigular

* test for captions

* mock fetchCaptionbyFileName

* corrected method name

* * used ? instead of !! (unsafe call on nullable)
* updated unit test for fetchCaptionByFilename()
This commit is contained in:
neslihanturan 2019-05-01 12:38:38 +03:00 committed by Ashish Kumar
parent 047059c490
commit 3c9b7ba7a8
9 changed files with 137 additions and 6 deletions

View file

@ -47,6 +47,7 @@ public class Media implements Parcelable {
protected String filename; protected String filename;
protected String description; // monolingual description on input... protected String description; // monolingual description on input...
protected String discussion; protected String discussion;
protected String caption;
protected long dataLength; protected long dataLength;
protected Date dateCreated; protected Date dateCreated;
protected @Nullable Date dateUploaded; protected @Nullable Date dateUploaded;
@ -240,6 +241,22 @@ public class Media implements Parcelable {
return imageUrl; return imageUrl;
} }
/**
* Sets the Caption of the file.
* @param caption
*/
public void setCaption(String caption) {
this.caption = caption;
}
/**
* Gets the file Caption as a string.
* @return file Caption as a string
*/
public String getCaption() {
return caption;
}
/** /**
* Gets the name of the file. * Gets the name of the file.
* @return file name as a string * @return file name as a string

View file

@ -1,5 +1,9 @@
package fr.free.nrw.commons; package fr.free.nrw.commons;
import android.util.Log;
import java.io.IOException;
import javax.inject.Inject; import javax.inject.Inject;
import javax.inject.Singleton; import javax.inject.Singleton;
@ -20,16 +24,18 @@ public class MediaDataExtractor {
private final MediaWikiApi mediaWikiApi; private final MediaWikiApi mediaWikiApi;
private final OkHttpJsonApiClient okHttpJsonApiClient; private final OkHttpJsonApiClient okHttpJsonApiClient;
@Inject @Inject
public MediaDataExtractor(MediaWikiApi mwApi, public MediaDataExtractor(MediaWikiApi mwApi,
OkHttpJsonApiClient okHttpJsonApiClient) { OkHttpJsonApiClient okHttpJsonApiClient) {
this.okHttpJsonApiClient = okHttpJsonApiClient; this.okHttpJsonApiClient = okHttpJsonApiClient;
this.mediaWikiApi = mwApi; this.mediaWikiApi = mwApi;
} }
/** /**
* Simplified method to extract all details required to show media details. * Simplified method to extract all details required to show media details.
* It fetches media object, deletion status and talk page for the filename * It fetches media object, deletion status, talk page and caption for the filename
* @param filename for which the details are to be fetched * @param filename for which the details are to be fetched
* @return full Media object with all details including deletion status and talk page * @return full Media object with all details including deletion status and talk page
*/ */
@ -37,8 +43,11 @@ public class MediaDataExtractor {
Single<Media> mediaSingle = getMediaFromFileName(filename); Single<Media> mediaSingle = getMediaFromFileName(filename);
Single<Boolean> pageExistsSingle = mediaWikiApi.pageExists("Commons:Deletion_requests/" + filename); Single<Boolean> pageExistsSingle = mediaWikiApi.pageExists("Commons:Deletion_requests/" + filename);
Single<String> discussionSingle = getDiscussion(filename); Single<String> discussionSingle = getDiscussion(filename);
return Single.zip(mediaSingle, pageExistsSingle, discussionSingle, (media, deletionStatus, discussion) -> { Single<String> captionSingle = getCaption(filename);
return Single.zip(mediaSingle, pageExistsSingle, discussionSingle, captionSingle, (media, deletionStatus, discussion, caption) -> {
media.setDiscussion(discussion); media.setDiscussion(discussion);
media.setCaption(caption);
if (deletionStatus) { if (deletionStatus) {
media.setRequestedDeletion(); media.setRequestedDeletion();
} }
@ -68,5 +77,19 @@ public class MediaDataExtractor {
Timber.e(throwable, "Error occurred while fetching discussion"); Timber.e(throwable, "Error occurred while fetching discussion");
return ""; return "";
}); });
}
/**
* Fetch caption from the MediaWiki API
* @param filename the filename we will return the caption for
* @return a single with caption string (an empty string if no caption)
*/
private Single<String> getCaption(String filename) {
return mediaWikiApi.fetchCaptionByFilename(filename)
.onErrorReturn(throwable -> {
Timber.e(throwable, "Error occurred while fetching caption");
return "";
});
} }
} }

View file

@ -101,6 +101,8 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment {
SimpleDraweeView image; SimpleDraweeView image;
@BindView(R.id.mediaDetailSpacer) @BindView(R.id.mediaDetailSpacer)
MediaDetailSpacer spacer; MediaDetailSpacer spacer;
@BindView(R.id.mediaDetailCaption)
TextView mediaCaption;
@BindView(R.id.mediaDetailTitle) @BindView(R.id.mediaDetailTitle)
TextView title; TextView title;
@BindView(R.id.mediaDetailDesc) @BindView(R.id.mediaDetailDesc)
@ -315,6 +317,7 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment {
coordinates.setText(prettyCoordinates(media)); coordinates.setText(prettyCoordinates(media));
uploadedDate.setText(prettyUploadedDate(media)); uploadedDate.setText(prettyUploadedDate(media));
mediaDiscussion.setText(prettyDiscussion(media)); mediaDiscussion.setText(prettyDiscussion(media));
mediaCaption.setText(prettyCaption(media));
categoryNames.clear(); categoryNames.clear();
categoryNames.addAll(media.getCategories()); categoryNames.addAll(media.getCategories());
@ -516,6 +519,16 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment {
return desc; return desc;
} }
} }
private String prettyCaption(Media media) {
String caption = media.getCaption().trim();
if (caption.equals("")) {
return getString(R.string.detail_caption_empty);
} else {
return caption;
}
}
private String prettyDiscussion(Media media) { private String prettyDiscussion(Media media) {
String disc = media.getDiscussion().trim(); String disc = media.getDiscussion().trim();
if (disc.equals("")) { if (disc.equals("")) {

View file

@ -3,6 +3,7 @@ package fr.free.nrw.commons.mwapi;
import android.content.Context; import android.content.Context;
import android.net.Uri; import android.net.Uri;
import android.text.TextUtils; import android.text.TextUtils;
import android.util.Log;
import com.google.gson.Gson; import com.google.gson.Gson;
@ -303,6 +304,26 @@ public class ApacheHttpClientMediaWikiApi implements MediaWikiApi {
.getString("/api/flow-parsoid-utils/@content")); .getString("/api/flow-parsoid-utils/@content"));
} }
/**
* fetches the Caption of the file with a given name.
* @param filename title of the file
* @return a single with media caption
*/
@Override
public Single<String> fetchCaptionByFilename(String filename) {
return Single.fromCallable(() -> {
CustomApiResult apiResult = api.action("wbgetentities")
.param("sites", "commonswiki")
.param("titles", filename)
.param("props", "labels")
.param("format", "xml")
.param("languages", Locale.getDefault().getLanguage())
.param("languagefallback", "1")
.get();
return apiResult.getString("/api/entities/entity/labels/label/@value");
});
}
@Override @Override
@NonNull @NonNull
public Single<MediaResult> fetchMediaByFilename(String filename) { public Single<MediaResult> fetchMediaByFilename(String filename) {

View file

@ -66,6 +66,8 @@ public interface MediaWikiApi {
Single<String> parseWikicode(String source); Single<String> parseWikicode(String source);
Single<String> fetchCaptionByFilename(String filename);
@NonNull @NonNull
Single<MediaResult> fetchMediaByFilename(String filename); Single<MediaResult> fetchMediaByFilename(String filename);

View file

@ -52,6 +52,34 @@
android:orientation="vertical" android:orientation="vertical"
android:padding="@dimen/standard_gap"> android:padding="@dimen/standard_gap">
<LinearLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:background="?attr/subBackground"
android:orientation="vertical"
android:padding="@dimen/standard_gap">
<TextView
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:paddingBottom="@dimen/tiny_gap"
android:text="@string/media_detail_caption"
android:textColor="@android:color/white"
android:textSize="@dimen/normal_text"
android:textStyle="bold" />
<TextView
android:id="@+id/mediaDetailCaption"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_gravity="start"
android:background="?attr/subBackground"
android:padding="@dimen/small_gap"
android:textColor="@android:color/white"
android:textSize="@dimen/description_text_size"
tools:text="Captions of the media" />
</LinearLayout>
<LinearLayout <LinearLayout
android:layout_width="match_parent" android:layout_width="match_parent"
android:layout_height="wrap_content" android:layout_height="wrap_content"

View file

@ -160,6 +160,7 @@
<string name="detail_panel_cats_none">None selected</string> <string name="detail_panel_cats_none">None selected</string>
<string name="detail_description_empty">No description</string> <string name="detail_description_empty">No description</string>
<string name="detail_discussion_empty">No discussion</string> <string name="detail_discussion_empty">No discussion</string>
<string name="detail_caption_empty">No caption</string>
<string name="detail_license_empty">Unknown license</string> <string name="detail_license_empty">Unknown license</string>
<string name="menu_refresh">Refresh</string> <string name="menu_refresh">Refresh</string>
<string name="storage_permission_title">Requesting Storage Permission</string> <string name="storage_permission_title">Requesting Storage Permission</string>
@ -173,6 +174,7 @@
<string name="upload_image_duplicate">This file already exists on Commons. Are you sure you want to proceed?</string> <string name="upload_image_duplicate">This file already exists on Commons. Are you sure you want to proceed?</string>
<string name="yes">Yes</string> <string name="yes">Yes</string>
<string name="no">No</string> <string name="no">No</string>
<string name="media_detail_caption">Caption</string>
<string name="media_detail_title">Title</string> <string name="media_detail_title">Title</string>
<string name="media_detail_description">Description</string> <string name="media_detail_description">Description</string>
<string name="media_detail_discussion">Discussion</string> <string name="media_detail_discussion">Discussion</string>

View file

@ -7,12 +7,9 @@ import io.reactivex.Single
import junit.framework.Assert.assertTrue import junit.framework.Assert.assertTrue
import org.junit.Before import org.junit.Before
import org.junit.Test import org.junit.Test
import org.mockito.ArgumentMatchers import org.mockito.*
import org.mockito.InjectMocks
import org.mockito.Mock
import org.mockito.Mockito.`when` import org.mockito.Mockito.`when`
import org.mockito.Mockito.mock import org.mockito.Mockito.mock
import org.mockito.MockitoAnnotations
/** /**
* Test methods in media data extractor * Test methods in media data extractor
@ -45,6 +42,8 @@ class MediaDataExtractorTest {
`when`(okHttpJsonApiClient?.getMedia(ArgumentMatchers.anyString(), ArgumentMatchers.anyBoolean())) `when`(okHttpJsonApiClient?.getMedia(ArgumentMatchers.anyString(), ArgumentMatchers.anyBoolean()))
.thenReturn(Single.just(mock(Media::class.java))) .thenReturn(Single.just(mock(Media::class.java)))
Mockito.`when`(mwApi?.fetchCaptionByFilename(ArgumentMatchers.anyString())).thenReturn(Single.just("test caption"))
`when`(mwApi?.pageExists(ArgumentMatchers.anyString())) `when`(mwApi?.pageExists(ArgumentMatchers.anyString()))
.thenReturn(Single.just(true)) .thenReturn(Single.just(true))

View file

@ -6,6 +6,7 @@ import fr.free.nrw.commons.BuildConfig
import fr.free.nrw.commons.TestCommonsApplication import fr.free.nrw.commons.TestCommonsApplication
import fr.free.nrw.commons.kvstore.JsonKvStore import fr.free.nrw.commons.kvstore.JsonKvStore
import fr.free.nrw.commons.utils.ConfigUtils import fr.free.nrw.commons.utils.ConfigUtils
import io.reactivex.observers.TestObserver
import okhttp3.OkHttpClient import okhttp3.OkHttpClient
import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockResponse
import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.MockWebServer
@ -23,6 +24,7 @@ import org.wikipedia.util.DateUtil
import java.net.URLDecoder import java.net.URLDecoder
import java.util.* import java.util.*
@RunWith(RobolectricTestRunner::class) @RunWith(RobolectricTestRunner::class)
@Config(constants = BuildConfig::class, sdk = intArrayOf(21), application = TestCommonsApplication::class) @Config(constants = BuildConfig::class, sdk = intArrayOf(21), application = TestCommonsApplication::class)
class ApacheHttpClientMediaWikiApiTest { class ApacheHttpClientMediaWikiApiTest {
@ -240,6 +242,30 @@ class ApacheHttpClientMediaWikiApiTest {
assertFalse(result) assertFalse(result)
} }
@Test
fun fetchCaptionByFilename() {
server.enqueue(MockResponse().setBody("<?xml version=\"1.0\"?><api success=\"1\"><entities><entity type=\"mediainfo\" id=\"M77157483\"><labels><label language=\"it\" value=\"Test\" /></labels><statements /></entity></entities></api>"))
val result = testObject.fetchCaptionByFilename("File:foo")
val testObserver = TestObserver<String>()
result.subscribe(testObserver)
assertBasicRequestParameters(server, "GET").let { request ->
parseQueryParams(request).let { params ->
assertEquals("xml", params["format"])
assertEquals("wbgetentities", params["action"])
assertEquals("commonswiki", params["sites"])
assertEquals("File:foo", params["titles"])
assertEquals("labels", params["props"])
assertEquals(Locale.getDefault().getLanguage(), params["languages"])
assertEquals("1", params["languagefallback"])
}
}
testObserver.assertResult("Test")
testObserver.assertNoErrors()
}
@Test @Test
fun isUserBlockedFromCommonsForInfinitelyBlockedUser() { fun isUserBlockedFromCommonsForInfinitelyBlockedUser() {
server.enqueue(MockResponse().setBody("<?xml version=\"1.0\"?><api><query><userinfo id=\"1000\" name=\"testusername\" blockid=\"3000\" blockedby=\"blockerusername\" blockedbyid=\"1001\" blockreason=\"testing\" blockedtimestamp=\"2018-05-24T15:32:09Z\" blockexpiry=\"infinite\"></userinfo></query></api>")) server.enqueue(MockResponse().setBody("<?xml version=\"1.0\"?><api><query><userinfo id=\"1000\" name=\"testusername\" blockid=\"3000\" blockedby=\"blockerusername\" blockedbyid=\"1001\" blockreason=\"testing\" blockedtimestamp=\"2018-05-24T15:32:09Z\" blockexpiry=\"infinite\"></userinfo></query></api>"))