Skip to content
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
8352bc5
fix:gracefully handle the missing presenter during configuration change
Kota-Jagadeesh Oct 19, 2025
38cd528
fix:safely retrieve fragments to prevent stale references after rotation
Kota-Jagadeesh Oct 19, 2025
33d3e44
Merge branch 'commons-app:main' into fix/upload-rotation-crash
Kota-Jagadeesh Nov 5, 2025
8f66028
Fix: Use safe calls to prevent NullPointerException on upload menu ac…
Kota-Jagadeesh Nov 9, 2025
e079de0
Refactor:used nullable presenter field and safe calls for injection r…
Kota-Jagadeesh Nov 9, 2025
e5a4fd3
fix: retrieve active fragment by tag to eliminate stale reference and…
Kota-Jagadeesh Nov 9, 2025
7522803
Merge branch 'main' into fix/upload-rotation-crash
Kota-Jagadeesh Nov 9, 2025
81b59a0
Revert: remove the nullable presenter and safe calls as injection is …
Kota-Jagadeesh Nov 10, 2025
7aa1c0f
fix: retain the fragment manager lookup to prevent stale references a…
Kota-Jagadeesh Nov 10, 2025
b79f540
remove redundant Presenter initialization check
Kota-Jagadeesh Nov 10, 2025
3dbb031
refactor:use robust FragmentManager 'find' by class to prevent stale …
Kota-Jagadeesh Nov 10, 2025
4f7033b
Polished some comments
Kota-Jagadeesh Nov 14, 2025
2e13a02
Merge branch 'main' into fix/upload-rotation-crash
Kota-Jagadeesh Nov 14, 2025
7b18d39
Merge branch 'main' into fix/upload-rotation-crash
nicolas-raoul Nov 15, 2025
6b3be73
Merge branch 'main' into fix/upload-rotation-crash
Kota-Jagadeesh Nov 20, 2025
4f8a2ad
Merge branch 'main' into fix/upload-rotation-crash
Kota-Jagadeesh Dec 15, 2025
7de8ebc
Merge branch 'main' into fix/upload-rotation-crash
Kota-Jagadeesh Dec 18, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class PendingUploadsFragment :
CommonsDaggerSupportFragment(),
PendingUploadsContract.View,
PendingUploadsAdapter.Callback {
// final-state:Non-nullable lateinit var as the Presenter is always initialized.
@Inject
lateinit var pendingUploadsPresenter: PendingUploadsPresenter

Expand Down Expand Up @@ -60,10 +61,6 @@ class PendingUploadsFragment :
return binding.root
}

fun initAdapter() {
adapter = PendingUploadsAdapter(this)
}

override fun onViewCreated(
view: View,
savedInstanceState: Bundle?,
Expand All @@ -72,6 +69,10 @@ class PendingUploadsFragment :
initRecyclerView()
}

fun initAdapter() {
adapter = PendingUploadsAdapter(this)
}

/**
* Initializes the recycler view.
*/
Expand All @@ -81,38 +82,38 @@ class PendingUploadsFragment :
pendingUploadsPresenter.setup()
pendingUploadsPresenter.totalContributionList
.observe(viewLifecycleOwner) { list: PagedList<Contribution> ->
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++
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)
}
}
}
}
}

/**
Expand Down Expand Up @@ -140,19 +141,24 @@ class PendingUploadsFragment :
/**
* Restarts all the paused uploads.
*/
fun restartUploads() = pendingUploadsPresenter.restartUploads(
contributionsList, 0, requireContext().applicationContext
)
fun restartUploads() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding was that we don't need any changes in this entire file, is there anything that I'm missing out?😕

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() {
//final change:thee redundant 'if (!::pendingUploadsPresenter.isInitialized) return' is removed.
val activity = requireActivity()
val locale = Locale.getDefault()
showAlertDialog(
Expand All @@ -175,4 +181,4 @@ class PendingUploadsFragment :
{},
)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -68,18 +66,39 @@ class UploadProgressActivity : BaseActivity() {
setTabs()
}

//final fix: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 {
it is PendingUploadsFragment && it.isAdded
} 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 {
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
* titles from resources to the `titleList`.
*/
fun setTabs() {
pendingUploadsFragment = PendingUploadsFragment()
failedUploadsFragment = FailedUploadsFragment()
val pendingUploadsFragment: Fragment
val failedUploadsFragment: Fragment

// using the robust getPendingUploadsFragment() helper
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()
}
Expand Down Expand Up @@ -119,7 +138,8 @@ class UploadProgressActivity : BaseActivity() {
getString(R.string.pause),
).setIcon(R.drawable.pause_icon)
.setOnMenuItemClickListener {
pendingUploadsFragment!!.pauseUploads()
//robust helper to find the correct instance
getPendingUploadsFragment()?.pauseUploads()
setPausedIcon(true)
true
}.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM)
Expand All @@ -133,7 +153,8 @@ class UploadProgressActivity : BaseActivity() {
getString(R.string.cancel),
).setIcon(R.drawable.ic_cancel_upload)
.setOnMenuItemClickListener {
pendingUploadsFragment!!.deleteUploads()
//robust helper to find the correct instance
getPendingUploadsFragment()?.deleteUploads()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will we evaluate this for every operation? Could you please elaborate if there are any use cases apart from configuration changes that need this? I thought adding it to setTabs() would suffice. Please check my previous comment for that approach and share the pros and cons of both the approaches.

true
}.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM)
}
Expand All @@ -147,7 +168,8 @@ class UploadProgressActivity : BaseActivity() {
getString(R.string.resume),
).setIcon(R.drawable.play_icon)
.setOnMenuItemClickListener {
pendingUploadsFragment!!.restartUploads()
//robust helper to find the correct instance
getPendingUploadsFragment()?.restartUploads()
setPausedIcon(false)
true
}.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM)
Expand All @@ -161,7 +183,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)
}
Expand All @@ -174,7 +196,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)
}
Expand Down