diff --git a/.github/ISSUE_TEMPLATE/bug-report.yml b/.github/ISSUE_TEMPLATE/bug-report.yml index dcbba0597..a4682fd3c 100644 --- a/.github/ISSUE_TEMPLATE/bug-report.yml +++ b/.github/ISSUE_TEMPLATE/bug-report.yml @@ -70,7 +70,7 @@ body: required: false - type: textarea attributes: - label: Screenshots + label: Screen-shots description: Add screenshots related to the issue (if available). Can be created by pressing the Volume Down and Power Button at the same time on Android 4.0 and higher. validations: required: false diff --git a/app/src/main/java/fr/free/nrw/commons/customselector/ui/selector/ImageFragment.kt b/app/src/main/java/fr/free/nrw/commons/customselector/ui/selector/ImageFragment.kt index db250dea9..4f37106cc 100644 --- a/app/src/main/java/fr/free/nrw/commons/customselector/ui/selector/ImageFragment.kt +++ b/app/src/main/java/fr/free/nrw/commons/customselector/ui/selector/ImageFragment.kt @@ -47,7 +47,6 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.launch -import timber.log.Timber import java.util.TreeMap import javax.inject.Inject import kotlin.collections.ArrayList @@ -212,12 +211,8 @@ class ImageFragment : savedInstanceState: Bundle?, ): View? { _binding = FragmentCustomSelectorBinding.inflate(inflater, container, false) - - // ensures imageAdapter is initialized - if (!::imageAdapter.isInitialized) { - imageAdapter = ImageAdapter(requireActivity(), activity as ImageSelectListener, imageLoader!!) - Timber.d("Initialized imageAdapter in onCreateView") - } + imageAdapter = + ImageAdapter(requireActivity(), activity as ImageSelectListener, imageLoader!!) // Set single selection mode if needed val singleSelection = (activity as? CustomSelectorActivity)?.intent?.getBooleanExtra(CustomSelectorActivity.EXTRA_SINGLE_SELECTION, false) == true imageAdapter.setSingleSelection(singleSelection) @@ -375,12 +370,7 @@ class ImageFragment : * notifyDataSetChanged, rebuild the holder views to account for deleted images. */ override fun onResume() { - if (::imageAdapter.isInitialized) { - imageAdapter.notifyDataSetChanged() - Timber.d("Notified imageAdapter in onResume") - } else { - Timber.w("imageAdapter not initialized in onResume") - } + imageAdapter.notifyDataSetChanged() super.onResume() } @@ -390,19 +380,14 @@ class ImageFragment : * Save the Image Fragment state. */ override fun onDestroy() { - if (::imageAdapter.isInitialized) { - imageAdapter.cleanUp() - Timber.d("Cleaned up imageAdapter in onDestroy") - } else { - Timber.w("imageAdapter not initialized in onDestroy, skipping cleanup") - } + imageAdapter.cleanUp() val position = - (selectorRV?.layoutManager as? GridLayoutManager) - ?.findFirstVisibleItemPosition() ?: -1 + (selectorRV?.layoutManager as GridLayoutManager) + .findFirstVisibleItemPosition() - // check for valid position and non-empty image list - if (position != -1 && filteredImages.isNotEmpty() && ::imageAdapter.isInitialized) { + // Check for empty RecyclerView. + if (position != -1 && filteredImages.size > 0) { context?.let { context -> context .getSharedPreferences( @@ -411,57 +396,34 @@ class ImageFragment : )?.let { prefs -> prefs.edit()?.let { editor -> editor.putLong("ItemId", imageAdapter.getImageIdAt(position))?.apply() - Timber.d("Saved last visible item ID: %d", imageAdapter.getImageIdAt(position)) } } } - } else { - Timber.d("Skipped saving item ID: position=%d, filteredImages.size=%d, imageAdapter initialized=%b", - position, filteredImages.size, ::imageAdapter.isInitialized) } super.onDestroy() } override fun onDestroyView() { _binding = null - selectorRV = null - loader = null - switch = null - progressLayout = null super.onDestroyView() } override fun refresh() { - if (::imageAdapter.isInitialized) { - imageAdapter.refresh(filteredImages, allImages, getUploadingContributions()) - Timber.d("Refreshed imageAdapter") - } else { - Timber.w("imageAdapter not initialized in refresh") - } + imageAdapter.refresh(filteredImages, allImages, getUploadingContributions()) } /** * Removes the image from the actionable image map */ fun removeImage(image: Image) { - if (::imageAdapter.isInitialized) { - imageAdapter.removeImageFromActionableImageMap(image) - Timber.d("Removed image from actionable image map") - } else { - Timber.w("imageAdapter not initialized in removeImage") - } + imageAdapter.removeImageFromActionableImageMap(image) } /** * Clears the selected images */ fun clearSelectedImages() { - if (::imageAdapter.isInitialized) { - imageAdapter.clearSelectedImages() - Timber.d("Cleared selected images") - } else { - Timber.w("imageAdapter not initialized in clearSelectedImages") - } + imageAdapter.clearSelectedImages() } /** @@ -472,15 +434,6 @@ class ImageFragment : selectedImages: ArrayList, shouldRefresh: Boolean, ) { - if (::imageAdapter.isInitialized) { - imageAdapter.setSelectedImages(selectedImages) - if (shouldRefresh) { - imageAdapter.refresh(filteredImages, allImages, getUploadingContributions()) - } - Timber.d("Passed %d selected images to imageAdapter, shouldRefresh=%b", selectedImages.size, shouldRefresh) - } else { - Timber.w("imageAdapter not initialized in passSelectedImages") - } } /** @@ -490,7 +443,6 @@ class ImageFragment : if (!progressDialog.isShowing) { progressDialogLayout.progressDialogText.text = text progressDialog.show() - Timber.d("Showing mark/unmark progress dialog: %s", text) } } @@ -500,7 +452,6 @@ class ImageFragment : fun dismissMarkUnmarkProgressDialog() { if (progressDialog.isShowing) { progressDialog.dismiss() - Timber.d("Dismissed mark/unmark progress dialog") } } @@ -510,4 +461,4 @@ class ImageFragment : listOf(Contribution.STATE_IN_PROGRESS, Contribution.STATE_FAILED, Contribution.STATE_QUEUED, Contribution.STATE_PAUSED), )?.subscribeOn(Schedulers.io()) ?.blockingGet() ?: emptyList() -} \ No newline at end of file +} diff --git a/app/src/main/java/fr/free/nrw/commons/explore/map/ExploreMapFragment.kt b/app/src/main/java/fr/free/nrw/commons/explore/map/ExploreMapFragment.kt index cad6bd057..0fc95dd17 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/map/ExploreMapFragment.kt +++ b/app/src/main/java/fr/free/nrw/commons/explore/map/ExploreMapFragment.kt @@ -287,8 +287,6 @@ class ExploreMapFragment : CommonsDaggerSupportFragment(), ExploreMapContract.Vi super.onPause() // unregistering the broadcastReceiver, as it was causing an exception and a potential crash unregisterNetworkReceiver() - locationManager.unregisterLocationManager() - locationManager.removeLocationListener(this) } fun requestLocationIfNeeded() { diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.kt b/app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.kt index 799d5b0f1..c2bed5fff 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.kt @@ -508,17 +508,24 @@ class UploadActivity : BaseActivity(), UploadContract.View, UploadBaseFragment.C fragments = mutableListOf() } + for (uploadableFile in uploadableFiles) { val uploadMediaDetailFragment = UploadMediaDetailFragment() - // set fragment properties but defer initialization - uploadMediaDetailFragment.uploadableFile = uploadableFile - uploadMediaDetailFragment.place = place - uploadMediaDetailFragment.inAppPictureLocation = if (!uploadIsOfAPlace) { + if (!uploadIsOfAPlace) { handleLocation() - currLocation + uploadMediaDetailFragment.setImageToBeUploaded( + uploadableFile, + place, + currLocation + ) + locationManager!!.unregisterLocationManager() } else { - currLocation + uploadMediaDetailFragment.setImageToBeUploaded( + uploadableFile, + place, + currLocation + ) } val uploadMediaDetailFragmentCallback: UploadMediaDetailFragmentCallback = @@ -573,19 +580,13 @@ class UploadActivity : BaseActivity(), UploadContract.View, UploadBaseFragment.C if (isFragmentsSaved) { val fragment = fragments!![0] as UploadMediaDetailFragment? fragment!!.fragmentCallback = uploadMediaDetailFragmentCallback - fragment.initializeFragment() } else { uploadMediaDetailFragment.fragmentCallback = uploadMediaDetailFragmentCallback fragments!!.add(uploadMediaDetailFragment) } } - // unregister location manager after loop if needed - if (!uploadIsOfAPlace) { - locationManager!!.unregisterLocationManager() - } - - // If fragments are not created, create them and add them to the fragments ArrayList + //If fragments are not created, create them and add them to the fragments ArrayList if (!isFragmentsSaved) { uploadCategoriesFragment = UploadCategoriesFragment() if (place != null) { diff --git a/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragment.kt b/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragment.kt index acf55670e..6ece53170 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragment.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragment.kt @@ -119,8 +119,8 @@ class UploadMediaDetailFragment : UploadBaseFragment(), UploadMediaDetailsContra private var basicKvStore: BasicKvStore? = null private val keyForShowingAlertDialog = "isNoNetworkAlertDialogShowing" - internal var uploadableFile: UploadableFile? = null - internal var place: Place? = null + private var uploadableFile: UploadableFile? = null + private var place: Place? = null private lateinit var uploadMediaDetailAdapter: UploadMediaDetailAdapter var indexOfFragment = 0 var isExpanded = true @@ -142,24 +142,19 @@ class UploadMediaDetailFragment : UploadBaseFragment(), UploadMediaDetailsContra } } + fun setImageToBeUploaded( + uploadableFile: UploadableFile?, place: Place?, inAppPictureLocation: LatLng? + ) { + this.uploadableFile = uploadableFile + this.place = place + this.inAppPictureLocation = inAppPictureLocation + } + override fun onCreateView( inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle? ): View { _binding = FragmentUploadMediaDetailFragmentBinding.inflate(inflater, container, false) _binding!!.mediaDetailCardView.handleKeyboardInsets() - // intialise the adapter early to prevent uninitialized access - uploadMediaDetailAdapter = UploadMediaDetailAdapter( - this, - defaultKvStore.getString(Prefs.DESCRIPTION_LANGUAGE, "")!!, - recentLanguagesDao, voiceInputResultLauncher - ) - uploadMediaDetailAdapter.callback = - UploadMediaDetailAdapter.Callback { titleStringID: Int, messageStringId: Int -> - showInfoAlert(titleStringID, messageStringId) - } - uploadMediaDetailAdapter.eventListener = this - binding.rvDescriptions.layoutManager = LinearLayoutManager(context) - binding.rvDescriptions.adapter = uploadMediaDetailAdapter return binding.root } @@ -168,48 +163,20 @@ class UploadMediaDetailFragment : UploadBaseFragment(), UploadMediaDetailsContra basicKvStore = BasicKvStore(requireActivity(), "CurrentUploadImageQualities") - // restore adapter items from savedInstanceState if available - if (savedInstanceState != null) { - val savedItems = savedInstanceState.getParcelableArrayList(UPLOAD_MEDIA_DETAILS) - 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") - } - } else { - // initialize with a default UploadMediaDetail if saved state is empty or null - uploadMediaDetailAdapter.items = mutableListOf(UploadMediaDetail()) - Timber.d("Initialized default UploadMediaDetail due to empty or null savedItems") - } - } else { - // intitialise with a default UploadMediaDetail for fresh fragment - if (uploadMediaDetailAdapter.items.isEmpty()) { - uploadMediaDetailAdapter.items = mutableListOf(UploadMediaDetail()) - Timber.d("Initialized default UploadMediaDetail for new fragment") - } - } - if (fragmentCallback != null) { indexOfFragment = fragmentCallback!!.getIndexInViewFlipper(this) - Timber.d("Fragment callback present, indexOfFragment = %d", indexOfFragment) initializeFragment() - } else { - Timber.w("Fragment callback is null, skipping initializeFragment") + } + + if (savedInstanceState != null) { + if (uploadMediaDetailAdapter.items.isEmpty() && fragmentCallback != null) { + uploadMediaDetailAdapter.items = savedInstanceState.getParcelableArrayList(UPLOAD_MEDIA_DETAILS)!! + presenter.setUploadMediaDetails(uploadMediaDetailAdapter.items, indexOfFragment) + } } try { - if (indexOfFragment >= 0 && !presenter.getImageQuality(indexOfFragment, inAppPictureLocation, requireActivity())) { - Timber.d("Image quality check failed, redirecting to MainActivity") + if (!presenter.getImageQuality(indexOfFragment, inAppPictureLocation, requireActivity())) { startActivityWithFlags( requireActivity(), MainActivity::class.java, @@ -217,12 +184,11 @@ class UploadMediaDetailFragment : UploadBaseFragment(), UploadMediaDetailsContra Intent.FLAG_ACTIVITY_SINGLE_TOP ) } - } catch (e: Exception) { - Timber.e(e, "Error during image quality check") + } catch (_: Exception) { } } - internal fun initializeFragment() { + private fun initializeFragment() { if (_binding == null) { return } @@ -240,6 +206,7 @@ class UploadMediaDetailFragment : UploadBaseFragment(), UploadMediaDetailsContra presenter.setupBasicKvStoreFactory { BasicKvStore(requireActivity(), it) } presenter.receiveImage(uploadableFile, place, inAppPictureLocation) + initRecyclerView() with (binding){ if (indexOfFragment == 0) { @@ -298,6 +265,24 @@ class UploadMediaDetailFragment : UploadBaseFragment(), UploadMediaDetailsContra } } + /** + * init the description recycler veiw and caption recyclerview + */ + private fun initRecyclerView() { + uploadMediaDetailAdapter = UploadMediaDetailAdapter( + this, + defaultKvStore.getString(Prefs.DESCRIPTION_LANGUAGE, "")!!, + recentLanguagesDao, voiceInputResultLauncher + ) + uploadMediaDetailAdapter.callback = + UploadMediaDetailAdapter.Callback { titleStringID: Int, messageStringId: Int -> + showInfoAlert(titleStringID, messageStringId) + } + uploadMediaDetailAdapter.eventListener = this + binding.rvDescriptions.layoutManager = LinearLayoutManager(context) + binding.rvDescriptions.adapter = uploadMediaDetailAdapter + } + private fun showInfoAlert(titleStringID: Int, messageStringId: Int) { showAlertDialog( requireActivity(), @@ -605,14 +590,16 @@ class UploadMediaDetailFragment : UploadBaseFragment(), UploadMediaDetailsContra var defaultLongitude = -122.431297 var defaultZoom = 16.0 + val locationPickerIntent: Intent + /* Retrieve image location from EXIF if present or check if user has provided location while using the in-app camera. Use location of last UploadItem if none of them is available */ - val locationPickerIntent: Intent if (uploadItem.gpsCoords != null && uploadItem.gpsCoords!! .decLatitude != 0.0 && uploadItem.gpsCoords!!.decLongitude != 0.0 ) { - defaultLatitude = uploadItem.gpsCoords!!.decLatitude + defaultLatitude = uploadItem.gpsCoords!! + .decLatitude defaultLongitude = uploadItem.gpsCoords!!.decLongitude defaultZoom = uploadItem.gpsCoords!!.zoomLevel @@ -628,7 +615,8 @@ class UploadMediaDetailFragment : UploadBaseFragment(), UploadMediaDetailsContra defaultLongitude = locationLatLng[1].toDouble() } if (defaultKvStore.getString(LAST_ZOOM) != null) { - defaultZoom = defaultKvStore.getString(LAST_ZOOM)!!.toDouble() + defaultZoom = defaultKvStore.getString(LAST_ZOOM)!! + .toDouble() } locationPickerIntent = LocationPicker.IntentBuilder() @@ -919,4 +907,4 @@ class UploadMediaDetailFragment : UploadBaseFragment(), UploadMediaDetailsContra const val UPLOADABLE_FILE: String = "uploadable_file" const val UPLOAD_MEDIA_DETAILS: String = "upload_media_detail_adapter" } -} \ No newline at end of file +} diff --git a/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaPresenter.kt b/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaPresenter.kt index 6e26e02a6..4d565adb2 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaPresenter.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaPresenter.kt @@ -69,18 +69,7 @@ class UploadMediaPresenter @Inject constructor( uploadMediaDetails: List, uploadItemIndex: Int ) { - val uploadItems = repository.getUploads() - if (uploadItemIndex >= 0 && uploadItemIndex < uploadItems.size) { - if (uploadMediaDetails.isNotEmpty()) { - uploadItems[uploadItemIndex].uploadMediaDetails = uploadMediaDetails.toMutableList() - Timber.d("Set uploadMediaDetails for index %d, size %d", uploadItemIndex, uploadMediaDetails.size) - } else { - uploadItems[uploadItemIndex].uploadMediaDetails = mutableListOf(UploadMediaDetail()) - Timber.w("Received empty uploadMediaDetails for index %d, initialized default", uploadItemIndex) - } - } else { - Timber.e("Invalid index %d for uploadItems size %d, skipping setUploadMediaDetails", uploadItemIndex, uploadItems.size) - } + repository.getUploads()[uploadItemIndex].uploadMediaDetails = uploadMediaDetails.toMutableList() } override fun setupBasicKvStoreFactory(factory: (String) -> BasicKvStore) { 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 0cab47c67..a37bcc927 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 @@ -145,8 +145,6 @@ class UploadMediaDetailFragmentUnitTest { ibExpandCollapse = view.findViewById(R.id.ib_expand_collapse) Whitebox.setInternalState(fragment, "uploadMediaDetailAdapter", uploadMediaDetailAdapter) - Whitebox.setInternalState(fragment, "defaultKvStore", defaultKvStore) - `when`(defaultKvStore.getString("description_language", "")).thenReturn("en") } @Test @@ -164,8 +162,14 @@ class UploadMediaDetailFragmentUnitTest { @Test @Throws(Exception::class) - fun testOnCreateView() { + fun testSetImageToBeUploaded() { Shadows.shadowOf(Looper.getMainLooper()).idle() + fragment.setImageToBeUploaded(null, null, location) + } + + @Test + @Throws(Exception::class) + fun testOnCreateView() { fragment.onCreateView(layoutInflater, null, savedInstanceState) } @@ -177,6 +181,34 @@ class UploadMediaDetailFragmentUnitTest { fragment.onViewCreated(view, savedInstanceState) } + @Test + @Throws(Exception::class) + fun testInit() { + Shadows.shadowOf(Looper.getMainLooper()).idle() + Whitebox.setInternalState(fragment, "presenter", presenter) + val method: Method = + UploadMediaDetailFragment::class.java.getDeclaredMethod( + "initializeFragment", + ) + method.isAccessible = true + method.invoke(fragment) + } + + @Test + @Throws(Exception::class) + fun testInitCaseGetIndexInViewFlipperNonZero() { + Shadows.shadowOf(Looper.getMainLooper()).idle() + Whitebox.setInternalState(fragment, "presenter", presenter) + `when`(callback.getIndexInViewFlipper(fragment)).thenReturn(1) + `when`(callback.totalNumberOfSteps).thenReturn(5) + val method: Method = + UploadMediaDetailFragment::class.java.getDeclaredMethod( + "initializeFragment", + ) + method.isAccessible = true + method.invoke(fragment) + } + @Test @Throws(Exception::class) fun testShowInfoAlert() { @@ -285,7 +317,7 @@ class UploadMediaDetailFragmentUnitTest { @Test @Throws(Exception::class) fun testShowExternalMap() { - Shadows.shadowOf(Looper.getMainLooper()).idle() + shadowOf(Looper.getMainLooper()).idle() `when`(uploadItem.gpsCoords).thenReturn(imageCoordinates) `when`(imageCoordinates.decLatitude).thenReturn(0.0) `when`(imageCoordinates.decLongitude).thenReturn(0.0) @@ -296,7 +328,7 @@ class UploadMediaDetailFragmentUnitTest { @Test @Throws(Exception::class) fun testOnCameraPositionCallbackOnMapIconClicked() { - Shadows.shadowOf(Looper.getMainLooper()).idle() + shadowOf(Looper.getMainLooper()).idle() Mockito.mock(LocationPicker::class.java) val intent = Mockito.mock(Intent::class.java) val cameraPosition = Mockito.mock(CameraPosition::class.java) @@ -325,7 +357,7 @@ class UploadMediaDetailFragmentUnitTest { @Test @Throws(Exception::class) fun testOnCameraPositionCallbackAddLocationDialog() { - Shadows.shadowOf(Looper.getMainLooper()).idle() + shadowOf(Looper.getMainLooper()).idle() Mockito.mock(LocationPicker::class.java) val intent = Mockito.mock(Intent::class.java) val cameraPosition = Mockito.mock(CameraPosition::class.java) @@ -395,7 +427,7 @@ class UploadMediaDetailFragmentUnitTest { @Test @Throws(Exception::class) fun testRememberedZoomLevelOnNull() { - Shadows.shadowOf(Looper.getMainLooper()).idle() + shadowOf(Looper.getMainLooper()).idle() Whitebox.setInternalState(fragment, "defaultKvStore", defaultKvStore) `when`(uploadItem.gpsCoords).thenReturn(null) `when`(defaultKvStore.getString(LAST_ZOOM)).thenReturn("13.0") @@ -411,7 +443,7 @@ class UploadMediaDetailFragmentUnitTest { @Test @Throws(Exception::class) fun testRememberedZoomLevelOnNotNull() { - Shadows.shadowOf(Looper.getMainLooper()).idle() + shadowOf(Looper.getMainLooper()).idle() `when`(uploadItem.gpsCoords).thenReturn(imageCoordinates) `when`(imageCoordinates.decLatitude).thenReturn(8.0) `when`(imageCoordinates.decLongitude).thenReturn(-8.0) @@ -424,4 +456,4 @@ class UploadMediaDetailFragmentUnitTest { val shadowIntent: ShadowIntent = shadowOf(startedIntent) Assert.assertEquals(shadowIntent.intentClass, LocationPickerActivity::class.java) } -} \ No newline at end of file +}