Remove usage of runBlocking from SettingsActivity
#5920
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR removes the usage of runBlocking from SettingsActivity as part of issue #5688. The change converts the synchronous blocking call into proper asynchronous coroutine usage.
Key changes:
- Converted
setAppActive(serverId, active)from usingrunBlockingto a propersuspendfunction - Updated the call site in
SettingsFragmentto uselifecycleScope.launchinstead of direct invocation - Removed the unnecessary
runBlockingimport
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| SettingsActivity.kt | Converted setAppActive to suspend function and removed runBlocking usage |
| SettingsFragment.kt | Updated call site to properly invoke the suspend function within a coroutine scope |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| (activity as? SettingsActivity)?.let { settingsActivity -> | ||
| lifecycleScope.launch { | ||
| settingsActivity.setAppActive(serverAuth, true) | ||
| } | ||
| } | ||
| parentFragmentManager.commit { |
There was a problem hiding this comment.
@jpelgrom @dshokouhi could you test if you see any issue on this change?
I did test on Android 9 old device, that is quite slow with 2 servers and 1 lock and it seems to be ok.
There was a problem hiding this comment.
The fragment's comment suggests a possible issue when toggling it on/off:
Now the code runs async so it might end up locking the app? Is that what you tested or just app lock on the main settings screen?
There was a problem hiding this comment.
I simply tested app lock in both dashboard and settings
|
After thinking about it a bit more: by setting the active state in the (This may also apply in other places, not only here) |
@jpelgrom Do we need to use an application-level scope here? |
Summary
As part of #5688, removing usage of
runBlockingfromSettingsActivity. This involved changingsetAppActiveinto asuspendfunction and then making sure it's invoked from a coroutine.Checklist