Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Unverified Commit 1cc9b4bd authored by Sunik Kupfer's avatar Sunik Kupfer Committed by GitHub
Browse files

Fix show only personal setting not updating the view immediately (#1238)



* Split pair of show only personal settings

* Create and consume showOnlyPersonal settings separately

* Fix showOnlyPersonal flow state change not triggering re-emission

* Combine if statements

* Minor refactoring (lift out "if")

* Create separate reload methods

* Reload on model creation

* Use viewmodel scope

* Move init after relevant method declarations

* Add kdoc

* Remove deprecated getShowOnlyPersonalPair

---------

Co-authored-by: default avatarRicki Hirner <hirner@bitfire.at>
parent 226583f1
Loading
Loading
Loading
Loading
+23 −13
Original line number Diff line number Diff line
@@ -286,15 +286,25 @@ class AccountSettings @AssistedInject constructor(

    // UI settings

    data class ShowOnlyPersonal(
        val onlyPersonal: Boolean,
        val locked: Boolean
    )
    /**
     * Whether to show only personal collections in the UI
     *
     * @return *true* if only personal collections shall be shown; *false* otherwise
     */
    fun getShowOnlyPersonal(): Boolean = when (settingsManager.getIntOrNull(KEY_SHOW_ONLY_PERSONAL)) {
        0 -> false
        1 -> true
        else /* including -1 */ -> accountManager.getUserData(account, KEY_SHOW_ONLY_PERSONAL) != null
    }

    fun getShowOnlyPersonal(): ShowOnlyPersonal {
        @Suppress("DEPRECATION")
        val pair = getShowOnlyPersonalPair()
        return ShowOnlyPersonal(onlyPersonal = pair.first, locked = !pair.second)
    /**
     * Whether the user shall be able to change the setting (= setting not locked)
     *
     * @return *true* if the setting is locked; *false* otherwise
     */
    fun getShowOnlyPersonalLocked(): Boolean = when (settingsManager.getIntOrNull(KEY_SHOW_ONLY_PERSONAL)) {
        0, 1 -> true
        else /* including -1 */ -> false
    }

    /**
+13 −11
Original line number Diff line number Diff line
@@ -69,7 +69,6 @@ import androidx.paging.compose.LazyPagingItems
import androidx.paging.compose.collectAsLazyPagingItems
import at.bitfire.davdroid.R
import at.bitfire.davdroid.db.Collection
import at.bitfire.davdroid.settings.AccountSettings
import at.bitfire.davdroid.ui.AppTheme
import at.bitfire.davdroid.ui.PermissionsActivity
import at.bitfire.davdroid.ui.account.AccountProgress
@@ -114,9 +113,8 @@ fun AccountScreen(
        error = model.error,
        onResetError = model::resetError,
        invalidAccount = model.invalidAccount.collectAsStateWithLifecycle(false).value,
        showOnlyPersonal = model.showOnlyPersonal.collectAsStateWithLifecycle(
            initialValue = AccountSettings.ShowOnlyPersonal(onlyPersonal = false, locked = false)
        ).value,
        showOnlyPersonal = model.showOnlyPersonal.collectAsStateWithLifecycle().value,
        showOnlyPersonalLocked = model.showOnlyPersonalLocked.collectAsStateWithLifecycle().value,
        onSetShowOnlyPersonal = model::setShowOnlyPersonal,
        hasCardDav = cardDavService != null,
        canCreateAddressBook = model.canCreateAddressBook.collectAsStateWithLifecycle(false).value,
@@ -169,7 +167,8 @@ fun AccountScreen(
    error: String? = null,
    onResetError: () -> Unit = {},
    invalidAccount: Boolean = false,
    showOnlyPersonal: AccountSettings.ShowOnlyPersonal,
    showOnlyPersonal: Boolean = false,
    showOnlyPersonalLocked: Boolean = false,
    onSetShowOnlyPersonal: (showOnlyPersonal: Boolean) -> Unit = {},
    hasCardDav: Boolean,
    canCreateAddressBook: Boolean,
@@ -257,6 +256,7 @@ fun AccountScreen(
                            canCreateCalendar = canCreateCalendar,
                            onCreateCalendar = onCreateCalendar,
                            showOnlyPersonal = showOnlyPersonal,
                            showOnlyPersonalLocked = showOnlyPersonalLocked,
                            onSetShowOnlyPersonal = onSetShowOnlyPersonal,
                            currentPage = pagerState.currentPage,
                            idxCardDav = idxCardDav,
@@ -440,7 +440,8 @@ fun AccountScreen_Actions(
    onCreateAddressBook: () -> Unit,
    canCreateCalendar: Boolean,
    onCreateCalendar: () -> Unit,
    showOnlyPersonal: AccountSettings.ShowOnlyPersonal,
    showOnlyPersonal: Boolean,
    showOnlyPersonalLocked: Boolean,
    onSetShowOnlyPersonal: (showOnlyPersonal: Boolean) -> Unit,
    currentPage: Int,
    idxCardDav: Int?,
@@ -513,8 +514,8 @@ fun AccountScreen_Actions(
                    LocalMinimumInteractiveComponentSize provides Dp.Unspecified
                ) {
                    Checkbox(
                        checked = showOnlyPersonal.onlyPersonal,
                        enabled = !showOnlyPersonal.locked,
                        checked = showOnlyPersonal,
                        enabled = !showOnlyPersonalLocked,
                        onCheckedChange = {
                            onSetShowOnlyPersonal(it)
                            overflowOpen = false
@@ -527,10 +528,10 @@ fun AccountScreen_Actions(
                Text(stringResource(R.string.account_only_personal))
            },
            onClick = {
                onSetShowOnlyPersonal(!showOnlyPersonal.onlyPersonal)
                onSetShowOnlyPersonal(!showOnlyPersonal)
                overflowOpen = false
            },
            enabled = !showOnlyPersonal.locked
            enabled = !showOnlyPersonalLocked
        )

        // rename account
@@ -651,7 +652,8 @@ fun AccountScreen_ServiceTab(
fun AccountScreen_Preview() {
    AccountScreen(
        accountName = "test@example.com",
        showOnlyPersonal = AccountSettings.ShowOnlyPersonal(onlyPersonal = false, locked = true),
        showOnlyPersonal = false,
        showOnlyPersonalLocked = true,
        hasCardDav = true,
        canCreateAddressBook = false,
        cardDavProgress = AccountProgress.Active,
+38 −24
Original line number Diff line number Diff line
@@ -9,11 +9,7 @@ import android.content.Context
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.setValue
import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.ViewModel
import androidx.lifecycle.asFlow
import androidx.lifecycle.switchMap
import androidx.lifecycle.viewModelScope
import at.bitfire.davdroid.R
import at.bitfire.davdroid.db.Collection
@@ -30,14 +26,15 @@ import dagger.assisted.AssistedFactory
import dagger.assisted.AssistedInject
import dagger.hilt.android.lifecycle.HiltViewModel
import dagger.hilt.android.qualifiers.ApplicationContext
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import java.util.logging.Level
import java.util.logging.Logger

@@ -62,26 +59,45 @@ class AccountScreenModel @AssistedInject constructor(
        fun create(account: Account): AccountScreenModel
    }

    /**
     * Only acquire account settings on a worker thread!
     */
    private val accountSettings by lazy { accountSettingsFactory.create(account) }

    /** whether the account is invalid and the screen shall be closed */
    val invalidAccount = accountRepository.getAllFlow().map { accounts ->
        !accounts.contains(account)
    }

    private val refreshSettingsSignal = MutableLiveData(Unit)
    val showOnlyPersonal = refreshSettingsSignal.switchMap<Unit, AccountSettings.ShowOnlyPersonal> {
        object : LiveData<AccountSettings.ShowOnlyPersonal>() {
            init {
                viewModelScope.launch(Dispatchers.IO) {
                    val settings = accountSettingsFactory.create(account)
                    postValue(settings.getShowOnlyPersonal())
    /**
     * Whether to show only personal collections.
     */
    private val _showOnlyPersonal = MutableStateFlow(false)
    val showOnlyPersonal = _showOnlyPersonal.asStateFlow()
    private suspend fun reloadShowOnlyPersonal() = withContext(Dispatchers.Default) {
        _showOnlyPersonal.value = accountSettings.getShowOnlyPersonal()
    }
    fun setShowOnlyPersonal(showOnlyPersonal: Boolean) {
        viewModelScope.launch {
            accountSettings.setShowOnlyPersonal(showOnlyPersonal)
            reloadShowOnlyPersonal()
        }
    }

    /**
     * Whether the user setting to show only personal collections is locked.
     */
    private var _showOnlyPersonalLocked = MutableStateFlow(false)
    val showOnlyPersonalLocked = _showOnlyPersonalLocked.asStateFlow()
    private suspend fun reloadShowOnlyPersonalLocked() = withContext(Dispatchers.Default) {
        _showOnlyPersonalLocked.value = accountSettings.getShowOnlyPersonalLocked()
    }

    init {
        viewModelScope.launch {
            reloadShowOnlyPersonal()
            reloadShowOnlyPersonalLocked()
        }
    }.asFlow()
    fun setShowOnlyPersonal(showOnlyPersonal: Boolean) = viewModelScope.launch(Dispatchers.IO) {
        val settings = accountSettingsFactory.create(account)
        settings.setShowOnlyPersonal(showOnlyPersonal)
        refreshSettingsSignal.postValue(Unit)
    }

    val cardDavSvc = serviceRepository
@@ -130,11 +146,9 @@ class AccountScreenModel @AssistedInject constructor(

    // actions

    private val notInterruptibleScope = CoroutineScope(SupervisorJob())

    /** Deletes the account from the system (won't touch collections on the server). */
    fun deleteAccount() {
        notInterruptibleScope.launch {
        viewModelScope.launch {
            accountRepository.delete(account.name)
        }
    }
@@ -154,7 +168,7 @@ class AccountScreenModel @AssistedInject constructor(
     * @param newName new account name
     */
    fun renameAccount(newName: String) {
        notInterruptibleScope.launch {
        viewModelScope.launch {
            try {
                accountRepository.rename(account.name, newName)

+9 −15
Original line number Diff line number Diff line
@@ -11,13 +11,12 @@ import androidx.paging.map
import at.bitfire.davdroid.db.Collection
import at.bitfire.davdroid.db.Service
import at.bitfire.davdroid.repository.DavCollectionRepository
import at.bitfire.davdroid.settings.AccountSettings
import at.bitfire.davdroid.settings.Settings
import at.bitfire.davdroid.settings.SettingsManager
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.flattenConcat
import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.flow.map
import javax.inject.Inject
@@ -43,35 +42,30 @@ class GetServiceCollectionPagerUseCase @Inject constructor(
    operator fun invoke(
        serviceFlow: Flow<Service?>,
        collectionType: String,
        showOnlyPersonalFlow: Flow<AccountSettings.ShowOnlyPersonal?>
        showOnlyPersonalFlow: Flow<Boolean>
    ): Flow<PagingData<Collection>> =
        combine(serviceFlow, showOnlyPersonalFlow, forceReadOnlyAddressBooksFlow) { service, onlyPersonal, forceReadOnlyAddressBooks ->
            if (service == null)
                flowOf(PagingData.empty<Collection>())
            else {
            service?.let { service ->
                val dataFlow = Pager(
                    config = PagingConfig(PAGER_SIZE),
                    pagingSourceFactory = {
                        if (onlyPersonal?.onlyPersonal == true)
                        if (onlyPersonal == true)
                            collectionRepository.pagePersonalByServiceAndType(service.id, collectionType)
                        else
                            collectionRepository.pageByServiceAndType(service.id, collectionType)
                    }
                ).flow

                // generate resulting flow; set "forceReadOnly" for every address book if requested
                if (forceReadOnlyAddressBooks)
                // set "forceReadOnly" for every address book if requested
                if (forceReadOnlyAddressBooks && collectionType == Collection.TYPE_ADDRESSBOOK)
                    dataFlow.map { pagingData ->
                        pagingData.map { collection ->
                            if (collectionType == Collection.TYPE_ADDRESSBOOK)
                            collection.copy(forceReadOnly = true)
                            else
                                collection
                        }
                    }
                else
                    dataFlow
            }
        }.flattenConcat()
            } ?: flowOf(PagingData.empty())
        }.flatMapLatest { it }

}
 No newline at end of file