diff --git a/app/src/main/java/fr/free/nrw/commons/description/DescriptionEditActivity.kt b/app/src/main/java/fr/free/nrw/commons/description/DescriptionEditActivity.kt index b1f1b7f9b8d..164108349a9 100644 --- a/app/src/main/java/fr/free/nrw/commons/description/DescriptionEditActivity.kt +++ b/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) diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadMediaDetailAdapter.kt b/app/src/main/java/fr/free/nrw/commons/upload/UploadMediaDetailAdapter.kt index b19da15e6b0..2ab3a42341b 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadMediaDetailAdapter.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadMediaDetailAdapter.kt @@ -123,6 +123,7 @@ class UploadMediaDetailAdapter : RecyclerView.Adapter= 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 */ diff --git a/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadMediaDetailAdapterUnitTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadMediaDetailAdapterUnitTest.kt index 7cc59b78dd1..6b8ce42abde 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadMediaDetailAdapterUnitTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadMediaDetailAdapterUnitTest.kt @@ -147,6 +147,33 @@ class UploadMediaDetailAdapterUnitTest { adapter.addDescription(uploadMediaDetail) val map: HashMap = selectedLanguages.get(adapter) as HashMap 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 = 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 = selectedLanguages.get(adapter) as HashMap Assert.assertEquals(map[list.size], null) + verify(eventListener).onMediaDetailsChanged() } @Test diff --git a/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadMediaPresenterTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadMediaPresenterTest.kt index da14438342b..54d2ce3166e 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadMediaPresenterTest.kt +++ b/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 */ diff --git a/app/src/test/kotlin/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragmentUnitTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragmentUnitTest.kt index 0cab47c67f0..0f8ad2cbec9 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragmentUnitTest.kt +++ b/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() {