#3101: Add image upload limit of 20 to custom selector (#5369)

* Add basic image limit warning to custom selector

* Block upload button when image limit is exceeded

* Complete basic functionality for upload limit: Disabled button, warning sign & toast

* Consolidate functionality between upload limit and not for upload marking

* Upload Limit: write unit tests and optimize control flow

* Upload Limit Tests: improve logic coverage and alter to stay valid if limit is changed

* Upload Limit: polish javadocs and add explanation to warning toast.

* Upload Limit: refactor variable names

* Upload Limit: change warning toast to dialog box, repurposing welcome dialog code & layout

* Upload Limit: remove error icon when the mark as not for upload button is pressed

app/src/main/java/fr/free/nrw/commons/customselector/ui/selector/CustomSelectorActivity.kt

@@ -28,9 +28,9 @@ import fr.free.nrw.commons.media.ZoomableActivity
 import fr.free.nrw.commons.theme.BaseActivity
 import fr.free.nrw.commons.upload.FileUtilsWrapper
 import fr.free.nrw.commons.utils.CustomSelectorUtils
-import kotlinx.android.synthetic.main.activity_login.view.title
 import kotlinx.coroutines.*
 import java.io.File
+import java.lang.Integer.max
 import javax.inject.Inject
 
 
@@ -67,6 +67,22 @@ class CustomSelectorActivity : BaseActivity(), FolderClickListener, ImageSelectL
      */
     private lateinit var prefs: SharedPreferences
 
+    /**
+     * Maximum number of images that can be selected.
+     */
+    private val uploadLimit: Int = 20
+
+    /**
+     * Flag that is marked true when the amount
+     * of selected images is greater than the upload limit.
+     */
+    private var uploadLimitExceeded: Boolean = false
+
+    /**
+     * Tracks the amount by which the upload limit has been exceeded.
+     */
+    private var uploadLimitExceededBy: Int = 0
+
     /**
      * View Model Factory.
      */
@@ -201,6 +217,7 @@ class CustomSelectorActivity : BaseActivity(), FolderClickListener, ImageSelectL
             i++
         }
         markAsNotForUpload(selectedImages)
+        toolbarBinding.imageLimitError.visibility = View.INVISIBLE
     }
 
     /**
@@ -314,6 +331,10 @@ class CustomSelectorActivity : BaseActivity(), FolderClickListener, ImageSelectL
     private fun setUpToolbar() {
         val back: ImageButton = findViewById(R.id.back)
         back.setOnClickListener { onBackPressed() }
+
+        val limitError: ImageButton = findViewById(R.id.image_limit_error)
+        limitError.visibility = View.INVISIBLE
+        limitError.setOnClickListener { displayUploadLimitWarning() }
     }
 
     /**
@@ -342,7 +363,19 @@ class CustomSelectorActivity : BaseActivity(), FolderClickListener, ImageSelectL
         viewModel.selectedImages.value = selectedImages
         changeTitle(bucketName, selectedImages.size)
 
-        if (selectedNotForUploadImages > 0) {
+        uploadLimitExceeded = selectedImages.size > uploadLimit
+        uploadLimitExceededBy = max(selectedImages.size - uploadLimit,0)
+
+        if (uploadLimitExceeded && selectedNotForUploadImages == 0) {
+            toolbarBinding.imageLimitError.visibility = View.VISIBLE
+            bottomSheetBinding.upload.text = resources.getString(
+                R.string.custom_selector_button_limit_text, uploadLimit)
+        } else {
+            toolbarBinding.imageLimitError.visibility = View.INVISIBLE
+            bottomSheetBinding.upload.text = resources.getString(R.string.upload)
+        }
+
+        if (uploadLimitExceeded || selectedNotForUploadImages > 0) {
             bottomSheetBinding.upload.isEnabled = false
             bottomSheetBinding.upload.alpha = 0.5f
         } else {
@@ -390,22 +423,22 @@ class CustomSelectorActivity : BaseActivity(), FolderClickListener, ImageSelectL
      * Get the selected images. Remove any non existent file, forward the data to finish selector.
      */
     fun onDone() {
-        val selectedImages = viewModel.selectedImages.value
-        if (selectedImages.isNullOrEmpty()) {
-            finishPickImages(arrayListOf())
-            return
-        }
-        var i = 0
-        while (i < selectedImages.size) {
-            val path = selectedImages[i].path
-            val file = File(path)
-            if (!file.exists()) {
-                selectedImages.removeAt(i)
-                i--
+            val selectedImages = viewModel.selectedImages.value
+            if (selectedImages.isNullOrEmpty()) {
+                finishPickImages(arrayListOf())
+                return
             }
-            i++
-        }
-        finishPickImages(selectedImages)
+            var i = 0
+            while (i < selectedImages.size) {
+                val path = selectedImages[i].path
+                val file = File(path)
+                if (!file.exists()) {
+                    selectedImages.removeAt(i)
+                    i--
+                }
+                i++
+            }
+            finishPickImages(selectedImages)
     }
 
     /**
@@ -432,6 +465,20 @@ class CustomSelectorActivity : BaseActivity(), FolderClickListener, ImageSelectL
         }
     }
 
+    /**
+     * Displays a dialog explaining the upload limit warning.
+     */
+    private fun displayUploadLimitWarning() {
+        val dialog = Dialog(this)
+        dialog.requestWindowFeature(Window.FEATURE_NO_TITLE)
+        dialog.setContentView(R.layout.custom_selector_limit_dialog)
+        (dialog.findViewById(R.id.btn_dismiss_limit_warning) as Button).setOnClickListener()
+        { dialog.dismiss() }
+        (dialog.findViewById(R.id.upload_limit_warning) as TextView).text = resources.getString(
+            R.string.custom_selector_over_limit_warning, uploadLimit, uploadLimitExceededBy)
+        dialog.show()
+    }
+
     /**
      * On activity destroy
      * If image fragment is open, overwrite its attributes otherwise discard the values.

app/src/main/res/layout/custom_selector_limit_dialog.xml

@@ -0,0 +1,34 @@
+<?xml version="1.0" encoding="utf-8"?>
+<androidx.constraintlayout.widget.ConstraintLayout
+  xmlns:android="http://schemas.android.com/apk/res/android"
+  android:layout_height="wrap_content"
+  android:layout_width="match_parent"
+  android:layout_margin="@dimen/dimen_10">
+  <ScrollView
+    android:layout_height="match_parent"
+    android:layout_width="match_parent">
+
+    <LinearLayout
+      android:layout_width="match_parent"
+      android:layout_height="wrap_content"
+      android:orientation="vertical" >
+
+      <TextView
+        android:id="@+id/upload_limit_warning"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:layout_gravity="center"
+        android:layout_margin="@dimen/dimen_10"
+        android:textSize="@dimen/description_text_size"
+        android:textStyle="bold" />
+
+      <Button
+        android:id="@+id/btn_dismiss_limit_warning"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:text="@string/custom_selector_dismiss_limit_warning_button_text"
+        android:textColor="@color/primaryTextColor" />
+
+    </LinearLayout>
+  </ScrollView>
+</androidx.constraintlayout.widget.ConstraintLayout>

app/src/main/res/layout/custom_selector_toolbar.xml

@@ -1,43 +1,56 @@
 <?xml version="1.0" encoding="utf-8"?>
-<merge xmlns:android="http://schemas.android.com/apk/res/android"
+<merge xmlns:tools="http://schemas.android.com/tools"
+  xmlns:android="http://schemas.android.com/apk/res/android"
   xmlns:app="http://schemas.android.com/apk/res-auto"
   >
-    <androidx.constraintlayout.widget.ConstraintLayout
-      android:id="@+id/toolbar_layout"
-      android:layout_width="match_parent"
-      android:layout_height="?attr/actionBarSize"
+
+  <androidx.constraintlayout.widget.ConstraintLayout
+    android:id="@+id/toolbar_layout"
+    android:layout_width="match_parent"
+    android:layout_height="?attr/actionBarSize"
+    android:background="?attr/mainBackground"
+    app:layout_constraintTop_toTopOf="parent">
+
+    <ImageButton
+      android:id="@+id/back"
+      android:layout_width="wrap_content"
+      android:layout_height="match_parent"
+      android:layout_centerVertical="true"
+      android:background="?attr/selectableItemBackgroundBorderless"
+      android:clickable="true"
+      android:contentDescription="@string/back"
+      android:focusable="true"
+      android:padding="@dimen/standard_gap"
+      app:layout_constraintBottom_toBottomOf="parent"
+      app:layout_constraintStart_toStartOf="parent"
       app:layout_constraintTop_toTopOf="parent"
-      android:background="?attr/mainBackground">
+      app:srcCompat="?attr/custom_selector_back" />
 
-      <ImageButton
-        android:id="@+id/back"
-        android:layout_width="wrap_content"
-        android:layout_height="match_parent"
-        app:layout_constraintBottom_toBottomOf="parent"
-        app:layout_constraintStart_toStartOf="parent"
-        app:layout_constraintTop_toTopOf="parent"
-        android:layout_centerVertical="true"
-        android:background="?attr/selectableItemBackgroundBorderless"
-        android:padding="@dimen/standard_gap"
-        android:clickable="true"
-        android:focusable="true"
-        app:srcCompat="?attr/custom_selector_back"
-        android:contentDescription="@string/back" />
+    <TextView
+      android:id="@+id/title"
+      style="@style/TextAppearance.AppCompat.Widget.ActionBar.Title"
+      android:layout_width="0dp"
+      android:layout_height="29dp"
+      android:ellipsize="end"
+      android:singleLine="true"
+      android:text="@string/custom_selector_title"
+      android:textAlignment="center"
+      app:layout_constraintBottom_toBottomOf="parent"
+      app:layout_constraintEnd_toStartOf="@+id/image_limit_error"
+      app:layout_constraintStart_toEndOf="@+id/back"
+      app:layout_constraintTop_toTopOf="parent" />
 
-      <TextView
-        android:id="@+id/title"
-        android:textAlignment="center"
-        android:layout_width="0dp"
-        android:layout_height="wrap_content"
-        android:layout_marginEnd="@dimen/dimen_20"
-        app:layout_constraintStart_toEndOf="@id/back"
-        app:layout_constraintEnd_toEndOf="parent"
-        app:layout_constraintTop_toTopOf="parent"
-        app:layout_constraintBottom_toBottomOf="parent"
-        android:singleLine="true"
-        android:ellipsize="end"
-        android:text="@string/custom_selector_title"
-        style="@style/TextAppearance.AppCompat.Widget.ActionBar.Title" />
+    <ImageButton
+      android:id="@+id/image_limit_error"
+      android:layout_width="48dp"
+      android:layout_height="0dp"
+      android:background="#00FFFFFF"
+      android:contentDescription="@string/custom_selector_limit_error_desc"
+      app:layout_constraintBottom_toBottomOf="parent"
+      app:layout_constraintEnd_toEndOf="parent"
+      app:layout_constraintTop_toTopOf="parent"
+      app:layout_constraintVertical_bias="1.0"
+      app:srcCompat="@drawable/ic_error_red_24dp" />
 
   </androidx.constraintlayout.widget.ConstraintLayout>
 </merge>

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

@@ -716,6 +716,10 @@ Upload your first media by tapping on the add button.</string>
   <string name="custom_selector_info_text2">Unlike the picture on the left, the picture on the right has the Commons logo indicating it is already uploaded. \n Touch and hold for image preview.</string>
   <string name="welcome_custom_selector_ok">Awesome</string>
   <string name="custom_selector_already_uploaded_image_text">This image has already been uploaded to Commons.</string>
+  <string name="custom_selector_over_limit_warning">For technical reasons, the app can\'t reliably upload more than %1$d pictures at once. The upload limit of %1$d has been exceeded by %2$d.</string>
+  <string name="custom_selector_dismiss_limit_warning_button_text">Dismiss</string>
+  <string name="custom_selector_button_limit_text">Max: %1$d</string>
+  <string name="custom_selector_limit_error_desc">Error: Upload Limit Exceeded</string>
   <string name="place_state_wlm">WLM</string>
   <string name="wlm_upload_info">This image will be entered into the Wiki Loves Monuments contest</string>
   <string name="display_monuments">Display monuments</string>

app/src/test/kotlin/fr/free/nrw/commons/customselector/ui/selector/CustomSelectorActivityTest.kt

@@ -7,8 +7,10 @@ import fr.free.nrw.commons.TestAppAdapter
 import fr.free.nrw.commons.TestCommonsApplication
 import fr.free.nrw.commons.customselector.model.Image
 import fr.free.nrw.commons.customselector.ui.adapter.ImageAdapter
+import kotlinx.android.synthetic.main.bottom_sheet_nearby.bottom_sheet
 import org.junit.Before
 import org.junit.Test
+import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.Assertions.assertNotNull
 import org.junit.runner.RunWith
 import org.mockito.Mockito
@@ -18,6 +20,7 @@ import org.robolectric.Robolectric
 import org.robolectric.RobolectricTestRunner
 import org.robolectric.annotation.Config
 import org.wikipedia.AppAdapter
+import java.lang.reflect.Field
 import java.lang.reflect.Method
 
 /**
@@ -210,4 +213,68 @@ class CustomSelectorActivityTest {
         method.isAccessible = true
         method.invoke(activity)
     }
+
+    /**
+     * Test displayUploadLimitWarning Function.
+     */
+    @Test
+    @Throws(Exception::class)
+    fun testDisplayUploadLimitWarning() {
+        val method: Method = CustomSelectorActivity::class.java.getDeclaredMethod(
+            "displayUploadLimitWarning"
+        )
+        method.isAccessible = true
+        method.invoke(activity)
+    }
+
+    /**
+     * Logic tests for the upload limit functionality.
+     */
+    @Test
+    @Throws(Exception::class)
+    fun testUploadLimit() {
+        val overLimit: Field =
+            CustomSelectorActivity::class.java.getDeclaredField("uploadLimitExceeded")
+        overLimit.isAccessible = true
+        val exceededBy: Field =
+            CustomSelectorActivity::class.java.getDeclaredField("uploadLimitExceededBy")
+        exceededBy.isAccessible = true
+        val limit: Field =
+            CustomSelectorActivity::class.java.getDeclaredField("uploadLimit")
+        limit.isAccessible = true
+
+        // all tests check integration with not for upload marking
+
+        // test with list size limit
+        for (i in 1..limit.getInt(activity)) {
+            images.add(Image(i.toLong(), i.toString(), uri,
+                "abc/abc", 1, "bucket1"))
+        }
+        activity.onFolderClick(1, "test", 0)
+        activity.onSelectedImagesChanged(images,0)
+        assertEquals(false, overLimit.getBoolean(activity))
+        assertEquals(0, exceededBy.getInt(activity))
+        activity.onSelectedImagesChanged(images, 1)
+        assertEquals(false, overLimit.getBoolean(activity))
+        assertEquals(0, exceededBy.getInt(activity))
+
+        // test with list size limit+1
+        images.add(image)
+        activity.onSelectedImagesChanged(images,0)
+        assertEquals(true, overLimit.getBoolean(activity))
+        assertEquals(1, exceededBy.getInt(activity))
+        activity.onSelectedImagesChanged(images,1)
+        assertEquals(true, overLimit.getBoolean(activity))
+        assertEquals(1, exceededBy.getInt(activity))
+
+        //test with list size 1
+        images.clear()
+        images.add(image)
+        activity.onSelectedImagesChanged(images,0)
+        assertEquals(false, overLimit.getBoolean(activity))
+        assertEquals(0, exceededBy.getInt(activity))
+        activity.onSelectedImagesChanged(images,1)
+        assertEquals(false, overLimit.getBoolean(activity))
+        assertEquals(0, exceededBy.getInt(activity))
+    }
 }