Remove usage of `runBlocking` from `SettingsActivity`
Summary
As part of #5688, removing usage of runBlocking from SettingsActivity. This involved changing setAppActive into a suspend function and then making sure it's invoked from a coroutine.
Checklist
- [ ] New or updated tests have been added to cover the changes following the testing guidelines.
- [x] The code follows the project's code style and best_practices.
- [x] The changes have been thoroughly tested, and edge cases have been considered.
- [x] Changes are backward compatible whenever feasible. Any breaking changes are documented in the changelog for users and/or in the code for developers depending on the relevance.
After thinking about it a bit more: by setting the active state in the lifecycleScope, we risk that the app is not set as inactive before the activity/fragment/app is quit, as the scope may be cancelled. That should never happen as it defeats the purpose of an app lock.
(This may also apply in other places, not only here)
After thinking about it a bit more: by setting the active state in the
lifecycleScope, we risk that the app is not set as inactive before the activity/fragment/app is quit, as the scope may be cancelled. That should never happen as it defeats the purpose of an app lock.(This may also apply in other places, not only here)
@jpelgrom Do we need to use an application-level scope here?
@jpelgrom Do we need to use an application-level scope here?
I missed that this PR was waiting for a response from me, sorry.
I'm tempted to say yes, but would like another opinion. Maybe I'm making this too big, and it would increase the scope of the PR. @TimoPtr what do you think?
@jpelgrom Do we need to use an application-level scope here?
I missed that this PR was waiting for a response from me, sorry.
I'm tempted to say yes, but would like another opinion. Maybe I'm making this too big, and it would increase the scope of the PR. @TimoPtr what do you think?
In general I'm not a big fan of application scope. It could be in a viewModel.
Now that I think a bit more about it, we could still have race condition if for some reason the localStorage is slow. We might need to make the IntegrationRepositoryImpl.appActive thread safe.
It could be made in a first PR before this one. Also I would like to have tests for such changes.