From c626f97fb95621dd0846d8c5fb06c5bd8fac7bdd Mon Sep 17 00:00:00 2001 From: Arin Modi Date: Mon, 7 Mar 2022 14:22:04 +0530 Subject: [PATCH] Fixed - #4844 : In peer review, deletion proposal is sent even if no reason is chosen (#4862) * Fixed - #4844 : [Bug]: In peer review, deletion proposal is sent even if no reason is chosen * Minor Changes * Minor Fixes * added required changes * added required changes - 1 * Added Test For OK Button * Minor Fixes --- .../free/nrw/commons/delete/DeleteHelper.java | 51 ++++++++++++++----- .../nrw/commons/delete/DeleteHelperTest.kt | 17 +++++++ 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/delete/DeleteHelper.java b/app/src/main/java/fr/free/nrw/commons/delete/DeleteHelper.java index 1c1fa925b..d21b82860 100644 --- a/app/src/main/java/fr/free/nrw/commons/delete/DeleteHelper.java +++ b/app/src/main/java/fr/free/nrw/commons/delete/DeleteHelper.java @@ -5,6 +5,7 @@ import static fr.free.nrw.commons.notification.NotificationHelper.NOTIFICATION_D import android.annotation.SuppressLint; import android.content.Context; import static fr.free.nrw.commons.utils.LangCodeUtils.getLocalizedResources; +import android.content.DialogInterface; import android.content.Intent; import android.net.Uri; import androidx.appcompat.app.AlertDialog; @@ -39,6 +40,8 @@ public class DeleteHelper { private final PageEditClient pageEditClient; private final ViewUtilWrapper viewUtil; private final String username; + private AlertDialog d; + private DialogInterface.OnMultiChoiceClickListener listener; @Inject public DeleteHelper(NotificationHelper notificationHelper, @@ -176,12 +179,17 @@ public class DeleteHelper { reasonListEnglish[2] = getLocalizedResources(context, Locale.ENGLISH).getString(R.string.delete_helper_ask_reason_copyright_logo); } - alert.setMultiChoiceItems(reasonList, checkedItems, (dialogInterface, position, isChecked) -> { + alert.setMultiChoiceItems(reasonList, checkedItems, listener = (dialogInterface, position, isChecked) -> { + if (isChecked) { mUserReason.add(position); } else { mUserReason.remove((Integer.valueOf(position))); } + + // disable the OK button if no reason selected + ((AlertDialog) dialogInterface).getButton(AlertDialog.BUTTON_POSITIVE).setEnabled( + !mUserReason.isEmpty()); }); alert.setPositiveButton(context.getString(R.string.ok), (dialogInterface, i) -> { @@ -200,20 +208,39 @@ public class DeleteHelper { String finalReason = reason; Single.defer((Callable>) () -> - makeDeletion(context, media, finalReason)) - .subscribeOn(Schedulers.io()) - .observeOn(AndroidSchedulers.mainThread()) - .subscribe(aBoolean -> { - if (aBoolean) { - reviewCallback.onSuccess(); - } else { - reviewCallback.onFailure(); - } - }); + makeDeletion(context, media, finalReason)) + .subscribeOn(Schedulers.io()) + .observeOn(AndroidSchedulers.mainThread()) + .subscribe(aBoolean -> { + if (aBoolean) { + reviewCallback.onSuccess(); + } else { + reviewCallback.onFailure(); + } + }); }); alert.setNegativeButton(context.getString(R.string.cancel), (dialog, which) -> reviewCallback.onFailure()); - AlertDialog d = alert.create(); + d = alert.create(); d.show(); + + // disable the OK button by default + d.getButton(AlertDialog.BUTTON_POSITIVE).setEnabled(false); + } + + /** + * returns the instance of shown AlertDialog, + * used for taking reference during unit test + * */ + public AlertDialog getDialog(){ + return d; + } + + /** + * returns the instance of shown DialogInterface.OnMultiChoiceClickListener, + * used for taking reference during unit test + * */ + public DialogInterface.OnMultiChoiceClickListener getListener(){ + return listener; } } diff --git a/app/src/test/kotlin/fr/free/nrw/commons/delete/DeleteHelperTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/delete/DeleteHelperTest.kt index 6bfe6ad47..90af5e1bc 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/delete/DeleteHelperTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/delete/DeleteHelperTest.kt @@ -1,5 +1,6 @@ package fr.free.nrw.commons.delete +import android.app.AlertDialog import android.content.Context import com.nhaarman.mockitokotlin2.eq import com.nhaarman.mockitokotlin2.mock @@ -15,6 +16,7 @@ import io.reactivex.Observable import io.reactivex.Single import org.junit.Assert.assertNotNull import org.junit.Assert.assertTrue +import org.junit.Assert.assertEquals import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -142,6 +144,21 @@ class DeleteHelperTest { deleteHelper.askReasonAndExecute(media, mContext, "My Question", ReviewController.DeleteReason.COPYRIGHT_VIOLATION, callback); } + @Test + fun alertDialogPositiveButtonDisableTest() { + val mContext = RuntimeEnvironment.getApplication().applicationContext + deleteHelper.askReasonAndExecute(media, mContext, "My Question", ReviewController.DeleteReason.COPYRIGHT_VIOLATION, callback); + assertEquals(false, deleteHelper.dialog.getButton(AlertDialog.BUTTON_POSITIVE).isEnabled) + } + + @Test + fun alertDialogPositiveButtonEnableTest() { + val mContext = RuntimeEnvironment.getApplication().applicationContext + deleteHelper.askReasonAndExecute(media, mContext, "My Question", ReviewController.DeleteReason.COPYRIGHT_VIOLATION, callback); + deleteHelper.listener.onClick(deleteHelper.dialog,1,true); + assertEquals(true, deleteHelper.dialog.getButton(AlertDialog.BUTTON_POSITIVE).isEnabled) + } + @Test(expected = RuntimeException::class) fun makeDeletionForEmptyCreatorName() { whenever(pageEditClient.prependEdit(ArgumentMatchers.anyString(), ArgumentMatchers.anyString(), ArgumentMatchers.anyString()))