Fixes commons-app#3694 Pre-select places as depictions (commons-app#4452)

* WikidataEditService: stop automatically adding WikidataPlace as a depiction

When the user initiates the upload process from Nearby and also manually adds the place as a depiction, the depiction is added twice. Since this behavior is invisible to the user, it is being removed in preparation for auto-selecting the place as a depiction on the DepictsFragment screen.

* DepictsFragment: auto-select place as a depiction

Pass the Place reference from UploadActivity to DepictsFragment and select the corresponding DepictedItem. Using the place id, retrieve the corresponding Entity to create and select a DepictedItem.

* UploadRepository: use Place from UploadItem to obtain a DepictedItem

Instead of passing a Place object from UploadActivity to DepictsFragment and then passing the Place object up the chain to obtain and select a DepictedItem, retrieve the Place object directly within UploadRepository

* DepictsFragment: select Place depiction when fragment becomes visible

* UploadDepictsAdapter: make adapter aware of selection state

Update selection state when recycled list items are automatically selected, preventing automatically selected items from appearing as unselected until they are forced to re-bind (i.e. after scrolling)

* DepictsFragment: pre-select place depictions for all UploadItems

If several images are selected and set to different places, pre-select all place depictions to reinforce the intended upload workflow philosophy (i.e. all images in a set are intended to be from/of the same place). See discussion in commons-app#3694

* DepictsFragment: scroll to the top every time list is updated

Author: byerlyb20

app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java

@@ -19,8 +19,11 @@
 import io.reactivex.Observable;
 import io.reactivex.Single;
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
+import java.util.Set;
 import javax.inject.Inject;
 import javax.inject.Singleton;
 
@@ -254,6 +257,23 @@ public Flowable<List<DepictedItem>> searchAllEntities(String query) {
         return depictModel.searchAllEntities(query);
     }
 
+    /**
+     * Gets the depiction for each unique {@link Place} associated with an {@link UploadItem}
+     * from {@link #getUploads()}
+     *
+     * @return a single that provides the depictions
+     */
+    public Single<List<DepictedItem>> getPlaceDepictions() {
+        final Set<Place> places = new HashSet<>();
+        for (final UploadItem item : getUploads()) {
+            final Place place = item.getPlace();
+            if (place != null) {
+                places.add(place);
+            }
+        }
+        return depictModel.getPlaceDepictions(new ArrayList<>(places));
+    }
+
     /**
      * Returns nearest place matching the passed latitude and longitude
      * @param decLatitude

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

@@ -60,6 +60,11 @@ interface UserActionListener extends BasePresenter<View> {
          */
         void searchForDepictions(String query);
 
+        /**
+         * Selects all associated places (if any) as depictions
+         */
+        void selectPlaceDepictions();
+
         /**
          * Check if depictions were selected
          * from the depiction list

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

@@ -100,6 +100,14 @@ private void initRecyclerView() {
         depictsRecyclerView.setAdapter(adapter);
     }
 
+    @Override
+    protected void onBecameVisible() {
+        super.onBecameVisible();
+        // Select Place depiction as the fragment becomes visible to ensure that the most up to date
+        // Place is used (i.e. if the user accepts a nearby place dialog)
+        presenter.selectPlaceDepictions();
+    }
+
     @Override
     public void goToNextScreen() {
         callback.onNextButtonClicked(callback.getIndexInViewFlipper(this));
@@ -146,6 +154,7 @@ public void showError(Boolean value) {
     @Override
     public void setDepictsList(List<DepictedItem> depictedItemList) {
         adapter.setItems(depictedItemList);
+        depictsRecyclerView.smoothScrollToPosition(0);
     }
 
     @OnClick(R.id.depicts_next)

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

@@ -84,6 +84,35 @@ class DepictsPresenter @Inject constructor(
         compositeDisposable.clear()
     }
 
+    /**
+     * Selects the place depictions retrieved by the repository
+     */
+    override fun selectPlaceDepictions() {
+        compositeDisposable.add(repository.placeDepictions
+            .subscribeOn(ioScheduler)
+            .observeOn(mainThreadScheduler)
+            .subscribe(::selectNewDepictions)
+        )
+    }
+
+    /**
+     * Selects each [DepictedItem] in a given list as if they were clicked by the user by calling
+     * [onDepictItemClicked] for each depiction and adding the depictions to [depictedItems]
+     */
+    private fun selectNewDepictions(toSelect: List<DepictedItem>) {
+        toSelect.forEach {
+            it.isSelected = true
+            repository.onDepictItemClicked(it)
+        }
+
+        // Add the new selections to the list of depicted items so that the selections appear
+        // immediately (i.e. without any search term queries)
+        depictedItems.value?.toMutableList()
+            ?.let { toSelect + it }
+            ?.distinctBy(DepictedItem::id)
+            ?.let { depictedItems.value = it }
+    }
+
     override fun onPreviousButtonClicked() {
         view.goToPreviousScreen()
     }

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

@@ -6,5 +6,6 @@ import fr.free.nrw.commons.upload.structure.depictions.DepictedItem
 class UploadDepictsAdapter(onDepictsClicked: (DepictedItem) -> Unit) :
     BaseDelegateAdapter<DepictedItem>(
         uploadDepictsDelegate(onDepictsClicked),
-        areItemsTheSame = { oldItem, newItem -> oldItem.id == newItem.id }
+        areItemsTheSame = { oldItem, newItem -> oldItem.id == newItem.id },
+        areContentsTheSame = { itemA, itemB -> itemA.isSelected == itemB.isSelected}
     )

app/src/main/java/fr/free/nrw/commons/upload/structure/depictions/DepictModel.kt

@@ -3,6 +3,7 @@ package fr.free.nrw.commons.upload.structure.depictions
 import fr.free.nrw.commons.explore.depictions.DepictsClient
 import fr.free.nrw.commons.nearby.Place
 import io.reactivex.Flowable
+import io.reactivex.Observable
 import io.reactivex.Single
 import io.reactivex.processors.BehaviorProcessor
 import timber.log.Timber
@@ -27,19 +28,29 @@ class DepictModel @Inject constructor(private val depictsClient: DepictsClient)
     fun searchAllEntities(query: String): Flowable<List<DepictedItem>> {
         return if (query.isBlank())
             nearbyPlaces.switchMap { places: List<Place> ->
-                depictsClient.getEntities(places.toIds())
-                    .map {
-                        it.entities()
-                            .values
-                            .mapIndexed { index, entity -> DepictedItem(entity, places[index]) }
-                    }
-                    .onErrorResumeWithEmptyList()
-                    .toFlowable()
+                getPlaceDepictions(places).toFlowable()
             }
         else
             networkItems(query)
     }
 
+    /**
+     * Provides [DepictedItem] instances via a [Single] for a given list of [Place], providing an
+     * empty list if no places are provided or if there is an error
+     */
+    fun getPlaceDepictions(places: List<Place>): Single<List<DepictedItem>> =
+        places.toIds().let { ids ->
+            if (ids.isNotEmpty())
+                depictsClient.getEntities(ids)
+                    .map{
+                        it.entities()
+                            .values
+                            .mapIndexed { index, entity ->  DepictedItem(entity, places[index])}
+                    }
+                    .onErrorResumeWithEmptyList()
+            else Single.just(emptyList())
+        }
+
     private fun networkItems(query: String): Flowable<List<DepictedItem>> {
         return depictsClient.searchForDepictions(query, SEARCH_DEPICTS_LIMIT, 0)
             .onErrorResumeWithEmptyList()

app/src/main/java/fr/free/nrw/commons/wikidata/WikidataEditService.java

@@ -206,12 +206,7 @@ private Observable<Boolean> captionEdits(Contribution contribution, Long fileEnt
   }
 
   private Observable<Boolean> depictionEdits(Contribution contribution, Long fileEntityId) {
-    final ArrayList<WikidataItem> depictedItems = new ArrayList<>(contribution.getDepictedItems());
-    final WikidataPlace wikidataPlace = contribution.getWikidataPlace();
-    if (wikidataPlace != null) {
-      depictedItems.add(wikidataPlace);
-    }
-    return Observable.fromIterable(depictedItems)
+    return Observable.fromIterable(contribution.getDepictedItems())
         .concatMap(wikidataItem -> addDepictsProperty(fileEntityId.toString(), wikidataItem));
   }
 }