From 28fa7b1a2023861a05a8d6b7452395f15b170f1a Mon Sep 17 00:00:00 2001 From: Ted <37825157+ted-gilbert@users.noreply.github.com> Date: Sun, 26 Oct 2025 01:24:39 +1100 Subject: [PATCH] Display specific, user-friendly error message when upload categories search API call returns an error (#6540) * 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 --- .../free/nrw/commons/OkHttpConnectionFactory.kt | 17 +++++++++++++++-- .../upload/categories/CategoriesContract.kt | 5 +++++ .../upload/categories/CategoriesPresenter.kt | 15 +++++++++++++-- .../categories/UploadCategoriesFragment.kt | 11 ++++++++++- .../commons/wikidata/mwapi/MwErrorResponse.kt | 7 +++++++ .../nrw/commons/wikidata/mwapi/MwIOException.kt | 5 +++++ .../wikidata/mwapi/MwLegacyServiceError.kt | 14 ++++++++++++++ .../UploadCategoriesFragmentUnitTests.kt | 7 +++++++ 8 files changed, 76 insertions(+), 5 deletions(-) create mode 100644 app/src/main/java/fr/free/nrw/commons/wikidata/mwapi/MwErrorResponse.kt create mode 100644 app/src/main/java/fr/free/nrw/commons/wikidata/mwapi/MwIOException.kt create mode 100644 app/src/main/java/fr/free/nrw/commons/wikidata/mwapi/MwLegacyServiceError.kt diff --git a/app/src/main/java/fr/free/nrw/commons/OkHttpConnectionFactory.kt b/app/src/main/java/fr/free/nrw/commons/OkHttpConnectionFactory.kt index 1c28d5fe4..c54c3aefb 100644 --- a/app/src/main/java/fr/free/nrw/commons/OkHttpConnectionFactory.kt +++ b/app/src/main/java/fr/free/nrw/commons/OkHttpConnectionFactory.kt @@ -1,7 +1,11 @@ package fr.free.nrw.commons 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.mwapi.MwErrorResponse +import fr.free.nrw.commons.wikidata.mwapi.MwIOException +import fr.free.nrw.commons.wikidata.mwapi.MwLegacyServiceError import okhttp3.Cache import okhttp3.Interceptor import okhttp3.OkHttpClient @@ -86,16 +90,25 @@ private class UnsuccessfulResponseInterceptor : Interceptor { rsp.peekBody(ERRORS_PREFIX.length.toLong()).use { responseBody -> if (ERRORS_PREFIX == responseBody.string()) { 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 if (suppressErrors) { Timber.d(e, "Suppressed (known / expected) error") } else { Timber.e(e) + throw e } } return rsp diff --git a/app/src/main/java/fr/free/nrw/commons/upload/categories/CategoriesContract.kt b/app/src/main/java/fr/free/nrw/commons/upload/categories/CategoriesContract.kt index 183c7cd93..29e5ba90b 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/categories/CategoriesContract.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/categories/CategoriesContract.kt @@ -17,6 +17,11 @@ interface CategoriesContract { fun showError(stringResourceId: Int) + /** + * Show a cancelable AlertDialog with a given message. + */ + fun showErrorDialog(message: String) + fun setCategories(categories: List?) fun goToNextScreen() diff --git a/app/src/main/java/fr/free/nrw/commons/upload/categories/CategoriesPresenter.kt b/app/src/main/java/fr/free/nrw/commons/upload/categories/CategoriesPresenter.kt index dbeeae6ff..a1a96f2ac 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/categories/CategoriesPresenter.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/categories/CategoriesPresenter.kt @@ -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.repository.UploadRepository import fr.free.nrw.commons.upload.depicts.proxy +import fr.free.nrw.commons.wikidata.mwapi.MwIOException import io.reactivex.Observable import io.reactivex.Scheduler import io.reactivex.android.schedulers.AndroidSchedulers @@ -75,7 +76,12 @@ class CategoriesPresenter }, { t: Throwable? -> 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) }, ), @@ -194,7 +200,12 @@ class CategoriesPresenter }, { t: Throwable? -> 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) }, ), diff --git a/app/src/main/java/fr/free/nrw/commons/upload/categories/UploadCategoriesFragment.kt b/app/src/main/java/fr/free/nrw/commons/upload/categories/UploadCategoriesFragment.kt index ee6af7bb6..ef4521431 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/categories/UploadCategoriesFragment.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/categories/UploadCategoriesFragment.kt @@ -10,6 +10,7 @@ import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import android.widget.Toast +import androidx.appcompat.app.AlertDialog import androidx.appcompat.app.AppCompatActivity import androidx.recyclerview.widget.LinearLayoutManager import com.jakewharton.rxbinding2.view.RxView @@ -32,7 +33,6 @@ import io.reactivex.Notification import io.reactivex.android.schedulers.AndroidSchedulers import io.reactivex.disposables.Disposable import timber.log.Timber -import java.util.Objects import java.util.concurrent.TimeUnit import javax.inject.Inject @@ -199,6 +199,15 @@ class UploadCategoriesFragment : UploadBaseFragment(), CategoriesContract.View { 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?) { if (adapter == null) { Timber.e("Adapter is null in setCategories") diff --git a/app/src/main/java/fr/free/nrw/commons/wikidata/mwapi/MwErrorResponse.kt b/app/src/main/java/fr/free/nrw/commons/wikidata/mwapi/MwErrorResponse.kt new file mode 100644 index 000000000..40f5afe68 --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/wikidata/mwapi/MwErrorResponse.kt @@ -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 +} \ No newline at end of file diff --git a/app/src/main/java/fr/free/nrw/commons/wikidata/mwapi/MwIOException.kt b/app/src/main/java/fr/free/nrw/commons/wikidata/mwapi/MwIOException.kt new file mode 100644 index 000000000..62ca87166 --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/wikidata/mwapi/MwIOException.kt @@ -0,0 +1,5 @@ +package fr.free.nrw.commons.wikidata.mwapi + +import java.io.IOException + +class MwIOException(string: String, val error: MwLegacyServiceError) : IOException(string) diff --git a/app/src/main/java/fr/free/nrw/commons/wikidata/mwapi/MwLegacyServiceError.kt b/app/src/main/java/fr/free/nrw/commons/wikidata/mwapi/MwLegacyServiceError.kt new file mode 100644 index 000000000..16ee29709 --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/wikidata/mwapi/MwLegacyServiceError.kt @@ -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 ?: "" +} \ No newline at end of file diff --git a/app/src/test/kotlin/fr/free/nrw/commons/upload/categories/UploadCategoriesFragmentUnitTests.kt b/app/src/test/kotlin/fr/free/nrw/commons/upload/categories/UploadCategoriesFragmentUnitTests.kt index 75d6b8a4f..55b8427e6 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/upload/categories/UploadCategoriesFragmentUnitTests.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/upload/categories/UploadCategoriesFragmentUnitTests.kt @@ -153,6 +153,13 @@ class UploadCategoriesFragmentUnitTests { fragment.showError(R.string.no_categories_found) } + @Test + @Throws(Exception::class) + fun testShowErrorDialog() { + Shadows.shadowOf(Looper.getMainLooper()).idle() + fragment.showErrorDialog("") + } + @Test @Throws(Exception::class) fun testSetCategoriesCaseNull() {