.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

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")
-        }
         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")
-        }
 
         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")
-        }
     }
 
     /**
      * 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")
-        }
     }
 
     /**
      * Clears the selected images
      */
     fun clearSelectedImages() {
-        if (::imageAdapter.isInitialized) {
         imageAdapter.clearSelectedImages()
-            Timber.d("Cleared selected images")
-        } else {
-            Timber.w("imageAdapter not initialized in clearSelectedImages")
-        }
     }
 
     /**
@@ -472,15 +434,6 @@ class ImageFragment :
         selectedImages: ArrayList<Image>,
         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")
         }
     }
 

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() {

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()
+                    uploadMediaDetailFragment.setImageToBeUploaded(
+                        uploadableFile,
+                        place,
                         currLocation
+                    )
+                    locationManager!!.unregisterLocationManager()
                 } else {
+                    uploadMediaDetailFragment.setImageToBeUploaded(
+                        uploadableFile,
+                        place,
                         currLocation
+                    )
                 }
 
                 val uploadMediaDetailFragmentCallback: UploadMediaDetailFragmentCallback =
@@ -573,18 +580,12 @@ 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 (!isFragmentsSaved) {
                 uploadCategoriesFragment = UploadCategoriesFragment()

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<UploadMediaDetail>(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")
-            }
+            initializeFragment()
         }
 
-        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()

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

@@ -69,18 +69,7 @@ class UploadMediaPresenter @Inject constructor(
         uploadMediaDetails: List<UploadMediaDetail>,
         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) {

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)