Custom picker: Show differently pictures that are currently being uploaded (#5618)

* Custom picker: Show differently pictures that are currently being uploaded

* fix tests

* fix test

* fix crash

* handle all cases

* handle all cases

* fix

* Hide Images When showAlreadyActioned is disabled

* Fix Tests

* cleanup
This commit is contained in:
Shashank Kumar 2024-03-22 18:43:19 +05:30 committed by GitHub
parent 72cdb5d0dd
commit 2d333a2af0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 129 additions and 39 deletions

View file

@ -13,6 +13,7 @@ import androidx.recyclerview.widget.RecyclerView
import com.bumptech.glide.Glide
import com.simplecityapps.recyclerview_fastscroll.views.FastScrollRecyclerView
import fr.free.nrw.commons.R
import fr.free.nrw.commons.contributions.Contribution
import fr.free.nrw.commons.customselector.helper.ImageHelper
import fr.free.nrw.commons.customselector.helper.ImageHelper.CUSTOM_SELECTOR_PREFERENCE_KEY
import fr.free.nrw.commons.customselector.helper.ImageHelper.SHOW_ALREADY_ACTIONED_IMAGES_PREFERENCE_KEY
@ -85,6 +86,8 @@ class ImageAdapter(
*/
private var actionableImagesMap: TreeMap<Int, Image> = TreeMap()
private var uploadingContributionList: List<Contribution> = ArrayList()
/**
* Stores already added positions of actionable images
*/
@ -156,9 +159,8 @@ class ImageAdapter(
} else {
holder.itemUnselected()
}
imageLoader.queryAndSetView(
holder, image, ioDispatcher, defaultDispatcher
holder, image, ioDispatcher, defaultDispatcher ,uploadingContributionList
)
scope.launch {
val sharedPreferences: SharedPreferences =
@ -169,7 +171,7 @@ class ImageAdapter(
// If the position is not already visited, that means the position is new then
// finds the next actionable image position from all images
if (!alreadyAddedPositions.contains(position)) {
processThumbnailForActionedImage(holder, position)
processThumbnailForActionedImage(holder, position, uploadingContributionList)
// If the position is already visited, that means the image is already present
// inside map, so it will fetch the image from the map and load in the holder
@ -207,11 +209,12 @@ class ImageAdapter(
*/
suspend fun processThumbnailForActionedImage(
holder: ImageViewHolder,
position: Int
position: Int,
uploadingContributionList: List<Contribution>
) {
val next = imageLoader.nextActionableImage(
allImages, ioDispatcher, defaultDispatcher,
nextImagePosition
nextImagePosition, uploadingContributionList
)
// If next actionable image is found, saves it, as the the search for
@ -331,12 +334,13 @@ class ImageAdapter(
/**
* Initialize the data set.
*/
fun init(newImages: List<Image>, fixedImages: List<Image>, emptyMap: TreeMap<Int, Image>) {
fun init(newImages: List<Image>, fixedImages: List<Image>, emptyMap: TreeMap<Int, Image>, uploadedImages: List<Contribution> = ArrayList()) {
allImages = fixedImages
val oldImageList:ArrayList<Image> = images
val newImageList:ArrayList<Image> = ArrayList(newImages)
actionableImagesMap = emptyMap
alreadyAddedPositions = ArrayList()
uploadingContributionList = uploadedImages
nextImagePosition = 0
reachedEndOfFolder = false
selectedImages = ArrayList()
@ -358,11 +362,11 @@ class ImageAdapter(
/**
* Refresh the data in the adapter
*/
fun refresh(newImages: List<Image>, fixedImages: List<Image>) {
fun refresh(newImages: List<Image>, fixedImages: List<Image>, uploadingImages: List<Contribution> = ArrayList()) {
numberOfSelectedImagesMarkedAsNotForUpload = 0
images.clear()
selectedImages = arrayListOf()
init(newImages, fixedImages, TreeMap())
init(newImages, fixedImages, TreeMap(),uploadingImages)
notifyDataSetChanged()
}
@ -452,6 +456,7 @@ class ImageAdapter(
class ImageViewHolder(itemView: View): RecyclerView.ViewHolder(itemView) {
val image: ImageView = itemView.findViewById(R.id.image_thumbnail)
private val uploadedGroup: Group = itemView.findViewById(R.id.uploaded_group)
private val uploadingGroup: Group = itemView.findViewById(R.id.uploading_group)
private val notForUploadGroup: Group = itemView.findViewById(R.id.not_for_upload_group)
private val selectedGroup: Group = itemView.findViewById(R.id.selected_group)
@ -476,6 +481,13 @@ class ImageAdapter(
uploadedGroup.visibility = View.VISIBLE
}
/**
* Item is uploading
*/
fun itemUploading() {
uploadingGroup.visibility = View.VISIBLE
}
/**
* Item is not for upload view
*/
@ -494,6 +506,13 @@ class ImageAdapter(
return notForUploadGroup.visibility == View.VISIBLE
}
/**
* Item is not uploading
*/
fun itemNotUploading() {
uploadingGroup.visibility = View.GONE
}
/**
* Item Not Uploaded view.
*/

View file

@ -93,7 +93,7 @@ class FolderFragment : CommonsDaggerSupportFragment() {
*/
override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? {
_binding = FragmentCustomSelectorBinding.inflate(inflater, container, false)
folderAdapter = FolderAdapter(activity!!, activity as FolderClickListener)
folderAdapter = FolderAdapter(requireActivity(), activity as FolderClickListener)
gridLayoutManager = GridLayoutManager(context, columnCount())
selectorRV = binding?.selectorRv
loader = binding?.loader
@ -159,4 +159,4 @@ class FolderFragment : CommonsDaggerSupportFragment() {
return 2
// todo change column count depending on the orientation of the device.
}
}
}

View file

@ -15,6 +15,8 @@ import androidx.lifecycle.Observer
import androidx.lifecycle.ViewModelProvider
import androidx.recyclerview.widget.GridLayoutManager
import androidx.recyclerview.widget.RecyclerView
import fr.free.nrw.commons.contributions.Contribution
import fr.free.nrw.commons.contributions.ContributionDao
import fr.free.nrw.commons.customselector.database.NotForUploadStatusDao
import fr.free.nrw.commons.customselector.database.UploadedStatusDao
import fr.free.nrw.commons.customselector.helper.ImageHelper
@ -34,6 +36,7 @@ import fr.free.nrw.commons.media.MediaClient
import fr.free.nrw.commons.theme.BaseActivity
import fr.free.nrw.commons.upload.FileProcessor
import fr.free.nrw.commons.upload.FileUtilsWrapper
import io.reactivex.schedulers.Schedulers
import java.util.*
import javax.inject.Inject
@ -134,6 +137,9 @@ class ImageFragment : CommonsDaggerSupportFragment(), RefreshUIListener, PassDat
@Inject
lateinit var mediaClient: MediaClient
@Inject
lateinit var contributionDao: ContributionDao
companion object {
/**
@ -236,7 +242,8 @@ class ImageFragment : CommonsDaggerSupportFragment(), RefreshUIListener, PassDat
editor.apply()
}
imageAdapter.init(allImages, allImages, TreeMap())
val uploadingContributions = getUploadingContributions()
imageAdapter.init(allImages, allImages, TreeMap(), uploadingContributions)
imageAdapter.notifyDataSetChanged()
}
@ -258,10 +265,12 @@ class ImageFragment : CommonsDaggerSupportFragment(), RefreshUIListener, PassDat
private fun handleResult(result: Result) {
if (result.status is CallbackStatus.SUCCESS) {
val images = result.images
val uploadingContributions = getUploadingContributions()
if (images.isNotEmpty()) {
filteredImages = ImageHelper.filterImages(images, bucketId)
allImages = ArrayList(filteredImages)
imageAdapter.init(filteredImages, allImages, TreeMap())
imageAdapter.init(filteredImages, allImages, TreeMap(), uploadingContributions)
selectorRV?.let {
it.visibility = View.VISIBLE
lastItemId?.let { pos ->
@ -336,7 +345,7 @@ class ImageFragment : CommonsDaggerSupportFragment(), RefreshUIListener, PassDat
}
override fun refresh() {
imageAdapter.refresh(filteredImages, allImages)
imageAdapter.refresh(filteredImages, allImages, getUploadingContributions())
}
/**
@ -359,8 +368,10 @@ class ImageFragment : CommonsDaggerSupportFragment(), RefreshUIListener, PassDat
override fun passSelectedImages(selectedImages: ArrayList<Image>, shouldRefresh: Boolean) {
imageAdapter.setSelectedImages(selectedImages)
val uploadingContributions = getUploadingContributions()
if (!showAlreadyActionedImages && shouldRefresh) {
imageAdapter.init(filteredImages, allImages, TreeMap())
imageAdapter.init(filteredImages, allImages, TreeMap(), uploadingContributions)
imageAdapter.setSelectedImages(selectedImages)
}
}
@ -384,4 +395,11 @@ class ImageFragment : CommonsDaggerSupportFragment(), RefreshUIListener, PassDat
}
}
private fun getUploadingContributions(): List<Contribution> {
return contributionDao.getContribution(
listOf(Contribution.STATE_IN_PROGRESS, Contribution.STATE_FAILED, Contribution.STATE_QUEUED, Contribution.STATE_PAUSED)
)?.subscribeOn(Schedulers.io())?.blockingGet() ?: emptyList()
}
}

View file

@ -3,6 +3,7 @@ package fr.free.nrw.commons.customselector.ui.selector
import android.content.Context
import android.content.SharedPreferences
import android.net.Uri
import fr.free.nrw.commons.contributions.Contribution
import fr.free.nrw.commons.customselector.database.NotForUploadStatusDao
import fr.free.nrw.commons.customselector.database.UploadedStatus
import fr.free.nrw.commons.customselector.database.UploadedStatusDao
@ -75,7 +76,8 @@ class ImageLoader @Inject constructor(
holder: ImageViewHolder,
image: Image,
ioDispatcher: CoroutineDispatcher,
defaultDispatcher: CoroutineDispatcher
defaultDispatcher: CoroutineDispatcher,
uploadedContributionsList : List<Contribution>
) {
/**
@ -84,6 +86,7 @@ class ImageLoader @Inject constructor(
mapHolderImage[holder] = image
holder.itemNotUploaded()
holder.itemForUpload()
holder.itemNotUploading()
scope.launch {
var result: Result = Result.NOTFOUND
@ -214,6 +217,17 @@ class ImageLoader @Inject constructor(
holder.itemNotForUpload()
} else holder.itemForUpload()
}
if (uploadedContributionsList.isNotEmpty()) {
for (contribution in uploadedContributionsList ) {
if (contribution.contentUri == image.uri && showAlreadyActionedImages) {
holder.itemUploading()
break
} else {
holder.itemNotUploading()
}
}
}
}
}
@ -223,17 +237,22 @@ class ImageLoader @Inject constructor(
suspend fun nextActionableImage(
allImages: List<Image>, ioDispatcher: CoroutineDispatcher,
defaultDispatcher: CoroutineDispatcher,
nextImagePosition: Int
nextImagePosition: Int,
currentlyUploadingImages: List<Contribution>
): Int {
var next: Int
// Traversing from given position to the end
for (i in nextImagePosition until allImages.size){
val it = allImages[i]
val imageSHA1: String = when (mapImageSHA1[it.uri] != null) {
true -> mapImageSHA1[it.uri]!!
val currentImage = allImages[i]
if (currentlyUploadingImages.any { it.contentUri == currentImage.uri }) {
continue // Skip this image as it's currently being uploaded
}
val imageSHA1: String = when (mapImageSHA1[currentImage.uri] != null) {
true -> mapImageSHA1[currentImage.uri]!!
else -> CustomSelectorUtils.getImageSHA1(
it.uri,
currentImage.uri,
ioDispatcher,
fileUtilsWrapper,
context.contentResolver
@ -253,7 +272,7 @@ class ImageLoader @Inject constructor(
// If the image is not present in the already uploaded table, checks for its
// modified SHA1 in already uploaded table
if (next <= 0) {
val modifiedImageSha1 = getSHA1(it, defaultDispatcher)
val modifiedImageSha1 = getSHA1(currentImage, defaultDispatcher)
next = uploadedStatusDao.findByModifiedImageSHA1(
modifiedImageSha1,
true
@ -360,4 +379,4 @@ class ImageLoader @Inject constructor(
const val INVALIDATE_DAY_COUNT: Long = 7
}
}
}

View file

@ -61,6 +61,13 @@
android:alpha="0.15"
android:background="@color/black"/>
<View
android:id="@+id/uploading_overlay"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:alpha="0.15"
android:background="@color/black"/>
<ImageView
android:id="@+id/uploaded_overlay_icon"
@ -71,6 +78,15 @@
app:srcCompat="@drawable/commons"
/>
<ImageView
android:id="@+id/uploading_overlay_icon"
android:layout_width="@dimen/dimen_50"
android:layout_height="@dimen/dimen_50"
android:rotationX="180"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:srcCompat="@drawable/menu_ic_download_24dp" />
<androidx.constraintlayout.widget.Group
android:id="@+id/uploaded_group"
android:layout_width="wrap_content"
@ -78,6 +94,13 @@
android:visibility="gone"
app:constraint_referenced_ids="uploaded_overlay,uploaded_overlay_icon"/>
<androidx.constraintlayout.widget.Group
android:id="@+id/uploading_group"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:visibility="gone"
app:constraint_referenced_ids="uploading_overlay,uploading_overlay_icon"/>
<ImageView
android:id="@+id/not_for_upload_overlay_icon"
android:layout_width="@dimen/dimen_50"
@ -98,4 +121,4 @@
</androidx.constraintlayout.widget.ConstraintLayout>
</androidx.cardview.widget.CardView>
</androidx.constraintlayout.widget.ConstraintLayout>
</androidx.constraintlayout.widget.ConstraintLayout>

View file

@ -128,8 +128,8 @@ class ImageAdapterTest {
fun processThumbnailForActionedImage() = runBlocking {
Whitebox.setInternalState(imageAdapter, "allImages", listOf(image))
whenever(imageLoader.nextActionableImage(listOf(image), Dispatchers.IO, Dispatchers.Default,
0)).thenReturn(0)
imageAdapter.processThumbnailForActionedImage(holder, 0)
0, emptyList())).thenReturn(0)
imageAdapter.processThumbnailForActionedImage(holder, 0, emptyList())
}
/**
@ -138,8 +138,8 @@ class ImageAdapterTest {
@Test
fun `processThumbnailForActionedImage when reached end of the folder`() = runBlocking {
whenever(imageLoader.nextActionableImage(ArrayList(), Dispatchers.IO, Dispatchers.Default,
0)).thenReturn(-1)
imageAdapter.processThumbnailForActionedImage(holder, 0)
0, emptyList())).thenReturn(-1)
imageAdapter.processThumbnailForActionedImage(holder, 0, emptyList())
}
/**
@ -243,4 +243,4 @@ class ImageAdapterTest {
imageAdapter.init(listOf(image), listOf(image), TreeMap())
Assertions.assertEquals(1, imageAdapter.getImageIdAt(0))
}
}
}

View file

@ -5,6 +5,7 @@ import android.net.Uri
import android.os.Bundle
import fr.free.nrw.commons.OkHttpConnectionFactory
import fr.free.nrw.commons.TestCommonsApplication
import fr.free.nrw.commons.contributions.ContributionDao
import fr.free.nrw.commons.createTestClient
import fr.free.nrw.commons.customselector.model.Image
import fr.free.nrw.commons.customselector.ui.adapter.ImageAdapter
@ -13,6 +14,7 @@ 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.Mock
import org.mockito.Mockito
import org.mockito.MockitoAnnotations
import org.powermock.reflect.Whitebox
@ -39,6 +41,9 @@ class CustomSelectorActivityTest {
private lateinit var image: Image
@Mock
lateinit var contributionDao: ContributionDao
/**
* Set up the tests.
*/
@ -58,6 +63,7 @@ class CustomSelectorActivityTest {
Whitebox.setInternalState(activity, "imageFragment", imageFragment)
Whitebox.setInternalState(imageFragment, "imageAdapter", Mockito.mock(ImageAdapter::class.java))
Whitebox.setInternalState(imageFragment,"contributionDao",contributionDao)
}
/**
@ -276,4 +282,4 @@ class CustomSelectorActivityTest {
assertEquals(false, overLimit.getBoolean(activity))
assertEquals(0, exceededBy.getInt(activity))
}
}
}

View file

@ -18,6 +18,7 @@ import com.nhaarman.mockitokotlin2.whenever
import fr.free.nrw.commons.OkHttpConnectionFactory
import fr.free.nrw.commons.R
import fr.free.nrw.commons.TestCommonsApplication
import fr.free.nrw.commons.contributions.ContributionDao
import fr.free.nrw.commons.createTestClient
import fr.free.nrw.commons.customselector.model.CallbackStatus
import fr.free.nrw.commons.customselector.model.Image
@ -67,6 +68,9 @@ class ImageFragmentTest {
@Mock
private lateinit var savedInstanceState: Bundle
@Mock
lateinit var contributionDao: ContributionDao
/**
* Setup the image fragment.
*/
@ -94,6 +98,7 @@ class ImageFragmentTest {
Whitebox.setInternalState(fragment, "selectorRV", selectorRV )
Whitebox.setInternalState(fragment, "loader", loader)
Whitebox.setInternalState(fragment, "filteredImages", arrayListOf(image,image))
Whitebox.setInternalState(fragment, "contributionDao", contributionDao)
viewModelField = fragment.javaClass.getDeclaredField("viewModel")
viewModelField.isAccessible = true
@ -180,4 +185,4 @@ class ImageFragmentTest {
func.invoke(fragment)
}
}
}

View file

@ -159,10 +159,10 @@ class ImageLoaderTest {
.thenReturn(Mockito.mock(SharedPreferences::class.java))
mapResult["testSha1"] = ImageLoader.Result.TRUE
imageLoader.queryAndSetView(holder, image, testDispacher, testDispacher)
imageLoader.queryAndSetView(holder, image, testDispacher, testDispacher, ArrayList())
mapResult["testSha1"] = ImageLoader.Result.FALSE
imageLoader.queryAndSetView(holder, image, testDispacher, testDispacher)
imageLoader.queryAndSetView(holder, image, testDispacher, testDispacher, ArrayList())
}
/**
@ -174,7 +174,7 @@ class ImageLoaderTest {
whenever(notForUploadStatusDao.find(any())).thenReturn(0)
whenever(context.getSharedPreferences("custom_selector", 0))
.thenReturn(Mockito.mock(SharedPreferences::class.java))
imageLoader.queryAndSetView(holder, image, testDispacher, testDispacher)
imageLoader.queryAndSetView(holder, image, testDispacher, testDispacher, ArrayList())
}
/**
@ -193,16 +193,16 @@ class ImageLoaderTest {
whenever(PickedFiles.pickedExistingPicture(context, Uri.parse("test"))).thenReturn(
uploadableFile
)
imageLoader.nextActionableImage(listOf(image), testDispacher, testDispacher, 0)
imageLoader.nextActionableImage(listOf(image), testDispacher, testDispacher, 0, emptyList())
whenever(notForUploadStatusDao.find(any())).thenReturn(1)
imageLoader.nextActionableImage(listOf(image), testDispacher, testDispacher, 0)
imageLoader.nextActionableImage(listOf(image), testDispacher, testDispacher, 0, emptyList())
whenever(uploadedStatusDao.findByImageSHA1(any(), any())).thenReturn(2)
imageLoader.nextActionableImage(listOf(image), testDispacher, testDispacher, 0)
imageLoader.nextActionableImage(listOf(image), testDispacher, testDispacher, 0, emptyList())
whenever(uploadedStatusDao.findByModifiedImageSHA1(any(), any())).thenReturn(2)
imageLoader.nextActionableImage(listOf(image), testDispacher, testDispacher, 0)
imageLoader.nextActionableImage(listOf(image), testDispacher, testDispacher, 0, emptyList())
}
/**
@ -249,4 +249,4 @@ class ImageLoaderTest {
imageLoader.getResultFromUploadedStatus(uploadedStatus))
}
}
}