Display specific, user-friendly error message when upload categories search API call returns an error (#6540)
Some checks are pending
Android CI / Run tests and generate APK (push) Waiting to run

* Make OkHttpConnectionFactory raise MwIOException when a non-suppressed API call returns an error

* Add AlertDialog displaying specific error message when categories search API call returns an error

* Add test for error alert dialog to UploadCategoriesFragment unit tests

* Add error handling when API call fails to CategoriesPresenter.onAttachViewWithMedia
This commit is contained in:
Ted 2025-10-26 01:24:39 +11:00 committed by GitHub
parent aae9d4a387
commit 28fa7b1a20
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 76 additions and 5 deletions

View file

@ -1,7 +1,11 @@
package fr.free.nrw.commons package fr.free.nrw.commons
import androidx.annotation.VisibleForTesting import androidx.annotation.VisibleForTesting
import fr.free.nrw.commons.wikidata.GsonUtil
import fr.free.nrw.commons.wikidata.cookies.CommonsCookieJar import fr.free.nrw.commons.wikidata.cookies.CommonsCookieJar
import fr.free.nrw.commons.wikidata.mwapi.MwErrorResponse
import fr.free.nrw.commons.wikidata.mwapi.MwIOException
import fr.free.nrw.commons.wikidata.mwapi.MwLegacyServiceError
import okhttp3.Cache import okhttp3.Cache
import okhttp3.Interceptor import okhttp3.Interceptor
import okhttp3.OkHttpClient import okhttp3.OkHttpClient
@ -86,16 +90,25 @@ private class UnsuccessfulResponseInterceptor : Interceptor {
rsp.peekBody(ERRORS_PREFIX.length.toLong()).use { responseBody -> rsp.peekBody(ERRORS_PREFIX.length.toLong()).use { responseBody ->
if (ERRORS_PREFIX == responseBody.string()) { if (ERRORS_PREFIX == responseBody.string()) {
rsp.body.use { body -> rsp.body.use { body ->
throw IOException(body!!.string()) val bodyString = body!!.string()
throw MwIOException(
"MediaWiki API returned error: $bodyString",
GsonUtil.defaultGson.fromJson(
bodyString,
MwErrorResponse::class.java
).error!!,
)
} }
} }
} }
} catch (e: IOException) { } catch (e: MwIOException) {
// Log the error as debug (and therefore, "expected") or at error level // Log the error as debug (and therefore, "expected") or at error level
if (suppressErrors) { if (suppressErrors) {
Timber.d(e, "Suppressed (known / expected) error") Timber.d(e, "Suppressed (known / expected) error")
} else { } else {
Timber.e(e) Timber.e(e)
throw e
} }
} }
return rsp return rsp

View file

@ -17,6 +17,11 @@ interface CategoriesContract {
fun showError(stringResourceId: Int) fun showError(stringResourceId: Int)
/**
* Show a cancelable AlertDialog with a given message.
*/
fun showErrorDialog(message: String)
fun setCategories(categories: List<CategoryItem>?) fun setCategories(categories: List<CategoryItem>?)
fun goToNextScreen() fun goToNextScreen()

View file

@ -12,6 +12,7 @@ import fr.free.nrw.commons.di.CommonsApplicationModule.Companion.IO_THREAD
import fr.free.nrw.commons.di.CommonsApplicationModule.Companion.MAIN_THREAD import fr.free.nrw.commons.di.CommonsApplicationModule.Companion.MAIN_THREAD
import fr.free.nrw.commons.repository.UploadRepository import fr.free.nrw.commons.repository.UploadRepository
import fr.free.nrw.commons.upload.depicts.proxy import fr.free.nrw.commons.upload.depicts.proxy
import fr.free.nrw.commons.wikidata.mwapi.MwIOException
import io.reactivex.Observable import io.reactivex.Observable
import io.reactivex.Scheduler import io.reactivex.Scheduler
import io.reactivex.android.schedulers.AndroidSchedulers import io.reactivex.android.schedulers.AndroidSchedulers
@ -75,7 +76,12 @@ class CategoriesPresenter
}, },
{ t: Throwable? -> { t: Throwable? ->
view.showProgress(false) view.showProgress(false)
view.showError(R.string.no_categories_found) view.showError(R.string.error_loading_categories)
val mwException = t as? MwIOException
view.showErrorDialog(
if (mwException == null) ""
else "\n${mwException.error.title} / ${mwException.error.details}"
)
Timber.e(t) Timber.e(t)
}, },
), ),
@ -194,7 +200,12 @@ class CategoriesPresenter
}, },
{ t: Throwable? -> { t: Throwable? ->
view.showProgress(false) view.showProgress(false)
view.showError(R.string.no_categories_found) view.showError(R.string.error_loading_categories)
val mwException = t as? MwIOException
view.showErrorDialog(
if (mwException == null) ""
else "\n${mwException.error.title} / ${mwException.error.details}"
)
Timber.e(t) Timber.e(t)
}, },
), ),

View file

@ -10,6 +10,7 @@ import android.view.LayoutInflater
import android.view.View import android.view.View
import android.view.ViewGroup import android.view.ViewGroup
import android.widget.Toast import android.widget.Toast
import androidx.appcompat.app.AlertDialog
import androidx.appcompat.app.AppCompatActivity import androidx.appcompat.app.AppCompatActivity
import androidx.recyclerview.widget.LinearLayoutManager import androidx.recyclerview.widget.LinearLayoutManager
import com.jakewharton.rxbinding2.view.RxView import com.jakewharton.rxbinding2.view.RxView
@ -32,7 +33,6 @@ import io.reactivex.Notification
import io.reactivex.android.schedulers.AndroidSchedulers import io.reactivex.android.schedulers.AndroidSchedulers
import io.reactivex.disposables.Disposable import io.reactivex.disposables.Disposable
import timber.log.Timber import timber.log.Timber
import java.util.Objects
import java.util.concurrent.TimeUnit import java.util.concurrent.TimeUnit
import javax.inject.Inject import javax.inject.Inject
@ -199,6 +199,15 @@ class UploadCategoriesFragment : UploadBaseFragment(), CategoriesContract.View {
binding?.tilContainerSearch?.error = getString(stringResourceId) binding?.tilContainerSearch?.error = getString(stringResourceId)
} }
override fun showErrorDialog(message: String) {
AlertDialog
.Builder(requireContext())
.setMessage(getString(R.string.error_loading_categories) + "\n" + message)
.setCancelable(false)
.setNegativeButton(R.string.ok){_,_ -> }
.show()
}
override fun setCategories(categories: List<CategoryItem>?) { override fun setCategories(categories: List<CategoryItem>?) {
if (adapter == null) { if (adapter == null) {
Timber.e("Adapter is null in setCategories") Timber.e("Adapter is null in setCategories")

View file

@ -0,0 +1,7 @@
package fr.free.nrw.commons.wikidata.mwapi
import fr.free.nrw.commons.wikidata.model.BaseModel
class MwErrorResponse : BaseModel() {
val error: MwLegacyServiceError? = null
}

View file

@ -0,0 +1,5 @@
package fr.free.nrw.commons.wikidata.mwapi
import java.io.IOException
class MwIOException(string: String, val error: MwLegacyServiceError) : IOException(string)

View file

@ -0,0 +1,14 @@
package fr.free.nrw.commons.wikidata.mwapi
import fr.free.nrw.commons.wikidata.model.BaseModel
class MwLegacyServiceError : BaseModel() {
val code: String? = null
private val info: String? = null
val title: String
get() = code ?: ""
val details: String
get() = info ?: ""
}

View file

@ -153,6 +153,13 @@ class UploadCategoriesFragmentUnitTests {
fragment.showError(R.string.no_categories_found) fragment.showError(R.string.no_categories_found)
} }
@Test
@Throws(Exception::class)
fun testShowErrorDialog() {
Shadows.shadowOf(Looper.getMainLooper()).idle()
fragment.showErrorDialog("")
}
@Test @Test
@Throws(Exception::class) @Throws(Exception::class)
fun testSetCategoriesCaseNull() { fun testSetCategoriesCaseNull() {