Skip to content

Commit f42ca8d

Browse files
committed
#3601 [structured-commons] Wrong thumbnails at the "depictions" step when uploading - minor cleanup
1 parent 39d0eee commit f42ca8d

File tree

4 files changed

+11
-20
lines changed

4 files changed

+11
-20
lines changed

app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsContract.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,7 @@ interface View {
4343
void setDepictsList(List<DepictedItem> depictedItemList);
4444

4545

46-
void updateUrlInAdapter(@NotNull DepictedItem depictedItem,
47-
@NotNull String response);
46+
void onUrlFetched(@NotNull DepictedItem depictedItem, @NotNull String url);
4847
}
4948

5049
interface UserActionListener extends BasePresenter<View> {

app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsFragment.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package fr.free.nrw.commons.upload.depicts;
22

33
import android.os.Bundle;
4-
import android.text.Editable;
54
import android.util.Pair;
65
import android.view.LayoutInflater;
76
import android.view.View;
@@ -146,10 +145,10 @@ public void setDepictsList(List<DepictedItem> depictedItemList) {
146145
}
147146

148147
@Override
149-
public void updateUrlInAdapter(@NotNull DepictedItem depictedItem, @NotNull String response) {
148+
public void onUrlFetched(@NotNull DepictedItem depictedItem, @NotNull String url) {
150149
final Pair<DepictedItem, Integer> itemAndPosition = returnItemAndPosition(depictedItem);
151150
if (itemAndPosition != null) {
152-
itemAndPosition.first.setImageUrl(response);
151+
itemAndPosition.first.setImageUrl(url);
153152
adapter.notifyItemChanged(itemAndPosition.second);
154153
}
155154
}

app/src/main/java/fr/free/nrw/commons/upload/depicts/DepictsPresenter.kt

+4-11
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ class DepictsPresenter @Inject constructor(
4040
private val depictedItems: MutableLiveData<List<DepictedItem>> = MutableLiveData()
4141
private val idsToImageUrls = mutableMapOf<String, String>()
4242

43-
4443
override fun onAttachView(view: DepictsContract.View) {
4544
this.view = view
4645
compositeDisposable.add(
@@ -68,9 +67,7 @@ class DepictsPresenter @Inject constructor(
6867
return Flowable.zip(
6968
searchResults(it),
7069
Flowable.just(it),
71-
BiFunction { results: List<DepictedItem>, term: String ->
72-
Pair(results, term)
73-
}
70+
BiFunction { results: List<DepictedItem>, term: String -> Pair(results, term) }
7471
)
7572
}
7673

@@ -84,9 +81,7 @@ class DepictsPresenter @Inject constructor(
8481

8582
private fun addImageUrlsFromCache(depictions: List<DepictedItem>) =
8683
depictions.map { item ->
87-
idsToImageUrls.getOrElse(item.id, { null })
88-
?.let { item.copy(imageUrl = it) }
89-
?: item
84+
idsToImageUrls[item.id]?.let { item.copy(imageUrl = it) } ?: item
9085
}
9186

9287
override fun onDetachView() {
@@ -120,9 +115,7 @@ class DepictsPresenter @Inject constructor(
120115
* from the depiction list
121116
*/
122117
override fun verifyDepictions() {
123-
val selectedDepictions =
124-
repository.selectedDepictions
125-
if (selectedDepictions.isNotEmpty()) {
118+
if (repository.selectedDepictions.isNotEmpty()) {
126119
view.goToNextScreen()
127120
} else {
128121
view.noDepictionSelected()
@@ -140,7 +133,7 @@ class DepictsPresenter @Inject constructor(
140133
.observeOn(mainThreadScheduler)
141134
.filter { it != NO_DEPICTED_IMAGE }
142135
.subscribe(
143-
{ view.updateUrlInAdapter(depictedItem, it) },
136+
{ view.onUrlFetched(depictedItem, it) },
144137
{ Timber.e(it) }
145138
)
146139
)

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

+4-4
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ class DepictsPresenterTest {
155155
whenever(depictsClient.getP18ForItem(depictedItem.id)).thenReturn(Single.just("url"))
156156
depictsPresenter.fetchThumbnailForEntityId(depictedItem)
157157
testScheduler.triggerActions()
158-
verify(view).updateUrlInAdapter(depictedItem, "url")
158+
verify(view).onUrlFetched(depictedItem, "url")
159159
}
160160

161161
@Test
@@ -165,7 +165,7 @@ class DepictsPresenterTest {
165165
.thenReturn(Single.just(NO_DEPICTED_IMAGE))
166166
depictsPresenter.fetchThumbnailForEntityId(depictedItem)
167167
testScheduler.triggerActions()
168-
verify(view, never()).updateUrlInAdapter(depictedItem, NO_DEPICTED_IMAGE)
168+
verify(view, never()).onUrlFetched(depictedItem, NO_DEPICTED_IMAGE)
169169
}
170170

171171
@Test
@@ -174,10 +174,10 @@ class DepictsPresenterTest {
174174
whenever(depictsClient.getP18ForItem(depictedItem.id)).thenReturn(Single.just("url"))
175175
depictsPresenter.fetchThumbnailForEntityId(depictedItem)
176176
testScheduler.triggerActions()
177-
verify(view).updateUrlInAdapter(depictedItem, "url")
177+
verify(view).onUrlFetched(depictedItem, "url")
178178
depictsPresenter.fetchThumbnailForEntityId(depictedItem)
179179
testScheduler.triggerActions()
180-
verify(view, times(2)).updateUrlInAdapter(depictedItem, "url")
180+
verify(view, times(2)).onUrlFetched(depictedItem, "url")
181181
}
182182
}
183183

0 commit comments

Comments
 (0)