Fix: multi-language captions not saved on upload (#6938)

app/src/main/java/fr/free/nrw/commons/description/DescriptionEditActivity.kt

@@ -157,6 +157,8 @@ class DescriptionEditActivity :
 
     override fun onPrimaryCaptionTextChange(isNotEmpty: Boolean) {}
 
+    override fun onMediaDetailsChanged() {}
+
     private fun onVoiceInput(result: ActivityResult) {
         if (result.resultCode == RESULT_OK && result.data != null) {
             val resultData = result.data!!.getStringArrayListExtra(RecognizerIntent.EXTRA_RESULTS)

app/src/main/java/fr/free/nrw/commons/upload/UploadMediaDetailAdapter.kt

@@ -123,6 +123,7 @@ class UploadMediaDetailAdapter : RecyclerView.Adapter<UploadMediaDetailAdapter.V
         selectedLanguages[uploadMediaDetails.size] = "en"
         uploadMediaDetails.add(uploadMediaDetail)
         notifyItemInserted(uploadMediaDetails.size)
+        eventListener?.onMediaDetailsChanged()
     }
 
     private fun startSpeechInput(locale: String) {
@@ -180,6 +181,9 @@ class UploadMediaDetailAdapter : RecyclerView.Adapter<UploadMediaDetailAdapter.V
         notifyItemRemoved(position)
         notifyItemRangeChanged(position, uploadMediaDetails.size - position)
         updateAddButtonVisibility()
+        // Without this, a removed row stays in the persisted UploadItem and still gets
+        // uploaded, since only addDescription previously notified the fragment (#6938).
+        eventListener?.onMediaDetailsChanged()
     }
 
     inner class ViewHolder(val binding: RowItemDescriptionBinding) :
@@ -558,6 +562,8 @@ class UploadMediaDetailAdapter : RecyclerView.Adapter<UploadMediaDetailAdapter.V
     interface EventListener {
         fun onPrimaryCaptionTextChange(isNotEmpty: Boolean)
         fun addLanguage()
+
+        fun onMediaDetailsChanged()
     }
 
     internal enum class SelectedVoiceIcon {

app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragment.kt

@@ -184,18 +184,7 @@ class UploadMediaDetailFragment : UploadBaseFragment(), UploadMediaDetailsContra
             Timber.d("Restoring state: savedItems size = %s", savedItems?.size ?: "null")
             if (savedItems != null && savedItems.isNotEmpty()) {
                 uploadMediaDetailAdapter.items = savedItems
-                // only call setUploadMediaDetails if indexOfFragment is valid
-                if (fragmentCallback != null) {
-                    indexOfFragment = fragmentCallback!!.getIndexInViewFlipper(this)
-                    if (indexOfFragment >= 0) {
-                        presenter.setUploadMediaDetails(uploadMediaDetailAdapter.items, indexOfFragment)
-                        Timber.d("Restored and set upload media details for index %d", indexOfFragment)
-                    } else {
-                        Timber.w("Invalid indexOfFragment %d, skipping setUploadMediaDetails", indexOfFragment)
-                    }
-                } else {
-                    Timber.w("fragmentCallback is null, skipping setUploadMediaDetails")
-                }
+                persistMediaDetails()
             } else {
                 // initialize with a default UploadMediaDetail if saved state is empty or null
                 uploadMediaDetailAdapter.items = mutableListOf(UploadMediaDetail())
@@ -486,6 +475,27 @@ class UploadMediaDetailFragment : UploadBaseFragment(), UploadMediaDetailsContra
         fragmentCallback!!.onNextButtonClicked(indexOfFragment)
     }
 
+    /**
+     * Fixes issue #6938: without this, rows added via "+" only existed in the adapter
+     * and were lost once the fragment was repopulated, so only the primary caption uploaded.
+     */
+    private fun persistMediaDetails() {
+        // only call setUploadMediaDetails if indexOfFragment is valid
+        if (fragmentCallback != null) {
+            indexOfFragment = fragmentCallback!!.getIndexInViewFlipper(this)
+            if (indexOfFragment >= 0) {
+                presenter.setUploadMediaDetails(uploadMediaDetailAdapter.items, indexOfFragment)
+                Timber.d("Restored and set upload media details for index %d", indexOfFragment)
+            } else {
+                Timber.w("Invalid indexOfFragment %d, skipping setUploadMediaDetails", indexOfFragment)
+            }
+        } else {
+            Timber.w("fragmentCallback is null, skipping setUploadMediaDetails")
+        }
+    }
+
+    override fun onMediaDetailsChanged() = persistMediaDetails()
+
     /**
      * This method gets called whenever the next/previous button is pressed
      */

app/src/test/kotlin/fr/free/nrw/commons/upload/UploadMediaDetailAdapterUnitTest.kt

@@ -147,6 +147,33 @@ class UploadMediaDetailAdapterUnitTest {
         adapter.addDescription(uploadMediaDetail)
         val map: HashMap<Int, String> = selectedLanguages.get(adapter) as HashMap<Int, String>
         Assert.assertEquals(map[list.size], null)
+        // Regression test for issue #6938: added rows must propagate to the UploadItem.
+        verify(eventListener).onMediaDetailsChanged()
+    }
+
+    @Test
+    @Throws(Exception::class)
+    fun testAddDescriptionSyncsToBackingListAndFormatCaptions() {
+        // Regression test for issue #6938: a caption added via "+" must reach
+        // Contribution.formatCaptions, not just the adapter's own copy of the list.
+        val realAdapter =
+            UploadMediaDetailAdapter(fragment, "", recentLanguagesDao, mockResultLauncher)
+        realAdapter.items = mutableListOf(UploadMediaDetail(languageCode = "en", captionText = "test"))
+        var uploadItemMediaDetails: List<UploadMediaDetail> = emptyList()
+        realAdapter.eventListener = object : UploadMediaDetailAdapter.EventListener {
+            override fun onPrimaryCaptionTextChange(isNotEmpty: Boolean) = Unit
+            override fun addLanguage() = Unit
+            override fun onMediaDetailsChanged() {
+                uploadItemMediaDetails = realAdapter.items
+            }
+        }
+
+        realAdapter.addDescription(UploadMediaDetail(languageCode = "ja", captionText = "テスト"))
+
+        val captions = fr.free.nrw.commons.contributions.Contribution.formatCaptions(uploadItemMediaDetails)
+        Assert.assertEquals(2, captions.size)
+        Assert.assertEquals("test", captions["en"])
+        Assert.assertEquals("テスト", captions["ja"])
     }
 
     @Test
@@ -164,6 +191,7 @@ class UploadMediaDetailAdapterUnitTest {
         adapter.removeDescription(uploadMediaDetail, list.size)
         val map: HashMap<Int, String> = selectedLanguages.get(adapter) as HashMap<Int, String>
         Assert.assertEquals(map[list.size], null)
+        verify(eventListener).onMediaDetailsChanged()
     }
 
     @Test

app/src/test/kotlin/fr/free/nrw/commons/upload/UploadMediaPresenterTest.kt

@@ -154,6 +154,32 @@ class UploadMediaPresenterTest {
         testScheduler.triggerActions()
     }
 
+    /**
+     * Regression test for #6938: with multiple images in one upload session, captions
+     * for one image must not be written onto another image's UploadItem.
+     */
+    @Test
+    fun setUploadMediaDetailsOnlyUpdatesTargetedItemInMultiImageUpload() {
+        val firstItem =
+            UploadItem(Uri.EMPTY, null, null, null, 0, null, null, null)
+        val firstItemDetails =
+            listOf(UploadMediaDetail(languageCode = "en", captionText = "first"))
+        firstItem.uploadMediaDetails = firstItemDetails.toMutableList()
+        val secondItem =
+            UploadItem(Uri.EMPTY, null, null, null, 0, null, null, null)
+        whenever(repository.getUploads()).thenReturn(listOf(firstItem, secondItem))
+
+        val secondItemDetails =
+            listOf(
+                UploadMediaDetail(languageCode = "en", captionText = "second"),
+                UploadMediaDetail(languageCode = "de", captionText = "zweite"),
+            )
+        uploadMediaPresenter.setUploadMediaDetails(secondItemDetails, 1)
+
+        assertEquals(secondItemDetails, secondItem.uploadMediaDetails)
+        assertEquals(firstItemDetails, firstItem.uploadMediaDetails)
+    }
+
     /**
      * Test for empty file name when the user presses the NEXT button
      */

app/src/test/kotlin/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragmentUnitTest.kt

@@ -33,6 +33,7 @@ import fr.free.nrw.commons.nearby.Place
 import fr.free.nrw.commons.upload.ImageCoordinates
 import fr.free.nrw.commons.upload.UploadActivity
 import fr.free.nrw.commons.upload.UploadItem
+import fr.free.nrw.commons.upload.UploadMediaDetail
 import fr.free.nrw.commons.upload.UploadMediaDetailAdapter
 import fr.free.nrw.commons.upload.mediaDetails.UploadMediaDetailFragment.Companion.LAST_ZOOM
 import org.junit.Assert
@@ -362,6 +363,26 @@ class UploadMediaDetailFragmentUnitTest {
         fragment.updateMediaDetails(mock())
     }
 
+    @Test
+    @Throws(Exception::class)
+    fun testOnMediaDetailsChangedPersistsAdapterItemsToUploadItem() {
+        // Regression test for #6938: a language row added via "+" must be written back into the
+        // UploadItem backing this fragment to ensure it gets uploaded
+        Shadows.shadowOf(Looper.getMainLooper()).idle()
+        Whitebox.setInternalState(fragment, "fragmentCallback", callback)
+        Whitebox.setInternalState(fragment, "presenter", presenter)
+        val itemsWithSecondLanguage = mutableListOf(
+            UploadMediaDetail(languageCode = "en", captionText = "test"),
+            UploadMediaDetail(languageCode = "ja", captionText = "テスト")
+        )
+        `when`(uploadMediaDetailAdapter.items).thenReturn(itemsWithSecondLanguage)
+
+        fragment.onMediaDetailsChanged()
+
+        Mockito.verify(presenter)
+            .setUploadMediaDetails(itemsWithSecondLanguage, 0)
+    }
+
     @Test
     @Throws(Exception::class)
     fun testOnDestroyView() {