From fa679ce91c363ed975491a153338906c9da44bfc Mon Sep 17 00:00:00 2001 From: anishamahuli Date: Tue, 3 Mar 2026 15:03:16 -0500 Subject: [PATCH 1/9] Add string resources for new components --- app/src/main/res/values/strings.xml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 950ffd0754..3166b57728 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -568,6 +568,8 @@ Upload your first media by tapping on the add button. a selfie which is not used in any article totally blurry nonsense, absolutely unusable in any article + Other: specify below + Other notes Press photo Random photo from internet Logo From bb080b20740ba0f60e688604f8e03717cb0c49e0 Mon Sep 17 00:00:00 2001 From: anishamahuli Date: Tue, 3 Mar 2026 15:23:51 -0500 Subject: [PATCH 2/9] Create custom dialog layout in dialog_deletion_reason.xml. --- .../res/layout/dialog_deletion_reason.xml | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 app/src/main/res/layout/dialog_deletion_reason.xml diff --git a/app/src/main/res/layout/dialog_deletion_reason.xml b/app/src/main/res/layout/dialog_deletion_reason.xml new file mode 100644 index 0000000000..b5ba15dddc --- /dev/null +++ b/app/src/main/res/layout/dialog_deletion_reason.xml @@ -0,0 +1,41 @@ + + + + + + + + + + + + + + + \ No newline at end of file From 1280b41a9fff685fa551e55e0c1544e743f87245 Mon Sep 17 00:00:00 2001 From: anishamahuli Date: Tue, 3 Mar 2026 16:21:50 -0500 Subject: [PATCH 3/9] Replace setMultiChoiceItems with custom layout which includes a text field and "other" option --- .../free/nrw/commons/delete/DeleteHelper.kt | 76 +++++++++++++++---- 1 file changed, 60 insertions(+), 16 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/delete/DeleteHelper.kt b/app/src/main/java/fr/free/nrw/commons/delete/DeleteHelper.kt index 3f9ae47ac1..bc4db10b42 100644 --- a/app/src/main/java/fr/free/nrw/commons/delete/DeleteHelper.kt +++ b/app/src/main/java/fr/free/nrw/commons/delete/DeleteHelper.kt @@ -5,7 +5,13 @@ import android.content.Context import android.content.DialogInterface import android.content.Intent import android.net.Uri +import android.text.Editable +import android.text.TextWatcher +import android.view.LayoutInflater +import android.widget.CheckBox +import android.widget.LinearLayout import androidx.appcompat.app.AlertDialog +import com.google.android.material.textfield.TextInputEditText import fr.free.nrw.commons.BuildConfig import fr.free.nrw.commons.Media import fr.free.nrw.commons.R @@ -206,7 +212,6 @@ class DeleteHelper @Inject constructor( alert.setCancelable(false) alert.setTitle(question) - val checkedItems = booleanArrayOf(false, false, false, false) val mUserReason = arrayListOf() val reasonList: Array @@ -217,7 +222,8 @@ class DeleteHelper @Inject constructor( reasonList = arrayOf( context.getString(R.string.delete_helper_ask_spam_selfie), context.getString(R.string.delete_helper_ask_spam_blurry), - context.getString(R.string.delete_helper_ask_spam_nonsense) + context.getString(R.string.delete_helper_ask_spam_nonsense), + context.getString(R.string.delete_helper_ask_spam_other) ) reasonListEnglish = arrayOf( getLocalizedResources(context, Locale.ENGLISH) @@ -225,7 +231,9 @@ class DeleteHelper @Inject constructor( getLocalizedResources(context, Locale.ENGLISH) .getString(R.string.delete_helper_ask_spam_blurry), getLocalizedResources(context, Locale.ENGLISH) - .getString(R.string.delete_helper_ask_spam_nonsense) + .getString(R.string.delete_helper_ask_spam_nonsense), + getLocalizedResources(context, Locale.ENGLISH) + .getString(R.string.delete_helper_ask_spam_other) ) } ReviewController.DeleteReason.COPYRIGHT_VIOLATION -> { @@ -256,21 +264,48 @@ class DeleteHelper @Inject constructor( } } - alert.setMultiChoiceItems( - reasonList, - checkedItems - ) { dialogInterface, position, isChecked -> - if (isChecked) { - mUserReason.add(position) - } else { - mUserReason.remove(position) - } + // Inflate custom layout + val inflater = LayoutInflater.from(context) + val customView = inflater.inflate(R.layout.dialog_deletion_reason, null) + val checkboxContainer = customView.findViewById(R.id.checkboxContainer) + val otherNotesEditText = customView.findViewById(R.id.otherNotesEditText) + + // Function to update OK button state (defined before use) + fun updateOkButtonState() { + val hasSelection = mUserReason.isNotEmpty() + val hasText = !otherNotesEditText?.text.isNullOrBlank() + d?.getButton(AlertDialog.BUTTON_POSITIVE)?.isEnabled = hasSelection || hasText + } - // Safely enable or disable the OK button based on selection - val dialog = dialogInterface as? AlertDialog - dialog?.getButton(AlertDialog.BUTTON_POSITIVE)?.isEnabled = mUserReason.isNotEmpty() + // Create checkboxes dynamically + val checkBoxes = mutableListOf() + reasonList.forEachIndexed { index, reason -> + val checkBox = CheckBox(context) + checkBox.text = reason + checkBox.setOnCheckedChangeListener { _, isChecked -> + if (isChecked) { + mUserReason.add(index) + } else { + mUserReason.remove(index) + } + updateOkButtonState() + } + checkBoxes.add(checkBox) + checkboxContainer.addView(checkBox) } + // Add text watcher for other notes field + otherNotesEditText?.addTextChangedListener(object : TextWatcher { + override fun afterTextChanged(s: Editable?) { + updateOkButtonState() + } + + override fun beforeTextChanged(s: CharSequence?, start: Int, count: Int, after: Int) {} + override fun onTextChanged(s: CharSequence?, start: Int, before: Int, count: Int) {} + }) + + alert.setView(customView) + alert.setPositiveButton(context.getString(R.string.ok)) { _, _ -> reviewCallback.disableButtons() @@ -281,12 +316,21 @@ class DeleteHelper @Inject constructor( ) append(" ") - mUserReason.forEachIndexed { index, position -> + mUserReason.sorted().forEachIndexed { index, position -> append(reasonListEnglish[position]) if (index != mUserReason.lastIndex) { append(", ") } } + + // Add other notes if provided + val otherNotes = otherNotesEditText?.text?.toString()?.trim() + if (!otherNotes.isNullOrBlank()) { + if (mUserReason.isNotEmpty()) { + append(". ") + } + append(otherNotes) + } } Timber.d("thread is askReasonAndExecute %s", Thread.currentThread().name) From 1b1b87579a8200f81a323e25b8027fe20fc787c5 Mon Sep 17 00:00:00 2001 From: anishamahuli Date: Tue, 3 Mar 2026 16:23:59 -0500 Subject: [PATCH 4/9] Remove unused xmlns:app namespace declaration; unused --- app/src/main/res/layout/dialog_deletion_reason.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/main/res/layout/dialog_deletion_reason.xml b/app/src/main/res/layout/dialog_deletion_reason.xml index b5ba15dddc..78a169b7a6 100644 --- a/app/src/main/res/layout/dialog_deletion_reason.xml +++ b/app/src/main/res/layout/dialog_deletion_reason.xml @@ -1,7 +1,6 @@ From 4203de15f9f89c175f78bc54fd40ea280761b08b Mon Sep 17 00:00:00 2001 From: anishamahuli Date: Tue, 3 Mar 2026 16:59:20 -0500 Subject: [PATCH 5/9] fix: logic to update OK button Initially had designed it to allow reviewers to submit only notes with no boxes checked. This doesn't make sense Also initially designed it to allow reviewers to submit image for deletion with only "Other" option checked but no notes provided. This also doesn't make sense. --- .../main/java/fr/free/nrw/commons/delete/DeleteHelper.kt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/delete/DeleteHelper.kt b/app/src/main/java/fr/free/nrw/commons/delete/DeleteHelper.kt index bc4db10b42..bdc528b6f3 100644 --- a/app/src/main/java/fr/free/nrw/commons/delete/DeleteHelper.kt +++ b/app/src/main/java/fr/free/nrw/commons/delete/DeleteHelper.kt @@ -270,11 +270,15 @@ class DeleteHelper @Inject constructor( val checkboxContainer = customView.findViewById(R.id.checkboxContainer) val otherNotesEditText = customView.findViewById(R.id.otherNotesEditText) - // Function to update OK button state (defined before use) + // Function to update OK button state fun updateOkButtonState() { val hasSelection = mUserReason.isNotEmpty() val hasText = !otherNotesEditText?.text.isNullOrBlank() - d?.getButton(AlertDialog.BUTTON_POSITIVE)?.isEnabled = hasSelection || hasText + val otherIndex = reasonList.size - 1 + val onlyOther = mUserReason.size == 1 && mUserReason.contains(otherIndex) + + d?.getButton(AlertDialog.BUTTON_POSITIVE)?.isEnabled = + hasSelection && (!onlyOther || hasText) } // Create checkboxes dynamically From b7134b1132e99915ef0110dd51804d672252dea7 Mon Sep 17 00:00:00 2001 From: anishamahuli Date: Fri, 6 Mar 2026 16:01:48 -0500 Subject: [PATCH 6/9] change hint text in text field from "Other notes" to "Justification" --- app/src/main/res/values/strings.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 3166b57728..8558c6026b 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -569,7 +569,7 @@ Upload your first media by tapping on the add button. totally blurry nonsense, absolutely unusable in any article Other: specify below - Other notes + Justification Press photo Random photo from internet Logo From 6cbaa014a42b05ff5f64f22432772dd1ffeebfc7 Mon Sep 17 00:00:00 2001 From: anishamahuli Date: Fri, 6 Mar 2026 16:21:24 -0500 Subject: [PATCH 7/9] require justification for submission the "OK" button does not enable until there is both a box checked AND some text in the justification text field. The second requirement is the new one --- app/src/main/java/fr/free/nrw/commons/delete/DeleteHelper.kt | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/delete/DeleteHelper.kt b/app/src/main/java/fr/free/nrw/commons/delete/DeleteHelper.kt index bdc528b6f3..bc7f55ddb5 100644 --- a/app/src/main/java/fr/free/nrw/commons/delete/DeleteHelper.kt +++ b/app/src/main/java/fr/free/nrw/commons/delete/DeleteHelper.kt @@ -274,11 +274,8 @@ class DeleteHelper @Inject constructor( fun updateOkButtonState() { val hasSelection = mUserReason.isNotEmpty() val hasText = !otherNotesEditText?.text.isNullOrBlank() - val otherIndex = reasonList.size - 1 - val onlyOther = mUserReason.size == 1 && mUserReason.contains(otherIndex) - d?.getButton(AlertDialog.BUTTON_POSITIVE)?.isEnabled = - hasSelection && (!onlyOther || hasText) + d?.getButton(AlertDialog.BUTTON_POSITIVE)?.isEnabled = hasSelection && hasText } // Create checkboxes dynamically From be176f53893ac056ec5de8a5e0047a8c06567c3a Mon Sep 17 00:00:00 2001 From: anishamahuli Date: Fri, 6 Mar 2026 17:30:42 -0500 Subject: [PATCH 8/9] test: replace false button state test with meaningful assertion The previous alertDialogPositiveButtonDisableTest() was a no-op. getListener() returned null so the simulated click was silently skipped, and the button defaulted to enabled regardless of any logic. The new test actually checks a checkbox via the view hierarchy and asserts the button stays disabled when no justification text has been entered. --- .../nrw/commons/delete/DeleteHelperTest.kt | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) 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 2300998108..cc25a6b9bb 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 @@ -2,6 +2,9 @@ package fr.free.nrw.commons.delete import android.app.AlertDialog import android.content.Context +import android.widget.CheckBox +import android.widget.LinearLayout +import com.google.android.material.textfield.TextInputEditText import com.nhaarman.mockitokotlin2.eq import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.verify @@ -181,7 +184,7 @@ class DeleteHelperTest { } @Test - fun alertDialogPositiveButtonDisableTest() { + fun buttonRemainsDisabledWhenCheckboxCheckedButNoText() { val mContext = RuntimeEnvironment.getApplication().applicationContext deleteHelper.askReasonAndExecute( media, @@ -190,15 +193,12 @@ class DeleteHelperTest { ReviewController.DeleteReason.COPYRIGHT_VIOLATION, callback ) - deleteHelper.getListener()?.onClick( - deleteHelper.getDialog(), - 1, - true - ) - assertEquals( - true, - deleteHelper.getDialog()?.getButton(AlertDialog.BUTTON_POSITIVE)?.isEnabled - ) + val dialog = deleteHelper.getDialog() + val container = dialog?.findViewById(R.id.checkboxContainer) + val checkbox = container?.getChildAt(0) as? CheckBox + checkbox?.isChecked = true + + assertEquals(false, dialog?.getButton(AlertDialog.BUTTON_POSITIVE)?.isEnabled) } @Test From 9ba71d10849a0bf91ba41a86621c5e2239f442de Mon Sep 17 00:00:00 2001 From: anishamahuli Date: Fri, 6 Mar 2026 17:36:59 -0500 Subject: [PATCH 9/9] test: replace false button enable test with real checkbox and text interaction The previous alertDialogPositiveButtonEnableTest() was a no-op. getListener() returns null so the simulated click was silently skipped, and the button defaulted to enabled regardless of any logic. The new test checks a checkbox and enters justification text via the view hierarchy, asserting the button enabled only when both conditions are met. --- .../nrw/commons/delete/DeleteHelperTest.kt | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) 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 cc25a6b9bb..c254ec4352 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 @@ -202,11 +202,25 @@ class DeleteHelperTest { } @Test - fun alertDialogPositiveButtonEnableTest() { + fun buttonEnabledWhenCheckboxCheckedAndTextEntered() { val mContext = RuntimeEnvironment.getApplication().applicationContext - deleteHelper.askReasonAndExecute(media, mContext, "My Question", ReviewController.DeleteReason.COPYRIGHT_VIOLATION, callback) - deleteHelper.getListener()?.onClick(deleteHelper.getDialog(), 1, true) - assertEquals(true, deleteHelper.getDialog()?.getButton(AlertDialog.BUTTON_POSITIVE)?.isEnabled) + deleteHelper.askReasonAndExecute( + media, + mContext, + "My Question", + ReviewController.DeleteReason.SPAM, + callback + ) + + val dialog = deleteHelper.getDialog() + val container = dialog?.findViewById(R.id.checkboxContainer) + val checkbox = container?.getChildAt(0) as? CheckBox + checkbox?.isChecked = true + + val editText = dialog?.findViewById(R.id.otherNotesEditText) + editText?.setText("some justification") + + assertEquals(true, dialog?.getButton(AlertDialog.BUTTON_POSITIVE)?.isEnabled) } @Test(expected = RuntimeException::class)