From 8352bc59255ef94c26d5b1c5dbafb601520fa71a Mon Sep 17 00:00:00 2001 From: Kota-Jagadeesh Date: Sun, 19 Oct 2025 23:01:40 +0530 Subject: [PATCH 01/10] fix:gracefully handle the missing presenter during configuration change --- .../commons/upload/PendingUploadsFragment.kt | 95 ++++++++++--------- 1 file changed, 52 insertions(+), 43 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsFragment.kt b/app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsFragment.kt index 45e9d90a3e..1737d08f92 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsFragment.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsFragment.kt @@ -28,8 +28,11 @@ class PendingUploadsFragment : CommonsDaggerSupportFragment(), PendingUploadsContract.View, PendingUploadsAdapter.Callback { + + //fix:public and nullable to allow the dagger injection and prevent crash on the rotation. @Inject - lateinit var pendingUploadsPresenter: PendingUploadsPresenter + @JvmField + var pendingUploadsPresenter: PendingUploadsPresenter? = null private lateinit var binding: FragmentPendingUploadsBinding @@ -55,15 +58,11 @@ class PendingUploadsFragment : ): View { super.onCreate(savedInstanceState) binding = FragmentPendingUploadsBinding.inflate(inflater, container, false) - pendingUploadsPresenter.onAttachView(this) + pendingUploadsPresenter?.onAttachView(this) initAdapter() return binding.root } - fun initAdapter() { - adapter = PendingUploadsAdapter(this) - } - override fun onViewCreated( view: View, savedInstanceState: Bundle?, @@ -72,47 +71,51 @@ class PendingUploadsFragment : initRecyclerView() } + fun initAdapter() { + adapter = PendingUploadsAdapter(this) + } + /** * Initializes the recycler view. */ private fun initRecyclerView() { binding.pendingUploadsRecyclerView.setLayoutManager(LinearLayoutManager(this.context)) binding.pendingUploadsRecyclerView.adapter = adapter - pendingUploadsPresenter.setup() - pendingUploadsPresenter.totalContributionList - .observe(viewLifecycleOwner) { list: PagedList -> - contributionsSize = list.size - contributionsList = mutableListOf() - var pausedOrQueuedUploads = 0 - list.forEach { - if (it != null) { - if (it.state == STATE_PAUSED || - it.state == STATE_QUEUED || - it.state == STATE_IN_PROGRESS - ) { - contributionsList.add(it) - } - if (it.state == STATE_PAUSED || it.state == STATE_QUEUED) { - pausedOrQueuedUploads++ + pendingUploadsPresenter?.setup() + pendingUploadsPresenter?.totalContributionList + ?.observe(viewLifecycleOwner) { list: PagedList -> + contributionsSize = list.size + contributionsList = mutableListOf() + var pausedOrQueuedUploads = 0 + list.forEach { + if (it != null) { + if (it.state == STATE_PAUSED || + it.state == STATE_QUEUED || + it.state == STATE_IN_PROGRESS + ) { + contributionsList.add(it) + } + if (it.state == STATE_PAUSED || it.state == STATE_QUEUED) { + pausedOrQueuedUploads++ + } } } - } - if (contributionsSize == 0) { - binding.nopendingTextView.visibility = View.VISIBLE - binding.pendingUplaodsLl.visibility = View.GONE - uploadProgressActivity.hidePendingIcons() - } else { - binding.nopendingTextView.visibility = View.GONE - binding.pendingUplaodsLl.visibility = View.VISIBLE - adapter.submitList(list) - binding.progressTextView.setText("$contributionsSize uploads left") - if ((pausedOrQueuedUploads == contributionsSize) || CommonsApplication.isPaused) { - uploadProgressActivity.setPausedIcon(true) + if (contributionsSize == 0) { + binding.nopendingTextView.visibility = View.VISIBLE + binding.pendingUplaodsLl.visibility = View.GONE + uploadProgressActivity.hidePendingIcons() } else { - uploadProgressActivity.setPausedIcon(false) + binding.nopendingTextView.visibility = View.GONE + binding.pendingUplaodsLl.visibility = View.VISIBLE + adapter.submitList(list) + binding.progressTextView.setText("$contributionsSize uploads left") + if ((pausedOrQueuedUploads == contributionsSize) || CommonsApplication.isPaused) { + uploadProgressActivity.setPausedIcon(true) + } else { + uploadProgressActivity.setPausedIcon(false) + } } } - } } /** @@ -129,7 +132,8 @@ class PendingUploadsFragment : String.format(locale, activity.getString(R.string.no)), { ViewUtil.showShortToast(context, R.string.cancelling_upload) - pendingUploadsPresenter.deleteUpload( + // uses the safe call directly + pendingUploadsPresenter?.deleteUpload( contribution, requireContext().applicationContext, ) }, @@ -140,19 +144,24 @@ class PendingUploadsFragment : /** * Restarts all the paused uploads. */ - fun restartUploads() = pendingUploadsPresenter.restartUploads( - contributionsList, 0, requireContext().applicationContext - ) + fun restartUploads() { + pendingUploadsPresenter?.restartUploads( + contributionsList, 0, requireContext().applicationContext + ) + } /** * Pauses all the ongoing uploads. */ - fun pauseUploads() = pendingUploadsPresenter.pauseUploads() + fun pauseUploads() { pendingUploadsPresenter?.pauseUploads() + } /** * Cancels all the uploads after getting a confirmation from the user using Dialog. */ fun deleteUploads() { + pendingUploadsPresenter ?: return + val activity = requireActivity() val locale = Locale.getDefault() showAlertDialog( @@ -164,7 +173,7 @@ class PendingUploadsFragment : { ViewUtil.showShortToast(context, R.string.cancelling_upload) uploadProgressActivity.hidePendingIcons() - pendingUploadsPresenter.deleteUploads( + pendingUploadsPresenter?.deleteUploads( listOf( STATE_QUEUED, STATE_IN_PROGRESS, @@ -175,4 +184,4 @@ class PendingUploadsFragment : {}, ) } -} +} \ No newline at end of file From 38cd528fc7cc054faa2236749acb5f24bf9ddbdd Mon Sep 17 00:00:00 2001 From: Kota-Jagadeesh Date: Sun, 19 Oct 2025 23:02:21 +0530 Subject: [PATCH 02/10] fix:safely retrieve fragments to prevent stale references after rotation --- .../commons/upload/UploadProgressActivity.kt | 45 +++++++++++++------ 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt b/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt index 665f106e22..08b1c5d665 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt @@ -21,8 +21,6 @@ import javax.inject.Inject */ class UploadProgressActivity : BaseActivity() { private lateinit var binding: ActivityUploadProgressBinding - private var pendingUploadsFragment: PendingUploadsFragment? = null - private var failedUploadsFragment: FailedUploadsFragment? = null var viewPagerAdapter: ViewPagerAdapter? = null var menu: Menu? = null @@ -68,18 +66,37 @@ class UploadProgressActivity : BaseActivity() { setTabs() } + private fun getPendingUploadsFragment(): PendingUploadsFragment? { + return supportFragmentManager.findFragmentByTag( + "android:switcher:${R.id.upload_progress_view_pager}:${0}", + ) as? PendingUploadsFragment + } + + + // the helper to retrieve the current, non-stale FailedUploadsFragment instance. + + private fun getFailedUploadsFragment(): FailedUploadsFragment? { + return supportFragmentManager.findFragmentByTag( + "android:switcher:${R.id.upload_progress_view_pager}:${1}", + ) as? FailedUploadsFragment + } + /** * Initializes and sets up the tabs data by creating instances of `PendingUploadsFragment` * and `FailedUploadsFragment`, adds them to the `fragmentList`, and assigns corresponding * titles from resources to the `titleList`. */ fun setTabs() { - pendingUploadsFragment = PendingUploadsFragment() - failedUploadsFragment = FailedUploadsFragment() + val pendingUploadsFragment: Fragment + val failedUploadsFragment: Fragment + + // checks if the fragmentManager already has the fragments (after the rotation) + pendingUploadsFragment = getPendingUploadsFragment() ?: PendingUploadsFragment() + failedUploadsFragment = getFailedUploadsFragment() ?: FailedUploadsFragment() viewPagerAdapter!!.setTabs( - R.string.pending to pendingUploadsFragment!!, - R.string.failed to failedUploadsFragment!! + R.string.pending to pendingUploadsFragment, + R.string.failed to failedUploadsFragment ) viewPagerAdapter!!.notifyDataSetChanged() } @@ -119,7 +136,8 @@ class UploadProgressActivity : BaseActivity() { getString(R.string.pause), ).setIcon(R.drawable.pause_icon) .setOnMenuItemClickListener { - pendingUploadsFragment!!.pauseUploads() + //retrieves the current fragment instance just before the use + getPendingUploadsFragment()?.pauseUploads() setPausedIcon(true) true }.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM) @@ -133,7 +151,8 @@ class UploadProgressActivity : BaseActivity() { getString(R.string.cancel), ).setIcon(R.drawable.ic_cancel_upload) .setOnMenuItemClickListener { - pendingUploadsFragment!!.deleteUploads() + //retrieve the current fragment instance just before thee use + getPendingUploadsFragment()?.deleteUploads() true }.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM) } @@ -147,7 +166,8 @@ class UploadProgressActivity : BaseActivity() { getString(R.string.resume), ).setIcon(R.drawable.play_icon) .setOnMenuItemClickListener { - pendingUploadsFragment!!.restartUploads() + //retrieve the current fragment instance just before the use + getPendingUploadsFragment()?.restartUploads() setPausedIcon(false) true }.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM) @@ -160,8 +180,7 @@ class UploadProgressActivity : BaseActivity() { menu!! .add(Menu.NONE, R.id.retry_icon, Menu.NONE, getString(R.string.retry)) .setIcon(R.drawable.ic_refresh_24dp) - .setOnMenuItemClickListener { - failedUploadsFragment!!.restartUploads() + .setOnMenuItemClickListener { getFailedUploadsFragment()?.restartUploads() true }.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM) } @@ -174,7 +193,7 @@ class UploadProgressActivity : BaseActivity() { getString(R.string.cancel), ).setIcon(R.drawable.ic_cancel_upload) .setOnMenuItemClickListener { - failedUploadsFragment!!.deleteUploads() + getFailedUploadsFragment()?.deleteUploads() true }.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM) } @@ -208,4 +227,4 @@ class UploadProgressActivity : BaseActivity() { isErrorIconsVisisble = visible updateMenuItems(binding.uploadProgressViewPager.currentItem) } -} +} \ No newline at end of file From 8f66028db6f63d00d1764d5c43e0c4374f0c3209 Mon Sep 17 00:00:00 2001 From: Kota-Jagadeesh Date: Sun, 9 Nov 2025 11:55:32 +0530 Subject: [PATCH 03/10] Fix: Use safe calls to prevent NullPointerException on upload menu actions --- .../commons/upload/UploadProgressActivity.kt | 44 ++++++------------- 1 file changed, 13 insertions(+), 31 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt b/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt index 08b1c5d665..d1ae55996f 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt @@ -21,6 +21,9 @@ import javax.inject.Inject */ class UploadProgressActivity : BaseActivity() { private lateinit var binding: ActivityUploadProgressBinding + // REVERT: added back the private fields to hold fragment instances + private var pendingUploadsFragment: PendingUploadsFragment? = null + private var failedUploadsFragment: FailedUploadsFragment? = null var viewPagerAdapter: ViewPagerAdapter? = null var menu: Menu? = null @@ -66,37 +69,18 @@ class UploadProgressActivity : BaseActivity() { setTabs() } - private fun getPendingUploadsFragment(): PendingUploadsFragment? { - return supportFragmentManager.findFragmentByTag( - "android:switcher:${R.id.upload_progress_view_pager}:${0}", - ) as? PendingUploadsFragment - } - - - // the helper to retrieve the current, non-stale FailedUploadsFragment instance. - - private fun getFailedUploadsFragment(): FailedUploadsFragment? { - return supportFragmentManager.findFragmentByTag( - "android:switcher:${R.id.upload_progress_view_pager}:${1}", - ) as? FailedUploadsFragment - } - /** * Initializes and sets up the tabs data by creating instances of `PendingUploadsFragment` * and `FailedUploadsFragment`, adds them to the `fragmentList`, and assigns corresponding * titles from resources to the `titleList`. */ fun setTabs() { - val pendingUploadsFragment: Fragment - val failedUploadsFragment: Fragment - - // checks if the fragmentManager already has the fragments (after the rotation) - pendingUploadsFragment = getPendingUploadsFragment() ?: PendingUploadsFragment() - failedUploadsFragment = getFailedUploadsFragment() ?: FailedUploadsFragment() + pendingUploadsFragment = PendingUploadsFragment() + failedUploadsFragment = FailedUploadsFragment() viewPagerAdapter!!.setTabs( - R.string.pending to pendingUploadsFragment, - R.string.failed to failedUploadsFragment + R.string.pending to pendingUploadsFragment!!, + R.string.failed to failedUploadsFragment!! ) viewPagerAdapter!!.notifyDataSetChanged() } @@ -136,8 +120,7 @@ class UploadProgressActivity : BaseActivity() { getString(R.string.pause), ).setIcon(R.drawable.pause_icon) .setOnMenuItemClickListener { - //retrieves the current fragment instance just before the use - getPendingUploadsFragment()?.pauseUploads() + pendingUploadsFragment?.pauseUploads() setPausedIcon(true) true }.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM) @@ -151,8 +134,7 @@ class UploadProgressActivity : BaseActivity() { getString(R.string.cancel), ).setIcon(R.drawable.ic_cancel_upload) .setOnMenuItemClickListener { - //retrieve the current fragment instance just before thee use - getPendingUploadsFragment()?.deleteUploads() + pendingUploadsFragment?.deleteUploads() true }.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM) } @@ -166,8 +148,7 @@ class UploadProgressActivity : BaseActivity() { getString(R.string.resume), ).setIcon(R.drawable.play_icon) .setOnMenuItemClickListener { - //retrieve the current fragment instance just before the use - getPendingUploadsFragment()?.restartUploads() + pendingUploadsFragment?.restartUploads() setPausedIcon(false) true }.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM) @@ -180,7 +161,8 @@ class UploadProgressActivity : BaseActivity() { menu!! .add(Menu.NONE, R.id.retry_icon, Menu.NONE, getString(R.string.retry)) .setIcon(R.drawable.ic_refresh_24dp) - .setOnMenuItemClickListener { getFailedUploadsFragment()?.restartUploads() + .setOnMenuItemClickListener { + failedUploadsFragment?.restartUploads() true }.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM) } @@ -193,7 +175,7 @@ class UploadProgressActivity : BaseActivity() { getString(R.string.cancel), ).setIcon(R.drawable.ic_cancel_upload) .setOnMenuItemClickListener { - getFailedUploadsFragment()?.deleteUploads() + failedUploadsFragment?.deleteUploads() true }.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM) } From e079de090330ec0b7f625dc7f98ca6b06aa495e6 Mon Sep 17 00:00:00 2001 From: Kota-Jagadeesh Date: Sun, 9 Nov 2025 13:45:59 +0530 Subject: [PATCH 04/10] Refactor:used nullable presenter field and safe calls for injection robustness --- .../commons/upload/PendingUploadsFragment.kt | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsFragment.kt b/app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsFragment.kt index 1737d08f92..6b1d81454b 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsFragment.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsFragment.kt @@ -28,12 +28,9 @@ class PendingUploadsFragment : CommonsDaggerSupportFragment(), PendingUploadsContract.View, PendingUploadsAdapter.Callback { - - //fix:public and nullable to allow the dagger injection and prevent crash on the rotation. @Inject @JvmField var pendingUploadsPresenter: PendingUploadsPresenter? = null - private lateinit var binding: FragmentPendingUploadsBinding private lateinit var uploadProgressActivity: UploadProgressActivity @@ -63,6 +60,10 @@ class PendingUploadsFragment : return binding.root } + fun initAdapter() { + adapter = PendingUploadsAdapter(this) + } + override fun onViewCreated( view: View, savedInstanceState: Bundle?, @@ -71,10 +72,6 @@ class PendingUploadsFragment : initRecyclerView() } - fun initAdapter() { - adapter = PendingUploadsAdapter(this) - } - /** * Initializes the recycler view. */ @@ -132,7 +129,7 @@ class PendingUploadsFragment : String.format(locale, activity.getString(R.string.no)), { ViewUtil.showShortToast(context, R.string.cancelling_upload) - // uses the safe call directly + //uses the safe call directly pendingUploadsPresenter?.deleteUpload( contribution, requireContext().applicationContext, ) @@ -153,8 +150,7 @@ class PendingUploadsFragment : /** * Pauses all the ongoing uploads. */ - fun pauseUploads() { pendingUploadsPresenter?.pauseUploads() - } + fun pauseUploads() { pendingUploadsPresenter?.pauseUploads() } /** * Cancels all the uploads after getting a confirmation from the user using Dialog. @@ -184,4 +180,5 @@ class PendingUploadsFragment : {}, ) } -} \ No newline at end of file +} + From e5a4fd3cb5ac6a36f8af5ec7d186c3498350e549 Mon Sep 17 00:00:00 2001 From: Kota-Jagadeesh Date: Sun, 9 Nov 2025 13:46:36 +0530 Subject: [PATCH 05/10] fix: retrieve active fragment by tag to eliminate stale reference and restore functionality --- .../commons/upload/UploadProgressActivity.kt | 41 +++++++++++++------ 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt b/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt index d1ae55996f..54ef35d8e0 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt @@ -21,9 +21,6 @@ import javax.inject.Inject */ class UploadProgressActivity : BaseActivity() { private lateinit var binding: ActivityUploadProgressBinding - // REVERT: added back the private fields to hold fragment instances - private var pendingUploadsFragment: PendingUploadsFragment? = null - private var failedUploadsFragment: FailedUploadsFragment? = null var viewPagerAdapter: ViewPagerAdapter? = null var menu: Menu? = null @@ -69,18 +66,34 @@ class UploadProgressActivity : BaseActivity() { setTabs() } + // fix:helper to retrieve the current, non-stale PendingUploadsFragment instance + private fun getPendingUploadsFragment(): PendingUploadsFragment? { + return supportFragmentManager.findFragmentByTag( + "android:switcher:${R.id.upload_progress_view_pager}:${0}", + ) as? PendingUploadsFragment + } + + private fun getFailedUploadsFragment(): FailedUploadsFragment? { + return supportFragmentManager.findFragmentByTag( + "android:switcher:${R.id.upload_progress_view_pager}:${1}", + ) as? FailedUploadsFragment + } + /** * Initializes and sets up the tabs data by creating instances of `PendingUploadsFragment` * and `FailedUploadsFragment`, adds them to the `fragmentList`, and assigns corresponding * titles from resources to the `titleList`. */ fun setTabs() { - pendingUploadsFragment = PendingUploadsFragment() - failedUploadsFragment = FailedUploadsFragment() + val pendingUploadsFragment: Fragment + val failedUploadsFragment: Fragment + //check if the fragmentManager already has the fragments (afterr the rotation) + pendingUploadsFragment = getPendingUploadsFragment() ?: PendingUploadsFragment() + failedUploadsFragment = getFailedUploadsFragment() ?: FailedUploadsFragment() viewPagerAdapter!!.setTabs( - R.string.pending to pendingUploadsFragment!!, - R.string.failed to failedUploadsFragment!! + R.string.pending to pendingUploadsFragment, + R.string.failed to failedUploadsFragment ) viewPagerAdapter!!.notifyDataSetChanged() } @@ -120,7 +133,8 @@ class UploadProgressActivity : BaseActivity() { getString(R.string.pause), ).setIcon(R.drawable.pause_icon) .setOnMenuItemClickListener { - pendingUploadsFragment?.pauseUploads() + //retrieves the current fragment instance just before use + getPendingUploadsFragment()?.pauseUploads() setPausedIcon(true) true }.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM) @@ -134,7 +148,7 @@ class UploadProgressActivity : BaseActivity() { getString(R.string.cancel), ).setIcon(R.drawable.ic_cancel_upload) .setOnMenuItemClickListener { - pendingUploadsFragment?.deleteUploads() + getPendingUploadsFragment()?.deleteUploads() true }.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM) } @@ -148,7 +162,7 @@ class UploadProgressActivity : BaseActivity() { getString(R.string.resume), ).setIcon(R.drawable.play_icon) .setOnMenuItemClickListener { - pendingUploadsFragment?.restartUploads() + getPendingUploadsFragment()?.restartUploads() setPausedIcon(false) true }.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM) @@ -162,7 +176,7 @@ class UploadProgressActivity : BaseActivity() { .add(Menu.NONE, R.id.retry_icon, Menu.NONE, getString(R.string.retry)) .setIcon(R.drawable.ic_refresh_24dp) .setOnMenuItemClickListener { - failedUploadsFragment?.restartUploads() + getFailedUploadsFragment()?.restartUploads() true }.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM) } @@ -175,7 +189,7 @@ class UploadProgressActivity : BaseActivity() { getString(R.string.cancel), ).setIcon(R.drawable.ic_cancel_upload) .setOnMenuItemClickListener { - failedUploadsFragment?.deleteUploads() + getFailedUploadsFragment()?.deleteUploads() true }.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM) } @@ -209,4 +223,5 @@ class UploadProgressActivity : BaseActivity() { isErrorIconsVisisble = visible updateMenuItems(binding.uploadProgressViewPager.currentItem) } -} \ No newline at end of file +} + From 81b59a05b7a0836bcabd6286deb370de94c77c8e Mon Sep 17 00:00:00 2001 From: Kota-Jagadeesh Date: Mon, 10 Nov 2025 17:45:29 +0530 Subject: [PATCH 06/10] Revert: remove the nullable presenter and safe calls as injection is proven robust --- .../commons/upload/PendingUploadsFragment.kt | 37 ++++++++++--------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsFragment.kt b/app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsFragment.kt index 6b1d81454b..a5609447eb 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsFragment.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsFragment.kt @@ -28,9 +28,9 @@ class PendingUploadsFragment : CommonsDaggerSupportFragment(), PendingUploadsContract.View, PendingUploadsAdapter.Callback { - @Inject - @JvmField - var pendingUploadsPresenter: PendingUploadsPresenter? = null + @Inject + lateinit var pendingUploadsPresenter: PendingUploadsPresenter + private lateinit var binding: FragmentPendingUploadsBinding private lateinit var uploadProgressActivity: UploadProgressActivity @@ -55,15 +55,11 @@ class PendingUploadsFragment : ): View { super.onCreate(savedInstanceState) binding = FragmentPendingUploadsBinding.inflate(inflater, container, false) - pendingUploadsPresenter?.onAttachView(this) + pendingUploadsPresenter.onAttachView(this) initAdapter() return binding.root } - fun initAdapter() { - adapter = PendingUploadsAdapter(this) - } - override fun onViewCreated( view: View, savedInstanceState: Bundle?, @@ -72,15 +68,19 @@ class PendingUploadsFragment : initRecyclerView() } + fun initAdapter() { + adapter = PendingUploadsAdapter(this) + } + /** * Initializes the recycler view. */ private fun initRecyclerView() { binding.pendingUploadsRecyclerView.setLayoutManager(LinearLayoutManager(this.context)) binding.pendingUploadsRecyclerView.adapter = adapter - pendingUploadsPresenter?.setup() - pendingUploadsPresenter?.totalContributionList - ?.observe(viewLifecycleOwner) { list: PagedList -> + pendingUploadsPresenter.setup() + pendingUploadsPresenter.totalContributionList + .observe(viewLifecycleOwner) { list: PagedList -> contributionsSize = list.size contributionsList = mutableListOf() var pausedOrQueuedUploads = 0 @@ -129,8 +129,7 @@ class PendingUploadsFragment : String.format(locale, activity.getString(R.string.no)), { ViewUtil.showShortToast(context, R.string.cancelling_upload) - //uses the safe call directly - pendingUploadsPresenter?.deleteUpload( + pendingUploadsPresenter.deleteUpload( contribution, requireContext().applicationContext, ) }, @@ -142,7 +141,7 @@ class PendingUploadsFragment : * Restarts all the paused uploads. */ fun restartUploads() { - pendingUploadsPresenter?.restartUploads( + pendingUploadsPresenter.restartUploads( contributionsList, 0, requireContext().applicationContext ) } @@ -150,13 +149,16 @@ class PendingUploadsFragment : /** * Pauses all the ongoing uploads. */ - fun pauseUploads() { pendingUploadsPresenter?.pauseUploads() } + fun pauseUploads() { + pendingUploadsPresenter.pauseUploads() + } /** * Cancels all the uploads after getting a confirmation from the user using Dialog. */ fun deleteUploads() { - pendingUploadsPresenter ?: return + // the check below is no longer needed but kept for original logic + if (!::pendingUploadsPresenter.isInitialized) return val activity = requireActivity() val locale = Locale.getDefault() @@ -169,7 +171,7 @@ class PendingUploadsFragment : { ViewUtil.showShortToast(context, R.string.cancelling_upload) uploadProgressActivity.hidePendingIcons() - pendingUploadsPresenter?.deleteUploads( + pendingUploadsPresenter.deleteUploads( listOf( STATE_QUEUED, STATE_IN_PROGRESS, @@ -181,4 +183,3 @@ class PendingUploadsFragment : ) } } - From 7aa1c0fe67d8af0e3c4eb565bd871f8013388077 Mon Sep 17 00:00:00 2001 From: Kota-Jagadeesh Date: Mon, 10 Nov 2025 17:46:06 +0530 Subject: [PATCH 07/10] fix: retain the fragment manager lookup to prevent stale references and restore menu functionality --- .../nrw/commons/upload/UploadProgressActivity.kt | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt b/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt index 54ef35d8e0..5a32f183eb 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt @@ -21,6 +21,7 @@ import javax.inject.Inject */ class UploadProgressActivity : BaseActivity() { private lateinit var binding: ActivityUploadProgressBinding + // fields for the fragments are removed as retrieval is done via FragmentManager var viewPagerAdapter: ViewPagerAdapter? = null var menu: Menu? = null @@ -66,13 +67,15 @@ class UploadProgressActivity : BaseActivity() { setTabs() } - // fix:helper to retrieve the current, non-stale PendingUploadsFragment instance + // FIX: the helelper to retrieve the current, non-stale PendingUploadsFragment instance. private fun getPendingUploadsFragment(): PendingUploadsFragment? { return supportFragmentManager.findFragmentByTag( "android:switcher:${R.id.upload_progress_view_pager}:${0}", ) as? PendingUploadsFragment } + + // FIX: helper to retrieve the current, non-stale FailedUploadsFragment instance. private fun getFailedUploadsFragment(): FailedUploadsFragment? { return supportFragmentManager.findFragmentByTag( "android:switcher:${R.id.upload_progress_view_pager}:${1}", @@ -87,7 +90,7 @@ class UploadProgressActivity : BaseActivity() { fun setTabs() { val pendingUploadsFragment: Fragment val failedUploadsFragment: Fragment - //check if the fragmentManager already has the fragments (afterr the rotation) + // using the FragmentManager lookup to get the correct, current instances after the rotation pendingUploadsFragment = getPendingUploadsFragment() ?: PendingUploadsFragment() failedUploadsFragment = getFailedUploadsFragment() ?: FailedUploadsFragment() @@ -133,7 +136,7 @@ class UploadProgressActivity : BaseActivity() { getString(R.string.pause), ).setIcon(R.drawable.pause_icon) .setOnMenuItemClickListener { - //retrieves the current fragment instance just before use + // FIX: retrive and use the current fragment instance with safe call to avoid the NPE if retrieval fails getPendingUploadsFragment()?.pauseUploads() setPausedIcon(true) true @@ -148,6 +151,7 @@ class UploadProgressActivity : BaseActivity() { getString(R.string.cancel), ).setIcon(R.drawable.ic_cancel_upload) .setOnMenuItemClickListener { + // FIX: retrive and use the current fragment instance getPendingUploadsFragment()?.deleteUploads() true }.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM) @@ -162,6 +166,7 @@ class UploadProgressActivity : BaseActivity() { getString(R.string.resume), ).setIcon(R.drawable.play_icon) .setOnMenuItemClickListener { + // FIX: retrive and use the current fragment instance getPendingUploadsFragment()?.restartUploads() setPausedIcon(false) true @@ -224,4 +229,3 @@ class UploadProgressActivity : BaseActivity() { updateMenuItems(binding.uploadProgressViewPager.currentItem) } } - From b79f540629a2d3f611a3728c471aa3a03e9f545a Mon Sep 17 00:00:00 2001 From: Kota-Jagadeesh Date: Mon, 10 Nov 2025 22:02:36 +0530 Subject: [PATCH 08/10] remove redundant Presenter initialization check --- .../fr/free/nrw/commons/upload/PendingUploadsFragment.kt | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsFragment.kt b/app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsFragment.kt index a5609447eb..78a3933f83 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsFragment.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsFragment.kt @@ -28,7 +28,8 @@ class PendingUploadsFragment : CommonsDaggerSupportFragment(), PendingUploadsContract.View, PendingUploadsAdapter.Callback { - @Inject + // final-state:Non-nullable lateinit var as the Presenter is always initialized. + @Inject lateinit var pendingUploadsPresenter: PendingUploadsPresenter private lateinit var binding: FragmentPendingUploadsBinding @@ -157,9 +158,7 @@ class PendingUploadsFragment : * Cancels all the uploads after getting a confirmation from the user using Dialog. */ fun deleteUploads() { - // the check below is no longer needed but kept for original logic - if (!::pendingUploadsPresenter.isInitialized) return - + //final change:thee redundant 'if (!::pendingUploadsPresenter.isInitialized) return' is removed. val activity = requireActivity() val locale = Locale.getDefault() showAlertDialog( @@ -182,4 +181,4 @@ class PendingUploadsFragment : {}, ) } -} +} \ No newline at end of file From 3dbb031081bc777c7e0e8e13bcfbe5db730be204 Mon Sep 17 00:00:00 2001 From: Kota-Jagadeesh Date: Mon, 10 Nov 2025 22:03:14 +0530 Subject: [PATCH 09/10] refactor:use robust FragmentManager 'find' by class to prevent stale references on rotation --- .../commons/upload/UploadProgressActivity.kt | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt b/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt index 5a32f183eb..e14953c3b2 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt @@ -21,7 +21,6 @@ import javax.inject.Inject */ class UploadProgressActivity : BaseActivity() { private lateinit var binding: ActivityUploadProgressBinding - // fields for the fragments are removed as retrieval is done via FragmentManager var viewPagerAdapter: ViewPagerAdapter? = null var menu: Menu? = null @@ -67,21 +66,23 @@ class UploadProgressActivity : BaseActivity() { setTabs() } - // FIX: the helelper to retrieve the current, non-stale PendingUploadsFragment instance. + //final fix:helper to retrieve the current Fragment by CLASS TYPE (as suggested). private fun getPendingUploadsFragment(): PendingUploadsFragment? { - return supportFragmentManager.findFragmentByTag( - "android:switcher:${R.id.upload_progress_view_pager}:${0}", - ) as? PendingUploadsFragment + //find the Fragment by class type, not index + return supportFragmentManager.fragments.find { + it is PendingUploadsFragment && it.isAdded + } as? PendingUploadsFragment } - - // FIX: helper to retrieve the current, non-stale FailedUploadsFragment instance. + //final fix:helper to retrieve the current Fragment by CLASS TYPE (as suggested). private fun getFailedUploadsFragment(): FailedUploadsFragment? { - return supportFragmentManager.findFragmentByTag( - "android:switcher:${R.id.upload_progress_view_pager}:${1}", - ) as? FailedUploadsFragment + //find the Fragment by class type, not index + return supportFragmentManager.fragments.find { + it is FailedUploadsFragment && it.isAdded + } as? FailedUploadsFragment } + /** * Initializes and sets up the tabs data by creating instances of `PendingUploadsFragment` * and `FailedUploadsFragment`, adds them to the `fragmentList`, and assigns corresponding @@ -90,7 +91,8 @@ class UploadProgressActivity : BaseActivity() { fun setTabs() { val pendingUploadsFragment: Fragment val failedUploadsFragment: Fragment - // using the FragmentManager lookup to get the correct, current instances after the rotation + + // using the robust getPendingUploadsFragment() helper pendingUploadsFragment = getPendingUploadsFragment() ?: PendingUploadsFragment() failedUploadsFragment = getFailedUploadsFragment() ?: FailedUploadsFragment() @@ -136,7 +138,7 @@ class UploadProgressActivity : BaseActivity() { getString(R.string.pause), ).setIcon(R.drawable.pause_icon) .setOnMenuItemClickListener { - // FIX: retrive and use the current fragment instance with safe call to avoid the NPE if retrieval fails + //robust helper to find the correct instance getPendingUploadsFragment()?.pauseUploads() setPausedIcon(true) true @@ -151,7 +153,7 @@ class UploadProgressActivity : BaseActivity() { getString(R.string.cancel), ).setIcon(R.drawable.ic_cancel_upload) .setOnMenuItemClickListener { - // FIX: retrive and use the current fragment instance + //robust helper to find the correct instance getPendingUploadsFragment()?.deleteUploads() true }.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM) @@ -166,7 +168,7 @@ class UploadProgressActivity : BaseActivity() { getString(R.string.resume), ).setIcon(R.drawable.play_icon) .setOnMenuItemClickListener { - // FIX: retrive and use the current fragment instance + //robust helper to find the correct instance getPendingUploadsFragment()?.restartUploads() setPausedIcon(false) true From 4f7033b857eee28d16a262fb4322d3fdaf84e791 Mon Sep 17 00:00:00 2001 From: Kota-Jagadeesh Date: Fri, 14 Nov 2025 16:10:20 +0530 Subject: [PATCH 10/10] Polished some comments --- .../java/fr/free/nrw/commons/upload/UploadProgressActivity.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt b/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt index e14953c3b2..0fafeb79b6 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt @@ -66,7 +66,7 @@ class UploadProgressActivity : BaseActivity() { setTabs() } - //final fix:helper to retrieve the current Fragment by CLASS TYPE (as suggested). + //helper to retrieve the current Fragment by CLASS TYPE (as suggested). private fun getPendingUploadsFragment(): PendingUploadsFragment? { //find the Fragment by class type, not index return supportFragmentManager.fragments.find { @@ -74,7 +74,6 @@ class UploadProgressActivity : BaseActivity() { } as? PendingUploadsFragment } - //final fix:helper to retrieve the current Fragment by CLASS TYPE (as suggested). private fun getFailedUploadsFragment(): FailedUploadsFragment? { //find the Fragment by class type, not index return supportFragmentManager.fragments.find {