Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 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
7c1726b
Merge branch 'main' into fix/upload-rotation-crash
Kota-Jagadeesh Dec 31, 2025
a83a43b
Merge branch 'main' into fix/upload-rotation-crash
Kota-Jagadeesh Jan 4, 2026
5ddd369
Remove unintended changes
Kota-Jagadeesh Jan 4, 2026
dd1bb05
Add: isInitialized safety checks for presenter access to prevent Unin…
Kota-Jagadeesh Jan 4, 2026
5c4d754
Refactor: fragment retrieval in UploadProgressActivity to use dynamic…
Kota-Jagadeesh Jan 4, 2026
fdc91dd
Merge branch 'main' into fix/upload-rotation-crash
Kota-Jagadeesh Jan 5, 2026
529c806
Merge branch 'main' into fix/upload-rotation-crash
Kota-Jagadeesh Jan 18, 2026
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
108 changes: 62 additions & 46 deletions app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsFragment.kt
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ class PendingUploadsFragment :
): View {
super.onCreate(savedInstanceState)
binding = FragmentPendingUploadsBinding.inflate(inflater, container, false)
pendingUploadsPresenter.onAttachView(this)
if (::pendingUploadsPresenter.isInitialized) {
pendingUploadsPresenter.onAttachView(this)
}
initAdapter()
return binding.root
}
Expand All @@ -78,40 +80,42 @@ class PendingUploadsFragment :
private fun initRecyclerView() {
binding.pendingUploadsRecyclerView.setLayoutManager(LinearLayoutManager(this.context))
binding.pendingUploadsRecyclerView.adapter = adapter
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 (::pendingUploadsPresenter.isInitialized) {
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++
}
}
}
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)
} else {
uploadProgressActivity.setPausedIcon(false)
}
}
}
}
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)
} else {
uploadProgressActivity.setPausedIcon(false)
}
}
}
}

Expand All @@ -129,9 +133,11 @@ class PendingUploadsFragment :
String.format(locale, activity.getString(R.string.no)),
{
ViewUtil.showShortToast(context, R.string.cancelling_upload)
pendingUploadsPresenter.deleteUpload(
contribution, requireContext().applicationContext,
)
if (::pendingUploadsPresenter.isInitialized) {
pendingUploadsPresenter.deleteUpload(
contribution, requireContext().applicationContext,
)
}
},
{},
)
Expand All @@ -140,14 +146,22 @@ class PendingUploadsFragment :
/**
* Restarts all the paused uploads.
*/
fun restartUploads() = pendingUploadsPresenter.restartUploads(
contributionsList, 0, requireContext().applicationContext
)
fun restartUploads() {
if (::pendingUploadsPresenter.isInitialized) {
pendingUploadsPresenter.restartUploads(
contributionsList, 0, requireContext().applicationContext
)
}
}

/**
* Pauses all the ongoing uploads.
*/
fun pauseUploads() = pendingUploadsPresenter.pauseUploads()
fun pauseUploads() {
if (::pendingUploadsPresenter.isInitialized) {
pendingUploadsPresenter.pauseUploads()
}
}

/**
* Cancels all the uploads after getting a confirmation from the user using Dialog.
Expand All @@ -164,13 +178,15 @@ class PendingUploadsFragment :
{
ViewUtil.showShortToast(context, R.string.cancelling_upload)
uploadProgressActivity.hidePendingIcons()
pendingUploadsPresenter.deleteUploads(
listOf(
STATE_QUEUED,
STATE_IN_PROGRESS,
STATE_PAUSED,
),
)
if (::pendingUploadsPresenter.isInitialized) {
pendingUploadsPresenter.deleteUploads(
listOf(
STATE_QUEUED,
STATE_IN_PROGRESS,
STATE_PAUSED,
),
)
}
},
{},
)
Expand Down
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,37 @@ class UploadProgressActivity : BaseActivity() {
setTabs()
}

/**
* Helper to retrieve the current PendingUploadsFragment by checking the fragment manager.
*/
private fun getPendingUploadsFragment(): PendingUploadsFragment? {
return supportFragmentManager.fragments.find {
it is PendingUploadsFragment && it.isAdded
} as? PendingUploadsFragment
}

/**
* Helper to retrieve the current FailedUploadsFragment by checking the fragment manager.
*/
private fun getFailedUploadsFragment(): FailedUploadsFragment? {
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 pendingFragment = getPendingUploadsFragment() ?: PendingUploadsFragment()
val failedFragment = getFailedUploadsFragment() ?: FailedUploadsFragment()

viewPagerAdapter!!.setTabs(
R.string.pending to pendingUploadsFragment!!,
R.string.failed to failedUploadsFragment!!
R.string.pending to pendingFragment,
R.string.failed to failedFragment
)
viewPagerAdapter!!.notifyDataSetChanged()
}
Expand Down Expand Up @@ -119,7 +136,7 @@ class UploadProgressActivity : BaseActivity() {
getString(R.string.pause),
).setIcon(R.drawable.pause_icon)
.setOnMenuItemClickListener {
pendingUploadsFragment!!.pauseUploads()
getPendingUploadsFragment()?.pauseUploads()
setPausedIcon(true)
true
}.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM)
Expand All @@ -133,7 +150,7 @@ class UploadProgressActivity : BaseActivity() {
getString(R.string.cancel),
).setIcon(R.drawable.ic_cancel_upload)
.setOnMenuItemClickListener {
pendingUploadsFragment!!.deleteUploads()
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 +164,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)
Expand All @@ -161,7 +178,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 +191,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