diff --git a/changelog.d/8141.feature b/changelog.d/8141.feature new file mode 100644 index 0000000000..8c03af1150 --- /dev/null +++ b/changelog.d/8141.feature @@ -0,0 +1 @@ +[Poll] Error handling for push rules synchronization diff --git a/library/ui-strings/src/main/res/values/strings.xml b/library/ui-strings/src/main/res/values/strings.xml index e06355a173..2ee623cf88 100644 --- a/library/ui-strings/src/main/res/values/strings.xml +++ b/library/ui-strings/src/main/res/values/strings.xml @@ -862,6 +862,8 @@ Keywords cannot start with \'.\' Keywords cannot contain \'%s\' + An error occurred when updating your notification preferences. Please try again. + Troubleshoot Notifications Troubleshooting diagnostics Run Tests diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/events/model/Event.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/events/model/Event.kt index 40c69ceb66..ae3e3a63c8 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/events/model/Event.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/events/model/Event.kt @@ -488,7 +488,7 @@ fun Event.getPollContent(): MessagePollContent? { } fun Event.supportsNotification() = - this.getClearType() in EventType.MESSAGE + EventType.POLL_START.values + EventType.STATE_ROOM_BEACON_INFO.values + this.getClearType() in EventType.MESSAGE + EventType.POLL_START.values + EventType.POLL_END.values + EventType.STATE_ROOM_BEACON_INFO.values fun Event.isContentReportable() = this.getClearType() in EventType.MESSAGE + EventType.STATE_ROOM_BEACON_INFO.values diff --git a/vector/src/main/java/im/vector/app/core/preference/VectorCheckboxPreference.kt b/vector/src/main/java/im/vector/app/core/preference/VectorCheckboxPreference.kt index 13b65e11b4..355bab8da8 100644 --- a/vector/src/main/java/im/vector/app/core/preference/VectorCheckboxPreference.kt +++ b/vector/src/main/java/im/vector/app/core/preference/VectorCheckboxPreference.kt @@ -32,6 +32,8 @@ class VectorCheckboxPreference : CheckBoxPreference { constructor(context: Context) : super(context) + var summaryTextColor: Int? = null + init { // Set to false to remove the space when there is no icon isIconSpaceReserved = true @@ -42,4 +44,9 @@ class VectorCheckboxPreference : CheckBoxPreference { (holder.findViewById(android.R.id.title) as? TextView)?.isSingleLine = false super.onBindViewHolder(holder) } + + override fun syncSummaryView(holder: PreferenceViewHolder) { + super.syncSummaryView(holder) + summaryTextColor?.let { (holder.findViewById(android.R.id.summary) as? TextView)?.setTextColor(it) } + } } diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/PushRuleDefinitions.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/PushRuleDefinitions.kt index fff6ffe933..9e19e2afb4 100644 --- a/vector/src/main/java/im/vector/app/features/settings/notifications/PushRuleDefinitions.kt +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/PushRuleDefinitions.kt @@ -42,8 +42,7 @@ fun getStandardAction(ruleId: String, index: NotificationIndex): StandardActions RuleIds.RULE_ID_POLL_START_ONE_TO_ONE, RuleIds.RULE_ID_POLL_START_ONE_TO_ONE_UNSTABLE, RuleIds.RULE_ID_POLL_END_ONE_TO_ONE, - RuleIds.RULE_ID_POLL_END_ONE_TO_ONE_UNSTABLE, - -> + RuleIds.RULE_ID_POLL_END_ONE_TO_ONE_UNSTABLE -> when (index) { NotificationIndex.OFF -> StandardActions.DontNotify NotificationIndex.SILENT -> StandardActions.Notify diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationFragment.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationFragment.kt index 0f991867fe..b7264f6cb3 100644 --- a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationFragment.kt +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationFragment.kt @@ -21,64 +21,79 @@ import android.view.View import androidx.preference.Preference import com.airbnb.mvrx.fragmentViewModel import com.airbnb.mvrx.withState +import im.vector.app.R import im.vector.app.core.preference.VectorCheckboxPreference import im.vector.app.features.settings.VectorSettingsBaseFragment +import im.vector.app.features.themes.ThemeUtils abstract class VectorSettingsPushRuleNotificationFragment : VectorSettingsBaseFragment() { - private val viewModel: VectorSettingsPushRuleNotificationViewModel by fragmentViewModel() + protected val viewModel: VectorSettingsPushRuleNotificationViewModel by fragmentViewModel() abstract val prefKeyToPushRuleId: Map override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) observeViewEvents() - viewModel.onEach(VectorSettingsPushRuleNotificationViewState::isLoading) { isLoading -> - if (isLoading) { - displayLoadingView() - } else { - hideLoadingView() - } - } + viewModel.onEach(VectorSettingsPushRuleNotificationViewState::allRules) { refreshPreferences() } + viewModel.onEach(VectorSettingsPushRuleNotificationViewState::isLoading) { updateLoadingView(it) } + viewModel.onEach(VectorSettingsPushRuleNotificationViewState::rulesOnError) { refreshErrors(it) } } private fun observeViewEvents() { viewModel.observeViewEvents { when (it) { - is VectorSettingsPushRuleNotificationViewEvent.Failure -> refreshDisplay() - is VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated -> updatePreference(it.ruleId, it.checked) + is VectorSettingsPushRuleNotificationViewEvent.Failure -> onFailure(it.ruleId) + is VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated -> { + updatePreference(it.ruleId, it.checked) + if (it.failure != null) { + onFailure(it.ruleId) + } + } } } } override fun bindPref() { - for (preferenceKey in prefKeyToPushRuleId.keys) { - val preference = findPreference(preferenceKey)!! - preference.isIconSpaceReserved = false - val ruleAndKind = prefKeyToPushRuleId[preferenceKey]?.let { viewModel.getPushRuleAndKind(it) } - if (ruleAndKind == null) { - // The rule is not defined, hide the preference - preference.isVisible = false - } else { - preference.isVisible = true - updatePreference(ruleAndKind.pushRule.ruleId, viewModel.isPushRuleChecked(ruleAndKind.pushRule.ruleId)) - preference.onPreferenceChangeListener = Preference.OnPreferenceChangeListener { _, newValue -> - viewModel.handle(VectorSettingsPushRuleNotificationViewAction.UpdatePushRule(ruleAndKind, newValue as Boolean)) + prefKeyToPushRuleId.forEach { (preferenceKey, ruleId) -> + findPreference(preferenceKey)?.apply { + isIconSpaceReserved = false + onPreferenceChangeListener = Preference.OnPreferenceChangeListener { _, newValue -> + viewModel.handle(VectorSettingsPushRuleNotificationViewAction.UpdatePushRule(ruleId, newValue as Boolean)) false } } } } - override fun invalidate() = withState(viewModel) { state -> - if (state.isLoading) { + private fun updateLoadingView(isLoading: Boolean) { + if (isLoading) { displayLoadingView() } else { hideLoadingView() } } + private fun refreshPreferences() { + prefKeyToPushRuleId.values.forEach { ruleId -> updatePreference(ruleId, viewModel.isPushRuleChecked(ruleId)) } + } + + private fun refreshErrors(rulesWithError: Set) { + if (withState(viewModel, VectorSettingsPushRuleNotificationViewState::isLoading)) return + prefKeyToPushRuleId.forEach { (preferenceKey, ruleId) -> + findPreference(preferenceKey)?.apply { + if (ruleId in rulesWithError) { + summaryTextColor = ThemeUtils.getColor(context, R.attr.colorError) + setSummary(R.string.settings_notification_error_on_update) + } else { + summaryTextColor = null + summary = null + } + } + } + } + protected fun refreshDisplay() { listView?.adapter?.notifyDataSetChanged() } @@ -86,6 +101,12 @@ abstract class VectorSettingsPushRuleNotificationFragment : private fun updatePreference(ruleId: String, checked: Boolean) { val preferenceKey = prefKeyToPushRuleId.entries.find { it.value == ruleId }?.key ?: return val preference = findPreference(preferenceKey) ?: return + val ruleIds = withState(viewModel) { state -> state.allRules.map { it.ruleId } } + preference.isVisible = ruleId in ruleIds preference.isChecked = checked } + + protected open fun onFailure(ruleId: String) { + refreshDisplay() + } } diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewAction.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewAction.kt index 61bf7c5b15..94b7666122 100644 --- a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewAction.kt +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewAction.kt @@ -17,8 +17,7 @@ package im.vector.app.features.settings.notifications import im.vector.app.core.platform.VectorViewModelAction -import org.matrix.android.sdk.api.session.pushrules.rest.PushRuleAndKind sealed interface VectorSettingsPushRuleNotificationViewAction : VectorViewModelAction { - data class UpdatePushRule(val pushRuleAndKind: PushRuleAndKind, val checked: Boolean) : VectorSettingsPushRuleNotificationViewAction + data class UpdatePushRule(val ruleId: String, val checked: Boolean) : VectorSettingsPushRuleNotificationViewAction } diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewEvent.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewEvent.kt index adfc17f827..42cb923a5e 100644 --- a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewEvent.kt +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewEvent.kt @@ -31,7 +31,8 @@ sealed interface VectorSettingsPushRuleNotificationViewEvent : VectorViewEvents /** * A failure has occurred. * + * @property ruleId the global rule id related to the failure. * @property throwable the related exception, if any. */ - data class Failure(val throwable: Throwable?) : VectorSettingsPushRuleNotificationViewEvent + data class Failure(val ruleId: String, val throwable: Throwable?) : VectorSettingsPushRuleNotificationViewEvent } diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewModel.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewModel.kt index 39969ec13e..31f7f9c531 100644 --- a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewModel.kt @@ -20,26 +20,31 @@ import com.airbnb.mvrx.MavericksViewModelFactory import dagger.assisted.Assisted import dagger.assisted.AssistedFactory import dagger.assisted.AssistedInject -import im.vector.app.core.di.ActiveSessionHolder import im.vector.app.core.di.MavericksAssistedViewModelFactory import im.vector.app.core.di.hiltMavericksViewModelFactory import im.vector.app.core.platform.VectorViewModel import im.vector.app.features.settings.notifications.VectorSettingsPushRuleNotificationViewEvent.Failure import im.vector.app.features.settings.notifications.VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated +import im.vector.app.features.settings.notifications.usecase.GetPushRulesOnInvalidStateUseCase import kotlinx.coroutines.launch import org.matrix.android.sdk.api.failure.Failure.ServerError import org.matrix.android.sdk.api.failure.MatrixError +import org.matrix.android.sdk.api.session.Session +import org.matrix.android.sdk.api.session.accountdata.UserAccountDataTypes import org.matrix.android.sdk.api.session.pushrules.Action import org.matrix.android.sdk.api.session.pushrules.RuleIds import org.matrix.android.sdk.api.session.pushrules.RuleKind import org.matrix.android.sdk.api.session.pushrules.rest.PushRuleAndKind +import org.matrix.android.sdk.flow.flow +import org.matrix.android.sdk.flow.unwrap private typealias ViewModel = VectorSettingsPushRuleNotificationViewModel private typealias ViewState = VectorSettingsPushRuleNotificationViewState class VectorSettingsPushRuleNotificationViewModel @AssistedInject constructor( @Assisted initialState: ViewState, - private val activeSessionHolder: ActiveSessionHolder, + private val session: Session, + private val getPushRulesOnInvalidStateUseCase: GetPushRulesOnInvalidStateUseCase, ) : VectorViewModel(initialState) { @@ -51,14 +56,28 @@ class VectorSettingsPushRuleNotificationViewModel @AssistedInject constructor( companion object : MavericksViewModelFactory by hiltMavericksViewModelFactory() + init { + session.flow() + .liveUserAccountData(UserAccountDataTypes.TYPE_PUSH_RULES) + .unwrap() + .setOnEach { + val allRules = session.pushRuleService().getPushRules().getAllRules() + val rulesOnError = getPushRulesOnInvalidStateUseCase.execute(session).map { it.ruleId }.toSet() + copy( + allRules = allRules, + rulesOnError = rulesOnError + ) + } + } + override fun handle(action: VectorSettingsPushRuleNotificationViewAction) { when (action) { - is VectorSettingsPushRuleNotificationViewAction.UpdatePushRule -> handleUpdatePushRule(action.pushRuleAndKind, action.checked) + is VectorSettingsPushRuleNotificationViewAction.UpdatePushRule -> handleUpdatePushRule(action.ruleId, action.checked) } } fun getPushRuleAndKind(ruleId: String): PushRuleAndKind? { - return activeSessionHolder.getSafeActiveSession()?.pushRuleService()?.getPushRules()?.findDefaultRule(ruleId) + return session.pushRuleService().getPushRules().findDefaultRule(ruleId) } fun isPushRuleChecked(ruleId: String): Boolean { @@ -66,13 +85,13 @@ class VectorSettingsPushRuleNotificationViewModel @AssistedInject constructor( return rulesGroup.mapNotNull { getPushRuleAndKind(it) }.any { it.pushRule.notificationIndex != NotificationIndex.OFF } } - private fun handleUpdatePushRule(pushRuleAndKind: PushRuleAndKind, checked: Boolean) { - val ruleId = pushRuleAndKind.pushRule.ruleId - val kind = pushRuleAndKind.kind + private fun handleUpdatePushRule(ruleId: String, checked: Boolean) { + val kind = getPushRuleAndKind(ruleId)?.kind ?: return val newIndex = if (checked) NotificationIndex.NOISY else NotificationIndex.OFF val standardAction = getStandardAction(ruleId, newIndex) ?: return val enabled = standardAction != StandardActions.Disabled val newActions = standardAction.actions + setState { copy(isLoading = true) } viewModelScope.launch { @@ -82,28 +101,37 @@ class VectorSettingsPushRuleNotificationViewModel @AssistedInject constructor( updatePushRule(kind, ruleId, enabled, newActions) } } - setState { copy(isLoading = false) } - val failure = results.firstNotNullOfOrNull { result -> + + val failures = results.mapNotNull { result -> // If the failure is a rule not found error, do not consider it result.exceptionOrNull()?.takeUnless { it is ServerError && it.error.code == MatrixError.M_NOT_FOUND } } - val newChecked = if (checked) { - // If any rule is checked, the global rule is checked - results.any { it.isSuccess } + val hasSuccess = results.any { it.isSuccess } + val hasFailures = failures.isNotEmpty() + + // Any rule has been checked or some rules have not been unchecked + val newChecked = (checked && hasSuccess) || (!checked && hasFailures) + if (hasSuccess) { + _viewEvents.post(PushRuleUpdated(ruleId, newChecked, failures.firstOrNull())) } else { - // If any rule has not been unchecked, the global rule remains checked - failure != null + _viewEvents.post(Failure(ruleId, failures.firstOrNull())) } - if (results.any { it.isSuccess }) { - _viewEvents.post(PushRuleUpdated(ruleId, newChecked, failure)) - } else { - _viewEvents.post(Failure(failure)) + + setState { + copy( + isLoading = false, + rulesOnError = when { + hasSuccess && hasFailures -> rulesOnError.plus(ruleId) // some failed + hasSuccess -> rulesOnError.minus(ruleId) // all succeed + else -> rulesOnError // all failed + } + ) } } } private suspend fun updatePushRule(kind: RuleKind, ruleId: String, enable: Boolean, newActions: List?) { - activeSessionHolder.getSafeActiveSession()?.pushRuleService()?.updatePushRuleActions( + session.pushRuleService().updatePushRuleActions( kind = kind, ruleId = ruleId, enable = enable, diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewState.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewState.kt index 477727aee6..091c183d49 100644 --- a/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewState.kt +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewState.kt @@ -17,7 +17,10 @@ package im.vector.app.features.settings.notifications import com.airbnb.mvrx.MavericksState +import org.matrix.android.sdk.api.session.pushrules.rest.PushRule data class VectorSettingsPushRuleNotificationViewState( val isLoading: Boolean = false, + val allRules: List = emptyList(), + val rulesOnError: Set = emptySet(), ) : MavericksState diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/usecase/GetPushRulesOnInvalidStateUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/usecase/GetPushRulesOnInvalidStateUseCase.kt new file mode 100644 index 0000000000..9b131f9a40 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/usecase/GetPushRulesOnInvalidStateUseCase.kt @@ -0,0 +1,44 @@ +/* + * Copyright (c) 2023 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.settings.notifications.usecase + +import im.vector.app.features.settings.notifications.getParentRule +import im.vector.app.features.settings.notifications.getSyncedRules +import org.matrix.android.sdk.api.extensions.orTrue +import org.matrix.android.sdk.api.session.Session +import org.matrix.android.sdk.api.session.pushrules.RuleIds +import org.matrix.android.sdk.api.session.pushrules.getActions +import org.matrix.android.sdk.api.session.pushrules.rest.PushRule +import javax.inject.Inject + +class GetPushRulesOnInvalidStateUseCase @Inject constructor() { + + fun execute(session: Session): List { + val allRules = session.pushRuleService().getPushRules().getAllRules() + return allRules.filter { it.isOnInvalidState(allRules) } + } + + private fun PushRule.isOnInvalidState(allRules: List): Boolean { + val parent = RuleIds.getParentRule(ruleId)?.let { parentId -> allRules.find { it.ruleId == parentId } } + val children = RuleIds.getSyncedRules(ruleId).mapNotNull { childId -> allRules.find { it.ruleId == childId } } + val isAlignedWithParent = parent?.let { isAlignedWithParentRule(it) }.orTrue() + return !isAlignedWithParent || !isAlignedWithChildrenRules(children) + } + + private fun PushRule.isAlignedWithParentRule(parent: PushRule) = this.getActions() == parent.getActions() && this.enabled == parent.enabled + private fun PushRule.isAlignedWithChildrenRules(children: List) = children.all { it.isAlignedWithParentRule(this) } +} diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/usecase/UpdatePushRulesIfNeededUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/usecase/UpdatePushRulesIfNeededUseCase.kt index 6998065f01..12fc3d480d 100644 --- a/vector/src/main/java/im/vector/app/features/settings/notifications/usecase/UpdatePushRulesIfNeededUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/usecase/UpdatePushRulesIfNeededUseCase.kt @@ -24,18 +24,18 @@ import org.matrix.android.sdk.api.session.pushrules.rest.PushRule import org.matrix.android.sdk.api.session.pushrules.rest.PushRuleAndKind import javax.inject.Inject -class UpdatePushRulesIfNeededUseCase @Inject constructor() { +class UpdatePushRulesIfNeededUseCase @Inject constructor( + private val getPushRulesOnInvalidStateUseCase: GetPushRulesOnInvalidStateUseCase, +) { suspend fun execute(session: Session) { + val rulesOnError = getPushRulesOnInvalidStateUseCase.execute(session).takeUnless { it.isEmpty() } ?: return + val ruleSet = session.pushRuleService().getPushRules() - val pushRules = ruleSet.getAllRules() - val rulesToUpdate = pushRules.mapNotNull { rule -> - val parent = RuleIds.getParentRule(rule.ruleId)?.let { ruleId -> ruleSet.findDefaultRule(ruleId) } - if (parent != null && (rule.enabled != parent.pushRule.enabled || rule.actions != parent.pushRule.actions)) { - PushRuleWithParent(rule, parent) - } else { - null - } + val rulesToUpdate = rulesOnError.mapNotNull { rule -> + RuleIds.getParentRule(rule.ruleId) + ?.let { ruleId -> ruleSet.findDefaultRule(ruleId) } + ?.let { PushRuleWithParent(rule, it) } } rulesToUpdate.forEach { diff --git a/vector/src/main/res/xml/vector_settings_notification_default.xml b/vector/src/main/res/xml/vector_settings_notification_default.xml index fb565d2230..9319ef2b39 100644 --- a/vector/src/main/res/xml/vector_settings_notification_default.xml +++ b/vector/src/main/res/xml/vector_settings_notification_default.xml @@ -25,4 +25,4 @@ android:title="@string/settings_encrypted_group_messages" /> - \ No newline at end of file + diff --git a/vector/src/test/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewModelTest.kt b/vector/src/test/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewModelTest.kt index 04a22bc21f..8a45f922e7 100644 --- a/vector/src/test/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewModelTest.kt +++ b/vector/src/test/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewModelTest.kt @@ -17,7 +17,9 @@ package im.vector.app.features.settings.notifications import com.airbnb.mvrx.test.MavericksTestRule -import im.vector.app.test.fakes.FakeActiveSessionHolder +import im.vector.app.features.settings.notifications.usecase.GetPushRulesOnInvalidStateUseCase +import im.vector.app.test.fakes.FakeSession +import im.vector.app.test.fixtures.PushRuleFixture import im.vector.app.test.test import im.vector.app.test.testDispatcher import io.mockk.coVerifyOrder @@ -32,25 +34,27 @@ import org.junit.Before import org.junit.Rule import org.junit.Test import org.matrix.android.sdk.api.session.pushrules.RuleIds -import org.matrix.android.sdk.api.session.pushrules.rest.PushRuleAndKind internal class VectorSettingsPushRuleNotificationViewModelTest { @get:Rule val mavericksTestRule = MavericksTestRule(testDispatcher = testDispatcher) - private val fakeActiveSessionHolder = FakeActiveSessionHolder() - private val fakePushRuleService = fakeActiveSessionHolder.fakeSession.fakePushRuleService + private val fakeSession = FakeSession() + private val fakePushRuleService = fakeSession.fakePushRuleService + private val fakeGetPushRulesOnInvalidStateUseCase = mockk() private val initialState = VectorSettingsPushRuleNotificationViewState() private fun createViewModel() = VectorSettingsPushRuleNotificationViewModel( initialState = initialState, - activeSessionHolder = fakeActiveSessionHolder.instance, + session = fakeSession, + fakeGetPushRulesOnInvalidStateUseCase, ) @Before fun setup() { mockkStatic("im.vector.app.features.settings.notifications.NotificationIndexKt") + every { fakeGetPushRulesOnInvalidStateUseCase.execute(any()) } returns emptyList() } @After @@ -65,12 +69,14 @@ internal class VectorSettingsPushRuleNotificationViewModelTest { val firstRuleId = RuleIds.RULE_ID_ONE_TO_ONE_ROOM val secondRuleId = RuleIds.RULE_ID_ALL_OTHER_MESSAGES_ROOMS + givenARuleId(firstRuleId) + givenARuleId(secondRuleId) fakePushRuleService.givenUpdatePushRuleActionsSucceed() // When val viewModelTest = viewModel.test() - viewModel.handle(VectorSettingsPushRuleNotificationViewAction.UpdatePushRule(givenARuleId(firstRuleId), true)) - viewModel.handle(VectorSettingsPushRuleNotificationViewAction.UpdatePushRule(givenARuleId(secondRuleId), false)) + viewModel.handle(VectorSettingsPushRuleNotificationViewAction.UpdatePushRule(firstRuleId, true)) + viewModel.handle(VectorSettingsPushRuleNotificationViewAction.UpdatePushRule(secondRuleId, false)) // Then coVerifyOrder { @@ -98,8 +104,8 @@ internal class VectorSettingsPushRuleNotificationViewModelTest { { copy(isLoading = false) }, ) .assertEvents( - VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated(RuleIds.RULE_ID_ONE_TO_ONE_ROOM, true), - VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated(RuleIds.RULE_ID_ALL_OTHER_MESSAGES_ROOMS, false), + VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated(firstRuleId, true), + VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated(secondRuleId, false), ) .finish() } @@ -111,10 +117,12 @@ internal class VectorSettingsPushRuleNotificationViewModelTest { val failure = mockk() val firstRuleId = RuleIds.RULE_ID_ONE_TO_ONE_ROOM + givenARuleId(firstRuleId) fakePushRuleService.givenUpdatePushRuleActionsSucceed() fakePushRuleService.givenUpdatePushRuleActionsFail(RuleIds.RULE_ID_POLL_START_ONE_TO_ONE_UNSTABLE, failure) val secondRuleId = RuleIds.RULE_ID_ALL_OTHER_MESSAGES_ROOMS + givenARuleId(secondRuleId) fakePushRuleService.givenUpdatePushRuleActionsFail(secondRuleId, failure) fakePushRuleService.givenUpdatePushRuleActionsFail(RuleIds.RULE_ID_POLL_START, failure) fakePushRuleService.givenUpdatePushRuleActionsFail(RuleIds.RULE_ID_POLL_START_UNSTABLE, failure) @@ -124,9 +132,9 @@ internal class VectorSettingsPushRuleNotificationViewModelTest { // When val viewModelTest = viewModel.test() // One rule failed to update - viewModel.handle(VectorSettingsPushRuleNotificationViewAction.UpdatePushRule(givenARuleId(firstRuleId), true)) + viewModel.handle(VectorSettingsPushRuleNotificationViewAction.UpdatePushRule(firstRuleId, true)) // All the rules failed to update - viewModel.handle(VectorSettingsPushRuleNotificationViewAction.UpdatePushRule(givenARuleId(secondRuleId), true)) + viewModel.handle(VectorSettingsPushRuleNotificationViewAction.UpdatePushRule(secondRuleId, true)) // Then coVerifyOrder { @@ -149,13 +157,13 @@ internal class VectorSettingsPushRuleNotificationViewModelTest { .assertStatesChanges( initialState, { copy(isLoading = true) }, - { copy(isLoading = false) }, + { copy(isLoading = false, rulesOnError = setOf(firstRuleId)) }, { copy(isLoading = true) }, { copy(isLoading = false) }, ) .assertEvents( - VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated(RuleIds.RULE_ID_ONE_TO_ONE_ROOM, true, failure), - VectorSettingsPushRuleNotificationViewEvent.Failure(failure), + VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated(firstRuleId, true, failure), + VectorSettingsPushRuleNotificationViewEvent.Failure(secondRuleId, failure), ) .finish() } @@ -167,10 +175,12 @@ internal class VectorSettingsPushRuleNotificationViewModelTest { val failure = mockk() val firstRuleId = RuleIds.RULE_ID_ONE_TO_ONE_ROOM + givenARuleId(firstRuleId) fakePushRuleService.givenUpdatePushRuleActionsSucceed() fakePushRuleService.givenUpdatePushRuleActionsFail(RuleIds.RULE_ID_POLL_START_ONE_TO_ONE_UNSTABLE, failure) val secondRuleId = RuleIds.RULE_ID_ALL_OTHER_MESSAGES_ROOMS + givenARuleId(secondRuleId) fakePushRuleService.givenUpdatePushRuleActionsFail(secondRuleId, failure) fakePushRuleService.givenUpdatePushRuleActionsFail(RuleIds.RULE_ID_POLL_START, failure) fakePushRuleService.givenUpdatePushRuleActionsFail(RuleIds.RULE_ID_POLL_START_UNSTABLE, failure) @@ -180,9 +190,9 @@ internal class VectorSettingsPushRuleNotificationViewModelTest { // When val viewModelTest = viewModel.test() // One rule failed to update - viewModel.handle(VectorSettingsPushRuleNotificationViewAction.UpdatePushRule(givenARuleId(firstRuleId), false)) + viewModel.handle(VectorSettingsPushRuleNotificationViewAction.UpdatePushRule(firstRuleId, false)) // All the rules failed to update - viewModel.handle(VectorSettingsPushRuleNotificationViewAction.UpdatePushRule(givenARuleId(secondRuleId), false)) + viewModel.handle(VectorSettingsPushRuleNotificationViewAction.UpdatePushRule(secondRuleId, false)) // Then coVerifyOrder { @@ -205,14 +215,14 @@ internal class VectorSettingsPushRuleNotificationViewModelTest { .assertStatesChanges( initialState, { copy(isLoading = true) }, - { copy(isLoading = false) }, + { copy(isLoading = false, rulesOnError = setOf(firstRuleId)) }, { copy(isLoading = true) }, { copy(isLoading = false) }, ) .assertEvents( // The global rule remains checked if all the rules are not unchecked - VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated(RuleIds.RULE_ID_ONE_TO_ONE_ROOM, true, failure), - VectorSettingsPushRuleNotificationViewEvent.Failure(failure), + VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated(firstRuleId, true, failure), + VectorSettingsPushRuleNotificationViewEvent.Failure(secondRuleId, failure), ) .finish() } @@ -244,15 +254,11 @@ internal class VectorSettingsPushRuleNotificationViewModelTest { secondResult shouldBe false } - private fun givenARuleId(ruleId: String, notificationIndex: NotificationIndex = NotificationIndex.NOISY): PushRuleAndKind { - val ruleAndKind = mockk { - every { pushRule.ruleId } returns ruleId - every { pushRule.notificationIndex } returns notificationIndex - every { kind } returns mockk() - } + private fun givenARuleId(ruleId: String, notificationIndex: NotificationIndex = NotificationIndex.NOISY) { + val pushRule = PushRuleFixture.aPushRule(ruleId) + every { pushRule.notificationIndex } returns notificationIndex + val ruleAndKind = PushRuleFixture.aPushRuleAndKind(pushRule) every { fakePushRuleService.getPushRules().findDefaultRule(ruleId) } returns ruleAndKind - - return ruleAndKind } } diff --git a/vector/src/test/java/im/vector/app/features/settings/notifications/usecase/GetPushRulesOnInvalidStateUseCaseTest.kt b/vector/src/test/java/im/vector/app/features/settings/notifications/usecase/GetPushRulesOnInvalidStateUseCaseTest.kt new file mode 100644 index 0000000000..a434ac93d3 --- /dev/null +++ b/vector/src/test/java/im/vector/app/features/settings/notifications/usecase/GetPushRulesOnInvalidStateUseCaseTest.kt @@ -0,0 +1,100 @@ +/* + * Copyright (c) 2023 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.settings.notifications.usecase + +import im.vector.app.test.fakes.FakeSession +import im.vector.app.test.fixtures.PushRuleFixture +import io.mockk.every +import io.mockk.mockkStatic +import io.mockk.unmockkAll +import org.amshove.kluent.shouldBeEqualTo +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.matrix.android.sdk.api.session.pushrules.Action +import org.matrix.android.sdk.api.session.pushrules.RuleIds +import org.matrix.android.sdk.api.session.pushrules.rest.PushRule + +internal class GetPushRulesOnInvalidStateUseCaseTest { + + private val fakeSession = FakeSession() + private val getPushRulesOnInvalidStateUseCase = GetPushRulesOnInvalidStateUseCase() + + @Before + fun setup() { + mockkStatic("org.matrix.android.sdk.api.session.pushrules.ActionKt") + } + + @After + fun tearDown() { + unmockkAll() + } + + @Test + fun `given a list of push rules with children not matching their parent when execute then returns the list of not matching rules`() { + // Given + val firstActions = listOf(Action.Notify) + val secondActions = listOf(Action.DoNotNotify) + givenARuleList( + listOf( + // first set of related rules + givenARuleId(RuleIds.RULE_ID_ONE_TO_ONE_ROOM, true, firstActions), + givenARuleId(RuleIds.RULE_ID_POLL_START_ONE_TO_ONE, true, listOf(Action.DoNotNotify)), // diff + givenARuleId(RuleIds.RULE_ID_POLL_START_ONE_TO_ONE_UNSTABLE, true, emptyList()), // diff + givenARuleId(RuleIds.RULE_ID_POLL_END_ONE_TO_ONE, false, listOf(Action.Notify)), // diff + givenARuleId(RuleIds.RULE_ID_POLL_END_ONE_TO_ONE_UNSTABLE, true, listOf(Action.Notify)), + // second set of related rules + givenARuleId(RuleIds.RULE_ID_ALL_OTHER_MESSAGES_ROOMS, false, secondActions), + givenARuleId(RuleIds.RULE_ID_POLL_START, true, listOf(Action.Notify)), // diff + givenARuleId(RuleIds.RULE_ID_POLL_START_UNSTABLE, false, listOf(Action.DoNotNotify)), + givenARuleId(RuleIds.RULE_ID_POLL_END, false, listOf(Action.Notify)), // diff + givenARuleId(RuleIds.RULE_ID_POLL_END_UNSTABLE, true, listOf()), // diff + // Another rule + givenARuleId(RuleIds.RULE_ID_CONTAIN_USER_NAME, true, listOf(Action.Notify)), + ) + ) + + // When + val result = getPushRulesOnInvalidStateUseCase.execute(fakeSession).map { it.ruleId } + + // Then + result shouldBeEqualTo listOf( + RuleIds.RULE_ID_ONE_TO_ONE_ROOM, // parent rule + RuleIds.RULE_ID_POLL_START_ONE_TO_ONE, + RuleIds.RULE_ID_POLL_START_ONE_TO_ONE_UNSTABLE, + RuleIds.RULE_ID_POLL_END_ONE_TO_ONE, + RuleIds.RULE_ID_ALL_OTHER_MESSAGES_ROOMS, // parent rule + RuleIds.RULE_ID_POLL_START, + RuleIds.RULE_ID_POLL_END, + RuleIds.RULE_ID_POLL_END_UNSTABLE, + ) + } + + private fun givenARuleList(rules: List) { + every { fakeSession.fakePushRuleService.getPushRules().getAllRules() } returns rules + } + + private fun givenARuleId(ruleId: String, enabled: Boolean, actions: List): PushRule { + val ruleAndKind = PushRuleFixture.aPushRuleAndKind( + PushRuleFixture.aPushRule(ruleId, enabled, actions), + ) + + every { fakeSession.fakePushRuleService.getPushRules().findDefaultRule(ruleId) } returns ruleAndKind + + return ruleAndKind.pushRule + } +} diff --git a/vector/src/test/java/im/vector/app/features/settings/notifications/usecase/UpdatePushRulesIfNeededUseCaseTest.kt b/vector/src/test/java/im/vector/app/features/settings/notifications/usecase/UpdatePushRulesIfNeededUseCaseTest.kt index 7dcbe68d4b..1f76a7f9a5 100644 --- a/vector/src/test/java/im/vector/app/features/settings/notifications/usecase/UpdatePushRulesIfNeededUseCaseTest.kt +++ b/vector/src/test/java/im/vector/app/features/settings/notifications/usecase/UpdatePushRulesIfNeededUseCaseTest.kt @@ -17,6 +17,7 @@ package im.vector.app.features.settings.notifications.usecase import im.vector.app.test.fakes.FakeSession +import im.vector.app.test.fixtures.PushRuleFixture import io.mockk.coVerifySequence import io.mockk.every import io.mockk.mockk @@ -28,14 +29,13 @@ import org.junit.Before import org.junit.Test import org.matrix.android.sdk.api.session.pushrules.Action import org.matrix.android.sdk.api.session.pushrules.RuleIds -import org.matrix.android.sdk.api.session.pushrules.getActions import org.matrix.android.sdk.api.session.pushrules.rest.PushRule -import org.matrix.android.sdk.api.session.pushrules.rest.PushRuleAndKind internal class UpdatePushRulesIfNeededUseCaseTest { private val fakeSession = FakeSession() - private val updatePushRulesIfNeededUseCase = UpdatePushRulesIfNeededUseCase() + private val fakeGetPushRulesOnInvalidStateUseCase = mockk() + private val updatePushRulesIfNeededUseCase = UpdatePushRulesIfNeededUseCase(fakeGetPushRulesOnInvalidStateUseCase) @Before fun setup() { @@ -50,25 +50,26 @@ internal class UpdatePushRulesIfNeededUseCaseTest { @Test fun test() = runTest { // Given - val firstActions = listOf(Action.Notify) - val secondActions = listOf(Action.DoNotNotify) - val rules = listOf( + val firstParentEnabled = true + val firstParentActions = listOf(Action.Notify) + val firstParent = givenARuleId(RuleIds.RULE_ID_ONE_TO_ONE_ROOM, firstParentEnabled, firstParentActions) + val secondParentEnabled = false + val secondParentActions = listOf(Action.DoNotNotify) + val secondParent = givenARuleId(RuleIds.RULE_ID_ALL_OTHER_MESSAGES_ROOMS, secondParentEnabled, secondParentActions) + val rulesOnError = listOf( // first set of related rules - givenARuleId(RuleIds.RULE_ID_ONE_TO_ONE_ROOM, true, firstActions), + firstParent, givenARuleId(RuleIds.RULE_ID_POLL_START_ONE_TO_ONE, true, listOf(Action.DoNotNotify)), // diff givenARuleId(RuleIds.RULE_ID_POLL_START_ONE_TO_ONE_UNSTABLE, true, emptyList()), // diff givenARuleId(RuleIds.RULE_ID_POLL_END_ONE_TO_ONE, false, listOf(Action.Notify)), // diff - givenARuleId(RuleIds.RULE_ID_POLL_END_ONE_TO_ONE_UNSTABLE, true, listOf(Action.Notify)), // second set of related rules - givenARuleId(RuleIds.RULE_ID_ALL_OTHER_MESSAGES_ROOMS, false, secondActions), + secondParent, givenARuleId(RuleIds.RULE_ID_POLL_START, true, listOf(Action.Notify)), // diff - givenARuleId(RuleIds.RULE_ID_POLL_START_UNSTABLE, false, listOf(Action.DoNotNotify)), givenARuleId(RuleIds.RULE_ID_POLL_END, false, listOf(Action.Notify)), // diff givenARuleId(RuleIds.RULE_ID_POLL_END_UNSTABLE, true, listOf()), // diff - // Another rule - givenARuleId(RuleIds.RULE_ID_CONTAIN_USER_NAME, true, listOf(Action.Notify)), ) - every { fakeSession.fakePushRuleService.getPushRules().getAllRules() } returns rules + every { fakeGetPushRulesOnInvalidStateUseCase.execute(fakeSession) } returns rulesOnError + every { fakeSession.fakePushRuleService.getPushRules().getAllRules() } returns rulesOnError // When updatePushRulesIfNeededUseCase.execute(fakeSession) @@ -77,30 +78,23 @@ internal class UpdatePushRulesIfNeededUseCaseTest { coVerifySequence { fakeSession.fakePushRuleService.getPushRules() // first set - fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_START_ONE_TO_ONE, true, firstActions) - fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_START_ONE_TO_ONE_UNSTABLE, true, firstActions) - fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_END_ONE_TO_ONE, true, firstActions) + fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_START_ONE_TO_ONE, firstParentEnabled, firstParentActions) + fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_START_ONE_TO_ONE_UNSTABLE, firstParentEnabled, firstParentActions) + fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_END_ONE_TO_ONE, firstParentEnabled, firstParentActions) // second set - fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_START, false, secondActions) - fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_END, false, secondActions) - fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_END_UNSTABLE, false, secondActions) + fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_START, secondParentEnabled, secondParentActions) + fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_END, secondParentEnabled, secondParentActions) + fakeSession.fakePushRuleService.updatePushRuleActions(any(), RuleIds.RULE_ID_POLL_END_UNSTABLE, secondParentEnabled, secondParentActions) } } private fun givenARuleId(ruleId: String, enabled: Boolean, actions: List): PushRule { - val pushRule = mockk { - every { this@mockk.ruleId } returns ruleId - every { this@mockk.enabled } returns enabled - every { this@mockk.actions } returns actions - every { getActions() } returns actions - } - val ruleAndKind = mockk { - every { this@mockk.pushRule } returns pushRule - every { kind } returns mockk() - } + val ruleAndKind = PushRuleFixture.aPushRuleAndKind( + pushRule = PushRuleFixture.aPushRule(ruleId, enabled, actions), + ) every { fakeSession.fakePushRuleService.getPushRules().findDefaultRule(ruleId) } returns ruleAndKind - return pushRule + return ruleAndKind.pushRule } } diff --git a/vector/src/test/java/im/vector/app/test/fixtures/PushRuleFixture.kt b/vector/src/test/java/im/vector/app/test/fixtures/PushRuleFixture.kt new file mode 100644 index 0000000000..2170d60228 --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fixtures/PushRuleFixture.kt @@ -0,0 +1,49 @@ +/* + * Copyright (c) 2023 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.test.fixtures + +import io.mockk.every +import io.mockk.mockk +import org.matrix.android.sdk.api.session.pushrules.Action +import org.matrix.android.sdk.api.session.pushrules.RuleSetKey +import org.matrix.android.sdk.api.session.pushrules.getActions +import org.matrix.android.sdk.api.session.pushrules.rest.PushRule +import org.matrix.android.sdk.api.session.pushrules.rest.PushRuleAndKind + +object PushRuleFixture { + + private const val A_RULE_ID = "a-rule-id" + + // Needs: mockkStatic("org.matrix.android.sdk.api.session.pushrules.ActionKt") + fun aPushRule( + ruleId: String = A_RULE_ID, + enabled: Boolean = true, + actions: List = listOf(Action.Notify), + ): PushRule = mockk { + every { this@mockk.ruleId } returns ruleId + every { this@mockk.enabled } returns enabled + every { getActions() } returns actions + } + + fun aPushRuleAndKind( + pushRule: PushRule = aPushRule(), + kind: RuleSetKey = RuleSetKey.UNDERRIDE, + ): PushRuleAndKind = mockk { + every { this@mockk.pushRule } returns pushRule + every { this@mockk.kind } returns kind + } +}