Fix infinite loading circular progress bar after nominating for deletion (#6324)

* fix: infinite loading progress bar after nominating for deletion

* add logs for testing

* refactor: use globalFileUsage instead of achievement to append in reason

Fetching achievements is a time consuming operation and globalFileUsage gives the similar result in optimal time

* test(ReasonBuilder): fix tests according to new behavior

* refactor: remove logs added for testing

* test: await for async getReason method call

---------

Co-authored-by: Neel Doshi <neeldoshi147@gmail.com>
Co-authored-by: Nicolas Raoul <nicolas.raoul@gmail.com>

app/build.gradle.kts

@@ -347,6 +347,7 @@ dependencies {
     // Kotlin + coroutines
     implementation(libs.androidx.work.runtime.ktx)
     implementation(libs.androidx.work.runtime)
+    implementation(libs.kotlinx.coroutines.rx2)
     testImplementation(libs.androidx.work.testing)
 
     //Glide

app/src/main/java/fr/free/nrw/commons/delete/DeleteHelper.kt

@@ -53,7 +53,6 @@ class DeleteHelper @Inject constructor(
         media: Media?,
         reason: String?
     ): Single<Boolean>? {
-
         if(context == null && media == null) {
             return null
         }
@@ -86,7 +85,6 @@ class DeleteHelper @Inject constructor(
      * @return
      */
     private fun delete(media: Media, reason: String): Observable<Boolean> {
-        Timber.d("thread is delete %s", Thread.currentThread().name)
         val summary = "Nominating ${media.filename} for deletion."
         val calendar = Calendar.getInstance()
         val fileDeleteString = """

app/src/main/java/fr/free/nrw/commons/delete/ReasonBuilder.kt

@@ -2,21 +2,19 @@ package fr.free.nrw.commons.delete
 
 import android.annotation.SuppressLint
 import android.content.Context
-
-import fr.free.nrw.commons.utils.DateUtil
-import java.util.Locale
-
-import javax.inject.Inject
-import javax.inject.Singleton
-
 import fr.free.nrw.commons.Media
 import fr.free.nrw.commons.R
-import fr.free.nrw.commons.profile.achievements.FeedbackResponse
 import fr.free.nrw.commons.auth.SessionManager
 import fr.free.nrw.commons.mwapi.OkHttpJsonApiClient
+import fr.free.nrw.commons.utils.DateUtil
 import fr.free.nrw.commons.utils.ViewUtilWrapper
 import io.reactivex.Single
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.rx2.rxSingle
 import timber.log.Timber
+import java.util.Locale
+import javax.inject.Inject
+import javax.inject.Singleton
 
 /**
  * This class handles the reason for deleting a Media object
@@ -29,6 +27,8 @@ class ReasonBuilder @Inject constructor(
     private val viewUtilWrapper: ViewUtilWrapper
 ) {
 
+    private val defaultFileUsagePageSize = 10
+
     /**
      * To process the reason and append the media's upload date and uploaded_by_me string
      * @param media
@@ -39,7 +39,7 @@ class ReasonBuilder @Inject constructor(
         if (media == null || reason == null) {
             return Single.just("Not known")
         }
-        return fetchArticleNumber(media, reason)
+        return getAndAppendFileUsage(media, reason)
     }
 
     /**
@@ -54,27 +54,36 @@ class ReasonBuilder @Inject constructor(
         }
     }
 
-    private fun fetchArticleNumber(media: Media, reason: String): Single<String> {
+    private fun getAndAppendFileUsage(media: Media, reason: String): Single<String> {
-        return if (checkAccount()) {
+        return rxSingle(context = Dispatchers.IO) {
-            okHttpJsonApiClient
+            if (!checkAccount()) return@rxSingle ""
-                .getAchievements(sessionManager.userName)
+
-                .map { feedbackResponse -> appendArticlesUsed(feedbackResponse, media, reason) }
+            try {
-        } else {
+                val globalFileUsage = okHttpJsonApiClient.getGlobalFileUsages(
-            Single.just("")
+                    fileName = media.filename,
+                    pageSize = defaultFileUsagePageSize
+                )
+                val globalUsages = globalFileUsage?.query?.pages?.sumOf { it.fileUsage.size } ?: 0
+
+                appendArticlesUsed(globalUsages, media, reason)
+            } catch (e: Exception) {
+                Timber.e(e, "Error fetching file usage")
+                throw e
+            }
         }
     }
 
     /**
-     * Takes the uploaded_by_me string, the upload date, name of articles using images
+     * Takes the uploaded_by_me string, the upload date, no. of articles using images
      * and appends it to the received reason
-     * @param feedBack object
+     * @param fileUsages No. of files/articles using this image
      * @param media whose upload data is to be fetched
-     * @param reason
+     * @param reason string to be appended
      */
     @SuppressLint("StringFormatInvalid")
-    private fun appendArticlesUsed(feedBack: FeedbackResponse, media: Media, reason: String): String {
+    private fun appendArticlesUsed(fileUsages: Int, media: Media, reason: String): String {
         val reason1Template = context.getString(R.string.uploaded_by_myself)
-        return reason + String.format(Locale.getDefault(), reason1Template, prettyUploadedDate(media), feedBack.articlesUsingImages)
+        return reason + String.format(Locale.getDefault(), reason1Template, prettyUploadedDate(media), fileUsages)
             .also { Timber.i("New Reason %s", it) }
     }
 

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

@@ -74,7 +74,6 @@ import fr.free.nrw.commons.BuildConfig
 import fr.free.nrw.commons.CameraPosition
 import fr.free.nrw.commons.CommonsApplication
 import fr.free.nrw.commons.CommonsApplication.Companion.instance
-import fr.free.nrw.commons.locationpicker.LocationPicker
 import fr.free.nrw.commons.Media
 import fr.free.nrw.commons.MediaDataExtractor
 import fr.free.nrw.commons.R
@@ -102,6 +101,7 @@ import fr.free.nrw.commons.explore.depictions.WikidataItemDetailsActivity
 import fr.free.nrw.commons.kvstore.JsonKvStore
 import fr.free.nrw.commons.language.AppLanguageLookUpTable
 import fr.free.nrw.commons.location.LocationServiceManager
+import fr.free.nrw.commons.locationpicker.LocationPicker
 import fr.free.nrw.commons.media.MediaDetailPagerFragment.MediaDetailProvider
 import fr.free.nrw.commons.profile.ProfileActivity
 import fr.free.nrw.commons.review.ReviewHelper
@@ -116,6 +116,7 @@ import fr.free.nrw.commons.utils.LangCodeUtils.getLocalizedResources
 import fr.free.nrw.commons.utils.PermissionUtils.PERMISSIONS_STORAGE
 import fr.free.nrw.commons.utils.PermissionUtils.checkPermissionsAndPerformAction
 import fr.free.nrw.commons.utils.PermissionUtils.hasPermission
+import fr.free.nrw.commons.utils.ViewUtil
 import fr.free.nrw.commons.utils.ViewUtil.showShortToast
 import fr.free.nrw.commons.utils.ViewUtilWrapper
 import fr.free.nrw.commons.wikidata.mwapi.MwQueryPage.Revision
@@ -125,6 +126,7 @@ import io.reactivex.android.schedulers.AndroidSchedulers
 import io.reactivex.schedulers.Schedulers
 import org.apache.commons.lang3.StringUtils
 import timber.log.Timber
+import java.lang.String.format
 import java.util.Date
 import java.util.Locale
 import java.util.Objects
@@ -1646,7 +1648,7 @@ class MediaDetailFragment : CommonsDaggerSupportFragment(), CategoryEditHelper.C
                 getString(R.string.cancel),
                 {
                     val reason: String = input.text.toString()
-                    onDeleteClickeddialogtext(reason)
+                    onDeleteClickedDialogText(reason)
                 },
                 {},
                 input
@@ -1700,26 +1702,48 @@ class MediaDetailFragment : CommonsDaggerSupportFragment(), CategoryEditHelper.C
         resultSingle
             .subscribeOn(Schedulers.io())
             .observeOn(AndroidSchedulers.mainThread())
-            .subscribe { _ ->
+            .subscribe(this::handleDeletionResult, this::handleDeletionError);
-                if (applicationKvStore.getBoolean(
+    }
-                        String.format(
+
-                            NOMINATING_FOR_DELETION_MEDIA, media!!.imageUrl
+    /**
-                        ), false
+     * Disables Progress Bar and Update delete button text.
-                    )
+     */
-                ) {
+    private fun disableProgressBar() {
-                    applicationKvStore.remove(
+        activity?.run {
-                        String.format(
+            runOnUiThread(Runnable {
-                            NOMINATING_FOR_DELETION_MEDIA,
+                binding.progressBarDeletion.visibility = View.GONE
-                            media!!.imageUrl
+            })
-                        )
+        } ?: return // Prevent NullPointerException when fragment is not attached to activity
-                    )
+    }
-                    callback!!.nominatingForDeletion(index)
+
-                }
+    private fun handleDeletionResult(success: Boolean) {
-            }
+        if (success) {
+            binding.nominateDeletion.text = getString(R.string.nominated_for_deletion_btn)
+            ViewUtil.showLongSnackbar(requireView(), getString(R.string.nominated_for_deletion))
+            disableProgressBar()
+            checkAndClearDeletionFlag()
+        } else {
+            disableProgressBar()
+        }
+    }
+
+    private fun handleDeletionError(throwable: Throwable) {
+        throwable.printStackTrace()
+        disableProgressBar()
+        checkAndClearDeletionFlag()
+    }
+
+    private fun checkAndClearDeletionFlag() {
+        if (applicationKvStore
+            .getBoolean(format(NOMINATING_FOR_DELETION_MEDIA, media!!.imageUrl), false)
+        ) {
+            applicationKvStore.remove(format(NOMINATING_FOR_DELETION_MEDIA, media!!.imageUrl))
+            callback!!.nominatingForDeletion(index)
+        }
     }
 
     @SuppressLint("CheckResult")
-    private fun onDeleteClickeddialogtext(reason: String) {
+    private fun onDeleteClickedDialogText(reason: String) {
         applicationKvStore.putBoolean(
             String.format(
                 NOMINATING_FOR_DELETION_MEDIA,
@@ -1736,22 +1760,7 @@ class MediaDetailFragment : CommonsDaggerSupportFragment(), CategoryEditHelper.C
         resultSingletext
             .subscribeOn(Schedulers.io())
             .observeOn(AndroidSchedulers.mainThread())
-            .subscribe { _ ->
+            .subscribe(this::handleDeletionResult, this::handleDeletionError);
-                if (applicationKvStore.getBoolean(
-                        String.format(
-                            NOMINATING_FOR_DELETION_MEDIA, media!!.imageUrl
-                        ), false
-                    )
-                ) {
-                    applicationKvStore.remove(
-                        String.format(
-                            NOMINATING_FOR_DELETION_MEDIA,
-                            media!!.imageUrl
-                        )
-                    )
-                    callback!!.nominatingForDeletion(index)
-                }
-            }
     }
 
     private fun onSeeMoreClicked() {

app/src/main/java/fr/free/nrw/commons/mwapi/OkHttpJsonApiClient.kt

@@ -281,6 +281,7 @@ class OkHttpJsonApiClient @Inject constructor(
                         FeedbackResponse::class.java
                     )
                 } catch (e: Exception) {
+                    e.printStackTrace()
                     return@fromCallable FeedbackResponse(0, 0, 0, FeaturedImages(0, 0), 0, "")
                 }
             }

app/src/main/res/values/strings.xml

@@ -431,7 +431,7 @@
   <string name="deletion_reason_no_longer_want_public">I changed my mind, I don\'t want it to be publicly visible anymore</string>
   <string name="deletion_reason_not_interesting">Sorry this picture is not interesting for an encyclopedia</string>
 
-  <string name="uploaded_by_myself">Uploaded by myself on %1$s, used in %2$d article(s).</string>
+  <string name="uploaded_by_myself">Uploaded by myself on %1$s, used in %2$d article(s) at least.</string>
 
   <string name="no_uploads">Welcome to Commons!\n
 Upload your first media by tapping on the add button.</string>
@@ -876,4 +876,5 @@ Upload your first media by tapping on the add button.</string>
   <string name="show_in_nearby">Show in Nearby</string>
   <string name="image_tag_line_created_and_uploaded_by">Created and uploaded by: %1$s</string>
   <string name="image_tag_line_created_by_and_uploaded_by">Created by %1$s and uploaded by %2$s</string>
+  <string name="nominated_for_deletion_btn">Nominated for Deletion</string>
 </resources>

app/src/test/kotlin/fr/free/nrw/commons/delete/ReasonBuilderTest.kt

@@ -5,13 +5,14 @@ import android.content.res.Resources
 import fr.free.nrw.commons.Media
 import fr.free.nrw.commons.R
 import fr.free.nrw.commons.auth.SessionManager
+import fr.free.nrw.commons.fileusages.GlobalFileUsagesResponse
 import fr.free.nrw.commons.mwapi.OkHttpJsonApiClient
-import fr.free.nrw.commons.profile.achievements.FeedbackResponse
 import fr.free.nrw.commons.profile.leaderboard.LeaderboardResponse
 import fr.free.nrw.commons.profile.leaderboard.UpdateAvatarResponse
 import fr.free.nrw.commons.utils.ViewUtilWrapper
 import io.reactivex.Observable
 import io.reactivex.Single
+import kotlinx.coroutines.test.runTest
 import media
 import org.junit.Before
 import org.junit.Test
@@ -58,16 +59,16 @@ class ReasonBuilderTest {
         PowerMockito.`when`(context?.getString(R.string.user_not_logged_in))
             .thenReturn("Log-in expired. Please log in again.")
 
-        reasonBuilder!!.getReason(mock(Media::class.java), "test")
+        reasonBuilder!!.getReason(mock(Media::class.java), "test").test().await()
         verify(sessionManager, times(1))!!.forceLogin(any(Context::class.java))
     }
 
     @Test
-    fun getReason() {
+    fun getReason()  = runTest {
         `when`(sessionManager?.userName).thenReturn("Testuser")
         `when`(sessionManager?.doesAccountExist()).thenReturn(true)
-        `when`(okHttpJsonApiClient!!.getAchievements(anyString()))
+        `when`(okHttpJsonApiClient!!.getGlobalFileUsages(anyString(), anyInt()))
-            .thenReturn(Single.just(mock(FeedbackResponse::class.java)))
+            .thenReturn(mock(GlobalFileUsagesResponse::class.java))
         `when`(okHttpJsonApiClient!!.getLeaderboard(anyString(), anyString(), anyString(), anyString(), anyString()))
             .thenReturn(Observable.just(mock(LeaderboardResponse::class.java)))
         `when`(okHttpJsonApiClient!!.setAvatar(anyString(), anyString()))
@@ -75,8 +76,8 @@ class ReasonBuilderTest {
 
         val media = media(filename = "test_file", dateUploaded = Date())
 
-        reasonBuilder!!.getReason(media, "test")
+        reasonBuilder!!.getReason(media, "test").test().await()
         verify(sessionManager, times(0))!!.forceLogin(any(Context::class.java))
-        verify(okHttpJsonApiClient, times(1))!!.getAchievements(anyString())
+        verify(okHttpJsonApiClient, times(1))!!.getGlobalFileUsages(anyString(), anyInt())
     }
 }

gradle/libs.versions.toml

@@ -16,6 +16,7 @@ constraintlayout = "1.1.3"
 coordinates2country = "1.8"
 dexcount = "4.0.0"
 githubTripletPlay = "2.7.2"
+kotlinxCoroutinesRx2 = "1.8.0"
 osmdroidAndroid = "6.1.17"
 testCore = "1.4.0"
 coreKtx = "1.9.0"
@@ -127,6 +128,7 @@ dagger-compiler = { module = "com.google.dagger:dagger-compiler", version.ref =
 facebook-fresco = { module = "com.facebook.fresco:fresco", version.ref = "frescoVersion" }
 glide-compiler = { module = "com.github.bumptech.glide:compiler", version.ref = "glide" }
 glide = { module = "com.github.bumptech.glide:glide", version.ref = "glide" }
+kotlinx-coroutines-rx2 = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-rx2", version.ref = "kotlinxCoroutinesRx2" }
 photoview = { module = "com.github.chrisbanes:PhotoView", version.ref = "photoviewVersion" }
 
 # RxJava and Reactive Programming