Skip to content

Commit 329a682

Browse files
authored
Improve credit line in image list (#6295)
- When author is not uploader, show both. - When failing to parse author from HTML, use structured data.
1 parent 3076297 commit 329a682

21 files changed

+363
-81
lines changed

app/build.gradle

+2
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,7 @@ android {
333333
buildConfigField "String", "TEST_USERNAME", "\"" + getTestUserName() + "\""
334334
buildConfigField "String", "TEST_PASSWORD", "\"" + getTestPassword() + "\""
335335
buildConfigField "String", "DEPICTS_PROPERTY", "\"P180\""
336+
buildConfigField "String", "CREATOR_PROPERTY", "\"P170\""
336337
dimension 'tier'
337338
}
338339

@@ -370,6 +371,7 @@ android {
370371
buildConfigField "String", "TEST_USERNAME", "\"" + getTestUserName() + "\""
371372
buildConfigField "String", "TEST_PASSWORD", "\"" + getTestPassword() + "\""
372373
buildConfigField "String", "DEPICTS_PROPERTY", "\"P245962\""
374+
buildConfigField "String", "CREATOR_PROPERTY", "\"P253075\""
373375
dimension 'tier'
374376
}
375377
}

app/src/main/java/fr/free/nrw/commons/Media.kt

+16
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ class Media constructor(
5353
*/
5454
var author: String? = null,
5555
var user: String? = null,
56+
var creatorName: String? = null,
5657
/**
5758
* Gets the categories the file falls under.
5859
* @return file categories as an ArrayList of Strings
@@ -66,6 +67,7 @@ class Media constructor(
6667
var captions: Map<String, String> = emptyMap(),
6768
var descriptions: Map<String, String> = emptyMap(),
6869
var depictionIds: List<String> = emptyList(),
70+
var creatorIds: List<String> = emptyList(),
6971
/**
7072
* This field was added to find non-hidden categories
7173
* Stores the mapping of category title to hidden attribute
@@ -130,6 +132,7 @@ class Media constructor(
130132
* returns user
131133
* @return Author or User
132134
*/
135+
@Deprecated("Use user for uploader username. Use attributedAuthor() for attribution. Note that the uploader may not be the creator/author.")
133136
fun getAuthorOrUser(): String? {
134137
return if (!author.isNullOrEmpty()) {
135138
author
@@ -138,6 +141,19 @@ class Media constructor(
138141
}
139142
}
140143

144+
/**
145+
* Returns author if it's not null or empty, otherwise
146+
* returns creator name
147+
* @return name of author or creator
148+
*/
149+
fun getAttributedAuthor(): String? {
150+
return if (!author.isNullOrEmpty()) {
151+
author
152+
} else{
153+
creatorName
154+
}
155+
}
156+
141157
/**
142158
* Gets media display title
143159
* @return Media title

app/src/main/java/fr/free/nrw/commons/MediaDataExtractor.kt

+13-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package fr.free.nrw.commons
22

33
import androidx.core.text.HtmlCompat
4-
import fr.free.nrw.commons.media.IdAndCaptions
4+
import fr.free.nrw.commons.media.IdAndLabels
55
import fr.free.nrw.commons.media.MediaClient
66
import fr.free.nrw.commons.media.PAGE_ID_PREFIX
77
import io.reactivex.Single
@@ -23,13 +23,23 @@ class MediaDataExtractor
2323
private val mediaClient: MediaClient,
2424
) {
2525
fun fetchDepictionIdsAndLabels(media: Media) =
26-
mediaClient
26+
mediaClient
2727
.getEntities(media.depictionIds)
2828
.map {
2929
it
3030
.entities()
3131
.mapValues { entry -> entry.value.labels().mapValues { it.value.value() } }
32-
}.map { it.map { (key, value) -> IdAndCaptions(key, value) } }
32+
}.map { it.map { (key, value) -> IdAndLabels(key, value) } }
33+
.onErrorReturn { emptyList() }
34+
35+
fun fetchCreatorIdsAndLabels(media: Media) =
36+
mediaClient
37+
.getEntities(media.creatorIds)
38+
.map {
39+
it
40+
.entities()
41+
.mapValues { entry -> entry.value.labels().mapValues { it.value.value() } }
42+
}.map { it.map { (key, value) -> IdAndLabels(key, value) } }
3343
.onErrorReturn { emptyList() }
3444

3545
fun checkDeletionRequestExists(media: Media) = mediaClient.checkPageExistsUsingTitle("Commons:Deletion_requests/" + media.filename)

app/src/main/java/fr/free/nrw/commons/contributions/ContributionViewHolder.kt

+34-4
Original file line numberDiff line numberDiff line change
@@ -8,23 +8,29 @@ import androidx.appcompat.app.AlertDialog
88
import androidx.recyclerview.widget.RecyclerView
99
import com.facebook.imagepipeline.request.ImageRequest
1010
import com.facebook.imagepipeline.request.ImageRequestBuilder
11+
import fr.free.nrw.commons.Media
12+
import fr.free.nrw.commons.utils.MediaAttributionUtil
13+
import fr.free.nrw.commons.MediaDataExtractor
1114
import fr.free.nrw.commons.R
1215
import fr.free.nrw.commons.databinding.LayoutContributionBinding
1316
import fr.free.nrw.commons.media.MediaClient
1417
import io.reactivex.android.schedulers.AndroidSchedulers
1518
import io.reactivex.disposables.CompositeDisposable
1619
import io.reactivex.schedulers.Schedulers
20+
import timber.log.Timber
1721
import java.io.File
1822

1923
class ContributionViewHolder internal constructor(
20-
private val parent: View, private val callback: ContributionsListAdapter.Callback,
21-
private val mediaClient: MediaClient
24+
parent: View,
25+
private val callback: ContributionsListAdapter.Callback,
26+
private val compositeDisposable: CompositeDisposable,
27+
private val mediaClient: MediaClient,
28+
private val mediaDataExtractor: MediaDataExtractor
2229
) : RecyclerView.ViewHolder(parent) {
2330
var binding: LayoutContributionBinding = LayoutContributionBinding.bind(parent)
2431

2532
private var position = 0
2633
private var contribution: Contribution? = null
27-
private val compositeDisposable = CompositeDisposable()
2834
private var isWikipediaButtonDisplayed = false
2935
private val pausingPopUp: AlertDialog
3036
var imageRequest: ImageRequest? = null
@@ -54,7 +60,7 @@ an upload might take a dozen seconds. */
5460
this.contribution = contribution
5561
this.position = position
5662
binding.contributionTitle.text = contribution.media.mostRelevantCaption
57-
binding.authorView.text = contribution.media.getAuthorOrUser()
63+
setAuthorText(contribution.media)
5864

5965
//Removes flicker of loading image.
6066
binding.contributionImage.hierarchy.fadeDuration = 0
@@ -93,6 +99,30 @@ an upload might take a dozen seconds. */
9399
checkIfMediaExistsOnWikipediaPage(contribution)
94100
}
95101

102+
fun updateAttribution() {
103+
if (contribution != null) {
104+
val media = contribution!!.media
105+
if (!media.getAttributedAuthor().isNullOrEmpty()) {
106+
return
107+
}
108+
compositeDisposable.addAll(
109+
mediaDataExtractor.fetchCreatorIdsAndLabels(media)
110+
.subscribeOn(Schedulers.io())
111+
.observeOn(AndroidSchedulers.mainThread())
112+
.subscribe(
113+
{ idAndLabels ->
114+
media.creatorName = MediaAttributionUtil.getCreatorName(idAndLabels)
115+
setAuthorText(media)
116+
},
117+
{ t: Throwable? -> Timber.e(t) })
118+
)
119+
}
120+
}
121+
122+
private fun setAuthorText(media: Media) {
123+
binding.authorView.text = MediaAttributionUtil.getTagLine(media, itemView.context)
124+
}
125+
96126
/**
97127
* Checks if a media exists on the corresponding Wikipedia article Currently the check is made
98128
* for the device's current language Wikipedia

app/src/main/java/fr/free/nrw/commons/contributions/ContributionsListAdapter.kt

+7-2
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,26 @@ import android.view.LayoutInflater
44
import android.view.ViewGroup
55
import androidx.paging.PagedListAdapter
66
import androidx.recyclerview.widget.DiffUtil
7+
import fr.free.nrw.commons.MediaDataExtractor
78
import fr.free.nrw.commons.R
89
import fr.free.nrw.commons.media.MediaClient
10+
import io.reactivex.disposables.CompositeDisposable
911

1012
/**
1113
* Represents The View Adapter for the List of Contributions
1214
*/
1315
class ContributionsListAdapter internal constructor(
1416
private val callback: Callback,
15-
private val mediaClient: MediaClient
17+
private val mediaClient: MediaClient,
18+
private val mediaDataExtractor: MediaDataExtractor,
19+
private val compositeDisposable: CompositeDisposable
1620
) : PagedListAdapter<Contribution, ContributionViewHolder>(DIFF_CALLBACK) {
1721
/**
1822
* Initializes the view holder with contribution data
1923
*/
2024
override fun onBindViewHolder(holder: ContributionViewHolder, position: Int) {
2125
holder.init(position, getItem(position))
26+
holder.updateAttribution()
2227
}
2328

2429
fun getContributionForPosition(position: Int): Contribution? {
@@ -36,7 +41,7 @@ class ContributionsListAdapter internal constructor(
3641
val viewHolder = ContributionViewHolder(
3742
LayoutInflater.from(parent.context)
3843
.inflate(R.layout.layout_contribution, parent, false),
39-
callback, mediaClient
44+
callback, compositeDisposable, mediaClient, mediaDataExtractor
4045
)
4146
return viewHolder
4247
}

app/src/main/java/fr/free/nrw/commons/contributions/ContributionsListFragment.kt

+6-1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import androidx.recyclerview.widget.RecyclerView.AdapterDataObserver
2727
import androidx.recyclerview.widget.RecyclerView.OnItemTouchListener
2828
import androidx.recyclerview.widget.SimpleItemAnimator
2929
import fr.free.nrw.commons.Media
30+
import fr.free.nrw.commons.MediaDataExtractor
3031
import fr.free.nrw.commons.R
3132
import fr.free.nrw.commons.Utils
3233
import fr.free.nrw.commons.auth.SessionManager
@@ -63,6 +64,10 @@ class ContributionsListFragment : CommonsDaggerSupportFragment(), ContributionsL
6364
@Inject
6465
var mediaClient: MediaClient? = null
6566

67+
@JvmField
68+
@Inject
69+
var mediaDataExtractor: MediaDataExtractor? = null
70+
6671
@JvmField
6772
@Named(NetworkingModule.NAMED_LANGUAGE_WIKI_PEDIA_WIKI_SITE)
6873
@Inject
@@ -231,7 +236,7 @@ class ContributionsListFragment : CommonsDaggerSupportFragment(), ContributionsL
231236
}
232237

233238
private fun initAdapter() {
234-
adapter = ContributionsListAdapter(this, mediaClient!!)
239+
adapter = ContributionsListAdapter(this, mediaClient!!, mediaDataExtractor!!, compositeDisposable)
235240
}
236241

237242
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {

app/src/main/java/fr/free/nrw/commons/explore/media/MediaConverter.kt

+31-20
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@ import javax.inject.Inject
1818
class MediaConverter
1919
@Inject
2020
constructor() {
21+
/**
22+
* Creating Media object from MWQueryPage.
23+
*
24+
* @param page response from the API
25+
* @return Media object
26+
*/
2127
fun convert(
2228
page: MwQueryPage,
2329
entity: Entities.Entity,
@@ -40,24 +46,17 @@ class MediaConverter
4046
metadata.prefixedLicenseUrl,
4147
getAuthor(metadata),
4248
imageInfo.getUser(),
49+
null,
4350
MediaDataExtractorUtil.extractCategoriesFromList(metadata.categories()),
4451
metadata.latLng,
4552
entity.labels().mapValues { it.value.value() },
4653
entity.descriptions().mapValues { it.value.value() },
4754
entity.depictionIds(),
55+
entity.creatorIds(),
4856
myMap,
4957
)
5058
}
5159

52-
/**
53-
* Creating Media object from MWQueryPage.
54-
* Earlier only basic details were set for the media object but going forward,
55-
* a full media object(with categories, descriptions, coordinates etc) can be constructed using this method
56-
*
57-
* @param page response from the API
58-
* @return Media object
59-
*/
60-
6160
private fun safeParseDate(dateStr: String): Date? =
6261
try {
6362
CommonsDateUtil.getMediaSimpleDateFormat().parse(dateStr)
@@ -66,24 +65,32 @@ class MediaConverter
6665
}
6766

6867
/**
69-
* This method extracts the Commons Username from the artist HTML information
68+
* This method extracts the Commons Username from the artist HTML information.
69+
* When the HTML is in customized formatting, it may fail to parse and return null.
7070
* @param metadata
7171
* @return
7272
*/
7373
private fun getAuthor(metadata: ExtMetadata): String? {
74-
return try {
75-
val authorHtml = metadata.artist()
76-
val anchorStartTagTerminalChars = "\">"
77-
val anchorCloseTag = "</a>"
74+
val authorHtml = metadata.artist()
75+
val anchorStartTagTerminalString = "\">"
76+
val anchorCloseTag = "</a>"
7877

79-
return authorHtml.substring(
80-
authorHtml.indexOf(anchorStartTagTerminalChars) +
81-
anchorStartTagTerminalChars
82-
.length,
78+
return if (!authorHtml.contains("<") && !authorHtml.contains(">") ) {
79+
authorHtml.trim()
80+
} else if (!authorHtml.contains(anchorStartTagTerminalString) || !authorHtml.endsWith(anchorCloseTag)) {
81+
null
82+
} else {
83+
84+
val authorText = authorHtml.substring(
85+
authorHtml.indexOf(anchorStartTagTerminalString) +
86+
anchorStartTagTerminalString.length,
8387
authorHtml.indexOf(anchorCloseTag),
8488
)
85-
} catch (ex: java.lang.Exception) {
86-
""
89+
if (authorText.contains("<") || authorText.contains(">")) {
90+
null
91+
} else {
92+
authorText
93+
}
8794
}
8895
}
8996
}
@@ -92,6 +99,10 @@ private fun Entities.Entity.depictionIds() =
9299
this[WikidataProperties.DEPICTS]?.mapNotNull { (it.mainSnak.dataValue as? DataValue.EntityId)?.value?.id }
93100
?: emptyList()
94101

102+
private fun Entities.Entity.creatorIds() =
103+
this[WikidataProperties.CREATOR]?.mapNotNull { (it.mainSnak.dataValue as? DataValue.EntityId)?.value?.id }
104+
?: emptyList()
105+
95106
private val ExtMetadata.prefixedLicenseUrl: String
96107
get() =
97108
licenseUrl().let {

app/src/main/java/fr/free/nrw/commons/explore/media/PageableMediaFragment.kt

+6-1
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,18 @@ import android.content.Context
44
import android.os.Bundle
55
import android.view.View
66
import fr.free.nrw.commons.Media
7+
import fr.free.nrw.commons.MediaDataExtractor
78
import fr.free.nrw.commons.R
89
import fr.free.nrw.commons.category.CategoryImagesCallback
910
import fr.free.nrw.commons.explore.paging.BasePagingFragment
1011
import fr.free.nrw.commons.media.MediaDetailPagerFragment.MediaDetailProvider
12+
import javax.inject.Inject
1113

1214
abstract class PageableMediaFragment :
1315
BasePagingFragment<Media>(),
1416
MediaDetailProvider {
1517
override val pagedListAdapter by lazy {
16-
PagedMediaAdapter(categoryImagesCallback::onMediaClicked)
18+
PagedMediaAdapter(categoryImagesCallback::onMediaClicked, mediaDataExtractor)
1719
}
1820

1921
override val errorTextId: Int = R.string.error_loading_images
@@ -22,6 +24,9 @@ abstract class PageableMediaFragment :
2224

2325
lateinit var categoryImagesCallback: CategoryImagesCallback
2426

27+
@Inject
28+
lateinit var mediaDataExtractor: MediaDataExtractor
29+
2530
override fun onAttach(context: Context) {
2631
super.onAttach(context)
2732
if (parentFragment != null) {

0 commit comments

Comments
 (0)