Skip to content

Revert "Fixes #2337 - Show captions of image in media details (confli… #2934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 1, 2019
Merged

Revert "Fixes #2337 - Show captions of image in media details (confli… #2934

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 0 additions & 17 deletions app/src/main/java/fr/free/nrw/commons/Media.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ public Media[] newArray(int i) {
protected String filename;
protected String description; // monolingual description on input...
protected String discussion;
protected String caption;
protected long dataLength;
protected Date dateCreated;
protected @Nullable Date dateUploaded;
Expand Down Expand Up @@ -241,22 +240,6 @@ public String getImageUrl() {
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.
* @return file name as a string
Expand Down
27 changes: 2 additions & 25 deletions app/src/main/java/fr/free/nrw/commons/MediaDataExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
package fr.free.nrw.commons;

import android.util.Log;

import java.io.IOException;

import javax.inject.Inject;
import javax.inject.Singleton;

Expand All @@ -24,30 +20,25 @@ public class MediaDataExtractor {
private final MediaWikiApi mediaWikiApi;
private final OkHttpJsonApiClient okHttpJsonApiClient;


@Inject
public MediaDataExtractor(MediaWikiApi mwApi,
OkHttpJsonApiClient okHttpJsonApiClient) {
this.okHttpJsonApiClient = okHttpJsonApiClient;
this.mediaWikiApi = mwApi;

}

/**
* Simplified method to extract all details required to show media details.
* It fetches media object, deletion status, talk page and caption for the filename
* It fetches media object, deletion status and talk page for the filename
* @param filename for which the details are to be fetched
* @return full Media object with all details including deletion status and talk page
*/
public Single<Media> fetchMediaDetails(String filename) {
Single<Media> mediaSingle = getMediaFromFileName(filename);
Single<Boolean> pageExistsSingle = mediaWikiApi.pageExists("Commons:Deletion_requests/" + filename);
Single<String> discussionSingle = getDiscussion(filename);
Single<String> captionSingle = getCaption(filename);

return Single.zip(mediaSingle, pageExistsSingle, discussionSingle, captionSingle, (media, deletionStatus, discussion, caption) -> {
return Single.zip(mediaSingle, pageExistsSingle, discussionSingle, (media, deletionStatus, discussion) -> {
media.setDiscussion(discussion);
media.setCaption(caption);
if (deletionStatus) {
media.setRequestedDeletion();
}
Expand Down Expand Up @@ -77,19 +68,5 @@ private Single<String> getDiscussion(String filename) {
Timber.e(throwable, "Error occurred while fetching discussion");
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 "";
});
}
}
13 changes: 0 additions & 13 deletions app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,6 @@ public static MediaDetailFragment forMedia(int index, boolean editable, boolean
SimpleDraweeView image;
@BindView(R.id.mediaDetailSpacer)
MediaDetailSpacer spacer;
@BindView(R.id.mediaDetailCaption)
TextView mediaCaption;
@BindView(R.id.mediaDetailTitle)
TextView title;
@BindView(R.id.mediaDetailDesc)
Expand Down Expand Up @@ -317,7 +315,6 @@ private void setTextFields(Media media) {
coordinates.setText(prettyCoordinates(media));
uploadedDate.setText(prettyUploadedDate(media));
mediaDiscussion.setText(prettyDiscussion(media));
mediaCaption.setText(prettyCaption(media));

categoryNames.clear();
categoryNames.addAll(media.getCategories());
Expand Down Expand Up @@ -519,16 +516,6 @@ private String prettyDescription(Media media) {
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) {
String disc = media.getDiscussion().trim();
if (disc.equals("")) {
Expand Down
21 changes: 0 additions & 21 deletions app/src/main/java/fr/free/nrw/commons/mwapi/ApacheHttpClientMediaWikiApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import android.content.Context;
import android.net.Uri;
import android.text.TextUtils;
import android.util.Log;

import com.google.gson.Gson;

Expand Down Expand Up @@ -304,26 +303,6 @@ public Single<String> parseWikicode(String source) {
.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
@NonNull
public Single<MediaResult> fetchMediaByFilename(String filename) {
Expand Down
2 changes: 0 additions & 2 deletions app/src/main/java/fr/free/nrw/commons/mwapi/MediaWikiApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ Single<UploadResult> uploadFileFinalize(String filename, String filekey,

Single<String> parseWikicode(String source);

Single<String> fetchCaptionByFilename(String filename);

@NonNull
Single<MediaResult> fetchMediaByFilename(String filename);

Expand Down
28 changes: 0 additions & 28 deletions app/src/main/res/layout/fragment_media_detail.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,34 +52,6 @@
android:orientation="vertical"
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
android:layout_width="match_parent"
android:layout_height="wrap_content"
Expand Down
2 changes: 0 additions & 2 deletions app/src/main/res/values/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,6 @@
<string name="detail_panel_cats_none">None selected</string>
<string name="detail_description_empty">No description</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="menu_refresh">Refresh</string>
<string name="storage_permission_title">Requesting Storage Permission</string>
Expand All @@ -174,7 +173,6 @@
<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="no">No</string>
<string name="media_detail_caption">Caption</string>
<string name="media_detail_title">Title</string>
<string name="media_detail_description">Description</string>
<string name="media_detail_discussion">Discussion</string>
Expand Down
7 changes: 4 additions & 3 deletions app/src/test/kotlin/fr/free/nrw/commons/MediaDataExtractorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,12 @@ import io.reactivex.Single
import junit.framework.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.mockito.*
import org.mockito.ArgumentMatchers
import org.mockito.InjectMocks
import org.mockito.Mock
import org.mockito.Mockito.`when`
import org.mockito.Mockito.mock
import org.mockito.MockitoAnnotations

/**
* Test methods in media data extractor
Expand Down Expand Up @@ -42,8 +45,6 @@ class MediaDataExtractorTest {
`when`(okHttpJsonApiClient?.getMedia(ArgumentMatchers.anyString(), ArgumentMatchers.anyBoolean()))
.thenReturn(Single.just(mock(Media::class.java)))

Mockito.`when`(mwApi?.fetchCaptionByFilename(ArgumentMatchers.anyString())).thenReturn(Single.just("test caption"))

`when`(mwApi?.pageExists(ArgumentMatchers.anyString()))
.thenReturn(Single.just(true))

Expand Down
26 changes: 0 additions & 26 deletions app/src/test/kotlin/fr/free/nrw/commons/mwapi/ApacheHttpClientMediaWikiApiTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import fr.free.nrw.commons.BuildConfig
import fr.free.nrw.commons.TestCommonsApplication
import fr.free.nrw.commons.kvstore.JsonKvStore
import fr.free.nrw.commons.utils.ConfigUtils
import io.reactivex.observers.TestObserver
import okhttp3.OkHttpClient
import okhttp3.mockwebserver.MockResponse
import okhttp3.mockwebserver.MockWebServer
Expand All @@ -24,7 +23,6 @@ import org.wikipedia.util.DateUtil
import java.net.URLDecoder
import java.util.*


@RunWith(RobolectricTestRunner::class)
@Config(constants = BuildConfig::class, sdk = intArrayOf(21), application = TestCommonsApplication::class)
class ApacheHttpClientMediaWikiApiTest {
Expand Down Expand Up @@ -242,30 +240,6 @@ class ApacheHttpClientMediaWikiApiTest {
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
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>"))
Expand Down